[libc++] Fix the behavior of throwing operator new under -fno-exceptions

[ldionne]

In D144319, Clang tried to land a change that would cause some functions that are not supposed to return nullptr to optimize better. As reported in https://reviews.llvm.org/D144319#4203982, libc++ started seeing failures in its CI shortly after this change was landed.

As explained in D146379, the reason for these failures is that libc++'s throwing operator new can in fact return nullptr when compiled with exceptions disabled. However, this contradicts the Standard, which clearly says that the throwing version of operator new(size_t) should never return nullptr. This is actually a long standing issue. I've previously seen a case where LTO would optimize incorrectly based on the assumption that operator new doesn't return nullptr, an assumption that was violated in that case because libc++.dylib was compiled with -fno-exceptions.

Unfortunately, fixing this is kind of tricky. The Standard has a few requirements for the allocation functions, some of which are impossible to satisfy under -fno-exceptions:

1. operator new(size_t) must never return nullptr
2. operator new(size_t, nothrow_t) must call the throwing version and return nullptr on failure to allocate
3. We can't throw exceptions when compiled with -fno-exceptions

In the case where exceptions are enabled, things work nicely. new(size_t) throws and new(size_t, nothrow_t) uses a try-catch to return nullptr. However, when compiling the library with -fno-exceptions, we can't throw an exception from new(size_t), and we can't catch anything from new(size_t, nothrow_t). The only thing we can do from new(size_t) is actually abort the program, which does not make it possible for new(size_t, nothrow_t) to catch something and return nullptr.

This patch makes the following changes:

1. When compiled with -fno-exceptions, the throwing version of operator new will now abort on failure instead of returning nullptr on failure. This resolves the issue that the compiler could mis-compile based on the assumption that nullptr is never returned. This constitutes an API and ABI breaking change for folks compiling the library with -fno-exceptions (which is not the general public, who merely uses libc++ headers but use a shared library that has already been compiled). This should mostly impact vendors and other folks who compile libc++.dylib themselves.

2. When the library is compiled with -fexceptions, the nothrow version of operator new has no change. When the library is compiled with -fno-exceptions, the nothrow version of operator new will now check whether the throwing version of operator new has been overridden. If it has not been overridden, then it will use an implementation equivalent to that of the throwing operator new, except it will return nullptr on failure to allocate (instead of terminating). However, if the throwing operator new has been overridden, it is now an error NOT to also override the nothrow operator new. Indeed, there is no way for us to implement a valid nothrow operator new without knowing the exact implementation of the throwing version.

In summary, this change will impact people who fall into the following intersection of conditions:

• They use the libc++ shared/static library built with -fno-exceptions
• They do not override operator new(..., std::nothrow_t)
• They override operator new(...) (the throwing version)
• They use operator new(..., std::nothrow_t)

We believe this represents a small number of people.

Fixes #60129
rdar://103958777

Differential Revision: https://reviews.llvm.org/D150610

[llvmbot]

@llvm/pr-subscribers-libcxxabi

@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)

Changes

In D144319, Clang tried to land a change that would cause some functions that are not supposed to return nullptr to optimize better. As reported in https://reviews.llvm.org/D144319#4203982, libc++ started seeing failures in its CI shortly after this change was landed.

As explained in D146379, the reason for these failures is that libc++'s throwing operator new can in fact return nullptr when compiled with exceptions disabled. However, this contradicts the Standard, which clearly says that the throwing version of operator new(size_t) should never return nullptr. This is actually a long standing issue. I've previously seen a case where LTO would optimize incorrectly based on the assumption that operator new doesn't return nullptr, an assumption that was violated in that case because libc++.dylib was compiled with -fno-exceptions.

Unfortunately, fixing this is kind of tricky. The Standard has a few requirements for the allocation functions, some of which are impossible to satisfy under -fno-exceptions:

1. operator new(size_t) must never return nullptr
2. operator new(size_t, nothrow_t) must call the throwing version and return nullptr on failure to allocate
3. We can't throw exceptions when compiled with -fno-exceptions

In the case where exceptions are enabled, things work nicely. new(size_t) throws and new(size_t, nothrow_t) uses a try-catch to return nullptr. However, when compiling the library with -fno-exceptions, we can't throw an exception from new(size_t), and we can't catch anything from new(size_t, nothrow_t). The only thing we can do from new(size_t) is actually abort the program, which does not make it possible for new(size_t, nothrow_t) to catch something and return nullptr.

This patch makes the following changes:

1. When compiled with -fno-exceptions, the throwing version of operator new will now abort on failure instead of returning nullptr on failure. This resolves the issue that the compiler could mis-compile based on the assumption that nullptr is never returned. This constitutes an API and ABI breaking change for folks compiling the library with -fno-exceptions (which is not the general public, who merely uses libc++ headers but use a shared library that has already been compiled). This should mostly impact vendors and other folks who compile libc++.dylib themselves.

2. When the library is compiled with -fexceptions, the nothrow version of operator new has no change. When the library is compiled with -fno-exceptions, the nothrow version of operator new will now check whether the throwing version of operator new has been overridden. If it has not been overridden, then it will use an implementation equivalent to that of the throwing operator new, except it will return nullptr on failure to allocate (instead of terminating). However, if the throwing operator new has been overridden, it is now an error NOT to also override the nothrow operator new. Indeed, there is no way for us to implement a valid nothrow operator new without knowing the exact implementation of the throwing version.

rdar://103958777

Differential Revision: https://reviews.llvm.org/D150610

---

Patch is 27.22 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/69498.diff

9 Files Affected:

• (modified) libcxx/docs/ReleaseNotes/18.rst (+23)
• (modified) libcxx/include/CMakeLists.txt (+1)
• (added) libcxx/include/__overridable_function (+38)
• (modified) libcxx/include/new (+5-4)
• (modified) libcxx/src/new.cpp (+57-22)
• (added) libcxx/test/libcxx/language.support/support.dynamic/assert.nothrow_new_not_overridden_fno_exceptions.pass.cpp (+56)
• (added) libcxx/test/libcxx/language.support/support.dynamic/new_dont_return_nullptr.pass.cpp (+37)
• (modified) libcxx/test/support/check_assertion.h (+6)
• (modified) libcxxabi/src/stdlib_new_delete.cpp (+68-22)

diff --git a/libcxx/docs/ReleaseNotes/18.rst b/libcxx/docs/ReleaseNotes/18.rst
index ac78563aa73848f..bf017613a01b892 100644
--- a/libcxx/docs/ReleaseNotes/18.rst
+++ b/libcxx/docs/ReleaseNotes/18.rst
@@ -118,6 +118,29 @@ LLVM 20
 ABI Affecting Changes
 ---------------------
 
+- When the shared/static library is built with ``-fno-exceptions``, the behavior of ``operator new`` was changed
+  to make it standards-conforming. In LLVM 17 and before, the throwing versions of ``operator new`` would return
+  ``nullptr`` upon failure to allocate, when the shared/static library was built with exceptions disabled. This
+  was non-conforming, since the throwing versions of ``operator new`` are never expected to return ``nullptr``, and
+  this non-conformance could actually lead to miscompiles in subtle cases.
+
+  Starting in LLVM 18, the throwing versions of ``operator new`` will abort the program when they fail to allocate
+  if the shared/static library has been built with ``-fno-exceptions``. This is consistent with the behavior of all
+  other potentially-throwing functions in the library, which abort the program instead of throwing when ``-fno-exceptions``
+  is used.
+
+  Furthermore, when the shared/static library is built with ``-fno-exceptions``, users who override the throwing
+  version of ``operator new`` will now need to also override the ``std::nothrow_t`` version of ``operator new`` if
+  they want to use it. Indeed, this is because there is no way to implement a conforming ``operator new(nothrow)``
+  from a conforming potentially-throwing ``operator new`` when compiled with ``-fno-exceptions``. In that case, using
+  ``operator new(nothrow)`` without overriding it explicitly but after overriding the throwing ``operator new`` will
+  result in an error.
+
+  Note that this change only impacts vendors/users that build the shared/static library themselves and pass
+  ``-DLIBCXX_ENABLE_EXCEPTIONS=OFF``, which is not the default configuration. If you are using the default
+  configuration of the library, the libc++ shared/static library will be built with exceptions enabled, and
+  there is no change between LLVM 17 and LLVM 18, even for users who build their own code using ``-fno-exceptions``.
+
 - The symbol of a non-visible function part of ``std::system_error`` was removed.
   This is not a breaking change as the private function ``__init`` was never referenced internally outside of the dylib
 
