Skip to content

Commit

Permalink
[TSAN] Add __tsan_check_no_mutexes_held helper (#71568)
Browse files Browse the repository at this point in the history
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 resubmits and fixes #69372 (was reverted because of build
breakage).
This also includes the followup change #71471 (to fix a land race).
  • Loading branch information
kennyyu authored Nov 8, 2023
1 parent 5f29555 commit 1146d96
Show file tree
Hide file tree
Showing 8 changed files with 70 additions and 3 deletions.
4 changes: 4 additions & 0 deletions compiler-rt/include/sanitizer/tsan_interface.h
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,10 @@ 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.
Expand Down
1 change: 1 addition & 0 deletions compiler-rt/lib/tsan/rtl/tsan.syms.extra
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion compiler-rt/lib/tsan/rtl/tsan_debugging.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 ReportTypeMutexHeldWrongContext:
return "mutex-held-in-wrong-context";
// No default case so compiler warns us if we miss one
}
UNREACHABLE("missing case");
}
Expand Down
22 changes: 22 additions & 0 deletions compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -435,4 +435,26 @@ 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"
4 changes: 3 additions & 1 deletion compiler-rt/lib/tsan/rtl/tsan_report.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 ReportTypeMutexHeldWrongContext:
return "mutex held in the wrong context";
// No default case so compiler warns us if we miss one
}
UNREACHABLE("missing case");
}
Expand Down
3 changes: 2 additions & 1 deletion compiler-rt/lib/tsan/rtl/tsan_report.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ enum ReportType {
ReportTypeMutexBadReadUnlock,
ReportTypeSignalUnsafe,
ReportTypeErrnoInSignal,
ReportTypeDeadlock
ReportTypeDeadlock,
ReportTypeMutexHeldWrongContext
};

struct ReportStack {
Expand Down
1 change: 1 addition & 0 deletions compiler-rt/lib/tsan/rtl/tsan_suppressions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ static const char *conv(ReportType typ) {
case ReportTypeMutexBadUnlock:
case ReportTypeMutexBadReadLock:
case ReportTypeMutexBadReadUnlock:
case ReportTypeMutexHeldWrongContext:
return kSuppressionMutex;
case ReportTypeSignalUnsafe:
case ReportTypeErrnoInSignal:
Expand Down
34 changes: 34 additions & 0 deletions compiler-rt/test/tsan/mutex_held_wrong_context.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// RUN: %clangxx_tsan -O1 %s -o %t && %deflake %run %t | FileCheck %s
#include "test.h"

pthread_mutex_t mtx;

__attribute__((noinline)) void Func1() {
pthread_mutex_lock(&mtx);
__tsan_check_no_mutexes_held();
pthread_mutex_unlock(&mtx);
}

__attribute__((noinline)) 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: {{.*}}__tsan_check_no_mutexes_held{{.*}}
// CHECK: {{.*}}Func1{{.*}}
// CHECK: {{.*}}main{{.*}}
// CHECK: Mutex {{.*}} created at:
// CHECK: {{.*}}pthread_mutex_init{{.*}}
// CHECK: {{.*}}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

0 comments on commit 1146d96

Please sign in to comment.