i#95: Detach on Linux

core/lib/globals_shared.h

@@ -933,9 +933,9 @@ enum {
 #else
     NUDGE_NUDGER_FREE_STACK = 0x02, /* nudger will free the nudge thread's stack so the
                                      * nudge thread itself shouldn't */
-    NUDGE_FREE_ARG = 0x04,          /* nudge arg is in a separate allocation and should
-                                     * be freed by the nudge thread */
 #endif
+    NUDGE_FREE_ARG = 0x04, /* nudge arg is in a separate allocation and should
+                            * be freed by the nudge thread */
 };
 
 typedef struct {

core/nudge.c

@@ -434,29 +434,30 @@ handle_nudge(dcontext_t *dcontext, nudge_arg_t *arg)
             SYSLOG_INTERNAL_WARNING("nudge reset ignored since resets are disabled");
         }
     }
-#ifdef WINDOWS
+#if defined(WINDOWS) || defined(LINUX)
     /* The detach handler is last since in the common case it doesn't return. */
     if (TEST(NUDGE_GENERIC(detach), nudge_action_mask)) {
+#    ifdef WINDOWS
         dcontext->free_app_stack = false;
         nudge_action_mask &= ~NUDGE_GENERIC(detach);
         detach_helper(DETACH_NORMAL_TYPE);
-    }
-#endif
-#ifdef LINUX
-    /* The detach handler is last since in the common case it doesn't return. */
-    if (TEST(NUDGE_GENERIC(detach), nudge_action_mask)) {
+#    else
         nudge_action_mask &= ~NUDGE_GENERIC(detach);
+        /* This is not using stack_alloc() because we can't have this being cleaned up
+         * via normal cleanup paths. */
         heap_error_code_t error_code_reserve, error_code_commit;
         void *d_r_detachstack =
             os_heap_reserve(NULL, DYNAMORIO_STACK_SIZE, &error_code_reserve, false);
+        /* XXX: This memory is not freed. */
         if (!os_heap_commit(d_r_detachstack, DYNAMORIO_STACK_SIZE,
                             MEMPROT_READ | MEMPROT_WRITE, &error_code_commit)) {
             ASSERT_NOT_REACHED();
         }
         call_switch_stack(dcontext,
                           (byte *)((ptr_uint_t)d_r_detachstack + DYNAMORIO_STACK_SIZE),
-                          (void (*)(void *))detach_externally_on_linux, NULL, true);
+                          (void (*)(void *))detach_externally_on_new_stack, NULL, true);
         ASSERT_NOT_REACHED();
+#    endif
     }
 #endif
 }

core/synch.c

@@ -1965,6 +1965,77 @@ send_all_other_threads_native(void)
     return;
 }
 
