[compiler-rt] [tsan] Mark `pthread_*_lock` functions as blocking (PR #84162)

via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 6 04:47:04 PST 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

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

Author: Nazım Can Altınova (canova)

<details>
<summary>Changes</summary>

Fixes #<!-- -->83561.

When a thread is blocked on a mutex and we send an async signal to that mutex, it never arrives because tsan thinks that `pthread_mutex_lock` is not a blocking function. This patch marks `pthread_*_lock` functions as blocking so we can successfully deliver async signals like `SIGPROF` when the thread is blocked on them.

See the issue also for more details. I also added a test, which is a simplified version of the compiler explorer example I posted in the issue.

Please let me know if you have any other ideas or things to improve! Happy to work on them.

Also I filed #<!-- -->83844 which is more tricky because we don't have a libc wrapper for `SYS_futex`. I'm not sure how to intercept this yet. Please let me know if you have ideas on that as well. Thanks!

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


2 Files Affected:

- (modified) compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp (+5-5) 
- (added) compiler-rt/test/tsan/signal_in_mutex_lock.cpp (+71) 


``````````diff
diff --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
index a9f6673ac44e90..4e92c2dbba11cf 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
@@ -1340,7 +1340,7 @@ TSAN_INTERCEPTOR(int, pthread_mutex_destroy, void *m) {
 TSAN_INTERCEPTOR(int, pthread_mutex_lock, void *m) {
   SCOPED_TSAN_INTERCEPTOR(pthread_mutex_lock, m);
   MutexPreLock(thr, pc, (uptr)m);
-  int res = REAL(pthread_mutex_lock)(m);
+  int res = BLOCK_REAL(pthread_mutex_lock)(m);
   if (res == errno_EOWNERDEAD)
     MutexRepair(thr, pc, (uptr)m);
   if (res == 0 || res == errno_EOWNERDEAD)
@@ -1385,7 +1385,7 @@ TSAN_INTERCEPTOR(int, pthread_mutex_clocklock, void *m,
                  __sanitizer_clockid_t clock, void *abstime) {
   SCOPED_TSAN_INTERCEPTOR(pthread_mutex_clocklock, m, clock, abstime);
   MutexPreLock(thr, pc, (uptr)m);
-  int res = REAL(pthread_mutex_clocklock)(m, clock, abstime);
+  int res = BLOCK_REAL(pthread_mutex_clocklock)(m, clock, abstime);
   if (res == errno_EOWNERDEAD)
     MutexRepair(thr, pc, (uptr)m);
   if (res == 0 || res == errno_EOWNERDEAD)
@@ -1403,7 +1403,7 @@ TSAN_INTERCEPTOR(int, pthread_mutex_clocklock, void *m,
 TSAN_INTERCEPTOR(int, __pthread_mutex_lock, void *m) {
   SCOPED_TSAN_INTERCEPTOR(__pthread_mutex_lock, m);
   MutexPreLock(thr, pc, (uptr)m);
-  int res = REAL(__pthread_mutex_lock)(m);
+  int res = BLOCK_REAL(__pthread_mutex_lock)(m);
   if (res == errno_EOWNERDEAD)
     MutexRepair(thr, pc, (uptr)m);
   if (res == 0 || res == errno_EOWNERDEAD)
@@ -1446,7 +1446,7 @@ TSAN_INTERCEPTOR(int, pthread_spin_destroy, void *m) {
 TSAN_INTERCEPTOR(int, pthread_spin_lock, void *m) {
   SCOPED_TSAN_INTERCEPTOR(pthread_spin_lock, m);
   MutexPreLock(thr, pc, (uptr)m);
-  int res = REAL(pthread_spin_lock)(m);
+  int res = BLOCK_REAL(pthread_spin_lock)(m);
   if (res == 0) {
     MutexPostLock(thr, pc, (uptr)m);
   }
@@ -1521,7 +1521,7 @@ TSAN_INTERCEPTOR(int, pthread_rwlock_timedrdlock, void *m, void *abstime) {
 TSAN_INTERCEPTOR(int, pthread_rwlock_wrlock, void *m) {
   SCOPED_TSAN_INTERCEPTOR(pthread_rwlock_wrlock, m);
   MutexPreLock(thr, pc, (uptr)m);
-  int res = REAL(pthread_rwlock_wrlock)(m);
+  int res = BLOCK_REAL(pthread_rwlock_wrlock)(m);
   if (res == 0) {
     MutexPostLock(thr, pc, (uptr)m);
   }
diff --git a/compiler-rt/test/tsan/signal_in_mutex_lock.cpp b/compiler-rt/test/tsan/signal_in_mutex_lock.cpp
new file mode 100644
index 00000000000000..ec99e23198400f
--- /dev/null
+++ b/compiler-rt/test/tsan/signal_in_mutex_lock.cpp
@@ -0,0 +1,71 @@
+// RUN: %clang_tsan %s -lstdc++ -o %t && %run %t 2>&1 | FileCheck %s
+
+#include "test.h"
+#include <pthread.h>
+#include <signal.h>
+#include <stdio.h>
+
+#include <cassert>
+#include <condition_variable>
+#include <mutex>
+
+std::mutex sampler_mutex; //dummy mutex to lock in the thread we spawn.
+std::mutex done_mutex;    // guards the cv and done variables.
+std::condition_variable cv;
+bool done = false;
+
+void *ThreadFunc(void *x) {
+  while (true) {
+    // Lock the mutex
+    std::lock_guard<std::mutex> guard(sampler_mutex);
+    // Mutex is released at the end
+  }
+
+  return nullptr;
+}
+
+static void SigprofHandler(int signal, siginfo_t *info, void *context) {
+  // Assuming we did some work, change the variable to let the main thread
+  // know that we are done.
+  {
+    std::unique_lock<std::mutex> lck(done_mutex);
+    done = true;
+    cv.notify_one();
+  }
+}
+
+int main() {
+  alarm(60); // Kill the test if it hangs.
+
+  // Install the signal handler
+  struct sigaction sa;
+  sa.sa_sigaction = SigprofHandler;
+  sigemptyset(&sa.sa_mask);
+  sa.sa_flags = SA_RESTART | SA_SIGINFO;
+  if (sigaction(SIGPROF, &sa, 0) != 0) {
+    fprintf(stderr, "failed to install signal handler\n");
+    abort();
+  }
+
+  // Spawn a thread that will just loop and get the mutex lock:
+  pthread_t thread;
+  pthread_create(&thread, NULL, ThreadFunc, NULL);
+
+  // Lock the mutex before sending the signal
+  std::lock_guard<std::mutex> guard(sampler_mutex);
+  // From now on thread 1 will be waiting for the lock
+
+  // Send the SIGPROF signal to thread.
+  int r = pthread_kill(thread, SIGPROF);
+  assert(r == 0);
+
+  // Wait until signal handler sends the data.
+  std::unique_lock lk(done_mutex);
+  cv.wait(lk, [] { return done; });
+
+  // We got the done variable from the signal handler. Exiting successfully.
+  fprintf(stderr, "PASS\n");
+}
+
+// CHECK-NOT: WARNING: ThreadSanitizer:
+// CHECK: PASS

``````````

</details>


https://github.com/llvm/llvm-project/pull/84162


More information about the llvm-commits mailing list