From 9b40ee8eb0c194f4b2787801ac6f9ef8fc1b8f46 Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Tue, 1 Oct 2019 09:34:58 -0400 Subject: [PATCH] [libc++] Define new/delete in libc++abi only by default Previously, we would define new/delete in both libc++ and libc++abi. Not only does this cause code bloat, but also it's technically an ODR violation since we don't know which operator will be selected. Furthermore, since those are weak definitions, we should strive to have as few of them as possible (to improve load times). My preferred choice would have been to put the operators in libc++ only by default, however that would create a circular dependency between libc++ and libc++abi, which GNU linkers don't handle. Folks who want to ship new/delete in libc++ instead of libc++abi are free to do so by turning on LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS at CMake configure time. On Apple platforms, this shouldn't be an ABI break because we re-export the new/delete symbols from libc++abi. This change actually makes libc++ behave closer to the system libc++ shipped on Apple platforms. On other platforms, this is an ABI break for people linking against libc++ but not libc++abi. However, vendors have been consulted in D68269 and no objection was raised. Furthermore, the definitions can be controlled to appear in libc++ instead with the CMake option. Differential Revision: https://reviews.llvm.org/D68269 --- libcxx/CMakeLists.txt | 8 +-- libcxx/cmake/caches/Apple.cmake | 2 - libcxx/docs/ReleaseNotes.rst | 8 ++- libcxx/lib/abi/CHANGELOG.TXT | 59 +++++++++++++++++++++ libcxx/lib/abi/CMakeLists.txt | 2 +- libcxx/lib/abi/x86_64-apple-darwin.v1.abilist | 60 ++++++++++++++-------- libcxx/lib/abi/x86_64-unknown-linux-gnu.v1.abilist | 27 +++------- libcxx/src/CMakeLists.txt | 5 -- libcxxabi/CMakeLists.txt | 9 ++-- libcxxabi/src/CMakeLists.txt | 2 +- 10 files changed, 121 insertions(+), 61 deletions(-) diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt index 69ff183..015a359 100644 --- a/libcxx/CMakeLists.txt +++ b/libcxx/CMakeLists.txt @@ -232,10 +232,10 @@ option(LIBCXX_ENABLE_ABI_LINKER_SCRIPT ${ENABLE_LINKER_SCRIPT_DEFAULT_VALUE}) option(LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS - "Build libc++ with definitions for operator new/delete. This option can - be used to disable the definitions when libc++abi is expected to provide - them" ON) - + "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) # 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." OFF) diff --git a/libcxx/cmake/caches/Apple.cmake b/libcxx/cmake/caches/Apple.cmake index 38f2c4c..e6221cd 100644 --- a/libcxx/cmake/caches/Apple.cmake +++ b/libcxx/cmake/caches/Apple.cmake @@ -9,11 +9,9 @@ set(LIBCXX_ENABLE_STATIC OFF CACHE BOOL "") set(LIBCXX_ENABLE_SHARED ON CACHE BOOL "") set(LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION "1" CACHE STRING "") set(LIBCXX_CXX_ABI libcxxabi CACHE STRING "") -set(LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS OFF CACHE BOOL "") set(LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT ON CACHE BOOL "") set(LIBCXX_ENABLE_DEBUG_MODE_SUPPORT OFF CACHE BOOL "") -set(LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS ON CACHE BOOL "") set(LIBCXXABI_ENABLE_PIC OFF CACHE BOOL "") set(LIBCXXABI_ENABLE_ASSERTIONS OFF CACHE BOOL "") set(LIBCXXABI_ENABLE_FORGIVING_DYNAMIC_CAST ON CACHE BOOL "") diff --git a/libcxx/docs/ReleaseNotes.rst b/libcxx/docs/ReleaseNotes.rst index 5157685..6079b6f 100644 --- a/libcxx/docs/ReleaseNotes.rst +++ b/libcxx/docs/ReleaseNotes.rst @@ -42,4 +42,10 @@ New Features API Changes ----------- -- ... +- By default, libc++ will _not_ include the definition for new and delete, + since those are provided in libc++abi. Vendors wishing to provide new and + delete in libc++ can build the library with ``-DLIBCXX_ENABLE_NEW_DELETE_DEFINITIONS=ON`` + to get back the old behavior. This was done to avoid providing new and delete + in both libc++ and libc++abi, which is technically an ODR violation. Also + note that we couldn't decide to put the operators in libc++ only, because + they are needed from libc++abi (which would create a circular dependency). diff --git a/libcxx/lib/abi/CHANGELOG.TXT b/libcxx/lib/abi/CHANGELOG.TXT index 1720e86..ff6954e 100644 --- a/libcxx/lib/abi/CHANGELOG.TXT +++ b/libcxx/lib/abi/CHANGELOG.TXT @@ -16,6 +16,65 @@ New entries should be added directly below the "Version" header. Version 12.0 ------------ +* XXXXXXX - [libc++] Define new/delete in libc++abi only by default + + By default, libc++ does not include the definition for new and delete anymore. + Those were previously defined in both libc++ and libc++abi, which was an + ODR violation. + + x86_64-apple-apple-darwin + ------------------------- + The following symbols are now re-exported from libc++abi instead of exported + by libc++ directly (this should not be an ABI break): + + Symbol moved: __ZdaPv + Symbol moved: __ZdaPvm + Symbol moved: __ZdaPvmSt11align_val_t + Symbol moved: __ZdaPvRKSt9nothrow_t + Symbol moved: __ZdaPvSt11align_val_t + Symbol moved: __ZdaPvSt11align_val_tRKSt9nothrow_t + Symbol moved: __ZdlPv + Symbol moved: __ZdlPvm + Symbol moved: __ZdlPvmSt11align_val_t + Symbol moved: __ZdlPvRKSt9nothrow_t + Symbol moved: __ZdlPvSt11align_val_t + Symbol moved: __ZdlPvSt11align_val_tRKSt9nothrow_t + Symbol moved: __Znam + Symbol moved: __ZnamRKSt9nothrow_t + Symbol moved: __ZnamSt11align_val_t + Symbol moved: __ZnamSt11align_val_tRKSt9nothrow_t + Symbol moved: __Znwm + Symbol moved: __ZnwmRKSt9nothrow_t + Symbol moved: __ZnwmSt11align_val_t + Symbol moved: __ZnwmSt11align_val_tRKSt9nothrow_t + + x86_64-unknown-linux-gnu + ------------------------ + The following symbols were moved to libc++abi, but are NOT being re-exported + by libc++. This constitutes an ABI break if one links against libc++ but + not libc++abi. + + Symbol moved: _ZdaPv + Symbol moved: _ZdaPvm + Symbol moved: _ZdaPvmSt11align_val_t + Symbol moved: _ZdaPvRKSt9nothrow_t + Symbol moved: _ZdaPvSt11align_val_t + Symbol moved: _ZdaPvSt11align_val_tRKSt9nothrow_t + Symbol moved: _ZdlPv + Symbol moved: _ZdlPvm + Symbol moved: _ZdlPvmSt11align_val_t + Symbol moved: _ZdlPvRKSt9nothrow_t + Symbol moved: _ZdlPvSt11align_val_t + Symbol moved: _ZdlPvSt11align_val_tRKSt9nothrow_t + Symbol moved: _Znam + Symbol moved: _ZnamRKSt9nothrow_t + Symbol moved: _ZnamSt11align_val_t + Symbol moved: _ZnamSt11align_val_tRKSt9nothrow_t + Symbol moved: _Znwm + Symbol moved: _ZnwmRKSt9nothrow_t + Symbol moved: _ZnwmSt11align_val_t + Symbol moved: _ZnwmSt11align_val_tRKSt9nothrow_t + * 4f13b9992971 - [libc++] Simplify how we re-export symbols from libc++abi We re-export some symbols that were exported from libc++abi but not from diff --git a/libcxx/lib/abi/CMakeLists.txt b/libcxx/lib/abi/CMakeLists.txt index cf7457f..2b3551d 100644 --- a/libcxx/lib/abi/CMakeLists.txt +++ b/libcxx/lib/abi/CMakeLists.txt @@ -23,7 +23,7 @@ if (EXISTS "${ABILIST_FILE}" (APPLE AND "${LIBCXX_CXX_ABI_LIBNAME}" STREQUAL "default")) AND NOT LIBCXX_ABI_UNSTABLE AND LIBCXX_ENABLE_EXCEPTIONS - AND LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS) + AND NOT LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS) add_custom_target(check-cxx-abilist ${SYMDIFF_EXE} --only-stdlib-symbols --strict ${ABILIST_FILE} $ diff --git a/libcxx/lib/abi/x86_64-apple-darwin.v1.abilist b/libcxx/lib/abi/x86_64-apple-darwin.v1.abilist index e141feb..485eea6 100644 --- a/libcxx/lib/abi/x86_64-apple-darwin.v1.abilist +++ b/libcxx/lib/abi/x86_64-apple-darwin.v1.abilist @@ -2258,26 +2258,46 @@ {'is_defined': True, 'name': '__ZTv0_n24_NSt3__114basic_iostreamIcNS_11char_traitsIcEEED1Ev', 'type': 'FUNC'} {'is_defined': True, 'name': '__ZTv0_n24_NSt3__19strstreamD0Ev', 'type': 'FUNC'} {'is_defined': True, 'name': '__ZTv0_n24_NSt3__19strstreamD1Ev', 'type': 'FUNC'} -{'is_defined': True, 'name': '__ZdaPv', 'type': 'FUNC'} -{'is_defined': True, 'name': '__ZdaPvRKSt9nothrow_t', 'type': 'FUNC'} -{'is_defined': True, 'name': '__ZdaPvSt11align_val_t', 'type': 'FUNC'} -{'is_defined': True, 'name': '__ZdaPvSt11align_val_tRKSt9nothrow_t', 'type': 'FUNC'} -{'is_defined': True, 'name': '__ZdaPvm', 'type': 'FUNC'} -{'is_defined': True, 'name': '__ZdaPvmSt11align_val_t', 'type': 'FUNC'} -{'is_defined': True, 'name': '__ZdlPv', 'type': 'FUNC'} -{'is_defined': True, 'name': '__ZdlPvRKSt9nothrow_t', 'type': 'FUNC'} -{'is_defined': True, 'name': '__ZdlPvSt11align_val_t', 'type': 'FUNC'} -{'is_defined': True, 'name': '__ZdlPvSt11align_val_tRKSt9nothrow_t', 'type': 'FUNC'} -{'is_defined': True, 'name': '__ZdlPvm', 'type': 'FUNC'} -{'is_defined': True, 'name': '__ZdlPvmSt11align_val_t', 'type': 'FUNC'} -{'is_defined': True, 'name': '__Znam', 'type': 'FUNC'} -{'is_defined': True, 'name': '__ZnamRKSt9nothrow_t', 'type': 'FUNC'} -{'is_defined': True, 'name': '__ZnamSt11align_val_t', 'type': 'FUNC'} -{'is_defined': True, 'name': '__ZnamSt11align_val_tRKSt9nothrow_t', 'type': 'FUNC'} -{'is_defined': True, 'name': '__Znwm', 'type': 'FUNC'} -{'is_defined': True, 'name': '__ZnwmRKSt9nothrow_t', 'type': 'FUNC'} -{'is_defined': True, 'name': '__ZnwmSt11align_val_t', 'type': 'FUNC'} -{'is_defined': True, 'name': '__ZnwmSt11align_val_tRKSt9nothrow_t', 'type': 'FUNC'} +{'is_defined': False, 'name': '__ZdaPv', 'type': 'U'} +{'is_defined': True, 'name': '__ZdaPv', 'type': 'I'} +{'is_defined': False, 'name': '__ZdaPvRKSt9nothrow_t', 'type': 'U'} +{'is_defined': True, 'name': '__ZdaPvRKSt9nothrow_t', 'type': 'I'} +{'is_defined': False, 'name': '__ZdaPvSt11align_val_t', 'type': 'U'} +{'is_defined': True, 'name': '__ZdaPvSt11align_val_t', 'type': 'I'} +{'is_defined': False, 'name': '__ZdaPvSt11align_val_tRKSt9nothrow_t', 'type': 'U'} +{'is_defined': True, 'name': '__ZdaPvSt11align_val_tRKSt9nothrow_t', 'type': 'I'} +{'is_defined': False, 'name': '__ZdaPvm', 'type': 'U'} +{'is_defined': True, 'name': '__ZdaPvm', 'type': 'I'} +{'is_defined': False, 'name': '__ZdaPvmSt11align_val_t', 'type': 'U'} +{'is_defined': True, 'name': '__ZdaPvmSt11align_val_t', 'type': 'I'} +{'is_defined': False, 'name': '__ZdlPv', 'type': 'U'} +{'is_defined': True, 'name': '__ZdlPv', 'type': 'I'} +{'is_defined': False, 'name': '__ZdlPvRKSt9nothrow_t', 'type': 'U'} +{'is_defined': True, 'name': '__ZdlPvRKSt9nothrow_t', 'type': 'I'} +{'is_defined': False, 'name': '__ZdlPvSt11align_val_t', 'type': 'U'} +{'is_defined': True, 'name': '__ZdlPvSt11align_val_t', 'type': 'I'} +{'is_defined': False, 'name': '__ZdlPvSt11align_val_tRKSt9nothrow_t', 'type': 'U'} +{'is_defined': True, 'name': '__ZdlPvSt11align_val_tRKSt9nothrow_t', 'type': 'I'} +{'is_defined': False, 'name': '__ZdlPvm', 'type': 'U'} +{'is_defined': True, 'name': '__ZdlPvm', 'type': 'I'} +{'is_defined': False, 'name': '__ZdlPvmSt11align_val_t', 'type': 'U'} +{'is_defined': True, 'name': '__ZdlPvmSt11align_val_t', 'type': 'I'} +{'is_defined': False, 'name': '__Znam', 'type': 'U'} +{'is_defined': True, 'name': '__Znam', 'type': 'I'} +{'is_defined': False, 'name': '__ZnamRKSt9nothrow_t', 'type': 'U'} +{'is_defined': True, 'name': '__ZnamRKSt9nothrow_t', 'type': 'I'} +{'is_defined': False, 'name': '__ZnamSt11align_val_t', 'type': 'U'} +{'is_defined': True, 'name': '__ZnamSt11align_val_t', 'type': 'I'} +{'is_defined': False, 'name': '__ZnamSt11align_val_tRKSt9nothrow_t', 'type': 'U'} +{'is_defined': True, 'name': '__ZnamSt11align_val_tRKSt9nothrow_t', 'type': 'I'} +{'is_defined': False, 'name': '__Znwm', 'type': 'U'} +{'is_defined': True, 'name': '__Znwm', 'type': 'I'} +{'is_defined': False, 'name': '__ZnwmRKSt9nothrow_t', 'type': 'U'} +{'is_defined': True, 'name': '__ZnwmRKSt9nothrow_t', 'type': 'I'} +{'is_defined': False, 'name': '__ZnwmSt11align_val_t', 'type': 'U'} +{'is_defined': True, 'name': '__ZnwmSt11align_val_t', 'type': 'I'} +{'is_defined': False, 'name': '__ZnwmSt11align_val_tRKSt9nothrow_t', 'type': 'U'} +{'is_defined': True, 'name': '__ZnwmSt11align_val_tRKSt9nothrow_t', 'type': 'I'} {'is_defined': False, 'name': '___cxa_allocate_exception', 'type': 'U'} {'is_defined': True, 'name': '___cxa_allocate_exception', 'type': 'I'} {'is_defined': False, 'name': '___cxa_atexit', 'type': 'U'} diff --git a/libcxx/lib/abi/x86_64-unknown-linux-gnu.v1.abilist b/libcxx/lib/abi/x86_64-unknown-linux-gnu.v1.abilist index e646b86..dad6856 100644 --- a/libcxx/lib/abi/x86_64-unknown-linux-gnu.v1.abilist +++ b/libcxx/lib/abi/x86_64-unknown-linux-gnu.v1.abilist @@ -1491,7 +1491,6 @@ {'is_defined': False, 'name': '_ZNSt9bad_allocC1Ev', 'type': 'FUNC'} {'is_defined': False, 'name': '_ZNSt9bad_allocD1Ev', 'type': 'FUNC'} {'is_defined': False, 'name': '_ZNSt9exceptionD2Ev', 'type': 'FUNC'} -{'is_defined': False, 'name': '_ZSt15get_new_handlerv', 'type': 'FUNC'} {'is_defined': True, 'name': '_ZSt17__throw_bad_allocv', 'type': 'FUNC'} {'is_defined': True, 'name': '_ZSt17current_exceptionv', 'type': 'FUNC'} {'is_defined': True, 'name': '_ZSt17rethrow_exceptionSt13exception_ptr', 'type': 'FUNC'} @@ -1882,26 +1881,12 @@ {'is_defined': True, 'name': '_ZTv0_n24_NSt3__114basic_iostreamIcNS_11char_traitsIcEEED1Ev', 'type': 'FUNC'} {'is_defined': True, 'name': '_ZTv0_n24_NSt3__19strstreamD0Ev', 'type': 'FUNC'} {'is_defined': True, 'name': '_ZTv0_n24_NSt3__19strstreamD1Ev', 'type': 'FUNC'} -{'is_defined': True, 'name': '_ZdaPv', 'type': 'FUNC'} -{'is_defined': True, 'name': '_ZdaPvRKSt9nothrow_t', 'type': 'FUNC'} -{'is_defined': True, 'name': '_ZdaPvSt11align_val_t', 'type': 'FUNC'} -{'is_defined': True, 'name': '_ZdaPvSt11align_val_tRKSt9nothrow_t', 'type': 'FUNC'} -{'is_defined': True, 'name': '_ZdaPvm', 'type': 'FUNC'} -{'is_defined': True, 'name': '_ZdaPvmSt11align_val_t', 'type': 'FUNC'} -{'is_defined': True, 'name': '_ZdlPv', 'type': 'FUNC'} -{'is_defined': True, 'name': '_ZdlPvRKSt9nothrow_t', 'type': 'FUNC'} -{'is_defined': True, 'name': '_ZdlPvSt11align_val_t', 'type': 'FUNC'} -{'is_defined': True, 'name': '_ZdlPvSt11align_val_tRKSt9nothrow_t', 'type': 'FUNC'} -{'is_defined': True, 'name': '_ZdlPvm', 'type': 'FUNC'} -{'is_defined': True, 'name': '_ZdlPvmSt11align_val_t', 'type': 'FUNC'} -{'is_defined': True, 'name': '_Znam', 'type': 'FUNC'} -{'is_defined': True, 'name': '_ZnamRKSt9nothrow_t', 'type': 'FUNC'} -{'is_defined': True, 'name': '_ZnamSt11align_val_t', 'type': 'FUNC'} -{'is_defined': True, 'name': '_ZnamSt11align_val_tRKSt9nothrow_t', 'type': 'FUNC'} -{'is_defined': True, 'name': '_Znwm', 'type': 'FUNC'} -{'is_defined': True, 'name': '_ZnwmRKSt9nothrow_t', 'type': 'FUNC'} -{'is_defined': True, 'name': '_ZnwmSt11align_val_t', 'type': 'FUNC'} -{'is_defined': True, 'name': '_ZnwmSt11align_val_tRKSt9nothrow_t', 'type': 'FUNC'} +{'is_defined': False, 'name': '_ZdaPv', 'type': 'FUNC'} +{'is_defined': False, 'name': '_ZdaPvSt11align_val_t', 'type': 'FUNC'} +{'is_defined': False, 'name': '_ZdlPv', 'type': 'FUNC'} +{'is_defined': False, 'name': '_Znam', 'type': 'FUNC'} +{'is_defined': False, 'name': '_ZnamSt11align_val_t', 'type': 'FUNC'} +{'is_defined': False, 'name': '_Znwm', 'type': 'FUNC'} {'is_defined': False, 'name': '__cxa_allocate_exception', 'type': 'FUNC'} {'is_defined': False, 'name': '__cxa_begin_catch', 'type': 'FUNC'} {'is_defined': False, 'name': '__cxa_current_primary_exception', 'type': 'FUNC'} diff --git a/libcxx/src/CMakeLists.txt b/libcxx/src/CMakeLists.txt index 3c5bc1e..22faf19 100644 --- a/libcxx/src/CMakeLists.txt +++ b/libcxx/src/CMakeLists.txt @@ -222,11 +222,6 @@ if (LIBCXX_ENABLE_SHARED) "-Wl,-reexported_symbols_list,${CMAKE_CURRENT_SOURCE_DIR}/../lib/libc++abi.v${LIBCXX_LIBCPPABI_VERSION}.exp" "-Wl,-force_symbols_not_weak_list,${CMAKE_CURRENT_SOURCE_DIR}/../lib/notweak.exp" "-Wl,-force_symbols_weak_list,${CMAKE_CURRENT_SOURCE_DIR}/../lib/weak.exp") - - if (NOT LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS) - target_link_libraries(cxx_shared PRIVATE - "-Wl,-reexported_symbols_list,${CMAKE_CURRENT_SOURCE_DIR}/../../libcxxabi/lib/new-delete.exp") - endif() endif() # Generate a linker script in place of a libc++.so symlink. diff --git a/libcxxabi/CMakeLists.txt b/libcxxabi/CMakeLists.txt index 5f0c726..923b3c6 100644 --- a/libcxxabi/CMakeLists.txt +++ b/libcxxabi/CMakeLists.txt @@ -87,13 +87,10 @@ When the dynamic_cast would normally fail, this option will cause the \ library to try comparing the type_info names to see if they are equal \ instead." OFF) -# FIXME: This option should default to off. Unfortunatly GCC 4.9 fails to link -# programs to due undefined references to new/delete in libc++abi. Once this -# has been fixed or worked around the default value should be changed. -# See https://reviews.llvm.org/D68269 for more details. option(LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS - "Build libc++abi with definitions for operator new/delete. Normally libc++ - provides these definitions" ON) + "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_BUILD_32_BITS "Build 32 bit libc++abi." ${LLVM_BUILD_32_BITS}) option(LIBCXXABI_INCLUDE_TESTS "Generate build targets for the libc++abi unit tests." ${LLVM_INCLUDE_TESTS}) set(LIBCXXABI_LIBDIR_SUFFIX "${LLVM_LIBDIR_SUFFIX}" CACHE STRING diff --git a/libcxxabi/src/CMakeLists.txt b/libcxxabi/src/CMakeLists.txt index c57d6fa..e9e4540 100644 --- a/libcxxabi/src/CMakeLists.txt +++ b/libcxxabi/src/CMakeLists.txt @@ -215,7 +215,7 @@ if (LIBCXXABI_ENABLE_SHARED) export_symbols("${CMAKE_CURRENT_SOURCE_DIR}/../lib/itanium-base.exp") if (LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS) - export_symbols("${CMAKE_CURRENT_SOURCE_DIR}/../lib/new-delete.exp") + reexport_symbols("${CMAKE_CURRENT_SOURCE_DIR}/../lib/new-delete.exp") endif() if (LIBCXXABI_ENABLE_EXCEPTIONS) -- 2.7.4