Implement classMustBeLoadedBeforeCodeIsRun in crossgen2 (#608)

* Implement classMustBeLoadedBeforeCodeIsRun in crossgen2

This fixes missing eager fixups for method call arguments and return
value for methods going through PreStub.
It fixes the coreclr JIT\Regression\CLR-x86-JIT\V1.2-M01\b02345\b02345
test that needed Span<byte> type during GC stack walk with PreStub of a
method with Span<byte> argument on the stack and failed as it couldn't
be loaded during the GC.
The same failure also happened in many if not all tests when I have
attempted to run them with GCStress 3.

* Reflect PR feedback on member / property naming and method location

diff --git a/src/coreclr/src/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/src/tools/Common/JitInterface/CorInfoImpl.cs
index 7f9e6ed..fa58c82 100644 (file)
--- a/src/coreclr/src/tools/Common/JitInterface/CorInfoImpl.cs
+++ b/src/coreclr/src/tools/Common/JitInterface/CorInfoImpl.cs
@@ -421,7 +421,6 @@ namespace Internal.JitInterface
             {
                 methodInfo->options |= CorInfoOptions.CORINFO_GENERICS_CTXT_FROM_METHODTABLE;
             }
-
             methodInfo->regionKind = CorInfoRegionKind.CORINFO_REGION_NONE;
             Get_CORINFO_SIG_INFO(method, &methodInfo->args);
             Get_CORINFO_SIG_INFO(methodIL.GetLocals(), &methodInfo->locals);
@@ -480,7 +479,7 @@ namespace Internal.JitInterface
 
             CorInfoType corInfoRetType = asCorInfoType(signature.ReturnType, &sig->retTypeClass);
             sig->_retType = (byte)corInfoRetType;
-            sig->retTypeSigClass = sig->retTypeClass; // The difference between the two is not relevant for ILCompiler
+            sig->retTypeSigClass = ObjectToHandle(signature.ReturnType);
 
             sig->flags = 0;    // used by IL stubs code
 
@@ -1655,10 +1654,6 @@ namespace Internal.JitInterface
             return CorInfoInitClassResult.CORINFO_INITCLASS_USE_HELPER;
         }
 
-        private void classMustBeLoadedBeforeCodeIsRun(CORINFO_CLASS_STRUCT_* cls)
-        {
-        }
-
         private CORINFO_CLASS_STRUCT_* getBuiltinClass(CorInfoClassId classId)
         {
             switch (classId)
@@ -2791,16 +2786,6 @@ namespace Internal.JitInterface
             // Nothing to do
         }
 
-        private void setEHcount(uint cEH)
-        {
-            _ehClauses = new CORINFO_EH_CLAUSE[cEH];
-        }
-
-        private void setEHinfo(uint EHnumber, ref CORINFO_EH_CLAUSE clause)
-        {
-            _ehClauses[EHnumber] = clause;
-        }
-
         private bool logMsg(uint level, byte* fmt, IntPtr args)
         {
             // Console.WriteLine(Marshal.PtrToStringAnsi((IntPtr)fmt));

diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodWithGCInfo.cs b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodWithGCInfo.cs
index 4581af7..8009d33 100644 (file)
--- a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodWithGCInfo.cs
+++ b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodWithGCInfo.cs
@@ -26,10 +26,12 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
         private OffsetMapping[] _debugLocInfos;
         private NativeVarInfo[] _debugVarInfos;
         private DebugEHClauseInfo[] _debugEHClauseInfos;
+        private List<ISymbolNode> _fixups;
 
         public MethodWithGCInfo(MethodDesc methodDesc, SignatureContext signatureContext)
         {
             GCInfoNode = new MethodGCInfoNode(this);
+            _fixups = new List<ISymbolNode>();
             _method = methodDesc;
             SignatureContext = signatureContext;
         }
@@ -42,6 +44,8 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
 
         public MethodDesc Method => _method;
 
+        public List<ISymbolNode> Fixups => _fixups;
+
         public bool IsEmpty => _methodCode.Data.Length == 0;
 
         public override ObjectData GetData(NodeFactory factory, bool relocsOnly)
@@ -105,6 +109,17 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
                 }
             }
 
