[compiler-rt] [TSan][compiler-rt] Move symbolization in ReportRace outside of locks on Apple platforms (PR #149516)
Dan Blackwell via llvm-commits
llvm-commits at lists.llvm.org
Fri Jul 18 07:01:32 PDT 2025
https://github.com/DanBlackwell created https://github.com/llvm/llvm-project/pull/149516
Other platforms should still symbolize within the locked region.
rdar://152829396
>From 64fa7e50c61a731af5a114fb440da8437a9a9ba9 Mon Sep 17 00:00:00 2001
From: Dan Blackwell <dan_blackwell at apple.com>
Date: Fri, 18 Jul 2025 14:47:21 +0100
Subject: [PATCH] [TSan][compiler-rt] Move symbolization in ReportRace outside
of locks on Apple platforms
---
.../lib/tsan/rtl/tsan_interceptors_posix.cpp | 1 +
.../lib/tsan/rtl/tsan_interface_ann.cpp | 1 +
compiler-rt/lib/tsan/rtl/tsan_mman.cpp | 1 +
compiler-rt/lib/tsan/rtl/tsan_report.h | 8 +
compiler-rt/lib/tsan/rtl/tsan_rtl.h | 1 +
compiler-rt/lib/tsan/rtl/tsan_rtl_access.cpp | 4 +
compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cpp | 9 +-
compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp | 185 ++++++++++++------
compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp | 1 +
9 files changed, 146 insertions(+), 65 deletions(-)
diff --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
index 14b25a8995dab..274a38390bda8 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
@@ -2146,6 +2146,7 @@ static void ReportErrnoSpoiling(ThreadState *thr, uptr pc, int sig) {
rep.SetSigNum(sig);
if (!IsFiredSuppression(ctx, ReportTypeErrnoInSignal, stack)) {
rep.AddStack(stack, true);
+ rep.SymbolizeStackElems();
OutputReport(thr, rep);
}
}
diff --git a/compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp b/compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp
index befd6a369026d..d16fd4cfb00d9 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp
@@ -446,6 +446,7 @@ static void ReportMutexHeldWrongContext(ThreadState *thr, uptr pc) {
VarSizeStackTrace trace;
ObtainCurrentStack(thr, pc, &trace);
rep.AddStack(trace, true);
+ rep.SymbolizeStackElems();
OutputReport(thr, rep);
}
diff --git a/compiler-rt/lib/tsan/rtl/tsan_mman.cpp b/compiler-rt/lib/tsan/rtl/tsan_mman.cpp
index 0ea83fb3b5982..3a0a6a1a4564f 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_mman.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_mman.cpp
@@ -185,6 +185,7 @@ static void SignalUnsafeCall(ThreadState *thr, uptr pc) {
ThreadRegistryLock l(&ctx->thread_registry);
ScopedReport rep(ReportTypeSignalUnsafe);
rep.AddStack(stack, true);
+ rep.SymbolizeStackElems();
OutputReport(thr, rep);
}
diff --git a/compiler-rt/lib/tsan/rtl/tsan_report.h b/compiler-rt/lib/tsan/rtl/tsan_report.h
index bfe470797f8f7..50501060840a4 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_report.h
+++ b/compiler-rt/lib/tsan/rtl/tsan_report.h
@@ -12,6 +12,8 @@
#ifndef TSAN_REPORT_H
#define TSAN_REPORT_H
+#include "sanitizer_common/sanitizer_internal_defs.h"
+#include "sanitizer_common/sanitizer_stacktrace.h"
#include "sanitizer_common/sanitizer_symbolizer.h"
#include "sanitizer_common/sanitizer_thread_registry.h"
#include "sanitizer_common/sanitizer_vector.h"
@@ -56,6 +58,7 @@ struct ReportMop {
bool atomic;
uptr external_tag;
Vector<ReportMopMutex> mset;
+ StackTrace stack_trace;
ReportStack *stack;
ReportMop();
@@ -79,6 +82,7 @@ struct ReportLocation {
int fd = 0;
bool fd_closed = false;
bool suppressable = false;
+ StackID stack_id = 0;
ReportStack *stack = nullptr;
};
@@ -89,12 +93,15 @@ struct ReportThread {
ThreadType thread_type;
char *name;
Tid parent_tid;
+ StackID stack_id;
ReportStack *stack;
+ bool suppressable;
};
struct ReportMutex {
int id;
uptr addr;
+ StackID stack_id;
ReportStack *stack;
};
@@ -105,6 +112,7 @@ class ReportDesc {
Vector<ReportStack*> stacks;
Vector<ReportMop*> mops;
Vector<ReportLocation*> locs;
+ Vector<uptr> added_location_addrs;
Vector<ReportMutex*> mutexes;
Vector<ReportThread*> threads;
Vector<Tid> unique_tids;
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl.h b/compiler-rt/lib/tsan/rtl/tsan_rtl.h
index dc32980e905f2..7122c15bb2c13 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl.h
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl.h
@@ -420,6 +420,7 @@ class ScopedReportBase {
void AddSleep(StackID stack_id);
void SetCount(int count);
void SetSigNum(int sig);
+ void SymbolizeStackElems(void);
const ReportDesc *GetReport() const;
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_access.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl_access.cpp
index 487fa490636eb..0d133aead1ee1 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl_access.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_access.cpp
@@ -419,6 +419,10 @@ NOINLINE void TraceRestartMemoryAccess(ThreadState* thr, uptr pc, uptr addr,
ALWAYS_INLINE USED void MemoryAccess(ThreadState* thr, uptr pc, uptr addr,
uptr size, AccessType typ) {
+#if SANITIZER_APPLE
+ if (thr->in_symbolizer)
+ return;
+#endif
RawShadow* shadow_mem = MemToShadow(addr);
UNUSED char memBuf[4][64];
DPrintf2("#%d: Access: %d@%d %p/%zd typ=0x%x {%s, %s, %s, %s}\n", thr->tid,
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cpp
index 2a8aa1915c9ae..f546931ff0e56 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cpp
@@ -62,6 +62,7 @@ static void ReportMutexMisuse(ThreadState *thr, uptr pc, ReportType typ,
ObtainCurrentStack(thr, pc, &trace);
rep.AddStack(trace, true);
rep.AddLocation(addr, 1);
+ rep.SymbolizeStackElems();
OutputReport(thr, rep);
}
@@ -539,13 +540,16 @@ void ReportDeadlock(ThreadState *thr, uptr pc, DDReport *r) {
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) {
- rep.AddStack(StackDepotGet(stk), true);
+ 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.
- rep.AddStack(StackTrace(&dummy_pc, 1), true);
+ stack = StackTrace(&dummy_pc, 1);
}
+ rep.AddStack(stack, true);
+ rep.SymbolizeStackElems();
}
}
OutputReport(thr, rep);
@@ -572,6 +576,7 @@ void ReportDestroyLocked(ThreadState *thr, uptr pc, uptr addr,
return;
rep.AddStack(trace, true);
rep.AddLocation(addr, 1);
+ rep.SymbolizeStackElems();
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 0820bf1adee43..991e5c60399f0 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp
@@ -10,6 +10,8 @@
//
//===----------------------------------------------------------------------===//
+#include <alloca.h>
+
#include "sanitizer_common/sanitizer_common.h"
#include "sanitizer_common/sanitizer_libc.h"
#include "sanitizer_common/sanitizer_placement_new.h"
@@ -187,10 +189,8 @@ void ScopedReportBase::AddMemoryAccess(uptr addr, uptr external_tag, Shadow s,
mop->size = size;
mop->write = !(typ & kAccessRead);
mop->atomic = typ & kAccessAtomic;
- mop->stack = SymbolizeStack(stack);
mop->external_tag = external_tag;
- if (mop->stack)
- mop->stack->suppressable = true;
+ mop->stack_trace = stack;
for (uptr i = 0; i < mset->Size(); i++) {
MutexSet::Desc d = mset->Get(i);
int id = this->AddMutex(d.addr, d.stack_id);
@@ -199,6 +199,45 @@ void ScopedReportBase::AddMemoryAccess(uptr addr, uptr external_tag, Shadow s,
}
}
+void ScopedReportBase::SymbolizeStackElems() {
+ // symbolize memory ops
+ for (usize i = 0; i < rep_->mops.Size(); i++) {
+ ReportMop *mop = rep_->mops[i];
+ mop->stack = SymbolizeStack(mop->stack_trace);
+ if (mop->stack)
+ mop->stack->suppressable = true;
+ }
+
+ // symbolize locations
+ for (usize i = 0; i < rep_->locs.Size(); i++) {
+ ReportLocation *loc = rep_->locs[i];
+ loc->stack = SymbolizeStackId(loc->stack_id);
+ }
+
+ for (usize i = 0; i < rep_->added_location_addrs.Size(); i++) {
+ if (ReportLocation *loc = SymbolizeData(rep_->added_location_addrs[i])) {
+ loc->suppressable = true;
+ // TODO: These all end up at the back of locs - line them up with their
+ // corresponding location
+ rep_->locs.PushBack(loc);
+ }
+ }
+
+ // symbolize threads
+ for (usize i = 0; i < rep_->threads.Size(); i++) {
+ ReportThread *rt = rep_->threads[i];
+ rt->stack = SymbolizeStackId(rt->stack_id);
+ if (rt->stack)
+ rt->stack->suppressable = rt->suppressable;
+ }
+
+ // symbolize mutexes
+ for (usize i = 0; i < rep_->mutexes.Size(); i++) {
+ ReportMutex *rm = rep_->mutexes[i];
+ rm->stack = SymbolizeStackId(rm->stack_id);
+ }
+}
+
void ScopedReportBase::AddUniqueTid(Tid unique_tid) {
rep_->unique_tids.PushBack(unique_tid);
}
@@ -216,10 +255,8 @@ void ScopedReportBase::AddThread(const ThreadContext *tctx, bool suppressable) {
rt->name = internal_strdup(tctx->name);
rt->parent_tid = tctx->parent_tid;
rt->thread_type = tctx->thread_type;
- rt->stack = 0;
- rt->stack = SymbolizeStackId(tctx->creation_stack_id);
- if (rt->stack)
- rt->stack->suppressable = suppressable;
+ rt->stack_id = tctx->creation_stack_id;
+ rt->suppressable = suppressable;
}
#if !SANITIZER_GO
@@ -270,7 +307,7 @@ int ScopedReportBase::AddMutex(uptr addr, StackID creation_stack_id) {
rep_->mutexes.PushBack(rm);
rm->id = rep_->mutexes.Size() - 1;
rm->addr = addr;
- rm->stack = SymbolizeStackId(creation_stack_id);
+ rm->stack_id = creation_stack_id;
return rm->id;
}
@@ -288,7 +325,7 @@ void ScopedReportBase::AddLocation(uptr addr, uptr size) {
loc->fd_closed = closed;
loc->fd = fd;
loc->tid = creat_tid;
- loc->stack = SymbolizeStackId(creat_stack);
+ loc->stack_id = creat_stack;
rep_->locs.PushBack(loc);
AddThread(creat_tid);
return;
@@ -310,7 +347,7 @@ void ScopedReportBase::AddLocation(uptr addr, uptr size) {
loc->heap_chunk_size = b->siz;
loc->external_tag = b->tag;
loc->tid = b->tid;
- loc->stack = SymbolizeStackId(b->stk);
+ loc->stack_id = b->stk;
rep_->locs.PushBack(loc);
AddThread(b->tid);
return;
@@ -324,11 +361,7 @@ void ScopedReportBase::AddLocation(uptr addr, uptr size) {
AddThread(tctx);
}
#endif
- if (ReportLocation *loc = SymbolizeData(addr)) {
- loc->suppressable = true;
- rep_->locs.PushBack(loc);
- return;
- }
+ rep_->added_location_addrs.PushBack(addr);
}
#if !SANITIZER_GO
@@ -761,65 +794,91 @@ void ReportRace(ThreadState *thr, RawShadow *shadow_mem, Shadow cur, Shadow old,
DynamicMutexSet mset1;
MutexSet *mset[kMop] = {&thr->mset, mset1};
- // We need to lock the slot during RestoreStack because it protects
- // the slot journal.
- Lock slot_lock(&ctx->slots[static_cast<uptr>(s[1].sid())].mtx);
- ThreadRegistryLock l0(&ctx->thread_registry);
- Lock slots_lock(&ctx->slot_mtx);
- if (SpuriousRace(old))
- return;
- if (!RestoreStack(EventType::kAccessExt, s[1].sid(), s[1].epoch(), addr1,
- size1, typ1, &tids[1], &traces[1], mset[1], &tags[1])) {
- StoreShadow(&ctx->last_spurious_race, old.raw());
- return;
- }
+#if SANITIZER_APPLE
+ static Mutex report_write_mutex;
+ Lock report_write_lock(&report_write_mutex);
+ ScopedReport *_rep = (ScopedReport *)alloca(sizeof(ScopedReport));
+#endif
+ // Take a new scope as Apple platforms need to release the below locks,
+ // before writing out the report, in order to avoid a deadlock
+ {
+ // We need to lock the slot during RestoreStack because it protects
+ // the slot journal.
+ Lock slot_lock(&ctx->slots[static_cast<uptr>(s[1].sid())].mtx);
+ ThreadRegistryLock l0(&ctx->thread_registry);
+ Lock slots_lock(&ctx->slot_mtx);
+ if (SpuriousRace(old))
+ return;
+ if (!RestoreStack(EventType::kAccessExt, s[1].sid(), s[1].epoch(), addr1,
+ size1, typ1, &tids[1], &traces[1], mset[1], &tags[1])) {
+ 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;
+ }
}
- }
- ScopedReport rep(rep_typ, tag);
- for (uptr i = 0; i < kMop; i++)
- rep.AddMemoryAccess(addr, tags[i], s[i], tids[i], traces[i], mset[i]);
+#if SANITIZER_APPLE
+ new (_rep) ScopedReport(rep_typ, tag);
+ ScopedReport &rep = *_rep;
+#else
+ ScopedReport rep(rep_typ, tag);
+#endif
+ 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++) {
- ThreadContext *tctx = static_cast<ThreadContext *>(
- ctx->thread_registry.GetThreadLocked(tids[i]));
- rep.AddThread(tctx);
- }
+ for (uptr i = 0; i < kMop; i++) {
+ ThreadContext *tctx = static_cast<ThreadContext *>(
+ ctx->thread_registry.GetThreadLocked(tids[i]));
+ rep.AddThread(tctx);
+ }
- 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;
- ThreadContext *parent_tctx = static_cast<ThreadContext *>(
- ctx->thread_registry.GetThreadLocked(parent_tid));
- rep.AddThread(parent_tctx);
+ 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;
+ ThreadContext *parent_tctx = static_cast<ThreadContext *>(
+ ctx->thread_registry.GetThreadLocked(parent_tid));
+ rep.AddThread(parent_tctx);
+ }
}
- }
#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
+
+#if SANITIZER_APPLE
+ } // Close this scope to release the locks
+
+ _rep->SymbolizeStackElems();
+ OutputReport(thr, *_rep);
+
+ // Need to manually destroy this because we used placement new to allocate
+ _rep->~ScopedReport();
+#else
+ rep.SymbolizeStackElems();
+ OutputReport(thr, rep);
+ }
#endif
- OutputReport(thr, rep);
}
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 8d29e25a6dd20..d7083e3dace7b 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp
@@ -96,6 +96,7 @@ void ThreadFinalize(ThreadState *thr) {
ScopedReport rep(ReportTypeThreadLeak);
rep.AddThread(leaks[i].tctx, true);
rep.SetCount(leaks[i].count);
+ rep.SymbolizeStackElems();
OutputReport(thr, rep);
}
#endif
More information about the llvm-commits
mailing list