Skip to content

Commit 6eea085

Browse files
committed
[TSan][compiler-rt] Move symbolization in ReportRace outside of locks on Apple platforms
1 parent 602d43c commit 6eea085

File tree

9 files changed

+146
-65
lines changed

9 files changed

+146
-65
lines changed

compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2146,6 +2146,7 @@ static void ReportErrnoSpoiling(ThreadState *thr, uptr pc, int sig) {
21462146
rep.SetSigNum(sig);
21472147
if (!IsFiredSuppression(ctx, ReportTypeErrnoInSignal, stack)) {
21482148
rep.AddStack(stack, true);
2149+
rep.SymbolizeStackElems();
21492150
OutputReport(thr, rep);
21502151
}
21512152
}

compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,7 @@ static void ReportMutexHeldWrongContext(ThreadState *thr, uptr pc) {
446446
VarSizeStackTrace trace;
447447
ObtainCurrentStack(thr, pc, &trace);
448448
rep.AddStack(trace, true);
449+
rep.SymbolizeStackElems();
449450
OutputReport(thr, rep);
450451
}
451452

compiler-rt/lib/tsan/rtl/tsan_mman.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,7 @@ static void SignalUnsafeCall(ThreadState *thr, uptr pc) {
185185
ThreadRegistryLock l(&ctx->thread_registry);
186186
ScopedReport rep(ReportTypeSignalUnsafe);
187187
rep.AddStack(stack, true);
188+
rep.SymbolizeStackElems();
188189
OutputReport(thr, rep);
189190
}
190191

compiler-rt/lib/tsan/rtl/tsan_report.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
#ifndef TSAN_REPORT_H
1313
#define TSAN_REPORT_H
1414

