[compiler-rt] 3b222ef - tsan: fixes to ThreadClock::releaseStoreAcquire and tests

[Daniel S Fava via llvm-commits]

Author: Daniel S Fava
Date: 2020-04-16T22:53:26+02:00
New Revision: 3b222ef246ef9298e57d00b7c8c1ffd92d0fb44d

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

LOG: tsan: fixes to ThreadClock::releaseStoreAcquire and tests

Fixes:
1. Setting the number of entries in a thread's clock to max between
   the thread and the SyncClock the thread is acquiring from
2. Setting last_acquire_

Unit- and stress-test for releaseStoreAcquire added to
tests/unit/tsan_clock_test.cpp

Added: 
    

Modified: 
    compiler-rt/lib/tsan/rtl/tsan_clock.cpp
    compiler-rt/lib/tsan/tests/unit/tsan_clock_test.cpp

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/tsan/rtl/tsan_clock.cpp b/compiler-rt/lib/tsan/rtl/tsan_clock.cpp
index 96bbfa1d4cc2..acbcf804194a 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_clock.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_clock.cpp
@@ -196,23 +196,36 @@ void ThreadClock::releaseStoreAcquire(ClockCache *c, SyncClock *sc) {
     return;
   }
 
-  // Check if we need to resize dst.
+  nclk_ = max(nclk_, (uptr) sc->size_);
+
+  // Check if we need to resize sc.
   if (sc->size_ < nclk_)
     sc->Resize(c, nclk_);
 
+  bool acquired = false;
+
   sc->Unshare(c);
   // Update sc->clk_.
   sc->FlushDirty();
   uptr i = 0;
   for (ClockElem &ce : *sc) {
     u64 tmp = clk_[i];
-    clk_[i] = max(ce.epoch, clk_[i]);
+    if (clk_[i] < ce.epoch) {
+      clk_[i] = ce.epoch;
+      acquired = true;
+    }
     ce.epoch = tmp;
     ce.reused = 0;
     i++;
   }
   sc->release_store_tid_ = kInvalidTid;
   sc->release_store_reused_ = 0;
+
+  if (acquired) {
+    CPP_STAT_INC(StatClockAcquiredSomething);
+    last_acquire_ = clk_[tid_];
+    ResetCached(c);
+  }
 }
 
 void ThreadClock::release(ClockCache *c, SyncClock *dst) {

diff  --git a/compiler-rt/lib/tsan/tests/unit/tsan_clock_test.cpp b/compiler-rt/lib/tsan/tests/unit/tsan_clock_test.cpp
index 6d835ba85c3b..cdaaf30b1b20 100644
--- a/compiler-rt/lib/tsan/tests/unit/tsan_clock_test.cpp
+++ b/compiler-rt/lib/tsan/tests/unit/tsan_clock_test.cpp
@@ -108,6 +108,31 @@ TEST(Clock, RepeatedAcquire) {
   sync.Reset(&cache);
 }
 
+TEST(Clock, releaseStoreAcquire) {
+  ThreadClock thr0(0);
+  thr0.tick();
+  ThreadClock thr1(1);
+  thr1.tick();
+  SyncClock syncA;
+  SyncClock syncB;
+  ASSERT_EQ(syncA.size(), 0U);
+  ASSERT_EQ(syncB.size(), 0U);
+  thr1.releaseStoreAcquire(&cache, &syncB);
+  ASSERT_EQ(syncB.size(), 2U); // T0 and T1
+  // releaseStoreAcquire to an empty SyncClock
+  thr0.releaseStoreAcquire(&cache, &syncA);
+  ASSERT_EQ(syncA.size(), 1U);
+  // releaseStoreAcquire from a non-empty SyncClock
+  // T0 learns about T1
+  thr0.releaseStoreAcquire(&cache, &syncB);
+  // releaseStoreAcquire to the originally empty SyncClock
+  // T0 deposits info about T1 into syncA
+  thr0.releaseStoreAcquire(&cache, &syncA);
+  ASSERT_EQ(syncA.size(), 2U);
+  syncA.Reset(&cache);
+  syncB.Reset(&cache);
+}
+
 TEST(Clock, ManyThreads) {
   SyncClock chunked;
   for (unsigned i = 0; i < 200; i++) {
@@ -336,6 +361,18 @@ struct SimpleThreadClock {
       dst->clock[i] = max(dst->clock[i], clock[i]);
   }
 
+  void releaseStoreAcquire(SimpleSyncClock *sc) {
+    if (sc->size < size)
+      sc->size = size;
+    else
+      size = sc->size;
+    for (uptr i = 0; i < kThreads; i++) {
+      uptr tmp = clock[i];
+      clock[i] = max(sc->clock[i], clock[i]);
+      sc->clock[i] = tmp;
+    }
+  }
+
   void acq_rel(SimpleSyncClock *dst) {
     acquire(dst);
     release(dst);
@@ -390,7 +427,7 @@ static bool ClockFuzzer(bool printing) {
     thr0[tid]->tick();
     thr1[tid]->tick();
 
-    switch (rand() % 6) {
+    switch (rand() % 7) {
     case 0:
       if (printing)
         printf("acquire thr%d <- clk%d\n", tid, cid);
@@ -422,6 +459,12 @@ static bool ClockFuzzer(bool printing) {
       sync1[cid]->Reset(&cache);
       break;
     case 5:
+      if (printing)
+        printf("releaseStoreAcquire thr%d -> clk%d\n", tid, cid);
+      thr0[tid]->releaseStoreAcquire(sync0[cid]);
+      thr1[tid]->releaseStoreAcquire(&cache, sync1[cid]);
+      break;
+    case 6:
       if (printing)
         printf("reset thr%d\n", tid);
       u64 epoch = thr0[tid]->clock[tid] + 1;