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.

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

[release/9.0-staging] Do not substitute return values of constrained calls #113462

Open
wants to merge 1 commit into
base: release/9.0-staging
Choose a base branch
from

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Mar 13, 2025

Backport of #111030 to release/9.0-staging

/cc @MichalStrehovsky

Customer Impact

  • Customer reported
  • Found internally

This was hit by two customers: #110932 and #113423.

Certain static interface method invocation patterns can get miscompiled into an infinite loop. Instead of calling the method, we loop forever.

Regression

  • Yes
  • No

Regressed when we added more dead code elimination. Dead code elimination incorrectly proves the branch as unreachable.

Testing

This was missed because it requires some luck to hit:

  • Static interface method call needs to be part of an if check.
  • The static interface method needs to have a default implementation.
  • The default implementation needs to be simple enough that we can statically analyze it.

We now have a regression test for this pattern.

Risk

Low: this has been fixed in the .NET 10 branch for two months and the fix is obvious (in retrospect).

IMPORTANT: If this backport is for a servicing release, please verify that:

  • The PR target branch is release/X.0-staging, not release/X.0.

Package authoring no longer needed in .NET 9

IMPORTANT: Starting with .NET 9, you no longer need to edit a NuGet package's csproj to enable building and bump the version.
Keep in mind that we still need package authoring in .NET 8 and older versions.

Fixes #110932.

The constraint would need to be resolved first.
Copy link
Contributor

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

@akoeplinger akoeplinger added the Servicing-consider Issue for next servicing release review label Mar 13, 2025
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 this to the 9.0.x milestone Mar 13, 2025
@leecow leecow added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Mar 18, 2025
@leecow leecow modified the milestones: 9.0.x, 9.0.5 Mar 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-NativeAOT-coreclr Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants