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