Fix ARM64 interpreter asm helpers#120958
Merged
janvorli merged 3 commits intodotnet:mainfrom Oct 22, 2025
Merged
Conversation
Contributor
|
Tagging subscribers to this area: @BrzVlad, @janvorli, @kg |
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR fixes multiple bugs in ARM64 interpreter assembly helper routines discovered through CoreCLR testing on macOS arm64. The fixes address incorrect register usage, stack pointer instability, and missing byte-level copy logic.
Key Changes:
- Fixed
InterpreterStubRetBuffto use register x8 directly instead of loading from uninitialized memory - Corrected
Copy_Refmacro loop logic to handle 8-byte and 1-byte copies properly - Added byte-level copy support in
Load_Stackfor non-8-byte-aligned sizes - Fixed
CallJittedMethodRetXXXfunctions to use stable frame pointer instead of modified stack pointer
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/coreclr/vm/arm64/asmhelpers.asm | Windows ARM64 assembly fixes for interpreter stubs, copy macros, and jitted method callers |
| src/coreclr/vm/arm64/asmhelpers.S | Unix/macOS ARM64 assembly fixes mirroring the .asm changes plus Load_Stack byte copy support |
d8d920b to
3321147
Compare
davidwrighton
approved these changes
Oct 21, 2025
BrzVlad
reviewed
Oct 22, 2025
BrzVlad
reviewed
Oct 22, 2025
Running the CoreCLR tests fully interpreted on macOS arm64 has revealed a number of bugs in the assembler helper routines for call stubs. * Load_Stack on macOS arm64 can get size that's not a multiple of 8 * InterpreterStubRetBuff was loading the return buffer address from a slot that is usually not initialized by the x8. And the x8 is actually valid at that place, so we can get the value from it. * The Copy_Ref was fixed in another PR recently, but the same bug that was fixed for the copying by 16 bytes was there for copying by 8 and 1 byte. * The CallJittedMethodRetXXX functions were restoring the x2 from a SP relative address, but SP was modified after the x2 was stored there. The FP relative offset is stable, so I've switched to that one. This change fixes all of these.
610402a to
d19785f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Running the CoreCLR tests fully interpreted on macOS arm64 has revealed a number of bugs in the assembler helper routines for call stubs.
This change fixes all of these.