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

Port fix for dotnet/runtime#32059 to release/3.1 (JIT: make sure to set compLongUsed during struct promotion) #28024

Merged

Conversation

@AndyAyersMS
Copy link
Member

AndyAyersMS commented Feb 24, 2020

If we're promoting a long field, make sure compLongUsed gets set.
Otherwise we may fail to properly decompose a long later on, leading
to access violations in the jit.

See dotnet/runtime#32059 for the original bug report, and
dotnet/runtime#32702 for the fix in 5.0.

Customer Impact

Unexpected and hard to diagnose crash in the jit. No easy workaround.

Regression?

Yes, introduced during the development 3.0 cycle. 2.x behaves correctly.

Testing

Verified the user's test case now passes; no diffs seen in any existing
framework or test code.

Risk

Low: fix is surgical and enables existing long operand handling in
the jit in one case that can be overlooked. Only impacts x86 and arm
codegen. Problematic IL patterns may not be reachable from C#.

If we're promoting a long field, make sure `compLongUsed` gets set.
Otherwise we may fail to properly decompose a long later on, leading
to access violations in the jit.

See dotnet/runtime#32059 for the original bug report, and
dotnet/runtime#32702 for the fix in 5.0.

## Customer Impact
Unexpected and hard to diagnose crash in the jit. No easy workaround.

## Regression?
Yes, introduced during the development 3.0 cycle. 2.x behaves correctly.

## Testing
Verified the user's test case now passes; no diffs seen in any existing
framework or test code.

## Risk
**Low**: fix is surgical and enables existing long operand handling in
the jit in one case that can be overlooked. Only impacts x86 and arm
codegen. Problematic IL patterns may not be reachable from C#.
@jeffschwMSFT jeffschwMSFT added this to the 3.1.x milestone Feb 24, 2020
@jeffschwMSFT

This comment has been minimized.

Copy link
Member

jeffschwMSFT commented Feb 24, 2020

Approved, will seek approval for April release

@AndyAyersMS

This comment has been minimized.

Copy link
Member Author

AndyAyersMS commented Feb 24, 2020

@AndyAyersMS

This comment has been minimized.

Copy link
Member Author

AndyAyersMS commented Feb 25, 2020

Two tests failed -- one timed out, the other got some Linux TLS error; both are unrelated.

@leecow leecow modified the milestones: 3.1.x, 3.1.4 Feb 25, 2020
@danmosemsft danmosemsft changed the title Port fix for dotnet/runtime#32059 to release/3.1 Port fix for dotnet/runtime#32059 to release/3.1 (JIT: make sure to set compLongUsed during struct promotion) Mar 24, 2020
@Anipik Anipik merged commit f5d8d52 into dotnet:release/3.1 Mar 25, 2020
33 of 35 checks passed
33 of 35 checks passed
coreclr-ci Build #20200224.1 had test failures
Details
coreclr-ci (Test Pri0 CoreFX Windows_NT x64 checked) Test Pri0 CoreFX Windows_NT x64 checked was canceled
Details
WIP Ready for review
Details
coreclr-ci (Linux arm checked) Linux arm checked succeeded
Details
coreclr-ci (Linux arm64 checked) Linux arm64 checked succeeded
Details
coreclr-ci (Linux arm64 release) Linux arm64 release succeeded
Details
coreclr-ci (Linux x64 checked) Linux x64 checked succeeded
Details
coreclr-ci (Linux_musl x64 checked) Linux_musl x64 checked succeeded
Details
coreclr-ci (Linux_musl x64 release) Linux_musl x64 release succeeded
Details
coreclr-ci (Linux_rhel6 x64 release) Linux_rhel6 x64 release succeeded
Details
coreclr-ci (OSX x64 checked) OSX x64 checked succeeded
Details
coreclr-ci (Test Pri0 CoreFX Linux x64 checked) Test Pri0 CoreFX Linux x64 checked succeeded
Details
coreclr-ci (Test Pri0 Linux arm checked) Test Pri0 Linux arm checked succeeded
Details
coreclr-ci (Test Pri0 Linux arm64 checked) Test Pri0 Linux arm64 checked succeeded
Details
coreclr-ci (Test Pri0 Linux x64 checked) Test Pri0 Linux x64 checked succeeded
Details
coreclr-ci (Test Pri0 Linux_musl x64 checked) Test Pri0 Linux_musl x64 checked succeeded
Details
coreclr-ci (Test Pri0 Linux_musl x64 release) Test Pri0 Linux_musl x64 release succeeded
Details
coreclr-ci (Test Pri0 OSX x64 checked) Test Pri0 OSX x64 checked succeeded
Details
coreclr-ci (Test Pri0 R2R Linux x64 checked) Test Pri0 R2R Linux x64 checked succeeded
Details
coreclr-ci (Test Pri0 R2R OSX x64 checked) Test Pri0 R2R OSX x64 checked succeeded
Details
coreclr-ci (Test Pri0 R2R Windows_NT x64 checked) Test Pri0 R2R Windows_NT x64 checked succeeded
Details
coreclr-ci (Test Pri0 R2R Windows_NT x86 checked) Test Pri0 R2R Windows_NT x86 checked succeeded
Details
coreclr-ci (Test Pri0 Windows_NT arm64 checked) Test Pri0 Windows_NT arm64 checked succeeded
Details
coreclr-ci (Test Pri0 Windows_NT x64 checked) Test Pri0 Windows_NT x64 checked succeeded
Details
coreclr-ci (Test Pri0 Windows_NT x86 checked) Test Pri0 Windows_NT x86 checked succeeded
Details
coreclr-ci (Windows_NT arm checked) Windows_NT arm checked succeeded
Details
coreclr-ci (Windows_NT arm release) Windows_NT arm release succeeded
Details
coreclr-ci (Windows_NT arm64 checked) Windows_NT arm64 checked succeeded
Details
coreclr-ci (Windows_NT arm64 release) Windows_NT arm64 release succeeded
Details
coreclr-ci (Windows_NT x64 checked) Windows_NT x64 checked succeeded
Details
coreclr-ci (Windows_NT x64 debug) Windows_NT x64 debug succeeded
Details
coreclr-ci (Windows_NT x64 release) Windows_NT x64 release succeeded
Details
coreclr-ci (Windows_NT x86 checked) Windows_NT x86 checked succeeded
Details
coreclr-ci (Windows_NT x86 debug) Windows_NT x86 debug succeeded
Details
license/cla All CLA requirements met.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.