Annotate function parameters with attribute 'noescape'

[ahatanak]

Summary

I recently tried to commit a patch that adds support for 'noescape' in clang (see https://reviews.llvm.org/D32210) but reverted it because clang failed to build compiler-rt.

The build fails because the block parameters of the following functions defined in the SDKs are preceded with DISPATCH_NOESCAPE, but the declarations in tsan_libdispatch_mac.cc do not have the attribute:

• dispatch_sync
• dispatch_barrier_sync
• dispatch_once
• dispatch_apply

This patch annotates the declarations in tsan_libdispatch_mac.cc with 'noescape'.

[kubamracek]

What happens when building this code with a compiler that doesn't understand __attribute__((__noescape__))? Is it just ignored?

[ahatanak]

In D38141#877782, @kubamracek wrote:

What happens when building this code with a compiler that doesn't understand __attribute__((__noescape__))? Is it just ignored?

clang will just ignore it and issue a warning ("unknown attribute 'noescape' ignored").

It's possible to guard the declarations with 'noescape' using "__has_attribute(noescape)".

https://clang.llvm.org/docs/LanguageExtensions.html

[kubamracek]

That's what we should do here. Since the file includes dispatch.h, we should be able to just use the DISPATCH_NOESCAPE macro, which already uses #if __has_attribute.

[kubamracek]

So is this going to generally break sources which do the same as tsan_libdispatch_mac.cc?

[ahatanak]

In D38141#877824, @kubamracek wrote:

That's what we should do here. Since the file includes dispatch.h, we should be able to just use the DISPATCH_NOESCAPE macro, which already uses #if __has_attribute.

OK, I'll use DISPATCH_NOESCAPE then.

In D38141#877876, @kubamracek wrote:

So is this going to generally break sources which do the same as tsan_libdispatch_mac.cc?

Yes, clang will reject it with message "error: conflicting types for ...".

[ahatanak]

Use DISPATCH_NOESCAPE instead of "attribute((noescape))".

[kubamracek]

LGTM with a nit.

lib/tsan/rtl/tsan_libdispatch_mac.cc

181

Please re-align the backslash at the end of the line.

[ahatanak]

Thanks, committed in r313929.

lib/tsan/rtl/tsan_libdispatch_mac.cc

TSAN_INTERCEPTOR(void, name, dispatch_queue_t q, dispatch_block_t 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, barrier)                             \
-  TSAN_INTERCEPTOR(void, name, dispatch_queue_t q, dispatch_block_t block) { \
+  TSAN_INTERCEPTOR(void, name, dispatch_queue_t q,                           \
+                   DISPATCH_NOESCAPE 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, false, true, barrier, 0};  \
     Release(thr, pc, (uptr)&new_context);                                    \
     SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_START();                           \

 // predicate), so the interceptor is always called, and let's add proper release
 // and acquire semantics. Since TSan does not see its own atomic stores, the
 // race on predicate won't be reported - the only accesses to it that TSan sees
 // are the loads on the fast path. Loads don't race. Secondly, dispatch_once is
 // both a macro and a real function, we want to intercept the function, so we
 // need to undefine the macro.
 #undef dispatch_once
 TSAN_INTERCEPTOR(void, dispatch_once, dispatch_once_t *predicate,
-                 dispatch_block_t block) {
+                 DISPATCH_NOESCAPE dispatch_block_t block) {
   SCOPED_INTERCEPTOR_RAW(dispatch_once, predicate, block);
   atomic_uint32_t *a = reinterpret_cast<atomic_uint32_t *>(predicate);
   u32 v = atomic_load(a, memory_order_acquire);
   if (v == 0 &&
       atomic_compare_exchange_strong(a, &v, 1, memory_order_relaxed)) {
     SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_START();
     block();
     SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_END();
if (handler == nullptr)
     return REAL(dispatch_source_set_registration_handler)(source, nullptr);
   dispatch_block_t block = ^(void) {
     handler(dispatch_get_context(source));
   };
   WRAP(dispatch_source_set_registration_handler)(source, block);
 }
 
 TSAN_INTERCEPTOR(void, dispatch_apply, size_t iterations,
-                 dispatch_queue_t queue, void (^block)(size_t)) {
+                 dispatch_queue_t queue,
+                 DISPATCH_NOESCAPE void (^block)(size_t)) {
   SCOPED_TSAN_INTERCEPTOR(dispatch_apply, iterations, queue, block);
 
   void *parent_to_child_sync = nullptr;
   uptr parent_to_child_sync_uptr = (uptr)&parent_to_child_sync;
   void *child_to_parent_sync = nullptr;
   uptr child_to_parent_sync_uptr = (uptr)&child_to_parent_sync;
 
   Release(thr, pc, parent_to_child_sync_uptr);