<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Feb 23, 2016 at 9:16 AM, Dmitry Vyukov via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Author: dvyukov<br>
Date: Tue Feb 23 11:16:26 2016<br>
New Revision: 261658<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=261658&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=261658&view=rev</a><br>
Log:<br>
tsan: fix signal handling in ignored libraries<br>
<br>
The first issue is that we longjmp from ScopedInterceptor scope<br>
when called from an ignored lib. This leaves thr->in_ignored_lib set.<br>
This, in turn, disables handling of sigaction. This, in turn,<br>
corrupts tsan state since signals delivered asynchronously.<br>
Another issue is that we can ignore synchronization in asignal<br>
handler, if the signal is delivered into an IgnoreSync region.<br>
Since signals are generally asynchronous, they should ignore<br>
memory access/synchronization/interceptor ignores.<br>
This could lead to false positives in signal handlers.<br>
<br>
<br>
Added:<br>
compiler-rt/trunk/test/tsan/ignore_lib4.cc<br>
compiler-rt/trunk/test/tsan/ignore_lib4.cc.supp<br>
compiler-rt/trunk/test/tsan/signal_sync2.cc<br>
Modified:<br>
compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc<br>
<br>
Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc?rev=261658&r1=261657&r2=261658&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc?rev=261658&r1=261657&r2=261658&view=diff</a><br>
==============================================================================<br>
--- compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc (original)<br>
+++ compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc Tue Feb 23 11:16:26 2016<br>
@@ -581,8 +581,11 @@ DEFINE_REAL(int, __sigsetjmp, void *env)<br>
#endif // SANITIZER_MAC<br>
<br>
TSAN_INTERCEPTOR(void, longjmp, uptr *env, int val) {<br>
+ // Note: if we call REAL(longjmp) in the context of ScopedInterceptor,<br>
+ // bad things will happen. We will jump over ScopedInterceptor dtor and can<br>
+ // leave thr->in_ignored_lib set.<br>
{<br>
- SCOPED_TSAN_INTERCEPTOR(longjmp, env, val);<br>
+ SCOPED_INTERCEPTOR_RAW(longjmp, env, val);<br>
}<br>
LongJmp(cur_thread(), env);<br>
REAL(longjmp)(env, val);<br>
@@ -590,7 +593,7 @@ TSAN_INTERCEPTOR(void, longjmp, uptr *en<br>
<br>
TSAN_INTERCEPTOR(void, siglongjmp, uptr *env, int val) {<br>
{<br>
- SCOPED_TSAN_INTERCEPTOR(siglongjmp, env, val);<br>
+ SCOPED_INTERCEPTOR_RAW(siglongjmp, env, val);<br>
}<br>
LongJmp(cur_thread(), env);<br>
REAL(siglongjmp)(env, val);<br>
@@ -1947,6 +1950,18 @@ static void CallUserSignalHandler(Thread<br>
bool sigact, int sig, my_siginfo_t *info, void *uctx) {<br>
if (acquire)<br>
Acquire(thr, 0, (uptr)&sigactions[sig]);<br>
+ // Signals are generally asynchronous, so if we receive a signals when<br>
+ // ignores are enabled we should disable ignores. This is critical for sync<br>
+ // and interceptors, because otherwise we can miss syncronization and report<br>
+ // false races.<br>
+ int ignore_reads_and_writes = thr->ignore_reads_and_writes;<br>
+ int ignore_interceptors = thr->ignore_interceptors;<br>
+ int ignore_sync = thr->ignore_sync;<br>
+ if (!ctx->after_multithreaded_fork) {<br>
+ thr->ignore_reads_and_writes = 0;<br>
+ thr->ignore_interceptors = 0;<br>
+ thr->ignore_sync = 0;<br>
+ }<br>
// Ensure that the handler does not spoil errno.<br>
const int saved_errno = errno;<br>
errno = 99;<br>
@@ -1962,6 +1977,11 @@ static void CallUserSignalHandler(Thread<br>
else<br>
((sighandler_t)pc)(sig);<br>
}<br>
+ if (!ctx->after_multithreaded_fork) {<br>
+ thr->ignore_reads_and_writes = ignore_reads_and_writes;<br>
+ thr->ignore_interceptors = ignore_interceptors;<br>
+ thr->ignore_sync = ignore_sync;<br>
+ }<br>
// We do not detect errno spoiling for SIGTERM,<br>
// because some SIGTERM handlers do spoil errno but reraise SIGTERM,<br>
// tsan reports false positive in such case.<br>
@@ -2033,11 +2053,8 @@ void ALWAYS_INLINE rtl_generic_sighandle<br>
if (sctx && atomic_load(&sctx->in_blocking_func, memory_order_relaxed)) {<br>
// We ignore interceptors in blocking functions,<br>
// temporary enbled them again while we are calling user function.<br></blockquote><div>^^</div><div>This comment is now obsolete?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
- int const i = thr->ignore_interceptors;<br>
- thr->ignore_interceptors = 0;<br>
atomic_store(&sctx->in_blocking_func, 0, memory_order_relaxed);<br>
CallUserSignalHandler(thr, sync, true, sigact, sig, info, ctx);<br>
- thr->ignore_interceptors = i;<br>
atomic_store(&sctx->in_blocking_func, 1, memory_order_relaxed);<br>
} else {<br>
// Be very conservative with when we do acquire in this case.<br>
@@ -2075,7 +2092,10 @@ static void rtl_sigaction(int sig, my_si<br>
}<br>
<br>
TSAN_INTERCEPTOR(int, sigaction, int sig, sigaction_t *act, sigaction_t *old) {<br>
- SCOPED_TSAN_INTERCEPTOR(sigaction, sig, act, old);<br>
+ // Note: if we call REAL(sigaction) directly for any reason without proxying<br>
+ // the signal handler through rtl_sigaction, very bad things will happen.<br>
+ // The handler will run synchronously and corrupt tsan per-thread state.<br>
+ SCOPED_INTERCEPTOR_RAW(sigaction, sig, act, old);<br>
if (old)<br>
internal_memcpy(old, &sigactions[sig], sizeof(*old));<br>
if (act == 0)<br>
<br>
Added: compiler-rt/trunk/test/tsan/ignore_lib4.cc<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/tsan/ignore_lib4.cc?rev=261658&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/tsan/ignore_lib4.cc?rev=261658&view=auto</a><br>
==============================================================================<br>
--- compiler-rt/trunk/test/tsan/ignore_lib4.cc (added)<br>
+++ compiler-rt/trunk/test/tsan/ignore_lib4.cc Tue Feb 23 11:16:26 2016<br>
@@ -0,0 +1,41 @@<br>
+// RUN: %clangxx_tsan -O1 %s -DLIB -fPIC -shared -o %T/libignore_lib4.so<br>
+// RUN: %clangxx_tsan -O1 %s -o %t<br>
+// RUN: %env_tsan_opts=suppressions='%s.supp' %run %t 2>&1 | FileCheck %s<br></blockquote><div><br></div><div>Nit: using </div><div>echo "called_from_lib:libignore_lib4.so" > %t.supp<br></div><div>and removing extra .supp file would make this test more self-contained.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+<br>
+// Longjmp assembly has not been implemented for mips64 yet<br>
+// XFAIL: mips64<br></blockquote><div><br></div><div>Does UNSUPPORTED: mips64 work here?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+<br>
+// Test longjmp in ignored lib.<br>
+// It used to crash since we jumped out of ScopedInterceptor scope.<br>
+<br>
+#include "test.h"<br>
+#include <setjmp.h><br>
+#include <string.h><br>
+#include <errno.h><br>
+#include <libgen.h><br>
+#include <string><br>
+<br>
+#ifdef LIB<br>
+<br>
+extern "C" void myfunc() {<br>
+ for (int i = 0; i < (1 << 20); i++) {<br>
+ jmp_buf env;<br>
+ if (!setjmp(env))<br>
+ longjmp(env, 1);<br>
+ }<br>
+}<br>
+<br>
+#else<br>
+<br>
+int main(int argc, char **argv) {<br>
+ std::string lib = std::string(dirname(argv[0])) + "/libignore_lib4.so";<br>
+ void *h = dlopen(lib.c_str(), RTLD_GLOBAL | RTLD_NOW);<br>
+ void (*func)() = (void(*)())dlsym(h, "myfunc");<br>
+ func();<br>
+ fprintf(stderr, "DONE\n");<br>
+ return 0;<br>
+}<br>
+<br>
+#endif<br>
+<br>
+// CHECK: DONE<br>
<br>
Added: compiler-rt/trunk/test/tsan/ignore_lib4.cc.supp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/tsan/ignore_lib4.cc.supp?rev=261658&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/tsan/ignore_lib4.cc.supp?rev=261658&view=auto</a><br>
==============================================================================<br>
--- compiler-rt/trunk/test/tsan/ignore_lib4.cc.supp (added)<br>
+++ compiler-rt/trunk/test/tsan/ignore_lib4.cc.supp Tue Feb 23 11:16:26 2016<br>
@@ -0,0 +1 @@<br>
+called_from_lib:libignore_lib4.so<br>
<br>
Added: compiler-rt/trunk/test/tsan/signal_sync2.cc<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/tsan/signal_sync2.cc?rev=261658&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/tsan/signal_sync2.cc?rev=261658&view=auto</a><br>
==============================================================================<br>
--- compiler-rt/trunk/test/tsan/signal_sync2.cc (added)<br>
+++ compiler-rt/trunk/test/tsan/signal_sync2.cc Tue Feb 23 11:16:26 2016<br>
@@ -0,0 +1,77 @@<br>
+// RUN: %clangxx_tsan -O1 %s -o %t && %run %t 2>&1 | FileCheck %s<br>
+// UNSUPPORTED: darwin<br>
+#include <pthread.h><br>
+#include <stdio.h><br>
+#include <stdlib.h><br>
+#include <signal.h><br>
+#include <sys/types.h><br>
+#include <sys/time.h><br>
+#include <unistd.h><br>
+#include <errno.h><br>
+<br>
+// Test synchronization in signal handled within IgnoreSync region.<br>
+<br>
+extern "C" void AnnotateIgnoreSyncBegin(const char *f, int l);<br>
+extern "C" void AnnotateIgnoreSyncEnd(const char *f, int l);<br>
+<br>
+const int kSignalCount = 500;<br>
+<br>
+__thread int process_signals;<br>
+int signals_handled;<br>
+int done;<br>
+int ready[kSignalCount];<br>
+long long data[kSignalCount];<br>
+<br>
+static void handler(int sig) {<br>
+ if (!__atomic_load_n(&process_signals, __ATOMIC_RELAXED))<br>
+ return;<br>
+ int pos = signals_handled++;<br>
+ if (pos >= kSignalCount)<br>
+ return;<br>
+ data[pos] = pos;<br>
+ __atomic_store_n(&ready[pos], 1, __ATOMIC_RELEASE);<br>
+}<br>
+<br>
+static void* thr(void *p) {<br>
+ AnnotateIgnoreSyncBegin(__FILE__, __LINE__);<br>
+ __atomic_store_n(&process_signals, 1, __ATOMIC_RELAXED);<br>
+ while (!__atomic_load_n(&done, __ATOMIC_RELAXED)) {<br>
+ }<br>
+ AnnotateIgnoreSyncEnd(__FILE__, __LINE__);<br>
+ return 0;<br>
+}<br>
+<br>
+int main() {<br>
+ struct sigaction act = {};<br>
+ act.sa_handler = handler;<br>
+ if (sigaction(SIGPROF, &act, 0)) {<br>
+ perror("sigaction");<br>
+ exit(1);<br>
+ }<br>
+ itimerval t;<br>
+ t.it_value.tv_sec = 0;<br>
+ t.it_value.tv_usec = 10;<br>
+ t.it_interval = t.it_value;<br>
+ if (setitimer(ITIMER_PROF, &t, 0)) {<br>
+ perror("setitimer");<br>
+ exit(1);<br>
+ }<br>
+<br>
+ pthread_t th;<br>
+ pthread_create(&th, 0, thr, 0);<br>
+ for (int pos = 0; pos < kSignalCount; pos++) {<br>
+ while (__atomic_load_n(&ready[pos], __ATOMIC_ACQUIRE) == 0) {<br>
+ }<br>
+ if (data[pos] != pos) {<br>
+ printf("at pos %d, expect %d, got %lld\n", pos, pos, data[pos]);<br>
+ exit(1);<br>
+ }<br>
+ }<br>
+ __atomic_store_n(&done, 1, __ATOMIC_RELAXED);<br>
+ pthread_join(th, 0);<br>
+ fprintf(stderr, "DONE\n");<br>
+ return 0;<br>
+}<br>
+<br>
+// CHECK-NOT: WARNING: ThreadSanitizer:<br>
+// CHECK: DONE<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature"><div dir="ltr">Alexey Samsonov<br><a href="mailto:vonosmas@gmail.com" target="_blank">vonosmas@gmail.com</a></div></div>
</div></div>