[PATCH] D39619: Correct atexit(3) support in TSan/NetBSD

[Kamil Rytarowski via Phabricator via llvm-commits]

krytarowski added a comment.

Hmm, I might be doing something incorrectly, but there is a problem with Vector. We fire the destructor for this LIFO container.. before actually calling atexit(3) functions from libc.

  diff --git a/lib/tsan/rtl/tsan_interceptors.cc b/lib/tsan/rtl/tsan_interceptors.cc
  index 0b4e873a0..106a3fd0e 100644
  --- a/lib/tsan/rtl/tsan_interceptors.cc
  +++ b/lib/tsan/rtl/tsan_interceptors.cc
  @@ -391,7 +391,22 @@ struct AtExitCtx {
     void *arg;
   };
   
  -static void at_exit_wrapper(void *arg) {
  +Vector<struct AtExitCtx *> AtExitStack(MBlockAtExit);
  +
  +static void at_exit_wrapper() {
  +  ThreadState *thr = cur_thread();
  +  uptr pc = 0;
  +
  +  // Pop AtExitCtx from the top of the stack of callback functions
  +  AtExitCtx *ctx = AtExitStack[AtExitStack.Size() - 1];
  +  AtExitStack.PopBack();
  +
  +  Acquire(thr, pc, (uptr)ctx);
  +  ((void(*)())ctx->f)();
  +  InternalFree(ctx);
  +}
  +
  +static void cxa_at_exit_wrapper(void *arg) {
     ThreadState *thr = cur_thread();
     uptr pc = 0;
     Acquire(thr, pc, (uptr)arg);
  @@ -430,7 +445,18 @@ static int setup_at_exit_wrapper(ThreadState *thr, uptr pc, void(*f)(),
     // 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);
  +  int res;
  +  if (dso == 0) {
  +    // NetBSD does not preserve the 2nd argument if dso is equal to 0
  +    // Store ctx in a local stack-like structure (LIFO order)
  +    res = REAL(__cxa_atexit)((void (*)(void *a))at_exit_wrapper, 0, 0);
  +    // Push AtExitCtx on the top of the stack of callback functions
  +    if (res == 0) {
  +      AtExitStack.PushBack(ctx);
  +    }
  +  } else {
  +    res = REAL(__cxa_atexit)(cxa_at_exit_wrapper, ctx, dso);
  +  }
     ThreadIgnoreEnd(thr, pc);
     return res;
   }

This means that we need to use `internal_malloc()` like in the proposed patch.


Repository:
  rL LLVM

https://reviews.llvm.org/D39619