From 6128397d130dbf24dcec0d3cedf74a2243d221a6 Mon Sep 17 00:00:00 2001 From: David Trevelyan Date: Thu, 17 Apr 2025 10:19:00 +0100 Subject: [PATCH 1/6] Add interceptor for os_unfair_lock_unlock (breaking) --- .../lib/rtsan/rtsan_interceptors_posix.cpp | 4 ++++ compiler-rt/lib/rtsan/tests/CMakeLists.txt | 3 +-- .../tests/rtsan_test_interceptors_posix.cpp | 24 +++++++++++++++---- 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp index 1b1ff9b211733..390f03670ae6f 100644 --- a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp +++ b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp @@ -729,6 +729,10 @@ INTERCEPTOR(void, os_unfair_lock_lock, os_unfair_lock_t lock) { return REAL(os_unfair_lock_lock)(lock); } +//INTERCEPTOR(void, os_unfair_lock_unlock, os_unfair_lock_t lock) { +// __rtsan_notify_intercepted_call("os_unfair_lock_unlock"); +// return REAL(os_unfair_lock_unlock)(lock); +//} #define RTSAN_MAYBE_INTERCEPT_OS_UNFAIR_LOCK_LOCK \ INTERCEPT_FUNCTION(os_unfair_lock_lock) #else diff --git a/compiler-rt/lib/rtsan/tests/CMakeLists.txt b/compiler-rt/lib/rtsan/tests/CMakeLists.txt index 0cf07b307d461..d0be42aeadf1d 100644 --- a/compiler-rt/lib/rtsan/tests/CMakeLists.txt +++ b/compiler-rt/lib/rtsan/tests/CMakeLists.txt @@ -34,8 +34,7 @@ set_target_properties(RtsanUnitTests PROPERTIES FOLDER "Compiler-RT Tests") set(RTSAN_UNITTEST_LINK_FLAGS ${COMPILER_RT_UNITTEST_LINK_FLAGS} ${COMPILER_RT_UNWINDER_LINK_LIBS} - ${SANITIZER_TEST_CXX_LIBRARIES} - -no-pie) + ${SANITIZER_TEST_CXX_LIBRARIES}) if (COMPILER_RT_USE_ATOMIC_LIBRARY) list(APPEND RTSAN_UNITTEST_LINK_FLAGS ${COMPILER_RT_ATOMIC_LIBRARY}) diff --git a/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp b/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp index 20e3b485f3be0..ada6c1ddcc941 100644 --- a/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp +++ b/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp @@ -142,7 +142,7 @@ TEST(TestRtsanInterceptors, VallocDiesWhenRealtime) { } #if __has_builtin(__builtin_available) && SANITIZER_APPLE -#define ALIGNED_ALLOC_AVAILABLE() (__builtin_available(macOS 10.15, *)) +#define ALIGNED_ALLOC_AVAILABLE() (__builtin_available(macos 10.15, *)) #else // We are going to assume this is true until we hit systems where it isn't #define ALIGNED_ALLOC_AVAILABLE() (true) @@ -462,7 +462,7 @@ TEST_F(RtsanFileTest, FcntlFlockDiesWhenRealtime) { ASSERT_THAT(fd, Ne(-1)); auto Func = [fd]() { - struct flock lock{}; + struct flock lock {}; lock.l_type = F_RDLCK; lock.l_whence = SEEK_SET; lock.l_start = 0; @@ -811,7 +811,7 @@ TEST(TestRtsanInterceptors, IoctlBehavesWithOutputPointer) { GTEST_SKIP(); } - struct ifreq ifr{}; + struct ifreq ifr {}; strncpy(ifr.ifr_name, ifaddr->ifa_name, IFNAMSIZ - 1); int retval = ioctl(sock, SIOCGIFADDR, &ifr); @@ -1203,6 +1203,7 @@ void OSSpinLockLock(volatile OSSpinLock *__lock); typedef volatile OSSpinLock *_os_nospin_lock_t; void _os_nospin_lock_lock(_os_nospin_lock_t lock); } +#pragma clang diagnostic pop //"-Wdeprecated-declarations" TEST(TestRtsanInterceptors, OsSpinLockLockDiesWhenRealtime) { auto Func = []() { @@ -1219,7 +1220,6 @@ TEST(TestRtsanInterceptors, OsNoSpinLockLockDiesWhenRealtime) { ExpectRealtimeDeath(Func, "_os_nospin_lock_lock"); ExpectNonRealtimeSurvival(Func); } -#pragma clang diagnostic pop //"-Wdeprecated-declarations" TEST(TestRtsanInterceptors, OsUnfairLockLockDiesWhenRealtime) { auto Func = []() { @@ -1229,7 +1229,21 @@ TEST(TestRtsanInterceptors, OsUnfairLockLockDiesWhenRealtime) { ExpectRealtimeDeath(Func, "os_unfair_lock_lock"); ExpectNonRealtimeSurvival(Func); } -#endif // SANITIZER_APPLE + +TEST(TestRtsanInterceptors, OsUnfairLockUnlockDiesWhenRealtime) { + auto Func = []() { + os_unfair_lock_s unfair_lock{}; + { + __rtsan_disable(); + os_unfair_lock_lock(&unfair_lock); + __rtsan_enable(); + } + os_unfair_lock_unlock(&unfair_lock); + }; + ExpectRealtimeDeath(Func, "os_unfair_lock_lock"); + ExpectNonRealtimeSurvival(Func); +} +#endif #if SANITIZER_LINUX TEST(TestRtsanInterceptors, SpinLockLockDiesWhenRealtime) { From 6d98e9547101290a414bebe18fba115526737d91 Mon Sep 17 00:00:00 2001 From: David Trevelyan Date: Fri, 16 May 2025 14:01:22 +0100 Subject: [PATCH 2/6] Fix os_unfair_lock_unlock test --- compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp b/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp index ada6c1ddcc941..8cb081e054b75 100644 --- a/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp +++ b/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp @@ -1240,7 +1240,7 @@ TEST(TestRtsanInterceptors, OsUnfairLockUnlockDiesWhenRealtime) { } os_unfair_lock_unlock(&unfair_lock); }; - ExpectRealtimeDeath(Func, "os_unfair_lock_lock"); + ExpectRealtimeDeath(Func, "os_unfair_lock_unlock"); ExpectNonRealtimeSurvival(Func); } #endif From 640584e9a802d0df677b2d94706ec23552c1309f Mon Sep 17 00:00:00 2001 From: David Trevelyan Date: Fri, 16 May 2025 14:02:21 +0100 Subject: [PATCH 3/6] Store Contexts in a __sanitizer::DenseMap --- compiler-rt/lib/rtsan/rtsan_context.cpp | 42 +++++++++++-------------- compiler-rt/lib/rtsan/rtsan_context.h | 7 ----- 2 files changed, 19 insertions(+), 30 deletions(-) diff --git a/compiler-rt/lib/rtsan/rtsan_context.cpp b/compiler-rt/lib/rtsan/rtsan_context.cpp index 536d62e81e2fb..d5ad33b98a51d 100644 --- a/compiler-rt/lib/rtsan/rtsan_context.cpp +++ b/compiler-rt/lib/rtsan/rtsan_context.cpp @@ -9,9 +9,9 @@ //===----------------------------------------------------------------------===// #include "rtsan/rtsan_context.h" -#include "rtsan/rtsan.h" #include "sanitizer_common/sanitizer_allocator_internal.h" +#include "sanitizer_common/sanitizer_dense_map.h" #include #include @@ -19,32 +19,28 @@ using namespace __sanitizer; using namespace __rtsan; -static pthread_key_t context_key; -static pthread_once_t key_once = PTHREAD_ONCE_INIT; +using ContextStorage = DenseMap; +static ContextStorage *context_storage = nullptr; -// InternalFree cannot be passed directly to pthread_key_create -// because it expects a signature with only one arg -static void InternalFreeWrapper(void *ptr) { __sanitizer::InternalFree(ptr); } +static void InitializeContextStorage() { + static constexpr size_t max_supported_num_threads = 4096; + context_storage = + static_cast(InternalAlloc(sizeof(ContextStorage))); + new (context_storage) ContextStorage(max_supported_num_threads); +} static __rtsan::Context &GetContextForThisThreadImpl() { - auto MakeThreadLocalContextKey = []() { - CHECK_EQ(pthread_key_create(&context_key, InternalFreeWrapper), 0); - }; - - pthread_once(&key_once, MakeThreadLocalContextKey); - Context *current_thread_context = - static_cast(pthread_getspecific(context_key)); - if (current_thread_context == nullptr) { - current_thread_context = - static_cast(InternalAlloc(sizeof(Context))); - new (current_thread_context) Context(); - pthread_setspecific(context_key, current_thread_context); - } - - return *current_thread_context; -} + if (context_storage == nullptr) + InitializeContextStorage(); -__rtsan::Context::Context() = default; + pthread_t const thread_id = pthread_self(); + if (context_storage->contains(thread_id)) + return context_storage->find(thread_id)->getSecond(); + + auto const [bucket_ptr, was_inserted] = + context_storage->try_emplace(pthread_self(), Context{}); + return bucket_ptr->getSecond(); +} void __rtsan::Context::RealtimePush() { realtime_depth_++; } diff --git a/compiler-rt/lib/rtsan/rtsan_context.h b/compiler-rt/lib/rtsan/rtsan_context.h index 97fd9b48062ec..604b0dcc7c75e 100644 --- a/compiler-rt/lib/rtsan/rtsan_context.h +++ b/compiler-rt/lib/rtsan/rtsan_context.h @@ -14,8 +14,6 @@ namespace __rtsan { class Context { public: - Context(); - void RealtimePush(); void RealtimePop(); @@ -25,11 +23,6 @@ class Context { bool InRealtimeContext() const; bool IsBypassed() const; - Context(const Context &) = delete; - Context(Context &&) = delete; - Context &operator=(const Context &) = delete; - Context &operator=(Context &&) = delete; - private: int realtime_depth_{0}; int bypass_depth_{0}; From dcd32cda419c927449836911f6f0a22f24e067e3 Mon Sep 17 00:00:00 2001 From: David Trevelyan Date: Fri, 16 May 2025 23:06:02 +0100 Subject: [PATCH 4/6] [rtsan] Remove libc++ dependency --- compiler-rt/lib/rtsan/CMakeLists.txt | 4 ++-- compiler-rt/lib/rtsan/rtsan_context.cpp | 3 ++- compiler-rt/lib/rtsan/rtsan_suppressions.cpp | 4 +++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/compiler-rt/lib/rtsan/CMakeLists.txt b/compiler-rt/lib/rtsan/CMakeLists.txt index a4413d9992b62..63ff6053d53da 100644 --- a/compiler-rt/lib/rtsan/CMakeLists.txt +++ b/compiler-rt/lib/rtsan/CMakeLists.txt @@ -28,10 +28,10 @@ set(RTSAN_HEADERS set(RTSAN_DEPS) set(RTSAN_CFLAGS - ${COMPILER_RT_COMMON_CFLAGS} + ${SANITIZER_COMMON_CFLAGS} ${COMPILER_RT_CXX_CFLAGS} -DSANITIZER_COMMON_NO_REDEFINE_BUILTINS) -set(RTSAN_LINK_FLAGS ${COMPILER_RT_COMMON_LINK_FLAGS}) +set(RTSAN_LINK_FLAGS ${SANITIZER_COMMON_LINK_FLAGS}) set(RTSAN_DYNAMIC_LIBS ${COMPILER_RT_UNWINDER_LINK_LIBS} ${SANITIZER_CXX_ABI_LIBRARIES} diff --git a/compiler-rt/lib/rtsan/rtsan_context.cpp b/compiler-rt/lib/rtsan/rtsan_context.cpp index d5ad33b98a51d..54075cc696103 100644 --- a/compiler-rt/lib/rtsan/rtsan_context.cpp +++ b/compiler-rt/lib/rtsan/rtsan_context.cpp @@ -13,7 +13,7 @@ #include "sanitizer_common/sanitizer_allocator_internal.h" #include "sanitizer_common/sanitizer_dense_map.h" -#include +#include #include using namespace __sanitizer; @@ -21,6 +21,7 @@ using namespace __rtsan; using ContextStorage = DenseMap; static ContextStorage *context_storage = nullptr; +inline void *operator new(size_t, void *ptr) noexcept { return ptr; } static void InitializeContextStorage() { static constexpr size_t max_supported_num_threads = 4096; diff --git a/compiler-rt/lib/rtsan/rtsan_suppressions.cpp b/compiler-rt/lib/rtsan/rtsan_suppressions.cpp index 2bcfbeed4195b..119fb2529351c 100644 --- a/compiler-rt/lib/rtsan/rtsan_suppressions.cpp +++ b/compiler-rt/lib/rtsan/rtsan_suppressions.cpp @@ -19,7 +19,7 @@ #include "sanitizer_common/sanitizer_suppressions.h" #include "sanitizer_common/sanitizer_symbolizer.h" -#include +#include using namespace __sanitizer; using namespace __rtsan; @@ -52,6 +52,8 @@ static const char *ConvertTypeToFlagName(ErrorType Type) { UNREACHABLE("unknown ErrorType!"); } +inline void *operator new(size_t, void *ptr) noexcept { return ptr; } + void __rtsan::InitializeSuppressions() { CHECK_EQ(nullptr, suppression_ctx); From d59ccc935cf3222f23c160eb4f0b5bfe7a896b13 Mon Sep 17 00:00:00 2001 From: David Trevelyan Date: Fri, 20 Jun 2025 16:52:31 +0100 Subject: [PATCH 5/6] Sketch implementation of multi-threaded context and more Apple lock interceptors --- compiler-rt/lib/rtsan/rtsan.cpp | 12 ++-- compiler-rt/lib/rtsan/rtsan_context.cpp | 65 ++++++++++--------- compiler-rt/lib/rtsan/rtsan_context.h | 18 ++++- .../lib/rtsan/rtsan_interceptors_posix.cpp | 13 ++-- .../lib/rtsan/tests/rtsan_test_context.cpp | 63 ++++++++++++++++++ .../tests/rtsan_test_interceptors_posix.cpp | 15 +++++ 6 files changed, 144 insertions(+), 42 deletions(-) diff --git a/compiler-rt/lib/rtsan/rtsan.cpp b/compiler-rt/lib/rtsan/rtsan.cpp index 73340b34f6d4b..8cdc6ed649c14 100644 --- a/compiler-rt/lib/rtsan/rtsan.cpp +++ b/compiler-rt/lib/rtsan/rtsan.cpp @@ -114,19 +114,19 @@ SANITIZER_INTERFACE_ATTRIBUTE bool __rtsan_is_initialized() { } SANITIZER_INTERFACE_ATTRIBUTE void __rtsan_realtime_enter() { - GetContextForThisThread().RealtimePush(); + GetContext().RealtimePush(); } SANITIZER_INTERFACE_ATTRIBUTE void __rtsan_realtime_exit() { - GetContextForThisThread().RealtimePop(); + GetContext().RealtimePop(); } SANITIZER_INTERFACE_ATTRIBUTE void __rtsan_disable() { - GetContextForThisThread().BypassPush(); + GetContext().BypassPush(); } SANITIZER_INTERFACE_ATTRIBUTE void __rtsan_enable() { - GetContextForThisThread().BypassPop(); + GetContext().BypassPop(); } SANITIZER_INTERFACE_ATTRIBUTE void @@ -137,7 +137,7 @@ __rtsan_notify_intercepted_call(const char *func_name) { __rtsan_ensure_initialized(); GET_CALLER_PC_BP; - ExpectNotRealtime(GetContextForThisThread(), + ExpectNotRealtime(GetContext(), {DiagnosticsInfoType::InterceptedCall, func_name, pc, bp}, OnViolation); } @@ -146,7 +146,7 @@ SANITIZER_INTERFACE_ATTRIBUTE void __rtsan_notify_blocking_call(const char *func_name) { __rtsan_ensure_initialized(); GET_CALLER_PC_BP; - ExpectNotRealtime(GetContextForThisThread(), + ExpectNotRealtime(GetContext(), {DiagnosticsInfoType::BlockingCall, func_name, pc, bp}, OnViolation); } diff --git a/compiler-rt/lib/rtsan/rtsan_context.cpp b/compiler-rt/lib/rtsan/rtsan_context.cpp index 54075cc696103..be33d8c335a1c 100644 --- a/compiler-rt/lib/rtsan/rtsan_context.cpp +++ b/compiler-rt/lib/rtsan/rtsan_context.cpp @@ -13,48 +13,55 @@ #include "sanitizer_common/sanitizer_allocator_internal.h" #include "sanitizer_common/sanitizer_dense_map.h" -#include #include +#include +#include using namespace __sanitizer; using namespace __rtsan; - -using ContextStorage = DenseMap; -static ContextStorage *context_storage = nullptr; inline void *operator new(size_t, void *ptr) noexcept { return ptr; } -static void InitializeContextStorage() { - static constexpr size_t max_supported_num_threads = 4096; - context_storage = - static_cast(InternalAlloc(sizeof(ContextStorage))); - new (context_storage) ContextStorage(max_supported_num_threads); -} +static Context *context = nullptr; -static __rtsan::Context &GetContextForThisThreadImpl() { - if (context_storage == nullptr) - InitializeContextStorage(); - - pthread_t const thread_id = pthread_self(); - if (context_storage->contains(thread_id)) - return context_storage->find(thread_id)->getSecond(); - - auto const [bucket_ptr, was_inserted] = - context_storage->try_emplace(pthread_self(), Context{}); - return bucket_ptr->getSecond(); +static void InitializeContext() { + context = static_cast(InternalAlloc(sizeof(Context))); + new (context) Context(); } -void __rtsan::Context::RealtimePush() { realtime_depth_++; } +static __rtsan::Context &GetContextImpl() { + if (context == nullptr) + InitializeContext(); + return *context; +} -void __rtsan::Context::RealtimePop() { realtime_depth_--; } +void __rtsan::Context::RealtimePush() { + __sanitizer::SpinMutexLock lock{&spin_mutex_}; + depths_[pthread_self()].realtime++; +} -void __rtsan::Context::BypassPush() { bypass_depth_++; } +void __rtsan::Context::RealtimePop() { + __sanitizer::SpinMutexLock lock{&spin_mutex_}; + depths_[pthread_self()].realtime--; +} -void __rtsan::Context::BypassPop() { bypass_depth_--; } +void __rtsan::Context::BypassPush() { + __sanitizer::SpinMutexLock lock{&spin_mutex_}; + depths_[pthread_self()].bypass++; +} -bool __rtsan::Context::InRealtimeContext() const { return realtime_depth_ > 0; } +void __rtsan::Context::BypassPop() { + __sanitizer::SpinMutexLock lock{&spin_mutex_}; + depths_[pthread_self()].bypass--; +} -bool __rtsan::Context::IsBypassed() const { return bypass_depth_ > 0; } +bool __rtsan::Context::InRealtimeContext() const { + __sanitizer::SpinMutexLock lock{&spin_mutex_}; + return depths_.lookup(pthread_self()).realtime > 0; +} -Context &__rtsan::GetContextForThisThread() { - return GetContextForThisThreadImpl(); +bool __rtsan::Context::IsBypassed() const { + __sanitizer::SpinMutexLock lock{&spin_mutex_}; + return depths_.lookup(pthread_self()).bypass > 0; } + +Context &__rtsan::GetContext() { return GetContextImpl(); } diff --git a/compiler-rt/lib/rtsan/rtsan_context.h b/compiler-rt/lib/rtsan/rtsan_context.h index 604b0dcc7c75e..a8d97f61d38b4 100644 --- a/compiler-rt/lib/rtsan/rtsan_context.h +++ b/compiler-rt/lib/rtsan/rtsan_context.h @@ -10,6 +10,10 @@ #pragma once +#include "sanitizer_common/sanitizer_dense_map.h" +#include "sanitizer_common/sanitizer_mutex.h" +#include + namespace __rtsan { class Context { @@ -23,9 +27,17 @@ class Context { bool InRealtimeContext() const; bool IsBypassed() const; + static Context &get(); + private: - int realtime_depth_{0}; - int bypass_depth_{0}; + static constexpr int max_concurrent_threads_{4096}; + struct Depth { + int realtime{0}; + int bypass{0}; + }; + + __sanitizer::DenseMap depths_{max_concurrent_threads_}; + mutable __sanitizer::SpinMutex spin_mutex_; }; class ScopedBypass { @@ -45,5 +57,5 @@ class ScopedBypass { Context &context_; }; -Context &GetContextForThisThread(); +Context &GetContext(); } // namespace __rtsan diff --git a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp index 390f03670ae6f..b9ce70e02fc8a 100644 --- a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp +++ b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp @@ -720,6 +720,11 @@ INTERCEPTOR(void, _os_nospin_lock_lock, _os_nospin_lock_t lock) { __rtsan_notify_intercepted_call("_os_nospin_lock_lock"); return REAL(_os_nospin_lock_lock)(lock); } + +INTERCEPTOR(void, _os_nospin_lock_unlock, _os_nospin_lock_t lock) { + __rtsan_notify_intercepted_call("_os_nospin_lock_unlock"); + return REAL(_os_nospin_lock_unlock)(lock); +} #pragma clang diagnostic pop // "-Wdeprecated-declarations" #endif // SANITIZER_APPLE @@ -729,10 +734,10 @@ INTERCEPTOR(void, os_unfair_lock_lock, os_unfair_lock_t lock) { return REAL(os_unfair_lock_lock)(lock); } -//INTERCEPTOR(void, os_unfair_lock_unlock, os_unfair_lock_t lock) { -// __rtsan_notify_intercepted_call("os_unfair_lock_unlock"); -// return REAL(os_unfair_lock_unlock)(lock); -//} +INTERCEPTOR(void, os_unfair_lock_unlock, os_unfair_lock_t lock) { + __rtsan_notify_intercepted_call("os_unfair_lock_unlock"); + return REAL(os_unfair_lock_unlock)(lock); +} #define RTSAN_MAYBE_INTERCEPT_OS_UNFAIR_LOCK_LOCK \ INTERCEPT_FUNCTION(os_unfair_lock_lock) #else diff --git a/compiler-rt/lib/rtsan/tests/rtsan_test_context.cpp b/compiler-rt/lib/rtsan/tests/rtsan_test_context.cpp index 2b6f53b4f572d..3295ae3b639d5 100644 --- a/compiler-rt/lib/rtsan/tests/rtsan_test_context.cpp +++ b/compiler-rt/lib/rtsan/tests/rtsan_test_context.cpp @@ -13,7 +13,10 @@ #include "rtsan/rtsan.h" #include "rtsan/rtsan_context.h" +#include +#include #include +#include using namespace __rtsan; using namespace ::testing; @@ -96,3 +99,63 @@ TEST_F(TestRtsanContext, BypassedStateIsStatefullyTracked) { context.BypassPush(); // depth 1 ExpectBypassed(true); } + +TEST_F(TestRtsanContext, IsProbablyThreadSafe) { + std::atomic num_threads_started{0}; + std::atomic all_threads_wait{true}; + std::atomic all_threads_continue{true}; + Context context{}; + + auto const expect_context_state = + [&context](bool expected_in_realtime_context, bool expected_is_bypassed) { + EXPECT_THAT(context.InRealtimeContext(), + Eq(expected_in_realtime_context)); + EXPECT_THAT(context.IsBypassed(), Eq(expected_is_bypassed)); + }; + + auto const test_thread_work = [&]() { + num_threads_started.fetch_add(1); + while (all_threads_wait.load()) + std::this_thread::yield(); + + while (all_threads_continue.load()) { + context.RealtimePush(); + expect_context_state(true, false); + context.RealtimePush(); + expect_context_state(true, false); + + context.BypassPush(); + expect_context_state(true, true); + context.BypassPop(); + expect_context_state(true, false); + + context.RealtimePop(); + expect_context_state(true, false); + context.RealtimePop(); + expect_context_state(false, false); + } + }; + + auto const time_now = []() { return std::chrono::steady_clock::now(); }; + + int const num_threads = 32; + std::vector test_threads{}; + auto const start_time = std::chrono::steady_clock::now(); + for (int n = 0; n < num_threads; ++n) + test_threads.push_back(std::thread(test_thread_work)); + + std::chrono::duration timeout = std::chrono::milliseconds(100); + while (num_threads_started.load() != num_threads) { + if ((time_now() - start_time) > timeout) { + FAIL(); + } + std::this_thread::yield(); + } + + all_threads_wait.store(false); + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + all_threads_continue.store(false); + + for (auto &test_thread : test_threads) + test_thread.join(); +} diff --git a/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp b/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp index 8cb081e054b75..d659a13ec7eae 100644 --- a/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp +++ b/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp @@ -1202,6 +1202,7 @@ void OSSpinLockLock(volatile OSSpinLock *__lock); // _os_nospin_lock_lock may replace OSSpinLockLock due to deprecation macro. typedef volatile OSSpinLock *_os_nospin_lock_t; void _os_nospin_lock_lock(_os_nospin_lock_t lock); +void _os_nospin_lock_unlock(_os_nospin_lock_t lock); } #pragma clang diagnostic pop //"-Wdeprecated-declarations" @@ -1221,6 +1222,20 @@ TEST(TestRtsanInterceptors, OsNoSpinLockLockDiesWhenRealtime) { ExpectNonRealtimeSurvival(Func); } +TEST(TestRtsanInterceptors, OsNoSpinLockUnlockDiesWhenRealtime) { + auto Func = [&]() { + OSSpinLock lock{}; + { + __rtsan_disable(); + _os_nospin_lock_lock(&lock); + __rtsan_enable(); + } + _os_nospin_lock_unlock(&lock); + }; + ExpectRealtimeDeath(Func, "_os_nospin_lock_unlock"); + ExpectNonRealtimeSurvival(Func); +} + TEST(TestRtsanInterceptors, OsUnfairLockLockDiesWhenRealtime) { auto Func = []() { os_unfair_lock_s unfair_lock{}; From b5becb1b51212c48a95b87bab04791c771f7549e Mon Sep 17 00:00:00 2001 From: David Trevelyan Date: Tue, 29 Jul 2025 15:33:32 +0100 Subject: [PATCH 6/6] Clean up Context entries on pop if possible --- compiler-rt/lib/rtsan/rtsan_context.cpp | 13 +++++++++++-- compiler-rt/lib/rtsan/rtsan_context.h | 12 ++++++++++-- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/compiler-rt/lib/rtsan/rtsan_context.cpp b/compiler-rt/lib/rtsan/rtsan_context.cpp index be33d8c335a1c..abf686a965d49 100644 --- a/compiler-rt/lib/rtsan/rtsan_context.cpp +++ b/compiler-rt/lib/rtsan/rtsan_context.cpp @@ -41,7 +41,9 @@ void __rtsan::Context::RealtimePush() { void __rtsan::Context::RealtimePop() { __sanitizer::SpinMutexLock lock{&spin_mutex_}; - depths_[pthread_self()].realtime--; + pthread_t const thread_id = pthread_self(); + depths_[thread_id].realtime--; + MaybeCleanup(thread_id); } void __rtsan::Context::BypassPush() { @@ -51,7 +53,9 @@ void __rtsan::Context::BypassPush() { void __rtsan::Context::BypassPop() { __sanitizer::SpinMutexLock lock{&spin_mutex_}; - depths_[pthread_self()].bypass--; + pthread_t const thread_id = pthread_self(); + depths_[thread_id].bypass--; + MaybeCleanup(thread_id); } bool __rtsan::Context::InRealtimeContext() const { @@ -64,4 +68,9 @@ bool __rtsan::Context::IsBypassed() const { return depths_.lookup(pthread_self()).bypass > 0; } +void __rtsan::Context::MaybeCleanup(pthread_t thread_id) { + if (depths_[thread_id] == Depth{}) + depths_.erase(thread_id); +} + Context &__rtsan::GetContext() { return GetContextImpl(); } diff --git a/compiler-rt/lib/rtsan/rtsan_context.h b/compiler-rt/lib/rtsan/rtsan_context.h index a8d97f61d38b4..78b83f5551950 100644 --- a/compiler-rt/lib/rtsan/rtsan_context.h +++ b/compiler-rt/lib/rtsan/rtsan_context.h @@ -27,15 +27,23 @@ class Context { bool InRealtimeContext() const; bool IsBypassed() const; - static Context &get(); - private: static constexpr int max_concurrent_threads_{4096}; struct Depth { int realtime{0}; int bypass{0}; + bool operator==(Depth const &other) const { + return realtime == other.realtime && bypass == other.bypass; + } }; + void MaybeCleanup(pthread_t thread_id); + + // This map serves as thread-local storage implemented entirely in user space. + // If an OS's implementation of pthread tls initialisation calls one of the + // intercepted functions in rtsan, an infinite recursion can occur when trying + // to initialize TLS for a thread. Using this user-space TLS avoids the problem + // entirely. __sanitizer::DenseMap depths_{max_concurrent_threads_}; mutable __sanitizer::SpinMutex spin_mutex_; };