Fix: preserved thread's system error over NativeCallback invocations

Though JavaScript function wrapped as a NativeCallback allows both inspecting and altering the current thread's system error, it wasn't possible to leave the system error as-is. Unless replaced by a value unequal to the original, the system error was being forced to 0 upon return from the NativeCallback. Fixed and tests added.

bindings/gumjs/gumquickcore.c

@@ -4447,6 +4447,9 @@ gum_quick_native_callback_invoke (ffi_cif * cif,
   int argc, i;
   JSValue * argv;
   JSValue result;
+  gint saved_system_error;
+
+  saved_system_error = gum_thread_get_system_error();
 
 #if defined (HAVE_I386) && defined (_MSC_VER)
   return_address = GPOINTER_TO_SIZE (_ReturnAddress ());
@@ -4556,6 +4559,8 @@ gum_quick_native_callback_invoke (ffi_cif * cif,
   JS_FreeValue (ctx, self->wrapper);
 
   _gum_quick_scope_leave (&scope);
+
+  gum_thread_set_system_error(saved_system_error);
 }
 
 GUMJS_DEFINE_FINALIZER (gumjs_callback_context_finalize)

bindings/gumjs/gumv8core.cpp

@@ -3434,6 +3434,7 @@ gum_v8_native_callback_invoke (ffi_cif * cif,
                                void ** args,
                                void * user_data)
 {
+  GumV8SystemErrorPreservationScope error_scope;
   guintptr return_address = 0;
   guintptr stack_pointer = 0;
   guintptr frame_pointer = 0;

bindings/gumjs/gumv8core.h

@@ -183,4 +183,21 @@ G_GNUC_INTERNAL void _gum_v8_core_post (GumV8Core * self, const gchar * message,
 G_GNUC_INTERNAL void _gum_v8_core_push_job (GumV8Core * self,
     GumScriptJobFunc job_func, gpointer data, GDestroyNotify data_destroy);
 
+class GumV8SystemErrorPreservationScope
+{
+public:
+    GumV8SystemErrorPreservationScope()
+        : saved_error(gum_thread_get_system_error())
+    {
+    }
+
+    ~GumV8SystemErrorPreservationScope()
+    {
+        gum_thread_set_system_error(saved_error);
+    }
+
+private:
+    gint saved_error;
+};
+
 #endif

bindings/gumjs/gumv8stalker.cpp

@@ -70,23 +70,6 @@ struct GumV8CallProbe
   GumV8Stalker * module;
 };
 
-class GumV8SystemErrorPreservationScope
-{
-public:
-  GumV8SystemErrorPreservationScope ()
-    : saved_error (gum_thread_get_system_error ())
-  {
-  }
-
-  ~GumV8SystemErrorPreservationScope ()
-  {
-    gum_thread_set_system_error (saved_error);
-  }
-
-private:
-  gint saved_error;
-};
-
 static gboolean gum_v8_stalker_on_flush_timer_tick (GumV8Stalker * self);
 
 GUMJS_DECLARE_GETTER (gumjs_stalker_get_trust_threshold)

tests/gumjs/script.c

@@ -67,6 +67,8 @@ TESTLIST_BEGIN (script)
     TESTENTRY (system_error_can_be_read_from_replacement_function)
     TESTENTRY (system_error_can_be_replaced_from_interceptor_listener)
     TESTENTRY (system_error_can_be_replaced_from_replacement_function)
+    TESTENTRY (system_error_unaffected_by_replacement_function_if_assigned_original_value)
+    TESTENTRY (system_error_unaffected_by_replacement_function_if_untouched)
     TESTENTRY (invocations_are_bound_on_tls_object)
     TESTENTRY (invocations_provide_thread_id)
     TESTENTRY (invocations_provide_call_depth)
@@ -368,6 +370,7 @@ TESTLIST_BEGIN (script)
     TESTENTRY (cmodule_should_support_global_callbacks)
     TESTENTRY (cmodule_should_provide_access_to_cpu_registers)
     TESTENTRY (cmodule_should_provide_access_to_system_error)
+    TESTENTRY (system_error_unaffected_by_native_callback_from_cmodule)
 #else
     TESTENTRY (cmodule_constructor_should_throw_not_available)
 #endif
@@ -6988,6 +6991,51 @@ TESTCASE (system_error_can_be_replaced_from_replacement_function)
 #endif
 }
 
+TESTCASE(system_error_unaffected_by_replacement_function_if_assigned_original_value)
+{
+#ifdef HAVE_WINDOWS
+    COMPILE_AND_LOAD_SCRIPT(
+        "Interceptor.replace(" GUM_PTR_CONST ","
+        "    new NativeCallback(function (arg) {"
+        "  this.lastError = 1337;"
+        "  return 0;"
+        "}, 'int', ['int']));", target_function_int);
+
+    SetLastError(1337);
+    target_function_int(7);
+    g_assert_cmpint(GetLastError(), == , 1337);
+#else
+    COMPILE_AND_LOAD_SCRIPT(
+        "Interceptor.replace(" GUM_PTR_CONST ","
+        "    new NativeCallback(function (arg) {"
+        "  this.errno = 1337;"
+        "  return 0;"
+        "}, 'int', ['int']));", target_function_int);
+
+    errno = 1337;
+    target_function_int(7);
+    g_assert_cmpint(errno, == , 1337);
+#endif
+}
+
+TESTCASE(system_error_unaffected_by_replacement_function_if_untouched)
+{
+    COMPILE_AND_LOAD_SCRIPT(
+        "Interceptor.replace(" GUM_PTR_CONST ","
+        "    new NativeCallback(function (arg) {"
+        "  return 0;"
+        "}, 'int', ['int']));", target_function_int);
+#ifdef HAVE_WINDOWS
+    SetLastError(1337);
+    target_function_int(7);
+    g_assert_cmpint(GetLastError(), == , 1337);
+#else
+    errno = 1337;
+    target_function_int(7);
+    g_assert_cmpint(errno, == , 1337);
+#endif
+}
+
 TESTCASE (invocations_are_bound_on_tls_object)
 {
   COMPILE_AND_LOAD_SCRIPT (
@@ -10195,6 +10243,33 @@ TESTCASE (cmodule_should_provide_access_to_system_error)
   g_assert_cmpint (gum_thread_get_system_error (), ==, 2);
 }
 
+TESTCASE(system_error_unaffected_by_native_callback_from_cmodule)
+{
+    COMPILE_AND_LOAD_SCRIPT(
+        "const cm = new CModule(`\\n"
+        "    #include <gum/guminterceptor.h>\\n"
+        "    extern void nativeCallback1();\\n"
+        "\\n"
+        "    void replacement(GumInvocationContext * ic) {\\n"
+        "        nativeCallback1(); }\\n"
+        "`,\n"
+        "{\n"
+        "'nativeCallback1' :\n"
+        "   new NativeCallback(() => {}, 'void', [])\n"
+        "});\n"
+        "Interceptor.replace(" GUM_PTR_CONST ",\n"
+        "    cm.replacement);", target_function_int);
+#ifdef HAVE_WINDOWS
+    SetLastError(1337);
+    target_function_int(7);
+    g_assert_cmpint(GetLastError(), == , 1337);
+#else
+    errno = 1337;
+    target_function_int(7);
+    g_assert_cmpint(errno, == , 1337);
+#endif
+}
+
 #else /* !HAVE_TINYCC */
 
 TESTCASE (cmodule_constructor_should_throw_not_available)