https://github.com/vitalybuka updated https://github.com/llvm/llvm-project/pull/228637
>From 20fb2e995b769fe07766dda4eb77b4fa6485998d Mon Sep 17 00:00:00 2001 From: Vitaly Buka <[email protected]> Date: Fri, 2 Oct 2026 19:41:27 -0700 Subject: [PATCH] [NFC][TSan] Allocate ScopedReport as a stack variable Now that ScopedReport is constructed before acquiring ThreadRegistryLock or slot locks across all reporting functions, it no longer needs to be constructed inside the lock scope via placement new on __builtin_alloca storage. Declare ScopedReport as a normal stack variable before the lock scope and remove the manual destructor calls. Assisted-by: Gemini Pull Request: https://github.com/llvm/llvm-project/pull/228637 --- .../lib/tsan/rtl/tsan_interceptors_posix.cpp | 13 ++--- .../lib/tsan/rtl/tsan_interface_ann.cpp | 13 ++--- compiler-rt/lib/tsan/rtl/tsan_mman.cpp | 11 ++-- compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cpp | 54 +++++++------------ compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp | 21 +++----- compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp | 13 ++--- 6 files changed, 41 insertions(+), 84 deletions(-) diff --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp index a8d502966bccaf..233985c419be1c 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp +++ b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp @@ -2176,24 +2176,19 @@ static void ReportErrnoSpoiling(ThreadState *thr, uptr pc, int sig) { // StackTrace::GetNestInstructionPc(pc) is used because return address is // expected, OutputReport() will undo this. ObtainCurrentStack(thr, StackTrace::GetNextInstructionPc(pc), &stack); - // Use alloca, because malloc during signal handling deadlocks - ScopedReport *rep = (ScopedReport *)__builtin_alloca(sizeof(ScopedReport)); + ScopedReport rep(ReportTypeErrnoInSignal); bool suppressed; // Release locks before symbolizing and outputting the report to avoid // deadlocks. { - new (rep) ScopedReport(ReportTypeErrnoInSignal); ThreadRegistryLock l(&ctx->thread_registry); - rep->SetSigNum(sig); + rep.SetSigNum(sig); suppressed = IsFiredSuppression(ctx, ReportTypeErrnoInSignal, stack); if (!suppressed) - rep->AddStack(stack, true); + rep.AddStack(stack, true); } if (!suppressed) - OutputReport(thr, *rep); - - // Need to manually destroy this because we used placement new to allocate - rep->~ScopedReport(); + OutputReport(thr, rep); } static void CallUserSignalHandler(ThreadState *thr, bool sync, bool acquire, diff --git a/compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp b/compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp index 390cae67d2170b..e280f5b91b61d8 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp +++ b/compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp @@ -441,25 +441,20 @@ void __tsan_mutex_post_divert(void *addr, unsigned flagz) { } static void ReportMutexHeldWrongContext(ThreadState *thr, uptr pc) { - // Use alloca, because malloc during signal handling deadlocks - ScopedReport *rep = (ScopedReport *)__builtin_alloca(sizeof(ScopedReport)); + ScopedReport rep(ReportTypeMutexHeldWrongContext); // Release locks before symbolizing and outputting the report to avoid // deadlocks. { - 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); + rep.AddMutex(desc.addr, desc.stack_id); } VarSizeStackTrace trace; ObtainCurrentStack(thr, pc, &trace); - rep->AddStack(trace, true); + rep.AddStack(trace, true); } - OutputReport(thr, *rep); - - // Need to manually destroy this because we used placement new to allocate - rep->~ScopedReport(); + OutputReport(thr, rep); } INTERFACE_ATTRIBUTE diff --git a/compiler-rt/lib/tsan/rtl/tsan_mman.cpp b/compiler-rt/lib/tsan/rtl/tsan_mman.cpp index 88cdf58af8d581..7a39b8a71a0a79 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_mman.cpp +++ b/compiler-rt/lib/tsan/rtl/tsan_mman.cpp @@ -182,19 +182,14 @@ static void SignalUnsafeCall(ThreadState *thr, uptr pc) { ObtainCurrentStack(thr, pc, &stack); if (IsFiredSuppression(ctx, ReportTypeSignalUnsafe, stack)) return; - // Use alloca, because malloc during signal handling deadlocks - ScopedReport *rep = (ScopedReport *)__builtin_alloca(sizeof(ScopedReport)); + ScopedReport rep(ReportTypeSignalUnsafe); // Release locks before symbolizing and outputting the report to avoid // deadlocks. { - new (rep) ScopedReport(ReportTypeSignalUnsafe); ThreadRegistryLock l(&ctx->thread_registry); - rep->AddStack(stack, true); + rep.AddStack(stack, true); } - OutputReport(thr, *rep); - - // Need to manually destroy this because we used placement new to allocate - rep->~ScopedReport(); + OutputReport(thr, rep); } diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cpp index 49759ab2974fd1..ea9260a9486b9c 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cpp +++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cpp @@ -11,7 +11,6 @@ //===----------------------------------------------------------------------===// #include <sanitizer_common/sanitizer_deadlock_detector_interface.h> -#include <sanitizer_common/sanitizer_placement_new.h> #include <sanitizer_common/sanitizer_stackdepot.h> #include "tsan_flags.h" @@ -56,23 +55,18 @@ static void ReportMutexMisuse(ThreadState *thr, uptr pc, ReportType typ, return; if (!ShouldReport(thr, typ)) return; - // Use alloca, because malloc during signal handling deadlocks - ScopedReport *rep = (ScopedReport *)__builtin_alloca(sizeof(ScopedReport)); + ScopedReport rep(typ); // Release locks before symbolizing and outputting the report to avoid // deadlocks. { - new (rep) ScopedReport(typ); ThreadRegistryLock l(&ctx->thread_registry); - rep->AddMutex(addr, creation_stack_id); + rep.AddMutex(addr, creation_stack_id); VarSizeStackTrace trace; ObtainCurrentStack(thr, pc, &trace); - rep->AddStack(trace, true); - rep->AddLocation(addr, 1); + rep.AddStack(trace, true); + rep.AddLocation(addr, 1); } - OutputReport(thr, *rep); - - // Need to manually destroy this because we used placement new to allocate - rep->~ScopedReport(); + OutputReport(thr, rep); } static void RecordMutexLock(ThreadState *thr, uptr pc, uptr addr, @@ -538,17 +532,15 @@ void AfterSleep(ThreadState *thr, uptr pc) { void ReportDeadlock(ThreadState *thr, uptr pc, DDReport *r) { if (r == 0 || !ShouldReport(thr, ReportTypeDeadlock)) return; - // Use alloca, because malloc during signal handling deadlocks - ScopedReport *rep = (ScopedReport *)__builtin_alloca(sizeof(ScopedReport)); + ScopedReport rep(ReportTypeDeadlock); // Release locks before symbolizing and outputting the report to avoid // deadlocks. { - 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); - rep->AddThread((int)r->loop[i].thr_ctx); + 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); } uptr dummy_pc = 0x42; for (int i = 0; i < r->n; i++) { @@ -562,49 +554,39 @@ void ReportDeadlock(ThreadState *thr, uptr pc, DDReport *r) { // but we should still produce some stack trace in the report. stack = StackTrace(&dummy_pc, 1); } - rep->AddStack(stack, true); + rep.AddStack(stack, true); } } } - OutputReport(thr, *rep); - - // Need to manually destroy this because we used placement new to allocate - rep->~ScopedReport(); + OutputReport(thr, rep); } void ReportDestroyLocked(ThreadState *thr, uptr pc, uptr addr, FastState last_lock, StackID creation_stack_id) { - // Use alloca, because malloc during signal handling deadlocks - ScopedReport *rep = (ScopedReport *)__builtin_alloca(sizeof(ScopedReport)); + ScopedReport rep(ReportTypeMutexDestroyLocked); // 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); - rep->AddMutex(addr, creation_stack_id); + rep.AddMutex(addr, creation_stack_id); VarSizeStackTrace trace; ObtainCurrentStack(thr, pc, &trace); - rep->AddStack(trace, true); + rep.AddStack(trace, true); Tid tid; DynamicMutexSet mset; uptr tag; if (!RestoreStack(EventType::kLock, last_lock.sid(), last_lock.epoch(), - addr, 0, kAccessWrite, &tid, &trace, mset, &tag)) { - rep->~ScopedReport(); + addr, 0, kAccessWrite, &tid, &trace, mset, &tag)) return; - } - rep->AddStack(trace, true); - rep->AddLocation(addr, 1); + rep.AddStack(trace, true); + rep.AddLocation(addr, 1); } - OutputReport(thr, *rep); - - // Need to manually destroy this because we used placement new to allocate - rep->~ScopedReport(); + OutputReport(thr, rep); } } // namespace __tsan diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp index 7d302955374de7..4a98b761b9d874 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp +++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp @@ -842,46 +842,41 @@ void ReportRace(ThreadState *thr, RawShadow *shadow_mem, Shadow cur, Shadow old, } } - // Use alloca, because malloc during signal handling deadlocks - ScopedReport* rep = (ScopedReport*)__builtin_alloca(sizeof(ScopedReport)); + ScopedReport rep(rep_typ, tag); // 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]); + 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); + rep.AddThread(tctx); } - 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(); + 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.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); + rep.AddSleep(thr->last_sleep_stack_id); #endif } - OutputReport(thr, *rep); - - // Need to manually destroy this because we used placement new to allocate - rep->~ScopedReport(); + 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 c408024e77e60d..5edf0c16fdd0d6 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp +++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp @@ -96,20 +96,15 @@ void ThreadFinalize(ThreadState *thr) { } for (uptr i = 0; i < leaks.Size(); i++) { - // Use alloca, because malloc during signal handling deadlocks - ScopedReport *rep = (ScopedReport *)__builtin_alloca(sizeof(ScopedReport)); + ScopedReport rep(ReportTypeThreadLeak); // Release locks before symbolizing and outputting the report to avoid // deadlocks. { - new (rep) ScopedReport(ReportTypeThreadLeak); 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); - - // Need to manually destroy this because we used placement new to allocate - rep->~ScopedReport(); + OutputReport(thr, rep); } #endif } _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
