-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
[TSan][Apple] Fix interceptor build error #124351
base: main
Are you sure you want to change the base?
Conversation
@llvm/pr-subscribers-compiler-rt-sanitizer Author: Julian Lettner (yln) ChangesIn certain cases, the SDK headers declare When enabled, building of TSan interceptors failed In a previous change [1], we misattributed this to [1] ae484c2 rdar://143193907 Full diff: https://github.com/llvm/llvm-project/pull/124351.diff 2 Files Affected:
diff --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors_mac.cpp b/compiler-rt/lib/tsan/rtl/tsan_interceptors_mac.cpp
index e0e4c5b9d36cd3..9d1f928b018bcc 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_interceptors_mac.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_interceptors_mac.cpp
@@ -25,7 +25,6 @@
# include "tsan_interceptors.h"
# include "tsan_interface.h"
# include "tsan_interface_ann.h"
-# include "tsan_spinlock_defs_mac.h"
# if defined(__has_include) && __has_include(<xpc/xpc.h>)
# include <xpc/xpc.h>
@@ -96,8 +95,7 @@ static constexpr morder kMacFailureOrder = mo_relaxed;
m_orig(int32_t, uint32_t, a32, f##32##OrigBarrier, \
__tsan_atomic32_##tsan_atomic_f, kMacOrderBarrier)
-# pragma clang diagnostic push
-// OSAtomic* functions are deprecated.
+# pragma clang diagnostic push // OSAtomic* deprecation
# pragma clang diagnostic ignored "-Wdeprecated-declarations"
OSATOMIC_INTERCEPTORS_ARITHMETIC(OSAtomicAdd, fetch_add,
OSATOMIC_INTERCEPTOR_PLUS_X)
@@ -111,6 +109,7 @@ OSATOMIC_INTERCEPTORS_BITWISE(OSAtomicAnd, fetch_and,
OSATOMIC_INTERCEPTOR_PLUS_X, OSATOMIC_INTERCEPTOR)
OSATOMIC_INTERCEPTORS_BITWISE(OSAtomicXor, fetch_xor,
OSATOMIC_INTERCEPTOR_PLUS_X, OSATOMIC_INTERCEPTOR)
+# pragma clang diagnostic pop // OSAtomic* deprecation
# define OSATOMIC_INTERCEPTORS_CAS(f, tsan_atomic_f, tsan_t, t) \
TSAN_INTERCEPTOR(bool, f, t old_value, t new_value, t volatile *ptr) { \
@@ -128,8 +127,7 @@ OSATOMIC_INTERCEPTORS_BITWISE(OSAtomicXor, fetch_xor,
kMacOrderBarrier, kMacFailureOrder); \
}
-# pragma clang diagnostic push
-// OSAtomicCompareAndSwap* functions are deprecated.
+# pragma clang diagnostic push // OSAtomicCompareAndSwap* deprecation
# pragma clang diagnostic ignored "-Wdeprecated-declarations"
OSATOMIC_INTERCEPTORS_CAS(OSAtomicCompareAndSwapInt, __tsan_atomic32, a32, int)
OSATOMIC_INTERCEPTORS_CAS(OSAtomicCompareAndSwapLong, __tsan_atomic64, a64,
@@ -140,7 +138,7 @@ OSATOMIC_INTERCEPTORS_CAS(OSAtomicCompareAndSwap32, __tsan_atomic32, a32,
int32_t)
OSATOMIC_INTERCEPTORS_CAS(OSAtomicCompareAndSwap64, __tsan_atomic64, a64,
int64_t)
-# pragma clang diagnostic pop
+# pragma clang diagnostic pop // OSAtomicCompareAndSwap* deprecation
# define OSATOMIC_INTERCEPTOR_BITOP(f, op, clear, mo) \
TSAN_INTERCEPTOR(bool, f, uint32_t n, volatile void *ptr) { \
@@ -156,9 +154,12 @@ OSATOMIC_INTERCEPTORS_CAS(OSAtomicCompareAndSwap64, __tsan_atomic64, a64,
OSATOMIC_INTERCEPTOR_BITOP(f, op, clear, kMacOrderNonBarrier) \
OSATOMIC_INTERCEPTOR_BITOP(f##Barrier, op, clear, kMacOrderBarrier)
+# pragma clang diagnostic push // OSAtomicTestAnd* deprecation
+# pragma clang diagnostic ignored "-Wdeprecated-declarations"
OSATOMIC_INTERCEPTORS_BITOP(OSAtomicTestAndSet, __tsan_atomic8_fetch_or, false)
OSATOMIC_INTERCEPTORS_BITOP(OSAtomicTestAndClear, __tsan_atomic8_fetch_and,
true)
+# pragma clang diagnostic pop // OSAtomicTestAnd* deprecation
TSAN_INTERCEPTOR(void, OSAtomicEnqueue, OSQueueHead *list, void *item,
size_t offset) {
@@ -196,6 +197,16 @@ TSAN_INTERCEPTOR(void *, OSAtomicFifoDequeue, OSFifoQueueHead *list,
# endif
+// If `OSSPINLOCK_USE_INLINED=1` is set, then SDK headers don't declare these
+// as functions, but macros that call non-deprecated APIs. Undefine these
+// macros so they don't interfere with the interceptor machinery.
+#undef OSSpinLockLock
+#undef OSSpinLockTry
+#undef OSSpinLockUnlock
+
+# pragma clang diagnostic push // OSSpinLock* deprecation
+# pragma clang diagnostic ignored "-Wdeprecated-declarations"
+
TSAN_INTERCEPTOR(void, OSSpinLockLock, volatile OSSpinLock *lock) {
CHECK(!cur_thread()->is_dead);
if (!cur_thread()->is_inited) {
@@ -227,6 +238,7 @@ TSAN_INTERCEPTOR(void, OSSpinLockUnlock, volatile OSSpinLock *lock) {
Release(thr, pc, (uptr)lock);
REAL(OSSpinLockUnlock)(lock);
}
+# pragma clang diagnostic pop // OSSpinLock* deprecation
TSAN_INTERCEPTOR(void, os_lock_lock, void *lock) {
CHECK(!cur_thread()->is_dead);
diff --git a/compiler-rt/lib/tsan/rtl/tsan_spinlock_defs_mac.h b/compiler-rt/lib/tsan/rtl/tsan_spinlock_defs_mac.h
deleted file mode 100644
index 1a99a81c030230..00000000000000
--- a/compiler-rt/lib/tsan/rtl/tsan_spinlock_defs_mac.h
+++ /dev/null
@@ -1,45 +0,0 @@
-//===-- tsan_spinlock_defs_mac.h -------------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-//
-// This file is a part of ThreadSanitizer (TSan), a race detector.
-//
-// Mac-specific forward-declared function defintions that may be
-// deprecated in later versions of the OS.
-// These are needed for interceptors.
-//
-//===----------------------------------------------------------------------===//
-
-#if SANITIZER_APPLE
-
-#ifndef TSAN_SPINLOCK_DEFS_MAC_H
-#define TSAN_SPINLOCK_DEFS_MAC_H
-
-#include <stdint.h>
-
-extern "C" {
-
-/*
-Provides forward declarations related to OSSpinLocks on Darwin. These functions are
-deprecated on macOS version 10.12 and later,
-and are no longer included in the system headers.
-
-However, the symbols are still available on the system, so we provide these forward
-declarations to prevent compilation errors in tsan_interceptors_mac.cpp, which
-references these functions when defining TSAN interceptor functions.
-*/
-
-typedef int32_t OSSpinLock;
-
-void OSSpinLockLock(volatile OSSpinLock *__lock);
-void OSSpinLockUnlock(volatile OSSpinLock *__lock);
-bool OSSpinLockTry(volatile OSSpinLock *__lock);
-
-}
-
-#endif //TSAN_SPINLOCK_DEFS_MAC_H
-#endif // SANITIZER_APPLE
|
In certain cases, the SDK headers declare `OSSpinLock*` APIs as macros (instead of functions), so users can be transparently forwarded to non-deprecated APIs. When enabled, building of TSan interceptors failed because these macros interfere with the interceptor machinery, i.e., they prevent proper forward declaration of intercepted APIs. In a previous change [1], we misattributed this to the deprecation of `OSSpinLock*` APIs. [1] ae484c2 rdar://143193907
Add missing `#pragma clang diagnostic pop` to prevent disabling deprecation warnings for the rest of the file.
f552c55
to
f6488f9
Compare
@@ -111,6 +109,7 @@ OSATOMIC_INTERCEPTORS_BITWISE(OSAtomicAnd, fetch_and, | |||
OSATOMIC_INTERCEPTOR_PLUS_X, OSATOMIC_INTERCEPTOR) | |||
OSATOMIC_INTERCEPTORS_BITWISE(OSAtomicXor, fetch_xor, | |||
OSATOMIC_INTERCEPTOR_PLUS_X, OSATOMIC_INTERCEPTOR) | |||
# pragma clang diagnostic pop // OSAtomic* deprecation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing pop
here deactivated deprecation checking for the rest of the file. @cachemeifyoucan
I added it and fixed the remaining warnings.
Alternatively, we could put the # pragma clang diagnostic ignored "-Wdeprecated-declarations"
at the start of the file to intentionally disable deprecation warnings for the entire file and relief ourselves from some tedium.
@vitalybuka, any style preferences?
This looks good. Makes sense now why my forward declaration was the wrong thing here. My previous concerns we discussed are not an issue. |
@fhahn will you need to do the same undefs in the OSSpinLock interceptor in rtsan? I see you have some forward declaring and intercepting of OSSpinLock as well. |
In certain cases, the SDK headers declare
OSSpinLock*
APIs as macros (instead offunctions), so users can be transparently
forwarded to non-deprecated APIs.
When enabled, building of TSan interceptors failed
because these macros interfere with the
interceptor machinery, i.e., they prevent proper
forward declaration of intercepted APIs.
In a previous change [1], we misattributed this to
the deprecation of
OSSpinLock*
APIs.[1] ae484c2
rdar://143193907