[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

diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index 69ff183..015a359 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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}
             $<TARGET_SONAME_FILE:cxx_shared>

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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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)