Skip to content

Conversation

@thetruestblue
Copy link
Contributor

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

@thetruestblue thetruestblue changed the title [RTSan] Cherry pick rtsan osspinlock [RTSan] Cherry pick rtsan osspinlock to release/20.x Apr 25, 2025
@thetruestblue thetruestblue changed the title [RTSan] Cherry pick rtsan osspinlock to release/20.x [RTSan] Cherry pick rtsan osspinlock fix to release/20.x Apr 25, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 25, 2025

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

Author: None (thetruestblue)

Changes

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


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

2 Files Affected:

  • (modified) compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp (+15-13)
  • (modified) compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp (+18-3)
diff --git a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
index 6816119065263..040f501ee52e9 100644
--- a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
+++ b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
@@ -21,18 +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);
-}
-#endif // TARGET_OS_MAC
-
 #include <libkern/OSAtomic.h>
 #include <os/lock.h>
 #endif // SANITIZER_APPLE
@@ -627,21 +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
+// _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 59663776366bb..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,7 +1073,7 @@ TEST(TestRtsanInterceptors, OsUnfairLockLockDiesWhenRealtime) {
   ExpectRealtimeDeath(Func, "os_unfair_lock_lock");
   ExpectNonRealtimeSurvival(Func);
 }
-#endif
+#endif // SANITIZER_APPLE
 
 #if SANITIZER_LINUX
 TEST(TestRtsanInterceptors, SpinLockLockDiesWhenRealtime) {

@thetruestblue thetruestblue added this to the LLVM 20.X Release milestone Apr 29, 2025
@github-project-automation github-project-automation bot moved this to Needs Triage in LLVM Release Status Apr 29, 2025
@thetruestblue thetruestblue deleted the cherry-pick/rtsan-osspinlock branch April 29, 2025 18:34
@thetruestblue thetruestblue restored the cherry-pick/rtsan-osspinlock branch April 29, 2025 18:35
@thetruestblue thetruestblue reopened this Apr 29, 2025
@davidtrevelyan
Copy link
Contributor

Hi all (@thetruestblue @wrotki @aemerson) - is there anything further you need from me or @cjappl for this one? Let us know if there's anything we can do.

@tstellar tstellar moved this from Needs Triage to Needs Review in LLVM Release Status May 10, 2025
@tstellar tstellar moved this from Needs Review to Needs Merge in LLVM Release Status May 10, 2025
davidtrevelyan and others added 2 commits May 10, 2025 11:01
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)
…ts (llvm#132867)

These changes align with these lock types and allows builds and tests to
pass with various SDKS.

rdar://147067322
(cherry picked from commit 7cc4472)
@tstellar tstellar force-pushed the cherry-pick/rtsan-osspinlock branch from c419514 to b7b834e Compare May 10, 2025 18:02
@tstellar tstellar merged commit b7b834e into llvm:release/20.x May 10, 2025
6 of 9 checks passed
@github-project-automation github-project-automation bot moved this from Needs Merge to Done in LLVM Release Status May 10, 2025
@github-actions
Copy link

@thetruestblue (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

5 participants