-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Fix up hijacking on arm32 (preserve async continuation register) #123526
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
base: main
Are you sure you want to change the base?
Conversation
…net#123057) Flag r2 (see [async calling convention](https://github.com/dotnet/runtime/blob/main/docs/design/coreclr/botr/clr-abi.md#returning-continuation)) during GC as it might contain an async continuation. Contributes to dotnet#122492. --------- Co-authored-by: Eduardo Velarde <[email protected]> Co-authored-by: Jan Kotas <[email protected]>
|
Tagging subscribers to this area: @agocke, @dotnet/ilc-contrib |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes ARM32 hijacking during GC to properly preserve the async continuation register (r2). The original code only saved r0 and r1 during hijacking, but ARM32's async calling convention uses r2 to return continuations (as documented in the CLR ABI). Without this fix, async continuations could be lost during GC, leading to incorrect behavior.
Changes:
- Added flag definitions for PTFF_SAVE_R1 and PTFF_SAVE_R2 to enable saving these registers during hijacking
- Modified GC probe frame to save and restore r0-r2 (was r0-r1) to preserve the async continuation register
- Updated stack frame layout calculations and alignment to accommodate the additional saved register
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/coreclr/nativeaot/Runtime/unix/unixasmmacrosarm.inc | Added PTFF_SAVE_R1 and PTFF_SAVE_R2 flag definitions matching PInvokeTransitionFrameFlags enum |
| src/coreclr/nativeaot/Runtime/arm/GcProbe.S | Updated PUSH_PROBE_FRAME, POP_PROBE_FRAME, and FixupHijackedCallstack macros to save/restore r0-r2, adjusted stack frame size from 88 to 96 bytes with proper 8-byte alignment, and updated RhpGcProbeHijack to set the new flags |
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Outer loop has a lot of crashes on linux arm32 that are likely caused by the PR |
I think that's likely. Is there a better way to check the changes than running the outerloop pipeline by any chance? |
|
Looking at this code, is there a problem with where the alignment that gets inserted between SP and R0? I think the alignment needs to be inserted above r2, so that sp and r0 are next to each other to match this code. |
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
The original PR #123057 led to some crashes in the outerloop runs so was reverted in #123474.
Given the code in
runtime/src/coreclr/nativeaot/Runtime/StackFrameIterator.cpp
Lines 171 to 187 in 0b9b34d
I think the stack has r0, r1, r2 but without the PTFF_SAVE_R1 flag, r1 gets skipped and PTFF_SAVE_R2 is actually used to preserve r1 instead.