[llvm-branch-commits] [NFC][TSan] Lock ThreadRegistryLock inside ScopedReport::AddThread and AddLocation (PR #228814)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Sat Oct 3 20:15:48 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-compiler-rt-sanitizer
Author: Vitaly Buka (vitalybuka)
<details>
<summary>Changes</summary>
Acquire ThreadRegistryLock inside ScopedReport::AddThread and ScopedReport::AddLocation instead of requiring callers to lock ctx->thread_registry around ScopedReport population.
Assisted-by: Gemini
---
Full diff: https://github.com/llvm/llvm-project/pull/228814.diff
4 Files Affected:
- (modified) compiler-rt/lib/tsan/rtl/tsan_rtl.h (+2)
- (modified) compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cpp (+23-38)
- (modified) compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp (+32-28)
- (modified) compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp (+2-7)
``````````diff
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl.h b/compiler-rt/lib/tsan/rtl/tsan_rtl.h
index a4bf790d5980b..5b4e5362779e7 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl.h
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl.h
@@ -440,6 +440,8 @@ class ScopedReport {
ScopedIgnoreInterceptors ignore_interceptors_;
ScopedErrorReportLock lock_;
+ void AddThreadLocked(const ThreadContext* tctx, bool suppressable = false);
+
ScopedReport(const ScopedReport&) = delete;
void operator=(const ScopedReport&) = delete;
};
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cpp
index 8ae9c1f4b20e9..8d5bad370a5a0 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cpp
@@ -58,14 +58,9 @@ static void ReportMutexMisuse(ThreadState *thr, uptr pc, ReportType typ,
VarSizeStackTrace trace;
ObtainCurrentStack(thr, pc, &trace);
ScopedReport rep(typ);
- // Release locks before symbolizing and outputting the report to avoid
- // deadlocks.
- {
- ThreadRegistryLock l(&ctx->thread_registry);
- rep.AddMutex(addr, creation_stack_id);
- rep.AddStack(trace, true);
- rep.AddLocation(addr, 1);
- }
+ rep.AddMutex(addr, creation_stack_id);
+ rep.AddStack(trace, true);
+ rep.AddLocation(addr, 1);
OutputReport(thr, rep);
}
@@ -534,28 +529,23 @@ void ReportDeadlock(ThreadState *thr, uptr pc, DDReport *r) {
return;
uptr dummy_pc = 0x42;
ScopedReport rep(ReportTypeDeadlock);
- // Release locks before symbolizing and outputting the report to avoid
- // deadlocks.
- {
- 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);
- rep.AddThread((int)r->loop[i].thr_ctx);
- }
- for (int i = 0; i < r->n; i++) {
- for (int j = 0; j < (flags()->second_deadlock_stack ? 2 : 1); j++) {
- u32 stk = r->loop[i].stk[j];
- StackTrace stack;
- if (stk && stk != kInvalidStackID) {
- stack = StackDepotGet(stk);
- } else {
- // Sometimes we fail to extract the stack trace (FIXME: investigate),
- // but we should still produce some stack trace in the report.
- stack = StackTrace(&dummy_pc, 1);
- }
- rep.AddStack(stack, true);
+ 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);
+ rep.AddThread((int)r->loop[i].thr_ctx);
+ }
+ for (int i = 0; i < r->n; i++) {
+ for (int j = 0; j < (flags()->second_deadlock_stack ? 2 : 1); j++) {
+ u32 stk = r->loop[i].stk[j];
+ StackTrace stack;
+ if (stk && stk != kInvalidStackID) {
+ stack = StackDepotGet(stk);
+ } else {
+ // Sometimes we fail to extract the stack trace (FIXME: investigate),
+ // but we should still produce some stack trace in the report.
+ stack = StackTrace(&dummy_pc, 1);
}
+ rep.AddStack(stack, true);
}
}
OutputReport(thr, rep);
@@ -583,15 +573,10 @@ void ReportDestroyLocked(ThreadState *thr, uptr pc, uptr addr,
ObtainCurrentStack(thr, pc, &trace);
ScopedReport rep(ReportTypeMutexDestroyLocked);
- // Release locks before symbolizing and outputting the report to avoid
- // deadlocks.
- {
- ThreadRegistryLock l0(&ctx->thread_registry);
- rep.AddMutex(addr, creation_stack_id);
- rep.AddStack(trace, true);
- rep.AddStack(last_lock_stack, true);
- rep.AddLocation(addr, 1);
- }
+ rep.AddMutex(addr, creation_stack_id);
+ rep.AddStack(trace, true);
+ rep.AddStack(last_lock_stack, true);
+ rep.AddLocation(addr, 1);
OutputReport(thr, rep);
}
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp
index 50448cfd1233b..1d76a886dc536 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp
@@ -267,6 +267,12 @@ void ScopedReport::AddUniqueTid(Tid unique_tid) {
}
void ScopedReport::AddThread(const ThreadContext* tctx, bool suppressable) {
+ ThreadRegistryLock l(&ctx->thread_registry);
+ AddThreadLocked(tctx, suppressable);
+}
+
+void ScopedReport::AddThreadLocked(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)
@@ -311,10 +317,10 @@ ThreadContext *IsThreadStackOrTls(uptr addr, bool *is_stack) {
#endif
void ScopedReport::AddThread(Tid tid, bool suppressable) {
- ctx->thread_registry.CheckLocked();
+ ThreadRegistryLock l(&ctx->thread_registry);
if (const auto* tctx = static_cast<ThreadContext*>(
ctx->thread_registry.GetThreadLocked(tid)))
- AddThread(tctx, suppressable);
+ AddThreadLocked(tctx, suppressable);
}
int ScopedReport::AddMutex(uptr addr, StackID creation_stack_id) {
@@ -372,12 +378,15 @@ void ScopedReport::AddLocation(uptr addr, uptr size) {
return;
}
bool is_stack = false;
- if (ThreadContext *tctx = IsThreadStackOrTls(addr, &is_stack)) {
- auto *loc = New<ReportLocation>();
- loc->type = is_stack ? ReportLocationStack : ReportLocationTLS;
- loc->tid = tctx->tid;
- rep_->locs.PushBack(loc);
- AddThread(tctx);
+ {
+ ThreadRegistryLock l(&ctx->thread_registry);
+ if (ThreadContext* tctx = IsThreadStackOrTls(addr, &is_stack)) {
+ auto* loc = New<ReportLocation>();
+ loc->type = is_stack ? ReportLocationStack : ReportLocationTLS;
+ loc->tid = tctx->tid;
+ rep_->locs.PushBack(loc);
+ AddThreadLocked(tctx);
+ }
}
#endif
rep_->added_location_addrs.PushBack({addr, rep_->locs.Size()});
@@ -843,33 +852,28 @@ void ReportRace(ThreadState *thr, RawShadow *shadow_mem, Shadow cur, Shadow old,
}
ScopedReport rep(rep_typ, tag);
- // Release locks before symbolizing and outputting the report to avoid
- // deadlocks.
- {
- 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]);
+ for (uptr i = 0; i < kMop; i++)
+ rep.AddMemoryAccess(addr, tags[i], s[i], tids[i], traces[i], mset[i]);
- for (uptr i = 0; i < kMop; i++) rep.AddThread(tids[i]);
+ for (uptr i = 0; i < kMop; i++) rep.AddThread(tids[i]);
- rep.AddLocation(addr_min, addr_max - addr_min);
+ rep.AddLocation(addr_min, addr_max - addr_min);
- if (flags()->print_full_thread_history) {
- const ReportDesc* rep_desc = rep.GetReport();
- for (uptr i = 0; i < rep_desc->threads.Size(); i++) {
- Tid parent_tid = rep_desc->threads[i]->parent_tid;
- if (parent_tid == kMainTid || parent_tid == kInvalidTid)
- continue;
- rep.AddThread(parent_tid);
- }
+ if (flags()->print_full_thread_history) {
+ const ReportDesc* rep_desc = rep.GetReport();
+ for (uptr i = 0; i < rep_desc->threads.Size(); i++) {
+ Tid parent_tid = rep_desc->threads[i]->parent_tid;
+ if (parent_tid == kMainTid || parent_tid == kInvalidTid)
+ continue;
+ rep.AddThread(parent_tid);
}
+ }
#if !SANITIZER_GO
- if (!((typ0 | typ1) & kAccessFree) &&
- s[1].epoch() <= thr->last_sleep_clock.Get(s[1].sid()))
- rep.AddSleep(thr->last_sleep_stack_id);
+ if (!((typ0 | typ1) & kAccessFree) &&
+ s[1].epoch() <= thr->last_sleep_clock.Get(s[1].sid()))
+ rep.AddSleep(thr->last_sleep_stack_id);
#endif
- }
OutputReport(thr, rep);
}
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp
index 5edf0c16fdd0d..da9e56a5b8f8d 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp
@@ -97,13 +97,8 @@ void ThreadFinalize(ThreadState *thr) {
for (uptr i = 0; i < leaks.Size(); i++) {
ScopedReport rep(ReportTypeThreadLeak);
- // Release locks before symbolizing and outputting the report to avoid
- // deadlocks.
- {
- ThreadRegistryLock l(&ctx->thread_registry);
- rep.AddThread(leaks[i].tctx, true);
- rep.SetCount(leaks[i].count);
- }
+ rep.AddThread(leaks[i].tctx, true);
+ rep.SetCount(leaks[i].count);
OutputReport(thr, rep);
}
#endif
``````````
</details>
https://github.com/llvm/llvm-project/pull/228814
More information about the llvm-branch-commits
mailing list