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

[Dmitry Vyukov via Phabricator via llvm-commits]

dvyukov added inline comments.


================
Comment at: lib/tsan/rtl/tsan_interceptors.cc:413
+  // Pop AtExitCtx from the top of the stack of callback functions
+  uptr element = interceptor_ctx()->AtExitStack.Size() - 1;
+  AtExitCtx *ctx = interceptor_ctx()->AtExitStack[element];
----------------
dvyukov wrote:
> krytarowski wrote:
> > dvyukov wrote:
> > > This also needs the mutex for 2 reasons:
> > > 1. It can race with setup_at_exit_wrapper in other threads.
> > > 2. You push onto the stack _after_ calling __cxa_atexit, so if another thread calls exit in between, at_exit_wrapper can see an empty stack.
> > atexit(3) are global, not per-thread.
> > 
> > I'm not convinced that races for 1 can impact execution of `at_exit_wrapper()`.
> > 
> > I agree that someone might try to keep registering new atexit(3) callbacks inside atexit(3) callback functions. I don't know whether this behavior is even defined or real-world.  I think that function-scoped mutex can cause deadlock with callback registration mutex, so I will protect only the access to `AtExitStack`.
> > I'm not convinced that races for 1 can impact execution of at_exit_wrapper().
> 
> Why? One thread executes callbacks, another registers new callbacks. AtExitStack is corrupted as the result. Everything explodes.
> 
> > I agree that someone might try to keep registering new atexit(3) callbacks inside atexit(3) callback functions. 
> 
> That's less of an issue. The problem are other threads that know nothing about process existing yet and continue registering callbacks.
And the second problem is: thread registers callback with __cxa_atexit but don't push onto AtExitStack yes; another thread calls exit and at_exit_wrapper() discovers empty AtExitStack.


Repository:
  rL LLVM

https://reviews.llvm.org/D39619