[libcxx-commits] [libcxxabi] [libc++] Fix the behavior of throwing `operator new` under -fno-exceptions (PR #69498)

Louis Dionne via libcxx-commits libcxx-commits at lists.llvm.org
Wed Oct 18 11:57:11 PDT 2023


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

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

>From 77c0256c3ae99808a8def68bfcf5eee2fad704ca Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Wed, 14 Jun 2023 17:49:22 -0700
Subject: [PATCH] [libc++] Fix the behavior of throwing `operator new` under
 -fno-exceptions

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
---
 libcxx/docs/ReleaseNotes/18.rst               | 23 +++++
 libcxx/include/CMakeLists.txt                 |  1 +
 libcxx/include/__overridable_function         | 38 ++++++++
 libcxx/include/new                            |  9 +-
 libcxx/src/new.cpp                            | 79 +++++++++++-----
 ...new_not_overridden_fno_exceptions.pass.cpp | 56 ++++++++++++
 .../new_dont_return_nullptr.pass.cpp          | 37 ++++++++
 libcxx/test/support/check_assertion.h         |  6 ++
 libcxxabi/src/stdlib_new_delete.cpp           | 90 ++++++++++++++-----
 9 files changed, 291 insertions(+), 48 deletions(-)
 create mode 100644 libcxx/include/__overridable_function
 create mode 100644 libcxx/test/libcxx/language.support/support.dynamic/assert.nothrow_new_not_overridden_fno_exceptions.pass.cpp
 create mode 100644 libcxx/test/libcxx/language.support/support.dynamic/new_dont_return_nullptr.pass.cpp

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_verbose_abort(char const* format, ...) {
   std::exit(DeathTest::RK_Terminate);
 }
 
+[[noreturn]] inline void abort_handler(int) {
+  std::exit(DeathTest::RK_Terminate);
+}
+
 template <class Func>
 inline bool ExpectDeath(const char* stmt, Func&& func, AssertionInfoMatcher Matcher) {
   std::set_terminate(terminate_handler);
+  std::signal(SIGABRT, abort_handler);
   DeathTest DT(Matcher);
   DeathTest::ResultKind RK = DT.Run(func);
   auto OnFailure = [&](const char* msg) {
diff --git a/libcxxabi/src/stdlib_new_delete.cpp b/libcxxabi/src/stdlib_new_delete.cpp
index 6c9990f063dde66..cfb8d78bda9d09b 100644
--- a/libcxxabi/src/stdlib_new_delete.cpp
+++ b/libcxxabi/src/stdlib_new_delete.cpp
@@ -7,7 +7,10 @@
 //===----------------------------------------------------------------------===//
 
 #include "__cxxabi_config.h"
+#include "abort_message.h"
 #include <__memory/aligned_alloc.h>
+#include <__overridable_function>
+#include <cstddef>
 #include <cstdlib>
 #include <new>
 
@@ -25,6 +28,20 @@
 #  error libc++ and libc++abi seem to disagree on whether exceptions are enabled
 #endif
 
+inline void __throw_bad_alloc_shim() {
+#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
+  throw std::bad_alloc();
+#else
+  abort_message("bad_alloc was thrown in -fno-exceptions mode");
+#endif
+}
+
+#define _LIBCPP_ASSERT_SHIM(expr, str)                                                                                 \
+  do {                                                                                                                 \
+    if (!expr)                                                                                                         \
+      abort_message(str);                                                                                              \
+  } while (false)
+
 // ------------------ BEGIN COPY ------------------
 // Implement all new and delete operators as weak definitions
 // in this shared library, so that they can be overridden by programs
@@ -49,25 +66,31 @@ 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
@@ -75,16 +98,24 @@ 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
@@ -130,25 +161,31 @@ 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
@@ -158,16 +195,25 @@ void* operator new[](size_t size, std::align_val_t alignment) _THROW_BAD_ALLOC {
 
 _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



More information about the libcxx-commits mailing list