From b2be09802638802deebace5237f1d804977517d0 Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Tue, 23 Feb 2016 17:16:26 +0000 Subject: [PATCH] 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. llvm-svn: 261658 --- compiler-rt/lib/tsan/rtl/tsan_interceptors.cc | 32 ++++++++--- compiler-rt/test/tsan/ignore_lib4.cc | 41 ++++++++++++++ compiler-rt/test/tsan/ignore_lib4.cc.supp | 1 + compiler-rt/test/tsan/signal_sync2.cc | 77 +++++++++++++++++++++++++++ 4 files changed, 145 insertions(+), 6 deletions(-) create mode 100644 compiler-rt/test/tsan/ignore_lib4.cc create mode 100644 compiler-rt/test/tsan/ignore_lib4.cc.supp create mode 100644 compiler-rt/test/tsan/signal_sync2.cc diff --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc b/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc index f9d5b0f..e3f2c53 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc +++ b/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc @@ -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 *env, int val) { 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(ThreadState *thr, bool sync, bool acquire, 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(ThreadState *thr, bool sync, bool acquire, 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_sighandler(bool sigact, int sig, 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_siginfo_t *info, void *ctx) { } 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) diff --git a/compiler-rt/test/tsan/ignore_lib4.cc b/compiler-rt/test/tsan/ignore_lib4.cc new file mode 100644 index 0000000..3ee34f3 --- /dev/null +++ b/compiler-rt/test/tsan/ignore_lib4.cc @@ -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 +#include +#include +#include +#include + +#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 diff --git a/compiler-rt/test/tsan/ignore_lib4.cc.supp b/compiler-rt/test/tsan/ignore_lib4.cc.supp new file mode 100644 index 0000000..3a94c58 --- /dev/null +++ b/compiler-rt/test/tsan/ignore_lib4.cc.supp @@ -0,0 +1 @@ +called_from_lib:libignore_lib4.so diff --git a/compiler-rt/test/tsan/signal_sync2.cc b/compiler-rt/test/tsan/signal_sync2.cc new file mode 100644 index 0000000..163f206 --- /dev/null +++ b/compiler-rt/test/tsan/signal_sync2.cc @@ -0,0 +1,77 @@ +// RUN: %clangxx_tsan -O1 %s -o %t && %run %t 2>&1 | FileCheck %s +// UNSUPPORTED: darwin +#include +#include +#include +#include +#include +#include +#include +#include + +// 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 -- 2.7.4