+void
+detach_set_mcontext_helper(thread_record_t *thread)
+{
+    priv_mcontext_t mc;
+    LOG(GLOBAL, LOG_ALL, 2, "Detach: translating " TIDFMT "\n", thread);
+    DEBUG_DECLARE(bool ok =)
+    thread_get_mcontext(thread, &mc);
+    ASSERT(ok);
+    /* For a thread at a syscall, we use SA_RESTART for our suspend signal,
+     * so the kernel will adjust the restart point back to the syscall for us
+     * where expected.  This is an artifical signal we're introducing, so an
+     * app that assumes no signals and assumes its non-auto-restart syscalls
+     * don't need loops could be broken.
+     */
+    LOG(GLOBAL, LOG_ALL, 3,
+        /* Having the code bytes can help diagnose post-detach where the code
+         * cache is gone.
+         */
+        "Detach: pre-xl8 pc=%p (%02x %02x %02x %02x %02x), xsp=%p "
+        "for thread " TIDFMT "\n",
+        mc.pc, *mc.pc, *(mc.pc + 1), *(mc.pc + 2), *(mc.pc + 3), *(mc.pc + 4), mc.xsp,
+        thread->id);
+    DEBUG_DECLARE(ok =)
+    translate_mcontext(thread, &mc, true /*restore mem*/, NULL /*f*/);
+    ASSERT(ok);
+    if (!thread->under_dynamo_control) {
+        LOG(GLOBAL, LOG_ALL, 1, "Detach : thread " TIDFMT " already running natively\n",
+            thread->id);
+        /* we do need to restore the app ret addr, for native_exec */
+        if (!DYNAMO_OPTION(thin_client) && DYNAMO_OPTION(native_exec) &&
+            !vmvector_empty(native_exec_areas)) {
+            put_back_native_retaddrs(thread->dcontext);
+        }
+    }
+    detach_finalize_translation(thread, &mc);
+    LOG(GLOBAL, LOG_ALL, 1, "Detach: pc=" PFX " for thread " TIDFMT "\n", mc.pc,
+        thread->id);
+    ASSERT(!is_dynamo_address(mc.pc) && !in_fcache(mc.pc));
+    /* XXX case 7457: if the thread is suspended after it received a fault
+     * but before the kernel copied the faulting context to the user mode
+     * structures for the handler, it could result in a codemod exception
+     * that wouldn't happen natively!
+     */
+    DEBUG_DECLARE(ok =)
+    thread_set_mcontext(thread, &mc);
+    ASSERT(ok);
+    /* i#249: restore app's PEB/TEB fields */
+    IF_WINDOWS(restore_peb_pointer_for_thread(thread->dcontext));
+}
+
+void
+detach_cleanup_helper(thread_record_t *thread _IF_WINDOWS(bool detach_stacked_callbacks))
+{
+    DEBUG_DECLARE(int exit_res =)
+    dynamo_shared_exit(thread _IF_WINDOWS(detach_stacked_callbacks));
+    ASSERT(exit_res == SUCCESS);
+    detach_finalize_cleanup();
+
+    stack_free(d_r_initstack, DYNAMORIO_STACK_SIZE);
+
+    dynamo_exit_post_detach();
+
+    doing_detach = false;
+    started_detach = false;
+
+    SELF_PROTECT_DATASEC(DATASEC_RARELY_PROT);
+    dynamo_detaching_flag = LOCK_FREE_STATE;
+    EXITING_DR();
+    options_detach();
+}
+
 void
 detach_on_permanent_stack(bool internal, bool do_cleanup, dr_stats_t *drstats)
 {
@@ -1977,8 +2048,6 @@ detach_on_permanent_stack(bool internal, bool do_cleanup, dr_stats_t *drstats)
     bool detach_stacked_callbacks;
     bool *cleanup_tpc;
 #endif
-    DEBUG_DECLARE(bool ok;)
-    DEBUG_DECLARE(int exit_res;)
 
     /* synch-all flags: */
     uint flags = 0;
@@ -2152,7 +2221,6 @@ detach_on_permanent_stack(bool internal, bool do_cleanup, dr_stats_t *drstats)
 
     LOG(GLOBAL, LOG_ALL, 1, "Detach: starting to translate contexts\n");
     for (i = 0; i < num_threads; i++) {
-        priv_mcontext_t mc;
         if (threads[i]->dcontext == my_dcontext) {
             my_idx = i;
             my_tr = threads[i];
@@ -2166,54 +2234,7 @@ detach_on_permanent_stack(bool internal, bool do_cleanup, dr_stats_t *drstats)
             LOG(GLOBAL, LOG_ALL, 2, "Detach: not translating " TIDFMT "\n",
                 threads[i]->id);
         } else {
-            LOG(GLOBAL, LOG_ALL, 2, "Detach: translating " TIDFMT "\n", threads[i]->id);
-            DEBUG_DECLARE(ok =)
-            thread_get_mcontext(threads[i], &mc);
-            ASSERT(ok);
-            /* For a thread at a syscall, we use SA_RESTART for our suspend signal,
-             * so the kernel will adjust the restart point back to the syscall for us
-             * where expected.  This is an artifical signal we're introducing, so an
-             * app that assumes no signals and assumes its non-auto-restart syscalls
-             * don't need loops could be broken.
-             */
-            LOG(GLOBAL, LOG_ALL, 3,
-                /* Having the code bytes can help diagnose post-detach where the code
-                 * cache is gone.
-                 */
-                "Detach: pre-xl8 pc=%p (%02x %02x %02x %02x %02x), xsp=%p "
-                "for thread " TIDFMT "\n",
-                mc.pc, *mc.pc, *(mc.pc + 1), *(mc.pc + 2), *(mc.pc + 3), *(mc.pc + 4),
-                mc.xsp, threads[i]->id);
-            DEBUG_DECLARE(ok =)
-            translate_mcontext(threads[i], &mc, true /*restore mem*/, NULL /*f*/);
-            ASSERT(ok);
-
-            if (!threads[i]->under_dynamo_control) {
-                LOG(GLOBAL, LOG_ALL, 1,
-                    "Detach : thread " TIDFMT " already running natively\n",
-                    threads[i]->id);
-                /* we do need to restore the app ret addr, for native_exec */
-                if (!DYNAMO_OPTION(thin_client) && DYNAMO_OPTION(native_exec) &&
-                    !vmvector_empty(native_exec_areas)) {
-                    put_back_native_retaddrs(threads[i]->dcontext);
-                }
-            }
-            detach_finalize_translation(threads[i], &mc);
-
-            LOG(GLOBAL, LOG_ALL, 1, "Detach: pc=" PFX " for thread " TIDFMT "\n", mc.pc,
-                threads[i]->id);
-            ASSERT(!is_dynamo_address(mc.pc) && !in_fcache(mc.pc));
-            /* XXX case 7457: if the thread is suspended after it received a fault
-             * but before the kernel copied the faulting context to the user mode
-             * structures for the handler, it could result in a codemod exception
-             * that wouldn't happen natively!
-             */
-            DEBUG_DECLARE(ok =)
-            thread_set_mcontext(threads[i], &mc);
-            ASSERT(ok);
-
-            /* i#249: restore app's PEB/TEB fields */
-            IF_WINDOWS(restore_peb_pointer_for_thread(threads[i]->dcontext));
+            detach_set_mcontext_helper(threads[i]);
         }
         /* Resumes the thread, which will do kernel-visible cleanup of
          * signal state. Resume happens within the synch_all region where
@@ -2272,26 +2293,12 @@ detach_on_permanent_stack(bool internal, bool do_cleanup, dr_stats_t *drstats)
     SYSLOG_INTERNAL_INFO("Detaching from process, entering final cleanup");
     if (drstats != NULL)
         stats_get_snapshot(drstats);
-    DEBUG_DECLARE(exit_res =)
-    dynamo_shared_exit(my_tr _IF_WINDOWS(detach_stacked_callbacks));
-    ASSERT(exit_res == SUCCESS);
-    detach_finalize_cleanup();
-
-    stack_free(d_r_initstack, DYNAMORIO_STACK_SIZE);
-
-    dynamo_exit_post_detach();
-
-    doing_detach = false;
-    started_detach = false;
-
-    SELF_PROTECT_DATASEC(DATASEC_RARELY_PROT);
-    dynamo_detaching_flag = LOCK_FREE_STATE;
-    EXITING_DR();
-    options_detach();
+    detach_cleanup_helper(my_tr _IF_WINDOWS(detach_stacked_callbacks));
 }
+
 #ifdef LINUX
 void
-detach_externally_on_linux()
+detach_externally_on_new_stack()
 {
     dcontext_t *my_dcontext;
     priv_mcontext_t my_mcontext;
@@ -2300,7 +2307,6 @@ detach_externally_on_linux()
     int i, num_threads, my_idx = -1;
     thread_id_t my_id;
     DEBUG_DECLARE(bool ok;)
-    DEBUG_DECLARE(int exit_res;)
     /* synch-all flags: */
     uint flags = 0;
     /* For Unix, such privilege problems are rarer but we would still prefer to
@@ -2373,7 +2379,6 @@ detach_externally_on_linux()
     dynamo_process_exit_with_thread_info();
     LOG(GLOBAL, LOG_ALL, 1, "Detach: starting to translate contexts\n");
     for (i = 0; i < num_threads; i++) {
-        priv_mcontext_t mc;
         if (threads[i]->dcontext == my_dcontext) {
             my_idx = i;
             my_tr = threads[i];
@@ -2392,50 +2397,7 @@ detach_externally_on_linux()
             LOG(GLOBAL, LOG_ALL, 2, "Detach: not translating " TIDFMT "\n",
                 threads[i]->id);
         } else {
-            LOG(GLOBAL, LOG_ALL, 2, "Detach: translating " TIDFMT "\n", threads[i]->id);
-            DEBUG_DECLARE(ok =)
-            thread_get_mcontext(threads[i], &mc);
-            ASSERT(ok);
-            /* For a thread at a syscall, we use SA_RESTART for our suspend signal,
-             * so the kernel will adjust the restart point back to the syscall for us
-             * where expected.  This is an artifical signal we're introducing, so an
-             * app that assumes no signals and assumes its non-auto-restart syscalls
-             * don't need loops could be broken.
-             */
-            LOG(GLOBAL, LOG_ALL, 3,
-                /* Having the code bytes can help diagnose post-detach where the code
-                 * cache is gone.
-                 */
-                "Detach: pre-xl8 pc=%p (%02x %02x %02x %02x %02x), xsp=%p "
-                "for thread " TIDFMT "\n",
-                mc.pc, *mc.pc, *(mc.pc + 1), *(mc.pc + 2), *(mc.pc + 3), *(mc.pc + 4),
-                mc.xsp, threads[i]->id);
-            DEBUG_DECLARE(ok =)
-            translate_mcontext(threads[i], &mc, true /*restore mem*/, NULL /*f*/);
-            ASSERT(ok);
-            if (!threads[i]->under_dynamo_control) {
-                dr_printf("Detach : thread " TIDFMT " already running natively\n",
-                          threads[i]->id);
-                LOG(GLOBAL, LOG_ALL, 1,
-                    "Detach : thread " TIDFMT " already running natively\n",
-                    threads[i]->id);
-                /* we do need to restore the app ret addr, for native_exec */
-                if (!DYNAMO_OPTION(thin_client) && DYNAMO_OPTION(native_exec) &&
-                    !vmvector_empty(native_exec_areas)) {
-                    put_back_native_retaddrs(threads[i]->dcontext);
-                }
-            }
-            LOG(GLOBAL, LOG_ALL, 1, "Detach: pc=" PFX " for thread " TIDFMT "\n", mc.pc,
-                threads[i]->id);
-            ASSERT(!is_dynamo_address(mc.pc) && !in_fcache(mc.pc));
-            /* XXX case 7457: if the thread is suspended after it received a fault
-             * but before the kernel copied the faulting context to the user mode
-             * structures for the handler, it could result in a codemod exception
-             * that wouldn't happen natively!
-             */
-            DEBUG_DECLARE(ok =)
-            thread_set_mcontext(threads[i], &mc);
-            ASSERT(ok);
+            detach_set_mcontext_helper(threads[i]);
         }
         /* Resumes the thread, which will do kernel-visible cleanup of
          * signal state. Resume happens within the synch_all region where
@@ -2472,18 +2434,7 @@ detach_externally_on_linux()
     threads = NULL;
     LOG(GLOBAL, LOG_ALL, 1, "Detach: Entering final cleanup and unload\n");
     SYSLOG_INTERNAL_INFO("Detaching from process, entering final cleanup");
-    DEBUG_DECLARE(exit_res =)
-    dynamo_shared_exit(my_tr _IF_WINDOWS(detach_stacked_callbacks));
-    ASSERT(exit_res == SUCCESS);
-    detach_finalize_cleanup();
-    stack_free(d_r_initstack, DYNAMORIO_STACK_SIZE);
-    dynamo_exit_post_detach();
-    doing_detach = false;
-    started_detach = false;
-    SELF_PROTECT_DATASEC(DATASEC_RARELY_PROT);
-    dynamo_detaching_flag = LOCK_FREE_STATE;
-    EXITING_DR();
-    options_detach();
+    detach_cleanup_helper(my_tr);
     thread_set_self_mcontext(&my_mcontext, true);
 }
 #endif

core/synch.h

@@ -260,12 +260,18 @@ translate_from_synchall_to_dispatch(thread_record_t *tr,
 void
 send_all_other_threads_native(void);
 
+void
+detach_cleanup_helper(thread_record_t *thread _IF_WINDOWS(bool detach_stacked_callbacks));
+
+void
+detach_set_mcontext_helper(thread_record_t *thread);
+
 void
 detach_on_permanent_stack(bool internal, bool do_cleanup, dr_stats_t *drstats);
 
 #ifdef LINUX
 void
-detach_externally_on_linux();
+detach_externally_on_new_stack();
 #endif
 
 /*** exported for detach only ***/

core/unix/os.c

@@ -3983,9 +3983,7 @@ thread_get_mcontext(thread_record_t *tr, priv_mcontext_t *mc)
 bool
 thread_get_nudged_mcontext(thread_record_t *tr, priv_mcontext_t *mc)
 {
-    /* PR 212090: only works when target is suspended by us, and
-     * we then take the signal context
-     */
+    /* This only works for a thread that just received a nduge signal. */
     os_thread_data_t *ostd = (os_thread_data_t *)tr->dcontext->os_field;
     ASSERT(ostd != NULL);
     ASSERT(ostd->nudged_sigcxt != NULL);

core/unix/os_private.h

@@ -166,7 +166,7 @@ typedef struct _os_thread_data_t {
     sig_full_cxt_t *suspended_sigcxt;
 
 #ifdef LINUX
-    /* For detachment on Linux*/
+    /* For detachment on Linux. */
     sig_full_cxt_t *nudged_sigcxt;
 #endif
 

suite/tests/client-interface/detach_test.dll.c

@@ -44,10 +44,12 @@ static bool saw_attach_event = false;
 static void
 dr_exit(void)
 {
+    if (!saw_attach_event)
+        dr_fprintf(STDERR, "Error: never saw attach event!\n");
 #ifdef WINDOWS
     dr_fprintf(STDERR, "done\n");
 #else
-    /* The app prints 'done' for us. */
+        /* The app prints 'done' for us. */
 #endif
 }