<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Oct 14, 2014 at 2:32 AM, Dmitry Vyukov <span dir="ltr"><<a href="mailto:dvyukov@google.com" target="_blank">dvyukov@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: dvyukov<br>
Date: Tue Oct 14 04:32:45 2014<br>
New Revision: 219675<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=219675&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=219675&view=rev</a><br>
Log:<br>
tsan: refactor atexit handling<br>
The current handling (manual execution of atexit callbacks)<br>
is overly complex and leads to constant problems due to mutual ordering of callbacks.<br>
Instead simply wrap callbacks into our wrapper to establish<br>
the necessary synchronization.<br>
Fixes issue <a href="https://code.google.com/p/thread-sanitizer/issues/detail?id=80" target="_blank">https://code.google.com/p/thread-sanitizer/issues/detail?id=80</a><br>
<br>
<br>
Added:<br>
compiler-rt/trunk/test/tsan/dlclose.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=219675&r1=219674&r2=219675&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc?rev=219675&r1=219674&r2=219675&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 Oct 14 04:32:45 2014<br>
@@ -105,11 +105,6 @@ typedef void (*sighandler_t)(int sig);<br>
<br>
#define errno (*__errno_location())<br>
<br>
-// 16K loaded modules should be enough for everyone.<br>
-static const uptr kMaxModules = 1 << 14;<br>
-static LoadedModule *modules;<br>
-static uptr nmodules;<br>
-<br>
struct sigaction_t {<br>
union {<br>
sighandler_t sa_handler;<br>
@@ -289,6 +284,7 @@ TSAN_INTERCEPTOR(int, nanosleep, void *r<br>
return res;<br>
}<br>
<br>
+/*<br>
class AtExitContext {<br>
public:<br>
AtExitContext()<br>
@@ -346,6 +342,26 @@ class AtExitContext {<br>
};<br>
<br>
static AtExitContext *atexit_ctx;<br>
+*/<br></blockquote><div><br></div><div>^^^</div><div>Is there a reason you've left an old implementation of AtExitContext commented out, or we can just nuke it?</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
+// The sole reason tsan wraps atexit callbacks is to establish synchronization<br>
+// between callback setup and callback execution.<br>
+struct AtExitCtx {<br>
+ void (*f)();<br>
+ void *arg;<br>
+};<br>
+<br>
+static void at_exit_wrapper(void *arg) {<br>
+ ThreadState *thr = cur_thread();<br>
+ uptr pc = 0;<br>
+ Acquire(thr, pc, (uptr)arg);<br>
+ AtExitCtx *ctx = (AtExitCtx*)arg;<br>
+ ((void(*)(void *arg))ctx->f)(ctx->arg);<br>
+ __libc_free(ctx);<br>
+}<br>
+<br>
+static int setup_at_exit_wrapper(ThreadState *thr, uptr pc, void(*f)(),<br>
+ void *arg, void *dso);<br>
<br>
TSAN_INTERCEPTOR(int, atexit, void (*f)()) {<br>
if (cur_thread()->in_symbolizer)<br>
@@ -353,40 +369,51 @@ TSAN_INTERCEPTOR(int, atexit, void (*f)(<br>
// We want to setup the atexit callback even if we are in ignored lib<br>
// or after fork.<br>
SCOPED_INTERCEPTOR_RAW(atexit, f);<br>
- return atexit_ctx->atexit(thr, pc, false, (void(*)())f, 0, 0);<br>
+ return setup_at_exit_wrapper(thr, pc, (void(*)())f, 0, 0);<br>
}<br>
<br>
-TSAN_INTERCEPTOR(int, on_exit, void(*f)(int, void*), void *arg) {<br>
+TSAN_INTERCEPTOR(int, __cxa_atexit, void (*f)(void *a), void *arg, void *dso) {<br>
if (cur_thread()->in_symbolizer)<br>
return 0;<br>
- SCOPED_TSAN_INTERCEPTOR(on_exit, f, arg);<br>
- return atexit_ctx->atexit(thr, pc, true, (void(*)())f, arg, 0);<br>
+ SCOPED_TSAN_INTERCEPTOR(__cxa_atexit, f, arg, dso);<br>
+ return setup_at_exit_wrapper(thr, pc, (void(*)())f, arg, dso);<br>
}<br>
<br>
-bool IsSaticModule(void *dso) {<br>
- if (modules == 0)<br>
- return false;<br>
- for (uptr i = 0; i < nmodules; i++) {<br>
- if (modules[i].containsAddress((uptr)dso))<br>
- return true;<br>
- }<br>
- return false;<br>
+static int setup_at_exit_wrapper(ThreadState *thr, uptr pc, void(*f)(),<br>
+ void *arg, void *dso) {<br>
+ AtExitCtx *ctx = (AtExitCtx*)__libc_malloc(sizeof(AtExitCtx));<br>
+ ctx->f = f;<br>
+ ctx->arg = arg;<br>
+ Release(thr, pc, (uptr)ctx);<br>
+ // Memory allocation in __cxa_atexit will race with free during exit,<br>
+ // because we do not see synchronization around atexit callback list.<br>
+ ThreadIgnoreBegin(thr, pc);<br>
+ int res = REAL(__cxa_atexit)(at_exit_wrapper, ctx, dso);<br>
+ ThreadIgnoreEnd(thr, pc);<br>
+ return res;<br>
}<br>
<br>
-TSAN_INTERCEPTOR(int, __cxa_atexit, void (*f)(void *a), void *arg, void *dso) {<br>
+static void on_exit_wrapper(int status, void *arg) {<br>
+ ThreadState *thr = cur_thread();<br>
+ uptr pc = 0;<br>
+ Acquire(thr, pc, (uptr)arg);<br>
+ AtExitCtx *ctx = (AtExitCtx*)arg;<br>
+ ((void(*)(int status, void *arg))ctx->f)(status, ctx->arg);<br>
+ __libc_free(ctx);<br>
+}<br>
+<br>
+TSAN_INTERCEPTOR(int, on_exit, void(*f)(int, void*), void *arg) {<br>
if (cur_thread()->in_symbolizer)<br>
return 0;<br>
- SCOPED_TSAN_INTERCEPTOR(__cxa_atexit, f, arg, dso);<br>
- // If it's the main executable or a statically loaded library,<br>
- // we will call the callback.<br>
- if (dso == 0 || IsSaticModule(dso))<br>
- return atexit_ctx->atexit(thr, pc, false, (void(*)())f, arg, dso);<br>
-<br>
- // Dynamically load module, don't know when to call the callback for it.<br>
+ SCOPED_TSAN_INTERCEPTOR(on_exit, f, arg);<br>
+ AtExitCtx *ctx = (AtExitCtx*)__libc_malloc(sizeof(AtExitCtx));<br>
+ ctx->f = (void(*)())f;<br>
+ ctx->arg = arg;<br>
+ Release(thr, pc, (uptr)ctx);<br>
// Memory allocation in __cxa_atexit will race with free during exit,<br>
// because we do not see synchronization around atexit callback list.<br>
ThreadIgnoreBegin(thr, pc);<br>
- int res = REAL(__cxa_atexit)(f, arg, dso);<br>
+ int res = REAL(on_exit)(on_exit_wrapper, ctx);<br>
ThreadIgnoreEnd(thr, pc);<br>
return res;<br>
}<br>
@@ -2244,8 +2271,6 @@ namespace __tsan {<br>
<br>
static void finalize(void *arg) {<br>
ThreadState *thr = cur_thread();<br>
- uptr pc = 0;<br>
- atexit_ctx->exit(thr, pc);<br>
int status = Finalize(thr);<br>
// Make sure the output is not lost.<br>
// Flushing all the streams here may freeze the process if a child thread is<br>
@@ -2433,9 +2458,6 @@ void InitializeInterceptors() {<br>
// Need to setup it, because interceptors check that the function is resolved.<br>
// But atexit is emitted directly into the module, so can't be resolved.<br>
REAL(atexit) = (int(*)(void(*)()))unreachable;<br>
- atexit_ctx = new(internal_alloc(MBlockAtExit, sizeof(AtExitContext)))<br>
- AtExitContext();<br>
-<br>
if (REAL(__cxa_atexit)(&finalize, 0, 0)) {<br>
Printf("ThreadSanitizer: failed to setup atexit callback\n");<br>
Die();<br>
@@ -2447,11 +2469,6 @@ void InitializeInterceptors() {<br>
}<br>
<br>
FdInit();<br>
-<br>
- // Remember list of loaded libraries for atexit interceptors.<br>
- modules = (LoadedModule*)MmapOrDie(sizeof(*modules)*kMaxModules,<br>
- "LoadedModule");<br>
- nmodules = GetListOfModules(modules, kMaxModules, 0);<br>
}<br>
<br>
void *internal_start_thread(void(*func)(void *arg), void *arg) {<br>
<br>
Added: compiler-rt/trunk/test/tsan/dlclose.cc<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/tsan/dlclose.cc?rev=219675&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/tsan/dlclose.cc?rev=219675&view=auto</a><br>
==============================================================================<br>
--- compiler-rt/trunk/test/tsan/dlclose.cc (added)<br>
+++ compiler-rt/trunk/test/tsan/dlclose.cc Tue Oct 14 04:32:45 2014<br>
@@ -0,0 +1,56 @@<br>
+// RUN: %clangxx_tsan -O1 %s -DBUILD_SO -fPIC -shared -o %t-so.so<br>
+// RUN: %clangxx_tsan -O1 %s -o %t && %run %t 2>&1 | FileCheck %s<br>
+<br>
+// Test case for<br>
+// <a href="https://code.google.com/p/thread-sanitizer/issues/detail?id=80" target="_blank">https://code.google.com/p/thread-sanitizer/issues/detail?id=80</a><br>
+<br>
+#ifdef BUILD_SO<br>
+<br>
+#include <stdio.h><br>
+<br>
+extern "C"<br>
+void sofunc() {<br>
+ fprintf(stderr, "HELLO FROM SO\n");<br>
+}<br>
+<br>
+#else // BUILD_SO<br>
+<br>
+#include <dlfcn.h><br>
+#include <stdio.h><br>
+#include <stddef.h><br>
+#include <unistd.h><br>
+#include <string><br>
+<br>
+void *lib;<br>
+void *lib2;<br>
+<br>
+struct Closer {<br>
+ ~Closer() {<br>
+ dlclose(lib);<br>
+ fprintf(stderr, "CLOSED SO\n");<br>
+ }<br>
+};<br>
+static Closer c;<br>
+<br>
+int main(int argc, char *argv[]) {<br>
+ lib = dlopen((std::string(argv[0]) + std::string("-so.so")).c_str(),<br>
+ RTLD_NOW|RTLD_NODELETE);<br>
+ if (lib == 0) {<br>
+ printf("error in dlopen: %s\n", dlerror());<br>
+ return 1;<br>
+ }<br>
+ void *f = dlsym(lib, "sofunc");<br>
+ if (f == 0) {<br>
+ printf("error in dlsym: %s\n", dlerror());<br>
+ return 1;<br>
+ }<br>
+ ((void(*)())f)();<br>
+ return 0;<br>
+}<br>
+<br>
+#endif // BUILD_SO<br>
+<br>
+// CHECK: HELLO FROM SO<br>
+// CHECK-NOT: Inconsistency detected by ld.so<br>
+// CHECK: CLOSED SO<br>
+<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div dir="ltr">Alexey Samsonov<br><a href="mailto:vonosmas@gmail.com" target="_blank">vonosmas@gmail.com</a></div>
</div></div>