Correct atexit(3) support in TSan/NetBSD

Summary:
The NetBSD specific implementation of cxa_atexit() does not
preserve the 2nd argument if dso is equal to NULL.

Changes:

 - Split paths of handling intercepted __cxa_atexit() and atexit(3).
   This affects all supported Operating Systems.
 - Add a local stack-like structure to hold the __cxa_atexit() context.
   atexit(3) is documented in the C standard as calling callback from the
   earliest to the oldest entry. This path also fixes potential ABI
   problem of passing an argument to a function from the atexit(3)
   callback mechanism.
 - Add new test to ensure LIFO style of atexit(3) callbacks: atexit3.cc

Proposal to change the behavior of __cxa_atexit() in NetBSD has been rejected.

With the above changes TSan/NetBSD with the current tsan_interceptors.cc
can bootstrap into operation.

Sponsored by <The NetBSD Foundation>

Reviewers: vitalybuka, dvyukov, joerg, kcc, eugenis

Reviewed By: dvyukov

Subscribers: kubamracek, llvm-commits, #sanitizers

Tags: #sanitizers

Differential Revision: https://reviews.llvm.org/D39619

llvm-svn: 317735

compiler-rt/lib/tsan/rtl/tsan_interceptors.cc

@@ -231,6 +231,13 @@ struct ThreadSignalContext {
   __sanitizer_sigset_t oldset;
 };
 
+// The sole reason tsan wraps atexit callbacks is to establish synchronization
+// between callback setup and callback execution.
+struct AtExitCtx {
+  void (*f)();
+  void *arg;
+};
+
 // InterceptorContext holds all global data required for interceptors.
 // It's explicitly constructed in InitializeInterceptors with placement new
 // and is never destroyed. This allows usage of members with non-trivial
@@ -244,8 +251,11 @@ struct InterceptorContext {
   unsigned finalize_key;
 #endif
 
+  BlockingMutex atexit_mu;
+  Vector<struct AtExitCtx *> AtExitStack;
+
   InterceptorContext()
-      : libignore(LINKER_INITIALIZED) {
+      : libignore(LINKER_INITIALIZED), AtExitStack(MBlockAtExit) {
   }
 };
 
@@ -398,17 +408,25 @@ TSAN_INTERCEPTOR(int, pause, int fake) {
   return BLOCK_REAL(pause)(fake);
 }
 
-// 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() {
+  AtExitCtx *ctx;
+  {
+    // Ensure thread-safety.
+    BlockingMutexLock l(&interceptor_ctx()->atexit_mu);
 
-static void at_exit_wrapper(void *arg) {
-  ThreadState *thr = cur_thread();
-  uptr pc = 0;
-  Acquire(thr, pc, (uptr)arg);
+    // Pop AtExitCtx from the top of the stack of callback functions
+    uptr element = interceptor_ctx()->AtExitStack.Size() - 1;
+    ctx = interceptor_ctx()->AtExitStack[element];
+    interceptor_ctx()->AtExitStack.PopBack();
+  }
+
+  Acquire(cur_thread(), (uptr)0, (uptr)ctx);
+  ((void(*)())ctx->f)();
+  InternalFree(ctx);
+}
+
+static void cxa_at_exit_wrapper(void *arg) {
+  Acquire(cur_thread(), 0, (uptr)arg);
   AtExitCtx *ctx = (AtExitCtx*)arg;
   ((void(*)(void *arg))ctx->f)(ctx->arg);
   InternalFree(ctx);
@@ -444,7 +462,22 @@ 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) {
+    // NetBSD does not preserve the 2nd argument if dso is equal to 0
+    // Store ctx in a local stack-like structure
+
+    // Ensure thread-safety.
+    BlockingMutexLock l(&interceptor_ctx()->atexit_mu);
+
+    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) {
+      interceptor_ctx()->AtExitStack.PushBack(ctx);
+    }
+  } else {
+    res = REAL(__cxa_atexit)(cxa_at_exit_wrapper, ctx, dso);
+  }
   ThreadIgnoreEnd(thr, pc);
   return res;
 }

compiler-rt/test/tsan/atexit3.cc

@@ -0,0 +1,41 @@
+// RUN: %clang_tsan -O1 %s -o %t && %run %t 2>&1 | FileCheck %s
+
+#include <stdio.h>
+#include <stdlib.h>
+
+static void atexit5() {
+  fprintf(stderr, "5");
+}
+
+static void atexit4() {
+  fprintf(stderr, "4");
+}
+
+static void atexit3() {
+  fprintf(stderr, "3");
+}
+
+static void atexit2() {
+  fprintf(stderr, "2");
+}
+
+static void atexit1() {
+  fprintf(stderr, "1");
+}
+
+static void atexit0() {
+  fprintf(stderr, "\n");
+}
+
+int main() {
+  atexit(atexit0);
+  atexit(atexit1);
+  atexit(atexit2);
+  atexit(atexit3);
+  atexit(atexit4);
+  atexit(atexit5);
+}
+
+// CHECK-NOT: FATAL: ThreadSanitizer
+// CHECK-NOT: WARNING: ThreadSanitizer
+// CHECK: 54321