From aadc2dcb8c74f658ba586468d9ee6d631f0c2d62 Mon Sep 17 00:00:00 2001 From: Mark Plesko Date: Wed, 11 Jan 2023 00:08:48 -0800 Subject: [PATCH 1/3] Add test --- .../CctorsWithSideEffects/CctorForWrite.cs | 34 +++++++++++++++++++ .../CctorForWrite.csproj | 9 +++++ 2 files changed, 43 insertions(+) create mode 100644 src/tests/Loader/classloader/TypeInitialization/CctorsWithSideEffects/CctorForWrite.cs create mode 100644 src/tests/Loader/classloader/TypeInitialization/CctorsWithSideEffects/CctorForWrite.csproj diff --git a/src/tests/Loader/classloader/TypeInitialization/CctorsWithSideEffects/CctorForWrite.cs b/src/tests/Loader/classloader/TypeInitialization/CctorsWithSideEffects/CctorForWrite.cs new file mode 100644 index 00000000000000..807110f8e3935f --- /dev/null +++ b/src/tests/Loader/classloader/TypeInitialization/CctorsWithSideEffects/CctorForWrite.cs @@ -0,0 +1,34 @@ +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 new file mode 100644 index 00000000000000..6946bed81bfd5b --- /dev/null +++ b/src/tests/Loader/classloader/TypeInitialization/CctorsWithSideEffects/CctorForWrite.csproj @@ -0,0 +1,9 @@ + + + Exe + True + + + + + From 0bc6b93ca1b9fa6f9f1e5bf4fdd56d05adc8578b Mon Sep 17 00:00:00 2001 From: Mark Plesko Date: Wed, 11 Jan 2023 00:08:57 -0800 Subject: [PATCH 2/3] Fix --- src/coreclr/jit/importer.cpp | 68 ++++++++++++++++++------------------ 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 4a3b20eb855ade..8cccd36362634a 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -9537,21 +9537,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 @@ -9590,6 +9575,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)) { @@ -9691,7 +9705,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)); @@ -9774,24 +9792,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; } From 8fe001524fc36df88e8e601389efb87f318737f1 Mon Sep 17 00:00:00 2001 From: Mark Plesko Date: Wed, 11 Jan 2023 00:59:52 -0800 Subject: [PATCH 3/3] Add comment to test --- .../CctorsWithSideEffects/CctorForWrite.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/tests/Loader/classloader/TypeInitialization/CctorsWithSideEffects/CctorForWrite.cs b/src/tests/Loader/classloader/TypeInitialization/CctorsWithSideEffects/CctorForWrite.cs index 807110f8e3935f..775ab72511a398 100644 --- a/src/tests/Loader/classloader/TypeInitialization/CctorsWithSideEffects/CctorForWrite.cs +++ b/src/tests/Loader/classloader/TypeInitialization/CctorsWithSideEffects/CctorForWrite.cs @@ -1,3 +1,10 @@ +// 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;