[libc++][NFC] Refactor the core logic of operator new into helper functions

[ldionne]

This will make it easier to implement new(nothrow) without calling the throwing version of new when exceptions are disabled. See https://llvm.org/D150610 for the full discussion.

[llvmbot]

@llvm/pr-subscribers-libcxx

@llvm/pr-subscribers-libcxxabi

Author: Louis Dionne (ldionne)

Changes

This will make it easier to implement new(nothrow) without calling the throwing version of new when exceptions are disabled. See https://llvm.org/D150610 for the full discussion.

---

Full diff: https://github.com/llvm/llvm-project/pull/69407.diff

2 Files Affected:

• (modified) libcxx/src/new.cpp (+29-25)
• (modified) libcxxabi/src/stdlib_new_delete.cpp (+29-25)

diff --git a/libcxx/src/new.cpp b/libcxx/src/new.cpp
index c435c5ffc809c61..d045b0d4570d096 100644
--- a/libcxx/src/new.cpp
+++ b/libcxx/src/new.cpp
@@ -20,30 +20,34 @@
 // in this shared library, so that they can be overridden by programs
 // that define non-weak copies of the functions.
 
-_LIBCPP_WEAK
-void *
-operator new(std::size_t size) _THROW_BAD_ALLOC
-{
+static void* operator_new_impl(std::size_t size) noexcept {
     if (size == 0)
         size = 1;
     void* p;
-    while ((p = std::malloc(size)) == nullptr)
-    {
+    while ((p = std::malloc(size)) == nullptr) {
         // If malloc fails and there is a new_handler,
         // call it to try free up memory.
         std::new_handler nh = std::get_new_handler();
         if (nh)
             nh();
         else
-#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
-            throw std::bad_alloc();
-#else
             break;
-#endif
     }
     return p;
 }
 
+_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
+    return p;
+}
+
 _LIBCPP_WEAK
 void*
 operator new(size_t size, const std::nothrow_t&) noexcept
@@ -133,10 +137,7 @@ operator delete[] (void* ptr, size_t) noexcept
 
 #if !defined(_LIBCPP_HAS_NO_LIBRARY_ALIGNED_ALLOCATION)
 
-_LIBCPP_WEAK
-void *
-operator new(std::size_t size, std::align_val_t alignment) _THROW_BAD_ALLOC
-{
+static void* operator_new_aligned_impl(std::size_t size, std::align_val_t alignment) noexcept {
     if (size == 0)
         size = 1;
     if (static_cast<size_t>(alignment) < sizeof(void*))
@@ -145,26 +146,29 @@ operator new(std::size_t size, std::align_val_t alignment) _THROW_BAD_ALLOC
     // Try allocating memory. If allocation fails and there is a new_handler,
     // call it to try free up memory, and try again until it succeeds, or until
     // the new_handler decides to terminate.
-    //
-    // If allocation fails and there is no new_handler, we throw bad_alloc
-    // (or return nullptr if exceptions are disabled).
     void* p;
-    while ((p = std::__libcpp_aligned_alloc(static_cast<std::size_t>(alignment), size)) == nullptr)
-    {
+    while ((p = std::__libcpp_aligned_alloc(static_cast<std::size_t>(alignment), size)) == nullptr) {
         std::new_handler nh = std::get_new_handler();
         if (nh)
             nh();
-        else {
-#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
-            throw std::bad_alloc();
-#else
+        else
             break;
-#endif
-        }
     }
     return p;
 }
 
+_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
+    return p;
+}
+
 _LIBCPP_WEAK
 void*
 operator new(size_t size, std::align_val_t alignment, const std::nothrow_t&) noexcept
diff --git a/libcxxabi/src/stdlib_new_delete.cpp b/libcxxabi/src/stdlib_new_delete.cpp
index 080f932ccc60ecf..7946d07a59b72a7 100644
--- a/libcxxabi/src/stdlib_new_delete.cpp
+++ b/libcxxabi/src/stdlib_new_delete.cpp
@@ -30,30 +30,34 @@
 // in this shared library, so that they can be overridden by programs
 // that define non-weak copies of the functions.
 
-_LIBCPP_WEAK
-void *
-operator new(std::size_t size) _THROW_BAD_ALLOC
-{
+static void* operator_new_impl(std::size_t size) noexcept {
     if (size == 0)
         size = 1;
     void* p;
-    while ((p = std::malloc(size)) == nullptr)
-    {
+    while ((p = std::malloc(size)) == nullptr) {
         // If malloc fails and there is a new_handler,
         // call it to try free up memory.
         std::new_handler nh = std::get_new_handler();
         if (nh)
             nh();
         else
-#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
-            throw std::bad_alloc();
-#else
             break;
-#endif
     }
     return p;
 }
 
+_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
+    return p;
+}
+
 _LIBCPP_WEAK
 void*
 operator new(size_t size, const std::nothrow_t&) noexcept
@@ -143,10 +147,7 @@ operator delete[] (void* ptr, size_t) noexcept
 
 #if !defined(_LIBCPP_HAS_NO_LIBRARY_ALIGNED_ALLOCATION)
 
-_LIBCPP_WEAK
-void *
-operator new(std::size_t size, std::align_val_t alignment) _THROW_BAD_ALLOC
-{
+static void* operator_new_aligned_impl(std::size_t size, std::align_val_t alignment) noexcept {
     if (size == 0)
         size = 1;
     if (static_cast<size_t>(alignment) < sizeof(void*))
@@ -155,26 +156,29 @@ operator new(std::size_t size, std::align_val_t alignment) _THROW_BAD_ALLOC
     // Try allocating memory. If allocation fails and there is a new_handler,
     // call it to try free up memory, and try again until it succeeds, or until
     // the new_handler decides to terminate.
-    //
-    // If allocation fails and there is no new_handler, we throw bad_alloc
-    // (or return nullptr if exceptions are disabled).
     void* p;
-    while ((p = std::__libcpp_aligned_alloc(static_cast<std::size_t>(alignment), size)) == nullptr)
-    {
+    while ((p = std::__libcpp_aligned_alloc(static_cast<std::size_t>(alignment), size)) == nullptr) {
         std::new_handler nh = std::get_new_handler();
         if (nh)
             nh();
-        else {
-#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
-            throw std::bad_alloc();
-#else
+        else
             break;
-#endif
-        }
     }
     return p;
 }
 
+_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
+    return p;
+}
+
 _LIBCPP_WEAK
 void*
 operator new(size_t size, std::align_val_t alignment, const std::nothrow_t&) noexcept

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[philnik777]

LGTM

[azhan92]

Since the standard allows the following behaviour for the new_handler function:

throw an exception of type bad_alloc or a class derived from bad_alloc;

nh() may throw an exception and cause some issues with the noexcept specification on operator_new_impl. Would you consider a change here to prevent problem?

[ldionne]

Yeah, I think this looks like an issue I introduced with this patch. It should be simple to fix, basically make the _impl function non-noexcept and add a test that catches that? A patch would definitely be appreciated.

[azhan92]

Sure, I'll create a patch to do that then. Thanks for the suggestions