[compiler-rt] r255494 - [tsan] Update dispatch_group support to avoid using a disposed group object

[Kuba Brecka via llvm-commits]

Author: kuba.brecka
Date: Mon Dec 14 07:32:57 2015
New Revision: 255494

URL: http://llvm.org/viewvc/llvm-project?rev=255494&view=rev
Log:
[tsan] Update dispatch_group support to avoid using a disposed group object

We're using the dispatch group itself to synchronize (to call Release() and Acquire() on it), but in dispatch group notifications, the group can already be disposed/deallocated. This causes a later assertion failure at `DCHECK_EQ(*meta, 0);` in `MetaMap::AllocBlock` when the same memory is reused (note that the failure only happens in debug builds).

Fixing this by retaining the group and releasing it in the notification. Adding a stress test case that reproduces this.

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


Added:
    compiler-rt/trunk/test/tsan/Darwin/gcd-groups-stress.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=255494&r1=255493&r2=255494&view=diff
==============================================================================
--- compiler-rt/trunk/lib/tsan/rtl/tsan_libdispatch_mac.cc (original)
+++ compiler-rt/trunk/lib/tsan/rtl/tsan_libdispatch_mac.cc Mon Dec 14 07:32:57 2015
@@ -34,6 +34,7 @@ typedef struct {
   void *orig_context;
   dispatch_function_t orig_work;
   uptr object_to_acquire;
+  dispatch_object_t object_to_release;
 } tsan_block_context_t;
 
 // The offsets of different fields of the dispatch_queue_t structure, exported
@@ -75,6 +76,7 @@ static tsan_block_context_t *AllocContex
   new_context->orig_context = orig_context;
   new_context->orig_work = orig_work;
   new_context->object_to_acquire = (uptr)new_context;
+  new_context->object_to_release = nullptr;
   return new_context;
 }
 
@@ -82,6 +84,13 @@ static void dispatch_callback_wrap_acqui
   SCOPED_INTERCEPTOR_RAW(dispatch_async_f_callback_wrap);
   tsan_block_context_t *context = (tsan_block_context_t *)param;
   Acquire(thr, pc, context->object_to_acquire);
+
+  // Extra retain/release is required for dispatch groups. We use the group
+  // itself to synchronize, but in a notification (dispatch_group_notify
+  // callback), it may be disposed already. To solve this, we retain the group
+  // and release it here.
+  if (context->object_to_release) dispatch_release(context->object_to_release);
+
   // In serial queues, work items can be executed on different threads, we need
   // to explicitly synchronize on the queue itself.
   if (IsQueueSerial(context->queue)) Acquire(thr, pc, (uptr)context->queue);
@@ -231,6 +240,11 @@ TSAN_INTERCEPTOR(void, dispatch_group_no
   tsan_block_context_t *new_context =
       AllocContext(thr, pc, q, heap_block, &invoke_and_release_block);
   new_context->object_to_acquire = (uptr)group;
+
+  // Will be released in dispatch_callback_wrap_acquire.
+  new_context->object_to_release = group;
+  dispatch_retain(group);
+
   Release(thr, pc, (uptr)group);
   REAL(dispatch_group_notify_f)(group, q, new_context,
                                 dispatch_callback_wrap_acquire);
@@ -241,6 +255,11 @@ TSAN_INTERCEPTOR(void, dispatch_group_no
   SCOPED_TSAN_INTERCEPTOR(dispatch_group_notify_f, group, q, context, work);
   tsan_block_context_t *new_context = AllocContext(thr, pc, q, context, work);
   new_context->object_to_acquire = (uptr)group;
+
+  // Will be released in dispatch_callback_wrap_acquire.
+  new_context->object_to_release = group;
+  dispatch_retain(group);
+
   Release(thr, pc, (uptr)group);
   REAL(dispatch_group_notify_f)(group, q, new_context,
                                 dispatch_callback_wrap_acquire);

Added: compiler-rt/trunk/test/tsan/Darwin/gcd-groups-stress.mm
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/tsan/Darwin/gcd-groups-stress.mm?rev=255494&view=auto
==============================================================================
--- compiler-rt/trunk/test/tsan/Darwin/gcd-groups-stress.mm (added)
+++ compiler-rt/trunk/test/tsan/Darwin/gcd-groups-stress.mm Mon Dec 14 07:32:57 2015
@@ -0,0 +1,43 @@
+// RUN: %clang_tsan %s -o %t -framework Foundation
+// RUN: %run %t 2>&1
+
+#import <Foundation/Foundation.h>
+
+void notify_callback(void *context) {
+  // Do nothing.
+}
+
+int main() {
+  NSLog(@"Hello world.");
+
+  dispatch_queue_t q = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0);
+  
+  for (int i = 0; i < 300000; i++) {
+    dispatch_group_t g = dispatch_group_create();
+    dispatch_group_enter(g);
+    dispatch_async(q, ^{
+      dispatch_group_leave(g);
+    });
+    dispatch_group_notify(g, q, ^{
+      // Do nothing.
+    });
+    dispatch_release(g);
+  }
+
+  for (int i = 0; i < 300000; i++) {
+    dispatch_group_t g = dispatch_group_create();
+    dispatch_group_enter(g);
+    dispatch_async(q, ^{
+      dispatch_group_leave(g);
+    });
+    dispatch_group_notify_f(g, q, nullptr, &notify_callback);
+    dispatch_release(g);
+  }
+
+  NSLog(@"Done.");
+}
+
+// CHECK: Hello world.
+// CHECK: Done.
+// CHECK-NOT: WARNING: ThreadSanitizer
+// CHECK-NOT: CHECK failed