[PATCH] D15288: tsan: fix test invisible barrier

[Dmitry Vyukov via llvm-commits]

dvyukov created this revision.
dvyukov added reviewers: kcc, eugenis.
dvyukov added subscribers: kubabrecka, llvm-commits, krasin.

Another attempt at fixing tsan_invisible_barrier.
Current implementation causes:
https://llvm.org/bugs/show_bug.cgi?id=25643

There were several unsuccessful iterations for this functionality:
 1. Initially it was implemented in user code using
    REAL(pthread_barrier_wait). But pthread_barrier_wait is not supported on
    MacOS. Futexes are linux-specific for this matter.
 2. Then we switched to atomics+usleep(10). But usleep produced parasitic
    "as-if synchronized via sleep" messages in reports which failed some
    output tests.
 3. Then we switched to atomics+sched_yield. But this produced tons of tsan-
    visible events, which lead to "failed to restore stack trace" failures.

Move implementation into runtime and use SleepForMillis in the wait loop.
This way tsan should see no events from the barrier, so not trace overflows and
no "as-if synchronized via sleep" messages.


http://reviews.llvm.org/D15288

Files:
  lib/tsan/rtl/tsan_interceptors.cc
  test/tsan/test.h

Index: test/tsan/test.h
===================================================================
--- test/tsan/test.h
+++ test/tsan/test.h
@@ -14,27 +14,20 @@
 // TSan-invisible barrier.
 // Tests use it to establish necessary execution order in a way that does not
 // interfere with tsan (does not establish synchronization between threads).
-// 8 lsb is thread count, the remaining are count of entered threads.
 typedef unsigned long long invisible_barrier_t;
 
+extern "C"
+void __tsan_invisible_barrier_init(invisible_barrier_t *barrier,
+    unsigned count);
+extern "C"
+void __tsan_invisible_barrier_wait(invisible_barrier_t *barrier);
+
 void barrier_init(invisible_barrier_t *barrier, unsigned count) {
-  if (count >= (1 << 8))
-      exit(fprintf(stderr, "barrier_init: count is too large (%d)\n", count));
-  *barrier = count;
+  __tsan_invisible_barrier_init(barrier, count);
 }
 
 void barrier_wait(invisible_barrier_t *barrier) {
-  unsigned old = __atomic_fetch_add(barrier, 1 << 8, __ATOMIC_RELAXED);
-  unsigned old_epoch = (old >> 8) / (old & 0xff);
-  for (;;) {
-    unsigned cur = __atomic_load_n(barrier, __ATOMIC_RELAXED);
-    unsigned cur_epoch = (cur >> 8) / (cur & 0xff);
-    if (cur_epoch != old_epoch)
-      return;
-    // Can't use usleep, because it leads to spurious "As if synchronized via
-    // sleep" messages which fail some output tests.
-    sched_yield();
-  }
+  __tsan_invisible_barrier_wait(barrier);
 }
 
 // Default instance of the barrier, but a test can declare more manually.
Index: lib/tsan/rtl/tsan_interceptors.cc
===================================================================
--- lib/tsan/rtl/tsan_interceptors.cc
+++ lib/tsan/rtl/tsan_interceptors.cc
@@ -2683,3 +2683,37 @@
 }
 
 }  // namespace __tsan
+
+// Invisible barrier for tests.
+// There were several unsuccessful iterations for this functionality:
+// 1. Initially it was implemented in user code using
+//    REAL(pthread_barrier_wait). But pthread_barrier_wait is not supported on
+//    MacOS. Futexes are linux-specific for this matter.
+// 2. Then we switched to atomics+usleep(10). But usleep produced parasitic
+//    "as-if synchronized via sleep" messages in reports which failed some
+//    output tests.
+// 3. Then we switched to atomics+sched_yield. But this produced tons of tsan-
+//    visible events, which lead to "failed to restore stack trace" failures.
+// That's why we now have what we have.
+extern "C"
+void __tsan_invisible_barrier_init(u64 *barrier, u32 count) {
+  if (count >= (1 << 8)) {
+      Printf("barrier_init: count is too large (%d)\n", count);
+      Die();
+  }
+  // 8 lsb is thread count, the remaining are count of entered threads.
+  *barrier = count;
+}
+
+extern "C"
+void __tsan_invisible_barrier_wait(u64 *barrier) {
+  unsigned old = __atomic_fetch_add(barrier, 1 << 8, __ATOMIC_RELAXED);
+  unsigned old_epoch = (old >> 8) / (old & 0xff);
+  for (;;) {
+    unsigned cur = __atomic_load_n(barrier, __ATOMIC_RELAXED);
+    unsigned cur_epoch = (cur >> 8) / (cur & 0xff);
+    if (cur_epoch != old_epoch)
+      return;
+    SleepForMillis(1);
+  }
+}


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D15288.42062.patch
Type: text/x-patch
Size: 3121 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151207/a3d4ab96/attachment.bin>