From 20a3c6e84e0955ac20762c35e8c2435017ae967d Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Mon, 8 May 2023 12:42:50 -0700 Subject: [LSAN] Use ThreadArgRetval in LSAN Fixes false leaks on thread retval. Reviewed By: thurston Differential Revision: https://reviews.llvm.org/D150165 --- compiler-rt/lib/lsan/lsan_interceptors.cpp | 70 ++++++++++++++-------- compiler-rt/lib/lsan/lsan_thread.cpp | 27 ++++++--- compiler-rt/lib/lsan/lsan_thread.h | 2 + .../test/lsan/TestCases/create_thread_leak.cpp | 5 +- 4 files changed, 65 insertions(+), 39 deletions(-) (limited to 'compiler-rt') diff --git a/compiler-rt/lib/lsan/lsan_interceptors.cpp b/compiler-rt/lib/lsan/lsan_interceptors.cpp index dc601cd646cf..4be34db28c77 100644 --- a/compiler-rt/lib/lsan/lsan_interceptors.cpp +++ b/compiler-rt/lib/lsan/lsan_interceptors.cpp @@ -415,16 +415,8 @@ INTERCEPTOR(char *, strerror, int errnum) { #if SANITIZER_POSIX -struct ThreadParam { - void *(*callback)(void *arg); - void *param; - atomic_uintptr_t tid; -}; - extern "C" void *__lsan_thread_start_func(void *arg) { - ThreadParam *p = (ThreadParam*)arg; - void* (*callback)(void *arg) = p->callback; - void *param = p->param; + atomic_uintptr_t *atomic_tid = (atomic_uintptr_t *)arg; // Wait until the last iteration to maximize the chance that we are the last // destructor to run. #if !SANITIZER_NETBSD && !SANITIZER_FREEBSD @@ -435,11 +427,15 @@ extern "C" void *__lsan_thread_start_func(void *arg) { } #endif int tid = 0; - while ((tid = atomic_load(&p->tid, memory_order_acquire)) == 0) + while ((tid = atomic_load(atomic_tid, memory_order_acquire)) == 0) internal_sched_yield(); ThreadStart(tid, GetTid()); - atomic_store(&p->tid, 0, memory_order_release); - return callback(param); + atomic_store(atomic_tid, 0, memory_order_release); + auto self = GetThreadSelf(); + auto args = GetThreadArgRetval().GetArgs(self); + void *retval = (*args.routine)(args.arg_retval); + GetThreadArgRetval().Finish(self, retval); + return retval; } INTERCEPTOR(int, pthread_create, void *th, void *attr, @@ -454,46 +450,63 @@ INTERCEPTOR(int, pthread_create, void *th, void *attr, AdjustStackSize(attr); int detached = 0; pthread_attr_getdetachstate(attr, &detached); - ThreadParam p; - p.callback = callback; - p.param = param; - atomic_store(&p.tid, 0, memory_order_relaxed); - int res; + atomic_uintptr_t atomic_tid = {}; + 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. ScopedInterceptorDisabler disabler; - res = REAL(pthread_create)(th, attr, __lsan_thread_start_func, &p); + GetThreadArgRetval().Create(detached, {callback, param}, [&]() -> uptr { + result = + REAL(pthread_create)(th, attr, __lsan_thread_start_func, &atomic_tid); + return result ? 0 : *(uptr *)(th); + }); } - if (res == 0) { + if (result == 0) { int tid = ThreadCreate(GetCurrentThreadId(), IsStateDetached(detached)); CHECK_NE(tid, kMainTid); - atomic_store(&p.tid, tid, memory_order_release); - while (atomic_load(&p.tid, memory_order_acquire) != 0) + atomic_store(&atomic_tid, tid, memory_order_release); + while (atomic_load(&atomic_tid, memory_order_acquire) != 0) internal_sched_yield(); } if (attr == &myattr) pthread_attr_destroy(&myattr); - return res; + return result; } INTERCEPTOR(int, pthread_join, void *thread, void **retval) { - return REAL(pthread_join)(thread, retval); + int result; + GetThreadArgRetval().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; + GetThreadArgRetval().Detach((uptr)thread, [&]() { + result = REAL(pthread_detach)(thread); + return !result; + }); + return result; } INTERCEPTOR(int, pthread_exit, void *retval) { + GetThreadArgRetval().Finish(GetThreadSelf(), retval); return REAL(pthread_exit)(retval); } # if SANITIZER_INTERCEPT_TRYJOIN INTERCEPTOR(int, pthread_tryjoin_np, void *thread, void **ret) { - return REAL(pthread_tryjoin_np)(thread, ret); + int result; + GetThreadArgRetval().Join((uptr)thread, [&]() { + result = REAL(pthread_tryjoin_np)(thread, ret); + return !result; + }); + return result; } # define LSAN_MAYBE_INTERCEPT_TRYJOIN INTERCEPT_FUNCTION(pthread_tryjoin_np) # else @@ -503,7 +516,12 @@ INTERCEPTOR(int, pthread_tryjoin_np, void *thread, void **ret) { # if SANITIZER_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; + GetThreadArgRetval().Join((uptr)thread, [&]() { + result = REAL(pthread_timedjoin_np)(thread, ret, abstime); + return !result; + }); + return result; } # define LSAN_MAYBE_INTERCEPT_TIMEDJOIN \ INTERCEPT_FUNCTION(pthread_timedjoin_np) diff --git a/compiler-rt/lib/lsan/lsan_thread.cpp b/compiler-rt/lib/lsan/lsan_thread.cpp index 5a00973ef802..e0ea622fa33c 100644 --- a/compiler-rt/lib/lsan/lsan_thread.cpp +++ b/compiler-rt/lib/lsan/lsan_thread.cpp @@ -24,6 +24,7 @@ namespace __lsan { static ThreadRegistry *thread_registry; +static ThreadArgRetval *thread_arg_retval; static Mutex mu_for_thread_context; static LowLevelAllocator allocator_for_thread_context; @@ -34,11 +35,18 @@ static ThreadContextBase *CreateThreadContext(u32 tid) { } void InitializeThreadRegistry() { - static ALIGNED(64) char thread_registry_placeholder[sizeof(ThreadRegistry)]; + static ALIGNED(alignof( + ThreadRegistry)) char thread_registry_placeholder[sizeof(ThreadRegistry)]; thread_registry = new (thread_registry_placeholder) ThreadRegistry(CreateThreadContext); + + static ALIGNED(alignof(ThreadArgRetval)) char + thread_arg_retval_placeholder[sizeof(ThreadArgRetval)]; + thread_arg_retval = new (thread_arg_retval_placeholder) ThreadArgRetval(); } +ThreadArgRetval &GetThreadArgRetval() { return *thread_arg_retval; } + ThreadContextLsanBase::ThreadContextLsanBase(int tid) : ThreadContextBase(tid) {} @@ -72,9 +80,15 @@ void GetThreadExtraStackRangesLocked(tid_t os_id, InternalMmapVector *ranges) {} void GetThreadExtraStackRangesLocked(InternalMmapVector *ranges) {} -void LockThreadRegistry() { thread_registry->Lock(); } +void LockThreadRegistry() { + thread_registry->Lock(); + thread_arg_retval->Lock(); +} -void UnlockThreadRegistry() { thread_registry->Unlock(); } +void UnlockThreadRegistry() { + thread_arg_retval->Unlock(); + thread_registry->Unlock(); +} ThreadRegistry *GetLsanThreadRegistryLocked() { thread_registry->CheckLocked(); @@ -93,12 +107,7 @@ void GetRunningThreadsLocked(InternalMmapVector *threads) { } void GetAdditionalThreadContextPtrsLocked(InternalMmapVector *ptrs) { - // 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 LSan `pthread_create()` interceptor - // blocks until the child thread starts which keeps the thread's `arg` pointer - // live. + GetThreadArgRetval().GetAllPtrsLocked(ptrs); } } // namespace __lsan diff --git a/compiler-rt/lib/lsan/lsan_thread.h b/compiler-rt/lib/lsan/lsan_thread.h index 709a02915c2d..afe83fb93401 100644 --- a/compiler-rt/lib/lsan/lsan_thread.h +++ b/compiler-rt/lib/lsan/lsan_thread.h @@ -14,6 +14,7 @@ #ifndef LSAN_THREAD_H #define LSAN_THREAD_H +#include "sanitizer_common/sanitizer_thread_arg_retval.h" #include "sanitizer_common/sanitizer_thread_registry.h" namespace __lsan { @@ -47,6 +48,7 @@ void InitializeThreadRegistry(); void InitializeMainThread(); ThreadRegistry *GetLsanThreadRegistryLocked(); +ThreadArgRetval &GetThreadArgRetval(); u32 ThreadCreate(u32 tid, bool detached, void *arg = nullptr); void ThreadFinish(); diff --git a/compiler-rt/test/lsan/TestCases/create_thread_leak.cpp b/compiler-rt/test/lsan/TestCases/create_thread_leak.cpp index 4074cd4e540e..92e70b2e37c3 100644 --- a/compiler-rt/test/lsan/TestCases/create_thread_leak.cpp +++ b/compiler-rt/test/lsan/TestCases/create_thread_leak.cpp @@ -4,10 +4,7 @@ // 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 LSAN. -// RUN: %run %if lsan-standalone %{ not %} %t 10 0 0 0 +// RUN: %run %t 10 0 0 0 #include #include -- cgit v1.2.1