[compiler-rt] r261658 - tsan: fix signal handling in ignored libraries

Dmitry Vyukov via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 23 09:16:28 PST 2016


Author: dvyukov
Date: Tue Feb 23 11:16:26 2016
New Revision: 261658

URL: http://llvm.org/viewvc/llvm-project?rev=261658&view=rev
Log:
tsan: fix signal handling in ignored libraries

The first issue is that we longjmp from ScopedInterceptor scope
when called from an ignored lib. This leaves thr->in_ignored_lib set.
This, in turn, disables handling of sigaction. This, in turn,
corrupts tsan state since signals delivered asynchronously.
Another issue is that we can ignore synchronization in asignal
handler, if the signal is delivered into an IgnoreSync region.
Since signals are generally asynchronous, they should ignore
memory access/synchronization/interceptor ignores.
This could lead to false positives in signal handlers.


Added:
    compiler-rt/trunk/test/tsan/ignore_lib4.cc
    compiler-rt/trunk/test/tsan/ignore_lib4.cc.supp
    compiler-rt/trunk/test/tsan/signal_sync2.cc
Modified:
    compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc

Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc?rev=261658&r1=261657&r2=261658&view=diff
==============================================================================
--- compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc (original)
+++ compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc Tue Feb 23 11:16:26 2016
@@ -581,8 +581,11 @@ DEFINE_REAL(int, __sigsetjmp, void *env)
 #endif  // SANITIZER_MAC
 
 TSAN_INTERCEPTOR(void, longjmp, uptr *env, int val) {
+  // Note: if we call REAL(longjmp) in the context of ScopedInterceptor,
+  // bad things will happen. We will jump over ScopedInterceptor dtor and can
+  // leave thr->in_ignored_lib set.
   {
-    SCOPED_TSAN_INTERCEPTOR(longjmp, env, val);
+    SCOPED_INTERCEPTOR_RAW(longjmp, env, val);
   }
   LongJmp(cur_thread(), env);
   REAL(longjmp)(env, val);
@@ -590,7 +593,7 @@ TSAN_INTERCEPTOR(void, longjmp, uptr *en
 
 TSAN_INTERCEPTOR(void, siglongjmp, uptr *env, int val) {
   {
-    SCOPED_TSAN_INTERCEPTOR(siglongjmp, env, val);
+    SCOPED_INTERCEPTOR_RAW(siglongjmp, env, val);
   }
   LongJmp(cur_thread(), env);
   REAL(siglongjmp)(env, val);
@@ -1947,6 +1950,18 @@ static void CallUserSignalHandler(Thread
     bool sigact, int sig, my_siginfo_t *info, void *uctx) {
   if (acquire)
     Acquire(thr, 0, (uptr)&sigactions[sig]);
+  // Signals are generally asynchronous, so if we receive a signals when
+  // ignores are enabled we should disable ignores. This is critical for sync
+  // and interceptors, because otherwise we can miss syncronization and report
+  // false races.
+  int ignore_reads_and_writes = thr->ignore_reads_and_writes;
+  int ignore_interceptors = thr->ignore_interceptors;
+  int ignore_sync = thr->ignore_sync;
+  if (!ctx->after_multithreaded_fork) {
+    thr->ignore_reads_and_writes = 0;
+    thr->ignore_interceptors = 0;
+    thr->ignore_sync = 0;
+  }
   // Ensure that the handler does not spoil errno.
   const int saved_errno = errno;
   errno = 99;
@@ -1962,6 +1977,11 @@ static void CallUserSignalHandler(Thread
     else
       ((sighandler_t)pc)(sig);
   }
+  if (!ctx->after_multithreaded_fork) {
+    thr->ignore_reads_and_writes = ignore_reads_and_writes;
+    thr->ignore_interceptors = ignore_interceptors;
+    thr->ignore_sync = ignore_sync;
+  }
   // We do not detect errno spoiling for SIGTERM,
   // because some SIGTERM handlers do spoil errno but reraise SIGTERM,
   // tsan reports false positive in such case.
@@ -2033,11 +2053,8 @@ void ALWAYS_INLINE rtl_generic_sighandle
     if (sctx && atomic_load(&sctx->in_blocking_func, memory_order_relaxed)) {
       // 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;
       atomic_store(&sctx->in_blocking_func, 0, memory_order_relaxed);
       CallUserSignalHandler(thr, sync, true, sigact, sig, info, ctx);
-      thr->ignore_interceptors = i;
       atomic_store(&sctx->in_blocking_func, 1, memory_order_relaxed);
     } else {
       // Be very conservative with when we do acquire in this case.
@@ -2075,7 +2092,10 @@ static void rtl_sigaction(int sig, my_si
 }
 
 TSAN_INTERCEPTOR(int, sigaction, int sig, sigaction_t *act, sigaction_t *old) {
-  SCOPED_TSAN_INTERCEPTOR(sigaction, sig, act, old);
+  // Note: if we call REAL(sigaction) directly for any reason without proxying
+  // the signal handler through rtl_sigaction, very bad things will happen.
+  // The handler will run synchronously and corrupt tsan per-thread state.
+  SCOPED_INTERCEPTOR_RAW(sigaction, sig, act, old);
   if (old)
     internal_memcpy(old, &sigactions[sig], sizeof(*old));
   if (act == 0)

Added: compiler-rt/trunk/test/tsan/ignore_lib4.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/tsan/ignore_lib4.cc?rev=261658&view=auto
==============================================================================
--- compiler-rt/trunk/test/tsan/ignore_lib4.cc (added)
+++ compiler-rt/trunk/test/tsan/ignore_lib4.cc Tue Feb 23 11:16:26 2016
@@ -0,0 +1,41 @@
+// RUN: %clangxx_tsan -O1 %s -DLIB -fPIC -shared -o %T/libignore_lib4.so
+// RUN: %clangxx_tsan -O1 %s -o %t
+// RUN: %env_tsan_opts=suppressions='%s.supp' %run %t 2>&1 | FileCheck %s
+
+// Longjmp assembly has not been implemented for mips64 yet
+// XFAIL: mips64
+
+// Test longjmp in ignored lib.
+// It used to crash since we jumped out of ScopedInterceptor scope.
+
+#include "test.h"
+#include <setjmp.h>
+#include <string.h>
+#include <errno.h>
+#include <libgen.h>
+#include <string>
+
+#ifdef LIB
+
+extern "C" void myfunc() {
+  for (int i = 0; i < (1 << 20); i++) {
+    jmp_buf env;
+    if (!setjmp(env))
+      longjmp(env, 1);
+  }
+}
+
+#else
+
+int main(int argc, char **argv) {
+  std::string lib = std::string(dirname(argv[0])) + "/libignore_lib4.so";
+  void *h = dlopen(lib.c_str(), RTLD_GLOBAL | RTLD_NOW);
+  void (*func)() = (void(*)())dlsym(h, "myfunc");
+  func();
+  fprintf(stderr, "DONE\n");
+  return 0;
+}
+
+#endif
+
+// CHECK: DONE

Added: compiler-rt/trunk/test/tsan/ignore_lib4.cc.supp
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/tsan/ignore_lib4.cc.supp?rev=261658&view=auto
==============================================================================
--- compiler-rt/trunk/test/tsan/ignore_lib4.cc.supp (added)
+++ compiler-rt/trunk/test/tsan/ignore_lib4.cc.supp Tue Feb 23 11:16:26 2016
@@ -0,0 +1 @@
+called_from_lib:libignore_lib4.so

Added: compiler-rt/trunk/test/tsan/signal_sync2.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/tsan/signal_sync2.cc?rev=261658&view=auto
==============================================================================
--- compiler-rt/trunk/test/tsan/signal_sync2.cc (added)
+++ compiler-rt/trunk/test/tsan/signal_sync2.cc Tue Feb 23 11:16:26 2016
@@ -0,0 +1,77 @@
+// RUN: %clangxx_tsan -O1 %s -o %t && %run %t 2>&1 | FileCheck %s
+// UNSUPPORTED: darwin
+#include <pthread.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <signal.h>
+#include <sys/types.h>
+#include <sys/time.h>
+#include <unistd.h>
+#include <errno.h>
+
+// Test synchronization in signal handled within IgnoreSync region.
+
+extern "C" void AnnotateIgnoreSyncBegin(const char *f, int l);
+extern "C" void AnnotateIgnoreSyncEnd(const char *f, int l);
+
+const int kSignalCount = 500;
+
+__thread int process_signals;
+int signals_handled;
+int done;
+int ready[kSignalCount];
+long long data[kSignalCount];
+
+static void handler(int sig) {
+  if (!__atomic_load_n(&process_signals, __ATOMIC_RELAXED))
+    return;
+  int pos = signals_handled++;
+  if (pos >= kSignalCount)
+    return;
+  data[pos] = pos;
+  __atomic_store_n(&ready[pos], 1, __ATOMIC_RELEASE);
+}
+
+static void* thr(void *p) {
+  AnnotateIgnoreSyncBegin(__FILE__, __LINE__);
+  __atomic_store_n(&process_signals, 1, __ATOMIC_RELAXED);
+  while (!__atomic_load_n(&done, __ATOMIC_RELAXED)) {
+  }
+  AnnotateIgnoreSyncEnd(__FILE__, __LINE__);
+  return 0;
+}
+
+int main() {
+  struct sigaction act = {};
+  act.sa_handler = handler;
+  if (sigaction(SIGPROF, &act, 0)) {
+    perror("sigaction");
+    exit(1);
+  }
+  itimerval t;
+  t.it_value.tv_sec = 0;
+  t.it_value.tv_usec = 10;
+  t.it_interval = t.it_value;
+  if (setitimer(ITIMER_PROF, &t, 0)) {
+    perror("setitimer");
+    exit(1);
+  }
+
+  pthread_t th;
+  pthread_create(&th, 0, thr, 0);
+  for (int pos = 0; pos < kSignalCount; pos++) {
+    while (__atomic_load_n(&ready[pos], __ATOMIC_ACQUIRE) == 0) {
+    }
+    if (data[pos] != pos) {
+      printf("at pos %d, expect %d, got %lld\n", pos, pos, data[pos]);
+      exit(1);
+    }
+  }
+  __atomic_store_n(&done, 1, __ATOMIC_RELAXED);
+  pthread_join(th, 0);
+  fprintf(stderr, "DONE\n");
+  return 0;
+}
+
+// CHECK-NOT: WARNING: ThreadSanitizer:
+// CHECK: DONE




More information about the llvm-commits mailing list