[compiler-rt] r261658 - tsan: fix signal handling in ignored libraries
Dmitry Vyukov via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 23 10:03:43 PST 2016
On Tue, Feb 23, 2016 at 6:51 PM, Alexey Samsonov <vonosmas at gmail.com> wrote:
>
> 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.
Done in r261660.
>> +
>> +// Longjmp assembly has not been implemented for mips64 yet
>> +// XFAIL: mips64
>
>
> Does UNSUPPORTED: mips64 work here?
Dunno. I used that's used in other tests.
Maybe they want the test at least to compile.
>> +
>> +// 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
More information about the llvm-commits
mailing list