[ASan, LSan] Improve tracking of thread creation.

In the current scheme of things, the call to ThreadStart() in the child
thread is not synchronized with the parent thread. So, if a pointer is passed to
pthread_create, there may be a window of time during which this pointer will not
be discoverable by LSan. I.e. the pthread_create interceptor has already
returneed and thus the pointer is no longer on the parent stack, but we don't
yet know the location of the child stack. This has caused bogus leak reports
(see http://llvm.org/bugs/show_bug.cgi?id=21621/).

This patch makes the pthread_create interceptor wait until the child thread is
properly registered before returning.

llvm-svn: 223419

compiler-rt/lib/asan/asan_interceptors.cc

@@ -165,29 +165,53 @@ DECLARE_REAL_AND_INTERCEPTOR(void, free, void *)
   } while (false)
 #include "sanitizer_common/sanitizer_common_syscalls.inc"
 
+struct ThreadStartParam {
+  atomic_uintptr_t t;
+  atomic_uintptr_t is_registered;
+};
+
 static thread_return_t THREAD_CALLING_CONV asan_thread_start(void *arg) {
-  AsanThread *t = (AsanThread*)arg;
+  ThreadStartParam *param = reinterpret_cast<ThreadStartParam *>(arg);
+  AsanThread *t = nullptr;
+  while ((t = reinterpret_cast<AsanThread *>(
+              atomic_load(&param->t, memory_order_acquire))) == 0)
+    internal_sched_yield();
   SetCurrentThread(t);
-  return t->ThreadStart(GetTid());
+  return t->ThreadStart(GetTid(), &param->is_registered);
 }
 
 #if ASAN_INTERCEPT_PTHREAD_CREATE
 INTERCEPTOR(int, pthread_create, void *thread,
     void *attr, void *(*start_routine)(void*), void *arg) {
   EnsureMainThreadIDIsCorrect();
-  // Strict init-order checking in thread-hostile.
+  // Strict init-order checking is thread-hostile.
   if (flags()->strict_init_order)
     StopInitOrderChecking();
   GET_STACK_TRACE_THREAD;
   int detached = 0;
   if (attr != 0)
     REAL(pthread_attr_getdetachstate)(attr, &detached);
-
-  u32 current_tid = GetCurrentTidOrInvalid();
-  AsanThread *t = AsanThread::Create(start_routine, arg);
-  CreateThreadContextArgs args = { t, &stack };
-  asanThreadRegistry().CreateThread(*(uptr*)t, detached, current_tid, &args);
-  return REAL(pthread_create)(thread, attr, asan_thread_start, t);
+  ThreadStartParam param;
+  atomic_store(&param.t, 0, memory_order_relaxed);
+  atomic_store(&param.is_registered, 0, memory_order_relaxed);
+  int result = REAL(pthread_create)(thread, attr, asan_thread_start, &param);
+  if (result == 0) {
+    u32 current_tid = GetCurrentTidOrInvalid();
+    AsanThread *t = AsanThread::Create(start_routine, arg);
+    CreateThreadContextArgs args = { t, &stack };
+    asanThreadRegistry().CreateThread(*reinterpret_cast<uptr *>(t), detached,
+                                      current_tid, &args);
+    atomic_store(&param.t, reinterpret_cast<uptr>(t), memory_order_release);
+    // Wait until the AsanThread object is initialized and the ThreadRegistry
+    // entry is in "started" state. One reason for this is that after this
+    // interceptor exits, the child thread's stack may be the only thing holding
+    // the |arg| pointer. This may cause LSan to report a leak if leak checking
+    // happens at a point when the interceptor has already exited, but the stack
+    // range for the child thread is not yet known.
+    while (atomic_load(&param.is_registered, memory_order_acquire) == 0)
+      internal_sched_yield();
+  }
+  return result;
 }
 #endif  // ASAN_INTERCEPT_PTHREAD_CREATE
 
@@ -703,7 +727,7 @@ INTERCEPTOR_WINAPI(DWORD, CreateThread,
                    void* security, uptr stack_size,
                    DWORD (__stdcall *start_routine)(void*), void* arg,
                    DWORD thr_flags, void* tid) {
-  // Strict init-order checking in thread-hostile.
+  // Strict init-order checking is thread-hostile.
   if (flags()->strict_init_order)
     StopInitOrderChecking();
   GET_STACK_TRACE_THREAD;

compiler-rt/lib/asan/asan_rtl.cc

@@ -680,7 +680,8 @@ static void AsanInitInternal() {
       0, true, 0, &create_main_args);
   CHECK_EQ(0, main_tid);
   SetCurrentThread(main_thread);
-  main_thread->ThreadStart(internal_getpid());
+  main_thread->ThreadStart(internal_getpid(),
+                           /* signal_thread_is_registered */ nullptr);
   force_interface_symbols();  // no-op.
   SanitizerInitializeUnwinder();
 

compiler-rt/lib/asan/asan_thread.cc

@@ -155,9 +155,13 @@ void AsanThread::Init() {
   AsanPlatformThreadInit();
 }
 
-thread_return_t AsanThread::ThreadStart(uptr os_id) {
+thread_return_t AsanThread::ThreadStart(
+    uptr os_id, atomic_uintptr_t *signal_thread_is_registered) {
   Init();
   asanThreadRegistry().StartThread(tid(), os_id, 0);
+  if (signal_thread_is_registered)
+    atomic_store(signal_thread_is_registered, 1, memory_order_release);
+
   if (common_flags()->use_sigaltstack) SetAlternateSignalStack();
 
   if (!start_routine_) {

compiler-rt/lib/asan/asan_thread.h

@@ -60,7 +60,8 @@ class AsanThread {
   void Destroy();
 
   void Init();  // Should be called from the thread itself.
-  thread_return_t ThreadStart(uptr os_id);
+  thread_return_t ThreadStart(uptr os_id,
+                              atomic_uintptr_t *signal_thread_is_registered);
 
   uptr stack_top() { return stack_top_; }
   uptr stack_bottom() { return stack_bottom_; }

compiler-rt/lib/lsan/lsan_interceptors.cc

@@ -215,9 +215,9 @@ extern "C" void *__lsan_thread_start_func(void *arg) {
   int tid = 0;
   while ((tid = atomic_load(&p->tid, memory_order_acquire)) == 0)
     internal_sched_yield();
-  atomic_store(&p->tid, 0, memory_order_release);
   SetCurrentThread(tid);
   ThreadStart(tid, GetTid());
+  atomic_store(&p->tid, 0, memory_order_release);
   return callback(param);
 }
 

compiler-rt/test/lsan/TestCases/leak_check_before_thread_started.cc

@@ -0,0 +1,15 @@
+// Regression test for http://llvm.org/bugs/show_bug.cgi?id=21621
+// This test relies on timing between threads, so any failures will be flaky.
+// RUN: LSAN_BASE="use_stacks=0:use_registers=0"
+// RUN: %clangxx_lsan %s -std=c++11 -o %t
+// RUN: %run %t
+#include <thread>
+#include <chrono>
+
+void func() {
+      std::this_thread::sleep_for(std::chrono::milliseconds(500));
+}
+
+int main() {
+      std::thread(func).detach();
+}