-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[rtsan] Remove legacy_pthread_cond support #155181
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-compiler-rt-sanitizer Author: Chris Apple (cjappl) ChangesPartial revert of #152947 There were two fixes that may have applied to #146120
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:
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;
}
|
Assuming this doesn't break the fix, LGTM |
Built and ran my example program with @cjappl ’s latest and it runs as expected with no segfaults. |
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. |
Partial revert of #152947
There were two fixes that may have applied to #146120
Adding the version protection around the pthread_cond_ interceptors
llvm-project/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
Lines 1695 to 1711 in 0723e81
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