[TSAN] Add __tsan_check_no_mutexes_held helper

[kennyyu]

This adds a new helper that can be called from application code to ensure that no mutexes are held on specific code paths. This is useful for multiple scenarios, including ensuring no locks are held:

• at thread exit
• in peformance-critical code
• when a coroutine is suspended (can cause deadlocks)

See this discourse thread for more discussion:
https://discourse.llvm.org/t/add-threadsanitizer-check-to-prevent-coroutine-suspending-while-holding-a-lock-potential-deadlock/74051

[dvyukov]

Some cosmetic comments, but overall looks good.

[llvmbot]

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Kenny Yu (kennyyu)

Changes

This adds a new helper that can be called from application code to ensure that no mutexes are held on specific code paths. This is useful for multiple scenarios, including ensuring no locks are held:

• at thread exit
• in peformance-critical code
• when a coroutine is suspended (can cause deadlocks)

See this discourse thread for more discussion:
https://discourse.llvm.org/t/add-threadsanitizer-check-to-prevent-coroutine-suspending-while-holding-a-lock-potential-deadlock/74051

---

Full diff: https://github.com/llvm/llvm-project/pull/69372.diff

9 Files Affected:

• (modified) compiler-rt/include/sanitizer/tsan_interface.h (+4)
• (modified) compiler-rt/lib/tsan/rtl/tsan.syms.extra (+1)
• (modified) compiler-rt/lib/tsan/rtl/tsan_debugging.cpp (+3-1)
• (modified) compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp (+22)
• (modified) compiler-rt/lib/tsan/rtl/tsan_report.cpp (+12-5)
• (modified) compiler-rt/lib/tsan/rtl/tsan_report.h (+2-1)
• (modified) compiler-rt/lib/tsan/rtl/tsan_suppressions.cpp (+1)
• (added) compiler-rt/test/tsan/mutex_cannot_be_locked.cpp (+24)
• (added) compiler-rt/test/tsan/mutex_cannot_be_locked_success.cpp (+19)

diff --git a/compiler-rt/include/sanitizer/tsan_interface.h b/compiler-rt/include/sanitizer/tsan_interface.h
index f19c79d79ba62ef..cd118441cb5ce24 100644
--- a/compiler-rt/include/sanitizer/tsan_interface.h
+++ b/compiler-rt/include/sanitizer/tsan_interface.h
@@ -126,6 +126,10 @@ void __tsan_mutex_post_signal(void *addr, unsigned flags);
 void __tsan_mutex_pre_divert(void *addr, unsigned flags);
 void __tsan_mutex_post_divert(void *addr, unsigned flags);
 
+// Annotate that no mutexes can be held. If we are holding mutexes, then
+// TSan will print a bug report.
+void __tsan_check_no_mutexes_held();
+
 // External race detection API.
 // Can be used by non-instrumented libraries to detect when their objects are
 // being used in an unsafe manner.
diff --git a/compiler-rt/lib/tsan/rtl/tsan.syms.extra b/compiler-rt/lib/tsan/rtl/tsan.syms.extra
index a5bd17176b12bfa..6416e5d47fc4131 100644
--- a/compiler-rt/lib/tsan/rtl/tsan.syms.extra
+++ b/compiler-rt/lib/tsan/rtl/tsan.syms.extra
@@ -22,6 +22,7 @@ __tsan_mutex_pre_signal
 __tsan_mutex_post_signal
 __tsan_mutex_pre_divert
 __tsan_mutex_post_divert
+__tsan_check_no_mutexes_held
 __tsan_get_current_fiber
 __tsan_create_fiber
 __tsan_destroy_fiber
diff --git a/compiler-rt/lib/tsan/rtl/tsan_debugging.cpp b/compiler-rt/lib/tsan/rtl/tsan_debugging.cpp
index 1e61c31c5a970cc..026d5b43299355b 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_debugging.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_debugging.cpp
@@ -35,7 +35,9 @@ static const char *ReportTypeDescription(ReportType typ) {
     case ReportTypeSignalUnsafe: return "signal-unsafe-call";
     case ReportTypeErrnoInSignal: return "errno-in-signal-handler";
     case ReportTypeDeadlock: return "lock-order-inversion";
-    // No default case so compiler warns us if we miss one
+    case ReportTypeMutexCannotBeLocked:
+      return "mutex-cannot-be-locked";
+      // No default case so compiler warns us if we miss one
   }
   UNREACHABLE("missing case");
 }
diff --git a/compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp b/compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp
index 6bd72e18d942580..712ce2d1a45a868 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp
@@ -435,4 +435,26 @@ void __tsan_mutex_post_divert(void *addr, unsigned flagz) {
   ThreadIgnoreBegin(thr, 0);
   ThreadIgnoreSyncBegin(thr, 0);
 }
+
+static void ReportMutexCannotBeLocked(ThreadState *thr, uptr pc) {
+  ThreadRegistryLock l(&ctx->thread_registry);
+  ScopedReport rep(ReportTypeMutexCannotBeLocked);
+  for (uptr i = 0; i < thr->mset.Size(); ++i) {
+    MutexSet::Desc desc = thr->mset.Get(i);
+    rep.AddMutex(desc.addr, desc.stack_id);
+  }
+  VarSizeStackTrace trace;
+  ObtainCurrentStack(thr, pc, &trace);
+  rep.AddStack(trace, true);
+  OutputReport(thr, rep);
+}
+
+INTERFACE_ATTRIBUTE
+void __tsan_check_no_mutexes_held() {
+  SCOPED_ANNOTATION(__tsan_check_no_mutexes_held);
+  if (thr->mset.Size() == 0) {
+    return;
+  }
+  ReportMutexCannotBeLocked(thr, pc);
+}
 }  // extern "C"
diff --git a/compiler-rt/lib/tsan/rtl/tsan_report.cpp b/compiler-rt/lib/tsan/rtl/tsan_report.cpp
index 4028d1810784095..673449a9e70af9c 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_report.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_report.cpp
@@ -93,7 +93,9 @@ static const char *ReportTypeString(ReportType typ, uptr tag) {
       return "signal handler spoils errno";
     case ReportTypeDeadlock:
       return "lock-order-inversion (potential deadlock)";
-    // No default case so compiler warns us if we miss one
+    case ReportTypeMutexCannotBeLocked:
+      return "mutex cannot be locked on this code path";
+      // No default case so compiler warns us if we miss one
   }
   UNREACHABLE("missing case");
 }
@@ -216,11 +218,16 @@ static void PrintMutexShortWithAddress(const ReportMutex *rm,
          reinterpret_cast<void *>(rm->addr), d.Default(), after);
 }
 
