[compiler-rt] 27156dd - Revert "[TSAN] Add __tsan_check_no_mutexes_held helper (#69372)"
Hans Wennborg via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 7 07:09:02 PST 2023
Author: Hans Wennborg
Date: 2023-11-07T16:08:01+01:00
New Revision: 27156dd575d34980c21f97fae4bfbd47a8ca16de
URL: https://github.com/llvm/llvm-project/commit/27156dd575d34980c21f97fae4bfbd47a8ca16de
DIFF: https://github.com/llvm/llvm-project/commit/27156dd575d34980c21f97fae4bfbd47a8ca16de.diff
LOG: Revert "[TSAN] Add __tsan_check_no_mutexes_held helper (#69372)"
The new lit test fails, see comment on the PR. This also reverts
the follow-up commit, see below.
> 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
This reverts commit bd841111f340a73eb23c1be70ff1be4c8a6afb0c.
This reverts commit 16a395b74d35c564f6f36ba4a167950a323badd9.
Added:
Modified:
compiler-rt/include/sanitizer/tsan_interface.h
compiler-rt/lib/tsan/rtl/tsan.syms.extra
compiler-rt/lib/tsan/rtl/tsan_debugging.cpp
compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp
compiler-rt/lib/tsan/rtl/tsan_report.cpp
compiler-rt/lib/tsan/rtl/tsan_report.h
compiler-rt/lib/tsan/rtl/tsan_suppressions.cpp
Removed:
compiler-rt/test/tsan/mutex_held_wrong_context.cpp
################################################################################
diff --git a/compiler-rt/include/sanitizer/tsan_interface.h b/compiler-rt/include/sanitizer/tsan_interface.h
index e11a4175cd8ed9f..3ef79ab81dd5312 100644
--- a/compiler-rt/include/sanitizer/tsan_interface.h
+++ b/compiler-rt/include/sanitizer/tsan_interface.h
@@ -127,10 +127,6 @@ void SANITIZER_CDECL __tsan_mutex_post_signal(void *addr, unsigned flags);
void SANITIZER_CDECL __tsan_mutex_pre_divert(void *addr, unsigned flags);
void SANITIZER_CDECL __tsan_mutex_post_divert(void *addr, unsigned flags);
-// Check that the current thread does not hold any mutexes,
-// report a bug report otherwise.
-void SANITIZER_CDECL __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 6416e5d47fc4131..a5bd17176b12bfa 100644
--- a/compiler-rt/lib/tsan/rtl/tsan.syms.extra
+++ b/compiler-rt/lib/tsan/rtl/tsan.syms.extra
@@ -22,7 +22,6 @@ __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 41fa293dbaaad0b..1e61c31c5a970cc 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_debugging.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_debugging.cpp
@@ -35,9 +35,7 @@ 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";
- case ReportTypeMutexHeldWrongContext:
- return "mutex-held-in-wrong-context";
- // No default case so compiler warns us if we miss one
+ // 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 5154662034c56d9..6bd72e18d942580 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp
@@ -435,26 +435,4 @@ void __tsan_mutex_post_divert(void *addr, unsigned flagz) {
ThreadIgnoreBegin(thr, 0);
ThreadIgnoreSyncBegin(thr, 0);
}
-
-static void ReportMutexHeldWrongContext(ThreadState *thr, uptr pc) {
- ThreadRegistryLock l(&ctx->thread_registry);
- ScopedReport rep(ReportTypeMutexHeldWrongContext);
- 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;
- }
- ReportMutexHeldWrongContext(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 35cb6710a54fa42..4028d1810784095 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_report.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_report.cpp
@@ -93,9 +93,7 @@ static const char *ReportTypeString(ReportType typ, uptr tag) {
return "signal handler spoils errno";
case ReportTypeDeadlock:
return "lock-order-inversion (potential deadlock)";
- case ReportTypeMutexHeldWrongContext:
- return "mutex held in the wrong context";
- // No default case so compiler warns us if we miss one
+ // No default case so compiler warns us if we miss one
}
UNREACHABLE("missing case");
}
diff --git a/compiler-rt/lib/tsan/rtl/tsan_report.h b/compiler-rt/lib/tsan/rtl/tsan_report.h
index bfe470797f8f77a..3c88864af147704 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_report.h
+++ b/compiler-rt/lib/tsan/rtl/tsan_report.h
@@ -34,8 +34,7 @@ enum ReportType {
ReportTypeMutexBadReadUnlock,
ReportTypeSignalUnsafe,
ReportTypeErrnoInSignal,
- ReportTypeDeadlock,
- ReportTypeMutexHeldWrongContext
+ ReportTypeDeadlock
};
struct ReportStack {
diff --git a/compiler-rt/lib/tsan/rtl/tsan_suppressions.cpp b/compiler-rt/lib/tsan/rtl/tsan_suppressions.cpp
index 70642124990d7bd..9cdfa32a9343029 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_suppressions.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_suppressions.cpp
@@ -81,7 +81,6 @@ static const char *conv(ReportType typ) {
case ReportTypeMutexBadUnlock:
case ReportTypeMutexBadReadLock:
case ReportTypeMutexBadReadUnlock:
- case ReportTypeMutexHeldWrongContext:
return kSuppressionMutex;
case ReportTypeSignalUnsafe:
case ReportTypeErrnoInSignal:
diff --git a/compiler-rt/test/tsan/mutex_held_wrong_context.cpp b/compiler-rt/test/tsan/mutex_held_wrong_context.cpp
deleted file mode 100644
index e9ded8fc1dc54d4..000000000000000
--- a/compiler-rt/test/tsan/mutex_held_wrong_context.cpp
+++ /dev/null
@@ -1,34 +0,0 @@
-// RUN: %clangxx_tsan -O1 %s -o %t && %deflake %run %t | FileCheck %s
-#include "test.h"
-
-pthread_mutex_t mtx;
-
-void Func1() {
- pthread_mutex_lock(&mtx);
- __tsan_check_no_mutexes_held();
- pthread_mutex_unlock(&mtx);
-}
-
-void Func2() {
- pthread_mutex_lock(&mtx);
- pthread_mutex_unlock(&mtx);
- __tsan_check_no_mutexes_held();
-}
-
-int main() {
- pthread_mutex_init(&mtx, NULL);
- Func1();
- Func2();
- return 0;
-}
-
-// CHECK: WARNING: ThreadSanitizer: mutex held in the wrong context
-// CHECK: #0 __tsan_check_no_mutexes_held
-// CHECK: #1 Func1
-// CHECK: #2 main
-// CHECK: Mutex {{.*}} created at:
-// CHECK: #0 pthread_mutex_init
-// CHECK: #1 main
-// CHECK: SUMMARY: ThreadSanitizer: mutex held in the wrong context {{.*}}mutex_held_wrong_context.cpp{{.*}}Func1
-
-// CHECK-NOT: SUMMARY: ThreadSanitizer: mutex held in the wrong context {{.*}}mutex_held_wrong_context.cpp{{.*}}Func2
More information about the llvm-commits
mailing list