From aa4eb884d79cbd63ffdac8c8c199258dce653ba0 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Mon, 9 Dec 2019 22:21:26 +0100 Subject: [PATCH] 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 type during GC stack walk with PreStub of a method with Span 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 --- .../src/tools/Common/JitInterface/CorInfoImpl.cs | 17 +------ .../ReadyToRun/MethodWithGCInfo.cs | 46 +++++++++++++++++- .../JitInterface/CorInfoImpl.ReadyToRun.cs | 55 ++++++++++++++++++++++ 3 files changed, 101 insertions(+), 17 deletions(-) diff --git a/src/coreclr/src/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/src/tools/Common/JitInterface/CorInfoImpl.cs index 7f9e6ed..fa58c82 100644 --- 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 --- 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 _fixups; public MethodWithGCInfo(MethodDesc methodDesc, SignatureContext signatureContext) { GCInfoNode = new MethodGCInfoNode(this); + _fixups = new List(); _method = methodDesc; SignatureContext = signatureContext; } @@ -42,6 +44,8 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun public MethodDesc Method => _method; + public List 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(); + } + + 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 --- 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; + } } } -- 2.7.4