[compiler-rt] cebadce - [TSan] Release locks before OutputReport on all platforms (#228554)
via llvm-commits
llvm-commits at lists.llvm.org
Sat Oct 3 09:53:07 PDT 2026
Author: Vitaly Buka
Date: 2026-10-03T09:53:00-07:00
New Revision: cebadce0f60f5542a816fff0c559032fd8751a3d
URL: https://github.com/llvm/llvm-project/commit/cebadce0f60f5542a816fff0c559032fd8751a3d
DIFF: https://github.com/llvm/llvm-project/commit/cebadce0f60f5542a816fff0c559032fd8751a3d.diff
LOG: [TSan] Release locks before OutputReport on all platforms (#228554)
https://github.com/llvm/llvm-project/pull/151495 moved OutputReport
outside the lock scope (ThreadRegistryLock, slot_lock, slots_lock) on
Apple platforms only.
Holding these locks during OutputReport can also deadlock on non-Apple
platforms. For example, when halt_on_error=1, abort_on_error=1, and
handle_abort=0 are set, OutputReport calls Die() -> Abort(), which
raises SIGABRT and invokes a user signal handler via
CallUserSignalHandler with TSan instrumentation re-enabled. If the
signal handler executes enough instrumented code to overflow the
active TracePart, TraceSwitchPartImpl -> TracePartAlloc attempts to
acquire ctx->slot_mtx and self-deadlocks because slots_lock is still
held by ReportRace.
Remove the SANITIZER_APPLE conditional compilation so locks are
released before OutputReport across all platforms.
Assisted-by: Gemini
Added:
compiler-rt/test/tsan/signal_abort_on_error.cpp
Modified:
compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp
compiler-rt/lib/tsan/rtl/tsan_mman.cpp
compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cpp
compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp
compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp
Removed:
################################################################################
diff --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
index c0a75a53e5a1fa..b2ba666741e891 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
@@ -2179,8 +2179,8 @@ static void ReportErrnoSpoiling(ThreadState *thr, uptr pc, int sig) {
// Use alloca, because malloc during signal handling deadlocks
ScopedReport *rep = (ScopedReport *)__builtin_alloca(sizeof(ScopedReport));
bool suppressed;
- // Take a new scope as Apple platforms require the below locks released
- // before symbolizing in order to avoid a deadlock
+ // Release locks before symbolizing and outputting the report to avoid
+ // deadlocks.
{
ThreadRegistryLock l(&ctx->thread_registry);
new (rep) ScopedReport(ReportTypeErrnoInSignal);
@@ -2188,17 +2188,12 @@ static void ReportErrnoSpoiling(ThreadState *thr, uptr pc, int sig) {
suppressed = IsFiredSuppression(ctx, ReportTypeErrnoInSignal, stack);
if (!suppressed)
rep->AddStack(stack, true);
-#if SANITIZER_APPLE
- } // Close this scope to release the locks before writing report
-#endif
- if (!suppressed)
- OutputReport(thr, *rep);
-
- // Need to manually destroy this because we used placement new to allocate
- rep->~ScopedReport();
-#if !SANITIZER_APPLE
}
-#endif
+ if (!suppressed)
+ OutputReport(thr, *rep);
+
+ // Need to manually destroy this because we used placement new to allocate
+ rep->~ScopedReport();
}
static void CallUserSignalHandler(ThreadState *thr, bool sync, bool acquire,
diff --git a/compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp b/compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp
index 490f6cb3bce01b..526dc5a753c24f 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp
@@ -443,8 +443,8 @@ void __tsan_mutex_post_divert(void *addr, unsigned flagz) {
static void ReportMutexHeldWrongContext(ThreadState *thr, uptr pc) {
// Use alloca, because malloc during signal handling deadlocks
ScopedReport *rep = (ScopedReport *)__builtin_alloca(sizeof(ScopedReport));
- // Take a new scope as Apple platforms require the below locks released
- // before symbolizing in order to avoid a deadlock
+ // Release locks before symbolizing and outputting the report to avoid
+ // deadlocks.
{
ThreadRegistryLock l(&ctx->thread_registry);
new (rep) ScopedReport(ReportTypeMutexHeldWrongContext);
@@ -455,16 +455,11 @@ static void ReportMutexHeldWrongContext(ThreadState *thr, uptr pc) {
VarSizeStackTrace trace;
ObtainCurrentStack(thr, pc, &trace);
rep->AddStack(trace, true);
-#if SANITIZER_APPLE
- } // Close this scope to release the locks
-#endif
- OutputReport(thr, *rep);
-
- // Need to manually destroy this because we used placement new to allocate
- rep->~ScopedReport();
-#if !SANITIZER_APPLE
}
-#endif
+ OutputReport(thr, *rep);
+
+ // Need to manually destroy this because we used placement new to allocate
+ rep->~ScopedReport();
}
INTERFACE_ATTRIBUTE
diff --git a/compiler-rt/lib/tsan/rtl/tsan_mman.cpp b/compiler-rt/lib/tsan/rtl/tsan_mman.cpp
index caacb3675825f9..cea833054551d3 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_mman.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_mman.cpp
@@ -184,22 +184,17 @@ static void SignalUnsafeCall(ThreadState *thr, uptr pc) {
return;
// Use alloca, because malloc during signal handling deadlocks
ScopedReport *rep = (ScopedReport *)__builtin_alloca(sizeof(ScopedReport));
- // Take a new scope as Apple platforms require the below locks released
- // before symbolizing in order to avoid a deadlock
+ // Release locks before symbolizing and outputting the report to avoid
+ // deadlocks.
{
ThreadRegistryLock l(&ctx->thread_registry);
new (rep) ScopedReport(ReportTypeSignalUnsafe);
rep->AddStack(stack, true);
-#if SANITIZER_APPLE
- } // Close this scope to release the locks
-#endif
- OutputReport(thr, *rep);
-
- // Need to manually destroy this because we used placement new to allocate
- rep->~ScopedReport();
-#if !SANITIZER_APPLE
}
-#endif
+ OutputReport(thr, *rep);
+
+ // Need to manually destroy this because we used placement new to allocate
+ rep->~ScopedReport();
}
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cpp
index 30f5e964939d03..bc2c62338c2628 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cpp
@@ -58,8 +58,8 @@ static void ReportMutexMisuse(ThreadState *thr, uptr pc, ReportType typ,
return;
// Use alloca, because malloc during signal handling deadlocks
ScopedReport *rep = (ScopedReport *)__builtin_alloca(sizeof(ScopedReport));
- // Take a new scope as Apple platforms require the below locks released
- // before symbolizing in order to avoid a deadlock
+ // Release locks before symbolizing and outputting the report to avoid
+ // deadlocks.
{
ThreadRegistryLock l(&ctx->thread_registry);
new (rep) ScopedReport(typ);
@@ -68,16 +68,11 @@ static void ReportMutexMisuse(ThreadState *thr, uptr pc, ReportType typ,
ObtainCurrentStack(thr, pc, &trace);
rep->AddStack(trace, true);
rep->AddLocation(addr, 1);
-#if SANITIZER_APPLE
- } // Close this scope to release the locks
-#endif
- OutputReport(thr, *rep);
-
- // Need to manually destroy this because we used placement new to allocate
- rep->~ScopedReport();
-#if !SANITIZER_APPLE
}
-#endif
+ OutputReport(thr, *rep);
+
+ // Need to manually destroy this because we used placement new to allocate
+ rep->~ScopedReport();
}
static void RecordMutexLock(ThreadState *thr, uptr pc, uptr addr,
@@ -545,8 +540,8 @@ void ReportDeadlock(ThreadState *thr, uptr pc, DDReport *r) {
return;
// Use alloca, because malloc during signal handling deadlocks
ScopedReport *rep = (ScopedReport *)__builtin_alloca(sizeof(ScopedReport));
- // Take a new scope as Apple platforms require the below locks released
- // before symbolizing in order to avoid a deadlock
+ // Release locks before symbolizing and outputting the report to avoid
+ // deadlocks.
{
ThreadRegistryLock l(&ctx->thread_registry);
new (rep) ScopedReport(ReportTypeDeadlock);
@@ -570,24 +565,19 @@ void ReportDeadlock(ThreadState *thr, uptr pc, DDReport *r) {
rep->AddStack(stack, true);
}
}
-#if SANITIZER_APPLE
- } // Close this scope to release the locks
-#endif
- OutputReport(thr, *rep);
-
- // Need to manually destroy this because we used placement new to allocate
- rep->~ScopedReport();
-#if !SANITIZER_APPLE
}
-#endif
+ OutputReport(thr, *rep);
+
+ // Need to manually destroy this because we used placement new to allocate
+ rep->~ScopedReport();
}
void ReportDestroyLocked(ThreadState *thr, uptr pc, uptr addr,
FastState last_lock, StackID creation_stack_id) {
// Use alloca, because malloc during signal handling deadlocks
ScopedReport *rep = (ScopedReport *)__builtin_alloca(sizeof(ScopedReport));
- // Take a new scope as Apple platforms require the below locks released
- // before symbolizing in order to avoid a deadlock
+ // Release locks before symbolizing and outputting the report to avoid
+ // deadlocks.
{
// We need to lock the slot during RestoreStack because it protects
// the slot journal.
@@ -608,16 +598,11 @@ void ReportDestroyLocked(ThreadState *thr, uptr pc, uptr addr,
return;
rep->AddStack(trace, true);
rep->AddLocation(addr, 1);
-#if SANITIZER_APPLE
- } // Close this scope to release the locks
-#endif
- OutputReport(thr, *rep);
-
- // Need to manually destroy this because we used placement new to allocate
- rep->~ScopedReport();
-#if !SANITIZER_APPLE
}
-#endif
+ OutputReport(thr, *rep);
+
+ // Need to manually destroy this because we used placement new to allocate
+ rep->~ScopedReport();
}
} // namespace __tsan
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp
index 3bf1d6a5a81e11..d527ba4d6ccc52 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp
@@ -815,8 +815,8 @@ void ReportRace(ThreadState *thr, RawShadow *shadow_mem, Shadow cur, Shadow old,
// Use alloca, because malloc during signal handling deadlocks
ScopedReport *rep = (ScopedReport *)__builtin_alloca(sizeof(ScopedReport));
- // Take a new scope as Apple platforms require the below locks released
- // before symbolizing in order to avoid a deadlock
+ // Release locks before symbolizing and outputting the report to avoid
+ // deadlocks.
{
// We need to lock the slot during RestoreStack because it protects
// the slot journal.
@@ -876,17 +876,11 @@ void ReportRace(ThreadState *thr, RawShadow *shadow_mem, Shadow cur, Shadow old,
s[1].epoch() <= thr->last_sleep_clock.Get(s[1].sid()))
rep->AddSleep(thr->last_sleep_stack_id);
#endif
-
-#if SANITIZER_APPLE
- } // Close this scope to release the locks
-#endif
- OutputReport(thr, *rep);
-
- // Need to manually destroy this because we used placement new to allocate
- rep->~ScopedReport();
-#if !SANITIZER_APPLE
}
-#endif
+ OutputReport(thr, *rep);
+
+ // Need to manually destroy this because we used placement new to allocate
+ rep->~ScopedReport();
}
void PrintCurrentStack(ThreadState *thr, uptr pc) {
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp
index 978d853b0bc7e1..afc6b8d4a0b1cd 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp
@@ -98,23 +98,18 @@ void ThreadFinalize(ThreadState *thr) {
for (uptr i = 0; i < leaks.Size(); i++) {
// Use alloca, because malloc during signal handling deadlocks
ScopedReport *rep = (ScopedReport *)__builtin_alloca(sizeof(ScopedReport));
- // Take a new scope as Apple platforms require the below locks released
- // before symbolizing in order to avoid a deadlock
+ // Release locks before symbolizing and outputting the report to avoid
+ // deadlocks.
{
ThreadRegistryLock l(&ctx->thread_registry);
new (rep) ScopedReport(ReportTypeThreadLeak);
rep->AddThread(leaks[i].tctx, true);
rep->SetCount(leaks[i].count);
-# if SANITIZER_APPLE
- } // Close this scope to release the locks
-# endif
- OutputReport(thr, *rep);
-
- // Need to manually destroy this because we used placement new to allocate
- rep->~ScopedReport();
-# if !SANITIZER_APPLE
}
-# endif
+ OutputReport(thr, *rep);
+
+ // Need to manually destroy this because we used placement new to allocate
+ rep->~ScopedReport();
}
#endif
}
diff --git a/compiler-rt/test/tsan/signal_abort_on_error.cpp b/compiler-rt/test/tsan/signal_abort_on_error.cpp
new file mode 100644
index 00000000000000..37612f2fc397b0
--- /dev/null
+++ b/compiler-rt/test/tsan/signal_abort_on_error.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_tsan -O1 %s -o %t && %env_tsan_opts=halt_on_error=1:abort_on_error=1:handle_abort=0 not %run %t 2>&1 | FileCheck %s
+#include "test.h"
+#include <signal.h>
+
+int Global;
+
+__attribute__((noinline)) void step() { asm volatile(""); }
+
+static void handler(int, siginfo_t *, void *) {
+ // Overflow the active 256KB TracePart (~32K events) in the SIGABRT handler
+ // to trigger TraceSwitchPartImpl -> TracePartAlloc, which acquires
+ // ctx->slot_mtx.
+ for (int i = 0; i < 100000; ++i)
+ step();
+ write(2, "SIGNAL\n", 7);
+ _exit(0);
+}
+
+void *Thread(void *x) {
+ Global = 42;
+ barrier_wait(&barrier);
+ return NULL;
+}
+
+int main() {
+ struct sigaction act = {};
+ act.sa_sigaction = &handler;
+ act.sa_flags = SA_SIGINFO;
+ sigaction(SIGABRT, &act, 0);
+
+ barrier_init(&barrier, 2);
+ pthread_t t;
+ pthread_create(&t, NULL, Thread, NULL);
+ pthread_detach(t);
+ barrier_wait(&barrier);
+ Global = 43;
+ return 0;
+}
+
+// CHECK: WARNING: ThreadSanitizer: data race
+// CHECK: SIGNAL
More information about the llvm-commits
mailing list