[PATCH] D31355: [tsan] Only Acquire/Release GCD queues if they're not NULL

[Kuba (Brecka) Mracek via Phabricator via llvm-commits]

kubamracek created this revision.

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.


https://reviews.llvm.org/D31355

Files:
  lib/tsan/rtl/tsan_libdispatch_mac.cc
  test/tsan/Darwin/gcd-after-null.mm


Index: test/tsan/Darwin/gcd-after-null.mm
===================================================================
--- test/tsan/Darwin/gcd-after-null.mm
+++ test/tsan/Darwin/gcd-after-null.mm
@@ -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
Index: lib/tsan/rtl/tsan_libdispatch_mac.cc
===================================================================
--- lib/tsan/rtl/tsan_libdispatch_mac.cc
+++ lib/tsan/rtl/tsan_libdispatch_mac.cc
@@ -97,11 +97,11 @@
   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 @@
   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 @@
   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);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D31355.93009.patch
Type: text/x-patch
Size: 2657 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170324/ede9ab74/attachment.bin>