[ASAN] Use ThreadArgRetval in ASAN

[vitalybuka]

Summary

Fixes false leaks on thread retval.

[vitalybuka]

fix variable name

[vitalybuka]

comments

[vitalybuka]

comments

[vitalybuka]

rebase

[thurston]

compiler-rt/lib/asan/asan_thread.cpp

497–501

'LockThreadRegistry' is now a misleading name because it locks both ThreadRegistry and ThreadArgRetval.

Ditto for 'UnlockThreadRegistry'.

[vitalybuka]

update

[vitalybuka]

compiler-rt/lib/asan/asan_thread.cpp

497–501

I will rename in follow up patch

[hans]

This broke some asan tests on mac, see https://bugs.chromium.org/p/chromium/issues/detail?id=1445676

compiler-rt/lib/asan/asan_interceptors.cpp

#define COMMON_SYSCALL_POST_WRITE_RANGE(p, s) \
   } while (false)
 #include "sanitizer_common/sanitizer_common_syscalls.inc"
 #include "sanitizer_common/sanitizer_syscalls_netbsd.inc"
 
 #if ASAN_INTERCEPT_PTHREAD_CREATE
 static thread_return_t THREAD_CALLING_CONV asan_thread_start(void *arg) {
   AsanThread *t = (AsanThread *)arg;
   SetCurrentThread(t);
-  return t->ThreadStart(GetTid());
+  auto self = GetThreadSelf();
+  auto args = asanThreadArgRetval().GetArgs(self);
+  thread_return_t retval = t->ThreadStart(GetTid());
+  asanThreadArgRetval().Finish(self, retval);
+  CHECK_EQ(args.arg_retval, t->get_arg());
+  return retval;
 }
 
