tsan: restructure signal handling to allow recursive handling

Fixes issue
https://code.google.com/p/thread-sanitizer/issues/detail?id=71

llvm-svn: 216903

compiler-rt/lib/tsan/rtl/tsan_interceptors.cc

@@ -124,9 +124,9 @@ struct SignalDesc {
 };
 
 struct SignalContext {
-  int in_blocking_func;
   int int_signal_send;
-  int pending_signal_count;
+  bool in_blocking_func;
+  bool have_pending_signals;
   SignalDesc pending_signals[kSigCount];
 };
 
@@ -222,11 +222,11 @@ ScopedInterceptor::~ScopedInterceptor() {
 struct BlockingCall {
   explicit BlockingCall(ThreadState *thr)
       : ctx(SigCtx(thr)) {
-    ctx->in_blocking_func++;
+    ctx->in_blocking_func = true;
   }
 
   ~BlockingCall() {
-    ctx->in_blocking_func--;
+    ctx->in_blocking_func = false;
   }
 
   SignalContext *ctx;
@@ -1683,9 +1683,10 @@ TSAN_INTERCEPTOR(int, epoll_wait, int epfd, void *ev, int cnt, int timeout) {
 
 namespace __tsan {
 
-static void CallUserSignalHandler(ThreadState *thr, bool sync, bool sigact,
-    int sig, my_siginfo_t *info, void *uctx) {
-  Acquire(thr, 0, (uptr)&sigactions[sig]);
+static void CallUserSignalHandler(ThreadState *thr, bool sync, bool acquire,
+    bool sigact, int sig, my_siginfo_t *info, void *uctx) {
+  if (acquire)
+    Acquire(thr, 0, (uptr)&sigactions[sig]);
   // Ensure that the handler does not spoil errno.
   const int saved_errno = errno;
   errno = 99;
@@ -1720,10 +1721,10 @@ static void CallUserSignalHandler(ThreadState *thr, bool sync, bool sigact,
 
 void ProcessPendingSignals(ThreadState *thr) {
   SignalContext *sctx = SigCtx(thr);
-  if (sctx == 0 || sctx->pending_signal_count == 0 || thr->in_signal_handler)
+  if (sctx == 0 || !sctx->have_pending_signals)
     return;
-  thr->in_signal_handler = true;
-  sctx->pending_signal_count = 0;
+  sctx->have_pending_signals = false;
+  atomic_fetch_add(&thr->in_signal_handler, 1, memory_order_relaxed);
   // These are too big for stack.
   static THREADLOCAL __sanitizer_sigset_t emptyset, oldset;
   REAL(sigfillset)(&emptyset);
@@ -1734,14 +1735,13 @@ void ProcessPendingSignals(ThreadState *thr) {
       signal->armed = false;
       if (sigactions[sig].sa_handler != SIG_DFL
           && sigactions[sig].sa_handler != SIG_IGN) {
-        CallUserSignalHandler(thr, false, signal->sigaction,
+        CallUserSignalHandler(thr, false, true, signal->sigaction,
             sig, &signal->siginfo, &signal->ctx);
       }
     }
   }
   pthread_sigmask(SIG_SETMASK, &oldset, 0);
-  CHECK_EQ(thr->in_signal_handler, true);
-  thr->in_signal_handler = false;
+  atomic_fetch_add(&thr->in_signal_handler, -1, memory_order_relaxed);
 }
 
 }  // namespace __tsan
@@ -1767,21 +1767,27 @@ void ALWAYS_INLINE rtl_generic_sighandler(bool sigact, int sig,
       // If we are in blocking function, we can safely process it now
       // (but check if we are in a recursive interceptor,
       // i.e. pthread_join()->munmap()).
-      (sctx && sctx->in_blocking_func == 1)) {
-    CHECK_EQ(thr->in_signal_handler, false);
-    thr->in_signal_handler = true;
-    if (sctx && sctx->in_blocking_func == 1) {
+      (sctx && sctx->in_blocking_func)) {
+    atomic_fetch_add(&thr->in_signal_handler, 1, memory_order_relaxed);
+    if (sctx && sctx->in_blocking_func) {
       // We ignore interceptors in blocking functions,
       // temporary enbled them again while we are calling user function.
       int const i = thr->ignore_interceptors;
       thr->ignore_interceptors = 0;
-      CallUserSignalHandler(thr, sync, sigact, sig, info, ctx);
+      sctx->in_blocking_func = false;
+      CallUserSignalHandler(thr, sync, true, sigact, sig, info, ctx);
       thr->ignore_interceptors = i;
+      sctx->in_blocking_func = true;
     } else {
-      CallUserSignalHandler(thr, sync, sigact, sig, info, ctx);
+      // Be very conservative with when we do acquire in this case.
+      // It's unsafe to do acquire in async handlers, because ThreadState
+      // can be in inconsistent state.
+      // SIGSYS looks relatively safe -- it's synchronous and can actually
+      // need some global state.
+      bool acq = (sig == SIGSYS);
+      CallUserSignalHandler(thr, sync, acq, sigact, sig, info, ctx);
     }
-    CHECK_EQ(thr->in_signal_handler, true);
-    thr->in_signal_handler = false;
+    atomic_fetch_add(&thr->in_signal_handler, -1, memory_order_relaxed);
     return;
   }
 
@@ -1795,7 +1801,7 @@ void ALWAYS_INLINE rtl_generic_sighandler(bool sigact, int sig,
       internal_memcpy(&signal->siginfo, info, sizeof(*info));
     if (ctx)
       internal_memcpy(&signal->ctx, ctx, sizeof(signal->ctx));
-    sctx->pending_signal_count++;
+    sctx->have_pending_signals = true;
   }
 }
 

compiler-rt/lib/tsan/rtl/tsan_mman.cc

@@ -63,7 +63,8 @@ void AllocatorPrintStats() {
 }
 
 static void SignalUnsafeCall(ThreadState *thr, uptr pc) {
-  if (!thr->in_signal_handler || !flags()->report_signal_unsafe)
+  if (atomic_load(&thr->in_signal_handler, memory_order_relaxed) == 0 ||
+      !flags()->report_signal_unsafe)
     return;
   StackTrace stack;
   stack.ObtainCurrent(thr, pc);

compiler-rt/lib/tsan/rtl/tsan_rtl.h

@@ -369,7 +369,7 @@ struct ThreadState {
   DDPhysicalThread *dd_pt;
   DDLogicalThread *dd_lt;
 
-  bool in_signal_handler;
+  atomic_uintptr_t in_signal_handler;
   SignalContext *signal_ctx;
 
   DenseSlabAllocCache block_cache;

compiler-rt/test/tsan/signal_recursive.cc

@@ -0,0 +1,132 @@
+// RUN: %clang_tsan -O1 %s -o %t && %run %t 2>&1 | FileCheck %s
+
+// Test case for recursive signal handlers, adopted from:
+// https://code.google.com/p/thread-sanitizer/issues/detail?id=71
+
+#include <pthread.h>
+#include <semaphore.h>
+#include <signal.h>
+#include <unistd.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <stdio.h>
+
+static const int kSigSuspend = SIGPWR;
+static const int kSigRestart = SIGXCPU;
+static sigset_t g_suspend_handler_mask;
+
+static sem_t g_thread_suspend_ack_sem;
+
+static bool g_busy_thread_received_restart;
+
+static volatile bool g_busy_thread_garbage_collected;
+
+static void SaveRegistersInStack() {
+  // Mono walks thread stacks to detect unreferenced objects.
+  // If last object reference is kept in register the object will be collected
+  // This is why threads can't be suspended with something like pthread_suspend
+};
+
+static void fail(const char *what) {
+  fprintf(stderr, "FAILED: %s (errno=%d)\n", what, errno);
+  exit(1);
+}
+
+static void SuspendHandler(int sig) {
+  int old_errno = errno;
+  SaveRegistersInStack();
+  // Acknowledge that thread is saved and suspended
+  if (sem_post(&g_thread_suspend_ack_sem) != 0)
+    fail("sem_post failed");
+
+  do {
+    g_busy_thread_received_restart = false;
+    if (sigsuspend(&g_suspend_handler_mask) != -1 || errno != EINTR)
+      fail("sigsuspend failed");
+  } while (!g_busy_thread_received_restart);
+
+  // Acknowledge that thread restarted
+  if (sem_post(&g_thread_suspend_ack_sem) != 0)
+    fail("sem_post failed");
+
+  g_busy_thread_garbage_collected = true;
+
+  errno = old_errno;
+}
+
+static void RestartHandler(int sig) {
+  g_busy_thread_received_restart = true;
+}
+
+static void StopWorld(pthread_t thread) {
+  int result = pthread_kill(thread, kSigSuspend);
+  if (result != 0)
+    fail("pthread_kill failed");
+
+  while ((result = sem_wait(&g_thread_suspend_ack_sem)) != 0) {
+    if (result != EINTR) {
+      fail("sem_wait failed");
+    }
+  }
+}
+
+static void StartWorld(pthread_t thread) {
+  int result = pthread_kill(thread, kSigRestart);
+  if (result != 0)
+    fail("pthread_kill failed");
+
+  while ((result = sem_wait(&g_thread_suspend_ack_sem)) != 0) {
+    if (result != EINTR) {
+      fail("sem_wait failed");
+    }
+  }
+}
+
+static void CollectGarbage(pthread_t thread) {
+  StopWorld(thread);
+  // Walk stacks
+    StartWorld(thread);
+}
+
+static void Init() {
+  if (sigfillset(&g_suspend_handler_mask) != 0)
+    fail("sigfillset failed");
+  if (sigdelset(&g_suspend_handler_mask, kSigRestart) != 0)
+    fail("sigdelset failed");
+  if (sem_init(&g_thread_suspend_ack_sem, 0, 0) != 0)
+    fail("sem_init failed");;
+
+  struct sigaction act = {};
+  act.sa_flags = SA_RESTART;
+  sigfillset(&act.sa_mask);
+  act.sa_handler = &SuspendHandler;
+  if (sigaction(kSigSuspend, &act, NULL) != 0)
+    fail("sigaction failed");
+  act.sa_handler = &RestartHandler;
+  if (sigaction(kSigRestart, &act, NULL) != 0)
+    fail("sigaction failed");
+}
+
+void* BusyThread(void *arg) {
+  (void)arg;
+  while (!g_busy_thread_garbage_collected) {
+    usleep(100); // Tsan deadlocks without these sleeps
+  }
+  return NULL;
+}
+
+int main(int argc, const char *argv[]) {
+  Init();
+  pthread_t busy_thread;
+  pthread_create(&busy_thread, NULL, &BusyThread, NULL);
+  sleep(1); // Tsan deadlocks without these sleeps
+  CollectGarbage(busy_thread);
+  pthread_join(busy_thread, 0);
+  fprintf(stderr, "DONE\n");
+  return 0;
+}
+
+// CHECK-NOT: FAILED
+// CHECK-NOT: ThreadSanitizer CHECK failed
+// CHECK-NOT: WARNING: ThreadSanitizer:
+// CHECK: DONE