Index: compiler-rt/lib/asan/asan_report.cc =================================================================== --- compiler-rt/lib/asan/asan_report.cc +++ compiler-rt/lib/asan/asan_report.cc @@ -122,53 +122,36 @@ // immediately after printing error report. class ScopedInErrorReport { public: + static const u32 kUnclaimedTid = kInvalidTid - 1; + explicit ScopedInErrorReport(bool fatal = false) { halt_on_error_ = fatal || flags()->halt_on_error; - - if (lock_.TryLock()) { - StartReporting(); - return; - } - - // ASan found two bugs in different threads simultaneously. - u32 current_tid = GetCurrentTidOrInvalid(); - if (reporting_thread_tid_ == current_tid || - reporting_thread_tid_ == kInvalidTid) { - // This is either asynch signal or nested error during error reporting. - // Fail simple to avoid deadlocks in Report(). - - // Can't use Report() here because of potential deadlocks - // in nested signal handlers. - static const char msg[] = - "AddressSanitizer: nested bug in the same thread, aborting.\n"; - CatastrophicErrorWrite(msg, sizeof(msg) - 1); - - internal__exit(common_flags()->exitcode); - } - - if (halt_on_error_) { - // Do not print more than one report, otherwise they will mix up. - // Error reporting functions shouldn't return at this situation, as - // they are effectively no-returns. - - Report("AddressSanitizer: while reporting a bug found another one. " - "Ignoring.\n"); - // Sleep long enough to make sure that the thread which started - // to print an error report will finish doing it. - SleepForSeconds(Max(100, flags()->sleep_before_dying + 1)); - - // If we're still not dead for some reason, use raw _exit() instead of - // Die() to bypass any additional checks. - internal__exit(common_flags()->exitcode); - } else { - // The other thread will eventually finish reporting - // so it's safe to wait - lock_.Lock(); + for (;;) { + u32 expected_tid = kUnclaimedTid; + if (atomic_compare_exchange_strong(&reporting_thread_tid_, &expected_tid, + current_tid, memory_order_relaxed)) { + // We've claimed reporting_thread_tid_ so proceed. + StartReporting(); + return; + } + + if (expected_tid == current_tid) { + // This is either asynch signal or nested error during error reporting. + // Fail simple to avoid deadlocks in Report(). + + // Can't use Report() here because of potential deadlocks in nested + // signal handlers. + static const char msg[] = + "AddressSanitizer: nested bug in the same thread, aborting.\n"; + CatastrophicErrorWrite(msg, sizeof(msg) - 1); + + internal__exit(common_flags()->exitcode); + } + + internal_sched_yield(); } - - StartReporting(); } ~ScopedInErrorReport() { @@ -216,13 +199,13 @@ if (!halt_on_error_) internal_memset(¤t_error_, 0, sizeof(current_error_)); - CommonSanitizerReportMutex.Unlock(); - reporting_thread_tid_ = kInvalidTid; - lock_.Unlock(); if (halt_on_error_) { Report("ABORTING\n"); Die(); } + + atomic_store_relaxed(&reporting_thread_tid_, kInvalidTid); + CommonSanitizerReportMutex.Unlock(); } void ReportError(const ErrorDescription &description) { @@ -237,27 +220,24 @@ private: void StartReporting() { + CommonSanitizerReportMutex.Lock(); // Make sure the registry and sanitizer report mutexes are locked while // we're printing an error report. // We can lock them only here to avoid self-deadlock in case of // recursive reports. asanThreadRegistry().Lock(); - CommonSanitizerReportMutex.Lock(); - reporting_thread_tid_ = GetCurrentTidOrInvalid(); - Printf("====================================================" - "=============\n"); + Printf( + "=================================================================\n"); } - static StaticSpinMutex lock_; - static u32 reporting_thread_tid_; + static atomic_uint32_t reporting_thread_tid_; // Error currently being reported. This enables the destructor to interact // with the debugger and point it to an error description. static ErrorDescription current_error_; bool halt_on_error_; }; -StaticSpinMutex ScopedInErrorReport::lock_; -u32 ScopedInErrorReport::reporting_thread_tid_ = kInvalidTid; +atomic_uint32_t ScopedInErrorReport::reporting_thread_tid_ = {kUnclaimedTid}; ErrorDescription ScopedInErrorReport::current_error_; void ReportDeadlySignal(const SignalContext &sig) { Index: compiler-rt/test/asan/TestCases/Posix/concurrent_overflow.cc =================================================================== --- /dev/null +++ compiler-rt/test/asan/TestCases/Posix/concurrent_overflow.cc @@ -0,0 +1,33 @@ +// RUN: %clangxx_asan -O0 -w %s -o %t && not %run %t 2>&1 | FileCheck %s + +// Checks that concurrent reports will not trigger false "nested bug" reports. +// Regression test for https://github.com/google/sanitizers/issues/858 + +#include +#include +#include + +static void *start_routine(void *arg) { + volatile int *counter = (volatile int *)arg; + char buf[8]; + __atomic_sub_fetch(counter, 1, __ATOMIC_SEQ_CST); + while (*counter) + ; + buf[0] = buf[9]; + return 0; +} + +int main(void) { + const int n_threads = 8; + int i, counter = n_threads; + pthread_t thread; + + for (i = 0; i < n_threads; ++i) + pthread_create(&thread, NULL, &start_routine, (void *)&counter); + sleep(5); + return 0; +} + +// CHECK-NOT: nested bug +// CHECK: ERROR: AddressSanitizer: stack-buffer-overflow on address +// CHECK: SUMMARY: AddressSanitizer: stack-buffer-overflow