[compiler-rt] 27969c4 - tsan: refactor trace tests

Dmitry Vyukov via llvm-commits llvm-commits at lists.llvm.org
Sun Oct 17 03:39:01 PDT 2021


Author: Dmitry Vyukov
Date: 2021-10-17T12:38:56+02:00
New Revision: 27969c4e00bb3fcfc8cc149864e1b1f7e87a6753

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

LOG: tsan: refactor trace tests

Instead of creating real threads for trace tests
create a new ThreadState in the main thread.
This makes the tests more unit-testy and 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.

This is resubmit of reverted D110546 with 2 changes:
1. The previous version patched ImitateTlsWrite to not
expect ThreadState to be allocated in TLS (the CHECK
failed for the fake test threads).
This added an ugly hack into production code and was still
logically wrong because we imitated write to the main
thread TLS/stack when we started the fake test thread
(which has nothing to do with the main thread TLS/stack).
This version uses ThreadType::Fiber instead of ThreadType::Regular
for the fake threads. This naturally makes ThreadStart skip
obtaining stack/tls and imitating writes to them.

2. This version still skips the tests on Darwin and PowerPC
to be on the safer side. Build bots reported failures for PowerPC
for the previous version.

Reviewed By: melver

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

Added: 
    

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

Removed: 
    


################################################################################
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 0863850e4f114..c2e852d941c04 100644
--- a/compiler-rt/lib/tsan/tests/unit/tsan_trace_test.cpp
+++ b/compiler-rt/lib/tsan/tests/unit/tsan_trace_test.cpp
@@ -16,108 +16,134 @@
 #include "gtest/gtest.h"
 #include "tsan_rtl.h"
 
+#if SANITIZER_MAC || !defined(__x86_64__)
+// These tests are currently crashing on Mac:
+// https://reviews.llvm.org/D107911
+// and on ppc64: https://reviews.llvm.org/D110546#3025422
+// due to the way we create thread contexts
+// (but they crashed on Mac with normal pthread_create as well).
+// There must be some 
diff erence in thread initialization
+// between normal execution and unit tests.
+#  define TRACE_TEST(SUITE, NAME) TEST(SUITE, DISABLED_##NAME)
+#else
+#  define TRACE_TEST(SUITE, NAME) TEST(SUITE, NAME)
+#endif
+
 namespace __tsan {
 
 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::Fiber);
+    }
+  }
 
-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]; }
+};
+
+TRACE_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);
+TRACE_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 +153,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);
+TRACE_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);
+TRACE_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