https://github.com/thetruestblue created https://github.com/llvm/llvm-project/pull/137353
This cherry pick updates the OS Spin Lock logic to fix to intercept both OS Spin Lock and os_nospin_lock which needs to be intercepted due to macro replacement of OSSpinLock within atomic_load compiler_rt call. This is causing a test to fail in release bot: https://green.lab.llvm.org/job/llvm.org/job/clang-stage1-RA-release-branch/205/testReport/ rdar://145488759 >From 2453cd24fd5fb9778b55155f77bf9d05a49539f9 Mon Sep 17 00:00:00 2001 From: davidtrevelyan <davidtrevel...@users.noreply.github.com> Date: Thu, 13 Mar 2025 10:18:25 +0000 Subject: [PATCH 1/2] [rtsan][Apple] Add interceptor for _os_nospin_lock_lock (#131034) Follows the discussion here: https://github.com/llvm/llvm-project/pull/129309 Recently, the test `TestRtsan.AccessingALargeAtomicVariableDiesWhenRealtime` has been failing on newer MacOS versions, because the internal locking mechanism in `std::atomic<T>::load` (for types `T` that are larger than the hardware lock-free limit), has changed to a function that wasn't being intercepted by rtsan. This PR introduces an interceptor for `_os_nospin_lock_lock`, which is the new internal locking mechanism. _Note: we'd probably do well to introduce interceptors for `_os_nospin_lock_unlock` (and `os_unfair_lock_unlock`) too, which also appear to have blocking implementations. This can follow in a separate PR._ (cherry picked from commit 481a55a3d9645a6bc1540d326319b78ad8ed8db1) --- .../lib/rtsan/rtsan_interceptors_posix.cpp | 11 +++++++++++ .../tests/rtsan_test_interceptors_posix.cpp | 19 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp index 6816119065263..4d602a88ba9ae 100644 --- a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp +++ b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp @@ -30,6 +30,12 @@ extern "C" { typedef int32_t OSSpinLock; void OSSpinLockLock(volatile OSSpinLock *__lock); +// A pointer to this type is in the interface for `_os_nospin_lock_lock`, but +// it's an internal implementation detail of `os/lock.c` on Darwin, and +// therefore not available in any headers. As a workaround, we forward declare +// it here, which is enough to facilitate interception of _os_nospin_lock_lock. +struct _os_nospin_lock_s; +using _os_nospin_lock_t = _os_nospin_lock_s *; } #endif // TARGET_OS_MAC @@ -642,6 +648,11 @@ INTERCEPTOR(void, os_unfair_lock_lock, os_unfair_lock_t lock) { __rtsan_notify_intercepted_call("os_unfair_lock_lock"); return REAL(os_unfair_lock_lock)(lock); } + +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); +} #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_interceptors_posix.cpp b/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp index 59663776366bb..75f723081c4b6 100644 --- a/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp +++ b/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp @@ -1058,6 +1058,25 @@ TEST(TestRtsanInterceptors, OsUnfairLockLockDiesWhenRealtime) { ExpectRealtimeDeath(Func, "os_unfair_lock_lock"); ExpectNonRealtimeSurvival(Func); } + +// We intercept _os_nospin_lock_lock because it's the internal +// locking mechanism for MacOS's atomic implementation for data +// types that are larger than the hardware's maximum lock-free size. +// However, it's a private implementation detail and not visible in any headers, +// so we must duplicate the required type definitions to forward declaration +// what we need here. +extern "C" { +struct _os_nospin_lock_s { + unsigned int oul_value; +}; +void _os_nospin_lock_lock(_os_nospin_lock_s *); +} +TEST(TestRtsanInterceptors, OsNoSpinLockLockDiesWhenRealtime) { + _os_nospin_lock_s lock{}; + auto Func = [&]() { _os_nospin_lock_lock(&lock); }; + ExpectRealtimeDeath(Func, "_os_nospin_lock_lock"); + ExpectNonRealtimeSurvival(Func); +} #endif #if SANITIZER_LINUX >From e2a5aa2526b06497869514287d26ea4732a14b95 Mon Sep 17 00:00:00 2001 From: thetruestblue <bbluecon...@gmail.com> Date: Fri, 18 Apr 2025 11:25:31 -0700 Subject: [PATCH 2/2] [RTSan][Darwin] Adjust OSSpinLock/_os_nospin_lock interceptor and tests (#132867) These changes align with these lock types and allows builds and tests to pass with various SDKS. rdar://147067322 (cherry picked from commit 7cc4472037b43971bd3ee373fe75b5043f5abca9) --- .../lib/rtsan/rtsan_interceptors_posix.cpp | 37 +++++++---------- .../tests/rtsan_test_interceptors_posix.cpp | 40 +++++++++---------- 2 files changed, 32 insertions(+), 45 deletions(-) diff --git a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp index 4d602a88ba9ae..040f501ee52e9 100644 --- a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp +++ b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp @@ -21,24 +21,6 @@ #include "rtsan/rtsan.h" #if SANITIZER_APPLE - -#if TARGET_OS_MAC -// On MacOS OSSpinLockLock is deprecated and no longer present in the headers, -// but the symbol still exists on the system. Forward declare here so we -// don't get compilation errors. -#include <stdint.h> -extern "C" { -typedef int32_t OSSpinLock; -void OSSpinLockLock(volatile OSSpinLock *__lock); -// A pointer to this type is in the interface for `_os_nospin_lock_lock`, but -// it's an internal implementation detail of `os/lock.c` on Darwin, and -// therefore not available in any headers. As a workaround, we forward declare -// it here, which is enough to facilitate interception of _os_nospin_lock_lock. -struct _os_nospin_lock_s; -using _os_nospin_lock_t = _os_nospin_lock_s *; -} -#endif // TARGET_OS_MAC - #include <libkern/OSAtomic.h> #include <os/lock.h> #endif // SANITIZER_APPLE @@ -633,26 +615,35 @@ INTERCEPTOR(mode_t, umask, mode_t cmask) { #pragma clang diagnostic push // OSSpinLockLock is deprecated, but still in use in libc++ #pragma clang diagnostic ignored "-Wdeprecated-declarations" +#undef OSSpinLockLock + INTERCEPTOR(void, OSSpinLockLock, volatile OSSpinLock *lock) { __rtsan_notify_intercepted_call("OSSpinLockLock"); return REAL(OSSpinLockLock)(lock); } -#pragma clang diagnostic pop + #define RTSAN_MAYBE_INTERCEPT_OSSPINLOCKLOCK INTERCEPT_FUNCTION(OSSpinLockLock) #else #define RTSAN_MAYBE_INTERCEPT_OSSPINLOCKLOCK #endif // SANITIZER_APPLE #if SANITIZER_APPLE -INTERCEPTOR(void, os_unfair_lock_lock, os_unfair_lock_t lock) { - __rtsan_notify_intercepted_call("os_unfair_lock_lock"); - return REAL(os_unfair_lock_lock)(lock); -} +// _os_nospin_lock_lock may replace OSSpinLockLock due to deprecation macro. +typedef volatile OSSpinLock *_os_nospin_lock_t; 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); } +#pragma clang diagnostic pop // "-Wdeprecated-declarations" +#endif // SANITIZER_APPLE + +#if SANITIZER_APPLE +INTERCEPTOR(void, os_unfair_lock_lock, os_unfair_lock_t lock) { + __rtsan_notify_intercepted_call("os_unfair_lock_lock"); + return REAL(os_unfair_lock_lock)(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_interceptors_posix.cpp b/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp index 75f723081c4b6..7eda884951c83 100644 --- a/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp +++ b/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp @@ -1036,10 +1036,18 @@ TEST(TestRtsanInterceptors, PthreadJoinDiesWhenRealtime) { } #if SANITIZER_APPLE - #pragma clang diagnostic push // OSSpinLockLock is deprecated, but still in use in libc++ #pragma clang diagnostic ignored "-Wdeprecated-declarations" +#undef OSSpinLockLock +extern "C" { +typedef int32_t OSSpinLock; +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); +} + TEST(TestRtsanInterceptors, OsSpinLockLockDiesWhenRealtime) { auto Func = []() { OSSpinLock spin_lock{}; @@ -1048,7 +1056,14 @@ TEST(TestRtsanInterceptors, OsSpinLockLockDiesWhenRealtime) { ExpectRealtimeDeath(Func, "OSSpinLockLock"); ExpectNonRealtimeSurvival(Func); } -#pragma clang diagnostic pop + +TEST(TestRtsanInterceptors, OsNoSpinLockLockDiesWhenRealtime) { + OSSpinLock lock{}; + auto Func = [&]() { _os_nospin_lock_lock(&lock); }; + ExpectRealtimeDeath(Func, "_os_nospin_lock_lock"); + ExpectNonRealtimeSurvival(Func); +} +#pragma clang diagnostic pop //"-Wdeprecated-declarations" TEST(TestRtsanInterceptors, OsUnfairLockLockDiesWhenRealtime) { auto Func = []() { @@ -1058,26 +1073,7 @@ TEST(TestRtsanInterceptors, OsUnfairLockLockDiesWhenRealtime) { ExpectRealtimeDeath(Func, "os_unfair_lock_lock"); ExpectNonRealtimeSurvival(Func); } - -// We intercept _os_nospin_lock_lock because it's the internal -// locking mechanism for MacOS's atomic implementation for data -// types that are larger than the hardware's maximum lock-free size. -// However, it's a private implementation detail and not visible in any headers, -// so we must duplicate the required type definitions to forward declaration -// what we need here. -extern "C" { -struct _os_nospin_lock_s { - unsigned int oul_value; -}; -void _os_nospin_lock_lock(_os_nospin_lock_s *); -} -TEST(TestRtsanInterceptors, OsNoSpinLockLockDiesWhenRealtime) { - _os_nospin_lock_s lock{}; - auto Func = [&]() { _os_nospin_lock_lock(&lock); }; - ExpectRealtimeDeath(Func, "_os_nospin_lock_lock"); - ExpectNonRealtimeSurvival(Func); -} -#endif +#endif // SANITIZER_APPLE #if SANITIZER_LINUX TEST(TestRtsanInterceptors, SpinLockLockDiesWhenRealtime) { _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
