[compiler-rt] r298820 - [tsan] Only Acquire/Release GCD queues if they're not NULL

[Kuba Mracek via llvm-commits]

Author: kuba.brecka
Date: Sun Mar 26 15:59:23 2017
New Revision: 298820

URL: http://llvm.org/viewvc/llvm-project?rev=298820&view=rev
Log:
[tsan] Only Acquire/Release GCD queues if they're not NULL

While it's usually a bug to call GCD APIs, such as dispatch_after, with NULL as a queue, this often "somehow" works and TSan should maintain binary compatibility with existing code. This patch makes sure we don't try to call Acquire and Release on NULL queues, and add one such testcase for dispatch_after.

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


Added:
    compiler-rt/trunk/test/tsan/Darwin/gcd-after-null.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=298820&r1=298819&r2=298820&view=diff
==============================================================================
--- compiler-rt/trunk/lib/tsan/rtl/tsan_libdispatch_mac.cc (original)
+++ compiler-rt/trunk/lib/tsan/rtl/tsan_libdispatch_mac.cc Sun Mar 26 15:59:23 2017
@@ -97,11 +97,11 @@ static tsan_block_context_t *AllocContex
   return new_context;
 }
 
-#define GET_QUEUE_SYNC_VARS(context, q)                      \
-  bool is_queue_serial = q && IsQueueSerial(q);              \
-  uptr sync_ptr = (uptr)q ?: context->non_queue_sync_object; \
-  uptr serial_sync = (uptr)sync_ptr;                         \
-  uptr concurrent_sync = ((uptr)sync_ptr) + sizeof(uptr);    \
+#define GET_QUEUE_SYNC_VARS(context, q)                                  \
+  bool is_queue_serial = q && IsQueueSerial(q);                          \
+  uptr sync_ptr = (uptr)q ?: context->non_queue_sync_object;             \
+  uptr serial_sync = (uptr)sync_ptr;                                     \
+  uptr concurrent_sync = sync_ptr ? ((uptr)sync_ptr) + sizeof(uptr) : 0; \
   bool serial_task = context->is_barrier_block || is_queue_serial
 
 static void dispatch_sync_pre_execute(ThreadState *thr, uptr pc,
@@ -112,8 +112,8 @@ static void dispatch_sync_pre_execute(Th
   dispatch_queue_t q = context->queue;
   do {
     GET_QUEUE_SYNC_VARS(context, q);
-    Acquire(thr, pc, serial_sync);
-    if (serial_task) Acquire(thr, pc, concurrent_sync);
+    if (serial_sync) Acquire(thr, pc, serial_sync);
+    if (serial_task && concurrent_sync) Acquire(thr, pc, concurrent_sync);
 
     if (q) q = GetTargetQueueFromQueue(q);
   } while (q);
@@ -127,7 +127,8 @@ static void dispatch_sync_post_execute(T
   dispatch_queue_t q = context->queue;
   do {
     GET_QUEUE_SYNC_VARS(context, q);
-    Release(thr, pc, serial_task ? serial_sync : concurrent_sync);
+    if (serial_task && serial_sync) Release(thr, pc, serial_sync);
+    if (!serial_task && concurrent_sync) Release(thr, pc, concurrent_sync);
 
     if (q) q = GetTargetQueueFromQueue(q);
   } while (q);

Added: compiler-rt/trunk/test/tsan/Darwin/gcd-after-null.mm
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/tsan/Darwin/gcd-after-null.mm?rev=298820&view=auto
==============================================================================
--- compiler-rt/trunk/test/tsan/Darwin/gcd-after-null.mm (added)
+++ compiler-rt/trunk/test/tsan/Darwin/gcd-after-null.mm Sun Mar 26 15:59:23 2017
@@ -0,0 +1,23 @@
+// Regression test to make sure we don't crash when dispatch_after is called with a NULL queue.
+
+// RUN: %clang_tsan %s -o %t -framework Foundation
+// RUN: %run %t 2>&1 | FileCheck %s
+
+#import <Foundation/Foundation.h>
+
+int main(int argc, const char *argv[]) {
+  fprintf(stderr, "start\n");
+
+  dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(10 * NSEC_PER_MSEC)), NULL, ^{
+    dispatch_async(dispatch_get_main_queue(), ^{
+      CFRunLoopStop(CFRunLoopGetMain());
+    });
+  });
+  CFRunLoopRun();
+
+  fprintf(stderr, "done\n");
+  return 0;
+}
+
+// CHECK: start
+// CHECK: done