From 41bdf62ffde37187032dfa4bb14c6a656c6d12f6 Mon Sep 17 00:00:00 2001 From: Fadi Hanna Date: Wed, 18 Mar 2020 11:03:46 -0700 Subject: [PATCH] Fix for issue 32731 (#33534) * Couple of crossgen2 fixes: 1) Fix an issue hitting an assert in the TypeSystem's virtual function resolution 2) Port some inlining rules from crossgen1 - One of them fixes an issue where we would incorrectly inline a virtual method that has a MethodImpl associated with it (test = self_override5) 3) Remove an assert from R2RDump related to the composite work (assert needs to be after loading _readyToRunHeaderRVA, and is actually already at the right place a few lines below) --- .../Common/MetadataVirtualMethodAlgorithm.cs | 2 +- .../Compiler/ReadyToRunCodegenCompilation.cs | 23 ++++++++++++++++++++++ .../ReadyToRunReader.cs | 1 - 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/coreclr/src/tools/Common/TypeSystem/Common/MetadataVirtualMethodAlgorithm.cs b/src/coreclr/src/tools/Common/TypeSystem/Common/MetadataVirtualMethodAlgorithm.cs index d27fbf6..dbd21a0 100644 --- a/src/coreclr/src/tools/Common/TypeSystem/Common/MetadataVirtualMethodAlgorithm.cs +++ b/src/coreclr/src/tools/Common/TypeSystem/Common/MetadataVirtualMethodAlgorithm.cs @@ -419,7 +419,7 @@ namespace Internal.TypeSystem foreach (MethodDesc memberMethod in unificationGroup) { MethodDesc nameSigMatchMemberMethod = FindMatchingVirtualMethodOnTypeByNameAndSigWithSlotCheck(memberMethod, currentType, reverseMethodSearch: true); - if (nameSigMatchMemberMethod != null) + if (nameSigMatchMemberMethod != null && nameSigMatchMemberMethod != memberMethod) { if (separatedMethods == null) separatedMethods = new MethodDescHashtable(); diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs index 03be7ba..77781f0 100644 --- a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs +++ b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs @@ -70,6 +70,17 @@ namespace ILCompiler public bool CanInline(MethodDesc caller, MethodDesc callee) { + if (JitConfigProvider.Instance.HasFlag(CorJitFlag.CORJIT_FLAG_DEBUG_CODE)) + { + // If the callee wants debuggable code, don't allow it to be inlined + return false; + } + + if (callee.IsNoInlining) + { + return false; + } + // Check to see if the method requires a security object. This means they call demand and // shouldn't be inlined. if (callee.RequireSecObject) @@ -77,6 +88,18 @@ namespace ILCompiler return false; } + // If the method is MethodImpl'd by another method within the same type, then we have + // an issue that the importer will import the wrong body. In this case, we'll just + // disallow inlining because getFunctionEntryPoint will do the right thing. + if (callee.IsVirtual) + { + MethodDesc calleeMethodImpl = callee.OwningType.FindVirtualFunctionTargetMethodOnObjectType(callee); + if (calleeMethodImpl != callee) + { + return false; + } + } + return NodeFactory.CompilationModuleGroup.CanInline(caller, callee); } diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.Reflection.ReadyToRun/ReadyToRunReader.cs b/src/coreclr/src/tools/crossgen2/ILCompiler.Reflection.ReadyToRun/ReadyToRunReader.cs index 4f7c0dc..fd60500 100644 --- a/src/coreclr/src/tools/crossgen2/ILCompiler.Reflection.ReadyToRun/ReadyToRunReader.cs +++ b/src/coreclr/src/tools/crossgen2/ILCompiler.Reflection.ReadyToRun/ReadyToRunReader.cs @@ -386,7 +386,6 @@ namespace ILCompiler.Reflection.ReadyToRun throw new BadImageFormatException("The file is not a ReadyToRun image"); } - Debug.Assert(!Composite); _assemblyCache.Add(metadata); DirectoryEntry r2rHeaderDirectory = PEReader.PEHeaders.CorHeader.ManagedNativeHeaderDirectory; -- 2.7.4