[libc++] These throwing allocation functions shouldn't return null.

[DianQK]

Summary

According to https://isocpp.org/wiki/faq/freestore-mgmt#new-never-returns-null and https://en.cppreference.com/w/cpp/memory/new/operator_new, a new operator that throws an exception should not return null.

This D20677 breaks that convention with -fno-exceptions.

I noticed the C++ standard doesn't mention such a convention under using -fno-exceptions. I'm new to the C++ standard library, so please let me know if I'm missing something.

But this patch breaks another convention.

5. Called by the non-throwing non-array new-expressions. The standard library implementation calls the version (1) and returns a null pointer on failure instead of propagating the exception.

I am submitting another candidate. (Mark the void* operator new ( std::size_t count ) as noexcept?)

If there are no relevant standards, are these the possible ways?

1. The current patch (Don't call the throwing function). This broken the standard.
2. Mark the void* operator new ( std::size_t count ) as noexcept.
3. Return the -1 in the throwing function? (Too tricky).
4. Remove the throwing function declaration (Breaking many api).
5. Revert D20677. Because the standard requires that exceptions be thrown.

[DianQK]

This comment was removed by DianQK.

[Mordante]

Thanks for the quick fix. I just saw on Discord @ldionne wants to look into this further.

libcxxabi/src/stdlib_new_delete.cpp

43

I really dislike std::__libcpp_unreachable since it just results in undefined behaviour. Either we should do nothing or assert(false);.

[BetzalelG]

In my opinion, it would be best to define a function here that will throw bad_alloc() under NO_EXCEPTIONS and otherwise abort. This is the same as throw_bad_array_new_length from cxa_vector.cpp.

namespace {
_LIBCXXABI_NORETURN
void throw_bad_alloc() {
#ifndef _LIBCXXABI_NO_EXCEPTIONS
    throw std::bad_alloc();
#else
    abort_message("operator new failed to allocate memory");
#endif
}

And later use:

if (nh)
    nh();
else
  throw_bad_alloc();

nothrow function doesn't have to be changed in any way (except possibly to remove unnecessary #ifndef _LIBCXXABI_NO_EXCEPTIONS and #endif lines). Just call new operator, and we will either not propagate the exception, or abort.

New with nothrow is allowed to abort! This is similar to the behavior in libstdc++ and in Microsoft's Visual Studio implementation. I think this is the best behavior because it retains the convention you quoted of calling version (1).

Either way, libcxx new.cpp should similarly be fixed to have this behavior. (In that case std::__throw_bad_alloc() can just be called, and there's no need to even add a new function)

[DianQK]

@BetzalelG This patch was just a fix I was trying at the time. For me, your opinion seems to be OK. But I'm not familiar with the libc++ standard library implementation.
@ldionne will complete the follow-up patch. How is it going?

[BetzalelG]

@DianQK I just looked at the implementation in the github mirror of gcc (libstdc++ v3):
https://github.com/gcc-mirror/gcc/tree/master/libstdc%2B%2B-v3
Particularly this file:
https://github.com/gcc-mirror/gcc/blob/master/libstdc%2B%2B-v3/libsupc%2B%2B/new_op.cc
Other operator new functions are in the other files in the folder, such as:
https://github.com/gcc-mirror/gcc/blob/master/libstdc%2B%2B-v3/libsupc%2B%2B/new_opnt.cc

The relevant line used in operator new is _GLIBCXX_THROW_OR_ABORT(bad_alloc());
My understanding of this implementation is:

• _GLIBCXX_THROW_OR_ABORT is defined to throw the given exception when __cpp_exceptions is defined, and abort otherwise
• __try and __catch are used in the nothrow version. (__try and __catch are defined so that if __cpp_exceptions isn't defined, they will be defined as if(true) and if(false) respectively)

So in summary:
When exceptions are defined - regular new will throw, nothrow will return null
When exceptions are undefined - regular new will abort, nothrow will abort as well

[ldionne]

Heads up: I have a series of patches to address this that I’ll upload soon. As discussed above, this is a bit messy, and actually it’s even worse than that. I’ll explain on the patch series.

[ldionne]

First patch in the series: D150408

[ldionne]

The change itself: https://reviews.llvm.org/D150610

[DianQK]

The -fno-exceptions seems to be something outside the C++ standard. Maybe define some additional rules? It may require some downstream adaptations.
Use D150610 instead of this patch.

libcxxabi/src/stdlib_new_delete.cpp

 //===----------------------------------------------------------------------===//
 //
 // 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
 //
 //
 // This file implements the new and delete operators.
 //===----------------------------------------------------------------------===//
 
 #include "__cxxabi_config.h"
 #include <__memory/aligned_alloc.h>
 #include <cstdlib>
 #include <new>
+#include <__utility/unreachable.h>
 
 #if !defined(_THROW_BAD_ALLOC) || !defined(_LIBCXXABI_WEAK)
 #error The _THROW_BAD_ALLOC and _LIBCXXABI_WEAK libc++ macros must \
        already be defined by libc++.
 #endif
 // Implement all new and delete operators as weak definitions
 // in this shared library, so that they can be overridden by programs
 // that define non-weak copies of the functions.
while ((p = ::malloc(size)) == nullptr)
         // call it to try free up memory.
         std::new_handler nh = std::get_new_handler();
         if (nh)
             nh();
         else
 #ifndef _LIBCXXABI_NO_EXCEPTIONS
             throw std::bad_alloc();
 #else
-            break;
+            std::__libcpp_unreachable();
 #endif
     }
     return p;
 }
 
 _LIBCXXABI_WEAK
-void*
-operator new(size_t size, const std::nothrow_t&) noexcept
-{
-    void* p = nullptr;
-#ifndef _LIBCXXABI_NO_EXCEPTIONS
-    try
-    {
-#endif // _LIBCXXABI_NO_EXCEPTIONS
-        p = ::operator new(size);
-#ifndef _LIBCXXABI_NO_EXCEPTIONS
-    }
-    catch (...)
-    {
+void* operator new(size_t size, const std::nothrow_t&) noexcept {
+    if (size == 0)
+        size = 1;
+    void* p;
+    while ((p = ::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
+            break;
     }
-#endif // _LIBCXXABI_NO_EXCEPTIONS
     return p;
 }
 
 _LIBCXXABI_WEAK
 void*
 operator new[](size_t size) _THROW_BAD_ALLOC
 {
     return ::operator new(size);
 }
 
 _LIBCXXABI_WEAK
-void*
-operator new[](size_t size, const std::nothrow_t&) noexcept
-{
-    void* p = nullptr;
-#ifndef _LIBCXXABI_NO_EXCEPTIONS
-    try
-    {
-#endif // _LIBCXXABI_NO_EXCEPTIONS
-        p = ::operator new[](size);
-#ifndef _LIBCXXABI_NO_EXCEPTIONS
-    }
-    catch (...)
-    {
-    }
-#endif // _LIBCXXABI_NO_EXCEPTIONS
-    return p;
-}
+void* operator new[](size_t size, const std::nothrow_t& nothrow) noexcept { return ::operator new(size, nothrow); }
 
 _LIBCXXABI_WEAK
 void
 operator delete(void* ptr) noexcept
 {
     ::free(ptr);
 }
 
operator new(std::size_t size, std::align_val_t alignment) _THROW_BAD_ALLOC
     {
         std::new_handler nh = std::get_new_handler();
         if (nh)
             nh();
         else {
 #ifndef _LIBCXXABI_NO_EXCEPTIONS
             throw std::bad_alloc();
 #else
-            break;
+            std::__libcpp_unreachable();
 #endif
         }
     }
     return p;
 }
 
 _LIBCXXABI_WEAK
 void*
 operator new(size_t size, std::align_val_t alignment, const std::nothrow_t&) noexcept
 {
-    void* p = nullptr;
-#ifndef _LIBCXXABI_NO_EXCEPTIONS
-    try
-    {
-#endif // _LIBCXXABI_NO_EXCEPTIONS
-        p = ::operator new(size, alignment);
-#ifndef _LIBCXXABI_NO_EXCEPTIONS
+    if (size == 0)
+        size = 1;
+    if (static_cast<size_t>(alignment) < sizeof(void*))
+        alignment = std::align_val_t(sizeof(void*));
+
+    // 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<size_t>(alignment), size)) == nullptr) {
+        std::new_handler nh = std::get_new_handler();
+        if (nh)
+            nh();
+        else {
+            break;
         }
-    catch (...)
-    {
     }
-#endif // _LIBCXXABI_NO_EXCEPTIONS
     return p;
 }
 
 _LIBCXXABI_WEAK
 void*
 operator new[](size_t size, std::align_val_t alignment) _THROW_BAD_ALLOC
 {
     return ::operator new(size, alignment);
 }
 
 _LIBCXXABI_WEAK
-void*
-operator new[](size_t size, std::align_val_t alignment, const std::nothrow_t&) noexcept
-{
-    void* p = nullptr;
-#ifndef _LIBCXXABI_NO_EXCEPTIONS
-    try
-    {
-#endif // _LIBCXXABI_NO_EXCEPTIONS
-        p = ::operator new[](size, alignment);
-#ifndef _LIBCXXABI_NO_EXCEPTIONS
-    }
-    catch (...)
-    {
-    }
-#endif // _LIBCXXABI_NO_EXCEPTIONS
-    return p;
+void* operator new[](size_t size, std::align_val_t alignment, const std::nothrow_t& nothrow) noexcept {
+    return ::operator new(size, alignment, nothrow);
 }
 
 _LIBCXXABI_WEAK
 void
 operator delete(void* ptr, std::align_val_t) noexcept
 {
     std::__libcpp_aligned_free(ptr);
 }