[compiler-rt] 7760272 - [lsan] Don't crash on ThreadRegistry::threads_ data race
Vitaly Buka via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 17 15:33:56 PDT 2023
Author: Vitaly Buka
Date: 2023-04-17T15:33:43-07:00
New Revision: 776027277818af1a2672dc396cdd64d79dacf495
URL: https://github.com/llvm/llvm-project/commit/776027277818af1a2672dc396cdd64d79dacf495
DIFF: https://github.com/llvm/llvm-project/commit/776027277818af1a2672dc396cdd64d79dacf495.diff
LOG: [lsan] Don't crash on ThreadRegistry::threads_ data race
Comment "No lock needed" in CurrentThreadContext was wrong.
Concurent ThreadRegistry::CreateThread can resize and relocate
ThreadRegistry::threads_ the same time CurrentThreadContext reads it.
To mitigate lock cost we store ThreadContext* instead of tid in
THREADLOCAL cache, we can tid from the ThreadContext*.
Reviewed By: kstoimenov, MaskRay
Differential Revision: https://reviews.llvm.org/D148281
Added:
compiler-rt/test/lsan/TestCases/thread_context_crash.cpp
Modified:
compiler-rt/lib/lsan/lsan.cpp
compiler-rt/lib/lsan/lsan_common_mac.cpp
compiler-rt/lib/lsan/lsan_linux.cpp
compiler-rt/lib/lsan/lsan_thread.cpp
compiler-rt/lib/lsan/lsan_thread.h
compiler-rt/test/lsan/lit.common.cfg.py
Removed:
################################################################################
diff --git a/compiler-rt/lib/lsan/lsan.cpp b/compiler-rt/lib/lsan/lsan.cpp
index 489c5ca01fed8..319f399e60f5d 100644
--- a/compiler-rt/lib/lsan/lsan.cpp
+++ b/compiler-rt/lib/lsan/lsan.cpp
@@ -36,7 +36,7 @@ void __sanitizer::BufferedStackTrace::UnwindImpl(
uptr pc, uptr bp, void *context, bool request_fast, u32 max_depth) {
using namespace __lsan;
uptr stack_top = 0, stack_bottom = 0;
- if (ThreadContext *t = CurrentThreadContext()) {
+ if (ThreadContextLsanBase *t = GetCurrentThread()) {
stack_top = t->stack_end();
stack_bottom = t->stack_begin();
}
diff --git a/compiler-rt/lib/lsan/lsan_common_mac.cpp b/compiler-rt/lib/lsan/lsan_common_mac.cpp
index b6b15095744d7..70ff6d57ad796 100644
--- a/compiler-rt/lib/lsan/lsan_common_mac.cpp
+++ b/compiler-rt/lib/lsan/lsan_common_mac.cpp
@@ -50,7 +50,7 @@ struct RegionScanState {
typedef struct {
int disable_counter;
- u32 current_thread_id;
+ ThreadContextLsanBase *current_thread;
AllocatorCache cache;
} thread_local_data_t;
@@ -99,12 +99,14 @@ void EnableInThisThread() {
--*disable_counter;
}
-u32 GetCurrentThread() {
+ThreadContextLsanBase *GetCurrentThread() {
thread_local_data_t *data = get_tls_val(false);
- return data ? data->current_thread_id : kInvalidTid;
+ return data ? data->current_thread : nullptr;
}
-void SetCurrentThread(u32 tid) { get_tls_val(true)->current_thread_id = tid; }
+void SetCurrentThread(ThreadContextLsanBase *tctx) {
+ get_tls_val(true)->current_thread = tctx;
+}
AllocatorCache *GetAllocatorCache() { return &get_tls_val(true)->cache; }
diff --git a/compiler-rt/lib/lsan/lsan_linux.cpp b/compiler-rt/lib/lsan/lsan_linux.cpp
index 47c2f21b5a6bc..5074cee1296a8 100644
--- a/compiler-rt/lib/lsan/lsan_linux.cpp
+++ b/compiler-rt/lib/lsan/lsan_linux.cpp
@@ -14,13 +14,14 @@
#if SANITIZER_LINUX || SANITIZER_NETBSD || SANITIZER_FUCHSIA
-#include "lsan_allocator.h"
+# include "lsan_allocator.h"
+# include "lsan_thread.h"
namespace __lsan {
-static THREADLOCAL u32 current_thread_tid = kInvalidTid;
-u32 GetCurrentThread() { return current_thread_tid; }
-void SetCurrentThread(u32 tid) { current_thread_tid = tid; }
+static THREADLOCAL ThreadContextLsanBase *current_thread = nullptr;
+ThreadContextLsanBase *GetCurrentThread() { return current_thread; }
+void SetCurrentThread(ThreadContextLsanBase *tctx) { current_thread = tctx; }
static THREADLOCAL AllocatorCache allocator_cache;
AllocatorCache *GetAllocatorCache() { return &allocator_cache; }
diff --git a/compiler-rt/lib/lsan/lsan_thread.cpp b/compiler-rt/lib/lsan/lsan_thread.cpp
index bc05021e98a99..f1075afe9ef6f 100644
--- a/compiler-rt/lib/lsan/lsan_thread.cpp
+++ b/compiler-rt/lib/lsan/lsan_thread.cpp
@@ -39,12 +39,12 @@ void InitializeThreadRegistry() {
ThreadContextLsanBase::ThreadContextLsanBase(int tid)
: ThreadContextBase(tid) {}
-void ThreadContextLsanBase::OnStarted(void *arg) { SetCurrentThread(tid); }
+void ThreadContextLsanBase::OnStarted(void *arg) { SetCurrentThread(this); }
void ThreadContextLsanBase::OnFinished() {
AllocatorThreadFinish();
DTLS_Destroy();
- SetCurrentThread(kInvalidTid);
+ SetCurrentThread(nullptr);
}
u32 ThreadCreate(u32 parent_tid, bool detached, void *arg) {
@@ -58,19 +58,9 @@ void ThreadContextLsanBase::ThreadStart(u32 tid, tid_t os_id,
void ThreadFinish() { thread_registry->FinishThread(GetCurrentThreadId()); }
-ThreadContext *CurrentThreadContext() {
- if (!thread_registry)
- return nullptr;
- if (GetCurrentThreadId() == kInvalidTid)
- return nullptr;
- // No lock needed when getting current thread.
- return (ThreadContext *)thread_registry->GetThreadLocked(
- GetCurrentThreadId());
-}
-
void EnsureMainThreadIDIsCorrect() {
if (GetCurrentThreadId() == kMainTid)
- CurrentThreadContext()->os_id = GetTid();
+ GetCurrentThread()->os_id = GetTid();
}
///// Interface to the common LSan module. /////
diff --git a/compiler-rt/lib/lsan/lsan_thread.h b/compiler-rt/lib/lsan/lsan_thread.h
index 5abbaff539764..709a02915c2d9 100644
--- a/compiler-rt/lib/lsan/lsan_thread.h
+++ b/compiler-rt/lib/lsan/lsan_thread.h
@@ -51,10 +51,12 @@ ThreadRegistry *GetLsanThreadRegistryLocked();
u32 ThreadCreate(u32 tid, bool detached, void *arg = nullptr);
void ThreadFinish();
-u32 GetCurrentThread();
-inline u32 GetCurrentThreadId() { return GetCurrentThread(); }
-void SetCurrentThread(u32 tid);
-ThreadContext *CurrentThreadContext();
+ThreadContextLsanBase *GetCurrentThread();
+inline u32 GetCurrentThreadId() {
+ ThreadContextLsanBase *ctx = GetCurrentThread();
+ return ctx ? ctx->tid : kInvalidTid;
+}
+void SetCurrentThread(ThreadContextLsanBase *tctx);
void EnsureMainThreadIDIsCorrect();
} // namespace __lsan
diff --git a/compiler-rt/test/lsan/TestCases/thread_context_crash.cpp b/compiler-rt/test/lsan/TestCases/thread_context_crash.cpp
new file mode 100644
index 0000000000000..cef6d58c98b53
--- /dev/null
+++ b/compiler-rt/test/lsan/TestCases/thread_context_crash.cpp
@@ -0,0 +1,33 @@
+// Check that concurent CurrentThreadContext does not crash.
+// RUN: %clangxx_lsan -O3 -pthread %s -o %t && %run %t 100
+
+// REQUIRES: lsan-standalone
+
+#include <pthread.h>
+#include <stdlib.h>
+#include <vector>
+
+#include <sanitizer/common_interface_defs.h>
+
+namespace __lsan {
+class ThreadContextLsanBase *GetCurrentThread();
+}
+
+void *null_func(void *args) {
+ for (int i = 0; i < 100000; ++i)
+ __lsan::GetCurrentThread();
+ return nullptr;
+}
+
+int main(int argc, char **argv) {
+ std::vector<pthread_t> threads;
+ for (int i = 0; i < atoi(argv[1]); ++i) {
+ threads.resize(10);
+ for (auto &thread : threads)
+ pthread_create(&thread, 0, null_func, NULL);
+
+ for (auto &thread : threads)
+ pthread_join(thread, nullptr);
+ }
+ return 0;
+}
diff --git a/compiler-rt/test/lsan/lit.common.cfg.py b/compiler-rt/test/lsan/lit.common.cfg.py
index c3df68f368001..cec5664f45d35 100644
--- a/compiler-rt/test/lsan/lit.common.cfg.py
+++ b/compiler-rt/test/lsan/lit.common.cfg.py
@@ -26,6 +26,7 @@ def get_required_attr(config, attr_name):
if lsan_lit_test_mode == "Standalone":
config.name = "LeakSanitizer-Standalone"
lsan_cflags = ["-fsanitize=leak"]
+ config.available_features.add('lsan-standalone')
elif lsan_lit_test_mode == "AddressSanitizer":
config.name = "LeakSanitizer-AddressSanitizer"
lsan_cflags = ["-fsanitize=address"]
More information about the llvm-commits
mailing list