i#2016: switch stacks when setting DR's alt stack

To handle being on the app's alt stack during thread init for delayed takeover (typically start/stop usage), we temporarily switch stacks to set DR's alt stack. Adds a test of an app that uses signals and the start/stop interface and static DR, to test i#2016 and also i#2018, i#2021, and i#1921. Fixes #2016 Review-URL: https://codereview.appspot.com/310500043
DynamoRIO · Oct 13, 2016 · d193665 · d193665
1 parent 4dd0651
commit d193665
Show file tree

Hide file tree

Showing 5 changed files with 222 additions and 6 deletions.
diff --git a/core/unix/signal.c b/core/unix/signal.c
@@ -455,12 +455,21 @@ signal_exit()
 #endif
 }
 
+#ifdef HAVE_SIGALTSTACK
+/* Separated out to run from the dstack (i#2016: see below). */
+static void
+set_our_alt_stack(void *arg)
+{
+    thread_sig_info_t *info = (thread_sig_info_t *) arg;
+    DEBUG_DECLARE(int rc =)
+        sigaltstack_syscall(&info->sigstack, &info->app_sigstack);
+    ASSERT(rc == 0);
+}
+#endif
+
 void
 signal_thread_init(dcontext_t *dcontext)
 {
-#ifdef HAVE_SIGALTSTACK
-    int rc;
-#endif
     thread_sig_info_t *info = HEAP_TYPE_ALLOC(dcontext, thread_sig_info_t,
                                               ACCT_OTHER, PROTECTED);
 
@@ -489,8 +498,17 @@ signal_thread_init(dcontext_t *dcontext)
     info->sigstack.ss_size = SIGSTACK_SIZE;
     /* kernel will set xsp to sp+size to grow down from there, we don't have to */
     info->sigstack.ss_flags = 0;
-    rc = sigaltstack_syscall(&info->sigstack, &info->app_sigstack);
-    ASSERT(rc == 0);
+
+    /* i#2016: for late takeover, this app thread may already be on its own alt
+     * stack.  Not setting SA_ONSTACK for SUSPEND_SIGNAL is not sufficient to avoid
+     * this, as our SUSPEND_SIGNAL can interrupt the app inside its own signal
+     * handler.  Thus, we simply swap to another stack temporarily to avoid the
+     * kernel complaining.  The dstack is set up but it has the clone record and
+     * initial mcxt, so we use the new alt stack.
+     */
+    call_switch_stack((void *)info,
+                      (byte *)info->sigstack.ss_sp + info->sigstack.ss_size,
+                      set_our_alt_stack, NULL, true/*return*/);
     LOG(THREAD, LOG_ASYNCH, 1, "signal stack is "PFX" - "PFX"\n",
         info->sigstack.ss_sp, info->sigstack.ss_sp + info->sigstack.ss_size);
     /* app_sigstack dealt with below, based on parentage */

diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt
@@ -2236,6 +2236,10 @@ if (CLIENT_INTERFACE)
     target_link_libraries(api.static_noinit ${libmath})
     tobuild_api(api.static_detach api/static_detach.c "" "" OFF ON)
     target_link_libraries(api.static_detach ${libmath})
+    if (NOT WIN32)
+      tobuild_api(api.static_signal api/static_signal.c "" "" OFF ON)
+      target_link_libraries(api.static_signal ${libmath} ${libpthread})
+    endif ()
   endif ()
 
   if (NOT X64 AND NOT ARM) # FIXME i#1551: port to ARM

diff --git a/suite/tests/api/startstop.c b/suite/tests/api/startstop.c
@@ -190,7 +190,7 @@ int main(void)
 #endif
     }
 
