[compiler-rt] r219675 - tsan: refactor atexit handling

Alexey Samsonov vonosmas at gmail.com
Tue Oct 14 11:00:13 PDT 2014


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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141014/860a7cdd/attachment.html>


More information about the llvm-commits mailing list