[compiler-rt] r274619 - [tsan] Fix false positives with GCD dispatch_source_*

[Kuba Brecka via llvm-commits]

Author: kuba.brecka
Date: Wed Jul  6 06:02:49 2016
New Revision: 274619

URL: http://llvm.org/viewvc/llvm-project?rev=274619&view=rev
Log:
[tsan] Fix false positives with GCD dispatch_source_*

We already have interceptors for dispatch_source API (e.g. dispatch_source_set_event_handler), but they currently only handle submission synchronization. We also need to synchronize based on the target queue (serial, concurrent), in other words, we need to use dispatch_callback_wrap. This patch implements that.

Differential Revision: http://reviews.llvm.org/D21999


Added:
    compiler-rt/trunk/test/tsan/Darwin/gcd-source-serial.mm
Modified:
    compiler-rt/trunk/lib/tsan/rtl/tsan_libdispatch_mac.cc

Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_libdispatch_mac.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_libdispatch_mac.cc?rev=274619&r1=274618&r2=274619&view=diff
==============================================================================
--- compiler-rt/trunk/lib/tsan/rtl/tsan_libdispatch_mac.cc (original)
+++ compiler-rt/trunk/lib/tsan/rtl/tsan_libdispatch_mac.cc Wed Jul  6 06:02:49 2016
@@ -67,6 +67,15 @@ static bool IsQueueSerial(dispatch_queue
   return width == 1;
 }
 
