[compiler-rt] b4c1e5c - tsan: fix and test detection of TLS races

[Dmitry Vyukov via llvm-commits]

Author: Dmitry Vyukov
Date: 2021-09-27T16:40:08+02:00
New Revision: b4c1e5cb73bd26e5853af77c2a235ca9f35e2577

URL: https://github.com/llvm/llvm-project/commit/b4c1e5cb73bd26e5853af77c2a235ca9f35e2577
DIFF: https://github.com/llvm/llvm-project/commit/b4c1e5cb73bd26e5853af77c2a235ca9f35e2577.diff

LOG: tsan: fix and test detection of TLS races

Currently detection of races with TLS/stack initialization
is broken because we imitate the write before thread initialization,
so it's modelled with a wrong thread/epoch.
Fix that and add a test.

Reviewed By: melver

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

Added: 
    compiler-rt/test/tsan/tls_race3.cpp

Modified: 
    compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp
    compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp b/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp
index a8b5403d0b82..c4545130d3ca 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp
@@ -453,6 +453,8 @@ static void InitializeLongjmpXorKey() {
 }
 #endif
 
+extern "C" void __tsan_tls_initialization() {}
+
 void ImitateTlsWrite(ThreadState *thr, uptr tls_addr, uptr tls_size) {
   // Check that the thr object is in tls;
   const uptr thr_beg = (uptr)thr;
@@ -462,9 +464,10 @@ void ImitateTlsWrite(ThreadState *thr, uptr tls_addr, uptr tls_size) {
   CHECK_GE(thr_end, tls_addr);
   CHECK_LE(thr_end, tls_addr + tls_size);
   // Since the thr object is huge, skip it.
-  MemoryRangeImitateWrite(thr, /*pc=*/2, tls_addr, thr_beg - tls_addr);
-  MemoryRangeImitateWrite(thr, /*pc=*/2, thr_end,
-                          tls_addr + tls_size - thr_end);
+  const uptr pc = StackTrace::GetNextInstructionPc(
+      reinterpret_cast<uptr>(__tsan_tls_initialization));
+  MemoryRangeImitateWrite(thr, pc, tls_addr, thr_beg - tls_addr);
+  MemoryRangeImitateWrite(thr, pc, thr_end, tls_addr + tls_size - thr_end);
 }
 
 // Note: this function runs with async signals enabled,

diff  --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp
index 89178a1699ba..67b12c804fb2 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp
@@ -156,13 +156,6 @@ void ThreadStart(ThreadState *thr, Tid tid, tid_t os_id,
   if (thread_type != ThreadType::Fiber)
     GetThreadStackAndTls(tid == kMainTid, &stk_addr, &stk_size, &tls_addr,
                          &tls_size);
-
-  if (tid != kMainTid) {
-    if (stk_addr && stk_size)
-      MemoryRangeImitateWrite(thr, /*pc=*/ 1, stk_addr, stk_size);
-
-    if (tls_addr && tls_size) ImitateTlsWrite(thr, tls_addr, tls_size);
-  }
 #endif
 
   ThreadRegistry *tr = &ctx->thread_registry;
@@ -178,6 +171,16 @@ void ThreadStart(ThreadState *thr, Tid tid, tid_t os_id,
     ThreadIgnoreSyncBegin(thr, 0);
   }
 #endif
+
+#if !SANITIZER_GO
+  if (tid != kMainTid) {
+    if (stk_addr && stk_size)
+      MemoryRangeImitateWrite(thr, /*pc=*/1, stk_addr, stk_size);
+
+    if (tls_addr && tls_size)
+      ImitateTlsWrite(thr, tls_addr, tls_size);
+  }
+#endif
 }
 
 void ThreadContext::OnStarted(void *arg) {

diff  --git a/compiler-rt/test/tsan/tls_race3.cpp b/compiler-rt/test/tsan/tls_race3.cpp
new file mode 100644
index 000000000000..63eee3940820
--- /dev/null
+++ b/compiler-rt/test/tsan/tls_race3.cpp
@@ -0,0 +1,35 @@
+// RUN: %clangxx_tsan %darwin_min_target_with_tls_support -O1 %s -o %t && \
+// RUN:   %deflake %run %t | FileCheck %s
+
+// Race with initial TLS initialization:
+// there is no explicit second write,
+// but the TLS variable is published unsafely.
+#include "test.h"
+
+__thread long X;
+long *P;
+
+void *Thread(void *a) {
+  __atomic_store_n(&P, &X, __ATOMIC_RELAXED);
+  barrier_wait(&barrier);
+  barrier_wait(&barrier);
+  return 0;
+}
+
+int main() {
+  barrier_init(&barrier, 2);
+  pthread_t t;
+  pthread_create(&t, NULL, Thread, NULL);
+  barrier_wait(&barrier);
+  long *p = __atomic_load_n(&P, __ATOMIC_RELAXED);
+  *p = 42;
+  barrier_wait(&barrier);
+  pthread_join(t, 0);
+}
+
+// CHECK: WARNING: ThreadSanitizer: data race
+// CHECK:   Write of size 8 at {{.*}} by main thread:
+// CHECK:     #0 main
+// CHECK:   Previous write of size 8 at {{.*}} by thread T1:
+// CHECK:     #0 __tsan_tls_initialization
+// CHECK:   Location is TLS of thread T1.