tsan: remove tracking of racy addresses

[dvyukov]

Summary

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.

[dvyukov]

On the second thought maybe we better to keep this... need to think more.

[dvyukov]

In D130240#3668266, @dvyukov wrote:

On the second thought maybe we better to keep this... need to think more.

Yes, I think this is still a good idea.

compiler-rt/lib/tsan/rtl/tsan_flags.inc

 
 TSAN_FLAG(bool, enable_annotations, true,
           "Enable dynamic annotations, otherwise they are no-ops.")
 // Suppress a race report if we've already output another race report
 // with the same stack.
 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?")
 TSAN_FLAG(bool, report_destroy_locked, true,
           "Report destruction of a locked mutex?")
 TSAN_FLAG(bool, report_mutex_bugs, true,
           "Report incorrect usages of mutexes and mutex annotations?")
 TSAN_FLAG(bool, report_signal_unsafe, true,

compiler-rt/lib/tsan/rtl/tsan_rtl.h

#endif
 
   void *background_thread;
   atomic_uint32_t stop_background_thread;
 
   ThreadRegistry thread_registry;
 
   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;
   DDetector *dd;
 
   Flags flags;
   fd_t memprof_fd;
 

compiler-rt/lib/tsan/rtl/tsan_rtl.cpp

Context::Context()
     : initialized(),
       report_mtx(MutexTypeReport),
       nreported(),
       thread_registry([](Tid tid) -> ThreadContextBase* {
         return new (Alloc(sizeof(ThreadContext))) ThreadContext(tid);
       }),
       racy_mtx(MutexTypeRacy),
       racy_stacks(),
-      racy_addresses(),
       fired_suppressions_mtx(MutexTypeFired),
       slot_mtx(MutexTypeSlots),
       resetting() {
   fired_suppressions.reserve(8);
   for (uptr i = 0; i < ARRAY_SIZE(slots); i++) {
     TidSlot* slot = &slots[i];
     slot->sid = static_cast<Sid>(i);
     slot_queue.PushBack(slot);

compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp

static bool HandleRacyStacks(ThreadState *thr, VarSizeStackTrace traces[2]) {
   }
   Lock lock(&ctx->racy_mtx);
   if (FindRacyStacks(hash))
     return true;
   ctx->racy_stacks.PushBack(hash);
   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.
   CHECK(flags()->report_bugs);
   CHECK(!thr->suppress_reports);
   atomic_store_relaxed(&ctx->last_symbolize_time_ns, NanoTime());
   const ReportDesc *rep = srep.GetReport();
   CHECK_EQ(thr->current_report, nullptr);
void ReportRace(ThreadState *thr, RawShadow *shadow_mem, Shadow cur, Shadow old,
   uptr addr0 = addr + addr_off0;
   uptr addr1 = addr + addr_off1;
   uptr end0 = addr0 + size0;
   uptr end1 = addr1 + size1;
   uptr addr_min = min(addr0, addr1);
   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))
     rep_typ = ReportTypeVptrUseAfterFree;
   else if (typ0 & kAccessVptr)
     rep_typ = ReportTypeVptrRace;
   else if (typ1 & kAccessFree)
     rep_typ = ReportTypeUseAfterFree;

compiler-rt/lib/tsan/tests/unit/tsan_flags_test.cpp

TEST(Flags, DefaultValues) {
   f.enable_annotations = false;
   InitializeFlags(&f, "");
   EXPECT_EQ(true, f.enable_annotations);
 }
 
 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"
   " report_mutex_bugs=0"
   " report_signal_unsafe=0"
   " report_atomic_races=0"
   " force_seq_cst_atomics=0"
   " halt_on_error=0"
   " atexit_sleep_ms=222"
   " profile_memory=qqq"
   " flush_memory_ms=444"
   " flush_symbolizer_ms=555"
   " memory_limit_mb=666"
   " stop_on_start=0"
   " running_on_valgrind=0"
   " history_size=5"
   " io_sync=1"
   " die_after_fork=true"
   "";
 
 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"
   " report_mutex_bugs=true"
   " report_signal_unsafe=true"
   " report_atomic_races=true"
   " force_seq_cst_atomics=true"
   " halt_on_error=true"
   " atexit_sleep_ms=123"
   " profile_memory=bbbbb"
   " flush_memory_ms=234"
   " flush_symbolizer_ms=345"
   " memory_limit_mb=456"
   " stop_on_start=true"
   " running_on_valgrind=true"
   " history_size=6"
   " io_sync=2"
   " die_after_fork=false"
   "";
 
 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);
   EXPECT_EQ(f->report_mutex_bugs, 0);
   EXPECT_EQ(f->report_signal_unsafe, 0);
   EXPECT_EQ(f->report_atomic_races, 0);
   EXPECT_EQ(f->force_seq_cst_atomics, 0);
   EXPECT_EQ(f->halt_on_error, 0);
   EXPECT_EQ(f->atexit_sleep_ms, 222);
   EXPECT_EQ(f->profile_memory, std::string("qqq"));
   EXPECT_EQ(f->flush_memory_ms, 444);
   EXPECT_EQ(f->flush_symbolizer_ms, 555);
   EXPECT_EQ(f->memory_limit_mb, 666);
   EXPECT_EQ(f->stop_on_start, 0);
   EXPECT_EQ(f->running_on_valgrind, 0);
   EXPECT_EQ(f->history_size, (uptr)5);
   EXPECT_EQ(f->io_sync, 1);
   EXPECT_EQ(f->die_after_fork, true);
 }
 
 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);
   EXPECT_EQ(f->report_mutex_bugs, true);
   EXPECT_EQ(f->report_signal_unsafe, true);
   EXPECT_EQ(f->report_atomic_races, true);
   EXPECT_EQ(f->force_seq_cst_atomics, true);
   EXPECT_EQ(f->halt_on_error, true);

compiler-rt/test/tsan/stress.cpp

 // 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>
 
 volatile long stop;
 long atomic, read_only, racy;
 int fds[2];