From 33c15c91a674045a47eadb559d41c77d5997af66 Mon Sep 17 00:00:00 2001 From: Kuba Brecka Date: Thu, 7 Apr 2016 11:33:44 +0000 Subject: [PATCH] [tsan] Fix synchronization in dispatch_sync In the interceptor for dispatch_sync, we're currently missing synchronization between the callback and the code *after* the call to dispatch_sync. This patch fixes this by adding an extra release+acquire pair to dispatch_sync() and similar APIs. Added a testcase. Differential Revision: http://reviews.llvm.org/D18502 llvm-svn: 265659 --- compiler-rt/lib/tsan/rtl/tsan_libdispatch_mac.cc | 86 +++++++++++++++++------- compiler-rt/test/tsan/Darwin/gcd-blocks.mm | 34 ++++++++++ 2 files changed, 96 insertions(+), 24 deletions(-) create mode 100644 compiler-rt/test/tsan/Darwin/gcd-blocks.mm diff --git a/compiler-rt/lib/tsan/rtl/tsan_libdispatch_mac.cc b/compiler-rt/lib/tsan/rtl/tsan_libdispatch_mac.cc index 617dc91..2b4dc34 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_libdispatch_mac.cc +++ b/compiler-rt/lib/tsan/rtl/tsan_libdispatch_mac.cc @@ -33,8 +33,10 @@ typedef struct { dispatch_queue_t queue; void *orig_context; dispatch_function_t orig_work; - uptr object_to_acquire; + uptr sync_object; dispatch_object_t object_to_release; + bool free_context_in_callback; + bool release_sync_object_in_callback; } tsan_block_context_t; // The offsets of different fields of the dispatch_queue_t structure, exported @@ -75,15 +77,18 @@ static tsan_block_context_t *AllocContext(ThreadState *thr, uptr pc, new_context->queue = queue; new_context->orig_context = orig_context; new_context->orig_work = orig_work; - new_context->object_to_acquire = (uptr)new_context; + new_context->sync_object = (uptr)new_context; new_context->object_to_release = nullptr; + new_context->free_context_in_callback = true; + new_context->release_sync_object_in_callback = false; return new_context; } -static void dispatch_callback_wrap_acquire(void *param) { - SCOPED_INTERCEPTOR_RAW(dispatch_async_f_callback_wrap); +static void dispatch_callback_wrap(void *param) { + SCOPED_INTERCEPTOR_RAW(dispatch_callback_wrap); tsan_block_context_t *context = (tsan_block_context_t *)param; - Acquire(thr, pc, context->object_to_acquire); + + Acquire(thr, pc, context->sync_object); // Extra retain/release is required for dispatch groups. We use the group // itself to synchronize, but in a notification (dispatch_group_notify @@ -98,7 +103,11 @@ static void dispatch_callback_wrap_acquire(void *param) { context->orig_work(context->orig_context); SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_END(); if (IsQueueSerial(context->queue)) Release(thr, pc, (uptr)context->queue); - user_free(thr, pc, context); + + if (context->release_sync_object_in_callback) + Release(thr, pc, context->sync_object); + + if (context->free_context_in_callback) user_free(thr, pc, context); } static void invoke_and_release_block(void *param) { @@ -110,15 +119,31 @@ static void invoke_and_release_block(void *param) { #define DISPATCH_INTERCEPT_B(name) \ TSAN_INTERCEPTOR(void, name, dispatch_queue_t q, dispatch_block_t block) { \ SCOPED_TSAN_INTERCEPTOR(name, q, block); \ - SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_START(); \ + SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_START(); \ dispatch_block_t heap_block = Block_copy(block); \ - SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_END(); \ + SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_END(); \ tsan_block_context_t *new_context = \ AllocContext(thr, pc, q, heap_block, &invoke_and_release_block); \ Release(thr, pc, (uptr)new_context); \ - SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_START(); \ - REAL(name##_f)(q, new_context, dispatch_callback_wrap_acquire); \ - SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_END(); \ + 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) \ + 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}; \ + new_context.sync_object = (uptr)&new_context; \ + 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(); \ + Acquire(thr, pc, (uptr)&new_context); \ } #define DISPATCH_INTERCEPT_F(name) \ @@ -128,9 +153,22 @@ static void invoke_and_release_block(void *param) { tsan_block_context_t *new_context = \ AllocContext(thr, pc, q, context, work); \ Release(thr, pc, (uptr)new_context); \ - SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_START(); \ - REAL(name)(q, new_context, dispatch_callback_wrap_acquire); \ - SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_END(); \ + 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) \ + 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}; \ + new_context.sync_object = (uptr)&new_context; \ + 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(); \ + Acquire(thr, pc, (uptr)&new_context); \ } // We wrap dispatch_async, dispatch_sync and friends where we allocate a new @@ -141,10 +179,10 @@ 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_B(dispatch_sync) -DISPATCH_INTERCEPT_B(dispatch_barrier_sync) -DISPATCH_INTERCEPT_F(dispatch_sync_f) -DISPATCH_INTERCEPT_F(dispatch_barrier_sync_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) // GCD's dispatch_once implementation has a fast path that contains a racy read // and it's inlined into user's code. Furthermore, this fast path doesn't @@ -253,30 +291,30 @@ TSAN_INTERCEPTOR(void, dispatch_group_notify, dispatch_group_t group, SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_END(); tsan_block_context_t *new_context = AllocContext(thr, pc, q, heap_block, &invoke_and_release_block); - new_context->object_to_acquire = (uptr)group; + new_context->sync_object = (uptr)group; - // Will be released in dispatch_callback_wrap_acquire. + // Will be released in dispatch_callback_wrap. 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); + 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->object_to_acquire = (uptr)group; + new_context->sync_object = (uptr)group; - // Will be released in dispatch_callback_wrap_acquire. + // Will be released in dispatch_callback_wrap. 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); + dispatch_callback_wrap); } } // namespace __tsan diff --git a/compiler-rt/test/tsan/Darwin/gcd-blocks.mm b/compiler-rt/test/tsan/Darwin/gcd-blocks.mm new file mode 100644 index 0000000..0dbff27 --- /dev/null +++ b/compiler-rt/test/tsan/Darwin/gcd-blocks.mm @@ -0,0 +1,34 @@ +// RUN: %clang_tsan %s -o %t -framework Foundation +// RUN: %env_tsan_opts=ignore_interceptors_accesses=1 %run %t 2>&1 | FileCheck %s + +#import + +int main() { + fprintf(stderr, "start\n"); + + dispatch_queue_t background_q = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0); + dispatch_queue_t main_q = dispatch_get_main_queue(); + + dispatch_async(background_q, ^{ + __block long block_var = 0; + + dispatch_sync(main_q, ^{ + block_var = 42; + }); + + fprintf(stderr, "block_var = %ld\n", block_var); + + dispatch_sync(dispatch_get_main_queue(), ^{ + CFRunLoopStop(CFRunLoopGetCurrent()); + }); + }); + + CFRunLoopRun(); + fprintf(stderr, "done\n"); +} + +// CHECK: start +// CHECK: block_var = 42 +// CHECK: done +// CHECK-NOT: WARNING: ThreadSanitizer +// CHECK-NOT: CHECK failed -- 2.7.4