Skip to content

Conversation

cjappl
Copy link
Contributor

@cjappl cjappl commented Aug 24, 2025

Partial revert of #152947

There were two fixes that may have applied to #146120

  1. Adding the version protection around the pthread_cond_ interceptors

    #if SANITIZER_GLIBC && !__GLIBC_PREREQ(2, 36) && \
    (defined(__x86_64__) || defined(__mips__) || SANITIZER_PPC64V1 || \
    defined(__s390x__))
    INTERCEPT_FUNCTION_VER(pthread_cond_init, "GLIBC_2.3.2");
    INTERCEPT_FUNCTION_VER(pthread_cond_signal, "GLIBC_2.3.2");
    INTERCEPT_FUNCTION_VER(pthread_cond_broadcast, "GLIBC_2.3.2");
    INTERCEPT_FUNCTION_VER(pthread_cond_wait, "GLIBC_2.3.2");
    INTERCEPT_FUNCTION_VER(pthread_cond_timedwait, "GLIBC_2.3.2");
    INTERCEPT_FUNCTION_VER(pthread_cond_destroy, "GLIBC_2.3.2");
    #else
    INTERCEPT_FUNCTION(pthread_cond_init);
    INTERCEPT_FUNCTION(pthread_cond_signal);
    INTERCEPT_FUNCTION(pthread_cond_broadcast);
    INTERCEPT_FUNCTION(pthread_cond_wait);
    INTERCEPT_FUNCTION(pthread_cond_timedwait);
    INTERCEPT_FUNCTION(pthread_cond_destroy);
    #endif

  2. Adding support for the legacy_pthread_cond variables (reverted here)

To see if this still fixes #146120, I'm going to put this up for review and ask @adrew0809 to test again. If his problem is still fixed I'll merge this.

(or I can revert the previous patch, and merge just adding the version protection if folks think that is cleaner)

Both Mac and Linux tests pass on my machine with this patch in this PR

@llvmbot
Copy link
Member

llvmbot commented Aug 24, 2025

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

Author: Chris Apple (cjappl)

Changes

Partial revert of #152947

There were two fixes that may have applied to #146120

  1. Adding the version protection around the pthread_cond_ interceptors

    #if SANITIZER_GLIBC && !__GLIBC_PREREQ(2, 36) && \
    (defined(__x86_64__) || defined(__mips__) || SANITIZER_PPC64V1 || \
    defined(__s390x__))
    INTERCEPT_FUNCTION_VER(pthread_cond_init, "GLIBC_2.3.2");
    INTERCEPT_FUNCTION_VER(pthread_cond_signal, "GLIBC_2.3.2");
    INTERCEPT_FUNCTION_VER(pthread_cond_broadcast, "GLIBC_2.3.2");
    INTERCEPT_FUNCTION_VER(pthread_cond_wait, "GLIBC_2.3.2");
    INTERCEPT_FUNCTION_VER(pthread_cond_timedwait, "GLIBC_2.3.2");
    INTERCEPT_FUNCTION_VER(pthread_cond_destroy, "GLIBC_2.3.2");
    #else
    INTERCEPT_FUNCTION(pthread_cond_init);
    INTERCEPT_FUNCTION(pthread_cond_signal);
    INTERCEPT_FUNCTION(pthread_cond_broadcast);
    INTERCEPT_FUNCTION(pthread_cond_wait);
    INTERCEPT_FUNCTION(pthread_cond_timedwait);
    INTERCEPT_FUNCTION(pthread_cond_destroy);
    #endif

  2. Adding support for the legacy_pthread_cond variables (reverted here)

To see if this still fixes #146120, I'm going to put this up for review and ask @adrew0809 to test again. If his problem is still fixed I'll merge this.

(or I can revert the previous patch, and merge just adding the version protection if folks think that is cleaner)

Both Mac and Linux tests pass on my machine with this patch in this PR


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

1 Files Affected:

  • (modified) compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp (+6-42)
diff --git a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
index d89cec88cc430..eaf2da745f047 100644
--- a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
+++ b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
@@ -47,41 +47,12 @@
 
 using namespace __sanitizer;
 
-DECLARE_REAL_AND_INTERCEPTOR(void *, malloc, usize size)
-DECLARE_REAL_AND_INTERCEPTOR(void, free, void *ptr)
-
 namespace {
 struct DlsymAlloc : public DlSymAllocator<DlsymAlloc> {
   static bool UseImpl() { return !__rtsan_is_initialized(); }
 };
 } // namespace
 
