From a43e98cc741656d10643a6c70c0a9b64fa38c8a4 Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Wed, 28 May 2014 18:03:32 +0000 Subject: [PATCH] tsan: refactor suppressions machinery The refactoring makes suppressions more flexible and allow to suppress based on arbitrary number of stacks. In particular it fixes: https://code.google.com/p/thread-sanitizer/issues/detail?id=64 "Make it possible to suppress deadlock reports by any stack (not just first)" llvm-svn: 209757 --- compiler-rt/lib/tsan/rtl/tsan_interceptors.cc | 4 +- compiler-rt/lib/tsan/rtl/tsan_mman.cc | 4 +- compiler-rt/lib/tsan/rtl/tsan_report.h | 2 + compiler-rt/lib/tsan/rtl/tsan_rtl.h | 12 ++---- compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cc | 14 +++---- compiler-rt/lib/tsan/rtl/tsan_rtl_report.cc | 39 ++++++++++--------- compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cc | 4 +- compiler-rt/lib/tsan/rtl/tsan_suppressions.cc | 5 ++- 8 files changed, 41 insertions(+), 43 deletions(-) diff --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc b/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc index 82c4da854f8e..60e4b3aed633 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc +++ b/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc @@ -1704,8 +1704,8 @@ static void CallUserSignalHandler(ThreadState *thr, bool sync, bool sigact, ThreadRegistryLock l(ctx->thread_registry); ScopedReport rep(ReportTypeErrnoInSignal); if (!IsFiredSuppression(ctx, rep, stack)) { - rep.AddStack(&stack); - OutputReport(ctx, rep, rep.GetReport()->stacks[0]); + rep.AddStack(&stack, true); + OutputReport(ctx, rep); } } errno = saved_errno; diff --git a/compiler-rt/lib/tsan/rtl/tsan_mman.cc b/compiler-rt/lib/tsan/rtl/tsan_mman.cc index 714b906cbf1e..4deeab5bfb3d 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_mman.cc +++ b/compiler-rt/lib/tsan/rtl/tsan_mman.cc @@ -95,8 +95,8 @@ static void SignalUnsafeCall(ThreadState *thr, uptr pc) { ThreadRegistryLock l(ctx->thread_registry); ScopedReport rep(ReportTypeSignalUnsafe); if (!IsFiredSuppression(ctx, rep, stack)) { - rep.AddStack(&stack); - OutputReport(ctx, rep, rep.GetReport()->stacks[0]); + rep.AddStack(&stack, true); + OutputReport(ctx, rep); } } diff --git a/compiler-rt/lib/tsan/rtl/tsan_report.h b/compiler-rt/lib/tsan/rtl/tsan_report.h index 3817bcdfc20d..8ea977444fc9 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_report.h +++ b/compiler-rt/lib/tsan/rtl/tsan_report.h @@ -42,6 +42,7 @@ struct ReportStack { char *file; int line; int col; + bool suppressable; }; struct ReportMopMutex { @@ -80,6 +81,7 @@ struct ReportLocation { char *name; char *file; int line; + bool suppressable; ReportStack *stack; }; diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl.h b/compiler-rt/lib/tsan/rtl/tsan_rtl.h index 4364ef803f8f..ed0e0b890289 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_rtl.h +++ b/compiler-rt/lib/tsan/rtl/tsan_rtl.h @@ -576,11 +576,11 @@ class ScopedReport { explicit ScopedReport(ReportType typ); ~ScopedReport(); - void AddStack(const StackTrace *stack); void AddMemoryAccess(uptr addr, Shadow s, const StackTrace *stack, const MutexSet *mset); - void AddThread(const ThreadContext *tctx); - void AddThread(int unique_tid); + void AddStack(const StackTrace *stack, bool suppressable = false); + void AddThread(const ThreadContext *tctx, bool suppressable = false); + void AddThread(int unique_tid, bool suppressable = false); void AddUniqueTid(int unique_tid); void AddMutex(const SyncVar *s); u64 AddMutex(u64 id); @@ -628,11 +628,7 @@ void ForkParentAfter(ThreadState *thr, uptr pc); void ForkChildAfter(ThreadState *thr, uptr pc); void ReportRace(ThreadState *thr); -bool OutputReport(Context *ctx, - const ScopedReport &srep, - const ReportStack *suppress_stack1, - const ReportStack *suppress_stack2 = 0, - const ReportLocation *suppress_loc = 0); +bool OutputReport(Context *ctx, const ScopedReport &srep); bool IsFiredSuppression(Context *ctx, const ScopedReport &srep, const StackTrace &trace); diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cc b/compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cc index 650cd624918a..66b789da7562 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cc +++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cc @@ -57,9 +57,9 @@ static void ReportMutexMisuse(ThreadState *thr, uptr pc, ReportType typ, rep.AddMutex(mid); StackTrace trace; trace.ObtainCurrent(thr, pc); - rep.AddStack(&trace); + rep.AddStack(&trace, true); rep.AddLocation(addr, 1); - OutputReport(ctx, rep, rep.GetReport()->stacks[0]); + OutputReport(ctx, rep); } void MutexCreate(ThreadState *thr, uptr pc, uptr addr, @@ -113,9 +113,9 @@ void MutexDestroy(ThreadState *thr, uptr pc, uptr addr) { rep.AddStack(&trace); FastState last(s->last_lock); RestoreStack(last.tid(), last.epoch(), &trace, 0); - rep.AddStack(&trace); + rep.AddStack(&trace, true); rep.AddLocation(s->addr, 1); - OutputReport(ctx, rep, rep.GetReport()->stacks[0]); + OutputReport(ctx, rep); } thr->mset.Remove(s->GetId()); DestroyAndFree(s); @@ -462,12 +462,10 @@ void ReportDeadlock(ThreadState *thr, uptr pc, DDReport *r) { // but we should still produce some stack trace in the report. stacks[i].Init(&dummy_pc, 1); } - rep.AddStack(&stacks[i]); + rep.AddStack(&stacks[i], true); } } - // FIXME: use all stacks for suppressions, not just the second stack of the - // first edge. - OutputReport(ctx, rep, rep.GetReport()->stacks[0]); + OutputReport(ctx, rep); } } // namespace __tsan diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cc b/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cc index 52edb5a3743d..b3e502e8e842 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cc +++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cc @@ -162,9 +162,10 @@ ScopedReport::~ScopedReport() { DestroyAndFree(rep_); } -void ScopedReport::AddStack(const StackTrace *stack) { +void ScopedReport::AddStack(const StackTrace *stack, bool suppressable) { ReportStack **rs = rep_->stacks.PushBack(); *rs = SymbolizeStack(*stack); + (*rs)->suppressable = suppressable; } void ScopedReport::AddMemoryAccess(uptr addr, Shadow s, @@ -178,6 +179,7 @@ void ScopedReport::AddMemoryAccess(uptr addr, Shadow s, mop->write = s.IsWrite(); mop->atomic = s.IsAtomic(); mop->stack = SymbolizeStack(*stack); + mop->stack->suppressable = true; for (uptr i = 0; i < mset->Size(); i++) { MutexSet::Desc d = mset->Get(i); u64 mid = this->AddMutex(d.id); @@ -190,7 +192,7 @@ void ScopedReport::AddUniqueTid(int unique_tid) { rep_->unique_tids.PushBack(unique_tid); } -void ScopedReport::AddThread(const ThreadContext *tctx) { +void ScopedReport::AddThread(const ThreadContext *tctx, bool suppressable) { for (uptr i = 0; i < rep_->threads.Size(); i++) { if ((u32)rep_->threads[i]->id == tctx->tid) return; @@ -205,6 +207,8 @@ void ScopedReport::AddThread(const ThreadContext *tctx) { rt->parent_tid = tctx->parent_tid; rt->stack = 0; rt->stack = SymbolizeStackId(tctx->creation_stack_id); + if (rt->stack) + rt->stack->suppressable = suppressable; } #ifndef TSAN_GO @@ -251,9 +255,9 @@ ThreadContext *IsThreadStackOrTls(uptr addr, bool *is_stack) { } #endif -void ScopedReport::AddThread(int unique_tid) { +void ScopedReport::AddThread(int unique_tid, bool suppressable) { #ifndef TSAN_GO - AddThread(FindThreadByUidLocked(unique_tid)); + AddThread(FindThreadByUidLocked(unique_tid), suppressable); #endif } @@ -356,6 +360,7 @@ void ScopedReport::AddLocation(uptr addr, uptr size) { } ReportLocation *loc = SymbolizeData(addr); if (loc) { + loc->suppressable = true; rep_->locs.PushBack(loc); return; } @@ -495,19 +500,19 @@ static void AddRacyStacks(ThreadState *thr, const StackTrace (&traces)[2], } } -bool OutputReport(Context *ctx, - const ScopedReport &srep, - const ReportStack *suppress_stack1, - const ReportStack *suppress_stack2, - const ReportLocation *suppress_loc) { +bool OutputReport(Context *ctx, const ScopedReport &srep) { atomic_store(&ctx->last_symbolize_time_ns, NanoTime(), memory_order_relaxed); const ReportDesc *rep = srep.GetReport(); Suppression *supp = 0; - uptr suppress_pc = IsSuppressed(rep->typ, suppress_stack1, &supp); - if (suppress_pc == 0) - suppress_pc = IsSuppressed(rep->typ, suppress_stack2, &supp); - if (suppress_pc == 0) - suppress_pc = IsSuppressed(rep->typ, suppress_loc, &supp); + uptr suppress_pc = 0; + for (uptr i = 0; suppress_pc == 0 && i < rep->mops.Size(); i++) + suppress_pc = IsSuppressed(rep->typ, rep->mops[i]->stack, &supp); + for (uptr i = 0; suppress_pc == 0 && i < rep->stacks.Size(); i++) + suppress_pc = IsSuppressed(rep->typ, rep->stacks[i], &supp); + for (uptr i = 0; suppress_pc == 0 && i < rep->threads.Size(); i++) + suppress_pc = IsSuppressed(rep->typ, rep->threads[i]->stack, &supp); + for (uptr i = 0; suppress_pc == 0 && i < rep->locs.Size(); i++) + suppress_pc = IsSuppressed(rep->typ, rep->locs[i], &supp); if (suppress_pc != 0) { FiredSuppression s = {srep.GetReport()->typ, suppress_pc, supp}; ctx->fired_suppressions.push_back(s); @@ -695,11 +700,7 @@ void ReportRace(ThreadState *thr) { } #endif - ReportLocation *suppress_loc = rep.GetReport()->locs.Size() ? - rep.GetReport()->locs[0] : 0; - if (!OutputReport(ctx, rep, rep.GetReport()->mops[0]->stack, - rep.GetReport()->mops[1]->stack, - suppress_loc)) + if (!OutputReport(ctx, rep)) return; AddRacyStacks(thr, traces, addr_min, addr_max); diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cc b/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cc index b2ac7bb12dae..3b416c04550e 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cc +++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cc @@ -205,9 +205,9 @@ void ThreadFinalize(ThreadState *thr) { MaybeReportThreadLeak, &leaks); for (uptr i = 0; i < leaks.Size(); i++) { ScopedReport rep(ReportTypeThreadLeak); - rep.AddThread(leaks[i].tctx); + rep.AddThread(leaks[i].tctx, true); rep.SetCount(leaks[i].count); - OutputReport(ctx, rep, rep.GetReport()->threads[0]->stack); + OutputReport(ctx, rep); } #endif } diff --git a/compiler-rt/lib/tsan/rtl/tsan_suppressions.cc b/compiler-rt/lib/tsan/rtl/tsan_suppressions.cc index 0ac2526ce524..0670396e27ef 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_suppressions.cc +++ b/compiler-rt/lib/tsan/rtl/tsan_suppressions.cc @@ -123,7 +123,8 @@ SuppressionType conv(ReportType typ) { uptr IsSuppressed(ReportType typ, const ReportStack *stack, Suppression **sp) { CHECK(g_ctx); - if (!g_ctx->SuppressionCount() || stack == 0) return 0; + if (!g_ctx->SuppressionCount() || stack == 0 || !stack->suppressable) + return 0; SuppressionType stype = conv(typ); if (stype == SuppressionNone) return 0; @@ -144,7 +145,7 @@ uptr IsSuppressed(ReportType typ, const ReportStack *stack, Suppression **sp) { uptr IsSuppressed(ReportType typ, const ReportLocation *loc, Suppression **sp) { CHECK(g_ctx); if (!g_ctx->SuppressionCount() || loc == 0 || - loc->type != ReportLocationGlobal) + loc->type != ReportLocationGlobal || !loc->suppressable) return 0; SuppressionType stype = conv(typ); if (stype == SuppressionNone)