[rtsan][Apple] Add interceptor for _os_nospin_lock_lock #131034

davidtrevelyan · 2025-03-12T21:25:58Z

Follows the discussion here: #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.

llvmbot · 2025-03-12T21:26:40Z

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: None (davidtrevelyan)

Changes

Follows the discussion here: #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.

Full diff: https://github.com/llvm/llvm-project/pull/131034.diff

2 Files Affected:

(modified) compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp (+11)
(modified) compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp (+19)

diff --git a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
index cd57107450a47..5bbefa3ce43f1 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
 
@@ -711,6 +717,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 47bcafff51a4d..9ef6887a9c848 100644
--- a/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp
+++ b/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp
@@ -1122,6 +1122,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

cjappl

LGTM thanks for your legwork on this one

cjappl · 2025-03-12T22:01:40Z

Also if this is merged and tests are happy, we may consider cherry picking to the llvm 20 release branch as this is "expected behavior" for that version.

It would be good to get in the next release if we feel good about it.

thetruestblue · 2025-03-12T22:16:24Z

Thanks to both of you. Note for your future development that these locks come in bundles, as do unfair locks:
os_nospin_lock* : lock, unlock, & try_lock

thetruestblue · 2025-03-14T19:04:21Z

In a follow-up you can you please wrap this with:

#  pragma clang diagnostic push
#  pragma clang diagnostic ignored "-Wdeprecated-declarations"

…#131034)" This reverts commit 481a55a. This change breaks compilation using Apple's internal SDK. This is a temporary revert while we figure out the correct path forward. Reverting upstream doesn't seem like the right thing to do given they don't have access to the internal SDK. rdar://147018451

davidtrevelyan · 2025-03-14T20:24:55Z

@thetruestblue sure - no problem. Which section(s) of code in this patch are you referring to wrapping here? If you'd find it easier to submit a PR to describe this, I'd be very happy to take a look.

@delcypher are you by any chance able to share details about how this broke your build?

delcypher · 2025-03-14T20:59:56Z

@davidtrevelyan Unfortunately it fails in a way that you won't be able to reproduce because the build fails when using Apple's internal SDKs which are different from the public ones. The error looks like this. I've had to redact a few things.

/Volumes/user_data/xc/tmp/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.4.Redacted.sdk/usr/include/libkern/OSSpinLockDeprecated.h:185:30: error: type alias redefinition with different types ('volatile OSSpinLock *' (aka 'volatile int *') vs '_os_nospin_lock_s *')
<redacted>
                             ^
/Volumes/user_data/dev/llvm/llvm.org/main/src/llvm-project/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp:38:7: note: previous definition is here
using _os_nospin_lock_t = _os_nospin_lock_s *;
      ^
/Volumes/user_data/dev/llvm/llvm.org/main/src/llvm-project/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp:705:19: error: conflicting types for '_os_nospin_lock_lock'
INTERCEPTOR(void, OSSpinLockLock, volatile OSSpinLock *lock) {
                  ^
/Volumes/user_data/xc/tmp/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.4.Redacted.sdk/usr/include/libkern/OSSpinLockDeprecated.h:190:6: note: previous declaration is here
<redacted>
     ^
/Volumes/user_data/dev/llvm/llvm.org/main/src/llvm-project/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp:707:15: error: no matching function for call to '_os_nospin_lock_lock'
  return REAL(OSSpinLockLock)(lock);
         ~~~~~^~~~~~~~~~~~~~~~~~~~~
/Volumes/user_data/dev/llvm/llvm.org/main/src/llvm-project/compiler-rt/lib/rtsan/../interception/interception.h:266:18: note: expanded from macro 'REAL'
# define REAL(x) x
                 ^
/Volumes/user_data/xc/tmp/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.4.Redacted.sdk/usr/include/libkern/OSSpinLockDeprecated.h:192:30: note: expanded from macro 'OSSpinLockLock'
<redacted>
                             ^~~~~~~~~~~~~~~~~~~~
/Volumes/user_data/xc/tmp/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.4.Redacted.sdk/usr/include/libkern/OSSpinLockDeprecated.h:190:6: note: candidate function not viable: no known conversion from 'volatile OSSpinLock *' (aka 'volatile int *') to '_os_nospin_lock_t' (aka '_os_nospin_lock_s *') for 1st argument
<redacted>
     ^

delcypher · 2025-03-14T21:00:54Z

@davidtrevelyan @thetruestblue Doing this fixed the build for me using the internal SDK.

diff --git a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
index 5bbefa3ce43f..71e47f97f8c0 100644
--- a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
+++ b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
@@ -35,7 +35,7 @@ void OSSpinLockLock(volatile OSSpinLock *__lock);
 // 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 *;
+using _os_nospin_lock_t = volatile OSSpinLock *;
 }
 #endif // TARGET_OS_MAC

Follows the discussion here: llvm#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._

…#131034)" This reverts commit 481a55a.

wrotki · 2025-03-25T20:50:46Z

/cherry-pick 6f99abc

wrotki · 2025-03-25T20:51:34Z

Cherry-picked to LLVM 20, as the tests are still failing on LLVM 20 release bot

llvmbot · 2025-03-25T20:56:21Z

Failed to cherry-pick: 6f99abc

https://github.com/llvm/llvm-project/actions/runs/14069840627

Please manually backport the fix and push it to your github fork. Once this is done, please create a pull request

wrotki · 2025-03-25T21:18:22Z

