[compiler-rt] 94ea366 - tsan: fix trace tests on darwin

Dmitry Vyukov via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 27 07:41:01 PDT 2021


Author: Dmitry Vyukov
Date: 2021-09-27T16:40:57+02:00
New Revision: 94ea36649ecc854d290c6797e6adb91bdfac756d

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

LOG: tsan: fix trace tests on darwin

The trace tests crashed on darwin because of some thread
initialization issues (thread initialization is somewhat
different on darwin).
Instead of starting real threads, create a new ThreadState
in the main thread. This makes the tests more unit-testy
and hopefully won't crash on darwin (there is almost no
platform-specific code involved now).
This will also help with future trace tests that will need
more than 1 thread. Creating more than 1 real thread and
dispatching test actions across multiple threads in the
required deterministic order is painful.

Depends on D110539.

Reviewed By: melver

Differential Revision: https://reviews.llvm.org/D110546

Added: 
    

Modified: 
    compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp
    compiler-rt/lib/tsan/tests/unit/tsan_trace_test.cpp

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp b/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp
index c4545130d3ca..6134a1be2bf5 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp
@@ -456,14 +456,12 @@ static void InitializeLongjmpXorKey() {
 extern "C" void __tsan_tls_initialization() {}
 
 void ImitateTlsWrite(ThreadState *thr, uptr tls_addr, uptr tls_size) {
-  // Check that the thr object is in tls;
   const uptr thr_beg = (uptr)thr;
   const uptr thr_end = (uptr)thr + sizeof(*thr);
-  CHECK_GE(thr_beg, tls_addr);
-  CHECK_LE(thr_beg, tls_addr + tls_size);
-  CHECK_GE(thr_end, tls_addr);
-  CHECK_LE(thr_end, tls_addr + tls_size);
-  // Since the thr object is huge, skip it.
+  // ThreadState is normally allocated in TLS and is large,
+  // so we skip it. But unit tests allocate ThreadState outside of TLS.
+  if (thr_beg < tls_addr || thr_end >= tls_addr + tls_size)
+    return;
   const uptr pc = StackTrace::GetNextInstructionPc(
       reinterpret_cast<uptr>(__tsan_tls_initialization));
   MemoryRangeImitateWrite(thr, pc, tls_addr, thr_beg - tls_addr);

diff  --git a/compiler-rt/lib/tsan/tests/unit/tsan_trace_test.cpp b/compiler-rt/lib/tsan/tests/unit/tsan_trace_test.cpp
index 0863850e4f11..fb0b2b3a08f0 100644
--- a/compiler-rt/lib/tsan/tests/unit/tsan_trace_test.cpp
+++ b/compiler-rt/lib/tsan/tests/unit/tsan_trace_test.cpp
@@ -22,102 +22,115 @@ using namespace v3;
 
 // We need to run all trace tests in a new thread,
 // so that the thread trace is empty initially.
-static void run_in_thread(void *(*f)(void *), void *arg = nullptr) {
-  pthread_t th;
-  pthread_create(&th, nullptr, f, arg);
-  pthread_join(th, nullptr);
-}
-
-#if SANITIZER_MAC
-// These tests are currently failing on Mac.
-// See https://reviews.llvm.org/D107911 for more details.
-#  define MAYBE_RestoreAccess DISABLED_RestoreAccess
-#  define MAYBE_MemoryAccessSize DISABLED_MemoryAccessSize
-#  define MAYBE_RestoreMutexLock DISABLED_RestoreMutexLock
-#  define MAYBE_MultiPart DISABLED_MultiPart
-#else
-#  define MAYBE_RestoreAccess RestoreAccess
-#  define MAYBE_MemoryAccessSize MemoryAccessSize
-#  define MAYBE_RestoreMutexLock RestoreMutexLock
-#  define MAYBE_MultiPart MultiPart
-#endif
+template <uptr N>
+struct ThreadArray {
+  ThreadArray() {
+    for (auto *&thr : threads) {
+      thr = static_cast<ThreadState *>(
+          MmapOrDie(sizeof(ThreadState), "ThreadState"));
+      Tid tid = ThreadCreate(cur_thread(), 0, 0, true);
+      Processor *proc = ProcCreate();
+      ProcWire(proc, thr);
+      ThreadStart(thr, tid, 0, ThreadType::Regular);
+    }
+  }
 
-TEST(Trace, MAYBE_RestoreAccess) {
-  struct Thread {
-    static void *Func(void *arg) {
-      // A basic test with some function entry/exit events,
-      // some mutex lock/unlock events and some other distracting
-      // memory events.
-      ThreadState *thr = cur_thread();
-      TraceFunc(thr, 0x1000);
-      TraceFunc(thr, 0x1001);
-      TraceMutexLock(thr, v3::EventType::kLock, 0x4000, 0x5000, 0x6000);
-      TraceMutexLock(thr, v3::EventType::kLock, 0x4001, 0x5001, 0x6001);
-      TraceMutexUnlock(thr, 0x5000);
-      TraceFunc(thr);
-      CHECK(TryTraceMemoryAccess(thr, 0x2001, 0x3001, 8, kAccessRead));
-      TraceMutexLock(thr, v3::EventType::kRLock, 0x4002, 0x5002, 0x6002);
-      TraceFunc(thr, 0x1002);
-      CHECK(TryTraceMemoryAccess(thr, 0x2000, 0x3000, 8, kAccessRead));
-      // This is the access we want to find.
-      // The previous one is equivalent, but RestoreStack must prefer
-      // the last of the matchig accesses.
-      CHECK(TryTraceMemoryAccess(thr, 0x2002, 0x3000, 8, kAccessRead));
-      Lock lock1(&ctx->slot_mtx);
-      ThreadRegistryLock lock2(&ctx->thread_registry);
-      VarSizeStackTrace stk;
-      MutexSet mset;
-      uptr tag = kExternalTagNone;
-      bool res =
-          RestoreStack(thr->tid, v3::EventType::kAccessExt, thr->sid,
-                       thr->epoch, 0x3000, 8, kAccessRead, &stk, &mset, &tag);
-      CHECK(res);
-      CHECK_EQ(stk.size, 3);
-      CHECK_EQ(stk.trace[0], 0x1000);
-      CHECK_EQ(stk.trace[1], 0x1002);
-      CHECK_EQ(stk.trace[2], 0x2002);
-      CHECK_EQ(mset.Size(), 2);
-      CHECK_EQ(mset.Get(0).addr, 0x5001);
-      CHECK_EQ(mset.Get(0).stack_id, 0x6001);
-      CHECK_EQ(mset.Get(0).write, true);
-      CHECK_EQ(mset.Get(1).addr, 0x5002);
-      CHECK_EQ(mset.Get(1).stack_id, 0x6002);
-      CHECK_EQ(mset.Get(1).write, false);
-      CHECK_EQ(tag, kExternalTagNone);
-      return nullptr;
+  ~ThreadArray() {
+    for (uptr i = 0; i < N; i++) {
+      if (threads[i])
+        Finish(i);
     }
-  };
-  run_in_thread(Thread::Func);
+  }
+
+  void Finish(uptr i) {
+    auto *thr = threads[i];
+    threads[i] = nullptr;
+    Processor *proc = thr->proc();
+    ThreadFinish(thr);
+    ProcUnwire(proc, thr);
+    ProcDestroy(proc);
+    UnmapOrDie(thr, sizeof(ThreadState));
+  }
+
+  ThreadState *threads[N];
+  ThreadState *operator[](uptr i) { return threads[i]; }
+  ThreadState *operator->() { return threads[0]; }
+  operator ThreadState *() { return threads[0]; }
+};
+
+TEST(Trace, RestoreAccess) {
+  // A basic test with some function entry/exit events,
+  // some mutex lock/unlock events and some other distracting
+  // memory events.
+  ThreadArray<1> thr;
+  TraceFunc(thr, 0x1000);
+  TraceFunc(thr, 0x1001);
+  TraceMutexLock(thr, v3::EventType::kLock, 0x4000, 0x5000, 0x6000);
+  TraceMutexLock(thr, v3::EventType::kLock, 0x4001, 0x5001, 0x6001);
+  TraceMutexUnlock(thr, 0x5000);
+  TraceFunc(thr);
+  CHECK(TryTraceMemoryAccess(thr, 0x2001, 0x3001, 8, kAccessRead));
+  TraceMutexLock(thr, v3::EventType::kRLock, 0x4002, 0x5002, 0x6002);
+  TraceFunc(thr, 0x1002);
+  CHECK(TryTraceMemoryAccess(thr, 0x2000, 0x3000, 8, kAccessRead));
+  // This is the access we want to find.
+  // The previous one is equivalent, but RestoreStack must prefer
+  // the last of the matchig accesses.
+  CHECK(TryTraceMemoryAccess(thr, 0x2002, 0x3000, 8, kAccessRead));
+  Lock lock1(&ctx->slot_mtx);
+  ThreadRegistryLock lock2(&ctx->thread_registry);
+  VarSizeStackTrace stk;
+  MutexSet mset;
+  uptr tag = kExternalTagNone;
+  bool res =
+      RestoreStack(thr->tid, v3::EventType::kAccessExt, thr->sid, thr->epoch,
+                   0x3000, 8, kAccessRead, &stk, &mset, &tag);
+  CHECK(res);
+  CHECK_EQ(stk.size, 3);
+  CHECK_EQ(stk.trace[0], 0x1000);
+  CHECK_EQ(stk.trace[1], 0x1002);
+  CHECK_EQ(stk.trace[2], 0x2002);
+  CHECK_EQ(mset.Size(), 2);
+  CHECK_EQ(mset.Get(0).addr, 0x5001);
+  CHECK_EQ(mset.Get(0).stack_id, 0x6001);
+  CHECK_EQ(mset.Get(0).write, true);
+  CHECK_EQ(mset.Get(1).addr, 0x5002);
+  CHECK_EQ(mset.Get(1).stack_id, 0x6002);
+  CHECK_EQ(mset.Get(1).write, false);
+  CHECK_EQ(tag, kExternalTagNone);
 }
 
-TEST(Trace, MAYBE_MemoryAccessSize) {
-  struct Thread {
-    struct Params {
-      uptr access_size, offset, size;
-      bool res;
-      int type;
-    };
-    static void *Func(void *arg) {
-      // Test tracing and matching of accesses of 
diff erent sizes.
-      const Params *params = static_cast<Params *>(arg);
+TEST(Trace, MemoryAccessSize) {
+  // Test tracing and matching of accesses of 
diff erent sizes.
+  struct Params {
+    uptr access_size, offset, size;
+    bool res;
+  };
+  Params tests[] = {
+      {1, 0, 1, true},  {4, 0, 2, true},
+      {4, 2, 2, true},  {8, 3, 1, true},
+      {2, 1, 1, true},  {1, 1, 1, false},
+      {8, 5, 4, false}, {4, static_cast<uptr>(-1l), 4, false},
+  };
+  for (auto params : tests) {
+    for (int type = 0; type < 3; type++) {
+      ThreadArray<1> thr;
       Printf("access_size=%zu, offset=%zu, size=%zu, res=%d, type=%d\n",
-             params->access_size, params->offset, params->size, params->res,
-             params->type);
-      ThreadState *thr = cur_thread();
+             params.access_size, params.offset, params.size, params.res, type);
       TraceFunc(thr, 0x1000);
-      switch (params->type) {
+      switch (type) {
         case 0:
           // This should emit compressed event.
-          CHECK(TryTraceMemoryAccess(thr, 0x2000, 0x3000, params->access_size,
+          CHECK(TryTraceMemoryAccess(thr, 0x2000, 0x3000, params.access_size,
                                      kAccessRead));
           break;
         case 1:
           // This should emit full event.
-          CHECK(TryTraceMemoryAccess(thr, 0x2000000, 0x3000,
-                                     params->access_size, kAccessRead));
+          CHECK(TryTraceMemoryAccess(thr, 0x2000000, 0x3000, params.access_size,
+                                     kAccessRead));
           break;
         case 2:
-          TraceMemoryAccessRange(thr, 0x2000000, 0x3000, params->access_size,
+          TraceMemoryAccessRange(thr, 0x2000000, 0x3000, params.access_size,
                                  kAccessRead);
           break;
       }
@@ -127,105 +140,82 @@ TEST(Trace, MAYBE_MemoryAccessSize) {
       MutexSet mset;
       uptr tag = kExternalTagNone;
       bool res = RestoreStack(thr->tid, v3::EventType::kAccessExt, thr->sid,
-                              thr->epoch, 0x3000 + params->offset, params->size,
+                              thr->epoch, 0x3000 + params.offset, params.size,
                               kAccessRead, &stk, &mset, &tag);
-      CHECK_EQ(res, params->res);
-      if (params->res) {
+      CHECK_EQ(res, params.res);
+      if (params.res) {
         CHECK_EQ(stk.size, 2);
         CHECK_EQ(stk.trace[0], 0x1000);
-        CHECK_EQ(stk.trace[1], params->type ? 0x2000000 : 0x2000);
+        CHECK_EQ(stk.trace[1], type ? 0x2000000 : 0x2000);
       }
-      return nullptr;
     }
-  };
-  Thread::Params tests[] = {
-      {1, 0, 1, true, 0},  {4, 0, 2, true, 0},
-      {4, 2, 2, true, 0},  {8, 3, 1, true, 0},
-      {2, 1, 1, true, 0},  {1, 1, 1, false, 0},
-      {8, 5, 4, false, 0}, {4, static_cast<uptr>(-1l), 4, false, 0},
-  };
-  for (auto params : tests) {
-    for (params.type = 0; params.type < 3; params.type++)
-      run_in_thread(Thread::Func, &params);
   }
 }
 
-TEST(Trace, MAYBE_RestoreMutexLock) {
-  struct Thread {
-    static void *Func(void *arg) {
-      // Check of restoration of a mutex lock event.
-      ThreadState *thr = cur_thread();
-      TraceFunc(thr, 0x1000);
-      TraceMutexLock(thr, v3::EventType::kLock, 0x4000, 0x5000, 0x6000);
-      TraceMutexLock(thr, v3::EventType::kRLock, 0x4001, 0x5001, 0x6001);
-      TraceMutexLock(thr, v3::EventType::kRLock, 0x4002, 0x5001, 0x6002);
-      Lock lock1(&ctx->slot_mtx);
-      ThreadRegistryLock lock2(&ctx->thread_registry);
-      VarSizeStackTrace stk;
-      MutexSet mset;
-      uptr tag = kExternalTagNone;
-      bool res = RestoreStack(thr->tid, v3::EventType::kLock, thr->sid,
-                              thr->epoch, 0x5001, 0, 0, &stk, &mset, &tag);
-      CHECK(res);
-      CHECK_EQ(stk.size, 2);
-      CHECK_EQ(stk.trace[0], 0x1000);
-      CHECK_EQ(stk.trace[1], 0x4002);
-      CHECK_EQ(mset.Size(), 2);
-      CHECK_EQ(mset.Get(0).addr, 0x5000);
-      CHECK_EQ(mset.Get(0).stack_id, 0x6000);
-      CHECK_EQ(mset.Get(0).write, true);
-      CHECK_EQ(mset.Get(1).addr, 0x5001);
-      CHECK_EQ(mset.Get(1).stack_id, 0x6001);
-      CHECK_EQ(mset.Get(1).write, false);
-      return nullptr;
-    }
-  };
-  run_in_thread(Thread::Func);
+TEST(Trace, RestoreMutexLock) {
+  // Check of restoration of a mutex lock event.
+  ThreadArray<1> thr;
+  TraceFunc(thr, 0x1000);
+  TraceMutexLock(thr, v3::EventType::kLock, 0x4000, 0x5000, 0x6000);
+  TraceMutexLock(thr, v3::EventType::kRLock, 0x4001, 0x5001, 0x6001);
+  TraceMutexLock(thr, v3::EventType::kRLock, 0x4002, 0x5001, 0x6002);
+  Lock lock1(&ctx->slot_mtx);
+  ThreadRegistryLock lock2(&ctx->thread_registry);
+  VarSizeStackTrace stk;
+  MutexSet mset;
+  uptr tag = kExternalTagNone;
+  bool res = RestoreStack(thr->tid, v3::EventType::kLock, thr->sid, thr->epoch,
+                          0x5001, 0, 0, &stk, &mset, &tag);
+  CHECK(res);
+  CHECK_EQ(stk.size, 2);
+  CHECK_EQ(stk.trace[0], 0x1000);
+  CHECK_EQ(stk.trace[1], 0x4002);
+  CHECK_EQ(mset.Size(), 2);
+  CHECK_EQ(mset.Get(0).addr, 0x5000);
+  CHECK_EQ(mset.Get(0).stack_id, 0x6000);
+  CHECK_EQ(mset.Get(0).write, true);
+  CHECK_EQ(mset.Get(1).addr, 0x5001);
+  CHECK_EQ(mset.Get(1).stack_id, 0x6001);
+  CHECK_EQ(mset.Get(1).write, false);
 }
 
-TEST(Trace, MAYBE_MultiPart) {
-  struct Thread {
-    static void *Func(void *arg) {
-      // Check replay of a trace with multiple parts.
-      ThreadState *thr = cur_thread();
-      TraceFunc(thr, 0x1000);
-      TraceFunc(thr, 0x2000);
-      TraceMutexLock(thr, v3::EventType::kLock, 0x4000, 0x5000, 0x6000);
-      const uptr kEvents = 3 * sizeof(TracePart) / sizeof(v3::Event);
-      for (uptr i = 0; i < kEvents; i++) {
-        TraceFunc(thr, 0x3000);
-        TraceMutexLock(thr, v3::EventType::kLock, 0x4002, 0x5002, 0x6002);
-        TraceMutexUnlock(thr, 0x5002);
-        TraceFunc(thr);
-      }
-      TraceFunc(thr, 0x4000);
-      TraceMutexLock(thr, v3::EventType::kRLock, 0x4001, 0x5001, 0x6001);
-      CHECK(TryTraceMemoryAccess(thr, 0x2002, 0x3000, 8, kAccessRead));
-      Lock lock1(&ctx->slot_mtx);
-      ThreadRegistryLock lock2(&ctx->thread_registry);
-      VarSizeStackTrace stk;
-      MutexSet mset;
-      uptr tag = kExternalTagNone;
-      bool res =
-          RestoreStack(thr->tid, v3::EventType::kAccessExt, thr->sid,
-                       thr->epoch, 0x3000, 8, kAccessRead, &stk, &mset, &tag);
-      CHECK(res);
-      CHECK_EQ(stk.size, 4);
-      CHECK_EQ(stk.trace[0], 0x1000);
-      CHECK_EQ(stk.trace[1], 0x2000);
-      CHECK_EQ(stk.trace[2], 0x4000);
-      CHECK_EQ(stk.trace[3], 0x2002);
-      CHECK_EQ(mset.Size(), 2);
-      CHECK_EQ(mset.Get(0).addr, 0x5000);
-      CHECK_EQ(mset.Get(0).stack_id, 0x6000);
-      CHECK_EQ(mset.Get(0).write, true);
-      CHECK_EQ(mset.Get(1).addr, 0x5001);
-      CHECK_EQ(mset.Get(1).stack_id, 0x6001);
-      CHECK_EQ(mset.Get(1).write, false);
-      return nullptr;
-    }
-  };
-  run_in_thread(Thread::Func);
+TEST(Trace, MultiPart) {
+  // Check replay of a trace with multiple parts.
+  ThreadArray<1> thr;
+  TraceFunc(thr, 0x1000);
+  TraceFunc(thr, 0x2000);
+  TraceMutexLock(thr, v3::EventType::kLock, 0x4000, 0x5000, 0x6000);
+  const uptr kEvents = 3 * sizeof(TracePart) / sizeof(v3::Event);
+  for (uptr i = 0; i < kEvents; i++) {
+    TraceFunc(thr, 0x3000);
+    TraceMutexLock(thr, v3::EventType::kLock, 0x4002, 0x5002, 0x6002);
+    TraceMutexUnlock(thr, 0x5002);
+    TraceFunc(thr);
+  }
+  TraceFunc(thr, 0x4000);
+  TraceMutexLock(thr, v3::EventType::kRLock, 0x4001, 0x5001, 0x6001);
+  CHECK(TryTraceMemoryAccess(thr, 0x2002, 0x3000, 8, kAccessRead));
+  Lock lock1(&ctx->slot_mtx);
+  ThreadRegistryLock lock2(&ctx->thread_registry);
+  VarSizeStackTrace stk;
+  MutexSet mset;
+  uptr tag = kExternalTagNone;
+  bool res =
+      RestoreStack(thr->tid, v3::EventType::kAccessExt, thr->sid, thr->epoch,
+                   0x3000, 8, kAccessRead, &stk, &mset, &tag);
+  CHECK(res);
+  CHECK_EQ(stk.size, 4);
+  CHECK_EQ(stk.trace[0], 0x1000);
+  CHECK_EQ(stk.trace[1], 0x2000);
+  CHECK_EQ(stk.trace[2], 0x4000);
+  CHECK_EQ(stk.trace[3], 0x2002);
+  CHECK_EQ(mset.Size(), 2);
+  CHECK_EQ(mset.Get(0).addr, 0x5000);
+  CHECK_EQ(mset.Get(0).stack_id, 0x6000);
+  CHECK_EQ(mset.Get(0).write, true);
+  CHECK_EQ(mset.Get(1).addr, 0x5001);
+  CHECK_EQ(mset.Get(1).stack_id, 0x6001);
+  CHECK_EQ(mset.Get(1).write, false);
 }
 
 }  // namespace __tsan


        


More information about the llvm-commits mailing list