[tsan] Support C++11 call_once in TSan on Darwin

[kubamracek]

Summary

This patch adds a wrapper for call_once, which uses an already-compiled helper __call_once with an atomic release which is invisible to TSan. To avoid false positives, the interceptor performs an explicit atomic release in the callback wrapper.

The test passes only after a real race in libc++ is fixed, patch at https://reviews.llvm.org/D24028.

[dvyukov]

lib/tsan/rtl/tsan_interceptors_mac.cc

355 ↗

(On Diff #70151)

Do we need an acquire here?
Consider that two threads fall onto this slow path. One calls the user function and does release. The other thread waits on some internal synchronization and returns. What's missing is release->acquire between the first thread and the second thread.

[kubamracek]

lib/tsan/rtl/tsan_interceptors_mac.cc

355 ↗

(On Diff #70151)

I don’t think it’s necessary, as __call_once uses a pthread condvar to suspend the other thread, which is TSan-visible and it already provides the release->acquire from thread A, after it has already run user code, to the waiting thread B.

[dvyukov]

asd

lib/tsan/rtl/tsan_interceptors_mac.cc

355 ↗

(On Diff #70151)

You mean pthread_mutex, right? Condition variables don't provide any synchronization.

[kubamracek]

lib/tsan/rtl/tsan_interceptors_mac.cc

355 ↗

(On Diff #70151)

I mean the thread waits by this:

pthread_mutex_lock(&mut);
while (flag == 1) pthread_condvar_wait(&cv, &mut);
pthread_mutex_unlock(&mut);

Doesn’t pthread_condvar_wait perform an “acquire” of the mutex before it returns?

[dvyukov]

lib/tsan/rtl/tsan_interceptors_mac.cc

355 ↗

(On Diff #70151)

It locks the mutex (maybe), but it's the mutex that does acquire.
OK, nevermind.

[zaks.anna]

compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors_mac.cc

352

This does not have a call to SCOPED_TSAN_INTERCEPTOR. Does it need it?

[dvyukov]

compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors_mac.cc

352

SCOPED_TSAN_INTERCEPTOR allows to skip/ignore an interceptor and process pending signals. It can also do logging in debug mode. All of that is not strictly necessary, but better to have here as well for consistency.

[kubamracek]

compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors_mac.cc

352

Also, adding SCOPED_TSAN_INTERCEPTOR disables tail-call optimization and enabling ignores here could cause false positives. These were the reasons why I left it out.

compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors_mac.cc

if (__tsan_atomic64_fetch_add(&o->shared_owners, -1, mo_release) == 0) {
     if (__tsan_atomic64_fetch_add(&o->shared_weak_owners, -1, mo_release) ==
         0) {
       Acquire(thr, pc, (uptr)&o->shared_weak_owners);
       o->on_zero_shared_weak();
     }
   }
 }
 
+namespace {
+struct call_once_callback_args {
+  void (*orig_func)(void *arg);
+  void *orig_arg;
+  void *flag;
+};
+
+void call_once_callback_wrapper(void *arg) {
+  call_once_callback_args *new_args = (call_once_callback_args *)arg;
+  new_args->orig_func(new_args->orig_arg);
+  __tsan_release(new_args->flag);
+}
+}  // namespace
+
+// This adds a libc++ interceptor for:
+//     void __call_once(volatile unsigned long&, void*, void(*)(void*));
+// C++11 call_once is implemented via an internal function __call_once which is
+// inside libc++.dylib, and the atomic release store inside it is thus
+// TSan-invisible. To avoid false positives, this interceptor wraps the callback
+// function and performs an explicit Release after the user code has run.
+STDCXX_INTERCEPTOR(void, _ZNSt3__111__call_onceERVmPvPFvS2_E, void *flag,
+                   void *arg, void (*func)(void *arg)) {
+  call_once_callback_args new_args = {func, arg, flag};
+  REAL(_ZNSt3__111__call_onceERVmPvPFvS2_E)(flag, &new_args,
+                                            call_once_callback_wrapper);
+}
+
 }  // namespace __tsan
 
 #endif  // SANITIZER_MAC

compiler-rt/trunk/test/tsan/Darwin/libcxx-call-once.mm

+// RUN: %clangxx_tsan %s -o %t -framework Foundation -std=c++11
+// RUN: %env_tsan_opts=ignore_interceptors_accesses=1 %run %t 2>&1 | FileCheck %s
+
+#import <Foundation/Foundation.h>
+
+#import <iostream>
+#import <thread>
+
+long my_global;
+std::once_flag once_token;
+
+void thread_func() {
+  std::call_once(once_token, [] {
+    my_global = 17;
+  });
+
+  long val = my_global;
+  fprintf(stderr, "my_global = %ld\n", val);
+}
+
+int main(int argc, const char *argv[]) {
+  fprintf(stderr, "Hello world.\n");
+
+  std::thread t1(thread_func);
+  std::thread t2(thread_func);
+  t1.join();
+  t2.join();
+
+  fprintf(stderr, "Done.\n");
+}
+
+// CHECK: Hello world.
+// CHECK-NOT: WARNING: ThreadSanitizer
+// CHECK: Done.