/cherry-pick 481a55a

llvmbot · 2025-03-25T21:24:12Z

/pull-request #132997

…#131034)" This reverts commit 481a55a.

Follows the discussion here: llvm#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 481a55a)

llvmbot · 2025-04-29T17:27:33Z

Failed to cherry-pick: 6f99abc

https://github.com/llvm/llvm-project/actions/runs/14737412738

Please manually backport the fix and push it to your github fork. Once this is done, please create a pull request

Follows the discussion here: llvm#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 481a55a)

[rtsan][Apple] Add interceptor for _os_nospin_lock_lock

6f99abc

davidtrevelyan requested review from thetruestblue and cjappl March 12, 2025 21:26

llvmbot added compiler-rt compiler-rt:sanitizer labels Mar 12, 2025

davidtrevelyan mentioned this pull request Mar 12, 2025

[RTSan][Apple] Disable AccessingALargeAtomicVariableDiesWhenRealtime #129309

Closed

cjappl approved these changes Mar 12, 2025

View reviewed changes

davidtrevelyan merged commit 481a55a into llvm:main Mar 13, 2025
13 checks passed

davidtrevelyan deleted the fix/large-atomic-lock-interception branch March 13, 2025 10:18

Michael137 added a commit to Michael137/llvm-project that referenced this pull request Mar 18, 2025

Revert "[rtsan][Apple] Add interceptor for _os_nospin_lock_lock (llvm…

26dabe1

…#131034)" This reverts commit 481a55a.

wrotki added this to the LLVM 20.X Release milestone Mar 25, 2025

github-project-automation bot added this to LLVM Release Status Mar 25, 2025

github-project-automation bot moved this to Needs Triage in LLVM Release Status Mar 25, 2025

llvmbot added the release:cherry-pick-failed label Mar 25, 2025

llvmbot removed the release:cherry-pick-failed label Mar 25, 2025

llvmbot moved this from Needs Triage to Done in LLVM Release Status Mar 25, 2025

Michael137 added a commit to Michael137/llvm-project that referenced this pull request Mar 26, 2025

Revert "[rtsan][Apple] Add interceptor for _os_nospin_lock_lock (llvm…

c41ac8b

…#131034)" This reverts commit 481a55a.

Michael137 added a commit to Michael137/llvm-project that referenced this pull request Mar 27, 2025

Revert "[rtsan][Apple] Add interceptor for _os_nospin_lock_lock (llvm…

efbe166

…#131034)" This reverts commit 481a55a.

Michael137 added a commit to Michael137/llvm-project that referenced this pull request Apr 4, 2025

Revert "[rtsan][Apple] Add interceptor for _os_nospin_lock_lock (llvm…

31c1828

…#131034)" This reverts commit 481a55a.

Michael137 added a commit to Michael137/llvm-project that referenced this pull request Apr 7, 2025

Revert "[rtsan][Apple] Add interceptor for _os_nospin_lock_lock (llvm…

b2d46a4

…#131034)" This reverts commit 481a55a.

llvmbot added the release:cherry-pick-failed label Apr 29, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[rtsan][Apple] Add interceptor for _os_nospin_lock_lock #131034

[rtsan][Apple] Add interceptor for _os_nospin_lock_lock #131034

Uh oh!

davidtrevelyan commented Mar 12, 2025 •

edited

Loading

Uh oh!

llvmbot commented Mar 12, 2025

Uh oh!

cjappl left a comment

Uh oh!

cjappl commented Mar 12, 2025

Uh oh!

thetruestblue commented Mar 12, 2025 •

edited

Loading

Uh oh!

Uh oh!

thetruestblue commented Mar 14, 2025 •

edited

Loading

Uh oh!

davidtrevelyan commented Mar 14, 2025

Uh oh!

delcypher commented Mar 14, 2025

Uh oh!

delcypher commented Mar 14, 2025

Uh oh!

wrotki commented Mar 25, 2025

Uh oh!

wrotki commented Mar 25, 2025

Uh oh!

llvmbot commented Mar 25, 2025

Uh oh!

wrotki commented Mar 25, 2025

Uh oh!

llvmbot commented Mar 25, 2025

Uh oh!

llvmbot commented Apr 29, 2025

Uh oh!

Uh oh!

[rtsan][Apple] Add interceptor for _os_nospin_lock_lock #131034

[rtsan][Apple] Add interceptor for _os_nospin_lock_lock #131034

Uh oh!

Conversation

davidtrevelyan commented Mar 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Mar 12, 2025

Uh oh!

cjappl left a comment

Choose a reason for hiding this comment

Uh oh!

cjappl commented Mar 12, 2025

Uh oh!

thetruestblue commented Mar 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

thetruestblue commented Mar 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davidtrevelyan commented Mar 14, 2025

Uh oh!

delcypher commented Mar 14, 2025

Uh oh!

delcypher commented Mar 14, 2025

Uh oh!

wrotki commented Mar 25, 2025

Uh oh!

wrotki commented Mar 25, 2025

Uh oh!

llvmbot commented Mar 25, 2025

Uh oh!

wrotki commented Mar 25, 2025

Uh oh!

llvmbot commented Mar 25, 2025

Uh oh!

llvmbot commented Apr 29, 2025

Uh oh!

Uh oh!

davidtrevelyan commented Mar 12, 2025 •

edited

Loading

thetruestblue commented Mar 12, 2025 •

edited

Loading

thetruestblue commented Mar 14, 2025 •

edited

Loading