+static dispatch_queue_t GetTargetQueueFromSource(dispatch_source_t source) {
+  CHECK_EQ(dispatch_queue_offsets.dqo_target_queue_size, 8);
+  dispatch_queue_t target_queue =
+      *(dispatch_queue_t *)(((uptr)source) +
+                            dispatch_queue_offsets.dqo_target_queue);
+  CHECK_NE(target_queue, 0);
+  return target_queue;
+}
+
 static tsan_block_context_t *AllocContext(ThreadState *thr, uptr pc,
                                           dispatch_queue_t queue,
                                           void *orig_context,
@@ -106,6 +115,11 @@ static void dispatch_callback_wrap(void
   if (context->free_context_in_callback) user_free(thr, pc, context);
 }
 
+static void invoke_block(void *param) {
+  dispatch_block_t block = (dispatch_block_t)param;
+  block();
+}
+
 static void invoke_and_release_block(void *param) {
   dispatch_block_t block = (dispatch_block_t)param;
   block();
@@ -342,15 +356,17 @@ TSAN_INTERCEPTOR(void, dispatch_source_s
   SCOPED_TSAN_INTERCEPTOR(dispatch_source_set_event_handler, source, handler);
   if (handler == nullptr)
     return REAL(dispatch_source_set_event_handler)(source, nullptr);
-  dispatch_block_t new_handler = ^(void) {
-    {
-      SCOPED_INTERCEPTOR_RAW(dispatch_source_set_event_handler_callback);
-      Acquire(thr, pc, (uptr)source);
-    }
-    handler();
-  };
-  Release(thr, pc, (uptr)source);
+  dispatch_queue_t q = GetTargetQueueFromSource(source);
+  __block tsan_block_context_t new_context = {
+      q, handler, &invoke_block, false, false, false };
+  dispatch_block_t new_handler = Block_copy(^(void) {
+    new_context.orig_context = handler;  // To explicitly capture "handler".
+    dispatch_callback_wrap(&new_context);
+  });
+  uptr submit_sync = (uptr)&new_context;
+  Release(thr, pc, submit_sync);
   REAL(dispatch_source_set_event_handler)(source, new_handler);
+  Block_release(new_handler);
 }
 
 TSAN_INTERCEPTOR(void, dispatch_source_set_event_handler_f,
@@ -369,15 +385,17 @@ TSAN_INTERCEPTOR(void, dispatch_source_s
   SCOPED_TSAN_INTERCEPTOR(dispatch_source_set_cancel_handler, source, handler);
   if (handler == nullptr)
     return REAL(dispatch_source_set_cancel_handler)(source, nullptr);
-  dispatch_block_t new_handler = ^(void) {
-    {
-      SCOPED_INTERCEPTOR_RAW(dispatch_source_set_cancel_handler_callback);
-      Acquire(thr, pc, (uptr)source);
-    }
-    handler();
-  };
-  Release(thr, pc, (uptr)source);
+  dispatch_queue_t q = GetTargetQueueFromSource(source);
+  __block tsan_block_context_t new_context = {
+      q, handler, &invoke_block, false, false, false };
+  dispatch_block_t new_handler = Block_copy(^(void) {
+    new_context.orig_context = handler;  // To explicitly capture "handler".
+    dispatch_callback_wrap(&new_context);
+  });
+  uptr submit_sync = (uptr)&new_context;
+  Release(thr, pc, submit_sync);
   REAL(dispatch_source_set_cancel_handler)(source, new_handler);
+  Block_release(new_handler);
 }
 
 TSAN_INTERCEPTOR(void, dispatch_source_set_cancel_handler_f,
@@ -398,15 +416,17 @@ TSAN_INTERCEPTOR(void, dispatch_source_s
                           handler);
   if (handler == nullptr)
     return REAL(dispatch_source_set_registration_handler)(source, nullptr);
-  dispatch_block_t new_handler = ^(void) {
-    {
-      SCOPED_INTERCEPTOR_RAW(dispatch_source_set_registration_handler_callback);
-      Acquire(thr, pc, (uptr)source);
-    }
-    handler();
-  };
-  Release(thr, pc, (uptr)source);
+  dispatch_queue_t q = GetTargetQueueFromSource(source);
+  __block tsan_block_context_t new_context = {
+      q, handler, &invoke_block, false, false, false };
+  dispatch_block_t new_handler = Block_copy(^(void) {
+    new_context.orig_context = handler;  // To explicitly capture "handler".
+    dispatch_callback_wrap(&new_context);
+  });
+  uptr submit_sync = (uptr)&new_context;
+  Release(thr, pc, submit_sync);
   REAL(dispatch_source_set_registration_handler)(source, new_handler);
+  Block_release(new_handler);
 }
 
 TSAN_INTERCEPTOR(void, dispatch_source_set_registration_handler_f,

Added: compiler-rt/trunk/test/tsan/Darwin/gcd-source-serial.mm
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/tsan/Darwin/gcd-source-serial.mm?rev=274619&view=auto
==============================================================================
--- compiler-rt/trunk/test/tsan/Darwin/gcd-source-serial.mm (added)
+++ compiler-rt/trunk/test/tsan/Darwin/gcd-source-serial.mm Wed Jul  6 06:02:49 2016
@@ -0,0 +1,33 @@
+// RUN: %clang_tsan %s -o %t -framework Foundation
+// RUN: %env_tsan_opts=ignore_interceptors_accesses=1 %run %t 2>&1 | FileCheck %s
+
+#import <Foundation/Foundation.h>
+
+long global;
+
+int main(int argc, const char *argv[]) {
+  fprintf(stderr, "Hello world.\n");
+
+  dispatch_queue_t q = dispatch_queue_create("my.queue", DISPATCH_QUEUE_SERIAL);
+  dispatch_semaphore_t sem = dispatch_semaphore_create(0);
+  dispatch_source_t timer = dispatch_source_create(DISPATCH_SOURCE_TYPE_TIMER, 0, 0, q);
+  long long interval_ms = 10;
+  dispatch_source_set_timer(timer, dispatch_time(DISPATCH_TIME_NOW, 0), interval_ms * NSEC_PER_MSEC, 0);
+  dispatch_source_set_event_handler(timer, ^{
+    fprintf(stderr, "timer\n");
+    global++;
+
+    if (global > 50) {
+      dispatch_semaphore_signal(sem);
+    }
+  });
+  dispatch_resume(timer);
+
+  dispatch_semaphore_wait(sem, DISPATCH_TIME_FOREVER);
+
+  fprintf(stderr, "Done.\n");
+}
+
+// CHECK: Hello world.
+// CHECK-NOT: WARNING: ThreadSanitizer
+// CHECK: Done.