diff --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt
index 9b03430a87d8338..064b9cd3ed8973b 100644
--- a/libcxx/include/CMakeLists.txt
+++ b/libcxx/include/CMakeLists.txt
@@ -565,6 +565,7 @@ set(files
   __numeric/transform_exclusive_scan.h
   __numeric/transform_inclusive_scan.h
   __numeric/transform_reduce.h
+  __overridable_function
   __random/bernoulli_distribution.h
   __random/binomial_distribution.h
   __random/cauchy_distribution.h
diff --git a/libcxx/include/__overridable_function b/libcxx/include/__overridable_function
new file mode 100644
index 000000000000000..941a731e7bc2120
--- /dev/null
+++ b/libcxx/include/__overridable_function
@@ -0,0 +1,38 @@
+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef _LIBCPP___OVERRIDABLE_FUNCTION
+#define _LIBCPP___OVERRIDABLE_FUNCTION
+
+#include <__config>
+#include <cstdint>
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+#define _LIBCPP_MAKE_OVERRIDABLE_FUNCTION_DETECTABLE                                                                   \
+  __attribute__((__section__("__TEXT,__lcxx_override,regular,pure_instructions")))
+
+template <class _Ret, class... _Args>
+_LIBCPP_HIDE_FROM_ABI bool __is_function_overridden(_Ret (*__ptr)(_Args...)) noexcept {
+  // The linker places these at the start/end of the __lcxx_override section,
+  // which is where we put the definitions for any overridable function, via
+  // _LIBCPP_MAKE_OVERRIDABLE_FUNCTION_DETECTABLE.
+  //
+  // Placing the function in a separate section allows for this logic to be applied
+  // even in the scenario where clients are statically linking against libc++/libc++abi.
+  extern const uintptr_t __lcxx_override_start __asm("section$start$__TEXT$__lcxx_override");
+  extern const uintptr_t __lcxx_override_end __asm("section$end$__TEXT$__lcxx_override");
+  uintptr_t* __uptr = reinterpret_cast<uintptr_t*>(__ptr);
+
+  return __uptr < &__lcxx_override_start || __uptr > &__lcxx_override_end;
+}
+
+_LIBCPP_END_NAMESPACE_STD
+
+#endif // _LIBCPP___OVERRIDABLE_FUNCTION
diff --git a/libcxx/include/new b/libcxx/include/new
index 0a97c3e37add574..caeb9a5c8b4b811 100644
--- a/libcxx/include/new
+++ b/libcxx/include/new
@@ -90,6 +90,7 @@ void  operator delete[](void* ptr, void*) noexcept;
 #include <__availability>
 #include <__config>
 #include <__exception/exception.h>
+#include <__overridable_function>
 #include <__type_traits/alignment_of.h>
 #include <__type_traits/is_function.h>
 #include <__type_traits/is_same.h>
@@ -214,7 +215,7 @@ inline constexpr destroying_delete_t destroying_delete{};
 
 #if !defined(_LIBCPP_ABI_VCRUNTIME)
 
-_LIBCPP_NODISCARD_AFTER_CXX17 _LIBCPP_OVERRIDABLE_FUNC_VIS void* operator new(std::size_t __sz) _THROW_BAD_ALLOC;
+_LIBCPP_NODISCARD_AFTER_CXX17 _LIBCPP_OVERRIDABLE_FUNC_VIS _LIBCPP_MAKE_OVERRIDABLE_FUNCTION_DETECTABLE void* operator new(std::size_t __sz) _THROW_BAD_ALLOC;
 _LIBCPP_NODISCARD_AFTER_CXX17 _LIBCPP_OVERRIDABLE_FUNC_VIS void* operator new(std::size_t __sz, const std::nothrow_t&) _NOEXCEPT _LIBCPP_NOALIAS;
 _LIBCPP_OVERRIDABLE_FUNC_VIS void  operator delete(void* __p) _NOEXCEPT;
 _LIBCPP_OVERRIDABLE_FUNC_VIS void  operator delete(void* __p, const std::nothrow_t&) _NOEXCEPT;
@@ -222,7 +223,7 @@ _LIBCPP_OVERRIDABLE_FUNC_VIS void  operator delete(void* __p, const std::nothrow
 _LIBCPP_OVERRIDABLE_FUNC_VIS _LIBCPP_AVAILABILITY_SIZED_NEW_DELETE void  operator delete(void* __p, std::size_t __sz) _NOEXCEPT;
 #endif
 
-_LIBCPP_NODISCARD_AFTER_CXX17 _LIBCPP_OVERRIDABLE_FUNC_VIS void* operator new[](std::size_t __sz) _THROW_BAD_ALLOC;
+_LIBCPP_NODISCARD_AFTER_CXX17 _LIBCPP_OVERRIDABLE_FUNC_VIS _LIBCPP_MAKE_OVERRIDABLE_FUNCTION_DETECTABLE void* operator new[](std::size_t __sz) _THROW_BAD_ALLOC;
 _LIBCPP_NODISCARD_AFTER_CXX17 _LIBCPP_OVERRIDABLE_FUNC_VIS void* operator new[](std::size_t __sz, const std::nothrow_t&) _NOEXCEPT _LIBCPP_NOALIAS;
 _LIBCPP_OVERRIDABLE_FUNC_VIS void  operator delete[](void* __p) _NOEXCEPT;
 _LIBCPP_OVERRIDABLE_FUNC_VIS void  operator delete[](void* __p, const std::nothrow_t&) _NOEXCEPT;
@@ -231,7 +232,7 @@ _LIBCPP_OVERRIDABLE_FUNC_VIS _LIBCPP_AVAILABILITY_SIZED_NEW_DELETE void  operato
 #endif
 
 #ifndef _LIBCPP_HAS_NO_LIBRARY_ALIGNED_ALLOCATION
-_LIBCPP_NODISCARD_AFTER_CXX17 _LIBCPP_OVERRIDABLE_FUNC_VIS void* operator new(std::size_t __sz, std::align_val_t) _THROW_BAD_ALLOC;
+_LIBCPP_NODISCARD_AFTER_CXX17 _LIBCPP_OVERRIDABLE_FUNC_VIS _LIBCPP_MAKE_OVERRIDABLE_FUNCTION_DETECTABLE void* operator new(std::size_t __sz, std::align_val_t) _THROW_BAD_ALLOC;
 _LIBCPP_NODISCARD_AFTER_CXX17 _LIBCPP_OVERRIDABLE_FUNC_VIS void* operator new(std::size_t __sz, std::align_val_t, const std::nothrow_t&) _NOEXCEPT _LIBCPP_NOALIAS;
 _LIBCPP_OVERRIDABLE_FUNC_VIS void  operator delete(void* __p, std::align_val_t) _NOEXCEPT;
 _LIBCPP_OVERRIDABLE_FUNC_VIS void  operator delete(void* __p, std::align_val_t, const std::nothrow_t&) _NOEXCEPT;
@@ -239,7 +240,7 @@ _LIBCPP_OVERRIDABLE_FUNC_VIS void  operator delete(void* __p, std::align_val_t,
 _LIBCPP_OVERRIDABLE_FUNC_VIS _LIBCPP_AVAILABILITY_SIZED_NEW_DELETE void  operator delete(void* __p, std::size_t __sz, std::align_val_t) _NOEXCEPT;
 #endif
 
-_LIBCPP_NODISCARD_AFTER_CXX17 _LIBCPP_OVERRIDABLE_FUNC_VIS void* operator new[](std::size_t __sz, std::align_val_t) _THROW_BAD_ALLOC;
+_LIBCPP_NODISCARD_AFTER_CXX17 _LIBCPP_OVERRIDABLE_FUNC_VIS _LIBCPP_MAKE_OVERRIDABLE_FUNCTION_DETECTABLE void* operator new[](std::size_t __sz, std::align_val_t) _THROW_BAD_ALLOC;
 _LIBCPP_NODISCARD_AFTER_CXX17 _LIBCPP_OVERRIDABLE_FUNC_VIS void* operator new[](std::size_t __sz, std::align_val_t, const std::nothrow_t&) _NOEXCEPT _LIBCPP_NOALIAS;
 _LIBCPP_OVERRIDABLE_FUNC_VIS void  operator delete[](void* __p, std::align_val_t) _NOEXCEPT;
 _LIBCPP_OVERRIDABLE_FUNC_VIS void  operator delete[](void* __p, std::align_val_t, const std::nothrow_t&) _NOEXCEPT;
diff --git a/libcxx/src/new.cpp b/libcxx/src/new.cpp
index 033bba5c1fc95b6..5cbebf74300f443 100644
--- a/libcxx/src/new.cpp
+++ b/libcxx/src/new.cpp
@@ -7,6 +7,8 @@
 //===----------------------------------------------------------------------===//
 
 #include <__memory/aligned_alloc.h>
+#include <__overridable_function>
+#include <cstddef>
 #include <cstdlib>
 #include <new>
 
@@ -15,6 +17,10 @@
 // The code below is copied as-is into libc++abi's libcxxabi/src/stdlib_new_delete.cpp
 // file. The version in this file is the canonical one.
 
+inline void __throw_bad_alloc_shim() { std::__throw_bad_alloc(); }
+
+#  define _LIBCPP_ASSERT_SHIM(expr, str) _LIBCPP_ASSERT(expr, str)
+
 // ------------------ BEGIN COPY ------------------
 // Implement all new and delete operators as weak definitions
 // in this shared library, so that they can be overridden by programs
@@ -38,39 +44,53 @@ static void* operator_new_impl(std::size_t size) noexcept {
 
 _LIBCPP_WEAK void* operator new(std::size_t size) _THROW_BAD_ALLOC {
   void* p = operator_new_impl(size);
-#  ifndef _LIBCPP_HAS_NO_EXCEPTIONS
   if (p == nullptr)
-    throw std::bad_alloc();
-#  endif
+    __throw_bad_alloc_shim();
   return p;
 }
 
 _LIBCPP_WEAK void* operator new(size_t size, const std::nothrow_t&) noexcept {
+#  ifdef _LIBCPP_HAS_NO_EXCEPTIONS
+  _LIBCPP_ASSERT_SHIM(
+      !std::__is_function_overridden(static_cast<void* (*)(std::size_t)>(&operator new)),
+      "libc++ was configured with exceptions disabled and `operator new(size_t)` has been overridden, "
+      "but `operator new(size_t, nothrow_t)` has not been overridden. This is problematic because "
+      "`operator new(size_t, nothrow_t)` must call `operator new(size_t)`, which will terminate in case "
+      "it fails to allocate, making it impossible for `operator new(size_t, nothrow_t)` to fulfill its "
+      "contract (since it should return nullptr upon failure).");
+
+  return operator_new_impl(size);
+#  else
   void* p = nullptr;
-#  ifndef _LIBCPP_HAS_NO_EXCEPTIONS
   try {
-#  endif // _LIBCPP_HAS_NO_EXCEPTIONS
     p = ::operator new(size);
-#  ifndef _LIBCPP_HAS_NO_EXCEPTIONS
   } catch (...) {
   }
-#  endif // _LIBCPP_HAS_NO_EXCEPTIONS
   return p;
+#  endif
 }
 
 _LIBCPP_WEAK void* operator new[](size_t size) _THROW_BAD_ALLOC { return ::operator new(size); }
 
 _LIBCPP_WEAK void* operator new[](size_t size, const std::nothrow_t&) noexcept {
+#  ifdef _LIBCPP_HAS_NO_EXCEPTIONS
+  _LIBCPP_ASSERT_SHIM(
+      !std::__is_function_overridden(static_cast<void* (*)(std::size_t)>(&operator new[])),
+      "libc++ was configured with exceptions disabled and `operator new[](size_t)` has been overridden, "
+      "but `operator new[](size_t, nothrow_t)` has not been overridden. This is problematic because "
+      "`operator new[](size_t, nothrow_t)` must call `operator new[](size_t)`, which will terminate in case "
+      "it fails to allocate, making it impossible for `operator new[](size_t, nothrow_t)` to fulfill its "
+      "contract (since it should return nullptr upon failure).");
+
+  return operator_new_impl(size);
+#  else
   void* p = nullptr;
-#  ifndef _LIBCPP_HAS_NO_EXCEPTIONS
   try {
-#  endif // _LIBCPP_HAS_NO_EXCEPTIONS
     p = ::operator new[](size);
-#  ifndef _LIBCPP_HAS_NO_EXCEPTIONS
   } catch (...) {
   }
-#  endif // _LIBCPP_HAS_NO_EXCEPTIONS
   return p;
+#  endif
 }
 
 _LIBCPP_WEAK void operator delete(void* ptr) noexcept { std::free(ptr); }
@@ -109,24 +129,30 @@ static void* operator_new_aligned_impl(std::size_t size, std::align_val_t alignm
 
 _LIBCPP_WEAK void* operator new(std::size_t size, std::align_val_t alignment) _THROW_BAD_ALLOC {
   void* p = operator_new_aligned_impl(size, alignment);
-#    ifndef _LIBCPP_HAS_NO_EXCEPTIONS
   if (p == nullptr)
-    throw std::bad_alloc();
-#    endif
+    __throw_bad_alloc_shim();
   return p;
 }
 
 _LIBCPP_WEAK void* operator new(size_t size, std::align_val_t alignment, const std::nothrow_t&) noexcept {
+#    ifdef _LIBCPP_HAS_NO_EXCEPTIONS
+  _LIBCPP_ASSERT_SHIM(
+      !std::__is_function_overridden(static_cast<void* (*)(std::size_t, std::align_val_t)>(&operator new)),
+      "libc++ was configured with exceptions disabled and `operator new(size_t, align_val_t)` has been overridden, "
+      "but `operator new(size_t, align_val_t, nothrow_t)` has not been overridden. This is problematic because "
+      "`operator new(size_t, align_val_t, nothrow_t)` must call `operator new(size_t, align_val_t)`, which will "
+      "terminate in case it fails to allocate, making it impossible for `operator new(size_t, align_val_t, nothrow_t)` "
+      "to fulfill its contract (since it should return nullptr upon failure).");
+
+  return operator_new_aligned_impl(size, alignment);
+#    else
   void* p = nullptr;
-#    ifndef _LIBCPP_HAS_NO_EXCEPTIONS
   try {
-#    endif // _LIBCPP_HAS_NO_EXCEPTIONS
     p = ::operator new(size, alignment);
-#    ifndef _LIBCPP_HAS_NO_EXCEPTIONS
   } catch (...) {
   }
-#    endif // _LIBCPP_HAS_NO_EXCEPTIONS
   return p;
+#    endif
 }
 
 _LIBCPP_WEAK void* operator new[](size_t size, std::align_val_t alignment) _THROW_BAD_ALLOC {
@@ -134,16 +160,25 @@ _LIBCPP_WEAK void* operator new[](size_t size, std::align_val_t alignment) _THRO
 }
 
 _LIBCPP_WEAK void* operator new[](size_t size, std::align_val_t alignment, const std::nothrow_t&) noexcept {
+#    ifdef _LIBCPP_HAS_NO_EXCEPTIONS
+  _LIBCPP_ASSERT_SHIM(
+      !std::__is_function_overridden(static_cast<void* (*)(std::size_t, std::align_val_t)>(&operator new[])),
+      "libc++ was configured with exceptions disabled and `operator new[](size_t, align_val_t)` has been overridden, "
+      "but `operator new[](size_t, align_val_t, nothrow_t)` has not been overridden. This is problematic because "
+      "`operator new[](size_t, align_val_t, nothrow_t)` must call `operator new[](size_t, align_val_t)`, which will "
+      "terminate in case it fails to allocate, making it impossible for `operator new[](size_t, align_val_t, "
+      "nothrow_t)` "
+      "to fulfill its contract (since it should return nullptr upon failure).");
+
+  return operator_new_aligned_impl(size, alignment);
+#    else
   void* p = nullptr;
-#    ifndef _LIBCPP_HAS_NO_EXCEPTIONS
   try {
-#    endif // _LIBCPP_HAS_NO_EXCEPTIONS
     p = ::operator new[](size, alignment);
-#    ifndef _LIBCPP_HAS_NO_EXCEPTIONS
   } catch (...) {
   }
-#    endif // _LIBCPP_HAS_NO_EXCEPTIONS
   return p;
+#    endif
 }
 
 _LIBCPP_WEAK void operator delete(void* ptr, std::align_val_t) noexcept { std::__libcpp_aligned_free(ptr); }
diff --git a/libcxx/test/libcxx/language.support/support.dynamic/assert.nothrow_new_not_overridden_fno_exceptions.pass.cpp b/libcxx/test/libcxx/language.support/support.dynamic/assert.nothrow_new_not_overridden_fno_exceptions.pass.cpp
new file mode 100644
index 000000000000000..82c8bb90b8d8c4c
--- /dev/null
+++ b/libcxx/test/libcxx/language.support/support.dynamic/assert.nothrow_new_not_overridden_fno_exceptions.pass.cpp
@@ -0,0 +1,56 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// void* operator new(std::size_t, const std::nothrow_t&);
+// void* operator new(std::size_t, std::align_val_t, const std::nothrow_t&);
+// void* operator new[](std::size_t, const std::nothrow_t&);
+// void* operator new[](std::size_t, std::align_val_t, const std::nothrow_t&);
+
+// This test ensures that we catch the case where `new` has been overridden but `new(nothrow)`
+// has not been overridden, and the library is compiled with -fno-exceptions.
+//
+// In that case, it is impossible for libc++ to provide a Standards conforming implementation
+// of `new(nothrow)`, so the only viable option is to terminate the program.
+
+// REQUIRES: has-unix-headers
+// UNSUPPORTED: c++03
+// XFAIL: availability-verbose_abort-missing
+
+// TODO: We currently don't have a way to express that the built library was
+//       compiled with -fno-exceptions, so if the library was built with support
+//       for exceptions but we run the test suite without exceptions, this will
+//       spuriously fail.
+// REQUIRES: no-exceptions
+
+#include <cstddef>
+#include <new>
+
+#include "check_assertion.h"
+
+// Override the throwing versions of operator new, but not the nothrow versions.
+alignas(32) char DummyData[32 * 3];
+void* operator new(std::size_t) { return DummyData; }
+void* operator new(std::size_t, std::align_val_t) { return DummyData; }
+void* operator new[](std::size_t) { return DummyData; }
+void* operator new[](std::size_t, std::align_val_t) { return DummyData; }
+
+void operator delete(void*) noexcept {}
+void operator delete(void*, std::align_val_t) noexcept {}
+void operator delete[](void*) noexcept {}
+void operator delete[](void*, std::align_val_t) noexcept {}
+
+int main(int, char**) {
+  std::size_t size       = 3;
+  std::align_val_t align = static_cast<std::align_val_t>(32);
+  EXPECT_DEATH((void)operator new(size, std::nothrow));
+  EXPECT_DEATH((void)operator new(size, align, std::nothrow));
+  EXPECT_DEATH((void)operator new[](size, std::nothrow));
+  EXPECT_DEATH((void)operator new[](size, align, std::nothrow));
+
+  return 0;
+}
diff --git a/libcxx/test/libcxx/language.support/support.dynamic/new_dont_return_nullptr.pass.cpp b/libcxx/test/libcxx/language.support/support.dynamic/new_dont_return_nullptr.pass.cpp
new file mode 100644
index 000000000000000..93a5a12349c97e6
--- /dev/null
+++ b/libcxx/test/libcxx/language.support/support.dynamic/new_dont_return_nullptr.pass.cpp
@@ -0,0 +1,37 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// void* operator new(std::size_t);
+// void* operator new(std::size_t, std::align_val_t);
+// void* operator new[](std::size_t);
+// void* operator new[](std::size_t, std::align_val_t);
+
+// This test ensures that we abort the program instead of returning nullptr
+// when we fail to satisfy the allocation request. The throwing versions of
+// `operator new` must never return nullptr on failure to allocate (per the
+// Standard) and the compiler actually relies on that for optimizations.
+// Returning nullptr from the throwing `operator new` can basically result
+// in miscompiles.
+
+// REQUIRES: has-unix-headers
+// REQUIRES: no-exceptions
+// UNSUPPORTED: c++03, c++11, c++14
+
+#include <cstddef>
+#include <limits>
+#include <new>
+
+#include "check_assertion.h"
+
+int main(int, char**) {
+    EXPECT_DEATH((void)operator new(std::numeric_limits<std::size_t>::max()));
+    EXPECT_DEATH((void)operator new(std::numeric_limits<std::size_t>::max(), static_cast<std::align_val_t>(32)));
+    EXPECT_DEATH((void)operator new[](std::numeric_limits<std::size_t>::max()));
+    EXPECT_DEATH((void)operator new[](std::numeric_limits<std::size_t>::max(), static_cast<std::align_val_t>(32)));
+    return 0;
+}
diff --git a/libcxx/test/support/check_assertion.h b/libcxx/test/support/check_assertion.h
index 98dd95b11556e6c..34e41e8f0d8eaf8 100644
--- a/libcxx/test/support/check_assertion.h
+++ b/libcxx/test/support/check_assertion.h
@@ -10,6 +10,7 @@
 #define TEST_SUPPORT_CHECK_ASSERTION_H
 
 #include <cassert>
+#include <csignal>
 #include <cstdarg>
 #include <cstddef>
 #include <cstdio>
@@ -257,9 +258,14 @@ void std::__libcpp_v...
[truncated]

[ldionne]

This change is extremely tricky but also pretty necessary -- pinging @llvm/libcxx-vendors .

If you are a vendor and you build the libc++ shared or static library with -fno-exceptions, you must take the time to read the commit message + release note and understand the implications of this change.

This change does not affect users merely building with -fno-exceptions if libc++.dylib/libc++.a has been built with exceptions enabled. But it affects anyone who's building libc++.dylib/libc++.a with exceptions disabled.

The alternative to this patch is to change the Standard to allow operator new to return nullptr when exceptions are disabled, which is not something that's acknowledged by the Standard. And then this patch would instead turn into a Clang patch that makes it stop assuming that operator new doesn't return nullptr.

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff b00aa1c77b0aade04adf89336c52d415a71c9477 609ba8db6ae80e93acdad7d85290a70c3990acf2 -- libcxx/src/include/overridable_function.h libcxx/test/libcxx/language.support/support.dynamic/assert.nothrow_new_not_overridden_fno_exceptions.pass.cpp libcxx/test/libcxx/language.support/support.dynamic/new_dont_return_nullptr.pass.cpp libcxx/src/new.cpp libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align_nothrow.replace.indirect.pass.cpp libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_nothrow.replace.indirect.pass.cpp libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_align_nothrow.replace.indirect.pass.cpp libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_nothrow.replace.indirect.pass.cpp libcxx/test/support/check_assertion.h libcxx/test/support/count_new.h libcxx/test/support/test.support/test_check_assertion.pass.cpp libcxxabi/src/stdlib_new_delete.cpp

View the diff from clang-format here.

diff --git a/libcxx/test/support/check_assertion.h b/libcxx/test/support/check_assertion.h
index c1d0bdcb33..cc841ceb23 100644
--- a/libcxx/test/support/check_assertion.h
+++ b/libcxx/test/support/check_assertion.h
@@ -135,7 +135,7 @@ std::string ToString(std::array<DeathCause, N> const& causes) {
   ss << "{";
   for (std::size_t i = 0; i != N; ++i) {
     ss << ToString(causes[i]);
-    if (i+1 != N)
+    if (i + 1 != N)
       ss << ", ";
   }
   ss << "}";
@@ -209,8 +209,8 @@ public:
 
     if (std::find(expected_causes.begin(), expected_causes.end(), cause) == expected_causes.end()) {
       std::stringstream failure_description;
-      failure_description                                             //
-          << "Child died, but with a different death cause\n"         //
+      failure_description                                               //
+          << "Child died, but with a different death cause\n"           //
           << "Expected cause(s): " << ToString(expected_causes) << "\n" //
           << "Actual cause:      " << ToString(cause) << "\n";
       return DeathTestResult(Outcome::UnexpectedCause, cause, failure_description.str());
@@ -349,7 +349,8 @@ void std::__libcpp_verbose_abort(char const* format, ...) {
 #endif // _LIBCPP_VERSION
 
 template <std::size_t N, class Func>
-bool ExpectDeath(const std::array<DeathCause, N>& expected_causes, const char* stmt, Func&& func, const Matcher& matcher) {
+bool ExpectDeath(
+    const std::array<DeathCause, N>& expected_causes, const char* stmt, Func&& func, const Matcher& matcher) {
   for (auto cause : expected_causes)
     assert(IsValidCause(cause));
 

[ldionne]

@var-const This is a rare example of a case where I think it makes sense to unconditionally have a _LIBCPP_ASSERT -- this should be enabled regardless of the hardening mode IMO. It is similar to when we call std::__throw_logic_error() and friends with -fno-exceptions -- that aborts the program regardless of the hardening mode.

[EricWF]

I think you're forgetting how hot of a path this is. I don't think we want to be doing anything more than we need in the majority of cases.

[ldionne]

We could make this part of the DEBUG_LITE mode then, I guess. Would that address your concerns?

[rnk]

+@jyknight for new + EH thoughts

[github-actions]

✅ With the latest revision this PR passed the Python code formatter.

[rnk]

These are just thoughts and suggestions to be taken or left. Other reviewers should please feel free to approve.

[EricWF]

Are there tests that we actually correctly call the overridden throwing version from the overridden non-throwing version?

Also, I wonder how much we really care about the library being built without exceptions vs allowing the library to be used without exceptions.

[EricWF]

I think you're forgetting how hot of a path this is. I don't think we want to be doing anything more than we need in the majority of cases.

[ldionne]

Are there tests that we actually correctly call the overridden throwing version from the overridden non-throwing version?

Yes, see for example libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_nothrow.replace.indirect.pass.cpp.

Also, I wonder how much we really care about the library being built without exceptions vs allowing the library to be used without exceptions.

We currently support both ways of building the library (-fexceptions and -fno-exceptions). I don't know how widely used the -fno-exceptions variant is used and we could discuss dropping it (and forcing people to always build the library itself with -fexceptions), but IMO this should be a separate change. In practice, I suspect that a lot of folks in the embedded world might be building the library with -fno-exceptions.

[EricWF]

I'm tempted to just do the non-conforming thing here (because -fno-exceptions is non-conforming), and just assume that when you're building with -fno-exceptions, you're not trying to replace the throwing operator new, and you'll be fine if we have the throwing kind call the non-throwing kind.

Is that totally insane?

[EricWF]

We currently support both ways of building the library (-fexceptions and -fno-exceptions). I don't know how widely used the -fno-exceptions variant is used and we could discuss dropping it (and forcing people to always build the library itself with -fexceptions), but IMO this should be a separate change. In practice, I suspect that a lot of folks in the embedded world might be building the library with -fno-exceptions.

My only concern is that this change introduces a bunch of black-magic, which I suspect is pretty fragile (but I don't know), and I would like to avoid that if possible.

[ldionne]

I'm tempted to just do the non-conforming thing here (because -fno-exceptions is non-conforming), and just assume that when you're building with -fno-exceptions, you're not trying to replace the throwing operator new, and you'll be fine if we have the throwing kind call the non-throwing kind.

So IIUC you are suggesting the following behavior under -fno-exceptions?

operator new(size_t) // We assume it is never overridden, we call new(size_t, nothrow_t) and we abort if that fails
operator new(size_t, nothrow_t) // Users may override it. Default implementation tries to allocate memory using malloc

Under -fexceptions, the behavior would stay as it is right now, i.e.

operator new(size_t) // Can be overridden, by default tries to allocate with malloc and throws if that fails
operator new(size_t, nothrow_t) // Users may override, by default it uses new(size_t) and uses try-catch to return nullptr on failure

Is that your suggestion? The problem I see with this is that we end up with inverted dependencies between new(size_t) and new(size_t, nothrow_t) depending on whether exceptions are enabled. With -fexceptions, we get the usual dependency new(size_t, nothrow_t) calls new(size_t), but with -fno-exceptions we have new(size_t) calls new(size_t, nothrow_t). IMO this change in semantics is an issue, especially considering that users can use a different exception mode than what the dylib is built with. That seems like a recipe for confusion.

Another issue is that concretely, 99% of people replace new(size_t) and probably never bother to replace (or even use) new(size_t, nothrow_t). So I feel like expecting users to override new(size_t, nothrow_t) instead of new(size_t) under -fno-exceptions is probably unrealistic.

The currently-proposed approach kinda sucks too (because we don't know how to implement new(size_t, nothrow_t) if you replace new(size_t)), but at least it's consistent in every other regard (AFAICT) and it is as Standards conforming as possible.

If we could design this from scratch, what would we do? I think we would likely specify that new(size_t) is implemented as calling new(size_t, nothrow_t) and doing if (result == nullptr) throw std::bad_alloc();. Then the common way to override new amongst users would be to override new(size_t, nothrow_t) instead of new(size_t) and they would immediately get the right behavior. Under -fno-exceptions, we would simply abort() inside new(size_t) in case the nothrow_t version returned nullptr. Do we think there is even a slim chance of ever changing that in the Standard?

[jyknight]

I think we can check whether it's been overridden more easily using an asm alias like this (when libc++ is built under -fno-exceptions):

#include <cstdlib>
#include <new>

__attribute__((weak)) void* operator new(std::size_t size) {
  void* mem = malloc(size);
  if (!mem) abort();
  return mem;
}

// TODO: ifdef for MSVC mangling.
static void* original_operator_new(std::size_t size) __attribute__((alias(__USER_LABEL_PREFIX__ "_Znwm")));

__attribute__((weak)) void* operator new(std::size_t size, std::nothrow_t) noexcept {
  if (static_cast<void*(*)(std::size_t)>(&::operator new) != original_operator_new)
    abort();
  return malloc(size);
}

[EricWF]

@ldionne You've fully convinced me on this approach. Thank you for the explanation.

[ldionne]

I think we can check whether it's been overridden more easily using an asm alias like this (when libc++ is built under -fno-exceptions):

This doesn't work on Darwin (apparently we don't support the alias attribute on that platform). I could investigate doing this all non-Darwin platforms, however that would make the utility a lot less generic than in my latest patch, since I'd have to handle each function I want to check for overrides individually.

[philnik777]

LGTM % nit.

[DianQK]

Thanks for the fix! I'll merge the blocked patches in a later two days.