tsan: rely on AnnotateRWLockCreateStatic to detect linker-initialized mutexes

The new annotation was added a while ago, but was not actually used.
Use the annotation to detect linker-initialized mutexes instead
of the broken IsGlobalVar which has both false positives and false
negatives. Remove IsGlobalVar mess.

llvm-svn: 271663

Author: dvyukov

compiler-rt/lib/tsan/rtl/tsan_platform.h

@@ -757,10 +757,6 @@ void CheckAndProtect();
 void InitializeShadowMemoryPlatform();
 void FlushShadowMemory();
 void WriteMemoryProfile(char *buf, uptr buf_size, uptr nthread, uptr nlive);
-
-// Says whether the addr relates to a global var.
-// Guesses with high probability, may yield both false positives and negatives.
-bool IsGlobalVar(uptr addr);
 int ExtractResolvFDs(void *state, int *fds, int nfd);
 int ExtractRecvmsgFDs(void *msg, int *fds, int nfd);
 

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

@@ -69,9 +69,6 @@ void *__libc_stack_end = 0;
 
 namespace __tsan {
 
-static uptr g_data_start;
-static uptr g_data_end;
-
 #ifdef TSAN_RUNTIME_VMA
 // Runtime detected VMA size.
 uptr vmaSize;
@@ -204,46 +201,6 @@ void InitializeShadowMemoryPlatform() {
   MapRodata();
 }
 
-static void InitDataSeg() {
-  MemoryMappingLayout proc_maps(true);
-  uptr start, end, offset;
-  char name[128];
-#if SANITIZER_FREEBSD
-  // On FreeBSD BSS is usually the last block allocated within the
-  // low range and heap is the last block allocated within the range
-  // 0x800000000-0x8ffffffff.
-  while (proc_maps.Next(&start, &end, &offset, name, ARRAY_SIZE(name),
-                        /*protection*/ 0)) {
-    DPrintf("%p-%p %p %s\n", start, end, offset, name);
-    if ((start & 0xffff00000000ULL) == 0 && (end & 0xffff00000000ULL) == 0 &&
-        name[0] == '\0') {
-      g_data_start = start;
-      g_data_end = end;
-    }
-  }
-#else
-  bool prev_is_data = false;
-  while (proc_maps.Next(&start, &end, &offset, name, ARRAY_SIZE(name),
-                        /*protection*/ 0)) {
-    DPrintf("%p-%p %p %s\n", start, end, offset, name);
-    bool is_data = offset != 0 && name[0] != 0;
-    // BSS may get merged with [heap] in /proc/self/maps. This is not very
-    // reliable.
-    bool is_bss = offset == 0 &&
-      (name[0] == 0 || internal_strcmp(name, "[heap]") == 0) && prev_is_data;
-    if (g_data_start == 0 && is_data)
-      g_data_start = start;
-    if (is_bss)
-      g_data_end = end;
-    prev_is_data = is_data;
-  }
-#endif
-  DPrintf("guessed data_start=%p data_end=%p\n",  g_data_start, g_data_end);
-  CHECK_LT(g_data_start, g_data_end);
-  CHECK_GE((uptr)&g_data_start, g_data_start);
-  CHECK_LT((uptr)&g_data_start, g_data_end);
-}
-
 #endif  // #ifndef SANITIZER_GO
 
 void InitializePlatformEarly() {
@@ -315,14 +272,9 @@ void InitializePlatform() {
 #ifndef SANITIZER_GO
   CheckAndProtect();
   InitTlsSize();
-  InitDataSeg();
 #endif
 }
 
-bool IsGlobalVar(uptr addr) {
-  return g_data_start && addr >= g_data_start && addr < g_data_end;
-}
-
 #ifndef SANITIZER_GO
 // Extract file descriptors passed to glibc internal __res_iclose function.
 // This is required to properly "close" the fds, because we do not see internal

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

@@ -189,10 +189,6 @@ int call_pthread_cancel_with_cleanup(int(*fn)(void *c, void *m,
 }
 #endif
 
-bool IsGlobalVar(uptr addr) {
-  return false;
-}
-
 }  // namespace __tsan
 
 #endif  // SANITIZER_MAC

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

@@ -84,21 +84,14 @@ void MutexCreate(ThreadState *thr, uptr pc, uptr addr,
 void MutexDestroy(ThreadState *thr, uptr pc, uptr addr) {
   DPrintf("#%d: MutexDestroy %zx\n", thr->tid, addr);
   StatInc(thr, StatMutexDestroy);
-#ifndef SANITIZER_GO
-  // Global mutexes not marked as LINKER_INITIALIZED
-  // cause tons of not interesting reports, so just ignore it.
-  if (IsGlobalVar(addr))
-    return;
-#endif
-  if (IsAppMem(addr)) {
-    CHECK(!thr->is_freeing);
-    thr->is_freeing = true;
-    MemoryWrite(thr, pc, addr, kSizeLog1);
-    thr->is_freeing = false;
-  }
   SyncVar *s = ctx->metamap.GetIfExistsAndLock(addr);
   if (s == 0)
     return;
+  if (s->is_linker_init) {
+    // Destroy is no-op for linker-initialized mutexes.
+    s->mtx.Unlock();
+    return;
+  }
   if (common_flags()->detect_deadlocks) {
     Callback cb(thr, pc);
     ctx->dd->MutexDestroy(&cb, &s->dd);
@@ -128,15 +121,23 @@ void MutexDestroy(ThreadState *thr, uptr pc, uptr addr) {
     rep.AddStack(trace, true);
     rep.AddLocation(addr, 1);
     OutputReport(thr, rep);
-  }
-  if (unlock_locked) {
+
     SyncVar *s = ctx->metamap.GetIfExistsAndLock(addr);
     if (s != 0) {
       s->Reset(thr->proc());
       s->mtx.Unlock();
     }
   }
   thr->mset.Remove(mid);
+  // Imitate a memory write to catch unlock-destroy races.
+  // Do this outside of sync mutex, because it can report a race which locks
+  // sync mutexes.
+  if (IsAppMem(addr)) {
+    CHECK(!thr->is_freeing);
+    thr->is_freeing = true;
+    MemoryWrite(thr, pc, addr, kSizeLog1);
+    thr->is_freeing = false;
+  }
   // s will be destroyed and freed in MetaMap::FreeBlock.
 }
 

compiler-rt/test/tsan/mutex_annotations.cc

@@ -0,0 +1,49 @@
+// RUN: %clangxx_tsan -O1 %s -o %t && %run %t 2>&1 | FileCheck %s
+#include "test.h"
+
+// Test that a linker-initialized mutex can be created/destroyed while in use.
+
+// Stub for testing, just invokes annotations.
+// Meant to be synchronized externally with test barrier.
+class Mutex {
+ public:
+  void Create(bool linker_initialized = false) {
+    if (linker_initialized)
+      ANNOTATE_RWLOCK_CREATE_STATIC(&state_);
+    else
+      ANNOTATE_RWLOCK_CREATE(&state_);
+  }
+
+  void Destroy() {
+    ANNOTATE_RWLOCK_DESTROY(&state_);
+  }
+
+  void Lock() {
+    ANNOTATE_RWLOCK_ACQUIRED(&state_, true);
+  }
+
+  void Unlock() {
+    ANNOTATE_RWLOCK_RELEASED(&state_, true);
+  }
+
+ private:
+  long long state_;
+};
+
+int main() {
+  Mutex m;
+
+  m.Lock();
+  m.Create(true);
+  m.Unlock();
+
+  m.Lock();
+  m.Destroy();
+  m.Unlock();
+
+  fprintf(stderr, "DONE\n");
+  return 0;
+}
+
+// CHECK-NOT: WARNING: ThreadSanitizer:
+// CHECK: DONE

compiler-rt/test/tsan/test.h

@@ -77,3 +77,28 @@ const int kPCInc = 8;
 #else
 const int kPCInc = 1;
 #endif
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+void AnnotateRWLockCreate(const char *f, int l, void *m);
+void AnnotateRWLockCreateStatic(const char *f, int l, void *m);
+void AnnotateRWLockDestroy(const char *f, int l, void *m);
+void AnnotateRWLockAcquired(const char *f, int l, void *m, long is_w);
+void AnnotateRWLockReleased(const char *f, int l, void *m, long is_w);
+
+#ifdef __cplusplus
+}
+#endif
+
+#define ANNOTATE_RWLOCK_CREATE(m) \
+    AnnotateRWLockCreate(__FILE__, __LINE__, m)
+#define ANNOTATE_RWLOCK_CREATE_STATIC(m) \
+    AnnotateRWLockCreateStatic(__FILE__, __LINE__, m)
+#define ANNOTATE_RWLOCK_DESTROY(m) \
+    AnnotateRWLockDestroy(__FILE__, __LINE__, m)
+#define ANNOTATE_RWLOCK_ACQUIRED(m, is_w) \
+    AnnotateRWLockAcquired(__FILE__, __LINE__, m, is_w)
+#define ANNOTATE_RWLOCK_RELEASED(m, is_w) \
+    AnnotateRWLockReleased(__FILE__, __LINE__, m, is_w)