JIT: Fix ordering of type initialization and stsfld (#80485)

Insertion of the type initializer before the tree for a stsfld could reorder them inappropriately. This includes those computation in the spill check. Fixes #72354.
dotnet · Mar 27, 2023 · e6e252b · e6e252b
1 parent 5856dde
commit e6e252b
Show file tree

Hide file tree

Showing 3 changed files with 84 additions and 34 deletions.
diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp
@@ -9489,21 +9489,6 @@ void Compiler::impImportBlockCode(BasicBlock* block)
                 int      aflags = CORINFO_ACCESS_SET;
                 GenTree* obj    = nullptr;
 
-                // Pull the value from the stack.
-                StackEntry se = impPopStack();
-                op2           = se.val;
-                clsHnd        = se.seTypeInfo.GetClassHandle();
-
-                if (opcode == CEE_STFLD)
-                {
-                    obj = impPopStack().val;
-
-                    if (impIsThis(obj))
-                    {
-                        aflags |= CORINFO_ACCESS_THIS;
-                    }
-                }
-
                 eeGetFieldInfo(&resolvedToken, (CORINFO_ACCESS_FLAGS)aflags, &fieldInfo);
 
                 // Figure out the type of the member.  We always call canAccessField, so you always need this
@@ -9542,6 +9527,35 @@ void Compiler::impImportBlockCode(BasicBlock* block)
 
                 impHandleAccessAllowed(fieldInfo.accessAllowed, &fieldInfo.accessCalloutHelper);
 
+                // Check if the class needs explicit initialization.
+                if (fieldInfo.fieldFlags & CORINFO_FLG_FIELD_INITCLASS)
+                {
+                    GenTree* helperNode = impInitClass(&resolvedToken);
+                    if (compDonotInline())
+                    {
+                        return;
+                    }
+                    if (helperNode != nullptr)
+                    {
+                        impAppendTree(helperNode, CHECK_SPILL_ALL, impCurStmtDI);
+                    }
+                }
+
+                // Pull the value from the stack.
+                StackEntry se = impPopStack();
+                op2           = se.val;
+                clsHnd        = se.seTypeInfo.GetClassHandle();
+
+                if (opcode == CEE_STFLD)
+                {
+                    obj = impPopStack().val;
+
+                    if (impIsThis(obj))
+                    {
+                        aflags |= CORINFO_ACCESS_THIS;
+                    }
+                }
+
                 // Raise InvalidProgramException if static store accesses non-static field
                 if (isStoreStatic && ((fieldInfo.fieldFlags & CORINFO_FLG_FIELD_STATIC) == 0))
                 {
@@ -9644,7 +9658,11 @@ void Compiler::impImportBlockCode(BasicBlock* block)
                         assert(!"Unexpected fieldAccessor");
                 }
 
-                if (lclTyp != TYP_STRUCT)
+                if (lclTyp == TYP_STRUCT)
+                {
+                    op1 = impAssignStruct(op1, op2, CHECK_SPILL_ALL);
+                }
+                else
                 {
                     assert(op1->OperIs(GT_FIELD, GT_IND));
 
@@ -9705,24 +9723,6 @@ void Compiler::impImportBlockCode(BasicBlock* block)
                     op1 = gtNewAssignNode(op1, op2);
                 }
 
-                // Check if the class needs explicit initialization.
-                if (fieldInfo.fieldFlags & CORINFO_FLG_FIELD_INITCLASS)
-                {
-                    GenTree* helperNode = impInitClass(&resolvedToken);
-                    if (compDonotInline())
-                    {
-                        return;
-                    }
-                    if (helperNode != nullptr)
-                    {
-                        impAppendTree(helperNode, CHECK_SPILL_ALL, impCurStmtDI);
-                    }
-                }
-
-                if (lclTyp == TYP_STRUCT)
-                {
-                    op1 = impAssignStruct(op1, op2, CHECK_SPILL_ALL);
-                }
                 goto SPILL_APPEND;
             }
 

diff --git a/src/tests/Loader/classloader/TypeInitialization/CctorsWithSideEffects/CctorForWrite.cs b/src/tests/Loader/classloader/TypeInitialization/CctorsWithSideEffects/CctorForWrite.cs
@@ -0,0 +1,41 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+// Showed a JIT importer bug where a tree would be constructed for
+// call / stsfld and then the type initialization call inserted before
+// the entire tree.  The call needs to happen before type initialization.
+
+using System;
+using System.Runtime.CompilerServices;
+
+public class CorrectException : Exception
+{
+}
+
+public class CCC
+{
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static int Call() => throw new CorrectException();
+
+    public static int Main()
+    {
+        try
+        {
+            ClassWithCctor.StaticField = Call();
+        }
+        catch (CorrectException)
+        {
+            return 100;
+        }
+        catch
+        {
+        }
+        return 1;
+    }
+}
+
+class ClassWithCctor
+{
+    public static int StaticField;
+    static ClassWithCctor() => throw new Exception();
+}
diff --git a/src/tests/Loader/classloader/TypeInitialization/CctorsWithSideEffects/CctorForWrite.csproj b/src/tests/Loader/classloader/TypeInitialization/CctorsWithSideEffects/CctorForWrite.csproj
@@ -0,0 +1,9 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+</Project>