i#5458: Switch suspend signal to SIGSTKFLT/SIGFPE (#5461)

Changes the signal that DR uses to suspend a thread from SIGUSR2,
which is sometimes blocked by the app at attach time, to SIGSTKFLT on
Linux and SIGFPE on Mac.  (SIGFPE was the first choice on Linux but
QEMU crashes when we use it.)  These are fatal normally-synchronous
signals and so are less likely to be blocked.

Manually tested on an attach to mysqld which failed with SIGUSR2 but
succeeds now.

Fixes #5458

api/docs/debugging.dox

@@ -1,5 +1,5 @@
 /* ******************************************************************************
- * Copyright (c) 2010-2021 Google, Inc.  All rights reserved.
+ * Copyright (c) 2010-2022 Google, Inc.  All rights reserved.
  * ******************************************************************************/
 
 /*
@@ -82,7 +82,7 @@ On Linux, DR sends itself a SIGILL signal during initialization, in order to mea
 
 Additionally, DR uses various "safe read" strategies where it may raise a SIGSEGV or SIGBUS while examining application memory.  Load symbols and look for `safe_read` variants on the call stack (such as `safe_read_tls_magic`, `safe_read_asm_pre`, `safe_read_fast`) to identify these.  Just like with the init-time SIGILL, simply continue past these.
 
-One final signal used by DR is SIGUSR2, which is sent to other threads to suspend or terminate them.
+One final signal used by DR is SIGSTKFLT (SIGFPE on MacOS), which is sent to other threads to suspend or terminate them.
 
 ## Attaching
 

core/unix/os.c

@@ -3697,7 +3697,7 @@ bool
 os_thread_terminate(thread_record_t *tr)
 {
     /* PR 297902: for NPTL sending SIGKILL will take down the whole group:
-     * so instead we send SIGUSR2 and have a flag set telling
+     * so instead we send SUSPEND_SIGNAL and have a flag set telling
      * target thread to execute SYS_exit
      */
     os_thread_data_t *ostd = (os_thread_data_t *)tr->dcontext->os_field;

core/unix/os_private.h

@@ -1,5 +1,5 @@
 /* **********************************************************
- * Copyright (c) 2011-2020 Google, Inc.  All rights reserved.
+ * Copyright (c) 2011-2022 Google, Inc.  All rights reserved.
  * Copyright (c) 2008-2010 VMware, Inc.  All rights reserved.
  * **********************************************************/
 
@@ -84,8 +84,17 @@
 
 #define MACHINE_TLS_IS_DR_TLS IF_X86_ELSE(INTERNAL_OPTION(mangle_app_seg), true)
 
-/* PR 212090: the signal we use to suspend threads */
-#define SUSPEND_SIGNAL SIGUSR2
+/* The signal we use to suspend threads.
+ * We choose a normally-synchronous signal for a lower chance that the app has
+ * blocked it when we attach to an already-running app.
+ * SIGFPE is a good choice, but when running under QEMU, QEMU crashes
+ * when we send it, so we use SIGSTKFLT (which does not exist on Mac:
+ * there we use SIGFPE).
+ * We could conceivably make this a variable controlled by a runtime option,
+ * but it has a number of limitations, and the checks for it being user-sent
+ * would need to be flexible: it doesn't seem worth the complexity at this point.
+ */
+#define SUSPEND_SIGNAL IF_MACOS_ELSE(SIGFPE, SIGSTKFLT)
 
 #ifdef MACOS
 /* While there is no clone system call, we use the same clone flags to share

core/unix/signal.c

@@ -305,8 +305,8 @@ static bool
 handle_alarm(dcontext_t *dcontext, int sig, kernel_ucontext_t *ucxt);
 
 static bool
-handle_suspend_signal(dcontext_t *dcontext, kernel_ucontext_t *ucxt,
-                      sigframe_rt_t *frame);
+handle_suspend_signal(dcontext_t *dcontext, kernel_siginfo_t *siginfo,
+                      kernel_ucontext_t *ucxt, sigframe_rt_t *frame);
 
 static bool
 handle_nudge_signal(dcontext_t *dcontext, kernel_siginfo_t *siginfo,
@@ -5635,7 +5635,7 @@ main_signal_handler_C(byte *xsp)
 
     /* PR 212090: the signal we use to suspend threads */
     case SUSPEND_SIGNAL:
-        if (handle_suspend_signal(dcontext, ucxt, frame)) {
+        if (handle_suspend_signal(dcontext, siginfo, ucxt, frame)) {
             /* i#1921: see comment above */
             ASSERT(tr == NULL || tr->under_dynamo_control || IS_CLIENT_THREAD(dcontext));
             record_pending_signal(dcontext, sig, ucxt, frame, false, NULL);
@@ -7832,13 +7832,23 @@ sig_detach(dcontext_t *dcontext, sigframe_rt_t *frame, KSYNCH_TYPE *detached)
 
 /* Returns whether to pass on to app */
 static bool
-handle_suspend_signal(dcontext_t *dcontext, kernel_ucontext_t *ucxt, sigframe_rt_t *frame)
+handle_suspend_signal(dcontext_t *dcontext, kernel_siginfo_t *siginfo,
+                      kernel_ucontext_t *ucxt, sigframe_rt_t *frame)
 {
     os_thread_data_t *ostd = (os_thread_data_t *)dcontext->os_field;
     kernel_sigset_t prevmask;
     sig_full_cxt_t sc_full;
     ASSERT(ostd != NULL);
 
+    /* Distinguish up front from a synchronous app signal by looking for
+     * si_code where <=0 means user-sent.
+     * We distinguish further below from the rare case of an app sending
+     * SUSPEND_SIGNAL asynchronously by looking for our particular state settings
+     * that correspond to the use of this signal by DR..
+     */
+    if (siginfo->si_code > 0 || siginfo->si_signo != SUSPEND_SIGNAL)
+        return true; /* pass to app */
+
     if (ostd->terminate) {
         /* PR 297902: exit this thread, without using the dstack */
         /* For MacOS, we need a stack as 32-bit syscalls take args on the stack.

suite/tests/api/detach_signal.cpp

@@ -1,5 +1,5 @@
 /* **********************************************************
- * Copyright (c) 2011-2021 Google, Inc.  All rights reserved.
+ * Copyright (c) 2011-2022 Google, Inc.  All rights reserved.
  * Copyright (c) 2003-2008 VMware, Inc.  All rights reserved.
  * **********************************************************/
 
@@ -61,6 +61,12 @@
 /* SIGSTKSZ*2 results in a fatal error from DR on fitting the copied frame. */
 #define ALT_STACK_SIZE (SIGSTKSZ * 4)
 
+#ifdef MACOS
+#    define DR_SUSPEND_SIGNAL SIGFPE /* DR's takeover signal. */
+#else
+#    define DR_SUSPEND_SIGNAL SIGSTKFLT /* DR's takeover signal. */
+#endif
+
 static volatile bool sideline_exit = false;
 static void *sideline_continue;
 static void *sideline_ready[NUM_THREADS];
@@ -88,7 +94,7 @@ handle_signal(int signal, siginfo_t *siginfo, ucontext_t *ucxt)
     sigset_t expect_mask2;
     memcpy(&expect_mask2, &handler_mask, sizeof(expect_mask2));
     sigaddset(&expect_mask2, signal);
-    sigaddset(&expect_mask2, SIGUSR2);
+    sigaddset(&expect_mask2, DR_SUSPEND_SIGNAL);
     assert(memcmp(&expect_mask1, &actual_mask, sizeof(expect_mask1)) == 0 ||
            memcmp(&expect_mask2, &actual_mask, sizeof(expect_mask2)) == 0);
 
@@ -109,7 +115,7 @@ sideline_spinner(void *arg)
          */
         sigset_t delay_attach_mask;
         sigemptyset(&delay_attach_mask);
-        sigaddset(&delay_attach_mask, SIGUSR2); /* DR's takeover signal. */
+        sigaddset(&delay_attach_mask, DR_SUSPEND_SIGNAL);
         int res = sigprocmask(SIG_SETMASK, &delay_attach_mask, NULL);
         assert(res == 0);
     }
@@ -118,8 +124,8 @@ sideline_spinner(void *arg)
     signal_cond_var(sideline_ready[idx]);
 
     if (idx == 0) {
-        /* Spend some time generating signals while SIGUSR2 is blocked to try and
-         * generate some after DR starts takeover and puts its own handler in place,
+        /* Spend some time generating signals while DR_SUSPEND_SIGNAL is blocked to try
+         * and generate some after DR starts takeover and puts its own handler in place,
          * but before it can take us over.
          */
         for (int i = 0; i < 10000; i++) {
@@ -217,7 +223,7 @@ main(void)
 
     sigset_t prior_mask;
     sigemptyset(&handler_mask);
-    sigaddset(&handler_mask, SIGUSR2);
+    sigaddset(&handler_mask, DR_SUSPEND_SIGNAL);
     int res = sigprocmask(SIG_SETMASK, &handler_mask, &prior_mask);
     assert(res == 0);
     res = sigprocmask(SIG_SETMASK, &prior_mask, NULL);