[tsan] Synchronize leaving a GCD group with notifications

[kubamracek]

Summary

In the patch that introduced support for GCD barrier blocks, I removed releasing a group when leaving it (in dispatch_group_leave). However, this is necessary to synchronize leaving a group and a notification callback (dispatch_group_notify). Adding this back, simplifying dispatch_group_notify_f and adding a test case.

compiler-rt/trunk/lib/tsan/rtl/tsan_libdispatch_mac.cc

TSAN_INTERCEPTOR(long_t, dispatch_group_wait, dispatch_group_t group,
   SCOPED_TSAN_INTERCEPTOR(dispatch_group_wait, group, timeout);
   long_t result = REAL(dispatch_group_wait)(group, timeout);
   if (result == 0) Acquire(thr, pc, (uptr)group);
   return result;
 }
 
 TSAN_INTERCEPTOR(void, dispatch_group_leave, dispatch_group_t group) {
   SCOPED_TSAN_INTERCEPTOR(dispatch_group_leave, group);
+  // Acquired in the group noticifaction callback in dispatch_group_notify[_f].
   Release(thr, pc, (uptr)group);
   REAL(dispatch_group_leave)(group);
 }
 
 TSAN_INTERCEPTOR(void, dispatch_group_async, dispatch_group_t group,
                  dispatch_queue_t queue, dispatch_block_t block) {
   SCOPED_TSAN_INTERCEPTOR(dispatch_group_async, group, queue, block);
   dispatch_retain(group);
WRAP(dispatch_async)(queue, ^(void) {
     WRAP(dispatch_group_leave)(group);
     dispatch_release(group);
   });
 }
 
 TSAN_INTERCEPTOR(void, dispatch_group_notify, dispatch_group_t group,
                  dispatch_queue_t q, dispatch_block_t block) {
   SCOPED_TSAN_INTERCEPTOR(dispatch_group_notify, group, q, block);
+
+  // To make sure the group is still available in the callback (otherwise
+  // it can be already destroyed).  Will be released in the callback.
+  dispatch_retain(group);
+
   SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_START();
-  dispatch_block_t heap_block = Block_copy(block);
+  dispatch_block_t heap_block = Block_copy(^(void) {
+    {
+      SCOPED_INTERCEPTOR_RAW(dispatch_read_callback);
+      // Released when leaving the group (dispatch_group_leave).
+      Acquire(thr, pc, (uptr)group);
+    }
+    dispatch_release(group);
+    block();
+  });
   SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_END();
   tsan_block_context_t *new_context =
       AllocContext(thr, pc, q, heap_block, &invoke_and_release_block);
   new_context->is_barrier_block = true;
   Release(thr, pc, (uptr)new_context);
-  REAL(dispatch_group_notify_f)(group, q, new_context,
-                                dispatch_callback_wrap);
+  REAL(dispatch_group_notify_f)(group, q, new_context, dispatch_callback_wrap);
 }
 
 TSAN_INTERCEPTOR(void, dispatch_group_notify_f, dispatch_group_t group,
                  dispatch_queue_t q, void *context, dispatch_function_t work) {
-  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->is_barrier_block = true;
-  Release(thr, pc, (uptr)new_context);
-  REAL(dispatch_group_notify_f)(group, q, new_context,
-                                dispatch_callback_wrap);
+  WRAP(dispatch_group_notify)(group, q, ^(void) { work(context); });
 }
 
 TSAN_INTERCEPTOR(void, dispatch_source_set_event_handler,
                  dispatch_source_t source, dispatch_block_t handler) {
   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) {

compiler-rt/trunk/test/tsan/Darwin/gcd-groups-leave.mm

+// 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>
+
+#import "../test.h"
+
+dispatch_semaphore_t sem;
+
+long global;
+long global2;
+
+void callback(void *context) {
+  global2 = 48;
+  barrier_wait(&barrier);
+
+  dispatch_semaphore_signal(sem);
+}
+
+int main() {
+  fprintf(stderr, "Hello world.\n");
+  barrier_init(&barrier, 2);
+
+  dispatch_queue_t q = dispatch_queue_create("my.queue", DISPATCH_QUEUE_CONCURRENT);
+  dispatch_group_t g = dispatch_group_create();
+  sem = dispatch_semaphore_create(0);
+
+  dispatch_group_enter(g);
+  dispatch_async(q, ^{
+    global = 47;
+    dispatch_group_leave(g);
+    barrier_wait(&barrier);
+  });
+  dispatch_group_notify(g, q, ^{
+    global = 48;
+    barrier_wait(&barrier);
+
+    dispatch_semaphore_signal(sem);
+  });
+  dispatch_semaphore_wait(sem, DISPATCH_TIME_FOREVER);
+
+  dispatch_group_enter(g);
+  dispatch_async(q, ^{
+    global2 = 47;
+    dispatch_group_leave(g);
+    barrier_wait(&barrier);
+  });
+  dispatch_group_notify_f(g, q, NULL, &callback);
+  dispatch_semaphore_wait(sem, DISPATCH_TIME_FOREVER);
+
+  fprintf(stderr, "Done.\n");
+}
+
+// CHECK: Hello world.
+// CHECK-NOT: WARNING: ThreadSanitizer
+// CHECK: Done.