Skip to content

Conversation

@jakobbotsch
Copy link
Member

The code path assumed we didn't have tailcalls on x86, but we do see them for CEE_JMP specifically. In that case the GS cookie check may need a register in AOT scenarios, so give it one.

Fix #121133

Copilot AI review requested due to automatic review settings October 29, 2025 11:31
@github-actions github-actions bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Oct 29, 2025
@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Copy link
Contributor

Copilot AI left a 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 a bug in the x86 GS cookie check logic by adding support for tail calls. Previously, the code asserted that tail calls were not supported on x86, but this was incorrect as explicit tail calls and CEE_JMP instructions can occur on this platform.

Key Changes

  • Removed the incorrect assert(!tailCall) on x86 that prevented tail calls from being handled
  • Added a tail call path that returns RBM_ESI (a callee-saved register) to avoid conflicts with argument registers
  • Updated comment to reflect the handling of both regular calls and tail calls

@jakobbotsch
Copy link
Member Author

PTAL @dotnet/jit-contrib. More edge cases around register picking for the GS cookie check. This kind of codegen that is not under LSRA's purview is clearly very hard to get right. If we make changes in the area again we should try hard to represent it as normal IR.

@jakobbotsch jakobbotsch requested a review from a team October 29, 2025 12:20
@jakobbotsch
Copy link
Member Author

/ba-g Helix timeout

@jakobbotsch jakobbotsch merged commit 19ca5df into dotnet:main Oct 30, 2025
115 of 119 checks passed
@jakobbotsch jakobbotsch deleted the fix-121133 branch October 30, 2025 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test failure: JIT/Directed/pinvoke/tail_pinvoke/tail_pinvoke.cmd

2 participants