[libc++] Replace the of use custom sections for detecting overriden functions - #208330
Conversation
…unctions This is a follow up to llvm#133876 and an alternative to llvm#120805 which doesn't rely on aliases and works across both ELF and Mach-O. This mechanism is preferable in baremetal environments since it doesn't require special handling of the custom sections.
|
@llvm/pr-subscribers-libcxx Author: Petr Hosek (petrhosek) ChangesThis is a follow up to #133876 and an alternative to #120805 which doesn't rely on aliases and works across both ELF and Mach-O. This mechanism is preferable in baremetal environments since it doesn't require special handling of the custom sections. Full diff: https://github.com/llvm/llvm-project/pull/208330.diff 1 Files Affected:
diff --git a/libcxx/src/include/overridable_function.h b/libcxx/src/include/overridable_function.h
index c8d9f30c74c3e..31bd96c3abd9c 100644
--- a/libcxx/src/include/overridable_function.h
+++ b/libcxx/src/include/overridable_function.h
@@ -11,11 +11,6 @@
#define _LIBCPP_SRC_INCLUDE_OVERRIDABLE_FUNCTION_H
#include <__config>
-#include <cstdint>
-
-#if __has_feature(ptrauth_calls)
-# include <ptrauth.h>
-#endif
#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
# pragma GCC system_header
@@ -42,18 +37,11 @@
// -------------------
//
// Let's say we want to check whether a weak function `f` has been overridden by the user.
-// The general mechanism works by placing `f`'s definition (in the libc++ built library)
-// inside a special section, which we do using the `__section__` attribute via the
-// OVERRIDABLE_FUNCTION macro.
-//
-// Then, when comes the time to check whether the function has been overridden, we take
-// the address of the function and we check whether it falls inside the special function
-// we created. This can be done by finding pointers to the start and the end of the section
-// (which is done differently for ELF and Mach-O), and then checking whether `f` falls
-// within those bounds. If it falls within those bounds, then `f` is still inside the
-// special section and so it is the version we defined in the libc++ built library, i.e.
-// it was not overridden. Otherwise, it was overridden by the user because it falls
-// outside of the section.
+// The general mechanism works by defining a local symbol `__impl_ref<f>::__impl_` with
+// the same address as `f` as a constant expression using direct PC-relative
+// materialization thus pointing at the symbol defined in the same TU. At runtime, it
+// compares the address of `__impl_ref<f>::__impl_` with the address of `f` loaded from
+// GOT: if `f` was overridden by the user in another TU, the addresses will be different.
//
// Important note
// --------------
@@ -63,64 +51,53 @@
// want to be defining special sections inside user's executables which use our headers.
//
-#if defined(_LIBCPP_OBJECT_FORMAT_MACHO)
+#if defined(_LIBCPP_OBJECT_FORMAT_MACHO) || (defined(_LIBCPP_OBJECT_FORMAT_ELF) && !defined(__NVPTX__))
# define _LIBCPP_CAN_DETECT_OVERRIDDEN_FUNCTION 1
-# define OVERRIDABLE_FUNCTION [[gnu::weak, gnu::section("__TEXT,__lcxx_override,regular,pure_instructions")]]
-
-_LIBCPP_BEGIN_NAMESPACE_STD template <typename T, T* _Func>
-_LIBCPP_HIDE_FROM_ABI inline bool __is_function_overridden() noexcept {
- // Declare two dummy bytes and give them these special `__asm` values. These values are
- // defined by the linker, which means that referring to `&__lcxx_override_start` will
- // effectively refer to the address where the section starts (and same for the end).
- extern char __lcxx_override_start __asm("section$start$__TEXT$__lcxx_override");
- extern char __lcxx_override_end __asm("section$end$__TEXT$__lcxx_override");
-
- // Now get a uintptr_t out of these locations, and out of the function pointer.
- uintptr_t __start = reinterpret_cast<uintptr_t>(&__lcxx_override_start);
- uintptr_t __end = reinterpret_cast<uintptr_t>(&__lcxx_override_end);
- uintptr_t __ptr = reinterpret_cast<uintptr_t>(_Func);
-
-# if __has_feature(ptrauth_calls)
- // We must pass a void* to ptrauth_strip since it only accepts a pointer type. Also, in particular,
- // we must NOT pass a function pointer, otherwise we will strip the function pointer, and then attempt
- // to authenticate and re-sign it when casting it to a uintptr_t again, which will fail because we just
- // stripped the function pointer. See rdar://122927845.
- __ptr = reinterpret_cast<uintptr_t>(ptrauth_strip(reinterpret_cast<void*>(__ptr), ptrauth_key_function_pointer));
-# endif
+# define OVERRIDABLE_FUNCTION [[gnu::weak]]
- // Finally, the function was overridden if it falls outside of the section's bounds.
- return __ptr < __start || __ptr > __end;
-}
-_LIBCPP_END_NAMESPACE_STD
+_LIBCPP_BEGIN_NAMESPACE_STD
-// The NVPTX linker cannot create '__start/__stop' sections.
-#elif defined(_LIBCPP_OBJECT_FORMAT_ELF) && !defined(__NVPTX__)
+namespace {
-# define _LIBCPP_CAN_DETECT_OVERRIDDEN_FUNCTION 1
-# define OVERRIDABLE_FUNCTION [[gnu::weak, gnu::section("__lcxx_override")]]
+template <typename T>
+[[nodiscard]] inline _LIBCPP_HIDE_FROM_ABI T* __libcpp_launder(T* __ptr) noexcept {
+ __asm__ volatile("" : "+r"(__ptr));
+ return __ptr;
+}
-// This is very similar to what we do for Mach-O above. The ELF linker will implicitly define
-// variables with those names corresponding to the start and the end of the section.
-//
-// See https://stackoverflow.com/questions/16552710/how-do-you-get-the-start-and-end-addresses-of-a-custom-elf-section
-extern char __start___lcxx_override;
-extern char __stop___lcxx_override;
+}
-_LIBCPP_BEGIN_NAMESPACE_STD
+template <auto* _Func>
+struct __impl_ref;
+
+// __impl_ref<...>::__impl_ is expected to be defined elsewhere, so the compiler emits
+// assembly references to the mangled symbol with no definition. This template saves us
+// the trouble of providing manual declarations for overloads with some other local name
+// for each function name being overloaded (operator new, operator new[], etc.).
+template <typename _Ret, typename... _Args, _Ret (*_Func)(_Args...)>
+struct __impl_ref<_Func> {
+ [[gnu::visibility("hidden")]] static _Ret __impl_(_Args...);
+};
+
+// This takes a function type template argument first so that the second non-type template
+// argument (pointer to the public function) gets the benefit of type-aware overload
+// resolution, rather than having to use a static_cast.
template <typename T, T* _Func>
_LIBCPP_HIDE_FROM_ABI inline bool __is_function_overridden() noexcept {
- uintptr_t __start = reinterpret_cast<uintptr_t>(&__start___lcxx_override);
- uintptr_t __end = reinterpret_cast<uintptr_t>(&__stop___lcxx_override);
- uintptr_t __ptr = reinterpret_cast<uintptr_t>(_Func);
-
-# if __has_feature(ptrauth_calls)
- // We must pass a void* to ptrauth_strip since it only accepts a pointer type. See full explanation above.
- __ptr = reinterpret_cast<uintptr_t>(ptrauth_strip(reinterpret_cast<void*>(__ptr), ptrauth_key_function_pointer));
+# if !defined(_LIBCPP_CLANG_VER) || _LIBCPP_CLANG_VER >= 2101
+ __asm__("%cc0 = %cc1" : : "X"(__impl_ref<_Func>::__impl_), "X"(_Func));
+# else
+ __asm__("%c0 = %c1" : : "X"(__impl_ref<_Func>::__impl_), "X"(_Func));
# endif
-
- return __ptr < __start || __ptr > __end;
+ // This just has the compiler compare the two symbols. For PIC mode, this will do a
+ // direct PC-relative materialization for __impl_ref<...>::__impl_ and a GOT load for
+ // the _Func symbol. The compiler thinks __impl_ref<...>::__impl_ is defined elsewhere
+ // at link time and will be an undefined symbol. It doesn't know that the __asm__ tells
+ // the assembler to define it as a local symbol.
+ return __libcpp_launder(_Func) != __impl_ref<_Func>::__impl_;
}
+
_LIBCPP_END_NAMESPACE_STD
#else
|
| template <typename T> | ||
| [[nodiscard]] inline _LIBCPP_HIDE_FROM_ABI T* __libcpp_launder(T* __ptr) noexcept { | ||
| __asm__ volatile("" : "+r"(__ptr)); | ||
| return __ptr; | ||
| } |
There was a problem hiding this comment.
How is this different from what std::launder does? If it's different, this seems like rather unfortunate naming.
There was a problem hiding this comment.
@petrhosek Could I get an explanation for this?
There was a problem hiding this comment.
std::launder cannot be used with function pointers. If you have a suggestion for a better name, I'm happy to send a follow up PR to rename this function.
There was a problem hiding this comment.
I think I'd be happy to make that explicit by naming it e.g. launder_function_pointer.
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
This addresses post-commit feedback raised in #208330.
| # if __has_feature(ptrauth_calls) | ||
| // We must pass a void* to ptrauth_strip since it only accepts a pointer type. See full explanation above. | ||
| __ptr = reinterpret_cast<uintptr_t>(ptrauth_strip(reinterpret_cast<void*>(__ptr), ptrauth_key_function_pointer)); | ||
| # if !defined(_LIBCPP_CLANG_VER) || _LIBCPP_CLANG_VER >= 2101 |
There was a problem hiding this comment.
@petrhosek I've started seeing this error downstream, where we build with arm64e:
error: invalid operand in inline asm: '${0:c} = ${1:c}'
__asm__("%cc0 = %cc1" : : "X"(__impl_ref<_Func>::__impl_), "X"(_Func));
^
This only happens when we build with arm64e. Note that this also doesn't work if I force it to use the %c0 = %c1 syntax below.
Do you think that's Clang not properly handling ptrauth in inline __asm__? Unless we have a clear way to fix this forward and cherry-pick to the release branch, I think we should go back to the previous implementation until we figure it out.
…unctions (llvm#208330) This is a follow up to llvm#133876 and an alternative to llvm#120805 which doesn't rely on aliases and works across both ELF and Mach-O. This mechanism is preferable in baremetal environments since it doesn't require special handling of the custom sections.
…209320) This addresses post-commit feedback raised in llvm#208330.
We found out that the new mechanism for detecting overriden functions does not support Arm Pointer Authentication (PAuth). Addressing this limitation is going to require changes to Clang. In the meantime, we switched back to the old mechanism when PAuth is enabled.
After partial revert of #208330 in #209928, the libcxx build started failing because of missing `<cstdint>` include required for `uintptr_t` declaration used only by code behind `ptrauth_calls` feature check. See https://lab.llvm.org/buildbot/#/builders/227/builds/3358 This patch adds the missing include.
We found out that the new mechanism for detecting overriden functions does not support Arm Pointer Authentication (PAuth). Addressing this limitation is going to require changes to Clang. In the meantime, we switched back to the old mechanism when PAuth is enabled. (cherry picked from commit db7356d)
After partial revert of llvm#208330 in llvm#209928, the libcxx build started failing because of missing `<cstdint>` include required for `uintptr_t` declaration used only by code behind `ptrauth_calls` feature check. See https://lab.llvm.org/buildbot/#/builders/227/builds/3358 This patch adds the missing include. (cherry picked from commit 12e1c7f)
After partial revert of llvm#208330 in llvm#209928, the libcxx build started failing because of missing `<cstdint>` include required for `uintptr_t` declaration used only by code behind `ptrauth_calls` feature check. See https://lab.llvm.org/buildbot/#/builders/227/builds/3358 This patch adds the missing include.
This is a follow up to #133876 and an alternative to #120805 which doesn't rely on aliases and works across both ELF and Mach-O. This mechanism is preferable in baremetal environments since it doesn't require special handling of the custom sections.