[compiler-rt] d2b434b - Revert "[LSAN] Use ThreadArgRetval in LSAN"
Vitaly Buka via llvm-commits
llvm-commits at lists.llvm.org
Tue May 16 10:50:04 PDT 2023
Author: Vitaly Buka
Date: 2023-05-16T10:49:45-07:00
New Revision: d2b434b4e963ed32df42fc31d9408decef436b3e
URL: https://github.com/llvm/llvm-project/commit/d2b434b4e963ed32df42fc31d9408decef436b3e
DIFF: https://github.com/llvm/llvm-project/commit/d2b434b4e963ed32df42fc31d9408decef436b3e.diff
LOG: Revert "[LSAN] Use ThreadArgRetval in LSAN"
https://bugs.chromium.org/p/chromium/issues/detail?id=1445676
This reverts commit 20a3c6e84e0955ac20762c35e8c2435017ae967d.
Added:
Modified:
compiler-rt/lib/lsan/lsan_interceptors.cpp
compiler-rt/lib/lsan/lsan_thread.cpp
compiler-rt/lib/lsan/lsan_thread.h
compiler-rt/test/lsan/TestCases/create_thread_leak.cpp
Removed:
################################################################################
diff --git a/compiler-rt/lib/lsan/lsan_interceptors.cpp b/compiler-rt/lib/lsan/lsan_interceptors.cpp
index 4be34db28c773..dc601cd646cf3 100644
--- a/compiler-rt/lib/lsan/lsan_interceptors.cpp
+++ b/compiler-rt/lib/lsan/lsan_interceptors.cpp
@@ -415,8 +415,16 @@ 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) {
- atomic_uintptr_t *atomic_tid = (atomic_uintptr_t *)arg;
+ ThreadParam *p = (ThreadParam*)arg;
+ void* (*callback)(void *arg) = p->callback;
+ void *param = p->param;
// Wait until the last iteration to maximize the chance that we are the last
// destructor to run.
#if !SANITIZER_NETBSD && !SANITIZER_FREEBSD
@@ -427,15 +435,11 @@ extern "C" void *__lsan_thread_start_func(void *arg) {
}
#endif
int tid = 0;
- while ((tid = atomic_load(atomic_tid, memory_order_acquire)) == 0)
+ while ((tid = atomic_load(&p->tid, memory_order_acquire)) == 0)
internal_sched_yield();
ThreadStart(tid, GetTid());
- 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;
+ atomic_store(&p->tid, 0, memory_order_release);
+ return callback(param);
}
INTERCEPTOR(int, pthread_create, void *th, void *attr,
@@ -450,63 +454,46 @@ INTERCEPTOR(int, pthread_create, void *th, void *attr,
AdjustStackSize(attr);
int detached = 0;
pthread_attr_getdetachstate(attr, &detached);
- atomic_uintptr_t atomic_tid = {};
- int result;
+ ThreadParam p;
+ p.callback = callback;
+ p.param = param;
+ atomic_store(&p.tid, 0, memory_order_relaxed);
+ int res;
{
// 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;
- GetThreadArgRetval().Create(detached, {callback, param}, [&]() -> uptr {
- result =
- REAL(pthread_create)(th, attr, __lsan_thread_start_func, &atomic_tid);
- return result ? 0 : *(uptr *)(th);
- });
+ res = REAL(pthread_create)(th, attr, __lsan_thread_start_func, &p);
}
- if (result == 0) {
+ if (res == 0) {
int tid = ThreadCreate(GetCurrentThreadId(), IsStateDetached(detached));
CHECK_NE(tid, kMainTid);
- atomic_store(&atomic_tid, tid, memory_order_release);
- while (atomic_load(&atomic_tid, memory_order_acquire) != 0)
+ atomic_store(&p.tid, tid, memory_order_release);
+ while (atomic_load(&p.tid, memory_order_acquire) != 0)
internal_sched_yield();
}
if (attr == &myattr)
pthread_attr_destroy(&myattr);
- return result;
+ return res;
}
INTERCEPTOR(int, pthread_join, void *thread, void **retval) {
- int result;
- GetThreadArgRetval().Join((uptr)thread, [&]() {
- result = REAL(pthread_join)(thread, retval);
- return !result;
- });
- return result;
+ return REAL(pthread_join)(thread, retval);
}
INTERCEPTOR(int, pthread_detach, void *thread) {
- int result;
- GetThreadArgRetval().Detach((uptr)thread, [&]() {
- result = REAL(pthread_detach)(thread);
- return !result;
- });
- return result;
+ return REAL(pthread_detach)(thread);
}
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) {
- int result;
- GetThreadArgRetval().Join((uptr)thread, [&]() {
- result = REAL(pthread_tryjoin_np)(thread, ret);
- return !result;
- });
- return result;
+ return REAL(pthread_tryjoin_np)(thread, ret);
}
# define LSAN_MAYBE_INTERCEPT_TRYJOIN INTERCEPT_FUNCTION(pthread_tryjoin_np)
# else
@@ -516,12 +503,7 @@ 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) {
- int result;
- GetThreadArgRetval().Join((uptr)thread, [&]() {
- result = REAL(pthread_timedjoin_np)(thread, ret, abstime);
- return !result;
- });
- return result;
+ return REAL(pthread_timedjoin_np)(thread, ret, abstime);
}
# 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 e0ea622fa33c1..5a00973ef8026 100644
--- a/compiler-rt/lib/lsan/lsan_thread.cpp
+++ b/compiler-rt/lib/lsan/lsan_thread.cpp
@@ -24,7 +24,6 @@
namespace __lsan {
static ThreadRegistry *thread_registry;
-static ThreadArgRetval *thread_arg_retval;
static Mutex mu_for_thread_context;
static LowLevelAllocator allocator_for_thread_context;
@@ -35,18 +34,11 @@ static ThreadContextBase *CreateThreadContext(u32 tid) {
}
void InitializeThreadRegistry() {
- static ALIGNED(alignof(
- ThreadRegistry)) char thread_registry_placeholder[sizeof(ThreadRegistry)];
+ static ALIGNED(64) 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) {}
@@ -80,15 +72,9 @@ void GetThreadExtraStackRangesLocked(tid_t os_id,
InternalMmapVector<Range> *ranges) {}
void GetThreadExtraStackRangesLocked(InternalMmapVector<Range> *ranges) {}
-void LockThreadRegistry() {
- thread_registry->Lock();
- thread_arg_retval->Lock();
-}
+void LockThreadRegistry() { thread_registry->Lock(); }
-void UnlockThreadRegistry() {
- thread_arg_retval->Unlock();
- thread_registry->Unlock();
-}
+void UnlockThreadRegistry() { thread_registry->Unlock(); }
ThreadRegistry *GetLsanThreadRegistryLocked() {
thread_registry->CheckLocked();
@@ -107,7 +93,12 @@ void GetRunningThreadsLocked(InternalMmapVector<tid_t> *threads) {
}
void GetAdditionalThreadContextPtrsLocked(InternalMmapVector<uptr> *ptrs) {
- GetThreadArgRetval().GetAllPtrsLocked(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.
}
} // namespace __lsan
diff --git a/compiler-rt/lib/lsan/lsan_thread.h b/compiler-rt/lib/lsan/lsan_thread.h
index afe83fb93401f..709a02915c2d9 100644
--- a/compiler-rt/lib/lsan/lsan_thread.h
+++ b/compiler-rt/lib/lsan/lsan_thread.h
@@ -14,7 +14,6 @@
#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 {
@@ -48,7 +47,6 @@ 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 92e70b2e37c3d..4074cd4e540e6 100644
--- a/compiler-rt/test/lsan/TestCases/create_thread_leak.cpp
+++ b/compiler-rt/test/lsan/TestCases/create_thread_leak.cpp
@@ -4,7 +4,10 @@
// 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
-// RUN: %run %t 10 0 0 0
+
+// FIXME: Remove "not". There is no leak.
+// False LEAK234 is broken for LSAN.
+// RUN: %run %if lsan-standalone %{ not %} %t 10 0 0 0
#include <pthread.h>
#include <stdlib.h>
More information about the llvm-commits
mailing list