Conversation
Libc++ and libc++abi have allowed picking whether `operator new` is defined inside libc++ or libc++abi for a long time. By default, the operators are defined inside libc++abi, and most platforms use that. The possibility of defining `operator new` in either library causes significant complexity since we have to maintain an includable source that must work from both libc++ and libc++abi. While working on llvm#215813 where we must detect whether `operator new` has been overridden from a function we'd want to define in `libc++.dylib`, it became apparent how brittle that setup is. This patch removes the option to define `operator new` in libc++. The operators are either defined in libc++abi or not at all. While this may seem like a major breakage, in reality I believe this does not affect most platforms. Indeed, platforms that use the default configuration are not affected. Of the platforms that explicitly put the operators in `libc++.dylib`, it seems that most of them are also merging libc++abi into libc++, so it does not actually matter where the operators are coming from. The one platform where a change would be required is FreeBSD, which uses libcxxrt and seems to really depend on the operators being defined in libc++.dylib.
|
@llvm/pr-subscribers-backend-amdgpu @llvm/pr-subscribers-libcxx Author: Louis Dionne (ldionne) ChangesLibc++ and libc++abi have allowed picking whether The possibility of defining This patch removes the option to define While this may seem like a major breakage, in reality I believe this does not affect most platforms. Indeed, platforms that use the default configuration are not affected. Of the platforms that explicitly put the operators in The one platform where a change would be required is FreeBSD, which uses libcxxrt and seems to really depend on the operators being defined in libc++.dylib. Patch is 20.08 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/218989.diff 24 Files Affected:
diff --git a/clang/cmake/caches/Apple-stage1.cmake b/clang/cmake/caches/Apple-stage1.cmake
index 36937f6ed0b4b..ed30fcfc9cd3d 100644
--- a/clang/cmake/caches/Apple-stage1.cmake
+++ b/clang/cmake/caches/Apple-stage1.cmake
@@ -34,9 +34,6 @@ set(COMPILER_RT_ENABLE_TVOS OFF CACHE BOOL "")
set(BOOTSTRAP_LLVM_ENABLE_LTO ON CACHE BOOL "")
set(CMAKE_BUILD_TYPE RelWithDebInfo CACHE STRING "")
-set(LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS OFF CACHE BOOL "")
-set(LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS ON CACHE BOOL "")
-
set(CLANG_BOOTSTRAP_TARGETS
generate-order-file
check-all
diff --git a/clang/cmake/caches/CrossWinToARMLinux.cmake b/clang/cmake/caches/CrossWinToARMLinux.cmake
index c47c4ac3bb73e..1a63ededd867d 100644
--- a/clang/cmake/caches/CrossWinToARMLinux.cmake
+++ b/clang/cmake/caches/CrossWinToARMLinux.cmake
@@ -21,7 +21,7 @@
# cmake -G Ninja ^
# -DTOOLCHAIN_TARGET_TRIPLE=aarch64-unknown-linux-gnu ^
# -DTOOLCHAIN_TARGET_SYSROOTFS=<path-to-develop-arm-linux-root-fs> ^
-# -DTOOLCHAIN_SHARED_LIBS=OFF ^
+# -DTOOLCHAIN_SHARED_LIBS=OFF ^
# -DCMAKE_INSTALL_PREFIX=../install ^
# -DCMAKE_CXX_FLAGS="-D__OPTIMIZE__" ^
# -DREMOTE_TEST_HOST="<hostname>" ^
@@ -205,21 +205,19 @@ set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_COMPILER_RT_CXX_LIBRARY
# The compiler-rt tests disable the clang configuration files during the execution by setting CLANG_NO_DEFAULT_CONFIG=1
# and drops out the --sysroot from there. Provide it explicity via the test flags here if target sysroot has been specified.
set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_COMPILER_RT_TEST_COMPILER_CFLAGS "--stdlib=libc++ ${sysroot_flags}" CACHE STRING "")
-
+
set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBUNWIND_ENABLE_SHARED ${TOOLCHAIN_SHARED_LIBS} CACHE BOOL "")
set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXXABI_ENABLE_STATIC_UNWINDER ${TOOLCHAIN_USE_STATIC_LIBS} CACHE BOOL "")
set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
-set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS OFF CACHE BOOL "")
set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXXABI_ENABLE_SHARED ${TOOLCHAIN_SHARED_LIBS} CACHE BOOL "")
set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXX_ENABLE_SHARED ${TOOLCHAIN_SHARED_LIBS} CACHE BOOL "")
set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXX_ABI_VERSION ${LIBCXX_ABI_VERSION} CACHE STRING "")
set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXX_CXX_ABI "libcxxabi" CACHE STRING "") #!!!
-set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS ON CACHE BOOL "")
# Merge libc++ and libc++abi libraries into the single libc++ library file.
set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXX_ENABLE_STATIC_ABI_LIBRARY ${TOOLCHAIN_USE_STATIC_LIBS} CACHE BOOL "")
# Forcely disable the libc++ benchmarks on Windows build hosts
diff --git a/clang/cmake/caches/Fuchsia-stage2.cmake b/clang/cmake/caches/Fuchsia-stage2.cmake
index 9d549fe94f43c..f22c657040701 100644
--- a/clang/cmake/caches/Fuchsia-stage2.cmake
+++ b/clang/cmake/caches/Fuchsia-stage2.cmake
@@ -282,7 +282,6 @@ if(FUCHSIA_SDK)
set(RUNTIMES_${target}+asan_LLVM_BUILD_COMPILER_RT OFF CACHE BOOL "")
set(RUNTIMES_${target}+asan_LLVM_USE_SANITIZER "Address" CACHE STRING "")
set(RUNTIMES_${target}+asan_LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS OFF CACHE BOOL "")
- set(RUNTIMES_${target}+asan_LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS OFF CACHE BOOL "")
set(RUNTIMES_${target}+noexcept_LLVM_BUILD_COMPILER_RT OFF CACHE BOOL "")
set(RUNTIMES_${target}+noexcept_LIBCXXABI_ENABLE_EXCEPTIONS OFF CACHE BOOL "")
@@ -291,7 +290,6 @@ if(FUCHSIA_SDK)
set(RUNTIMES_${target}+asan+noexcept_LLVM_BUILD_COMPILER_RT OFF CACHE BOOL "")
set(RUNTIMES_${target}+asan+noexcept_LLVM_USE_SANITIZER "Address" CACHE STRING "")
set(RUNTIMES_${target}+asan+noexcept_LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS OFF CACHE BOOL "")
- set(RUNTIMES_${target}+asan+noexcept_LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS OFF CACHE BOOL "")
set(RUNTIMES_${target}+asan+noexcept_LIBCXXABI_ENABLE_EXCEPTIONS OFF CACHE BOOL "")
set(RUNTIMES_${target}+asan+noexcept_LIBCXX_ENABLE_EXCEPTIONS OFF CACHE BOOL "")
@@ -307,13 +305,11 @@ if(FUCHSIA_SDK)
set(RUNTIMES_aarch64-unknown-fuchsia+hwasan_LLVM_BUILD_COMPILER_RT OFF CACHE BOOL "")
set(RUNTIMES_aarch64-unknown-fuchsia+hwasan_LLVM_USE_SANITIZER "HWAddress" CACHE STRING "")
set(RUNTIMES_aarch64-unknown-fuchsia+hwasan_LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS OFF CACHE BOOL "")
- set(RUNTIMES_aarch64-unknown-fuchsia+hwasan_LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS OFF CACHE BOOL "")
# HWASan+noexcept
set(RUNTIMES_aarch64-unknown-fuchsia+hwasan+noexcept_LLVM_BUILD_COMPILER_RT OFF CACHE BOOL "")
set(RUNTIMES_aarch64-unknown-fuchsia+hwasan+noexcept_LLVM_USE_SANITIZER "HWAddress" CACHE STRING "")
set(RUNTIMES_aarch64-unknown-fuchsia+hwasan+noexcept_LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS OFF CACHE BOOL "")
- set(RUNTIMES_aarch64-unknown-fuchsia+hwasan+noexcept_LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS OFF CACHE BOOL "")
set(RUNTIMES_aarch64-unknown-fuchsia+hwasan+noexcept_LIBCXXABI_ENABLE_EXCEPTIONS OFF CACHE BOOL "")
set(RUNTIMES_aarch64-unknown-fuchsia+hwasan+noexcept_LIBCXX_ENABLE_EXCEPTIONS OFF CACHE BOOL "")
diff --git a/clang/cmake/caches/cross-linux-toolchain.cmake b/clang/cmake/caches/cross-linux-toolchain.cmake
index 8065961f7fcfe..fc456831a7892 100644
--- a/clang/cmake/caches/cross-linux-toolchain.cmake
+++ b/clang/cmake/caches/cross-linux-toolchain.cmake
@@ -273,7 +273,6 @@ foreach(target ${LLVM_RUNTIME_TARGETS})
set(RUNTIMES_${target}_LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
set(RUNTIMES_${target}_LIBCXX_CXX_ABI "libcxxabi" CACHE STRING "") #!!!
- set(RUNTIMES_${target}_LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS ON CACHE BOOL "")
if (DEFINED TOOLCHAIN_SHARED_LIBS)
diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index e28bbfaa0605d..fcd7b1e6095f3 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -296,11 +296,12 @@ option(LIBCXX_ENABLE_ABI_LINKER_SCRIPT
"Use and install a linker script for the given ABI library"
${ENABLE_LINKER_SCRIPT_DEFAULT_VALUE})
-option(LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS
- "Build libc++ with definitions for operator new/delete. These are normally
- defined in libc++abi, but this option can be used to define them in libc++
- instead. If you define them in libc++, make sure they are NOT defined in
- libc++abi. Doing otherwise is an ODR violation." OFF)
+# TODO: Remove in LLVM 25
+if (LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS)
+ message(FATAL_ERROR "LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS is not supported anymore, operator new is now either
+ provided by an ABI library like libc++abi.dylib, or not at all")
+endif()
+
# Build libc++abi with libunwind. We need this option to determine whether to
# link with libunwind or libgcc_s while running the test cases.
option(LIBCXXABI_USE_LLVM_UNWINDER "Build and use the LLVM unwinder." ON)
diff --git a/libcxx/cmake/caches/AMDGPU.cmake b/libcxx/cmake/caches/AMDGPU.cmake
index ed6e5d78149e7..bf4ba024f7d93 100644
--- a/libcxx/cmake/caches/AMDGPU.cmake
+++ b/libcxx/cmake/caches/AMDGPU.cmake
@@ -4,7 +4,6 @@ set(LIBCXX_ENABLE_EXCEPTIONS OFF CACHE BOOL "")
set(LIBCXX_ENABLE_FILESYSTEM OFF CACHE BOOL "")
set(LIBCXX_ENABLE_LOCALIZATION ON CACHE BOOL "")
set(LIBCXX_ENABLE_MONOTONIC_CLOCK ON CACHE BOOL "")
-set(LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS ON CACHE BOOL "")
set(LIBCXX_ENABLE_RANDOM_DEVICE OFF CACHE BOOL "")
set(LIBCXX_ENABLE_RTTI OFF CACHE BOOL "")
set(LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
@@ -22,7 +21,6 @@ set(RUNTIMES_USE_LIBC "llvm-libc" CACHE STRING "")
# Configuration options for libcxxabi.
set(LIBCXXABI_BAREMETAL ON CACHE BOOL "")
set(LIBCXXABI_ENABLE_EXCEPTIONS OFF CACHE BOOL "")
-set(LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS OFF CACHE BOOL "")
set(LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
set(LIBCXXABI_ENABLE_THREADS OFF CACHE BOOL "")
set(LIBCXXABI_USE_LLVM_UNWINDER OFF CACHE BOOL "")
diff --git a/libcxx/cmake/caches/FreeBSD.cmake b/libcxx/cmake/caches/FreeBSD.cmake
index 7a7d917922b53..7ca4ae863f001 100644
--- a/libcxx/cmake/caches/FreeBSD.cmake
+++ b/libcxx/cmake/caches/FreeBSD.cmake
@@ -5,4 +5,3 @@ set(LIBCXX_ABI_VERSION "1" CACHE STRING "")
set(LIBCXX_ENABLE_STATIC ON CACHE BOOL "")
set(LIBCXX_ENABLE_SHARED ON CACHE BOOL "")
set(LIBCXX_CXX_ABI libcxxrt CACHE STRING "")
-set(LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS ON CACHE BOOL "")
diff --git a/libcxx/cmake/caches/NVPTX.cmake b/libcxx/cmake/caches/NVPTX.cmake
index 34e9cb9a1ddd1..d2733141a4297 100644
--- a/libcxx/cmake/caches/NVPTX.cmake
+++ b/libcxx/cmake/caches/NVPTX.cmake
@@ -4,7 +4,6 @@ set(LIBCXX_ENABLE_EXCEPTIONS OFF CACHE BOOL "")
set(LIBCXX_ENABLE_FILESYSTEM OFF CACHE BOOL "")
set(LIBCXX_ENABLE_LOCALIZATION ON CACHE BOOL "")
set(LIBCXX_ENABLE_MONOTONIC_CLOCK ON CACHE BOOL "")
-set(LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS ON CACHE BOOL "")
set(LIBCXX_ENABLE_RANDOM_DEVICE OFF CACHE BOOL "")
set(LIBCXX_ENABLE_RTTI OFF CACHE BOOL "")
set(LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
@@ -22,7 +21,6 @@ set(RUNTIMES_USE_LIBC "llvm-libc" CACHE STRING "")
# Configuration options for libcxxabi.
set(LIBCXXABI_BAREMETAL ON CACHE BOOL "")
set(LIBCXXABI_ENABLE_EXCEPTIONS OFF CACHE BOOL "")
-set(LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS OFF CACHE BOOL "")
set(LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
set(LIBCXXABI_ENABLE_THREADS OFF CACHE BOOL "")
set(LIBCXXABI_USE_LLVM_UNWINDER OFF CACHE BOOL "")
diff --git a/libcxx/docs/ReleaseNotes/24.rst b/libcxx/docs/ReleaseNotes/24.rst
index 9cf08b5732970..948b364741ee5 100644
--- a/libcxx/docs/ReleaseNotes/24.rst
+++ b/libcxx/docs/ReleaseNotes/24.rst
@@ -91,3 +91,6 @@ ABI Affecting Changes
Build System Changes
--------------------
+
+- The ``LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS`` option has been removed. The various ``operator new`` functions are
+ now either defined in libc++abi or not at all (which still allows for them being provided by an external library).
diff --git a/libcxx/docs/VendorDocumentation.rst b/libcxx/docs/VendorDocumentation.rst
index 38fd5ac73e222..d217250229be2 100644
--- a/libcxx/docs/VendorDocumentation.rst
+++ b/libcxx/docs/VendorDocumentation.rst
@@ -581,7 +581,9 @@ Using libcxxrt on Linux
You will need to keep the source tree of `libcxxrt`_ available
on your build machine and your copy of the libcxxrt shared library must
-be placed where your linker will find it.
+be placed where your linker will find it. You must use a recent version
+of libcxxrt since historical versions did not provide the necessary
+definitions of ``operator new``.
We can now run CMake like:
@@ -590,7 +592,6 @@ We can now run CMake like:
$ cmake -G Ninja -S runtimes -B build \
-DLLVM_ENABLE_RUNTIMES="libcxx" \
-DLIBCXX_CXX_ABI=libcxxrt \
- -DLIBCXX_ENABLE_NEW_DELETE_DEFINITIONS=ON \
-DLIBCXXABI_USE_LLVM_UNWINDER=OFF \
-DLIBCXX_CXX_ABI_INCLUDE_PATHS=path/to/libcxxrt-sources/src
$ ninja -C build install-cxx
diff --git a/libcxx/lib/abi/CMakeLists.txt b/libcxx/lib/abi/CMakeLists.txt
index 8f277aad2dcd5..d3151e1939973 100644
--- a/libcxx/lib/abi/CMakeLists.txt
+++ b/libcxx/lib/abi/CMakeLists.txt
@@ -7,7 +7,7 @@
# Right now, this is done by using the ABI identifier as the filename containing
# the list of symbols exported by libc++ for that configuration, however we could
# make it more sophisticated if the number of ABI-affecting parameters grew.
-function(cxx_abi_list_identifier result triple abi_library abi_version unstable exceptions new_delete_in_libcxx)
+function(cxx_abi_list_identifier result triple abi_library abi_version unstable exceptions)
set(abi_properties)
if ("${triple}" MATCHES "darwin")
@@ -33,11 +33,6 @@ function(cxx_abi_list_identifier result triple abi_library abi_version unstable
else()
list(APPEND abi_properties "noexceptions")
endif()
- if (${new_delete_in_libcxx})
- list(APPEND abi_properties "new")
- else()
- list(APPEND abi_properties "nonew")
- endif()
list(JOIN abi_properties "." tmp)
set(${result} "${tmp}" PARENT_SCOPE)
@@ -54,7 +49,6 @@ cxx_abi_list_identifier(abi_list_identifier
"${LIBCXX_ABI_VERSION}"
"${LIBCXX_ABI_UNSTABLE}"
"${LIBCXX_ENABLE_EXCEPTIONS}"
- "${LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS}"
)
if (TARGET cxx_shared)
diff --git a/libcxx/lib/abi/arm64-apple-darwin.libcxxabi.v1.stable.exceptions.nonew.abilist b/libcxx/lib/abi/arm64-apple-darwin.libcxxabi.v1.stable.exceptions.abilist
similarity index 100%
rename from libcxx/lib/abi/arm64-apple-darwin.libcxxabi.v1.stable.exceptions.nonew.abilist
rename to libcxx/lib/abi/arm64-apple-darwin.libcxxabi.v1.stable.exceptions.abilist
diff --git a/libcxx/lib/abi/i686-linux-android23.libcxxabi.v1.stable.exceptions.nonew.abilist b/libcxx/lib/abi/i686-linux-android23.libcxxabi.v1.stable.exceptions.abilist
similarity index 100%
rename from libcxx/lib/abi/i686-linux-android23.libcxxabi.v1.stable.exceptions.nonew.abilist
rename to libcxx/lib/abi/i686-linux-android23.libcxxabi.v1.stable.exceptions.abilist
diff --git a/libcxx/lib/abi/powerpc-ibm-aix.libcxxabi.v1.stable.exceptions.nonew.abilist b/libcxx/lib/abi/powerpc-ibm-aix.libcxxabi.v1.stable.exceptions.abilist
similarity index 100%
rename from libcxx/lib/abi/powerpc-ibm-aix.libcxxabi.v1.stable.exceptions.nonew.abilist
rename to libcxx/lib/abi/powerpc-ibm-aix.libcxxabi.v1.stable.exceptions.abilist
diff --git a/libcxx/lib/abi/powerpc64-ibm-aix.libcxxabi.v1.stable.exceptions.nonew.abilist b/libcxx/lib/abi/powerpc64-ibm-aix.libcxxabi.v1.stable.exceptions.abilist
similarity index 100%
rename from libcxx/lib/abi/powerpc64-ibm-aix.libcxxabi.v1.stable.exceptions.nonew.abilist
rename to libcxx/lib/abi/powerpc64-ibm-aix.libcxxabi.v1.stable.exceptions.abilist
diff --git a/libcxx/lib/abi/x86_64-apple-darwin.libcxxabi.v1.stable.exceptions.nonew.abilist b/libcxx/lib/abi/x86_64-apple-darwin.libcxxabi.v1.stable.exceptions.abilist
similarity index 100%
rename from libcxx/lib/abi/x86_64-apple-darwin.libcxxabi.v1.stable.exceptions.nonew.abilist
rename to libcxx/lib/abi/x86_64-apple-darwin.libcxxabi.v1.stable.exceptions.abilist
diff --git a/libcxx/lib/abi/x86_64-linux-android23.libcxxabi.v1.stable.exceptions.nonew.abilist b/libcxx/lib/abi/x86_64-linux-android23.libcxxabi.v1.stable.exceptions.abilist
similarity index 100%
rename from libcxx/lib/abi/x86_64-linux-android23.libcxxabi.v1.stable.exceptions.nonew.abilist
rename to libcxx/lib/abi/x86_64-linux-android23.libcxxabi.v1.stable.exceptions.abilist
diff --git a/libcxx/lib/abi/x86_64-unknown-freebsd.libcxxabi.v1.stable.exceptions.nonew.abilist b/libcxx/lib/abi/x86_64-unknown-freebsd.libcxxabi.v1.stable.exceptions.abilist
similarity index 100%
rename from libcxx/lib/abi/x86_64-unknown-freebsd.libcxxabi.v1.stable.exceptions.nonew.abilist
rename to libcxx/lib/abi/x86_64-unknown-freebsd.libcxxabi.v1.stable.exceptions.abilist
diff --git a/libcxx/lib/abi/x86_64-unknown-linux-gnu.libcxxabi.v1.stable.exceptions.nonew.abilist b/libcxx/lib/abi/x86_64-unknown-linux-gnu.libcxxabi.v1.stable.exceptions.abilist
similarity index 100%
rename from libcxx/lib/abi/x86_64-unknown-linux-gnu.libcxxabi.v1.stable.exceptions.nonew.abilist
rename to libcxx/lib/abi/x86_64-unknown-linux-gnu.libcxxabi.v1.stable.exceptions.abilist
diff --git a/libcxx/lib/abi/x86_64-unknown-linux-gnu.libcxxabi.v1.stable.noexceptions.nonew.abilist b/libcxx/lib/abi/x86_64-unknown-linux-gnu.libcxxabi.v1.stable.noexceptions.abilist
similarity index 100%
rename from libcxx/lib/abi/x86_64-unknown-linux-gnu.libcxxabi.v1.stable.noexceptions.nonew.abilist
rename to libcxx/lib/abi/x86_64-unknown-linux-gnu.libcxxabi.v1.stable.noexceptions.abilist
diff --git a/libcxx/src/CMakeLists.txt b/libcxx/src/CMakeLists.txt
index e029f9f29d5ea..d2354b74db7ce 100644
--- a/libcxx/src/CMakeLists.txt
+++ b/libcxx/src/CMakeLists.txt
@@ -135,12 +135,6 @@ if (LIBCXX_ENABLE_FILESYSTEM)
endif()
endif()
-if (LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS)
- list(APPEND LIBCXX_SOURCES
- new.cpp
- )
-endif()
-
if (APPLE AND LLVM_USE_SANITIZER)
if (("${LLVM_USE_SANITIZER}" STREQUAL "Address") OR
("${LLVM_USE_SANITIZER}" STREQUAL "Address;Undefined") OR
@@ -277,22 +271,6 @@ set_target_properties(cxx_static
)
cxx_add_common_build_flags(cxx_static)
-if (LIBCXX_HERMETIC_STATIC_LIBRARY)
- # If the hermetic library doesn't define the operator new/delete functions
- # then its code shouldn't declare them with hidden visibility. They might
- # actually be provided by a shared library at link time.
- if (LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS)
- append_flags_if_supported(CXX_STATIC_LIBRARY_FLAGS -fvisibility-global-new-delete=force-hidden)
- if (NOT CXX_SUPPORTS_FVISIBILITY_GLOBAL_NEW_DELETE_EQ_FORCE_HIDDEN_FLAG)
- append_flags_if_supported(CXX_STATIC_LIBRARY_FLAGS -fvisibility-global-new-delete-hidden)
- endif()
- endif()
- target_compile_options(cxx_static PRIVATE ${CXX_STATIC_LIBRARY_FLAGS})
- # _LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS can be defined in __config_site
- # too. Define it in the same way here, to avoid redefinition conflicts.
- target_compile_definitions(cxx_static PRIVATE _LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS=)
-endif()
-
# Attempt to merge the libc++.a archive and the ABI library archive into one.
if (LIBCXX_STATICALLY_LINK_ABI_IN_STATIC_LIBRARY)
target_link_libraries(cxx_static PRIVATE libcxx-abi-static-objects)
diff --git a/libcxx/src/new.cpp b/libcxx/src/new.cpp
deleted file mode 100644
index 618b4348f1f94..0000000000000
--- a/libcxx/src/new.cpp
+++ /dev/null
@@ -1,20 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// 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
-//
-//===----------------------------------------------------------------------===//
-
-#include <__assert>
-#include <new>
-
-#if !defined(__GLIBCXX__) && !defined(_LIBCPP_ABI_VCRUNTIME)
-
-inline void __throw_bad_alloc_shim() { std::__throw_bad_alloc(); }
-
-# define _LIBCPP_ASSERT_SHIM(expr, str) _LIBCPP_ASSERT(expr, str)
-
-# include "support/new.ipp"
-
-#endif // !__GLIBCXX__ && !_LIBCPP_ABI_VCRUNTIME
diff --git a/libcxx/src/support/new.ipp b/libcxx/src/support/new.ipp
index 2f3af29f99378..a745388be958b 100644
--- a/libcxx/src/support/new.ipp
+++ b/libcxx/src/support/new.ipp
@@ -6,6 +6,8 @@
//
//===----------------------------------------------------------------------===//
+// TODO: This file is now only used from libc++abi. Simplify the implementation accordingly.
+
#include <cstddef>
#include <cstdlib>
#include <new>
diff --git a/libcxxabi/CMakeLists.txt b/libcxxabi/CMakeLists.txt
index 902d4247fe6f7..bc008cef2f887 100644
--- a/libcxxabi/CMakeLists.txt
+++ b/libcxxabi/CMakeLists.txt
@@ -83,9 +83,8 @@ library to try comparing the type_info names to see if they are equal \
instead." OFF)
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)
+ "Build libc++abi with definitions for operator new/delete. It is possible to turn this off if
+ these definitions are provided by another library (e.g. sanitizers)." ON)
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 suppo...
[truncated]
|
ldionne
left a comment
There was a problem hiding this comment.
Pinging the relevant people for each platform.
| set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXX_ENABLE_SHARED ${TOOLCHAIN_SHARED_LIBS} CACHE BOOL "") | ||
| set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXX_ABI_VERSION ${LIBCXX_ABI_VERSION} CACHE STRING "") | ||
| set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXX_CXX_ABI "libcxxabi" CACHE STRING "") #!!! | ||
| set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS ON CACHE BOOL "") |
There was a problem hiding this comment.
@vvereschaka I am assuming that this change would not impact your configuration, since you seem to be merging libc++abi into libc++.
Do you have a mechanism to re-export operator new symbols from libc++abi to libc++ when you don't merge the two libraries? If not, then perhaps a program that links against -lc++ but not explicitly -lc++abi today would fail to link after this change, but I don't think that's something that should ever happen?
| set(RUNTIMES_${target}+asan_LLVM_BUILD_COMPILER_RT OFF CACHE BOOL "") | ||
| set(RUNTIMES_${target}+asan_LLVM_USE_SANITIZER "Address" CACHE STRING "") | ||
| set(RUNTIMES_${target}+asan_LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS OFF CACHE BOOL "") | ||
| set(RUNTIMES_${target}+asan_LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS OFF CACHE BOOL "") |
There was a problem hiding this comment.
@petrhosek This should be a no-op for you since you were already turning this off so they can be overridden by the sanitizer versions.
| set(LIBCXX_ENABLE_FILESYSTEM OFF CACHE BOOL "") | ||
| set(LIBCXX_ENABLE_LOCALIZATION ON CACHE BOOL "") | ||
| set(LIBCXX_ENABLE_MONOTONIC_CLOCK ON CACHE BOOL "") | ||
| set(LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS ON CACHE BOOL "") |
There was a problem hiding this comment.
@jhuber6 Since you're already merging libc++abi into libc++, I don't think it matters where those operators are defined.
| set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "") | ||
| set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXXABI_ENABLE_STATIC_UNWINDER ${TOOLCHAIN_USE_STATIC_LIBS} CACHE BOOL "") | ||
| set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "") | ||
| set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS OFF CACHE BOOL "") |
There was a problem hiding this comment.
Since the default is ON, this removal basically makes the definitions live in libc++abi instead of libc++ for this configuration.
|
|
||
| set(RUNTIMES_${target}_LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "") | ||
| set(RUNTIMES_${target}_LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "") | ||
| set(RUNTIMES_${target}_LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS OFF CACHE BOOL "") |
There was a problem hiding this comment.
@vvereschaka Similarly here, this should only move operator new from libc++ to libc++abi, which should work assuming you link against both (and I assume you do).
| set(BOOTSTRAP_LLVM_ENABLE_LTO ON CACHE BOOL "") | ||
| set(CMAKE_BUILD_TYPE RelWithDebInfo CACHE STRING "") | ||
|
|
||
| set(LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS OFF CACHE BOOL "") |
There was a problem hiding this comment.
This is just re-stating the defaults, so this is a no-op.
| set(LIBCXX_ENABLE_STATIC ON CACHE BOOL "") | ||
| set(LIBCXX_ENABLE_SHARED ON CACHE BOOL "") | ||
| set(LIBCXX_CXX_ABI libcxxrt CACHE STRING "") | ||
| set(LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS ON CACHE BOOL "") |
There was a problem hiding this comment.
@emaste @jrtc27 This change is the only one in this patch that I expect to cause problems. FreeBSD currently defines operator new in libc++.so because (AFAICT) it uses libcxxrt, which does not define all the necessary variants of operator new. I believe the best path forward here would be to ask libcxxrt to implement those missing variants of operator new, and to simply not use operator new from either libc++ or libc++abi. This would align FreeBSD with what other platforms are doing.
There was a problem hiding this comment.
It's a bit of a pain, but probably a maintenance task that is overdue. cc @davidchisnall for possible libcxxrt input. :)
There was a problem hiding this comment.
We can add the missing ones to libcxxrt quite easily. We didn't because it's mostly used with libc++, which defined them and we regarded them as a small set to be used with things that didn't link a standard library.
| set(LIBCXX_ENABLE_FILESYSTEM OFF CACHE BOOL "") | ||
| set(LIBCXX_ENABLE_LOCALIZATION ON CACHE BOOL "") | ||
| set(LIBCXX_ENABLE_MONOTONIC_CLOCK ON CACHE BOOL "") | ||
| set(LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS ON CACHE BOOL "") |
There was a problem hiding this comment.
@jhuber6 Same here: since libc++abi and libc++ are merged, I don't think it matters where the operators are defined. So I expect this change makes no visible difference.
|
Pinging @llvm/libcxx-vendors for awareness. |
Libc++ and libc++abi have allowed picking whether
operator newis defined inside libc++ or libc++abi for a long time. By default, the operators are defined inside libc++abi, and most platforms use that.The possibility of defining
operator newin either library causes significant complexity since we have to maintain an includable source that must work from both libc++ and libc++abi. While working on #215813 where we must detect whetheroperator newhas been overridden from a function we'd want to define inlibc++.dylib, it became apparent how brittle that setup is.This patch removes the option to define
operator newin libc++. The operators are either defined in libc++abi or not at all.While this may seem like a major breakage, in reality I believe this does not affect most platforms. Indeed, platforms that use the default configuration are not affected. Of the platforms that explicitly put the operators in
libc++.dylib, it seems that most of them are also merging libc++abi into libc++, so it does not actually matter where the operators are coming from.The one platform where a change would be required is FreeBSD, which uses libcxxrt and seems to really depend on the operators being defined in libc++.dylib.