[compiler-rt] d3849dd - Revert "[TSan] Optimize handling of racy address"

[Joachim Protze via llvm-commits]

Author: Joachim Protze
Date: 2020-07-15T17:40:28+02:00
New Revision: d3849dddd267af300d76b57c055e89f1ad2622d0

URL: https://github.com/llvm/llvm-project/commit/d3849dddd267af300d76b57c055e89f1ad2622d0
DIFF: https://github.com/llvm/llvm-project/commit/d3849dddd267af300d76b57c055e89f1ad2622d0.diff

LOG: Revert "[TSan] Optimize handling of racy address"

This reverts commit 00e3a1ddec95c0b48ce216220d7e3481dab3bc78.
The commit broke most build bots, investigating.

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp
index 3354546c2a10..949beac1c551 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp
@@ -439,61 +439,65 @@ void RestoreStack(int tid, const u64 epoch, VarSizeStackTrace *stk,
   ExtractTagFromStack(stk, tag);
 }
 
-static bool FindRacyStacks(const RacyStacks &hash) {
-  for (uptr i = 0; i < ctx->racy_stacks.Size(); i++) {
-    if (hash == ctx->racy_stacks[i]) {
-      VPrintf(2, "ThreadSanitizer: suppressing report as doubled (stack)\n");
-      return true;
-    }
-  }
-  return false;
-}
-
-static bool HandleRacyStacks(ThreadState *thr, VarSizeStackTrace traces[2]) {
-  if (!flags()->suppress_equal_stacks)
-    return false;
+static bool HandleRacyStacks(ThreadState *thr, VarSizeStackTrace traces[2],
+                             uptr addr_min, uptr addr_max) {
+  bool equal_stack = false;
   RacyStacks hash;
-  hash.hash[0] = md5_hash(traces[0].trace, traces[0].size * sizeof(uptr));
-  hash.hash[1] = md5_hash(traces[1].trace, traces[1].size * sizeof(uptr));
+  bool equal_address = false;
+  RacyAddress ra0 = {addr_min, addr_max};
   {
     ReadLock lock(&ctx->racy_mtx);
-    if (FindRacyStacks(hash))
-      return true;
-  }
-  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;
+    if (flags()->suppress_equal_stacks) {
+      hash.hash[0] = md5_hash(traces[0].trace, traces[0].size * sizeof(uptr));
+      hash.hash[1] = md5_hash(traces[1].trace, traces[1].size * sizeof(uptr));
+      for (uptr i = 0; i < ctx->racy_stacks.Size(); i++) {
+        if (hash == ctx->racy_stacks[i]) {
+          VPrintf(2,
+              "ThreadSanitizer: suppressing report as doubled (stack)\n");
+          equal_stack = true;
+          break;
+        }
+      }
+    }
+    if (flags()->suppress_equal_addresses) {
+      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");
+          equal_address = true;
+          break;
+        }
+      }
     }
   }
-  return false;
-}
-
-static bool HandleRacyAddress(ThreadState *thr, uptr addr_min, uptr addr_max) {
-  if (!flags()->suppress_equal_addresses)
+  if (!equal_stack && !equal_address)
     return false;
-  RacyAddress ra0 = {addr_min, addr_max};
-  {
-    ReadLock lock(&ctx->racy_mtx);
-    if (FindRacyAddress(ra0))
-      return true;
+  if (!equal_stack) {
+    Lock lock(&ctx->racy_mtx);
+    ctx->racy_stacks.PushBack(hash);
+  }
+  if (!equal_address) {
+    Lock lock(&ctx->racy_mtx);
+    ctx->racy_addresses.PushBack(ra0);
   }
+  return true;
+}
+
+static void AddRacyStacks(ThreadState *thr, VarSizeStackTrace traces[2],
+                          uptr addr_min, uptr addr_max) {
   Lock lock(&ctx->racy_mtx);
-  if (FindRacyAddress(ra0))
-    return true;
-  ctx->racy_addresses.PushBack(ra0);
-  return false;
+  if (flags()->suppress_equal_stacks) {
+    RacyStacks hash;
+    hash.hash[0] = md5_hash(traces[0].trace, traces[0].size * sizeof(uptr));
+    hash.hash[1] = md5_hash(traces[1].trace, traces[1].size * sizeof(uptr));
+    ctx->racy_stacks.PushBack(hash);
+  }
+  if (flags()->suppress_equal_addresses) {
+    RacyAddress ra0 = {addr_min, addr_max};
+    ctx->racy_addresses.PushBack(ra0);
+  }
 }
 
 bool OutputReport(ThreadState *thr, const ScopedReport &srep) {
@@ -614,8 +618,6 @@ void ReportRace(ThreadState *thr) {
     if (IsExpectedReport(addr_min, addr_max - addr_min))
       return;
   }
-  if (HandleRacyAddress(thr, addr_min, addr_max))
-    return;
 
   ReportType typ = ReportTypeRace;
   if (thr->is_vptr_access && freed)
@@ -666,7 +668,7 @@ void ReportRace(ThreadState *thr) {
   if (IsFiredSuppression(ctx, typ, traces[1]))
     return;
 
-  if (HandleRacyStacks(thr, traces))
+  if (HandleRacyStacks(thr, traces, addr_min, addr_max))
     return;
 
   // If any of the accesses has a tag, treat this as an "external" race.
@@ -709,6 +711,7 @@ void ReportRace(ThreadState *thr) {
   if (!OutputReport(thr, rep))
     return;
 
+  AddRacyStacks(thr, traces, addr_min, addr_max);
 }
 
 void PrintCurrentStack(ThreadState *thr, uptr pc) {