[Apple][RTSan] Realtime sanitizers are failing in Apple CI #124873
Conversation
The two tests commented out here are failing apple internal builds. These seems specific to macOS, but I am not familiar with how to disable these gtests only in our downstream. And am curious if there's a way we can help you test this? rdar://142496236 rdar://142500404
|
@llvm/pr-subscribers-compiler-rt-sanitizer Author: None (thetruestblue) ChangesThe two tests commented out here are failing apple internal builds. These seems specific to macOS, but I am not familiar with how to disable these gtests only in our downstream. And am curious if there's a way we can help you test this? rdar://142496236 Full diff: https://github.com/llvm/llvm-project/pull/124873.diff 2 Files Affected:
diff --git a/compiler-rt/lib/rtsan/tests/rtsan_test_functional.cpp b/compiler-rt/lib/rtsan/tests/rtsan_test_functional.cpp
index e05d7ae78b5d99..c6664683f19a57 100644
--- a/compiler-rt/lib/rtsan/tests/rtsan_test_functional.cpp
+++ b/compiler-rt/lib/rtsan/tests/rtsan_test_functional.cpp
@@ -170,23 +170,23 @@ TEST(TestRtsan, CopyingALambdaWithLargeCaptureDiesWhenRealtime) {
ExpectNonRealtimeSurvival(Func);
}
-TEST(TestRtsan, AccessingALargeAtomicVariableDiesWhenRealtime) {
- std::atomic<float> small_atomic{0.0f};
- ASSERT_TRUE(small_atomic.is_lock_free());
- RealtimeInvoke([&small_atomic]() {
- float x = small_atomic.load();
- return x;
- });
-
- std::atomic<std::array<float, 2048>> large_atomic;
- ASSERT_FALSE(large_atomic.is_lock_free());
- auto Func = [&]() {
- std::array<float, 2048> x = large_atomic.load();
- return x;
- };
- ExpectRealtimeDeath(Func);
- ExpectNonRealtimeSurvival(Func);
-}
+// TEST(TestRtsan, AccessingALargeAtomicVariableDiesWhenRealtime) {
+// std::atomic<float> small_atomic{0.0f};
+// ASSERT_TRUE(small_atomic.is_lock_free());
+// RealtimeInvoke([&small_atomic]() {
+// float x = small_atomic.load();
+// return x;
+// });
+
+// std::atomic<std::array<float, 2048>> large_atomic;
+// ASSERT_FALSE(large_atomic.is_lock_free());
+// auto Func = [&]() {
+// std::array<float, 2048> x = large_atomic.load();
+// return x;
+// };
+// ExpectRealtimeDeath(Func);
+// ExpectNonRealtimeSurvival(Func);
+// }
TEST(TestRtsan, FirstCoutDiesWhenRealtime) {
auto Func = []() { std::cout << "Hello, world!" << std::endl; };
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 59663776366bb3..05044f84dd0eca 100644
--- a/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp
+++ b/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp
@@ -1037,18 +1037,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"
-TEST(TestRtsanInterceptors, OsSpinLockLockDiesWhenRealtime) {
- auto Func = []() {
- OSSpinLock spin_lock{};
- OSSpinLockLock(&spin_lock);
- };
- ExpectRealtimeDeath(Func, "OSSpinLockLock");
- ExpectNonRealtimeSurvival(Func);
-}
-#pragma clang diagnostic pop
+// #pragma clang diagnostic push
+// // OSSpinLockLock is deprecated, but still in use in libc++
+// #pragma clang diagnostic ignored "-Wdeprecated-declarations"
+// TEST(TestRtsanInterceptors, OsSpinLockLockDiesWhenRealtime) {
+// auto Func = []() {
+// OSSpinLock spin_lock{};
+// OSSpinLockLock(&spin_lock);
+// };
+// ExpectRealtimeDeath(Func, "OSSpinLockLock");
+// ExpectNonRealtimeSurvival(Func);
+// }
+// #pragma clang diagnostic pop
TEST(TestRtsanInterceptors, OsUnfairLockLockDiesWhenRealtime) {
auto Func = []() {
|
|
Hi @thetruestblue! Thanks for reporting this! Happy to help figure out what is going on. Would you post the test output of the failing tests for us to look at? Also, is this a new failure? Do you know the commit that caused it? |
The two usual approaches are https://google.github.io/googletest/advanced.html#skipping-test-execution |
| // #pragma clang diagnostic push | ||
| // // OSSpinLockLock is deprecated, but still in use in libc++ | ||
| // #pragma clang diagnostic ignored "-Wdeprecated-declarations" | ||
| // TEST(TestRtsanInterceptors, OsSpinLockLockDiesWhenRealtime) { |
There was a problem hiding this comment.
For this test, is it possible that your internal build of MacOS completely killed off OsSpinLock, and this call no longer does anything? That's my very naive hunch seeing as it's wrapped in in -Wno-deprecated-declarations
There was a problem hiding this comment.
Ah, I think I have an explanation for this one. The locks were changed to macros that point to other locking mechanisms
See: a34159f
| // }); | ||
|
|
||
| // std::atomic<std::array<float, 2048>> large_atomic; | ||
| // ASSERT_FALSE(large_atomic.is_lock_free()); |
There was a problem hiding this comment.
To explain what is going on here (just being explicit so you have as much info as possible, you may know any/all of this).
Small types which are made atomics don't need locks, they rely on arch specific instructions. Luckily for us real-time programmers, C++ standard added the .is_lock_free assertion so we could sanity check ourselves before using these in timing critical code.
The first half of these test determines that a basic float may be used atomically without an underlying lock happening, we don't intercept anything and don't die.
The second half test uses a mega 2048 float buffer, which cannot be atomically updated without a lock (on any arch I know of). We then test that we intercept the lock call that happens implicitly under the hood. On my mac this intercepts a pthread_mutex_lock I believe.
This passes on my mac, at least, but perhaps Apple changed the internal implementation of the STL to use a different type of lock we don't currently intercept? That fix would be easy enough to do, we could just add the new call into our interceptors.
If you have found some way to make an atomic update of 2048 floats at once with NO LOCKS, please share the secret sauce 👀
both test failures just fail to die. I can try to look for a commit to blame, but primary scanning shows it's been failing for a while. |
|
Ah!
|
Hmm, that's strange!! This should be a symbol available in the C++ runtime: https://en.cppreference.com/w/cpp/atomic/atomic/is_lock_free I just pulled latest master and ran on my mac and things are green there. Let me know if you have any more leads, happy to provide any more assistance. |
|
Would you elaborate on what was going wrong for future info? I'm curious! |
To @jroelofs point I just skipped these internally for now. I think the OSSpinLock behavior is likely explained by how you're forward declaring the symbol, and how there are situations where we are replacing these with a macro, our TSan tests started failing in the same way recently (I pointed you to the TSan fix). I don't have a good explanation for the ___atomic_is_lock_free not being available at this point (though in one of our jobs this fails without that error). I'm going to schedule some time to look into this, and I'll put up a PR when I have a chance to actually investigate. |
4 participants
The two tests commented out here are failing apple internal builds. I plan on disabling these for now in Apple's fork. But am curious if there's a way we can help you test this?
rdar://142496236
rdar://142500404