[llvm-branch-commits] [compiler-rt] [RTSan] Cherry pick rtsan osspinlock fix to release/20.x (PR #137353)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Tue Apr 29 10:09:47 PDT 2025
https://github.com/thetruestblue updated https://github.com/llvm/llvm-project/pull/137353
>From 2453cd24fd5fb9778b55155f77bf9d05a49539f9 Mon Sep 17 00:00:00 2001
From: davidtrevelyan <davidtrevelyan at 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 <bblueconway at 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) {
More information about the llvm-branch-commits
mailing list