[tsan] Fix scoping of ScopedInteceptor in libdispatch support

[kubamracek]

Summary

Some interceptors in tsan_libdispatch_mac.cc currently wrongly use TSAN_SCOPED_INTERCEPTOR/ScopedInterceptor. Its constructor can start ignoring memory accesses, and the destructor the stops this -- however, e.g. dispatch_sync can call user's code, so the ignoring will extend to user's code as well. This is not expected and we should only limit the scope of ScopedInterceptor to TSan code.

[dvyukov]

It does not look good to me because we lose all the common functionality that we have in ScopedInterceptor ctor/dtor; without some kind of local object we also lose ability to execute any code on interceptor exit (as the result we lose ability to do any kind of "scoped" functionality).
For example, absence of ProcessPendingSignals pretty much means user reports about deadlocks (undelivered signals).
For example, you check in_ignored_lib, but it is never set (normally the flag is set by ScopedInterceptor, which is not executed in this case). So I wonder what is purpose of this check.

Can e.g. dispatch_async execute user code at all?

I think we need something like ScopedUserCallback class that carefully setups thread state for execution of user code, and then dtor undoes that.
There seem to be several other places that we need it: pthread_once seems to call user callback inside of scoped interceptor, dl_iterate_phdr -- the same, pthread_cleanup callbacks called on cleanup inside of a pthread function.

[kubamracek]

Updating patch to only annotate callbacks into user's code.

[dvyukov]

lib/tsan/rtl/tsan_interceptors.cc

299 ↗

(On Diff #42868)

in the other patch you added thr->in_interceptor_count
you will need to decrement it here either in this patch or in that patch depending on order of submission

compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.h

 #ifndef TSAN_INTERCEPTORS_H
 #define TSAN_INTERCEPTORS_H
 
 #include "sanitizer_common/sanitizer_stacktrace.h"
 #include "tsan_rtl.h"
 
 namespace __tsan {
 
 class ScopedInterceptor {
  public:
   ScopedInterceptor(ThreadState *thr, const char *fname, uptr pc);
   ~ScopedInterceptor();
+  void UserCallbackStart();
+  void UserCallbackEnd();
  private:
   ThreadState *const thr_;
   const uptr pc_;
   bool in_ignored_lib_;
 };
 
 }  // namespace __tsan
 
#define SCOPED_TSAN_INTERCEPTOR(func, ...) \
     if (REAL(func) == 0) { \
       Report("FATAL: ThreadSanitizer: failed to intercept %s\n", #func); \
       Die(); \
     }                                                    \
     if (thr->ignore_interceptors || thr->in_ignored_lib) \
       return REAL(func)(__VA_ARGS__); \
 /**/
 
+#define SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_START() \
+    si.UserCallbackStart();
+
+#define SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_END() \
+    si.UserCallbackEnd();
+
 #define TSAN_INTERCEPTOR(ret, func, ...) INTERCEPTOR(ret, func, __VA_ARGS__)
 
 #if SANITIZER_FREEBSD
 #define __libc_free __free
 #define __libc_malloc __malloc
 #endif
 
 extern "C" void __libc_free(void *ptr);
 extern "C" void *__libc_malloc(uptr size);
 
 #endif  // TSAN_INTERCEPTORS_H

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

ScopedInterceptor::~ScopedInterceptor() {
   }
   if (!thr_->ignore_interceptors) {
     ProcessPendingSignals(thr_);
     FuncExit(thr_);
     CheckNoLocks(thr_);
   }
 }
 
+void ScopedInterceptor::UserCallbackStart() {
+  if (in_ignored_lib_) {
+    thr_->in_ignored_lib = false;
+    ThreadIgnoreEnd(thr_, pc_);
+  }
+}
+
+void ScopedInterceptor::UserCallbackEnd() {
+  if (in_ignored_lib_) {
+    thr_->in_ignored_lib = true;
+    ThreadIgnoreBegin(thr_, pc_);
+  }
+}
+
 #define TSAN_INTERCEPT(func) INTERCEPT_FUNCTION(func)
 #if SANITIZER_FREEBSD
 # define TSAN_INTERCEPT_VER(func, ver) INTERCEPT_FUNCTION(func)
 #else
 # define TSAN_INTERCEPT_VER(func, ver) INTERCEPT_FUNCTION_VER(func, ver)
 #endif
 
 #define READ_STRING_OF_LEN(thr, pc, s, len, n)                 \

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

static void dispatch_callback_wrap_acquire(void *param) {
   // itself to synchronize, but in a notification (dispatch_group_notify
   // callback), it may be disposed already. To solve this, we retain the group
   // and release it here.
   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);
+  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);
   user_free(thr, pc, context);
 }
 
 static void invoke_and_release_block(void *param) {
   dispatch_block_t block = (dispatch_block_t)param;
   block();
   Block_release(block);
 }
 
 #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(); \
     dispatch_block_t heap_block = Block_copy(block);                         \
+    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(); \
   }
 
 #define DISPATCH_INTERCEPT_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 =                           \
         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(); \
   }
 
 // We wrap dispatch_async, dispatch_sync and friends where we allocate a new
 // 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)

 #undef dispatch_once
 TSAN_INTERCEPTOR(void, dispatch_once, dispatch_once_t *predicate,
                  dispatch_block_t block) {
   SCOPED_TSAN_INTERCEPTOR(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();
     Release(thr, pc, (uptr)a);
     atomic_store(a, 2, memory_order_release);
   } else {
     while (v != 2) {
       internal_sched_yield();
       v = atomic_load(a, memory_order_acquire);
     }
     Acquire(thr, pc, (uptr)a);
   }
 }
 
 #undef dispatch_once_f
 TSAN_INTERCEPTOR(void, dispatch_once_f, dispatch_once_t *predicate,
                  void *context, dispatch_function_t function) {
   SCOPED_TSAN_INTERCEPTOR(dispatch_once_f, predicate, context, function);
+  SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_START();
   WRAP(dispatch_once)(predicate, ^(void) {
     function(context);
   });
+  SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_END();
 }
 
 TSAN_INTERCEPTOR(long_t, dispatch_semaphore_signal,
                  dispatch_semaphore_t dsema) {
   SCOPED_TSAN_INTERCEPTOR(dispatch_semaphore_signal, dsema);
   Release(thr, pc, (uptr)dsema);
   return REAL(dispatch_semaphore_signal)(dsema);
 }
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);
+  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 =
       AllocContext(thr, pc, q, heap_block, &invoke_and_release_block);
   new_context->object_to_acquire = (uptr)group;
 
   // Will be released in dispatch_callback_wrap_acquire.
   new_context->object_to_release = group;
   dispatch_retain(group);