Fix for Issue 44895 (#45284)

* 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

diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp
index 1f26aa8..04dfb9f 100644 (file)
--- a/src/coreclr/jit/gentree.cpp
+++ b/src/coreclr/jit/gentree.cpp
@@ -15492,6 +15492,13 @@ GenTree* Compiler::gtNewTempAssign(
     }
 #endif
 
+    // Added this noway_assert for runtime\issue 44895, to protect against silent bad codegen
+    //
+    if ((dstTyp == TYP_STRUCT) && (valTyp == TYP_REF))
+    {
+        noway_assert(!"Incompatible types for gtNewTempAssign");
+    }
+
     // Floating Point assignments can be created during inlining
     // see "Zero init inlinee locals:" in fgInlinePrependStatements
     // thus we may need to set compFloatingPointUsed to true here.

diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp
index bc69b8c..b0f895d 100644 (file)
--- a/src/coreclr/jit/morph.cpp
+++ b/src/coreclr/jit/morph.cpp
@@ -17951,16 +17951,38 @@ void Compiler::fgMorphStructField(GenTree* tree, GenTree* parent)
                         }
                         // Access the promoted field as a field of a non-promoted struct with the same class handle.
                     }
-#ifdef DEBUG
-                    else if (tree->TypeGet() == TYP_STRUCT)
+                    else
                     {
-                        // The field tree accesses it as a struct, but the promoted lcl var for the field
-                        // says that it has another type. It can happen only if struct promotion faked
-                        // field type for a struct of single field of scalar type aligned at their natural boundary.
+                        // As we already checked this above, we must have a tree with a TYP_STRUCT type
+                        //
+                        assert(tree->TypeGet() == TYP_STRUCT);
+
+                        // The field tree accesses it as a struct, but the promoted LCL_VAR field
+                        // says that it has another type. This happens when struct promotion unwraps
+                        // a single field struct to get to its ultimate type.
+                        //
+                        // Note that currently, we cannot have a promoted LCL_VAR field with a struct type.
+                        //
+                        // This mismatch in types can lead to problems for some parent node type like GT_RETURN.
+                        // So we check the parent node and only allow this optimization when we have
+                        // a GT_ADDR or a GT_ASG.
+                        //
+                        // Note that for a GT_ASG we have to do some additional work,
+                        // see below after the SetOper(GT_LCL_VAR)
+                        //
+                        if (!parent->OperIs(GT_ADDR, GT_ASG))
+                        {
+                            // Don't transform other operations such as GT_RETURN
+                            //
+                            return;
+                        }
+#ifdef DEBUG
+                        // This is an additional DEBUG-only sanity check
+                        //
                         assert(structPromotionHelper != nullptr);
                         structPromotionHelper->CheckRetypedAsScalar(field->gtFldHnd, fieldType);
-                    }
 #endif // DEBUG
+                    }
                 }
 
                 tree->SetOper(GT_LCL_VAR);
@@ -17970,6 +17992,9 @@ void Compiler::fgMorphStructField(GenTree* tree, GenTree* parent)
 
                 if (parent->gtOper == GT_ASG)
                 {
+                    // If we are changing the left side of an assignment, we need to set
+                    // these two flags:
+                    //
                     if (parent->AsOp()->gtOp1 == tree)
                     {
                         tree->gtFlags |= GTF_VAR_DEF;

diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_44895/Runtime_44895.cs b/src/tests/JIT/Regression/JitBlue/Runtime_44895/Runtime_44895.cs
new file mode 100644 (file)
index 0000000..3c33f89
--- /dev/null
+++ b/src/tests/JIT/Regression/JitBlue/Runtime_44895/Runtime_44895.cs
@@ -0,0 +1,37 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+
+public struct Text
+{
+    private readonly string _value;
+    public Text(string value) => _value = value;
+    public string Value => _value ?? string.Empty; 
+}
+
+public class TextProperty 
+{
+    public Text GetValue(Text? a = null, Text? b = null, Text? c = null, Text? d = null)
+    {
+        if (a.HasValue) return a.Value;
+        if (b.HasValue) return b.Value;
+        if (c.HasValue) return c.Value;
+        if (d.HasValue) return d.Value;
+        return default;
+    }
+}
+
+public class Repro
+{
+    public static int Main()
+    {
+        string test = "test";
+        TextProperty t = new TextProperty();
+        Text gv = t.GetValue(new Text(test));
+        bool result = test.Equals(gv.Value);
+        Console.WriteLine(result ? "Pass" : "Fail");
+        if (!result) Console.WriteLine($"got '{gv.Value}', expected '{test}'");
+        return result ? 100 : -1;
+    }
+}
\ No newline at end of file

diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_44895/Runtime_44895.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_44895/Runtime_44895.csproj
new file mode 100644 (file)
index 0000000..1100f42
--- /dev/null
+++ b/src/tests/JIT/Regression/JitBlue/Runtime_44895/Runtime_44895.csproj
@@ -0,0 +1,10 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <DebugType>None</DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+</Project>