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

[via llvm-commits]

Author: Nazım Can Altınova
Date: 2024-03-26T12:32:52+01:00
New Revision: f01377d8ebcb1f52fe75a236cdf34f1b8b1e99fb

URL: https://github.com/llvm/llvm-project/commit/f01377d8ebcb1f52fe75a236cdf34f1b8b1e99fb
DIFF: https://github.com/llvm/llvm-project/commit/f01377d8ebcb1f52fe75a236cdf34f1b8b1e99fb.diff

LOG: [tsan] Mark `pthread_*_lock` functions as blocking (#84162)

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!

Added: 
    compiler-rt/test/tsan/signal_in_mutex_lock.cpp

Modified: 
    compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
index 8ffc703b05eace..26c2cfef4e7709 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