-// See note in tsan as to why this is necessary
-static pthread_cond_t *init_cond(pthread_cond_t *c, bool force = false) {
-  if (!common_flags()->legacy_pthread_cond)
-    return c;
-
-  atomic_uintptr_t *p = (atomic_uintptr_t *)c;
-  uptr cond = atomic_load(p, memory_order_acquire);
-  if (!force && cond != 0)
-    return (pthread_cond_t *)cond;
-  void *newcond = WRAP(malloc)(sizeof(pthread_cond_t));
-  internal_memset(newcond, 0, sizeof(pthread_cond_t));
-  if (atomic_compare_exchange_strong(p, &cond, (uptr)newcond,
-                                     memory_order_acq_rel))
-    return (pthread_cond_t *)newcond;
-  WRAP(free)(newcond);
-  return (pthread_cond_t *)cond;
-}
-
-static void destroy_cond(pthread_cond_t *cond) {
-  if (common_flags()->legacy_pthread_cond) {
-    // Free our aux cond and zero the pointer to not leave dangling pointers.
-    WRAP(free)(cond);
-    atomic_store((atomic_uintptr_t *)cond, 0, memory_order_relaxed);
-  }
-}
-
 // Filesystem
 
 INTERCEPTOR(int, open, const char *path, int oflag, ...) {
@@ -799,41 +770,34 @@ INTERCEPTOR(int, pthread_join, pthread_t thread, void **value_ptr) {
 INTERCEPTOR(int, pthread_cond_init, pthread_cond_t *cond,
             const pthread_condattr_t *a) {
   __rtsan_notify_intercepted_call("pthread_cond_init");
-  pthread_cond_t *c = init_cond(cond, true);
-  return REAL(pthread_cond_init)(c, a);
+  return REAL(pthread_cond_init)(cond, a);
 }
 
 INTERCEPTOR(int, pthread_cond_signal, pthread_cond_t *cond) {
   __rtsan_notify_intercepted_call("pthread_cond_signal");
-  pthread_cond_t *c = init_cond(cond);
-  return REAL(pthread_cond_signal)(c);
+  return REAL(pthread_cond_signal)(cond);
 }
 
 INTERCEPTOR(int, pthread_cond_broadcast, pthread_cond_t *cond) {
   __rtsan_notify_intercepted_call("pthread_cond_broadcast");
-  pthread_cond_t *c = init_cond(cond);
-  return REAL(pthread_cond_broadcast)(c);
+  return REAL(pthread_cond_broadcast)(cond);
 }
 
 INTERCEPTOR(int, pthread_cond_wait, pthread_cond_t *cond,
             pthread_mutex_t *mutex) {
   __rtsan_notify_intercepted_call("pthread_cond_wait");
-  pthread_cond_t *c = init_cond(cond);
-  return REAL(pthread_cond_wait)(c, mutex);
+  return REAL(pthread_cond_wait)(cond, mutex);
 }
 
 INTERCEPTOR(int, pthread_cond_timedwait, pthread_cond_t *cond,
             pthread_mutex_t *mutex, const timespec *ts) {
   __rtsan_notify_intercepted_call("pthread_cond_timedwait");
-  pthread_cond_t *c = init_cond(cond);
-  return REAL(pthread_cond_timedwait)(c, mutex, ts);
+  return REAL(pthread_cond_timedwait)(cond, mutex, ts);
 }
 
 INTERCEPTOR(int, pthread_cond_destroy, pthread_cond_t *cond) {
   __rtsan_notify_intercepted_call("pthread_cond_destroy");
-  pthread_cond_t *c = init_cond(cond);
-  int res = REAL(pthread_cond_destroy)(c);
-  destroy_cond(c);
+  int res = REAL(pthread_cond_destroy)(cond);
   return res;
 }
 

@davidtrevelyan
Copy link
Contributor

Assuming this doesn't break the fix, LGTM

@adrew0809
Copy link

Built and ran my example program with @cjappl ’s latest and it runs as expected with no segfaults.

@cjappl
Copy link
Contributor Author

cjappl commented Aug 29, 2025

Thanks a lot @adrew0809 !

For sanity purposes, what I'm going to do is close this PR, revert the original and apply just the part that fixes the problem. That means we only need to cherry pick one small thing to the 21 branch.

I will ping everyone on subsequent merges.

@cjappl cjappl closed this Aug 29, 2025
cjappl added a commit that referenced this pull request Aug 29, 2025
…5963)

This reverts commit aa4bc2e.

As discussed on #155181 , this introduced some unneeded code. Reverting
and applying the smaller version of the fix
cjappl added a commit that referenced this pull request Aug 29, 2025
This fixes #146120, confirmed by the original reporter

Previously reviewed as #155181, but re-submitting for better
book-keeping.


Adds versioned pthread_cond interceptors, and the
pthread_cond_init/_destroy interceptors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Real-time Sanitizer causes pthread_cond_signal to segfault when a thread is doing a timed wait on a condition variable
5 participants