Fix for issue 32731 (#33534)
authorFadi Hanna <fadim@microsoft.com>
Wed, 18 Mar 2020 18:03:46 +0000 (11:03 -0700)
committerGitHub <noreply@github.com>
Wed, 18 Mar 2020 18:03:46 +0000 (11:03 -0700)
* 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)

src/coreclr/src/tools/Common/TypeSystem/Common/MetadataVirtualMethodAlgorithm.cs
src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs
src/coreclr/src/tools/crossgen2/ILCompiler.Reflection.ReadyToRun/ReadyToRunReader.cs

index d27fbf6..dbd21a0 100644 (file)
@@ -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();
index 03be7ba..77781f0 100644 (file)
@@ -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);
         }
 
index 4f7c0dc..fd60500 100644 (file)
@@ -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;