-    /* Initialized DR */
+    /* Initialize DR */
     dr_app_setup();
     /* XXX: Calling the client interface from the app is not supported.  We're
      * just using it for testing.

diff --git a/suite/tests/api/static_signal.c b/suite/tests/api/static_signal.c
@@ -0,0 +1,179 @@
+/* **********************************************************
+ * Copyright (c) 2016 Google, Inc.  All rights reserved.
+ * **********************************************************/
+
+/*
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * * Redistributions of source code must retain the above copyright notice,
+ *   this list of conditions and the following disclaimer.
+ *
+ * * Redistributions in binary form must reproduce the above copyright notice,
+ *   this list of conditions and the following disclaimer in the documentation
+ *   and/or other materials provided with the distribution.
+ *
+ * * Neither the name of Google, Inc. nor the names of its contributors may be
+ *   used to endorse or promote products derived from this software without
+ *   specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL GOOGLE, INC. OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
+ * DAMAGE.
+ */
+
+#include "configure.h"
+#ifndef UNIX
+# error UNIX-only
+#endif
+#include "dr_api.h"
+#include "tools.h"
+#include <pthread.h>
+#include "condvar.h"
+#include <math.h>
+#include <unistd.h>
+#include <signal.h>
+#include <stdlib.h>
+
+#define ALT_STACK_SIZE  (SIGSTKSZ*2)
+
+static int num_bbs;
+static int num_signals;
+
+static void *thread_ready;
+static void *thread_exit;
+static void *got_signal;
+
+static void
+signal_handler(int sig, siginfo_t *siginfo, ucontext_t *ucxt)
+{
+    if (sig == SIGUSR1) {
+        print("Got SIGUSR1\n");
+        signal_cond_var(got_signal);
+    } else {
+        print("Got unexpected signal %d\n", sig);
+    }
+}
+
+static void *
+thread_func(void *arg)
+{
+    stack_t sigstack;
+    int rc;
+    sigstack.ss_sp = (char *) malloc(ALT_STACK_SIZE);
+    sigstack.ss_size = ALT_STACK_SIZE;
+    sigstack.ss_flags = SS_ONSTACK;
+    rc = sigaltstack(&sigstack, NULL);
+    ASSERT_NOERR(rc);
+    signal_cond_var(thread_ready);
+    wait_cond_var(thread_exit);
+    free(sigstack.ss_sp);
+    return NULL;
+}
+
+static dr_emit_flags_t
+event_bb(void *drcontext, void *tag, instrlist_t *bb, bool for_trace,
+         bool translating)
+{
+    num_bbs++;
+    return DR_EMIT_DEFAULT;
+}
+
+static dr_signal_action_t
+event_signal(void *drcontext, dr_siginfo_t *info)
+{
+    dr_atomic_add32_return_sum(&num_signals, 1);
+    return DR_SIGNAL_DELIVER;
+}
+
+static void
+event_exit(void)
+{
+    dr_fprintf(STDERR, "Saw %s bb events\n", num_bbs > 0 ? "some" : "no");
+    dr_fprintf(STDERR, "Saw %d signal(s)\n", num_signals);
+}
+
+DR_EXPORT void
+dr_client_main(client_id_t id, int argc, const char *argv[])
+{
+    print("in dr_client_main\n");
+    dr_register_bb_event(event_bb);
+    dr_register_signal_event(event_signal);
+    dr_register_exit_event(event_exit);
+}
+
+static int
+do_some_work(void)
+{
+    static int iters = 8192;
+    int i;
+    double val = num_bbs;
+    for (i = 0; i < iters; ++i) {
+        val += sin(val);
+    }
+    return (val > 0);
+}
+
+int
+main(int argc, const char *argv[])
+{
+    pthread_t thread;
+    intercept_signal(SIGUSR1, signal_handler, true/*sigstack*/);
+    thread_ready = create_cond_var();
+    thread_exit = create_cond_var();
+    got_signal = create_cond_var();
+    pthread_create(&thread, NULL, thread_func, NULL);
+    wait_cond_var(thread_ready);
+
+    print("Sending SIGUSR1 pre-DR-init\n");
+    pthread_kill(thread, SIGUSR1);
+    wait_cond_var(got_signal);
+    reset_cond_var(got_signal);
+
+    print("pre-DR init\n");
+    dr_app_setup();
+    assert(!dr_app_running_under_dynamorio());
+
+    print("Sending SIGUSR1 pre-DR-start\n");
+    pthread_kill(thread, SIGUSR1);
+    wait_cond_var(got_signal);
+    reset_cond_var(got_signal);
+
+    print("pre-DR start\n");
+    dr_app_start();
+    assert(dr_app_running_under_dynamorio());
+
+    if (do_some_work() < 0)
+        print("error in computation\n");
+
+    print("Sending SIGUSR1 under DR\n");
+    pthread_kill(thread, SIGUSR1);
+    wait_cond_var(got_signal);
+    reset_cond_var(got_signal);
+
+    print("pre-DR stop\n");
+    // i#95: today we don't have full support for separating stop from cleanup:
+    // we rely on the app joining threads prior to cleanup.
+    // We do support a full detach on dr_app_stop_and_cleanup() which we use here.
+    dr_app_stop_and_cleanup();
+    assert(!dr_app_running_under_dynamorio());
+
+    print("Sending SIGUSR1 post-DR-stop\n");
+    pthread_kill(thread, SIGUSR1);
+    wait_cond_var(got_signal);
+    reset_cond_var(got_signal);
+
+    signal_cond_var(thread_exit);
+    pthread_join(thread, NULL);
+
+    print("all done\n");
+    return 0;
+}
diff --git a/suite/tests/api/static_signal.expect b/suite/tests/api/static_signal.expect
@@ -0,0 +1,15 @@
+Sending SIGUSR1 pre-DR-init
+Got SIGUSR1
+pre-DR init
+in dr_client_main
+Sending SIGUSR1 pre-DR-start
+Got SIGUSR1
+pre-DR start
+Sending SIGUSR1 under DR
+Got SIGUSR1
+pre-DR stop
+Saw some bb events
+Saw 1 signal(s)
+Sending SIGUSR1 post-DR-stop
+Got SIGUSR1
+all done