tsan: clean up dynamic TLS memory between reuse

[dvyukov]

Summary

Currently the added test produces false race reports with glibc 2.19,
because DLTS memory is reused by pthread under the hood.

Use the DTLS machinery to intercept new DTLS ranges.
__tls_get_addr known to cause issues for tsan in the past,
so write the interceptor more carefully.

[kcc]

lib/tsan/rtl/tsan_rtl_thread.cc

140 ↗

(On Diff #59426)

Please, can you stop adding #ifdefs?

[dvyukov]

lib/tsan/rtl/tsan_rtl_thread.cc

140 ↗

(On Diff #59426)

Done

[kcc]

LGTM, assuming the test fails w/o your patch and passes with it.

[Eugene.Zelenko]

Committed in r271568.

lib/tsan/rtl/tsan_flags.cc

if (kGoMode) {
       cf.abort_on_error = false;
       // Go does not have mutexes.
     } else {
       cf.detect_deadlocks = true;
     }
     cf.print_suppressions = false;
     cf.stack_trace_format = "    #%n %f %S %M";
     cf.exitcode = 66;
+    cf.intercept_tls_get_addr = true;
     OverrideCommonFlags(cf);
   }
 
   f->SetDefaults();
 
   FlagParser parser;
   RegisterTsanFlags(&parser, f);
   RegisterCommonFlags(&parser);

lib/tsan/rtl/tsan_interceptors.cc

 //===----------------------------------------------------------------------===//
 
 #include "sanitizer_common/sanitizer_atomic.h"
 #include "sanitizer_common/sanitizer_libc.h"
 #include "sanitizer_common/sanitizer_linux.h"
 #include "sanitizer_common/sanitizer_platform_limits_posix.h"
 #include "sanitizer_common/sanitizer_placement_new.h"
 #include "sanitizer_common/sanitizer_stacktrace.h"
+#include "sanitizer_common/sanitizer_tls_get_addr.h"
 #include "interception/interception.h"
 #include "tsan_interceptors.h"
 #include "tsan_interface.h"
 #include "tsan_platform.h"
 #include "tsan_suppressions.h"
 #include "tsan_rtl.h"
 #include "tsan_mman.h"
 #include "tsan_fd.h"
void DestroyThreadState() {
   ThreadFinish(thr);
   ProcUnwire(proc, thr);
   ProcDestroy(proc);
   ThreadSignalContext *sctx = thr->signal_ctx;
   if (sctx) {
     thr->signal_ctx = 0;
     UnmapOrDie(sctx, sizeof(*sctx));
   }
+  DTLS_Destroy();
   cur_thread_finalize();
 }
 }  // namespace __tsan
 
 #if !SANITIZER_MAC
 static void thread_finalize(void *v) {
   uptr iter = (uptr)v;
   if (iter > 1) {

 // function "writes to" the buffer. Then, the same memory is "written to"
 // twice, first as buf and then as pwbufp (both of them refer to the same
 // addresses).
 #undef SANITIZER_INTERCEPT_GETPWENT
 #undef SANITIZER_INTERCEPT_GETPWENT_R
 #undef SANITIZER_INTERCEPT_FGETPWENT
 #undef SANITIZER_INTERCEPT_GETPWNAM_AND_FRIENDS
 #undef SANITIZER_INTERCEPT_GETPWNAM_R_AND_FRIENDS
-// __tls_get_addr can be called with mis-aligned stack due to:
-// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58066
-// There are two potential issues:
-// 1. Sanitizer code contains a MOVDQA spill (it does not seem to be the case
-// right now). or 2. ProcessPendingSignal calls user handler which contains
-// MOVDQA spill (this happens right now).
-// Since the interceptor only initializes memory for msan, the simplest solution
-// is to disable the interceptor in tsan (other sanitizers do not call
-// signal handlers from COMMON_INTERCEPTOR_ENTER).
-// As __tls_get_addr has been intercepted in the past, to avoid breaking
-// libtsan ABI, keep it around, but just call the real function.
+// We define our own.
 #if SANITIZER_INTERCEPT_TLS_GET_ADDR
 #define NEED_TLS_GET_ADDR
 #endif
 #undef SANITIZER_INTERCEPT_TLS_GET_ADDR
 
 #define COMMON_INTERCEPT_FUNCTION(name) INTERCEPT_FUNCTION(name)
 #define COMMON_INTERCEPT_FUNCTION_VER(name, ver)                          \
   INTERCEPT_FUNCTION_VER(name, ver)
#define COMMON_SYSCALL_PRE_FORK() \
   syscall_pre_fork(GET_CALLER_PC())
 
 #define COMMON_SYSCALL_POST_FORK(res) \
   syscall_post_fork(GET_CALLER_PC(), res)
 
 #include "sanitizer_common/sanitizer_common_syscalls.inc"
 
 #ifdef NEED_TLS_GET_ADDR
+// Define own interceptor instead of sanitizer_common's for three reasons:
+// 1. It must not process pending signals.
+//    Signal handlers may contain MOVDQA instruction (see below).
+// 2. It must be as simple as possible to not contain MOVDQA.
+// 3. Sanitizer_common version uses COMMON_INTERCEPTOR_INITIALIZE_RANGE which
+//    is empty for tsan (meant only for msan).
+// Note: __tls_get_addr can be called with mis-aligned stack due to:
+// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58066
+// So the interceptor must work with mis-aligned stack, in particular, does not
+// execute MOVDQA with stack addresses.
 TSAN_INTERCEPTOR(void *, __tls_get_addr, void *arg) {
-  return REAL(__tls_get_addr)(arg);
+  void *res = REAL(__tls_get_addr)(arg);
+  ThreadState *thr = cur_thread();
+  if (!thr)
+    return res;
+  DTLS::DTV *dtv = DTLS_on_tls_get_addr(arg, res, thr->tls_addr, thr->tls_size);
+  if (!dtv)
+    return res;
+  // New DTLS block has been allocated.
+  MemoryResetRange(thr, 0, dtv->beg, dtv->size);
+  return res;
 }
 #endif
 
 namespace __tsan {
 
 static void finalize(void *arg) {
   ThreadState *thr = cur_thread();
   int status = Finalize(thr);

test/tsan/dtls.c

+// RUN: %clang_tsan %s -o %t
+// RUN: %clang_tsan %s -DBUILD_SO -fPIC -o %t-so.so -shared
+// RUN: %run %t 2>&1 | FileCheck %s
+
+// Test that tsan cleans up dynamic TLS memory between reuse.
+
+#include "test.h"
+
+#ifndef BUILD_SO
+#include <assert.h>
+#include <dlfcn.h>
+
+typedef volatile long *(* get_t)();
+get_t GetTls;
+
+void *Thread1(void *arg) {
+  pthread_detach(pthread_self());
+  volatile long *x = GetTls();
+  *x = 42;
+  fprintf(stderr, "stack: %p dtls: %p\n", &x, x);
+  barrier_wait(&barrier);
+  return 0;
+}
+
+void *Thread2(void *arg) {
+  volatile long *x = GetTls();
+  *x = 42;
+  fprintf(stderr, "stack: %p dtls: %p\n", &x, x);
+  return 0;
+}
+
+int main(int argc, char *argv[]) {
+  char path[4096];
+  snprintf(path, sizeof(path), "%s-so.so", argv[0]);
+
+  void *handle = dlopen(path, RTLD_LAZY);
+  if (!handle) fprintf(stderr, "%s\n", dlerror());
+  assert(handle != 0);
+  GetTls = (get_t)dlsym(handle, "GetTls");
+  assert(dlerror() == 0);
+
+  barrier_init(&barrier, 2);
+  pthread_t t[2];
+  pthread_create(&t[0], 0, Thread1, 0);
+  barrier_wait(&barrier);
+  // Wait for actual thread termination without using pthread_join,
+  // which would synchronize threads.
+  sleep(1);
+  pthread_create(&t[1], 0, Thread2, 0);
+  pthread_join(t[1], 0);
+  fprintf(stderr, "DONE\n");
+  return 0;
+}
+#else  // BUILD_SO
+__thread long huge_thread_local_array[1 << 17];
+long *GetTls() {
+  return &huge_thread_local_array[0];
+}
+#endif
+
+// CHECK-NOT: ThreadSanitizer: data race
+// CHECK: DONE