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/6.0] Port #58410: Fix zero initialization use of "initReg" #62207

Merged
merged 1 commit into from
Dec 15, 2021

Conversation

BruceForstall
Copy link
Member

@BruceForstall BruceForstall commented Nov 30, 2021

Backport #58410 to release/6.0

Fixes #62103

Customer Impact

Silent bad codegen: in certain rare cases, a variable is not correctly zero initialized, possibly leading to incorrect results or crashes. Typically, this requires a single local variable that is a register-sized struct, marked as requiring zero initialization by the JIT.

The issue was originally found with internal "stress" testing late in .NET 6 product development, but the fix was not ported to the .NET 6 release branch. However, it was subsequently reported by an external F# customer on the 6.0 release.

This was a regression between .NET 5 and .NET 6 due to extensive struct optimization work in the JIT.

Testing

The fix has been in 'main' for 3 months, but wasn't ported to release/6.0 before release.

Risk

Low

…Reg"

If the only local is an enregistered must-init struct,
we were setting `initReg` to its register (in this case, `xmm0`).
However, `initReg` is expected to be an integer register. In the
test case, with GS cookie stress, the GS cookie code asserted
that `initReg` was an integer register, but it wasn't.

The fix is the change the condition to use the actual register
assigned to the variable (in this case, `xmm0`), not the variable
type (here, `TYP_STRUCT`).

No spmi asm diffs.

Fixes dotnet#57911
@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 Nov 30, 2021
@ghost
Copy link

ghost commented Nov 30, 2021

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

Issue Details

Backport #58410 to release/6.0

Customer Impact

Silent bad codegen: in certain rare cases, a variable is not correctly zero initialized, possibly leading to incorrect results or crashes.

The issue was originally found with internal "stress" testing, but was reported by an external F# customer on the 6.0 release.

Testing

The fix has been in 'main' for 3 months, but wasn't ported to release/6.0 before release.

Risk

Low

Author: BruceForstall
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@BruceForstall
Copy link
Member Author

@jakobbotsch @dotnet/jit-contrib PTAL: back-port of fix to .NET 6

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. We will take for consideration in 6.0.x

@jeffschwMSFT jeffschwMSFT added the Servicing-consider Issue for next servicing release review label Dec 1, 2021
@jeffschwMSFT jeffschwMSFT added this to the 6.0.x milestone Dec 1, 2021
@Pilchie Pilchie added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Dec 2, 2021
@Pilchie Pilchie modified the milestones: 6.0.x, 6.0.2 Dec 2, 2021
@safern safern merged commit 229abfc into dotnet:release/6.0 Dec 15, 2021
@BruceForstall BruceForstall deleted the Port58410 branch December 15, 2021 18:52
@ghost ghost locked as resolved and limited conversation to collaborators Jan 14, 2022
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.

6 participants