Skip to content

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

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Oct 18, 2023
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 21 additions & 15 deletions libcxx/src/new.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
// 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 {
Copy link
Contributor

@azhan92 azhan92 Nov 29, 2023 •
edited by hubert-reinterpretcast
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member Author

@ldionne ldionne Nov 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

if (size == 0)
size = 1;
void* p;
Expand All @@ -31,15 +31,20 @@ _LIBCPP_WEAK void* operator new(std::size_t size) _THROW_BAD_ALLOC {
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 {
void* p = nullptr;
# ifndef _LIBCPP_HAS_NO_EXCEPTIONS
Expand Down Expand Up @@ -82,7 +87,7 @@ _LIBCPP_WEAK void operator delete[](void* ptr, size_t) noexcept { ::operator del

# 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*))
Expand All @@ -91,25 +96,26 @@ _LIBCPP_WEAK void* operator new(std::size_t size, std::align_val_t alignment) _T
// 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 {
# 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 {
void* p = nullptr;
# ifndef _LIBCPP_HAS_NO_EXCEPTIONS
Expand Down
40 changes: 23 additions & 17 deletions libcxxabi/src/stdlib_new_delete.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@
// 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;
Expand All @@ -42,15 +41,21 @@ void* operator new(std::size_t size) _THROW_BAD_ALLOC {
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 {
void* p = nullptr;
Expand Down Expand Up @@ -102,8 +107,7 @@ void operator delete[](void* ptr, size_t) noexcept { ::operator delete[](ptr); }

#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*))
Expand All @@ -112,25 +116,27 @@ void* operator new(std::size_t size, std::align_val_t alignment) _THROW_BAD_ALLO
// 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 {
# 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 {
void* p = nullptr;
Expand Down