[compiler-rt] r261658 - tsan: fix signal handling in ignored libraries
Alexey Samsonov via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 23 09:51:06 PST 2016
On Tue, Feb 23, 2016 at 9:16 AM, Dmitry Vyukov via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> 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.
>
^^
This comment is now obsolete?
> - 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
>
Nit: using
echo "called_from_lib:libignore_lib4.so" > %t.supp
and removing extra .supp file would make this test more self-contained.
> +
> +// Longjmp assembly has not been implemented for mips64 yet
> +// XFAIL: mips64
>
Does UNSUPPORTED: mips64 work here?
> +
> +// 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
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
--
Alexey Samsonov
vonosmas at gmail.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160223/7a6b16eb/attachment.html>
More information about the llvm-commits
mailing list