Add nothrow operator new/delete to src/coreclr/jit/alloc.cpp#124715
Add nothrow operator new/delete to src/coreclr/jit/alloc.cpp#124715gwr wants to merge 1 commit intodotnet:mainfrom
Conversation
github-actions
bot
added
the
area-CodeGen-coreclr
dotnet-policy-service
bot
added
the
community-contribution
|
Here's the analysis from Copilot helping track down this problem. Call chain causing the crash:
|
There was a problem hiding this comment.
Pull request overview
This PR adds nothrow operator new/delete overrides to the JIT's standalone build to prevent libstdc++'s nothrow implementation from calling through to the JIT's assert-enabled throwing operator new. The issue arises because the JIT overrides the throwing operator new to add assertions, but libstdc++'s nothrow variants internally delegate to the throwing version, triggering unwanted assertions.
Changes:
- Added nothrow variants of operator new and operator new[] that directly call malloc without assertions
- Added temporary
#if 1preprocessor directive with XXX comment indicating the code needs proper conditional guards
| void* __cdecl operator new(std::size_t size, const std::nothrow_t&) noexcept | ||
| { | ||
| if (size == 0) | ||
| { | ||
| size++; | ||
| } | ||
|
|
||
| return malloc(size); | ||
| } | ||
|
|
||
| void* __cdecl operator new[](std::size_t size, const std::nothrow_t&) noexcept | ||
| { | ||
| if (size == 0) | ||
| { | ||
| size++; | ||
| } | ||
|
|
||
| return malloc(size); | ||
| } |
There was a problem hiding this comment.
The nothrow operator new implementations are missing corresponding nothrow operator delete overloads. According to C++ standard, when providing placement new operators (including nothrow variants), matching placement delete operators should be provided to handle cleanup if construction throws. Add void operator delete(void* ptr, const std::nothrow_t&) noexcept and void operator delete[](void* ptr, const std::nothrow_t&) noexcept that call free(ptr) to match the pattern of the regular delete operators.
The JIT's standalone build overrides the throwing operator new but not the nothrow variants. When libstdc++'s nothrow implementation calls the throwing version internally, it gets the JIT's assert-enabled override instead of the standard library's throwing version. Solution needed: Add nothrow operator new/delete overrides to src/coreclr/jit/alloc.cpp to prevent libstdc++ from calling through to the asserting throwing version.
How do we end up calling these from the JIT? We should not be allocating heap memory in the JIT, so it sounds like a bug and the same assert should be added in these new versions. |
It's a bit complicated. See the doc I attached in #124715 (comment) |
Makes sense, overwriting global operator new always comes with issues like that. The change needs a better comment than just "Also need to override the "nothrow" variants". Alternative fixes that I can think of:
|
Yes, maybe that is best. |
|
Would marking the operators with |
Blocking of This invariant does not hold for combination of Win32 PAL and SunOS C++ runtime library: Win32 PAL depends on non-throwing operator new, SunOS C++ runtime library implements the non-throwing operator new by forwarding to throwing operator new, and all this gets linked into the JIT .dll/.so. This invariant can be broken by changes in implementation details of any code linked into the JIT. For example, if utilcode (that the JIT depends on currently) was refactored in modern C++ using standard C++ library, we would likely end up with the same problem, even on Windows.
I doubt it would help. We mark all symbols as visibility hidden, except the explicitly exported ones. |
|
Ah, ok, I thought that some other part of the system outside the JIT was somehow getting the JIT's overridden operator new. |
Here are the gory details from gdb: |
|
I do wonder why this seems to be exposed only on SunOS. I wonder if it's simply because I built everything with configuration=Debug? In any case, it seems like the fix might be technically correct on all platforms, even though the bug does not appear to be exposed elsewhere. |
3 participants
The JIT's standalone build overrides the throwing operator new but not the nothrow variants. When libstdc++'s nothrow implementation calls the throwing version internally, it gets the JIT's assert-enabled override instead of the standard library's throwing version.
Solution needed: Add nothrow operator new/delete overrides to src/coreclr/jit/alloc.cpp to prevent libstdc++ from calling through to the asserting throwing version.
Without these additions, dotnet crashes with an assertion.