[compiler-rt] [TSAN] Add __tsan_check_no_mutexes_held helper (PR #69372)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 23 22:43:07 PDT 2023
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-compiler-rt-sanitizer
Author: Kenny Yu (kennyyu)
<details>
<summary>Changes</summary>
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
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
``````````
</details>
https://github.com/llvm/llvm-project/pull/69372
More information about the llvm-commits
mailing list