Uh oh!
There was an error while loading. Please reload this page.
Fix up hijacking on x64 platforms in presence of runtime async - #122631
Conversation
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Pull request overview
This PR adds support for preserving the RCX register during GC hijacking on x64 platforms to handle async continuation return values. This is part of fixing runtime async support as mentioned in issue #122492.
Key Changes
- Adds PTFF_SAVE_RCX flag constant to indicate RCX should be preserved during hijacking
- Updates hijacking frame macros to save and restore RCX alongside RAX (and RDX on Unix)
- Changes register usage from RCX to R8 for passing the register bitmask to avoid conflicts with preserved RCX
- Adjusts stack calculations to account for the additional saved register
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/coreclr/nativeaot/Runtime/unix/unixasmmacrosamd64.inc | Adds PTFF_SAVE_RCX constant definition for Unix x64 platforms |
| src/coreclr/nativeaot/Runtime/amd64/GcProbe.asm | Updates Windows x64 hijacking code to preserve RCX, changes bitmask register from RCX to R8, adjusts stack offsets |
| src/coreclr/nativeaot/Runtime/amd64/GcProbe.S | Updates Unix x64 hijacking code to preserve RCX, changes bitmask register from RCX to R8, adjusts stack offsets and alignment |
| src/coreclr/nativeaot/Runtime/amd64/AsmMacros.inc | Adds PTFF_SAVE_RCX constant definition for Windows x64 platforms |
Uh oh!
There was an error while loading. Please reload this page.
MichalStrehovsky
commented
Dec 18, 2025
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
VSadov
commented
Dec 19, 2025
Yes, I think so. What registers may need updating when GC happens is encoded by the JIT for every possible GC site. But in order to update, these registers must be saved/restored in a state that GC can modify. I did not look in details at the changes yet, but the LLM-like approach by analogy would totally make sense. On every platform we need to add one more register to the set that is stored/restored by the hijacking. The tracking and updating will happen by itself, but it needs a stored value to work with. |
Uh oh!
There was an error while loading. Please reload this page.
VSadov
commented
Dec 20, 2025
LGTM. Are you going to do the same for other platforms too? |
am11
commented
Dec 20, 2025
Others are blocked on #121871. Would be nice to prioritize that so we can start testing async everywhere earlier in .NET 11 development cycle. |
Co-authored-by: Vladimir Sadov <vsadov@microsoft.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
MichalStrehovsky
commented
Dec 22, 2025
Andy was suggesting I don't do all the runtime async work, so not going to work on this for now. I needed x64 in #122526 because all the crashing was causing too much noise and I didn't know if we have any other bugs.
Reporting the register doesn't need to wait for that one. We report the extra register no matter if the called method is async or not (or has a return value at all). The other platforms will probably look the same - report extra register no matter what. If the assembly is not right, it's going to crash without runtime async too. |
am11
commented
Dec 22, 2025
Yup, no problem implementing them in code but I meant testing won't be possible as non-x64 64-bit platforms seem to be having problem during native link step (with runtime-async:on). |
Uh oh!
There was an error while loading. Please reload this page.
Contributes to #122492.
(Please review as if LLM wrote this. LLM didn't write this, but there was a lot of pattern matching without understanding.)
Cc @dotnet/ilc-contrib