[libcxx-commits] [libcxx] [libcxxabi] [libc++] Return to using DYLD_LIBRARY_PATH for the system libc++ tests (PR #98070)

Louis Dionne via libcxx-commits libcxx-commits at lists.llvm.org
Mon Jul 8 13:00:12 PDT 2024


https://github.com/ldionne created https://github.com/llvm/llvm-project/pull/98070

This is the only way to reliably test the system-built libc++. This effectively reverts 2cf2f1b, which wasn't the right solution to the problem I encountered. Instead of avoiding the use of DYLD_LIBRARY_PATH, we instead add some shims that allow the just-built libc++ to behave almost the same as the real system libc++.

Both solutions are kind of brittle. However, not using DYLD_LIBRARY_PATH means that we can't test against libc++ the way it is built on Apple platforms (with an install name of /usr/lib), which is a big problem. IMO, it makes more sense to instead just shim the rare functionality that isn't available upstream.

>From f343e7f4ee5d842feac525828b8917f19cad4656 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Mon, 8 Jul 2024 15:15:25 -0400
Subject: [PATCH] [libc++] Return to using DYLD_LIBRARY_PATH for the system
 libc++ tests

This is the only way to reliably test the system-built libc++. This
effectively reverts 2cf2f1b, which wasn't the right solution to the
problem I encountered. Instead of avoiding the use of DYLD_LIBRARY_PATH,
we instead add some shims that allow the just-built libc++ to behave
almost the same as the real system libc++.

Both solutions are kind of brittle. However, not using DYLD_LIBRARY_PATH
means that we can't test against libc++ the way it is built on Apple
platforms (with an install name of /usr/lib), which is a big problem.
IMO, it makes more sense to instead just shim the rare functionality
that isn't available upstream.
---
 libcxx/cmake/caches/Apple.cmake               |  4 ++
 .../test/configs/apple-libc++-shared.cfg.in   | 35 ++++++++++
 libcxx/utils/ci/apple-install-libcxx.sh       |  6 +-
 libcxxabi/CMakeLists.txt                      |  4 ++
 libcxxabi/lib/typed-operator-new.exp          | 10 +++
 libcxxabi/src/CMakeLists.txt                  | 10 +++
 libcxxabi/src/vendor/apple/shims.cpp          | 65 +++++++++++++++++++
 .../configs/apple-libc++abi-shared.cfg.in     | 29 +++++++++
 8 files changed, 160 insertions(+), 3 deletions(-)
 create mode 100644 libcxx/test/configs/apple-libc++-shared.cfg.in
 create mode 100644 libcxxabi/lib/typed-operator-new.exp
 create mode 100644 libcxxabi/src/vendor/apple/shims.cpp
 create mode 100644 libcxxabi/test/configs/apple-libc++abi-shared.cfg.in

diff --git a/libcxx/cmake/caches/Apple.cmake b/libcxx/cmake/caches/Apple.cmake
index 1038f1aed54e3..d48ec566b81dc 100644
--- a/libcxx/cmake/caches/Apple.cmake
+++ b/libcxx/cmake/caches/Apple.cmake
@@ -14,4 +14,8 @@ set(LIBCXXABI_HERMETIC_STATIC_LIBRARY ON CACHE BOOL "")
 
 set(LIBCXXABI_ENABLE_ASSERTIONS OFF CACHE BOOL "")
 set(LIBCXXABI_ENABLE_FORGIVING_DYNAMIC_CAST ON CACHE BOOL "")
+set(LIBCXXABI_ENABLE_TYPED_NEW_DELETE_DEFINITIONS ON CACHE BOOL "")
 set(LIBCXXABI_USE_LLVM_UNWINDER OFF CACHE BOOL "") # libunwind is built separately
+
+set(LIBCXX_TEST_CONFIG "apple-libc++-shared.cfg.in" CACHE STRING "")
+set(LIBCXXABI_TEST_CONFIG "apple-libc++abi-shared.cfg.in" CACHE STRING "")
diff --git a/libcxx/test/configs/apple-libc++-shared.cfg.in b/libcxx/test/configs/apple-libc++-shared.cfg.in
new file mode 100644
index 0000000000000..2d0aee3ae905e
--- /dev/null
+++ b/libcxx/test/configs/apple-libc++-shared.cfg.in
@@ -0,0 +1,35 @@
+# Testing configuration for Apple's system libc++.
+#
+# This configuration differs from a normal LLVM shared library configuration in
+# that we must use DYLD_LIBRARY_PATH to run the tests against the just-built library,
+# since Apple's libc++ has an absolute install_name.
+#
+# We also don't use a per-target include directory layout, so we have only one
+# include directory for the libc++ headers.
+
+lit_config.load_config(config, '@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg')
+
+config.substitutions.append(('%{flags}',
+    '-isysroot {}'.format('@CMAKE_OSX_SYSROOT@') if '@CMAKE_OSX_SYSROOT@' else ''
+))
+config.substitutions.append(('%{compile_flags}',
+    '-nostdinc++ -I %{include-dir} -I %{libcxx-dir}/test/support'
+))
+config.substitutions.append(('%{link_flags}',
+    '-nostdlib++ -L %{lib-dir} -lc++'
+))
+config.substitutions.append(('%{exec}',
+    '%{executor} --execdir %T --env DYLD_LIBRARY_PATH=%{lib-dir} -- '
+))
+
+config.stdlib = 'apple-libc++'
+
+import os, site
+site.addsitedir(os.path.join('@LIBCXX_SOURCE_DIR@', 'utils'))
+import libcxx.test.params, libcxx.test.config
+libcxx.test.config.configure(
+    libcxx.test.params.DEFAULT_PARAMETERS,
+    libcxx.test.features.DEFAULT_FEATURES,
+    config,
+    lit_config
+)
diff --git a/libcxx/utils/ci/apple-install-libcxx.sh b/libcxx/utils/ci/apple-install-libcxx.sh
index ddefabe77264f..15504f58a0f48 100755
--- a/libcxx/utils/ci/apple-install-libcxx.sh
+++ b/libcxx/utils/ci/apple-install-libcxx.sh
@@ -127,9 +127,9 @@ for arch in ${architectures}; do
                 -DCMAKE_OSX_ARCHITECTURES="${arch}" \
                 -DLIBCXXABI_LIBRARY_VERSION="${version}" \
                 -DLIBCXX_LIBRARY_VERSION="${version}" \
-                -DLIBCXX_TEST_PARAMS="target_triple=${target};stdlib=apple-libc++" \
-                -DLIBCXXABI_TEST_PARAMS="target_triple=${target};stdlib=apple-libc++" \
-                -DLIBUNWIND_TEST_PARAMS="target_triple=${target};stdlib=apple-libc++"
+                -DLIBCXX_TEST_PARAMS="target_triple=${target}" \
+                -DLIBCXXABI_TEST_PARAMS="target_triple=${target}" \
+                -DLIBUNWIND_TEST_PARAMS="target_triple=${target}"
 
     if [ "$headers_only" = true ]; then
         xcrun cmake --build "${build_dir}/${arch}" --target install-cxx-headers install-cxxabi-headers -- -v
diff --git a/libcxxabi/CMakeLists.txt b/libcxxabi/CMakeLists.txt
index 43400c6e8d9af..a0f935327cf36 100644
--- a/libcxxabi/CMakeLists.txt
+++ b/libcxxabi/CMakeLists.txt
@@ -70,6 +70,10 @@ option(LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS
   "Build libc++abi with definitions for operator new/delete. These are normally
    defined in libc++abi, but it is also possible to define them in libc++, in
    which case the definition in libc++abi should be turned off." ON)
+
+option(LIBCXXABI_ENABLE_TYPED_NEW_DELETE_DEFINITIONS
+  "Build libc++abi with definitions for typed operator new/delete." OFF)
+
 option(LIBCXXABI_BUILD_32_BITS "Build 32 bit multilib libc++abi. This option is not supported anymore when building the runtimes. Please specify a full triple instead." ${LLVM_BUILD_32_BITS})
 if (LIBCXXABI_BUILD_32_BITS)
   message(FATAL_ERROR "LIBCXXABI_BUILD_32_BITS is not supported anymore when building the runtimes, please specify a full triple instead.")
diff --git a/libcxxabi/lib/typed-operator-new.exp b/libcxxabi/lib/typed-operator-new.exp
new file mode 100644
index 0000000000000..9ec703165496e
--- /dev/null
+++ b/libcxxabi/lib/typed-operator-new.exp
@@ -0,0 +1,10 @@
+__ZdaPvmSt19__type_descriptor_t
+__ZdaPvRKSt9nothrow_tSt19__type_descriptor_t
+__ZdaPvSt19__type_descriptor_t
+__ZdlPvmSt19__type_descriptor_t
+__ZdlPvRKSt9nothrow_tSt19__type_descriptor_t
+__ZdlPvSt19__type_descriptor_t
+__ZnamRKSt9nothrow_tSt19__type_descriptor_t
+__ZnamSt19__type_descriptor_t
+__ZnwmRKSt9nothrow_tSt19__type_descriptor_t
+__ZnwmSt19__type_descriptor_t
diff --git a/libcxxabi/src/CMakeLists.txt b/libcxxabi/src/CMakeLists.txt
index c1a7bcb14eb19..967d310598bae 100644
--- a/libcxxabi/src/CMakeLists.txt
+++ b/libcxxabi/src/CMakeLists.txt
@@ -25,6 +25,12 @@ if (LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS)
   )
 endif()
 
+if (LIBCXXABI_ENABLE_TYPED_NEW_DELETE_DEFINITIONS)
+  list(APPEND LIBCXXABI_SOURCES
+    vendor/apple/shims.cpp
+  )
+endif()
+
 if (LIBCXXABI_ENABLE_EXCEPTIONS)
   list(APPEND LIBCXXABI_SOURCES
     cxa_exception.cpp
@@ -231,6 +237,10 @@ if (LIBCXXABI_ENABLE_SHARED)
     reexport_symbols("${CMAKE_CURRENT_SOURCE_DIR}/../lib/new-delete.exp")
   endif()
 
+  if (LIBCXXABI_ENABLE_TYPED_NEW_DELETE_DEFINITIONS)
+    reexport_symbols("${CMAKE_CURRENT_SOURCE_DIR}/../lib/typed-operator-new.exp")
+  endif()
+
   # Note that std:: exception types are always defined by the library regardless of
   # whether the exception runtime machinery is provided.
   reexport_symbols("${CMAKE_CURRENT_SOURCE_DIR}/../lib/std-exceptions.exp")
diff --git a/libcxxabi/src/vendor/apple/shims.cpp b/libcxxabi/src/vendor/apple/shims.cpp
new file mode 100644
index 0000000000000..65152f7e0e8c9
--- /dev/null
+++ b/libcxxabi/src/vendor/apple/shims.cpp
@@ -0,0 +1,65 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+//
+// These shims implement symbols that are present in the system libc++ on Apple platforms
+// but are not implemented in upstream libc++. This allows testing libc++ under a system
+// library configuration, which requires the just-built libc++ to be ABI compatible with
+// the system library it is replacing.
+//
+
+#include <cstddef>
+#include <new>
+
+namespace std { // purposefully not versioned, like align_val_t
+enum class __type_descriptor_t : unsigned long long;
+}
+
+_LIBCPP_OVERRIDABLE_FUNC_VIS void* operator new(std::size_t __sz, std::__type_descriptor_t) {
+  return ::operator new(__sz);
+}
+
+_LIBCPP_OVERRIDABLE_FUNC_VIS void* operator new(std::size_t __sz, const std::nothrow_t& __nt,
+                                                std::__type_descriptor_t) noexcept {
+  return ::operator new(__sz, __nt);
+}
+
+_LIBCPP_OVERRIDABLE_FUNC_VIS void* operator new[](std::size_t __sz, std::__type_descriptor_t) {
+  return ::operator new[](__sz);
+}
+
+_LIBCPP_OVERRIDABLE_FUNC_VIS void* operator new[](std::size_t __sz, const std::nothrow_t& __nt,
+                                                  std::__type_descriptor_t) noexcept {
+  return ::operator new(__sz, __nt);
+}
+
+_LIBCPP_OVERRIDABLE_FUNC_VIS void operator delete(void* __p, std::__type_descriptor_t) noexcept {
+  return ::operator delete(__p);
+}
+
+_LIBCPP_OVERRIDABLE_FUNC_VIS void operator delete(void* __p, const std::nothrow_t& __nt,
+                                                  std::__type_descriptor_t) noexcept {
+  return ::operator delete(__p, __nt);
+}
+
+_LIBCPP_OVERRIDABLE_FUNC_VIS void operator delete[](void* __p, std::__type_descriptor_t) noexcept {
+  return ::operator delete[](__p);
+}
+
+_LIBCPP_OVERRIDABLE_FUNC_VIS void operator delete[](void* __p, const std::nothrow_t& __nt,
+                                                    std::__type_descriptor_t) noexcept {
+  return ::operator delete[](__p, __nt);
+}
+
+_LIBCPP_OVERRIDABLE_FUNC_VIS void operator delete(void* __p, std::size_t __sz, std::__type_descriptor_t) noexcept {
+  return ::operator delete(__p, __sz);
+}
+
+_LIBCPP_OVERRIDABLE_FUNC_VIS void operator delete[](void* __p, std::size_t __sz, std::__type_descriptor_t) noexcept {
+  return ::operator delete[](__p, __sz);
+}
diff --git a/libcxxabi/test/configs/apple-libc++abi-shared.cfg.in b/libcxxabi/test/configs/apple-libc++abi-shared.cfg.in
new file mode 100644
index 0000000000000..ec0c93b0134a4
--- /dev/null
+++ b/libcxxabi/test/configs/apple-libc++abi-shared.cfg.in
@@ -0,0 +1,29 @@
+# Testing configuration for Apple's system libc++abi.
+
+lit_config.load_config(config, '@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg')
+
+config.substitutions.append(('%{flags}',
+    '-isysroot {}'.format('@CMAKE_OSX_SYSROOT@') if '@CMAKE_OSX_SYSROOT@' else ''
+))
+config.substitutions.append(('%{compile_flags}',
+    '-nostdinc++ -I %{include} -I %{cxx-include} -I %{cxx-target-include} %{maybe-include-libunwind} -D_LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS ' +
+    '-I %{libcxx}/test/support -I %{libcxx}/src'
+))
+config.substitutions.append(('%{link_flags}',
+    '-nostdlib++ -L %{lib} -lc++ -lc++abi'
+))
+config.substitutions.append(('%{exec}',
+    '%{executor} --execdir %T --env DYLD_LIBRARY_PATH=%{lib} -- '
+))
+
+config.stdlib = 'apple-libc++'
+
+import os, site
+site.addsitedir(os.path.join('@LIBCXXABI_LIBCXX_PATH@', 'utils'))
+import libcxx.test.params, libcxx.test.config
+libcxx.test.config.configure(
+    libcxx.test.params.DEFAULT_PARAMETERS,
+    libcxx.test.features.DEFAULT_FEATURES,
+    config,
+    lit_config
+)



More information about the libcxx-commits mailing list