Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[release/9.0-staging] Fix calling convention mismatch in GC callouts #111105

Merged
merged 3 commits into from
Jan 8, 2025
Merged

[release/9.0-staging] Fix calling convention mismatch in GC callouts #111105

merged 3 commits into from
Jan 8, 2025

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Jan 6, 2025 •
edited by AaronRobinsonMSFT
Loading

Backport of #110685 to release/9.0-staging

/cc @AaronRobinsonMSFT @MichalStrehovsky

Customer Impact

  • Customer reported
  • Found internally

The GC will crash in .NET 9 when published with native AOT on win-x86. A GC callback was defined with an incorrect calling convention. This was reported by a customer - #110607.

Related servicing issue: #110815

Regression

  • Yes
  • No

Support for x86_32 was introduced in .NET 9.

Testing

Validated this with the help of Windows Store owners.

Risk

Low

@MichalStrehovsky
Fix calling convention mismatch in GC callouts
3d03a9e 
Fixes #110607.

The native side expects fastcall.

Filed #110684 on the test hole. We would have caught it during x86 bringup if this had _any_ tests since this is a guaranteed stack corruption and crash.
@MichalStrehovsky
Change native side instead
fe399fb
@dotnet-policy-service Dotnet Policy Service
Copy link
Contributor

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

@AaronRobinsonMSFT AaronRobinsonMSFT added the Servicing-consider Issue for next servicing release review label Jan 6, 2025
@AaronRobinsonMSFT AaronRobinsonMSFT added this to the 9.0.x milestone Jan 6, 2025
jeffschwMSFT
jeffschwMSFT approved these changes Jan 6, 2025
View reviewed changes
Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. we will take for consideration in 9.0.x

@jeffschwMSFT jeffschwMSFT added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Jan 8, 2025
@jeffschwMSFT jeffschwMSFT modified the milestones: 9.0.x, 9.0.2 Jan 8, 2025
@jeffschwMSFT jeffschwMSFT merged commit 7a95474 into release/9.0-staging Jan 8, 2025
10 of 16 checks passed
@jkotas jkotas deleted the backport/pr-110685-to-release/9.0-staging branch January 8, 2025 19:37
This was referenced Jan 8, 2025
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AaronRobinsonMSFT AaronRobinsonMSFT AaronRobinsonMSFT approved these changes

@jeffschwMSFT jeffschwMSFT jeffschwMSFT approved these changes

@jkotas jkotas jkotas approved these changes

@MichalStrehovsky MichalStrehovsky Awaiting requested review from MichalStrehovsky MichalStrehovsky is a code owner

Assignees
No one assigned
Labels
area-NativeAOT-coreclr Servicing-approved Approved for servicing release
Projects
None yet
Milestone
9.0.2
Development

Successfully merging this pull request may close these issues.
4 participants
@jkotas @jeffschwMSFT @AaronRobinsonMSFT @MichalStrehovsky