[compiler-rt] [TSan] Lock ScopedErrorReportLock before slot and thread_registry locks (PR #228614)
Vitaly Buka via llvm-commits
llvm-commits at lists.llvm.org
Sat Oct 3 14:23:45 PDT 2026
https://github.com/vitalybuka updated https://github.com/llvm/llvm-project/pull/228614
>From 4b2ad9cf95aa914fc34d6dc78c2d50841e3288ef Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Fri, 2 Oct 2026 16:07:34 -0700
Subject: [PATCH] [TSan] Lock ScopedErrorReportLock before slot and
thread_registry locks
OutputReport runs while ScopedErrorReportLock is held after slot_mtx and
thread_registry have been unlocked. Because code executed during
OutputReport (symbolizer, callbacks, or signal handlers) can acquire
slot_mtx or thread_registry, ScopedErrorReportLock must precede slot and
thread_registry locks in the lock hierarchy to avoid AB-BA deadlocks
between concurrent reports or fork().
- Move ScopedErrorReportLock::Lock() before slot.mtx, thread_registry,
and slot_mtx in ForkBefore (and unlock in reverse order in ForkAfter).
- Replace ctx->thread_registry.CheckLocked() in ScopedReportBase's
constructor with CheckedMutex::CheckNoLocks(), and add CheckLocked() to
AddThread(const ThreadContext *) and CheckNoLocks() to OutputReport.
- Construct ScopedReport before acquiring ThreadRegistryLock across all
reporting functions, and close the RestoreStack lock scope before
constructing ScopedReport in ReportRace.
Assisted-by: Gemini
Pull Request: https://github.com/llvm/llvm-project/pull/228614
---
.../lib/tsan/rtl/tsan_interceptors_posix.cpp | 2 +-
.../lib/tsan/rtl/tsan_interface_ann.cpp | 2 +-
compiler-rt/lib/tsan/rtl/tsan_mman.cpp | 2 +-
compiler-rt/lib/tsan/rtl/tsan_rtl.cpp | 4 +-
compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cpp | 6 +--
compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp | 39 +++++++++++--------
compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp | 2 +-
7 files changed, 31 insertions(+), 26 deletions(-)
diff --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
index b2ba666741e89..a8d502966bcca 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
@@ -2182,8 +2182,8 @@ static void ReportErrnoSpoiling(ThreadState *thr, uptr pc, int sig) {
// Release locks before symbolizing and outputting the report to avoid
// deadlocks.
{
- ThreadRegistryLock l(&ctx->thread_registry);
new (rep) ScopedReport(ReportTypeErrnoInSignal);
+ ThreadRegistryLock l(&ctx->thread_registry);
rep->SetSigNum(sig);
suppressed = IsFiredSuppression(ctx, ReportTypeErrnoInSignal, stack);
if (!suppressed)
diff --git a/compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp b/compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp
index 526dc5a753c24..390cae67d2170 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp
@@ -446,8 +446,8 @@ static void ReportMutexHeldWrongContext(ThreadState *thr, uptr pc) {
// Release locks before symbolizing and outputting the report to avoid
// deadlocks.
{
- ThreadRegistryLock l(&ctx->thread_registry);
new (rep) ScopedReport(ReportTypeMutexHeldWrongContext);
+ ThreadRegistryLock l(&ctx->thread_registry);
for (uptr i = 0; i < thr->mset.Size(); ++i) {
MutexSet::Desc desc = thr->mset.Get(i);
rep->AddMutex(desc.addr, desc.stack_id);
diff --git a/compiler-rt/lib/tsan/rtl/tsan_mman.cpp b/compiler-rt/lib/tsan/rtl/tsan_mman.cpp
index cea833054551d..88cdf58af8d58 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_mman.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_mman.cpp
@@ -187,8 +187,8 @@ static void SignalUnsafeCall(ThreadState *thr, uptr pc) {
// Release locks before symbolizing and outputting the report to avoid
// deadlocks.
{
- ThreadRegistryLock l(&ctx->thread_registry);
new (rep) ScopedReport(ReportTypeSignalUnsafe);
+ ThreadRegistryLock l(&ctx->thread_registry);
rep->AddStack(stack, true);
}
OutputReport(thr, *rep);
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp
index c6e7dce5ecb18..7328caab0a918 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp
@@ -841,10 +841,10 @@ void ForkBefore(ThreadState* thr, uptr pc) SANITIZER_NO_THREAD_SAFETY_ANALYSIS {
// Detaching from the slot makes OnUserFree skip writing to the shadow.
// The slot will be locked so any attempts to use it will deadlock anyway.
SlotDetach(thr);
+ ScopedErrorReportLock::Lock();
for (auto& slot : ctx->slots) slot.mtx.Lock();
ctx->thread_registry.Lock();
ctx->slot_mtx.Lock();
- ScopedErrorReportLock::Lock();
AllocatorLockBeforeFork();
// Suppress all reports in the pthread_atfork callbacks.
// Reports may deadlock.
@@ -871,10 +871,10 @@ static void ForkAfter(ThreadState* thr,
thr->ignore_interceptors--;
thr->ignore_reads_and_writes--;
AllocatorUnlockAfterFork(child);
- ScopedErrorReportLock::Unlock();
ctx->slot_mtx.Unlock();
ctx->thread_registry.Unlock();
for (auto& slot : ctx->slots) slot.mtx.Unlock();
+ ScopedErrorReportLock::Unlock();
SlotAttachAndLock(thr);
SlotUnlock(thr);
GlobalProcessorUnlock();
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cpp
index da3604a2b840d..49759ab2974fd 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cpp
@@ -61,8 +61,8 @@ static void ReportMutexMisuse(ThreadState *thr, uptr pc, ReportType typ,
// Release locks before symbolizing and outputting the report to avoid
// deadlocks.
{
- ThreadRegistryLock l(&ctx->thread_registry);
new (rep) ScopedReport(typ);
+ ThreadRegistryLock l(&ctx->thread_registry);
rep->AddMutex(addr, creation_stack_id);
VarSizeStackTrace trace;
ObtainCurrentStack(thr, pc, &trace);
@@ -543,8 +543,8 @@ void ReportDeadlock(ThreadState *thr, uptr pc, DDReport *r) {
// Release locks before symbolizing and outputting the report to avoid
// deadlocks.
{
- ThreadRegistryLock l(&ctx->thread_registry);
new (rep) ScopedReport(ReportTypeDeadlock);
+ ThreadRegistryLock l(&ctx->thread_registry);
for (int i = 0; i < r->n; i++) {
rep->AddMutex(r->loop[i].mtx_ctx0, r->loop[i].stk[0]);
rep->AddUniqueTid((int)r->loop[i].thr_ctx);
@@ -579,12 +579,12 @@ void ReportDestroyLocked(ThreadState *thr, uptr pc, uptr addr,
// Release locks before symbolizing and outputting the report to avoid
// deadlocks.
{
+ new (rep) ScopedReport(ReportTypeMutexDestroyLocked);
// We need to lock the slot during RestoreStack because it protects
// the slot journal.
Lock slot_lock(&ctx->slots[static_cast<uptr>(last_lock.sid())].mtx);
ThreadRegistryLock l0(&ctx->thread_registry);
Lock slots_lock(&ctx->slot_mtx);
- new (rep) ScopedReport(ReportTypeMutexDestroyLocked);
rep->AddMutex(addr, creation_stack_id);
VarSizeStackTrace trace;
ObtainCurrentStack(thr, pc, &trace);
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp
index 8fd1471b6fa2c..7d302955374de 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp
@@ -164,7 +164,7 @@ bool ShouldReport(ThreadState *thr, ReportType typ) {
}
ScopedReportBase::ScopedReportBase(ReportType typ, uptr tag) {
- ctx->thread_registry.CheckLocked();
+ CheckedMutex::CheckNoLocks();
rep_ = New<ReportDesc>();
rep_->typ = typ;
rep_->tag = tag;
@@ -256,6 +256,7 @@ void ScopedReportBase::AddUniqueTid(Tid unique_tid) {
}
void ScopedReportBase::AddThread(const ThreadContext *tctx, bool suppressable) {
+ ctx->thread_registry.CheckLocked();
for (uptr i = 0; i < rep_->threads.Size(); i++) {
if ((u32)rep_->threads[i]->id == tctx->tid)
return;
@@ -676,6 +677,7 @@ static bool HandleRacyStacks(ThreadState *thr, VarSizeStackTrace traces[2]) {
}
bool OutputReport(ThreadState *thr, ScopedReport &srep) {
+ CheckedMutex::CheckNoLocks();
// These should have been checked in ShouldReport.
// It's too late to check them here, we have already taken locks.
CHECK(flags()->report_bugs);
@@ -809,10 +811,6 @@ void ReportRace(ThreadState *thr, RawShadow *shadow_mem, Shadow cur, Shadow old,
DynamicMutexSet mset1;
MutexSet *mset[kMop] = {&thr->mset, mset1};
- // Use alloca, because malloc during signal handling deadlocks
- ScopedReport *rep = (ScopedReport *)__builtin_alloca(sizeof(ScopedReport));
- // 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.
@@ -826,24 +824,31 @@ void ReportRace(ThreadState *thr, RawShadow *shadow_mem, Shadow cur, Shadow old,
StoreShadow(&ctx->last_spurious_race, old.raw());
return;
}
+ }
- if (IsFiredSuppression(ctx, rep_typ, traces[1]))
- return;
+ if (IsFiredSuppression(ctx, rep_typ, traces[1]))
+ return;
- if (HandleRacyStacks(thr, traces))
- return;
+ if (HandleRacyStacks(thr, traces))
+ return;
- // If any of the accesses has a tag, treat this as an "external" race.
- uptr tag = kExternalTagNone;
- for (uptr i = 0; i < kMop; i++) {
- if (tags[i] != kExternalTagNone) {
- rep_typ = ReportTypeExternalRace;
- tag = tags[i];
- break;
- }
+ // If any of the accesses has a tag, treat this as an "external" race.
+ uptr tag = kExternalTagNone;
+ for (uptr i = 0; i < kMop; i++) {
+ if (tags[i] != kExternalTagNone) {
+ rep_typ = ReportTypeExternalRace;
+ tag = tags[i];
+ break;
}
+ }
+ // Use alloca, because malloc during signal handling deadlocks
+ ScopedReport* rep = (ScopedReport*)__builtin_alloca(sizeof(ScopedReport));
+ // Release locks before symbolizing and outputting the report to avoid
+ // deadlocks.
+ {
new (rep) ScopedReport(rep_typ, tag);
+ ThreadRegistryLock l0(&ctx->thread_registry);
for (uptr i = 0; i < kMop; i++)
rep->AddMemoryAccess(addr, tags[i], s[i], tids[i], traces[i], mset[i]);
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp
index afc6b8d4a0b1c..c408024e77e60 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp
@@ -101,8 +101,8 @@ void ThreadFinalize(ThreadState *thr) {
// Release locks before symbolizing and outputting the report to avoid
// deadlocks.
{
- ThreadRegistryLock l(&ctx->thread_registry);
new (rep) ScopedReport(ReportTypeThreadLeak);
+ ThreadRegistryLock l(&ctx->thread_registry);
rep->AddThread(leaks[i].tctx, true);
rep->SetCount(leaks[i].count);
}
More information about the llvm-commits
mailing list