[PATCH] D14745: [tsan] Implement basic GCD interceptors for OS X

[Dmitry Vyukov via llvm-commits]

dvyukov added inline comments.

================
Comment at: lib/tsan/rtl/tsan_libdispatch_mac.cc:1
@@ +1,2 @@
+//===-- tsan_platform_mac.cc ----------------------------------------------===//
+//
----------------
s/tsan_platform_mac.cc/tsan_libdispatch_mac.cc/

================
Comment at: lib/tsan/rtl/tsan_libdispatch_mac.cc:162
@@ +161,3 @@
+// with this in instrumented code, but there's not much we can do about it in
+// system libraries. Let's instead ignore all accesses from the initializer, so
+// we don't see false positives.
----------------
I see the issue with non-instrumented system libraries.
Does it actually happen?


================
Comment at: lib/tsan/rtl/tsan_libdispatch_mac.cc:171
@@ +170,3 @@
+  REAL(dispatch_once)(predicate, ^{
+    ThreadIgnoreBegin(thr, pc);
+    block();
----------------
Why do you use a code block here, but don't use it in dispatch_once_f?
Can't we simply wrap REAL(dispatch_once) in ThreadIgnoreBegin/End here as well?

================
Comment at: lib/tsan/rtl/tsan_suppressions.cc:39
@@ +38,3 @@
+"called_from_lib:CoreFoundation\n"
+"called_from_lib:libxpc.dylib\n"
+"called_from_lib:libobjc.A.dylib\n"
----------------
Note that if you ignore interceptors, this can also introduce new false positives because of missed synchronization. So if you ignore a library you need to annotate all synchonization this library provides to user code.

================
Comment at: test/tsan/Darwin/gcd-simple.mm:20
@@ +19,2 @@
+// CHECK: Hello world
+// CHECK-NOT: WARNING: ThreadSanitizer
----------------
Also print DONE at the end of the program and check that it is printed. It happened several times that interceptors crash badly, but tests pass.


http://reviews.llvm.org/D14745