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/8.0] JIT: Fix invalid zero-init suppression for untracked variables #91840

Merged
merged 3 commits into from
Sep 11, 2023

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Sep 9, 2023

Backport of #91580 to release/8.0

/cc @jakobbotsch

Customer Impact

The JIT may incorrectly remove zero-initialization of some local variables used in finally blocks, resulting in silent bad codegen through use of an uninitialized value. The bug occurs when:

  1. The local is initialized right before a try/finally at the beginning of a function
  2. The local is also defined inside the try block, but there is a statement that can throw an exception before this definition
  3. The local has its address taken
  4. The local is also used inside the finally block

In these cases the JIT may mistakenly remove the initialization before the try. If an exception is then thrown inside the try before the definition there, the finally block can observe an uninitialized value for the local.

Testing

Regression test included.

Risk

Low.

optRemoveRedundantZeroInits has logic to remove unnecessary zero inits
if we can determine that the local will be zeroed in the prolog. In
addition, it also has logic to suppress the prolog zero init if there is
a dominating initialization already. The latter logic was trying to
reason about liveness for untracked locals, which does not make sense.

Fix #91576
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Sep 9, 2023
@ghost
Copy link

ghost commented Sep 9, 2023

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

Issue Details

Backport of #91580 to release/8.0

/cc @jakobbotsch

Customer Impact

Testing

Risk

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.

  • If the change touches code that ships in a NuGet package, you have added the necessary package authoring and gotten it explicitly reviewed.

Author: github-actions[bot]
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

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.

approved. this can be merged once ready

@jeffschwMSFT jeffschwMSFT added the Servicing-approved Approved for servicing release label Sep 11, 2023
@jeffschwMSFT jeffschwMSFT added this to the 8.0.0 milestone Sep 11, 2023
@jeffschwMSFT jeffschwMSFT merged commit 1ce9687 into release/8.0 Sep 11, 2023
8 checks passed
@jkotas jkotas deleted the backport/pr-91580-to-release/8.0 branch September 11, 2023 20:19
@radical radical mentioned this pull request Sep 26, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Oct 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants