diff --git a/compiler-rt/lib/asan/asan_allocator.cpp b/compiler-rt/lib/asan/asan_allocator.cpp --- a/compiler-rt/lib/asan/asan_allocator.cpp +++ b/compiler-rt/lib/asan/asan_allocator.cpp @@ -1125,12 +1125,45 @@ } void ForEachRegisteredThreadContextCb(ThreadContextBase *tctx, void *arg) { - // This function can be used to treat memory reachable from `tctx` as live. - // This is useful for threads that have been created but not yet started. - - // This is currently a no-op because the ASan `pthread_create()` interceptor - // blocks until the child thread starts which keeps the thread's `arg` pointer - // live. + // The code here looks for created but not yet started threads and treats the + // thread's `arg` pointer as reachable if it is a heap object. This is + // necessary to prevent leak false positives if nothing else holds a live + // reference to the heap object. This can happen because the ASan + // `pthread_create()` interceptor doesn't wait for the child thread to + // start before exiting. + + __asan::AsanThreadContext *atctx = + reinterpret_cast<__asan::AsanThreadContext *>(tctx); + __asan::AsanThread *asanThread = atctx->thread; + + // We only care about threads that have been created but not yet + // started. + if (atctx->status != ThreadStatusCreated || !asanThread) + return; + + void *threadArg = asanThread->get_arg(); + if (!threadArg) + return; + + uptr chunk = PointsIntoChunk(threadArg); + if (!chunk) + return; + + // Add chunk to frontier and mark it as reachable. + Frontier *frontier = reinterpret_cast(arg); + frontier->push_back(chunk); + LsanMetadata m(chunk); + m.set_tag(kReachable); + + // __lsan::flags() has to be guarded by this macro because it uses data that + // is not defined when !CAN_SANITIZE_LEAKS. +#if CAN_SANITIZE_LEAKS + if (flags()->log_pointers) + Report( + "Treating arg pointer of created but not started thread T%d as live: " + "%p\n", + atctx->tid, threadArg); +#endif } uptr GetUserBegin(uptr chunk) { diff --git a/compiler-rt/lib/asan/asan_interceptors.cpp b/compiler-rt/lib/asan/asan_interceptors.cpp --- a/compiler-rt/lib/asan/asan_interceptors.cpp +++ b/compiler-rt/lib/asan/asan_interceptors.cpp @@ -189,20 +189,11 @@ #include "sanitizer_common/sanitizer_common_syscalls.inc" #include "sanitizer_common/sanitizer_syscalls_netbsd.inc" -struct ThreadStartParam { - atomic_uintptr_t t; - atomic_uintptr_t is_registered; -}; - #if ASAN_INTERCEPT_PTHREAD_CREATE static thread_return_t THREAD_CALLING_CONV asan_thread_start(void *arg) { - ThreadStartParam *param = reinterpret_cast(arg); - AsanThread *t = nullptr; - while ((t = reinterpret_cast( - atomic_load(¶m->t, memory_order_acquire))) == nullptr) - internal_sched_yield(); + AsanThread *t = (AsanThread *)arg; SetCurrentThread(t); - return t->ThreadStart(GetTid(), ¶m->is_registered); + return t->ThreadStart(GetTid()); } INTERCEPTOR(int, pthread_create, void *thread, @@ -215,9 +206,11 @@ int detached = 0; if (attr) REAL(pthread_attr_getdetachstate)(attr, &detached); - ThreadStartParam param; - atomic_store(¶m.t, 0, memory_order_relaxed); - atomic_store(¶m.is_registered, 0, memory_order_relaxed); + + 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 @@ -227,21 +220,7 @@ #if CAN_SANITIZE_LEAKS __lsan::ScopedInterceptorDisabler disabler; #endif - result = REAL(pthread_create)(thread, attr, asan_thread_start, ¶m); - } - if (result == 0) { - u32 current_tid = GetCurrentTidOrInvalid(); - AsanThread *t = - AsanThread::Create(start_routine, arg, current_tid, &stack, detached); - atomic_store(¶m.t, reinterpret_cast(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(¶m.is_registered, memory_order_acquire) == 0) - internal_sched_yield(); + result = REAL(pthread_create)(thread, attr, asan_thread_start, t); } return result; } diff --git a/compiler-rt/lib/asan/asan_thread.h b/compiler-rt/lib/asan/asan_thread.h --- a/compiler-rt/lib/asan/asan_thread.h +++ b/compiler-rt/lib/asan/asan_thread.h @@ -69,8 +69,7 @@ struct InitOptions; void Init(const InitOptions *options = nullptr); - thread_return_t ThreadStart(tid_t os_id, - atomic_uintptr_t *signal_thread_is_registered); + thread_return_t ThreadStart(tid_t os_id); uptr stack_top(); uptr stack_bottom(); @@ -132,6 +131,8 @@ void *extra_spill_area() { return &extra_spill_area_; } + void *get_arg() { return arg_; } + private: // NOTE: There is no AsanThread constructor. It is allocated // via mmap() and *must* be valid in zero-initialized state. diff --git a/compiler-rt/lib/asan/asan_thread.cpp b/compiler-rt/lib/asan/asan_thread.cpp --- a/compiler-rt/lib/asan/asan_thread.cpp +++ b/compiler-rt/lib/asan/asan_thread.cpp @@ -253,12 +253,9 @@ // SetThreadStackAndTls. #if !SANITIZER_FUCHSIA && !SANITIZER_RTEMS -thread_return_t AsanThread::ThreadStart( - tid_t os_id, atomic_uintptr_t *signal_thread_is_registered) { +thread_return_t AsanThread::ThreadStart(tid_t os_id) { Init(); asanThreadRegistry().StartThread(tid(), os_id, ThreadType::Regular, nullptr); - if (signal_thread_is_registered) - atomic_store(signal_thread_is_registered, 1, memory_order_release); if (common_flags()->use_sigaltstack) SetAlternateSignalStack(); @@ -288,8 +285,7 @@ /* start_routine */ nullptr, /* arg */ nullptr, /* parent_tid */ 0, /* stack */ nullptr, /* detached */ true); SetCurrentThread(main_thread); - main_thread->ThreadStart(internal_getpid(), - /* signal_thread_is_registered */ nullptr); + main_thread->ThreadStart(internal_getpid()); return main_thread; }