Index: lib/asan/asan_rtl.cc =================================================================== --- lib/asan/asan_rtl.cc +++ lib/asan/asan_rtl.cc @@ -37,7 +37,10 @@ namespace __asan { +const unsigned kAsanBuggyPcPoolSize = 25; + uptr AsanMappingProfile[kAsanMappingProfileSize]; +__sanitizer::atomic_uint64_t AsanBuggyPcPool[kAsanBuggyPcPoolSize]; static void AsanDie() { static atomic_uint32_t num_calls; @@ -107,6 +110,43 @@ PoisonShadow(ptr, size, kAsanInternalHeapMagic); } +// -------------- SuppressErrorReport -------------- {{{1 +// Avoid error reports duplicating for ASan recover mode. Don't use mutex here +// due to performance issue. +static bool SuppressErrorReport(uptr pc) { + if (!common_flags()->suppress_equal_pcs) return false; + static __sanitizer::atomic_uint32_t pc_num; + + // If we have exceeded reasonable number of different PCs, subsequent errors + // are likely artificial, so die. + if (atomic_load_relaxed(&pc_num) >= kAsanBuggyPcPoolSize) + Die(); + + bool add_new_pc = false; + for (unsigned i = 0; i < kAsanBuggyPcPoolSize; ++i) { + u64 zero = 0; + // Find available slot for new PC. Don't override already acquired records. + if (atomic_compare_exchange_strong(&AsanBuggyPcPool[i], &zero, pc, + memory_order_acq_rel)) { + add_new_pc = true; + break; + } + // Reset zero if atomic_compare_exchange_strong failed. + zero = 0; + if (pc == atomic_load_relaxed(&AsanBuggyPcPool[i])) return true; + } + + // We haven't found a free slot for PC, because we have exceeded + // kAsanBuggyPcPoolSize. Die here. + if (!add_new_pc) + Die(); + + // This is racy, but we use it for fast path only. We don't need to be exact + // precise here. + atomic_fetch_add(&pc_num, 1, memory_order_relaxed); + return false; +} + // -------------------------- Run-time entry ------------------- {{{1 // exported functions #define ASAN_REPORT_ERROR(type, is_write, size) \ @@ -123,6 +163,7 @@ extern "C" NOINLINE INTERFACE_ATTRIBUTE \ void __asan_report_ ## type ## size ## _noabort(uptr addr) { \ GET_CALLER_PC_BP_SP; \ + if (SuppressErrorReport(pc)) return; \ ReportGenericError(pc, bp, sp, addr, is_write, size, 0, false); \ } \ @@ -151,6 +192,7 @@ extern "C" NOINLINE INTERFACE_ATTRIBUTE \ void __asan_report_ ## type ## _n_noabort(uptr addr, uptr size) { \ GET_CALLER_PC_BP_SP; \ + if (SuppressErrorReport(pc)) return; \ ReportGenericError(pc, bp, sp, addr, is_write, size, 0, false); \ } \ @@ -169,6 +211,7 @@ *__asan_test_only_reported_buggy_pointer = addr; \ } else { \ GET_CALLER_PC_BP_SP; \ + if (SuppressErrorReport(pc)) return; \ ReportGenericError(pc, bp, sp, addr, is_write, size, exp_arg, \ fatal); \ } \ @@ -223,6 +266,7 @@ void __asan_loadN_noabort(uptr addr, uptr size) { if (__asan_region_is_poisoned(addr, size)) { GET_CALLER_PC_BP_SP; + if (SuppressErrorReport(pc)) return; ReportGenericError(pc, bp, sp, addr, false, size, 0, false); } } @@ -250,6 +294,7 @@ void __asan_storeN_noabort(uptr addr, uptr size) { if (__asan_region_is_poisoned(addr, size)) { GET_CALLER_PC_BP_SP; + if (SuppressErrorReport(pc)) return; ReportGenericError(pc, bp, sp, addr, true, size, 0, false); } } Index: lib/sanitizer_common/sanitizer_flags.inc =================================================================== --- lib/sanitizer_common/sanitizer_flags.inc +++ lib/sanitizer_common/sanitizer_flags.inc @@ -192,3 +192,6 @@ bool, abort_on_error, SANITIZER_MAC, "If set, the tool calls abort() instead of _exit() after printing the " "error report.") +COMMON_FLAG(bool, suppress_equal_pcs, true, + "Deduplicate multiple reports for single source location in " + "halt_on_error = false mode.") Index: test/asan/TestCases/Posix/halt_on_error-signals.c =================================================================== --- test/asan/TestCases/Posix/halt_on_error-signals.c +++ test/asan/TestCases/Posix/halt_on_error-signals.c @@ -3,7 +3,7 @@ // RUN: %clang_asan -fsanitize-recover=address -pthread %s -o %t // // RUN: rm -f %t.log -// RUN: %env_asan_opts=halt_on_error=false %run %t 100 >%t.log 2>&1 || true +// RUN: %env_asan_opts=halt_on_error=false:suppress_equal_pcs=false %run %t 100 >%t.log 2>&1 || true // Collision will almost always get triggered but we still need to check the unlikely case: // RUN: FileCheck --check-prefix=CHECK-COLLISION %s < %t.log || FileCheck --check-prefix=CHECK-NO-COLLISION %s < %t.log Index: test/asan/TestCases/Posix/halt_on_error-torture.cc =================================================================== --- test/asan/TestCases/Posix/halt_on_error-torture.cc +++ test/asan/TestCases/Posix/halt_on_error-torture.cc @@ -2,12 +2,18 @@ // // RUN: %clangxx_asan -fsanitize-recover=address -pthread %s -o %t // -// RUN: %env_asan_opts=halt_on_error=false %run %t 1 10 >1.txt 2>&1 +// RUN: %env_asan_opts=halt_on_error=false:suppress_equal_pcs=false %run %t 1 10 >1.txt 2>&1 // RUN: FileCheck %s < 1.txt // RUN: [ $(grep -c 'ERROR: AddressSanitizer: use-after-poison' 1.txt) -eq 10 ] // RUN: FileCheck --check-prefix=CHECK-NO-COLLISION %s < 1.txt // // Collisions are unlikely but still possible so we need the ||. +// RUN: %env_asan_opts=halt_on_error=false:suppress_equal_pcs=false %run %t 10 20 >10.txt 2>&1 || true +// This one is racy although _very_ unlikely to fail: +// RUN: FileCheck %s < 10.txt +// RUN: FileCheck --check-prefix=CHECK-COLLISION %s < 1.txt || FileCheck --check-prefix=CHECK-NO-COLLISION %s < 1.txt +// +// Collisions are unlikely but still possible so we need the ||. // RUN: %env_asan_opts=halt_on_error=false %run %t 10 20 >10.txt 2>&1 || true // This one is racy although _very_ unlikely to fail: // RUN: FileCheck %s < 10.txt Index: test/asan/TestCases/halt_on_error-2.c =================================================================== --- /dev/null +++ test/asan/TestCases/halt_on_error-2.c @@ -0,0 +1,45 @@ +// Test reports dedupication for recovery mode. +// +// RUN: %clang_asan -fsanitize-recover=address -DCHECK_SUPPRESSION=1 %s -o %t +// RUN: %env_asan_opts=halt_on_error=false %run %t 2>&1 | FileCheck --check-prefix=CHECK-SUPPRESSION %s + +// RUN: %clang_asan -fsanitize-recover=address -DCHECK_THRESHOLD=1 %s -o %t +// RUN: %env_asan_opts=halt_on_error=false %run %t >3.txt 2>&1 || true +// RUN: [ $(grep -c 'ERROR: AddressSanitizer: stack-buffer-overflow' 3.txt) -eq 25 ] + +#define ACCESS_ARRAY_FIVE_ELEMENTS(array, i) \ + array[i] = i; \ + array[i + 1] = i + 1; \ + array[i + 2] = i + 2; \ + array[i + 3] = i + 3; \ + array[i + 4] = i + 4; \ + +volatile int ten = 10; +unsigned kNumIterations = 10; + +int main() { + char a[10]; + char b[10]; + + for (int i = 0; i < kNumIterations; ++i) { +#ifdef CHECK_SUPPRESSION + // CHECK-SUPPRESSION: READ of size 1 + volatile int res = a[ten + i]; + // CHECK-SUPPRESSION: WRITE of size 1 + a[i + ten] = res + 3; + // CHECK-SUPPRESSION: READ of size 1 + res = a[ten + i]; + // CHECK-SUPPRESSION-NOT: READ of size 1 + // CHECK-SUPPRESSION-NOT: WRITE of size 1 +#else + ACCESS_ARRAY_FIVE_ELEMENTS(a, ten); + ACCESS_ARRAY_FIVE_ELEMENTS(a, ten + 5); + ACCESS_ARRAY_FIVE_ELEMENTS(a, ten + 10); + ACCESS_ARRAY_FIVE_ELEMENTS(b, ten); + ACCESS_ARRAY_FIVE_ELEMENTS(b, ten + 5); + ACCESS_ARRAY_FIVE_ELEMENTS(b, ten + 10); +#endif + } + return 0; +} +