Index: lib/tsan/rtl/tsan_libdispatch_mac.cc =================================================================== --- lib/tsan/rtl/tsan_libdispatch_mac.cc +++ lib/tsan/rtl/tsan_libdispatch_mac.cc @@ -37,6 +37,7 @@ dispatch_object_t object_to_release; bool free_context_in_callback; bool release_sync_object_in_callback; + bool is_barrier_block; } tsan_block_context_t; // The offsets of different fields of the dispatch_queue_t structure, exported @@ -81,6 +82,7 @@ new_context->object_to_release = nullptr; new_context->free_context_in_callback = true; new_context->release_sync_object_in_callback = false; + new_context->is_barrier_block = false; return new_context; } @@ -88,7 +90,18 @@ SCOPED_INTERCEPTOR_RAW(dispatch_callback_wrap); tsan_block_context_t *context = (tsan_block_context_t *)param; + // We need two more objects to synchronize on: One for the HB edge to a + // barrier block from all previous blocks, and one for the HB edge from the + // barrier block to all following blocks. Let's use queue+0x8 and queue+0x10 + // for this purpose. The internals of queue objects are never touched by + // instrumented code. + uptr barrier_sync_object1 = ((uptr)context->queue) + sizeof(uptr); + uptr barrier_sync_object2 = ((uptr)context->queue) + 2 * sizeof(uptr); + + bool queue_serial = IsQueueSerial(context->queue); + Acquire(thr, pc, context->sync_object); + if (!queue_serial) Acquire(thr, pc, barrier_sync_object2); // Extra retain/release is required for dispatch groups. We use the group // itself to synchronize, but in a notification (dispatch_group_notify @@ -97,12 +110,21 @@ 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); + // to explicitly synchronize on the queue itself. In concurrent queues, a + // barrier block is synchronized with all other blocks. + if (queue_serial) Acquire(thr, pc, (uptr)context->queue); + if (!queue_serial && context->is_barrier_block) + Acquire(thr, pc, barrier_sync_object1); + SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_START(); context->orig_work(context->orig_context); SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_END(); - if (IsQueueSerial(context->queue)) Release(thr, pc, (uptr)context->queue); + + if (queue_serial) Release(thr, pc, (uptr)context->queue); + if (!queue_serial && context->is_barrier_block) + Release(thr, pc, barrier_sync_object2); + + Release(thr, pc, barrier_sync_object1); if (context->release_sync_object_in_callback) Release(thr, pc, context->sync_object); @@ -116,7 +138,7 @@ Block_release(block); } -#define DISPATCH_INTERCEPT_B(name) \ +#define DISPATCH_INTERCEPT_B(name, barrier) \ TSAN_INTERCEPTOR(void, name, dispatch_queue_t q, dispatch_block_t block) { \ SCOPED_TSAN_INTERCEPTOR(name, q, block); \ SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_START(); \ @@ -124,20 +146,21 @@ 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 = barrier; \ Release(thr, pc, (uptr)new_context); \ SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_START(); \ REAL(name##_f)(q, new_context, dispatch_callback_wrap); \ SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_END(); \ } -#define DISPATCH_INTERCEPT_SYNC_B(name) \ +#define DISPATCH_INTERCEPT_SYNC_B(name, barrier) \ TSAN_INTERCEPTOR(void, name, dispatch_queue_t q, dispatch_block_t block) { \ SCOPED_TSAN_INTERCEPTOR(name, q, block); \ SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_START(); \ dispatch_block_t heap_block = Block_copy(block); \ SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_END(); \ - tsan_block_context_t new_context = { \ - q, heap_block, &invoke_and_release_block, 0, 0, false, true}; \ + tsan_block_context_t new_context = {q, heap_block, \ + &invoke_and_release_block, 0, 0, false, true, barrier}; \ new_context.sync_object = (uptr)&new_context; \ Release(thr, pc, (uptr)&new_context); \ SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_START(); \ @@ -146,23 +169,25 @@ Acquire(thr, pc, (uptr)&new_context); \ } -#define DISPATCH_INTERCEPT_F(name) \ +#define DISPATCH_INTERCEPT_F(name, barrier) \ TSAN_INTERCEPTOR(void, name, dispatch_queue_t q, void *context, \ dispatch_function_t work) { \ SCOPED_TSAN_INTERCEPTOR(name, q, context, work); \ tsan_block_context_t *new_context = \ AllocContext(thr, pc, q, context, work); \ + new_context->is_barrier_block = barrier; \ Release(thr, pc, (uptr)new_context); \ SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_START(); \ REAL(name)(q, new_context, dispatch_callback_wrap); \ SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_END(); \ } -#define DISPATCH_INTERCEPT_SYNC_F(name) \ +#define DISPATCH_INTERCEPT_SYNC_F(name, barrier) \ TSAN_INTERCEPTOR(void, name, dispatch_queue_t q, void *context, \ dispatch_function_t work) { \ SCOPED_TSAN_INTERCEPTOR(name, q, context, work); \ - tsan_block_context_t new_context = {q, context, work, 0, 0, false, true}; \ + tsan_block_context_t new_context = {q, context, work, 0, 0, false, true, \ + barrier}; \ new_context.sync_object = (uptr)&new_context; \ Release(thr, pc, (uptr)&new_context); \ SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_START(); \ @@ -175,14 +200,14 @@ // context, which is used to synchronize (we release the context before // submitting, and the callback acquires it before executing the original // callback). -DISPATCH_INTERCEPT_B(dispatch_async) -DISPATCH_INTERCEPT_B(dispatch_barrier_async) -DISPATCH_INTERCEPT_F(dispatch_async_f) -DISPATCH_INTERCEPT_F(dispatch_barrier_async_f) -DISPATCH_INTERCEPT_SYNC_B(dispatch_sync) -DISPATCH_INTERCEPT_SYNC_B(dispatch_barrier_sync) -DISPATCH_INTERCEPT_SYNC_F(dispatch_sync_f) -DISPATCH_INTERCEPT_SYNC_F(dispatch_barrier_sync_f) +DISPATCH_INTERCEPT_B(dispatch_async, false) +DISPATCH_INTERCEPT_B(dispatch_barrier_async, true) +DISPATCH_INTERCEPT_F(dispatch_async_f, false) +DISPATCH_INTERCEPT_F(dispatch_barrier_async_f, true) +DISPATCH_INTERCEPT_SYNC_B(dispatch_sync, false) +DISPATCH_INTERCEPT_SYNC_B(dispatch_barrier_sync, true) +DISPATCH_INTERCEPT_SYNC_F(dispatch_sync_f, false) +DISPATCH_INTERCEPT_SYNC_F(dispatch_barrier_sync_f, true) TSAN_INTERCEPTOR(void, dispatch_after, dispatch_time_t when, dispatch_queue_t queue, dispatch_block_t block) { Index: test/tsan/Darwin/gcd-barrier-race.mm =================================================================== --- test/tsan/Darwin/gcd-barrier-race.mm +++ test/tsan/Darwin/gcd-barrier-race.mm @@ -0,0 +1,48 @@ +// RUN: %clang_tsan %s -o %t -framework Foundation +// RUN: %env_tsan_opts=ignore_interceptors_accesses=1 %deflake %run %t 2>&1 | FileCheck %s + +#import + +#import "../test.h" + +long global; + +int main() { + fprintf(stderr, "Hello world.\n"); + print_address("addr=", 1, &global); + barrier_init(&barrier, 2); + + dispatch_queue_t q = dispatch_queue_create("my.queue", DISPATCH_QUEUE_CONCURRENT); + dispatch_queue_t bgq = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0); + + dispatch_barrier_sync(q, ^{ + global = 42; + }); + + dispatch_async(bgq, ^{ + dispatch_sync(q, ^{ + global = 43; + barrier_wait(&barrier); + }); + }); + + dispatch_async(bgq, ^{ + dispatch_sync(q, ^{ + barrier_wait(&barrier); + global = 44; + + dispatch_sync(dispatch_get_main_queue(), ^{ + CFRunLoopStop(CFRunLoopGetCurrent()); + }); + }); + }); + + CFRunLoopRun(); + fprintf(stderr, "Done.\n"); +} + +// CHECK: Hello world. +// CHECK: addr=[[ADDR:0x[0-9,a-f]+]] +// CHECK: WARNING: ThreadSanitizer: data race +// CHECK: Location is global 'global' {{(of size 8 )?}}at [[ADDR]] (gcd-barrier-race.mm.tmp+0x{{[0-9,a-f]+}}) +// CHECK: Done. Index: test/tsan/Darwin/gcd-barrier.mm =================================================================== --- test/tsan/Darwin/gcd-barrier.mm +++ test/tsan/Darwin/gcd-barrier.mm @@ -0,0 +1,49 @@ +// RUN: %clang_tsan %s -o %t -framework Foundation +// RUN: %env_tsan_opts=ignore_interceptors_accesses=1 %run %t 2>&1 | FileCheck %s + +#import + +#import "../test.h" + +long global; + +int main() { + fprintf(stderr, "Hello world.\n"); + print_address("addr=", 1, &global); + barrier_init(&barrier, 2); + + dispatch_queue_t q = dispatch_queue_create("my.queue", DISPATCH_QUEUE_CONCURRENT); + dispatch_queue_t bgq = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0); + + dispatch_async(bgq, ^{ + dispatch_sync(q, ^{ + global = 42; + }); + barrier_wait(&barrier); + }); + + dispatch_async(bgq, ^{ + barrier_wait(&barrier); + dispatch_barrier_sync(q, ^{ + global = 43; + }); + + dispatch_async(bgq, ^{ + barrier_wait(&barrier); + global = 44; + }); + + barrier_wait(&barrier); + + dispatch_sync(dispatch_get_main_queue(), ^{ + CFRunLoopStop(CFRunLoopGetCurrent()); + }); + }); + + CFRunLoopRun(); + fprintf(stderr, "Done.\n"); +} + +// CHECK: Hello world. +// CHECK: Done. +// CHECK-NOT: WARNING: ThreadSanitizer