[compiler-rt] c483140 - tsan: improve detection of stack/tls races

[Dmitry Vyukov via llvm-commits]

Author: Dmitry Vyukov
Date: 2021-10-05T15:32:39+02:00
New Revision: c483140f3ce28ea243d414dd0dee0c0f13c5b01c

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

LOG: tsan: improve detection of stack/tls races

Print meaningful stack frames for stack/tls races
(instead of PC 1/2 that don't symbolize).

Imitate stack/tls writes after we create and initialize
the new thread, otherwise the races are not detected.

This is re-submit of the following reverted commits,
but without tests as they failed on a number of OSes/arches:
"tsan: fix and test detection of TLS races"
"tsan: fix tls_race3 test on darwin"
"tsan: print a meaningful frame for stack races"

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

Added: 
    

Modified: 
    compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp
    compiler-rt/lib/tsan/rtl/tsan_platform_mac.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 63c751af4a85..2fb753dd080c 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_platform_mac.cpp b/compiler-rt/lib/tsan/rtl/tsan_platform_mac.cpp
index 0ca068a2b318..388b3836d7d1 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_platform_mac.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_platform_mac.cpp
@@ -283,13 +283,17 @@ uptr ExtractLongJmpSp(uptr *env) {
 }
 
 #if !SANITIZER_GO
+extern "C" void __tsan_tls_initialization() {}
+
 void ImitateTlsWrite(ThreadState *thr, uptr tls_addr, uptr tls_size) {
   // The pointer to the ThreadState object is stored in the shadow memory
   // of the tls.
   uptr tls_end = tls_addr + tls_size;
   uptr thread_identity = (uptr)pthread_self();
+  const uptr pc = StackTrace::GetNextInstructionPc(
+      reinterpret_cast<uptr>(__tsan_tls_initialization));
   if (thread_identity == main_thread_identity) {
-    MemoryRangeImitateWrite(thr, /*pc=*/2, tls_addr, tls_size);
+    MemoryRangeImitateWrite(thr, pc, tls_addr, tls_size);
   } else {
     uptr thr_state_start = thread_identity;
     uptr thr_state_end = thr_state_start + sizeof(uptr);
@@ -297,10 +301,8 @@ void ImitateTlsWrite(ThreadState *thr, uptr tls_addr, uptr tls_size) {
     CHECK_LE(thr_state_start, tls_addr + tls_size);
     CHECK_GE(thr_state_end, tls_addr);
     CHECK_LE(thr_state_end, tls_addr + tls_size);
-    MemoryRangeImitateWrite(thr, /*pc=*/2, tls_addr,
-                            thr_state_start - tls_addr);
-    MemoryRangeImitateWrite(thr, /*pc=*/2, thr_state_end,
-                            tls_end - thr_state_end);
+    MemoryRangeImitateWrite(thr, pc, tls_addr, thr_state_start - tls_addr);
+    MemoryRangeImitateWrite(thr, pc, thr_state_end, tls_end - thr_state_end);
   }
 }
 #endif

diff  --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp
index 89178a1699ba..61133a4a3e7e 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp
@@ -138,6 +138,8 @@ void ThreadContext::OnCreated(void *arg) {
   creation_stack_id = CurrentStackId(args->thr, args->pc);
 }
 
+extern "C" void __tsan_stack_initialization() {}
+
 struct OnStartedArgs {
   ThreadState *thr;
   uptr stk_addr;
@@ -156,13 +158,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 +173,22 @@ void ThreadStart(ThreadState *thr, Tid tid, tid_t os_id,
     ThreadIgnoreSyncBegin(thr, 0);
   }
 #endif
+
+#if !SANITIZER_GO
+  // Don't imitate stack/TLS writes for the main thread,
+  // because its initialization is synchronized with all
+  // subsequent threads anyway.
+  if (tid != kMainTid) {
+    if (stk_addr && stk_size) {
+      const uptr pc = StackTrace::GetNextInstructionPc(
+          reinterpret_cast<uptr>(__tsan_stack_initialization));
+      MemoryRangeImitateWrite(thr, pc, stk_addr, stk_size);
+    }
+
+    if (tls_addr && tls_size)
+      ImitateTlsWrite(thr, tls_addr, tls_size);
+  }
+#endif
 }
 
 void ThreadContext::OnStarted(void *arg) {