-static void PrintMutex(const ReportMutex *rm) {
+static void PrintMutex(const ReportMutex *rm, ReportType typ) {
   Decorator d;
   Printf("%s", d.Mutex());
-  Printf("  Mutex M%u (%p) created at:\n", rm->id,
-         reinterpret_cast<void *>(rm->addr));
+  if (typ != ReportTypeMutexCannotBeLocked) {
+    Printf("  Mutex M%u (%p) created at:\n", rm->id,
+           reinterpret_cast<void *>(rm->addr));
+  } else {
+    Printf("  Mutex M%u (%p) acquired at:\n", rm->id,
+           reinterpret_cast<void *>(rm->addr));
+  }
   Printf("%s", d.Default());
   PrintStack(rm->stack);
 }
@@ -354,7 +361,7 @@ void PrintReport(const ReportDesc *rep) {
 
   if (rep->typ != ReportTypeDeadlock) {
     for (uptr i = 0; i < rep->mutexes.Size(); i++)
-      PrintMutex(rep->mutexes[i]);
+      PrintMutex(rep->mutexes[i], rep->typ);
   }
 
   for (uptr i = 0; i < rep->threads.Size(); i++)
diff --git a/compiler-rt/lib/tsan/rtl/tsan_report.h b/compiler-rt/lib/tsan/rtl/tsan_report.h
index 3c88864af147704..0e1a27655bfa4ca 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_report.h
+++ b/compiler-rt/lib/tsan/rtl/tsan_report.h
@@ -34,7 +34,8 @@ enum ReportType {
   ReportTypeMutexBadReadUnlock,
   ReportTypeSignalUnsafe,
   ReportTypeErrnoInSignal,
-  ReportTypeDeadlock
+  ReportTypeDeadlock,
+  ReportTypeMutexCannotBeLocked
 };
 
 struct ReportStack {
diff --git a/compiler-rt/lib/tsan/rtl/tsan_suppressions.cpp b/compiler-rt/lib/tsan/rtl/tsan_suppressions.cpp
index 9cdfa32a9343029..3b249a716ed2c31 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_suppressions.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_suppressions.cpp
@@ -81,6 +81,7 @@ static const char *conv(ReportType typ) {
     case ReportTypeMutexBadUnlock:
     case ReportTypeMutexBadReadLock:
     case ReportTypeMutexBadReadUnlock:
+    case ReportTypeMutexCannotBeLocked:
       return kSuppressionMutex;
     case ReportTypeSignalUnsafe:
     case ReportTypeErrnoInSignal:
diff --git a/compiler-rt/test/tsan/mutex_cannot_be_locked.cpp b/compiler-rt/test/tsan/mutex_cannot_be_locked.cpp
new file mode 100644
index 000000000000000..77ce2d7053374ea
--- /dev/null
+++ b/compiler-rt/test/tsan/mutex_cannot_be_locked.cpp
@@ -0,0 +1,24 @@
+// RUN: %clangxx_tsan -O1 %s -o %t && %deflake %run %t | FileCheck %s
+#include "test.h"
+
+pthread_mutex_t mtx;
+
+void *ThreadFunc(void *) {
+  pthread_mutex_lock(&mtx);
+  __tsan_check_no_mutexes_held();
+}
+
+int main() {
+  pthread_t th;
+  pthread_create(&th, 0, ThreadFunc, NULL);
+  pthread_join(th, 0);
+  return 0;
+}
+
+// CHECK: WARNING: ThreadSanitizer: mutex cannot be locked on this code path
+// CHECK:     #0 __tsan_check_no_mutexes_held
+// CHECK:     #1 ThreadFunc
+// CHECK:   Mutex {{.*}} acquired at:
+// CHECK:     #0 pthread_mutex_lock
+// CHECK:     #1 ThreadFunc
+// CHECK: SUMMARY: ThreadSanitizer: mutex cannot be locked on this code path {{.*}}mutex_cannot_be_locked.cpp{{.*}}ThreadFunc
diff --git a/compiler-rt/test/tsan/mutex_cannot_be_locked_success.cpp b/compiler-rt/test/tsan/mutex_cannot_be_locked_success.cpp
new file mode 100644
index 000000000000000..7834dcc89477806
--- /dev/null
+++ b/compiler-rt/test/tsan/mutex_cannot_be_locked_success.cpp
@@ -0,0 +1,19 @@
+// RUN: %clangxx_tsan -O1 %s -o %t && %deflake %run %t | FileCheck %s
+#include "test.h"
+
+pthread_mutex_t mtx;
+
+void *ThreadFunc(void *) {
+  pthread_mutex_lock(&mtx);
+  pthread_mutex_unlock(&mtx);
+  __tsan_check_no_mutexes_held();
+}
+
+int main() {
+  pthread_t th;
+  pthread_create(&th, 0, ThreadFunc, NULL);
+  pthread_join(th, 0);
+  return 0;
+}
+
+// CHECK-NOT: WARNING: ThreadSanitizer: mutex cannot be locked on this code path

[kennyyu]

Thanks for the suggestions @dvyukov !

[zmodem]

The new test appears to be failing.
In Chromium: https://bugs.chromium.org/p/chromium/issues/detail?id=1500268
On Green Dragon: https://green.lab.llvm.org/green/job/clang-san-iossim/20248/

I'll revert to green.

[kennyyu]

@zmodem Thanks for catching it! Sorry for the breakage! This is my first time contributing to llvm, and I wasn't sure which signals to verify. I'll resubmit the PR with the fix.

[kennyyu]

#71568 resubmits this PR with the fix.