Skip to content

Commit

Permalink
tsan: remove tracking of racy addresses
Browse files Browse the repository at this point in the history
We used to deduplicate based on the race address to prevent lots
of repeated reports about the same race.

But now we clear the shadow for the racy address in DoReportRace:

  // This prevents trapping on this address in future.
  for (uptr i = 0; i < kShadowCnt; i++)
    StoreShadow(&shadow_mem[i], i == 0 ? Shadow::kRodata : Shadow::kEmpty);

It should have the same effect of not reporting duplicates
(and actually better because it's automatically reset when the memory is reallocated).

So drop the address deduplication code. Both simpler and faster.

Reviewed By: melver

Differential Revision: https://reviews.llvm.org/D130240
  • Loading branch information
dvyukov authored and memfrob committed Oct 5, 2022
1 parent bb5ea59 commit a9eeddb
Show file tree
Hide file tree
Showing 6 changed files with 1 addition and 42 deletions.
4 changes: 0 additions & 4 deletions compiler-rt/lib/tsan/rtl/tsan_flags.inc
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,6 @@ TSAN_FLAG(bool, enable_annotations, true,
TSAN_FLAG(bool, suppress_equal_stacks, true,
"Suppress a race report if we've already output another race report "
"with the same stack.")
TSAN_FLAG(bool, suppress_equal_addresses, true,
"Suppress a race report if we've already output another race report "
"on the same address.")

TSAN_FLAG(bool, report_bugs, true,
"Turns off bug reporting entirely (useful for benchmarking).")
TSAN_FLAG(bool, report_thread_leaks, true, "Report thread leaks at exit?")
Expand Down
1 change: 0 additions & 1 deletion compiler-rt/lib/tsan/rtl/tsan_rtl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,6 @@ Context::Context()
}),
racy_mtx(MutexTypeRacy),
racy_stacks(),
racy_addresses(),
fired_suppressions_mtx(MutexTypeFired),
slot_mtx(MutexTypeSlots),
resetting() {
Expand Down
1 change: 0 additions & 1 deletion compiler-rt/lib/tsan/rtl/tsan_rtl.h
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,6 @@ struct Context {

Mutex racy_mtx;
Vector<RacyStacks> racy_stacks;
Vector<RacyAddress> racy_addresses;
// Number of fired suppressions may be large enough.
Mutex fired_suppressions_mtx;
InternalMmapVector<FiredSuppression> fired_suppressions;
Expand Down
31 changes: 0 additions & 31 deletions compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -629,35 +629,6 @@ static bool HandleRacyStacks(ThreadState *thr, VarSizeStackTrace traces[2]) {
return false;
}

static bool FindRacyAddress(const RacyAddress &ra0) {
for (uptr i = 0; i < ctx->racy_addresses.Size(); i++) {
RacyAddress ra2 = ctx->racy_addresses[i];
uptr maxbeg = max(ra0.addr_min, ra2.addr_min);
uptr minend = min(ra0.addr_max, ra2.addr_max);
if (maxbeg < minend) {
VPrintf(2, "ThreadSanitizer: suppressing report as doubled (addr)\n");
return true;
}
}
return false;
}

static bool HandleRacyAddress(ThreadState *thr, uptr addr_min, uptr addr_max) {
if (!flags()->suppress_equal_addresses)
return false;
RacyAddress ra0 = {addr_min, addr_max};
{
ReadLock lock(&ctx->racy_mtx);
if (FindRacyAddress(ra0))
return true;
}
Lock lock(&ctx->racy_mtx);
if (FindRacyAddress(ra0))
return true;
ctx->racy_addresses.PushBack(ra0);
return false;
}

bool OutputReport(ThreadState *thr, const ScopedReport &srep) {
// These should have been checked in ShouldReport.
// It's too late to check them here, we have already taken locks.
Expand Down Expand Up @@ -761,8 +732,6 @@ void ReportRace(ThreadState *thr, RawShadow *shadow_mem, Shadow cur, Shadow old,
uptr addr_max = max(end0, end1);
if (IsExpectedReport(addr_min, addr_max - addr_min))
return;
if (HandleRacyAddress(thr, addr_min, addr_max))
return;

ReportType rep_typ = ReportTypeRace;
if ((typ0 & kAccessVptr) && (typ1 & kAccessFree))
Expand Down
4 changes: 0 additions & 4 deletions compiler-rt/lib/tsan/tests/unit/tsan_flags_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ TEST(Flags, DefaultValues) {
static const char *options1 =
" enable_annotations=0"
" suppress_equal_stacks=0"
" suppress_equal_addresses=0"
" report_bugs=0"
" report_thread_leaks=0"
" report_destroy_locked=0"
Expand All @@ -58,7 +57,6 @@ static const char *options1 =
static const char *options2 =
" enable_annotations=true"
" suppress_equal_stacks=true"
" suppress_equal_addresses=true"
" report_bugs=true"
" report_thread_leaks=true"
" report_destroy_locked=true"
Expand All @@ -82,7 +80,6 @@ static const char *options2 =
void VerifyOptions1(Flags *f) {
EXPECT_EQ(f->enable_annotations, 0);
EXPECT_EQ(f->suppress_equal_stacks, 0);
EXPECT_EQ(f->suppress_equal_addresses, 0);
EXPECT_EQ(f->report_bugs, 0);
EXPECT_EQ(f->report_thread_leaks, 0);
EXPECT_EQ(f->report_destroy_locked, 0);
Expand All @@ -106,7 +103,6 @@ void VerifyOptions1(Flags *f) {
void VerifyOptions2(Flags *f) {
EXPECT_EQ(f->enable_annotations, true);
EXPECT_EQ(f->suppress_equal_stacks, true);
EXPECT_EQ(f->suppress_equal_addresses, true);
EXPECT_EQ(f->report_bugs, true);
EXPECT_EQ(f->report_thread_leaks, true);
EXPECT_EQ(f->report_destroy_locked, true);
Expand Down
2 changes: 1 addition & 1 deletion compiler-rt/test/tsan/stress.cpp
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// This run stresses global reset happenning concurrently with everything else.
// RUN: %clangxx_tsan -O1 %s -o %t && %env_tsan_opts=flush_memory_ms=1:flush_symbolizer_ms=1:memory_limit_mb=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-NORACE
// This run stresses race reporting happenning concurrently with everything else.
// RUN: %clangxx_tsan -O1 %s -DRACE=1 -o %t && %env_tsan_opts=suppress_equal_stacks=0:suppress_equal_addresses=0 %deflake %run %t | FileCheck %s --check-prefix=CHECK-RACE
// RUN: %clangxx_tsan -O1 %s -DRACE=1 -o %t && %env_tsan_opts=suppress_equal_stacks=0 %deflake %run %t | FileCheck %s --check-prefix=CHECK-RACE
#include "test.h"
#include <fcntl.h>
#include <string.h>
Expand Down

0 comments on commit a9eeddb

Please sign in to comment.