Repository navigation
ARM/Linux Regression Fix of Exception Handling - #5596
Conversation
|
CC: @parjong @jyoungyun |
This fixes the regression caused by 594b424, which intended to fix #5358 while breaking many of exception handling unit test cases. The commit fixing #5358 was an incorrect translation of x86 assembly code. Fix #5595, #5358. Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
|
In the "original" x86 version of the code being amended here, x86 was saving PC to stack only because it's their calling convention and x86 was not loading LR because it does not have LR. So, following the calling convention of ARM, we do not touch the stack (remove push) and we load LR, which acts as the RSP+4 in x86. Note that the stack-pushed saved-PC value is NEVER used by the CoreCLR source code itself. It's just following x86 calling convention; thus, we should never follow it in ARM. |
| // Added r7 as a dummy to keep the stack aligned by 8 bytes. | ||
| push {r2, r7} | ||
| ldr lr, [r0, #(CONTEXT_Pc)] | ||
|
|
There was a problem hiding this comment.
Is it required to use CONTEXT_Pc instead of CONTEXT_Lr to fix PAL test failure?
There was a problem hiding this comment.
Yes. We are returning to this context (r0, the first argument).
Note that the twin code of x86 has
mov rax, [rdi + CONTEXT_Rip]
push_register rax
, which is translated as
"Save first_argument->PC(IP) to the return address location in the stack",
which is translated as
ldr lr, [r0, #(CONTEXT_Pc)]
for ARM.
There was a problem hiding this comment.
Oh, thanks. I found the following comment from pal/src/exception/seh.cpp:
This function creates a stack frame right below the target frame, restores all callee
saved registers from the passed in context, sets the RSP to that frame and sets the
return address to the target frame's RIP.
Then it uses the ThrowExceptionHelper to throw the passed in exception from that context.
It looks like that the semantics of this function requires one more stack frame to be established as you expalined.
|
LGTM, thank you |
This fixes the regression caused by dotnet/coreclr@594b424, which intended to fix dotnet/coreclr#5358 while breaking many of exception handling unit test cases. The commit fixing dotnet/coreclr#5358 was an incorrect translation of x86 assembly code. Fix dotnet/coreclr#5595, dotnet/coreclr#5358. Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com> Commit migrated from dotnet/coreclr@6287c78
This fixes the regression caused by
594b424, which
intended to fix #5358 while breaking many of
exception handling unit test cases.
The commit fixing #5358 was an incorrect translation
of x86 assembly code.
Fix #5595, #5358.
Signed-off-by: MyungJoo Ham myungjoo.ham@samsung.com