Fix interpreter ResumeAfterCatch on arm64 Unix#122504
Conversation
The DispatchManagedException and DispatchRethrownManagedException were marked as NORETURN. That caused a problem when the C++ compiler has optimized the invocation of such methods so that it has removed its epilog and the return address was in a completely different function. The Regressions/coreclr/GitHub_88128/test88128 was failing due to this om macOS arm64. I have removed all the NORETURN markings from these methods to fix it.
|
Tagging subscribers to this area: @BrzVlad, @janvorli, @kg |
There was a problem hiding this comment.
Pull request overview
This PR fixes an interpreter issue on arm64 Unix where ResumeAfterCatch was incorrectly restoring non-volatile registers. The problem occurred because DispatchManagedException and DispatchRethrownManagedException were marked as DECLSPEC_NORETURN, which allowed the C++ compiler to optimize away epilogs, causing the return address to point to a different function. This broke the native unwind loop.
Key changes:
- Removed
DECLSPEC_NORETURNmarking from all exception dispatch functions - Removed
UNREACHABLE()calls after invocations of these dispatch functions - Added
UNREACHABLE()to one location where it's still needed since the parent function remains NORETURN
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/vm/interpexec.cpp | Removed UNREACHABLE() after DispatchManagedException and DispatchRethrownManagedException calls, replaced one with break statement |
| src/coreclr/vm/exceptmacros.h | Removed DECLSPEC_NORETURN from DispatchManagedException declaration for TARGET_UNIX |
| src/coreclr/vm/exceptionhandling.h | Removed DECLSPEC_NORETURN from all dispatch exception function declarations |
| src/coreclr/vm/exceptionhandling.cpp | Removed DECLSPEC_NORETURN from function definitions and removed/commented out UNREACHABLE() calls |
| src/coreclr/vm/excep.cpp | Added UNREACHABLE() after DispatchManagedException call in UnwindAndContinueRethrowHelperAfterCatch (still NORETURN) |
|
@BrzVlad in the end, I've decided to solve the problem by adjusting the instruction pointer before the |
janvorli
force-pushed
the
fix-resume-after-catch-arm64
branch
from
c3a7c2c to
335d600
Compare
|
The JIT\Regression\JitBlue\GitHub_19444 test crashes in the libunwind, I assume it is due to this change. I will investigate it. |
The handling of the case when the frame is a hardware exception frame needs to stay the same for the LLVM libunwind too.
|
/ba-g deadletter iOS test |
The
DispatchManagedExceptionandDispatchRethrownManagedExceptionwere marked asNORETURN. That caused a problem when the C++ compiler has optimized the invocation of such methods so that it has removed its epilog and the return address was in a completely different function. That has broken the native unwind loop in theInterpreterCodeManager::ResumeAfterCatch, as it restored some non-volatile registers incorrectly.The Regressions/coreclr/GitHub_88128/test88128 was failing due to this om macOS arm64.
I have fixed it in the same way as C++ EH handles such case. The PAL_VirtualUnwind moves the PC back in case it is not a frame of a hardware exception so that when a function ends with a call followed by nothing else, it gets an instruction pointer in the function for unwinding purposes.