Skip to content

Conversation

azhan92
Copy link
Contributor

@azhan92 azhan92 commented Dec 4, 2023

This PR removes the noexcept specification introduced in 69407 since the standard allows

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

as a behaviour of a new_handler function. The PR also adds a test to catch this.

@azhan92 azhan92 requested a review from a team as a code owner December 4, 2023 15:28
@llvmbot llvmbot added the libc++abi libc++abi C++ Runtime Library. Not libc++. label Dec 4, 2023
@llvmbot
Copy link
Member

llvmbot commented Dec 4, 2023

@llvm/pr-subscribers-libcxxabi

Author: None (azhan92)

Changes

This PR removes the noexcept specification introduced in 69407 since the standard allows

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

as a behaviour of a new_handler function. The PR also adds a test to catch this.


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

2 Files Affected:

  • (modified) libcxxabi/src/stdlib_new_delete.cpp (+17-23)
  • (added) libcxxabi/test/test_memory_alloc.pass.cpp (+33)
diff --git a/libcxxabi/src/stdlib_new_delete.cpp b/libcxxabi/src/stdlib_new_delete.cpp
index 6c9990f063dde..71c98793cae40 100644
--- a/libcxxabi/src/stdlib_new_delete.cpp
+++ b/libcxxabi/src/stdlib_new_delete.cpp
@@ -30,7 +30,8 @@
 // in this shared library, so that they can be overridden by programs
 // that define non-weak copies of the functions.
 
-static void* operator_new_impl(std::size_t size) noexcept {
+_LIBCPP_WEAK
+void* operator new(std::size_t size) _THROW_BAD_ALLOC {
   if (size == 0)
     size = 1;
   void* p;
@@ -41,18 +42,12 @@ static void* operator_new_impl(std::size_t size) noexcept {
     if (nh)
       nh();
     else
-      break;
-  }
-  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();
+      throw std::bad_alloc();
+#else
+      break;
 #endif
+  }
   return p;
 }
 
@@ -107,7 +102,8 @@ void operator delete[](void* ptr, size_t) noexcept { ::operator delete[](ptr); }
 
 #if !defined(_LIBCPP_HAS_NO_LIBRARY_ALIGNED_ALLOCATION)
 
-static void* operator_new_aligned_impl(std::size_t size, std::align_val_t alignment) noexcept {
+_LIBCPP_WEAK
+void* operator new(std::size_t size, std::align_val_t alignment) _THROW_BAD_ALLOC {
   if (size == 0)
     size = 1;
   if (static_cast<size_t>(alignment) < sizeof(void*))
@@ -116,24 +112,22 @@ static void* operator_new_aligned_impl(std::size_t size, std::align_val_t alignm
   // 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) {
     std::new_handler nh = std::get_new_handler();
     if (nh)
       nh();
-    else
-      break;
-  }
-  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);
+    else {
 #  ifndef _LIBCPP_HAS_NO_EXCEPTIONS
-  if (p == nullptr)
-    throw std::bad_alloc();
+      throw std::bad_alloc();
+#  else
+      break;
 #  endif
+    }
+  }
   return p;
 }
 
diff --git a/libcxxabi/test/test_memory_alloc.pass.cpp b/libcxxabi/test/test_memory_alloc.pass.cpp
new file mode 100644
index 0000000000000..3eb618e54f1a8
--- /dev/null
+++ b/libcxxabi/test/test_memory_alloc.pass.cpp
@@ -0,0 +1,33 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===// 
+
+#include <new>
+#include <cassert>
+#include <limits>
+
+int new_handler_called = 0;
+
+void my_new_handler() {
+  ++new_handler_called;
+  throw std::bad_alloc();
+}
+
+int main(int, char**) {
+  std::set_new_handler(my_new_handler);
+  try {
+    void* x = operator new[] (std::numeric_limits<std::size_t>::max());
+    (void)x;
+    assert(false);
+  }
+  catch (std::bad_alloc const&) {
+    assert(new_handler_called == 1);
+  } catch (...) {
+    assert(false);
+  }
+  return 0;
+}

Copy link

github-actions bot commented Dec 4, 2023

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

You can test this locally with the following command:
git-clang-format --diff 374e8288e047da640090629879072e4fa3af31fe f5d466383694fe717e26a930c39798a9710280fb -- libcxxabi/test/test_memory_alloc.pass.cpp libcxxabi/src/stdlib_new_delete.cpp
View the diff from clang-format here.
diff --git a/libcxxabi/test/test_memory_alloc.pass.cpp b/libcxxabi/test/test_memory_alloc.pass.cpp
index 3eb618e54f..a718cc642c 100644
--- a/libcxxabi/test/test_memory_alloc.pass.cpp
+++ b/libcxxabi/test/test_memory_alloc.pass.cpp
@@ -4,7 +4,7 @@
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
-//===----------------------------------------------------------------------===// 
+//===----------------------------------------------------------------------===//
 
 #include <new>
 #include <cassert>
@@ -20,11 +20,10 @@ void my_new_handler() {
 int main(int, char**) {
   std::set_new_handler(my_new_handler);
   try {
-    void* x = operator new[] (std::numeric_limits<std::size_t>::max());
+    void* x = operator new[](std::numeric_limits<std::size_t>::max());
     (void)x;
     assert(false);
-  }
-  catch (std::bad_alloc const&) {
+  } catch (std::bad_alloc const&) {
     assert(new_handler_called == 1);
   } catch (...) {
     assert(false);

@azhan92 azhan92 changed the title [libc++] Remove noexcept from _impl functions [libc++] Fix noexcept behaviour in _impl functions Dec 4, 2023
@azhan92 azhan92 marked this pull request as draft December 4, 2023 16:18
@azhan92 azhan92 closed this Dec 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++abi libc++abi C++ Runtime Library. Not libc++.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants