Skip to content

[release/5.0] Fix for Issue 44895 - Incorrect codegen with multiple returns of a field of a Nullable or struct type - #45989

Merged
Anipik merged 1 commit into
dotnet:release/5.0from
briansull:fix-44895-5.0
Jan 14, 2021
Merged

[release/5.0] Fix for Issue 44895 - Incorrect codegen with multiple returns of a field of a Nullable or struct type#45989
Anipik merged 1 commit into
dotnet:release/5.0from
briansull:fix-44895-5.0

Conversation

@briansull

@briansullbriansull commented Dec 12, 2020

Copy link
Copy Markdown
Contributor

Backport of #45284 to release/5.0

Fixes: #44895

Customer Impact

Reported by customer running .net 5.0. When optimizing we would incorrectly optimize a method that returns a nullable type.
The method needed to have more than 4 returns in the method body. This is silent bad codegen when optimizing.
The user would see an invalid return value from the function call.
Note that this issue also applies to other struct types beyond the nullable types. It is just more commonly seen with a nullable type.
The return statement must be field of a struct that was struct promoted and the return value must be a struct with a single ref field.

Regression?

Yes, this is a regression from .NET Core 3.1

Testing

Manual, new unit test, CLR outerloop, asm diffs.

Risk

Low

  • Fix for Issue 44895
  • Fix: Don't allow an unwrapped promoted field of TYP_REF to be returned when we are expecting a TYP_STRUCT
  • Allow only GT_ADDR and GT_ASG as a parent node

  • Add test case Runtime_44895.cs

  • Change assert about Incompatible types to be a noway_assert in gtNewTempAssign

  • Added noway_assert in release build for an assignment of a TYP_REF to a TYP_STRUCT

* Fix for Issue 44895
- Fix: Don't allow an unwrapped promoted field of TYP_REF to be returned when we are expecting a TYP_STRUCT
Backout change in gtGetStructHandleIfPresent for GT_RETURN as it isn't needed for this fix
Deoptimize all GT_RETURN's with mismatched types for promoted struct fields.
* Allow both GT_ADDR and GT_ASG as a parent node
* Add second test case Repro2_44895.cs
* Change assert about Incompatible types to be a noway_assert in gtNewTempAssign
* Only use the smaller repro case for Runtime_44895.cs
* Added noway_assert in release build for an assignment of a TYP_REF to a TYP_STRUCT
* rerun jit-format
@Dotnet-GitSync-BotDotnet-GitSync-Bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Dec 12, 2020
@briansullbriansull added Servicing-consider Issue for next servicing release review regression-from-last-release labels Dec 12, 2020
@briansullbriansull changed the title Fix for Issue 44895 Incorrect codegen with multiple returns of Nullable typeFix for Issue 44895 Incorrect codegen with multiple returns of a field of a Nullable typeDec 12, 2020
@briansullbriansull changed the title Fix for Issue 44895 Incorrect codegen with multiple returns of a field of a Nullable typeFix for Issue 44895 Incorrect codegen with multiple returns of a field of a Nullable or struct typeDec 12, 2020
@briansullbriansull changed the title Fix for Issue 44895 Incorrect codegen with multiple returns of a field of a Nullable or struct typeFix for Issue 44895 - Incorrect codegen with multiple returns of a field of a Nullable or struct typeDec 12, 2020
@JulieLeeMSFTJulieLeeMSFT changed the title Fix for Issue 44895 - Incorrect codegen with multiple returns of a field of a Nullable or struct type[release/5.0] Fix for Issue 44895 - Incorrect codegen with multiple returns of a field of a Nullable or struct typeDec 13, 2020
@JulieLeeMSFTJulieLeeMSFT added this to the 5.0.x milestone Dec 13, 2020
@leecowleecow added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Dec 15, 2020
@leecowleecow modified the milestones: 5.0.x, 5.0.3Dec 15, 2020
@Anipik
Anipik merged commit a66f040 into dotnet:release/5.0Jan 14, 2021
@ghostghost locked as resolved and limited conversation to collaborators Feb 13, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMIregression-from-last-releaseServicing-approvedApproved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@briansull@AndyAyersMS@leecow@Anipik@Dotnet-GitSync-Bot@JulieLeeMSFT