[PATCH] D21889: [tsan] Add support for GCD IO channels on Darwin

[Dmitry Vyukov via llvm-commits]

dvyukov accepted this revision.
This revision is now accepted and ready to land.

================
Comment at: lib/tsan/rtl/tsan_libdispatch_mac.cc:512
@@ +511,3 @@
+  SCOPED_TSAN_INTERCEPTOR(dispatch_read, fd, length, q, h);
+  __block tsan_block_context_t new_context = {
+      q, nullptr, &invoke_block, false, false, false, 0};
----------------
kubabrecka wrote:
> dvyukov wrote:
> > Here is something I am missing.
> > Is new_context captured by reference or by value? Internet says that __block makes it captured by reference. But then doesn't new_h refer to a dangling stack location? new_context is stack allocated in this function, but new_h will be executed some time later.
> > And if it is captured by value, then synchronizing on address &new_context is pointless -- closure will have a different address.
> > What am I missing?
> > 
> `__block` turns the variable into a reference-counted object that is captured by reference.  It’s actually allocated as part of the block.  When the block is promoted to a heap object (via Block_copy), the variable is promoted as well.  Note that `&new_context` is different before the Block_copy and after it (before it points to the stack, after it points to the heap).
> 
> Does this make sense?  You can take a look at this via `clang -rewrite-objc file.m -o -` which will rewrite all Obj-C and all block stuff into pure C/C++ code.  But the full mechanism is pretty complex and the generated code is hard to read.
Okay

================
Comment at: lib/tsan/rtl/tsan_libdispatch_mac.cc:514
@@ +513,3 @@
+      q, nullptr, &invoke_block, false, false, false, 0};
+  fd_handler_t new_h = Block_copy(^(dispatch_data_t data, int error) {
+    new_context.orig_context = ^(void) {
----------------
I wonder if it is possible to make dispatch_callback_wrap forward any number of arguments with some vaargs template magic. Then we could write just:
REAL(dispatch_read)(fd, length, q, dispatch_callback_wrap<dispatch_data_t, int>);
Up to you. No need to do in this change.


http://reviews.llvm.org/D21889