-INTERCEPTOR(int, pthread_create, void *thread,
-    void *attr, void *(*start_routine)(void*), void *arg) {
+INTERCEPTOR(int, pthread_create, void *thread, void *attr,
+            void *(*start_routine)(void *), void *arg) {
   EnsureMainThreadIDIsCorrect();
   // Strict init-order checking is thread-hostile.
   if (flags()->strict_init_order)
     StopInitOrderChecking();
   GET_STACK_TRACE_THREAD;
-  int detached = 0;
-  if (attr)
-    REAL(pthread_attr_getdetachstate)(attr, &detached);
+  bool detached = [attr]() {
+    int d = 0;
+    return attr && !REAL(pthread_attr_getdetachstate)(attr, &d) &&
+           IsStateDetached(d);
+  }();
 
   u32 current_tid = GetCurrentTidOrInvalid();
   AsanThread *t =
       AsanThread::Create(start_routine, arg, current_tid, &stack, detached);
 
   int result;
   {
     // Ignore all allocations made by pthread_create: thread stack/TLS may be
     // stored by pthread for future reuse even after thread destruction, and
     // the linked list it's stored in doesn't even hold valid pointers to the
     // objects, the latter are calculated by obscure pointer arithmetic.
-#if CAN_SANITIZE_LEAKS
+#    if CAN_SANITIZE_LEAKS
     __lsan::ScopedInterceptorDisabler disabler;
-#endif
+#    endif
+    asanThreadArgRetval().Create(detached, {start_routine, arg}, [&]() -> uptr {
       result = REAL(pthread_create)(thread, attr, asan_thread_start, t);
+      return result ? 0 : *(uptr *)(thread);
+    });
   }
   if (result != 0) {
     // If the thread didn't start delete the AsanThread to avoid leaking it.
     // Note AsanThreadContexts never get destroyed so the AsanThreadContext
     // that was just created for the AsanThread is wasted.
     t->Destroy();
   }
   return result;
 }
 
 INTERCEPTOR(int, pthread_join, void *thread, void **retval) {
-  return REAL(pthread_join)(thread, retval);
+  int result;
+  asanThreadArgRetval().Join((uptr)thread, [&]() {
+    result = REAL(pthread_join)(thread, retval);
+    return !result;
+  });
+  return result;
 }
 
 INTERCEPTOR(int, pthread_detach, void *thread) {
-  return REAL(pthread_detach)(thread);
+  int result;
+  asanThreadArgRetval().Detach((uptr)thread, [&]() {
+    result = REAL(pthread_detach)(thread);
+    return !result;
+  });
+  return result;
 }
 
 INTERCEPTOR(int, pthread_exit, void *retval) {
+  asanThreadArgRetval().Finish(GetThreadSelf(), retval);
   return REAL(pthread_exit)(retval);
 }
 
 #    if ASAN_INTERCEPT_TRYJOIN
 INTERCEPTOR(int, pthread_tryjoin_np, void *thread, void **ret) {
-  return REAL(pthread_tryjoin_np)(thread, ret);
+  int result;
+  asanThreadArgRetval().Join((uptr)thread, [&]() {
+    result = REAL(pthread_tryjoin_np)(thread, ret);
+    return !result;
+  });
+  return result;
 }
 #    endif
 
 #    if ASAN_INTERCEPT_TIMEDJOIN
 INTERCEPTOR(int, pthread_timedjoin_np, void *thread, void **ret,
             const struct timespec *abstime) {
-  return REAL(pthread_timedjoin_np)(thread, ret, abstime);
+  int result;
+  asanThreadArgRetval().Join((uptr)thread, [&]() {
+    result = REAL(pthread_timedjoin_np)(thread, ret, abstime);
+    return !result;
+  });
+  return result;
 }
 #    endif
 
 DEFINE_REAL_PTHREAD_FUNCTIONS
 #endif  // ASAN_INTERCEPT_PTHREAD_CREATE
 
 #if ASAN_INTERCEPT_SWAPCONTEXT
 static void ClearShadowMemoryForContextStack(uptr stack, uptr ssize) {

compiler-rt/lib/asan/asan_thread.h

 #define ASAN_THREAD_H
 
 #include "asan_allocator.h"
 #include "asan_fake_stack.h"
 #include "asan_internal.h"
 #include "asan_stats.h"
 #include "sanitizer_common/sanitizer_common.h"
 #include "sanitizer_common/sanitizer_libc.h"
+#include "sanitizer_common/sanitizer_thread_arg_retval.h"
 #include "sanitizer_common/sanitizer_thread_registry.h"
 
 namespace __sanitizer {
 struct DTLS;
 }  // namespace __sanitizer
 
 namespace __asan {
 
private:
   AsanThreadLocalMallocStorage malloc_storage_;
   AsanStats stats_;
   bool unwinding_;
   uptr extra_spill_area_;
 };
 
 // Returns a single instance of registry.
 ThreadRegistry &asanThreadRegistry();
+ThreadArgRetval &asanThreadArgRetval();
 
 // Must be called under ThreadRegistryLock.
 AsanThreadContext *GetThreadContextByTidLocked(u32 tid);
 
 // Get the current thread. May return 0.
 AsanThread *GetCurrentThread();
 void SetCurrentThread(AsanThread *t);
 u32 GetCurrentTidOrInvalid();
 AsanThread *FindThreadByStackAddress(uptr addr);
 
 // Used to handle fork().
 void EnsureMainThreadIDIsCorrect();
 } // namespace __asan
 
 #endif // ASAN_THREAD_H

compiler-rt/lib/asan/asan_thread.cpp

 }
 
 void AsanThreadContext::OnFinished() {
   // Drop the link to the AsanThread object.
   thread = nullptr;
 }
 
 static ThreadRegistry *asan_thread_registry;
+static ThreadArgRetval *thread_data;
+
 static Mutex mu_for_thread_context;
 static LowLevelAllocator allocator_for_thread_context;
 
 static ThreadContextBase *GetAsanThreadContext(u32 tid) {
   Lock lock(&mu_for_thread_context);
   return new (allocator_for_thread_context) AsanThreadContext(tid);
 }
 
 static void InitThreads() {
   static bool initialized;
   // Don't worry about thread_safety - this should be called when there is
   // a single thread.
   if (LIKELY(initialized))
     return;
   // Never reuse ASan threads: we store pointer to AsanThreadContext
   // in TSD and can't reliably tell when no more TSD destructors will
   // be called. It would be wrong to reuse AsanThreadContext for another
   // thread before all TSD destructors will be called for it.
 
   // MIPS requires aligned address
   static ALIGNED(alignof(
       ThreadRegistry)) char thread_registry_placeholder[sizeof(ThreadRegistry)];
+  static ALIGNED(alignof(
+      ThreadArgRetval)) char thread_data_placeholder[sizeof(ThreadArgRetval)];
 
   asan_thread_registry =
       new (thread_registry_placeholder) ThreadRegistry(GetAsanThreadContext);
+  thread_data = new (thread_data_placeholder) ThreadArgRetval();
   initialized = true;
 }
 
 ThreadRegistry &asanThreadRegistry() {
   InitThreads();
   return *asan_thread_registry;
 }
 
+ThreadArgRetval &asanThreadArgRetval() {
+  InitThreads();
+  return *thread_data;
+}
+
 AsanThreadContext *GetThreadContextByTidLocked(u32 tid) {
   return static_cast<AsanThreadContext *>(
       asanThreadRegistry().GetThreadLocked(tid));
 }
 
 // AsanThread implementation.
 
 AsanThread *AsanThread::Create(thread_callback_t start_routine, void *arg,
__asan::AsanThread *GetAsanThreadByOsIDLocked(tid_t os_id) {
   if (!context)
     return nullptr;
   return context->thread;
 }
 }  // namespace __asan
 
 // --- Implementation of LSan-specific functions --- {{{1
 namespace __lsan {
-void LockThreadRegistry() { __asan::asanThreadRegistry().Lock(); }
+void LockThreadRegistry() {
+  __asan::asanThreadRegistry().Lock();
+  __asan::asanThreadArgRetval().Lock();
+}
 
-void UnlockThreadRegistry() { __asan::asanThreadRegistry().Unlock(); }
+void UnlockThreadRegistry() {
+  __asan::asanThreadArgRetval().Unlock();
+  __asan::asanThreadRegistry().Unlock();
+}
 
 static ThreadRegistry *GetAsanThreadRegistryLocked() {
   __asan::asanThreadRegistry().CheckLocked();
   return &__asan::asanThreadRegistry();
 }
 
 void EnsureMainThreadIDIsCorrect() { __asan::EnsureMainThreadIDIsCorrect(); }
 
GetAsanThreadRegistryLocked()->RunCallbackForEachThreadLocked(
       [](ThreadContextBase *tctx, void *arg) {
         GetThreadExtraStackRangesLocked(
             tctx->os_id, reinterpret_cast<InternalMmapVector<Range> *>(arg));
       },
       ranges);
 }
 
 void GetAdditionalThreadContextPtrsLocked(InternalMmapVector<uptr> *ptrs) {
-  GetAsanThreadRegistryLocked()->RunCallbackForEachThreadLocked(
-      [](ThreadContextBase *tctx, void *ptrs) {
-        // Look for the arg pointer of threads that have been created or are
-        // running. This is necessary to prevent false positive leaks due to the
-        // AsanThread holding the only live reference to a heap object.  This
-        // can happen because the `pthread_create()` interceptor doesn't wait
-        // for the child thread to start before returning and thus loosing the
-        // the only live reference to the heap object on the stack.
-
-        __asan::AsanThreadContext *atctx =
-            static_cast<__asan::AsanThreadContext *>(tctx);
-
-        // Note ThreadStatusRunning is required because there is a small window
-        // where the thread status switches to `ThreadStatusRunning` but the
-        // `arg` pointer still isn't on the stack yet.
-        if (atctx->status != ThreadStatusCreated &&
-            atctx->status != ThreadStatusRunning)
-          return;
-
-        uptr thread_arg = reinterpret_cast<uptr>(atctx->thread->get_arg());
-        if (!thread_arg)
-          return;
-
-        auto ptrsVec = reinterpret_cast<InternalMmapVector<uptr> *>(ptrs);
-        ptrsVec->push_back(thread_arg);
-      },
-      ptrs);
+  __asan::asanThreadArgRetval().GetAllPtrsLocked(ptrs);
 }
 
 void GetRunningThreadsLocked(InternalMmapVector<tid_t> *threads) {
   GetAsanThreadRegistryLocked()->RunCallbackForEachThreadLocked(
       [](ThreadContextBase *tctx, void *threads) {
         if (tctx->status == ThreadStatusRunning)
           reinterpret_cast<InternalMmapVector<tid_t> *>(threads)->push_back(
               tctx->os_id);

compiler-rt/lib/sanitizer_common/sanitizer_thread_arg_retval.h

void Detach(uptr thread, const DetachFn& fn) {
     if (fn())
       DetachLocked(thread);
   }
 
   // Joins the thread.
   template <typename JoinFn /* returns true on success */>
   void Join(uptr thread, const JoinFn& fn) {
     // Remember internal id of the thread to prevent re-use of the thread
-    // between fn() and DetachLocked() calls. We can't just lock JoinFn
-    // like in Detach() implementation.
+    // between fn() and AfterJoin() calls. Locking JoinFn, like in
+    // Detach(), implementation can cause deadlock.
     auto gen = BeforeJoin(thread);
     if (fn())
       AfterJoin(thread, gen);
   }
 
   // Returns all arg and retval which are considered alive.
   void GetAllPtrsLocked(InternalMmapVector<uptr>* ptrs);
 

compiler-rt/lib/sanitizer_common/sanitizer_thread_arg_retval.cpp

 }
 
 void ThreadArgRetval::DetachLocked(uptr thread) {
   CheckLocked();
   auto t = data_.find(thread);
   CHECK(t);
   CHECK(!t->second.detached);
   if (t->second.done) {
-    // Detached thread has no use after it started and returned args.
+    // We can't retrive retval after detached thread finished.
     data_.erase(t);
     return;
   }
   t->second.detached = true;
 }
 
 void ThreadArgRetval::GetAllPtrsLocked(InternalMmapVector<uptr>* ptrs) {
   CheckLocked();
   CHECK(ptrs);
   data_.forEach([&](DenseMap<uptr, Data>::value_type& kv) -> bool {
     ptrs->push_back((uptr)kv.second.args.arg_retval);
     return true;
   });
 }
 
 }  // namespace __sanitizer

compiler-rt/test/lsan/TestCases/create_thread_leak.cpp

 // Test handling of arg and retval of child thread.
 
 // RUN: %clangxx_lsan -pthread %s -o %t
 // RUN: %run not %t 10 1 0 0 2>&1 | FileCheck %s --check-prefixes=LEAK,LEAK123
 // RUN: %run not %t 10 0 1 0 2>&1 | FileCheck %s --check-prefixes=LEAK,LEAK234
 // RUN: %run not %t 10 0 0 1 2>&1 | FileCheck %s --check-prefixes=LEAK,LEAK234
 
 // FIXME: Remove "not". There is no leak.
-// False LEAK234 is broken for ASAN, LSAN.
-// RUN: %run %if asan %{ not %} %if lsan-standalone %{ not %} %t 10 0 0 0
+// False LEAK234 is broken for LSAN.
+// RUN: %run %if lsan-standalone %{ not %} %t 10 0 0 0
 
 #include <pthread.h>
 #include <stdlib.h>
 
 #include <sanitizer/lsan_interface.h>
 
 int detach;
 int leak_arg;