15+
#include "sanitizer_common/sanitizer_internal_defs.h"
16+
#include "sanitizer_common/sanitizer_stacktrace.h"
1517
#include "sanitizer_common/sanitizer_symbolizer.h"
1618
#include "sanitizer_common/sanitizer_thread_registry.h"
1719
#include "sanitizer_common/sanitizer_vector.h"
@@ -56,6 +58,7 @@ struct ReportMop {
5658
bool atomic;
5759
uptr external_tag;
5860
Vector<ReportMopMutex> mset;
61+
StackTrace stack_trace;
5962
ReportStack *stack;
6063

6164
ReportMop();
@@ -79,6 +82,7 @@ struct ReportLocation {
7982
int fd = 0;
8083
bool fd_closed = false;
8184
bool suppressable = false;
85+
StackID stack_id = 0;
8286
ReportStack *stack = nullptr;
8387
};
8488

@@ -89,12 +93,15 @@ struct ReportThread {
8993
ThreadType thread_type;
9094
char *name;
9195
Tid parent_tid;
96+
StackID stack_id;
9297
ReportStack *stack;
98+
bool suppressable;
9399
};
94100

95101
struct ReportMutex {
96102
int id;
97103
uptr addr;
104+
StackID stack_id;
98105
ReportStack *stack;
99106
};
100107

@@ -105,6 +112,7 @@ class ReportDesc {
105112
Vector<ReportStack*> stacks;
106113
Vector<ReportMop*> mops;
107114
Vector<ReportLocation*> locs;
115+
Vector<uptr> added_location_addrs;
108116
Vector<ReportMutex*> mutexes;
109117
Vector<ReportThread*> threads;
110118
Vector<Tid> unique_tids;

compiler-rt/lib/tsan/rtl/tsan_rtl.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,7 @@ class ScopedReportBase {
420420
void AddSleep(StackID stack_id);
421421
void SetCount(int count);
422422
void SetSigNum(int sig);
423+
void SymbolizeStackElems(void);
423424

424425
const ReportDesc *GetReport() const;
425426

compiler-rt/lib/tsan/rtl/tsan_rtl_access.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -419,6 +419,10 @@ NOINLINE void TraceRestartMemoryAccess(ThreadState* thr, uptr pc, uptr addr,
419419

420420
ALWAYS_INLINE USED void MemoryAccess(ThreadState* thr, uptr pc, uptr addr,
421421
uptr size, AccessType typ) {
422+
#if SANITIZER_APPLE && !SANITIZER_GO
423+
if (thr->in_symbolizer)
424+
return;
425+
#endif
422426
RawShadow* shadow_mem = MemToShadow(addr);
423427
UNUSED char memBuf[4][64];
424428
DPrintf2("#%d: Access: %d@%d %p/%zd typ=0x%x {%s, %s, %s, %s}\n", thr->tid,

compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ static void ReportMutexMisuse(ThreadState *thr, uptr pc, ReportType typ,
6262
ObtainCurrentStack(thr, pc, &trace);
6363
rep.AddStack(trace, true);
6464
rep.AddLocation(addr, 1);
65+
rep.SymbolizeStackElems();
6566
OutputReport(thr, rep);
6667
}
6768

@@ -539,13 +540,16 @@ void ReportDeadlock(ThreadState *thr, uptr pc, DDReport *r) {
539540
for (int i = 0; i < r->n; i++) {
540541
for (int j = 0; j < (flags()->second_deadlock_stack ? 2 : 1); j++) {
541542
u32 stk = r->loop[i].stk[j];
543+
StackTrace stack;
542544
if (stk && stk != kInvalidStackID) {
543-
rep.AddStack(StackDepotGet(stk), true);
545+
stack = StackDepotGet(stk);
544546
} else {
545547
// Sometimes we fail to extract the stack trace (FIXME: investigate),
546548
// but we should still produce some stack trace in the report.
547-
rep.AddStack(StackTrace(&dummy_pc, 1), true);
549+
stack = StackTrace(&dummy_pc, 1);
548550
}
551+
rep.AddStack(stack, true);
552+
rep.SymbolizeStackElems();
549553
}
550554
}
551555
OutputReport(thr, rep);
@@ -572,6 +576,7 @@ void ReportDestroyLocked(ThreadState *thr, uptr pc, uptr addr,
572576
return;
573577
rep.AddStack(trace, true);
574578
rep.AddLocation(addr, 1);
579+
rep.SymbolizeStackElems();
575580
OutputReport(thr, rep);
576581
}
577582

compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp

Lines changed: 122 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
//
1111
//===----------------------------------------------------------------------===//
1212

13+
#include <alloca.h>
14+
1315
#include "sanitizer_common/sanitizer_common.h"
1416
#include "sanitizer_common/sanitizer_libc.h"
1517
#include "sanitizer_common/sanitizer_placement_new.h"
@@ -187,10 +189,8 @@ void ScopedReportBase::AddMemoryAccess(uptr addr, uptr external_tag, Shadow s,
187189
mop->size = size;
188190
mop->write = !(typ & kAccessRead);
189191
mop->atomic = typ & kAccessAtomic;
190-
mop->stack = SymbolizeStack(stack);
191192
mop->external_tag = external_tag;
192-
if (mop->stack)
193-
mop->stack->suppressable = true;
193+
mop->stack_trace = stack;
194194
for (uptr i = 0; i < mset->Size(); i++) {
195195
MutexSet::Desc d = mset->Get(i);
196196
int id = this->AddMutex(d.addr, d.stack_id);
@@ -199,6 +199,45 @@ void ScopedReportBase::AddMemoryAccess(uptr addr, uptr external_tag, Shadow s,
199199
}
200200
}
201201

202+
void ScopedReportBase::SymbolizeStackElems() {
203+
// symbolize memory ops
204+
for (usize i = 0; i < rep_->mops.Size(); i++) {
205+
ReportMop *mop = rep_->mops[i];
206+
mop->stack = SymbolizeStack(mop->stack_trace);
207+
if (mop->stack)
208+
mop->stack->suppressable = true;
209+
}
210+
211+
// symbolize locations
212+
for (usize i = 0; i < rep_->locs.Size(); i++) {
213+
ReportLocation *loc = rep_->locs[i];
214+
loc->stack = SymbolizeStackId(loc->stack_id);
215+
}
216+
217+
for (usize i = 0; i < rep_->added_location_addrs.Size(); i++) {
218+
if (ReportLocation *loc = SymbolizeData(rep_->added_location_addrs[i])) {
219+
loc->suppressable = true;
220+
// TODO: These all end up at the back of locs - line them up with their
221+
// corresponding location
222+
rep_->locs.PushBack(loc);
223+
}
224+
}
225+
226+
// symbolize threads
227+
for (usize i = 0; i < rep_->threads.Size(); i++) {
228+
ReportThread *rt = rep_->threads[i];
229+
rt->stack = SymbolizeStackId(rt->stack_id);
230+
if (rt->stack)
231+
rt->stack->suppressable = rt->suppressable;
232+
}
233+
234+
// symbolize mutexes
235+
for (usize i = 0; i < rep_->mutexes.Size(); i++) {
236+
ReportMutex *rm = rep_->mutexes[i];
237+
rm->stack = SymbolizeStackId(rm->stack_id);
238+
}
239+
}
240+
202241
void ScopedReportBase::AddUniqueTid(Tid unique_tid) {
203242
rep_->unique_tids.PushBack(unique_tid);
204243
}
@@ -216,10 +255,8 @@ void ScopedReportBase::AddThread(const ThreadContext *tctx, bool suppressable) {
216255
rt->name = internal_strdup(tctx->name);
217256
rt->parent_tid = tctx->parent_tid;
218257
rt->thread_type = tctx->thread_type;
219-
rt->stack = 0;
220-
rt->stack = SymbolizeStackId(tctx->creation_stack_id);
221-
if (rt->stack)
222-
rt->stack->suppressable = suppressable;
258+
rt->stack_id = tctx->creation_stack_id;
259+
rt->suppressable = suppressable;
223260
}
224261

225262
#if !SANITIZER_GO
@@ -270,7 +307,7 @@ int ScopedReportBase::AddMutex(uptr addr, StackID creation_stack_id) {
270307
rep_->mutexes.PushBack(rm);
271308
rm->id = rep_->mutexes.Size() - 1;
272309
rm->addr = addr;
273-
rm->stack = SymbolizeStackId(creation_stack_id);
310+
rm->stack_id = creation_stack_id;
274311
return rm->id;
275312
}
276313

@@ -288,7 +325,7 @@ void ScopedReportBase::AddLocation(uptr addr, uptr size) {
288325
loc->fd_closed = closed;
289326
loc->fd = fd;
290327
loc->tid = creat_tid;
291-
loc->stack = SymbolizeStackId(creat_stack);
328+
loc->stack_id = creat_stack;
292329
rep_->locs.PushBack(loc);
293330
AddThread(creat_tid);
294331
return;
@@ -310,7 +347,7 @@ void ScopedReportBase::AddLocation(uptr addr, uptr size) {
310347
loc->heap_chunk_size = b->siz;
311348
loc->external_tag = b->tag;
312349
loc->tid = b->tid;
313-
loc->stack = SymbolizeStackId(b->stk);
350+
loc->stack_id = b->stk;
314351
rep_->locs.PushBack(loc);
315352
AddThread(b->tid);
316353
return;
@@ -324,11 +361,7 @@ void ScopedReportBase::AddLocation(uptr addr, uptr size) {
324361
AddThread(tctx);
325362
}
326363
#endif
327-
if (ReportLocation *loc = SymbolizeData(addr)) {
328-
loc->suppressable = true;
329-
rep_->locs.PushBack(loc);
330-
return;
331-
}
364+
rep_->added_location_addrs.PushBack(addr);
332365
}
333366

334367
#if !SANITIZER_GO
@@ -761,65 +794,91 @@ void ReportRace(ThreadState *thr, RawShadow *shadow_mem, Shadow cur, Shadow old,
761794
DynamicMutexSet mset1;
762795
MutexSet *mset[kMop] = {&thr->mset, mset1};
763796

764-
// We need to lock the slot during RestoreStack because it protects
765-
// the slot journal.
766-
Lock slot_lock(&ctx->slots[static_cast<uptr>(s[1].sid())].mtx);
767-
ThreadRegistryLock l0(&ctx->thread_registry);
768-
Lock slots_lock(&ctx->slot_mtx);
769-
if (SpuriousRace(old))
770-
return;
771-
if (!RestoreStack(EventType::kAccessExt, s[1].sid(), s[1].epoch(), addr1,
772-
size1, typ1, &tids[1], &traces[1], mset[1], &tags[1])) {
773-
StoreShadow(&ctx->last_spurious_race, old.raw());
774-
return;
775-
}
797+
#if SANITIZER_APPLE
798+
static Mutex report_write_mutex;
799+
Lock report_write_lock(&report_write_mutex);
800+
ScopedReport *_rep = (ScopedReport *)alloca(sizeof(ScopedReport));
801+
#endif
802+
// Take a new scope as Apple platforms need to release the below locks,
803+
// before writing out the report, in order to avoid a deadlock
804+
{
805+
// We need to lock the slot during RestoreStack because it protects
806+
// the slot journal.
807+
Lock slot_lock(&ctx->slots[static_cast<uptr>(s[1].sid())].mtx);
808+
ThreadRegistryLock l0(&ctx->thread_registry);
809+
Lock slots_lock(&ctx->slot_mtx);
810+
if (SpuriousRace(old))
811+
return;
812+
if (!RestoreStack(EventType::kAccessExt, s[1].sid(), s[1].epoch(), addr1,
813+
size1, typ1, &tids[1], &traces[1], mset[1], &tags[1])) {
814+
StoreShadow(&ctx->last_spurious_race, old.raw());
815+
return;
816+
}
776817

777-
if (IsFiredSuppression(ctx, rep_typ, traces[1]))
778-
return;
818+
if (IsFiredSuppression(ctx, rep_typ, traces[1]))
819+
return;
779820

780-
if (HandleRacyStacks(thr, traces))
781-
return;
821+
if (HandleRacyStacks(thr, traces))
822+
return;
782823

783-
// If any of the accesses has a tag, treat this as an "external" race.
784-
uptr tag = kExternalTagNone;
785-
for (uptr i = 0; i < kMop; i++) {
786-
if (tags[i] != kExternalTagNone) {
787-
rep_typ = ReportTypeExternalRace;
788-
tag = tags[i];
789-
break;
824+
// If any of the accesses has a tag, treat this as an "external" race.
825+
uptr tag = kExternalTagNone;
826+
for (uptr i = 0; i < kMop; i++) {
827+
if (tags[i] != kExternalTagNone) {
828+
rep_typ = ReportTypeExternalRace;
829+
tag = tags[i];
830+
break;
831+
}
790832
}
791-
}
792833

793-
ScopedReport rep(rep_typ, tag);
794-
for (uptr i = 0; i < kMop; i++)
795-
rep.AddMemoryAccess(addr, tags[i], s[i], tids[i], traces[i], mset[i]);
834+
#if SANITIZER_APPLE
835+
new (_rep) ScopedReport(rep_typ, tag);
836+
ScopedReport &rep = *_rep;
837+
#else
838+
ScopedReport rep(rep_typ, tag);
839+
#endif
840+
for (uptr i = 0; i < kMop; i++)
841+
rep.AddMemoryAccess(addr, tags[i], s[i], tids[i], traces[i], mset[i]);
796842

797-
for (uptr i = 0; i < kMop; i++) {
798-
ThreadContext *tctx = static_cast<ThreadContext *>(
799-
ctx->thread_registry.GetThreadLocked(tids[i]));
800-
rep.AddThread(tctx);
801-
}
843+
for (uptr i = 0; i < kMop; i++) {
844+
ThreadContext *tctx = static_cast<ThreadContext *>(
845+
ctx->thread_registry.GetThreadLocked(tids[i]));
846+
rep.AddThread(tctx);
847+
}
802848

803-
rep.AddLocation(addr_min, addr_max - addr_min);
804-
805-
if (flags()->print_full_thread_history) {
806-
const ReportDesc *rep_desc = rep.GetReport();
807-
for (uptr i = 0; i < rep_desc->threads.Size(); i++) {
808-
Tid parent_tid = rep_desc->threads[i]->parent_tid;
809-
if (parent_tid == kMainTid || parent_tid == kInvalidTid)
810-
continue;
811-
ThreadContext *parent_tctx = static_cast<ThreadContext *>(
812-
ctx->thread_registry.GetThreadLocked(parent_tid));
813-
rep.AddThread(parent_tctx);
849+
rep.AddLocation(addr_min, addr_max - addr_min);
850+
851+
if (flags()->print_full_thread_history) {
852+
const ReportDesc *rep_desc = rep.GetReport();
853+
for (uptr i = 0; i < rep_desc->threads.Size(); i++) {
854+
Tid parent_tid = rep_desc->threads[i]->parent_tid;
855+
if (parent_tid == kMainTid || parent_tid == kInvalidTid)
856+
continue;
857+
ThreadContext *parent_tctx = static_cast<ThreadContext *>(
858+
ctx->thread_registry.GetThreadLocked(parent_tid));
859+
rep.AddThread(parent_tctx);
860+
}
814861
}
815-
}
816862

817863
#if !SANITIZER_GO
818-
if (!((typ0 | typ1) & kAccessFree) &&
819-
s[1].epoch() <= thr->last_sleep_clock.Get(s[1].sid()))
820-
rep.AddSleep(thr->last_sleep_stack_id);
864+
if (!((typ0 | typ1) & kAccessFree) &&
865+
s[1].epoch() <= thr->last_sleep_clock.Get(s[1].sid()))
866+
rep.AddSleep(thr->last_sleep_stack_id);
867+
#endif
868+
869+
#if SANITIZER_APPLE
870+
} // Close this scope to release the locks
871+
872+
_rep->SymbolizeStackElems();
873+
OutputReport(thr, *_rep);
874+
875+
// Need to manually destroy this because we used placement new to allocate
876+
_rep->~ScopedReport();
877+
#else
878+
rep.SymbolizeStackElems();
879+
OutputReport(thr, rep);
880+
}
821881
#endif
822-
OutputReport(thr, rep);
823882
}
824883

825884
void PrintCurrentStack(ThreadState *thr, uptr pc) {

compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ void ThreadFinalize(ThreadState *thr) {
9696
ScopedReport rep(ReportTypeThreadLeak);
9797
rep.AddThread(leaks[i].tctx, true);
9898
rep.SetCount(leaks[i].count);
99+
rep.SymbolizeStackElems();
99100
OutputReport(thr, rep);
100101
}
101102
#endif

0 commit comments

Comments
 (0)