Skip to content
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

Operator new and -fno-exceptions #60129

Closed
asl opened this issue Jan 18, 2023 · 1 comment · Fixed by #69498
Closed

Operator new and -fno-exceptions #60129

asl opened this issue Jan 18, 2023 · 1 comment · Fixed by #69498
Assignees
Labels
libc++abi libc++abi C++ Runtime Library. Not libc++. libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. quality-of-implementation

Comments

@asl
Copy link
Collaborator

asl commented Jan 18, 2023

Current implementation of operator new in libcxx / libcxxabi effectively makes plain new and nothrow new to coincide when exceptions are disabled: both will return nullptr. However, there is one important difference: for nothrow new the constructor is not called when the return value is null. This does not happen for ordinary new in -fno-exceptions case making impossible to check the return value as constructor might just segfault. From the user perspective it might be better to stick to one of the following:

  • Just call abort(). This seems what is currently done for things like std::__throw_out_of_range in -fno-except case
  • Make operator new and nothrow operator new to behave identically and do not call constructor

Any thoughts on what might be the better solution?

@asl asl added question A question, not bug report. Check out https://llvm.org/docs/GettingInvolved.html instead! libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. libc++abi libc++abi C++ Runtime Library. Not libc++. quality-of-implementation labels Jan 18, 2023
@BetzalelG
Copy link

Calling abort() is the right call in my opinion.

  • This is how libstdc++ and VS implementations handle it.
  • This is how it's done for every other exception in libcxx, including other places where __throw_bad_alloc() is called.
  • This will retain the C++ standard that says operator new shouldn't return a nullptr.

ldionne added a commit that referenced this issue Jan 23, 2024
…tions (#69498)

In D144319, Clang tried to land a change that would cause some functions
that are not supposed to return nullptr to optimize better. As reported
in https://reviews.llvm.org/D144319#4203982, libc++ started seeing
failures in its CI shortly after this change was landed.

As explained in D146379, the reason for these failures is that libc++'s
throwing `operator new` can in fact return nullptr when compiled with
exceptions disabled. However, this contradicts the Standard, which
clearly says that the throwing version of `operator new(size_t)` should
never return nullptr. This is actually a long standing issue. I've
previously seen a case where LTO would optimize incorrectly based on the
assumption that `operator new` doesn't return nullptr, an assumption
that was violated in that case because libc++.dylib was compiled with
-fno-exceptions.

Unfortunately, fixing this is kind of tricky. The Standard has a few
requirements for the allocation functions, some of which are impossible
to satisfy under -fno-exceptions:
1. `operator new(size_t)` must never return nullptr
2. `operator new(size_t, nothrow_t)` must call the throwing version and
return nullptr on failure to allocate
3. We can't throw exceptions when compiled with -fno-exceptions

In the case where exceptions are enabled, things work nicely.
`new(size_t)` throws and `new(size_t, nothrow_t)` uses a try-catch to
return nullptr. However, when compiling the library with
-fno-exceptions, we can't throw an exception from `new(size_t)`, and we
can't catch anything from `new(size_t, nothrow_t)`. The only thing we
can do from `new(size_t)` is actually abort the program, which does not
make it possible for `new(size_t, nothrow_t)` to catch something and
return nullptr.

This patch makes the following changes:
1. When compiled with -fno-exceptions, the throwing version of `operator
new` will now abort on failure instead of returning nullptr on failure.
This resolves the issue that the compiler could mis-compile based on the
assumption that nullptr is never returned. This constitutes an API and
ABI breaking change for folks compiling the library with -fno-exceptions
(which is not the general public, who merely uses libc++ headers but use
a shared library that has already been compiled). This should mostly
impact vendors and other folks who compile libc++.dylib themselves.

2. When the library is compiled with -fexceptions, the nothrow version
of `operator new` has no change. When the library is compiled with
-fno-exceptions, the nothrow version of `operator new` will now check
whether the throwing version of `operator new` has been overridden. If
it has not been overridden, then it will use an implementation
equivalent to that of the throwing `operator new`, except it will return
nullptr on failure to allocate (instead of terminating). However, if the
throwing `operator new` has been overridden, it is now an error NOT to
also override the nothrow `operator new`. Indeed, there is no way for us
to implement a valid nothrow `operator new` without knowing the exact
implementation of the throwing version.

In summary, this change will impact people who fall into the following
intersection of conditions:
- They use the libc++ shared/static library built with `-fno-exceptions`
- They do not override `operator new(..., std::nothrow_t)`
- They override `operator new(...)` (the throwing version)
- They use `operator new(..., std::nothrow_t)`

We believe this represents a small number of people.

Fixes #60129
rdar://103958777

Differential Revision: https://reviews.llvm.org/D150610
@ldionne ldionne self-assigned this Jan 23, 2024
@EugeneZelenko EugeneZelenko removed the question A question, not bug report. Check out https://llvm.org/docs/GettingInvolved.html instead! label Jan 23, 2024
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++. libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. quality-of-implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants