Uh oh!
There was an error while loading. Please reload this page.
JIT: avoid GS cookie conflict with secret stub argument in non-standard register - #133235
Open
jakobbotsch wants to merge 1 commit into
Open
JIT: avoid GS cookie conflict with secret stub argument in non-standard register#133235jakobbotsch wants to merge 1 commit into
jakobbotsch wants to merge 1 commit into
Conversation
AMD64 fast tailcall epilogs use R10 as a scratch register for GS cookie checks. SecretStubParam is also passed in the non-standard R10 argument register, so the epilog could overwrite the argument before jumping to the target. Use RAX for the GS cookie check when the fast tailcall carries SecretStubParam, and pass the tailcall context through LSRA so register kills and target candidates remain consistent with code generation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5aa2771a-ac3f-4516-bcac-b16c7c0a698d
|
Azure Pipelines: Successfully started running 6 pipeline(s). 10 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new AMD64 SecretStubParam path selects RAX for the GS cookie check, which can conflict with fast-tailcall R2R indirection-cell passing that also uses RAX.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR adjusts JIT tailcall GS cookie-check register selection so that AMD64 fast tailcalls carrying SecretStubParam (in R10) don’t have that argument clobbered by the epilog’s scratch usage, and it threads tailcall context into LSRA/codegen so register-kill modeling and call-target candidates match actual codegen.
Changes:
- Extend
genGetGSCookieTempRegsto optionally consider the tailcall node and switch AMD64 tailcall temp register selection whenSecretStubParamis present. - Pass the tailcall node through LSRA so cookie-check register kills and control-expression register candidates remain consistent.
- Deduplicate/debug-centralize the “tailcall args not trashed by epilog” validation via
genCheckTailCallEpilogRegisters.
File summaries
| File | Description |
|---|---|
| src/coreclr/jit/lsraxarch.cpp | Excludes GS-cookie temp regs (now call-aware) from fast-tailcall control-expression candidates. |
| src/coreclr/jit/lsrabuild.cpp | Models GS-cookie-check register kills with awareness of the fast-tailcall node. |
| src/coreclr/jit/codegenxarch.cpp | Makes GS-cookie temp-reg selection call-aware for tailcalls; adds debug epilog-register validation hook. |
| src/coreclr/jit/codegenarmarch.cpp | Uses shared debug tailcall epilog-register validation helper. |
| src/coreclr/jit/codegenriscv64.cpp | Uses shared debug tailcall epilog-register validation helper. |
| src/coreclr/jit/codegenloongarch64.cpp | Uses shared debug tailcall epilog-register validation helper. |
| src/coreclr/jit/codegeninterface.h | Updates genGetGSCookieTempRegs signature to accept an optional tailcall node. |
| src/coreclr/jit/codegencommon.cpp | Implements shared debug validator and adds AMD64 tailcall temp-reg selection for SecretStubParam. |
| src/coreclr/jit/codegen.h | Declares genCheckTailCallEpilogRegisters for debug builds. |
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 1
- Review effort level: Lite
Comment on lines
+2983
to
+2987
| if ((tailCallNode != nullptr) && | ||
| (tailCallNode->gtArgs.FindWellKnownArg(WellKnownArg::SecretStubParam) != nullptr)) | ||
| { | ||
| return RBM_RAX; | ||
| } |
This was referenced Sep 4, 2026
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
AMD64 fast tailcall epilogs use R10 as a scratch register for GS cookie checks. SecretStubParam is also passed in the non-standard R10 argument register, so the epilog could overwrite the argument before jumping to the target.
Use RAX for the GS cookie check when the fast tailcall carries SecretStubParam, and pass the tailcall context through LSRA so register kills and target candidates remain consistent with code generation.
Fix#132801
Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com