[libcxx-commits] [libcxx] Revert "[libc++] Replace the of use custom sections for detecting overriden functions" (PR #208157)

[via libcxx-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libcxx

Author: Petr Hosek (petrhosek)

<details>
<summary>Changes</summary>

Reverts llvm/llvm-project#<!-- -->175896 since it broke the build of internal symbolizer used by sanitizers.

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


1 Files Affected:

- (modified) libcxx/src/include/overridable_function.h (+65-30) 


``````````diff
diff --git a/libcxx/src/include/overridable_function.h b/libcxx/src/include/overridable_function.h
index 085662e446332..c8d9f30c74c3e 100644
--- a/libcxx/src/include/overridable_function.h
+++ b/libcxx/src/include/overridable_function.h
@@ -11,6 +11,11 @@
 #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
@@ -37,11 +42,18 @@
 // -------------------
 //
 // Let's say we want to check whether a weak function `f` has been overridden by the user.
-// The general mechanism works by defining a local symbol `ImplRef<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
-// `ImplRef<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.
+// 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.
 //
 // Important note
 // --------------
@@ -51,40 +63,63 @@
 // want to be defining special sections inside user's executables which use our headers.
 //
 
-#if defined(_LIBCPP_OBJECT_FORMAT_MACHO) || (defined(_LIBCPP_OBJECT_FORMAT_ELF) && !defined(__NVPTX__))
+#if defined(_LIBCPP_OBJECT_FORMAT_MACHO)
 
-template <auto* _Func>
-struct ImplRef;
+#  define _LIBCPP_CAN_DETECT_OVERRIDDEN_FUNCTION 1
+#  define OVERRIDABLE_FUNCTION [[gnu::weak, gnu::section("__TEXT,__lcxx_override,regular,pure_instructions")]]
 
-// ImplRef<...>::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 ImplRef<_Func> {
-  [[gnu::visibility("hidden")]] static _Ret Impl(_Args...);
-};
+_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
+
+  // Finally, the function was overridden if it falls outside of the section's bounds.
+  return __ptr < __start || __ptr > __end;
+}
+_LIBCPP_END_NAMESPACE_STD
+
+// The NVPTX linker cannot create '__start/__stop' sections.
+#elif defined(_LIBCPP_OBJECT_FORMAT_ELF) && !defined(__NVPTX__)
 
 #  define _LIBCPP_CAN_DETECT_OVERRIDDEN_FUNCTION 1
-#  define OVERRIDABLE_FUNCTION [[gnu::weak]]
+#  define OVERRIDABLE_FUNCTION [[gnu::weak, gnu::section("__lcxx_override")]]
+
+// 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
-// 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 {
-#  if !defined(_LIBCPP_CLANG_VER) || _LIBCPP_CLANG_VER >= 2101
-  __asm__("%cc0 = %cc1" : : "X"(ImplRef<_Func>::Impl), "X"(_Func));
-#  else
-  __asm__("%c0 = %c1" : : "X"(ImplRef<_Func>::Impl), "X"(_Func));
+  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));
 #  endif
-  // This just has the compiler compare the two symbols. For PIC mode, this will do
-  // a direct PC-relative materialization for ImplRef<...>::Impl and a GOT load for
-  // the _Func symbol. The compiler thinks ImplRef<...>::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 _Func != ImplRef<_Func>::Impl;
+
+  return __ptr < __start || __ptr > __end;
 }
 _LIBCPP_END_NAMESPACE_STD
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/208157