+            foreach (ISymbolNode node in _fixups)
+            {
+                if (fixupCells == null)
+                {
+                    fixupCells = new List<FixupCell>();
+                }
+
+                Import fixupCell = (Import)node;
+                fixupCells.Add(new FixupCell(fixupCell.Table.IndexFromBeginningOfArray, fixupCell.OffsetFromBeginningOfArray));
+            }
+
             if (fixupCells == null)
             {
                 return null;
@@ -112,6 +127,28 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
 
             fixupCells.Sort(FixupCell.Comparer);
 
+            // Deduplicate fixupCells
+            int j = 0;
+            for (int i = 1; i < fixupCells.Count; i++)
+            {
+                if (FixupCell.Comparer.Compare(fixupCells[j], fixupCells[i]) != 0)
+                {
+                    j++;
+                    if (i != j)
+                    {
+                        fixupCells[j] = fixupCells[i];
+                    }
+                }
+            }
+
+            // Move j to point after the last valid fixupCell in the array
+            j++;
+
+            if (j < fixupCells.Count)
+            {
+                fixupCells.RemoveRange(j, fixupCells.Count - j);
+            }
+
             NibbleWriter writer = new NibbleWriter();
 
             int curTableIndex = -1;
@@ -162,7 +199,14 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
 
         protected override DependencyList ComputeNonRelocationBasedDependencies(NodeFactory factory)
         {
-            return new DependencyList(new DependencyListEntry[] { new DependencyListEntry(GCInfoNode, "Unwind & GC info") });
+            DependencyList dependencyList = new DependencyList(new DependencyListEntry[] { new DependencyListEntry(GCInfoNode, "Unwind & GC info") });
+
+            foreach (ISymbolNode node in _fixups)
+            {
+                dependencyList.Add(node, "classMustBeLoadedBeforeCodeIsRun");
+            }
+
+            return dependencyList;
         }
 
         public override bool StaticDependenciesAreComputed => _methodCode != null;

diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs
index e9ac2ad..1c64328 100644 (file)
--- a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs
+++ b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs
@@ -1360,6 +1360,18 @@ namespace Internal.JitInterface
             return attribs;
         }
 
+        private void classMustBeLoadedBeforeCodeIsRun(CORINFO_CLASS_STRUCT_* cls)
+        {
+            TypeDesc type = HandleToObject(cls);
+            classMustBeLoadedBeforeCodeIsRun(type);
+        }
+
+        private void classMustBeLoadedBeforeCodeIsRun(TypeDesc type)
+        {
+            ISymbolNode node = _compilation.SymbolNodeFactory.CreateReadyToRunHelper(ReadyToRunHelperId.TypeHandle, type, GetSignatureContext());
+            ((MethodWithGCInfo)_methodCodeNode).Fixups.Add(node);
+        }
+
         private void getCallInfo(ref CORINFO_RESOLVED_TOKEN pResolvedToken, CORINFO_RESOLVED_TOKEN* pConstrainedResolvedToken, CORINFO_METHOD_STRUCT_* callerHandle, CORINFO_CALLINFO_FLAGS flags, CORINFO_CALL_INFO* pResult)
         {
             MethodDesc methodToCall;
@@ -2048,5 +2060,48 @@ namespace Internal.JitInterface
         }
 
         private int SizeOfPInvokeTransitionFrame => ReadyToRunRuntimeConstants.READYTORUN_PInvokeTransitionFrameSizeInPointerUnits * _compilation.NodeFactory.Target.PointerSize;
+
+        private void setEHcount(uint cEH)
+        {
+            _ehClauses = new CORINFO_EH_CLAUSE[cEH];
+        }
+
+        private void setEHinfo(uint EHnumber, ref CORINFO_EH_CLAUSE clause)
+        {
+            // Filters don't have class token in the clause.ClassTokenOrOffset
+            if ((clause.Flags & CORINFO_EH_CLAUSE_FLAGS.CORINFO_EH_CLAUSE_FILTER) == 0)
+            {
+                if (clause.ClassTokenOrOffset != 0)
+                {
+                    MethodIL methodIL = _compilation.GetMethodIL(MethodBeingCompiled);
+                    mdToken classToken = (mdToken)clause.ClassTokenOrOffset;
+                    TypeDesc clauseType = (TypeDesc)ResolveTokenInScope(methodIL, MethodBeingCompiled, classToken);
+
+                    CORJIT_FLAGS flags = default(CORJIT_FLAGS);
+                    getJitFlags(ref flags, 0);
+
+                    if (flags.IsSet(CorJitFlag.CORJIT_FLAG_IL_STUB))
+                    {
+                        // IL stub tokens are 'private' and do not resolve correctly in their parent module's metadata.
+
+                        // Currently, the only place we are using a token here is for a COM-to-CLR exception-to-HRESULT
+                        // mapping catch clause.  We want this catch clause to catch all exceptions, so we override the
+                        // token to be mdTypeRefNil, which used by the EH system to mean catch(...)
+                        Debug.Assert(clauseType.IsObject);
+                        clause.ClassTokenOrOffset = 0;
+                    }
+                    else
+                    {
+                        // For all clause types add fixup to ensure the types are loaded before the code of the method
+                        // containing the catch blocks is executed. This ensures that a failure to load the types would
+                        // not happen when the exception handling is in progress and it is looking for a catch handler.
+                        // At that point, we could only fail fast.
+                        classMustBeLoadedBeforeCodeIsRun(clauseType);
+                    }
+                }
+            }
+
+            _ehClauses[EHnumber] = clause;
+        }
     }
 }