[rtsan][compiler-rt] Prevent UB hang in rtsan lock unit tests

[cjappl]

It is undefined behavior to lock or unlock an uninitialized lock, and unlock a lock which isn't locked.

Later in one of my dev branches this was hanging on ubuntu.

This review introduces a fixture to set up and tear down the locks where appropriate, and separates them into two tests (realtime death and non realtime survival) so each test is guaranteed a fresh lock.

[llvmbot]

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

Author: Chris Apple (cjappl)

Changes

It is undefined behavior to lock or unlock an uninitialized lock, and unlock a lock which isn't locked.

Later in one of my dev branches this was hanging on ubuntu.

This review introduces a fixture to set up and tear down the locks where appropriate, and separates them into two tests (realtime death and non realtime survival) so each test is guaranteed a fresh lock.

---

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

1 Files Affected:

• (modified) compiler-rt/lib/rtsan/tests/rtsan_test_interceptors.cpp (+107-23)

diff --git a/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors.cpp b/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors.cpp
index f5b016089087df..966e5514062881 100644
--- a/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors.cpp
+++ b/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors.cpp
@@ -328,26 +328,64 @@ TEST(TestRtsanInterceptors, PthreadCreateDiesWhenRealtime) {
   ExpectNonRealtimeSurvival(Func);
 }
 
-TEST(TestRtsanInterceptors, PthreadMutexLockDiesWhenRealtime) {
-  auto Func = []() {
-    pthread_mutex_t mutex{};
+class PthreadMutexLockTest : public ::testing::Test {
+protected:
+  void SetUp() override {
+    pthread_mutex_init(&mutex, nullptr);
+    is_locked = false;
+  }
+
+  void TearDown() override {
+    if (is_locked) {
+      Unlock();
+    }
+    pthread_mutex_destroy(&mutex);
+  }
+
+  void Lock() {
+    ASSERT_TRUE(!is_locked);
     pthread_mutex_lock(&mutex);
-  };
+    is_locked = true;
+  }
+
+  void Unlock() {
+    ASSERT_TRUE(is_locked);
+    pthread_mutex_unlock(&mutex);
+    is_locked = false;
+  }
+
+private:
+  pthread_mutex_t mutex;
+  bool is_locked;
+};
+
+TEST_F(PthreadMutexLockTest, PthreadMutexLockDiesWhenRealtime) {
+  auto Func = [this]() { Lock(); };
 
   ExpectRealtimeDeath(Func, "pthread_mutex_lock");
+}
+
+TEST_F(PthreadMutexLockTest, PthreadMutexLockSurvivesWhenNotRealtime) {
+  auto Func = [this]() { Lock(); };
+
   ExpectNonRealtimeSurvival(Func);
 }
 
-TEST(TestRtsanInterceptors, PthreadMutexUnlockDiesWhenRealtime) {
-  auto Func = []() {
-    pthread_mutex_t mutex{};
-    pthread_mutex_unlock(&mutex);
-  };
+TEST_F(PthreadMutexLockTest, PthreadMutexUnlockDiesWhenRealtime) {
+  Lock();
+  auto Func = [this]() { Unlock(); };
 
   ExpectRealtimeDeath(Func, "pthread_mutex_unlock");
   ExpectNonRealtimeSurvival(Func);
 }
 
+TEST_F(PthreadMutexLockTest, PthreadMutexUnlockSurvivesWhenNotRealtime) {
+  Lock();
+  auto Func = [this]() { Unlock(); };
+
+  ExpectNonRealtimeSurvival(Func);
+}
+
 TEST(TestRtsanInterceptors, PthreadMutexJoinDiesWhenRealtime) {
   auto Func = []() {
     pthread_t thread{};
@@ -431,30 +469,76 @@ TEST(TestRtsanInterceptors, PthreadCondWaitDiesWhenRealtime) {
   pthread_mutex_destroy(&mutex);
 }
 
-TEST(TestRtsanInterceptors, PthreadRwlockRdlockDiesWhenRealtime) {
-  auto Func = []() {
-    pthread_rwlock_t rw_lock;
+class PthreadRwlockTest : public ::testing::Test {
+protected:
+  void SetUp() override {
+    pthread_rwlock_init(&rw_lock, nullptr);
+    is_locked = false;
+  }
+
+  void TearDown() override {
+    if (is_locked) {
+      Unlock();
+    }
+    pthread_rwlock_destroy(&rw_lock);
+  }
+
+  void RdLock() {
+    ASSERT_TRUE(!is_locked);
     pthread_rwlock_rdlock(&rw_lock);
-  };
+    is_locked = true;
+  }
+
+  void WrLock() {
+    ASSERT_TRUE(!is_locked);
+    pthread_rwlock_wrlock(&rw_lock);
+    is_locked = true;
+  }
+
+  void Unlock() {
+    ASSERT_TRUE(is_locked);
+    pthread_rwlock_unlock(&rw_lock);
+    is_locked = false;
+  }
+
+private:
+  pthread_rwlock_t rw_lock;
+  bool is_locked;
+};
+
+TEST_F(PthreadRwlockTest, PthreadRwlockRdlockDiesWhenRealtime) {
+  auto Func = [this]() { RdLock(); };
   ExpectRealtimeDeath(Func, "pthread_rwlock_rdlock");
+}
+
+TEST_F(PthreadRwlockTest, PthreadRwlockRdlockSurvivesWhenNonRealtime) {
+  auto Func = [this]() { RdLock(); };
   ExpectNonRealtimeSurvival(Func);
 }
 
-TEST(TestRtsanInterceptors, PthreadRwlockUnlockDiesWhenRealtime) {
-  auto Func = []() {
-    pthread_rwlock_t rw_lock;
-    pthread_rwlock_unlock(&rw_lock);
-  };
+TEST_F(PthreadRwlockTest, PthreadRwlockUnlockDiesWhenRealtime) {
+  RdLock();
+
+  auto Func = [this]() { Unlock(); };
   ExpectRealtimeDeath(Func, "pthread_rwlock_unlock");
+}
+
+TEST_F(PthreadRwlockTest, PthreadRwlockUnlockSurvivesWhenNonRealtime) {
+  RdLock();
+
+  auto Func = [this]() { Unlock(); };
   ExpectNonRealtimeSurvival(Func);
 }
 
-TEST(TestRtsanInterceptors, PthreadRwlockWrlockDiesWhenRealtime) {
-  auto Func = []() {
-    pthread_rwlock_t rw_lock;
-    pthread_rwlock_wrlock(&rw_lock);
-  };
+TEST_F(PthreadRwlockTest, PthreadRwlockWrlockDiesWhenRealtime) {
+  auto Func = [this]() { WrLock(); };
+
   ExpectRealtimeDeath(Func, "pthread_rwlock_wrlock");
+}
+
+TEST_F(PthreadRwlockTest, PthreadRwlockWrlockSurvivesWhenNonRealtime) {
+  auto Func = [this]() { WrLock(); };
+
   ExpectNonRealtimeSurvival(Func);
 }
 

[cjappl]

Notice here, for instance, we were locking an uninitialized lock

[cjappl]

Unlocking an uninited, and not-locked lock

[cjappl]

CC @davidtrevelyan

[MaskRay]

(I'm starting a 3-week vacation this Friday and will have limited availability.)