[tsan] Add Mutex annotation flag for constant-initialized __tsan_mutex_linker_init behavior

[gfalcon]

Summary

Add a new flag, _⁠_tsan_mutex_not_static, which has the opposite sense of _⁠_tsan_mutex_linker_init. When the new _⁠_tsan_mutex_not_static flag is passed to _⁠_tsan_mutex_destroy, tsan ignores the destruction unless the mutex was also created with the _⁠_tsan_mutex_not_static flag.

This is useful for constructors that otherwise woud set _⁠_tsan_mutex_linker_init but cannot, because they are declared constexpr.

Google has a custom mutex with two constructors, a "linker initialized" constructor that relies on zero-initialization and sets ⁠_⁠_tsan_mutex_linker_init, and a normal one which sets no tsan flags. The "linker initialized" constructor is morally constexpr, but we can't declare it constexpr because of the need to call into tsan as a side effect.

With this new flag, the normal c'tor can set _⁠_tsan_mutex_not_static, the "linker initialized" constructor can rely on tsan's lazy initialization, and _⁠_tsan_mutex_destroy can still handle both cases correctly.

[dvyukov]

LGTM except that ninja check-tsan fails:

compiler-rt/test/tsan/custom_mutex4.cc:17: Extra space before ( in function call [whitespace/parens] [4]
compiler-rt/test/tsan/custom_mutex4.cc:24: Extra space before ( in function call [whitespace/parens] [4]
ninja: build stopped: subcommand failed.

[dvyukov]

Submitted in http://llvm.org/viewvc/llvm-project?view=revision&revision=316209

[dvyukov]

+ http://llvm.org/viewvc/llvm-project?view=revision&revision=316210

include/sanitizer/tsan_interface.h

 
 // Mutex has static storage duration and no-op constructor and destructor.
 // This effectively makes tsan ignore destroy annotation.
 const unsigned __tsan_mutex_linker_init      = 1 << 0;
 // Mutex is write reentrant.
 const unsigned __tsan_mutex_write_reentrant  = 1 << 1;
 // Mutex is read reentrant.
 const unsigned __tsan_mutex_read_reentrant   = 1 << 2;
+// Mutex does not have static storage duration, and must not be used after
+// its destructor runs.  The opposite of __tsan_mutex_linker_init.
+// If this flag is passed to __tsan_mutex_destroy, then the destruction
+// is ignored unless this flag was previously set on the mutex.
+const unsigned __tsan_mutex_not_static       = 1 << 8;
 
 // Mutex operation flags:
 
 // Denotes read lock operation.
 const unsigned __tsan_mutex_read_lock        = 1 << 3;
 // Denotes try lock operation.
 const unsigned __tsan_mutex_try_lock         = 1 << 4;
 // Denotes that a try lock operation has failed to acquire the mutex.

 
 // Annotate creation of a mutex.
 // Supported flags: mutex creation flags.
 void __tsan_mutex_create(void *addr, unsigned flags);
 
 // Annotate destruction of a mutex.
 // Supported flags:
 //   - __tsan_mutex_linker_init
+//   - __tsan_mutex_not_static
 void __tsan_mutex_destroy(void *addr, unsigned flags);
 
 // Annotate start of lock operation.
 // Supported flags:
 //   - __tsan_mutex_read_lock
 //   - __tsan_mutex_try_lock
 //   - all mutex creation flags
 void __tsan_mutex_pre_lock(void *addr, unsigned flags);

lib/tsan/rtl/tsan_rtl_mutex.cc

 }
 
 void MutexDestroy(ThreadState *thr, uptr pc, uptr addr, u32 flagz) {
   DPrintf("#%d: MutexDestroy %zx\n", thr->tid, addr);
   StatInc(thr, StatMutexDestroy);
   SyncVar *s = ctx->metamap.GetIfExistsAndLock(addr, true);
   if (s == 0)
     return;
-  if ((flagz & MutexFlagLinkerInit) || s->IsFlagSet(MutexFlagLinkerInit)) {
+  if ((flagz & MutexFlagLinkerInit)
+      || s->IsFlagSet(MutexFlagLinkerInit)
+      || ((flagz & MutexFlagNotStatic) && !s->IsFlagSet(MutexFlagNotStatic))) {
     // 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);
     ctx->dd->MutexInit(&cb, &s->dd);

lib/tsan/rtl/tsan_sync.h

enum MutexFlags {
   MutexFlagLinkerInit          = 1 << 0, // __tsan_mutex_linker_init
   MutexFlagWriteReentrant      = 1 << 1, // __tsan_mutex_write_reentrant
   MutexFlagReadReentrant       = 1 << 2, // __tsan_mutex_read_reentrant
   MutexFlagReadLock            = 1 << 3, // __tsan_mutex_read_lock
   MutexFlagTryLock             = 1 << 4, // __tsan_mutex_try_lock
   MutexFlagTryLockFailed       = 1 << 5, // __tsan_mutex_try_lock_failed
   MutexFlagRecursiveLock       = 1 << 6, // __tsan_mutex_recursive_lock
   MutexFlagRecursiveUnlock     = 1 << 7, // __tsan_mutex_recursive_unlock
+  MutexFlagNotStatic           = 1 << 8, // __tsan_mutex_not_static
 
   // The following flags are runtime private.
   // Mutex API misuse was detected, so don't report any more.
   MutexFlagBroken              = 1 << 30,
   // We did not intercept pre lock event, so handle it on post lock.
   MutexFlagDoPreLockOnPostLock = 1 << 29,
   // Must list all mutex creation flags.
   MutexCreationFlagMask        = MutexFlagLinkerInit |
                                  MutexFlagWriteReentrant |
-                                 MutexFlagReadReentrant,
+                                 MutexFlagReadReentrant |
+                                 MutexFlagNotStatic,
 };
 
 struct SyncVar {
   SyncVar();
 
   static const int kInvalidTid = -1;
 
   uptr addr;  // overwritten by DenseSlabAlloc freelist

test/tsan/custom_mutex.h

 #include "test.h"
 #include <atomic>
 #include <vector>
 #include <sanitizer/tsan_interface.h>
 
 // A very primitive mutex annotated with tsan annotations.
 class Mutex {
  public:
-  Mutex(bool prof, unsigned flags)
+  Mutex(bool prof, unsigned create_flags, unsigned destroy_flags=0)
       : prof_(prof)
       , locked_(false)
-      , seq_(0) {
-    __tsan_mutex_create(this, flags);
+      , seq_(0)
+      , destroy_flags_(destroy_flags) {
+    __tsan_mutex_create(this, create_flags);
   }
 
   ~Mutex() {
-    __tsan_mutex_destroy(this, 0);
+    __tsan_mutex_destroy(this, destroy_flags_);
   }
 
   void Lock() {
     __tsan_mutex_pre_lock(this, 0);
     LockImpl();
     __tsan_mutex_post_lock(this, 0, 0);
   }
 
void Broadcast() {
     UnlockImpl();
     __tsan_mutex_post_signal(this, 0);
   }
 
  private:
   const bool prof_;
   std::atomic<bool> locked_;
   int seq_;
+  unsigned destroy_flags_;
 
   // This models mutex profiling subsystem.
   static Mutex prof_mu_;
   static int prof_data_;
 
   void LockImpl(bool prof = true) {
     while (!TryLockImpl())
       usleep(100);

test/tsan/custom_mutex4.cc

+// RUN: %clangxx_tsan -O1 --std=c++11 %s -o %t && %run %t 2>&1 | FileCheck %s
+#include "custom_mutex.h"
+
+#include <type_traits>
+
+// Test that the destruction events of a mutex are ignored when the
+// annotations request this.
+//
+// Use after destruction is UB, but __tsan_mutex_linker_init and
+// __tsan_mutex_not_static exist to support global variables of mutex type,
+// which might be accessed during program shutdown after the class's destructor
+// has run.
+
+int main() {
+  std::aligned_storage<sizeof(Mutex), alignof(Mutex)>::type mu1_store;
+  Mutex* mu1 = reinterpret_cast<Mutex*>(&mu1_store);
+  new (&mu1_store) Mutex(false, __tsan_mutex_linker_init);
+  mu1->Lock();
+  mu1->~Mutex();
+  mu1->Unlock();
+
+  std::aligned_storage<sizeof(Mutex), alignof(Mutex)>::type mu2_store;
+  Mutex* mu2 = reinterpret_cast<Mutex*>(&mu2_store);
+  new (&mu2_store) Mutex(false, 0, __tsan_mutex_not_static);
+  mu2->Lock();
+  mu2->~Mutex();
+  mu2->Unlock();
+
+  fprintf(stderr, "DONE\n");
+  return 0;
+}
+
+// CHECK: DONE

test/tsan/custom_mutex5.cc

+// RUN: %clangxx_tsan -O1 --std=c++11 %s -o %t && %deflake %run %t 2>&1 | FileCheck %s
+#include "custom_mutex.h"
+
+#include <type_traits>
+
+// Test that we detect the destruction of an in-use mutex when the
+// thread annotations don't otherwise disable the check.
+
+int main() {
+  std::aligned_storage<sizeof(Mutex), alignof(Mutex)>::type mu1_store;
+  Mutex* mu1 = reinterpret_cast<Mutex*>(&mu1_store);
+  new(&mu1_store) Mutex(false, 0);
+  mu1->Lock();
+  mu1->~Mutex();
+  mu1->Unlock();
+
+  std::aligned_storage<sizeof(Mutex), alignof(Mutex)>::type mu2_store;
+  Mutex* mu2 = reinterpret_cast<Mutex*>(&mu2_store);
+  new(&mu2_store)
+      Mutex(false, __tsan_mutex_not_static, __tsan_mutex_not_static);
+  mu2->Lock();
+  mu2->~Mutex();
+  mu2->Unlock();
+
+  fprintf(stderr, "DONE\n");
+  return 0;
+}
+
+// CHECK: WARNING: ThreadSanitizer: destroy of a locked mutex
+// CHECK:   main {{.*}}custom_mutex5.cc:14
+// CHECK: WARNING: ThreadSanitizer: destroy of a locked mutex
+// CHECK:   main {{.*}}custom_mutex5.cc:22
+// CHECK: DONE