Skip to content

[libc++] Fix noexcept behaviour in _impl functions #74330

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

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