Support for LDVIRTFTN on GVM's requiring instantiating stubs (#6906)

I found out that one bucket of the remaining CoreCLR CPAOT failures
is caused by missing support for instantiating stubs. We must emit
instantiating stubs whenever we hit LDVIRTFTN on a generic method,
otherwise JIT crashes with an assertion failure.

This case was already supposed to be taken care of by means of the
DynamicHelperCell representing the instantiating stub but
the implementation was incomplete. This change properly wires up
the instantiation stub logic and fixes various transitive issues
it uncovered.

Most notably this involved fixing the calls to DynamicHelperCell
to pass around the exact method, rooting the canonical method for
compilation when emitting an instantiating stub and fixing method
fixup signature formatting to include the instantiating & unboxing
stub flag, otherwise we were hitting a spurious assertion failure
regarding output symbol duplicates in the CPAOT object emitter.

Thanks

Tomas

src/ILCompiler.ReadyToRun/src/Compiler/DependencyAnalysis/ReadyToRun/DelayLoadHelperMethodImport.cs

@@ -3,6 +3,8 @@
 // See the LICENSE file in the project root for more information.
 
 using System.Collections.Generic;
+
+using Internal.JitInterface;
 using Internal.Text;
 using Internal.TypeSystem;
 
@@ -15,24 +17,32 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
     /// </summary>
     public class DelayLoadHelperMethodImport : DelayLoadHelperImport, IMethodNode
     {
-        private readonly MethodDesc _methodDesc;
+        private readonly MethodWithToken _method;
+
+        private readonly bool _useInstantiatingStub;
 
         private readonly ReadyToRunHelper _helper;
 
         private readonly ImportThunk _delayLoadHelper;
 
+        private readonly SignatureContext _signatureContext;
+
         public DelayLoadHelperMethodImport(
             ReadyToRunCodegenNodeFactory factory, 
             ImportSectionNode importSectionNode, 
             ReadyToRunHelper helper, 
-            MethodDesc methodDesc,
+            MethodWithToken method,
+            bool useInstantiatingStub,
             Signature instanceSignature, 
+            SignatureContext signatureContext,
             string callSite = null)
             : base(factory, importSectionNode, helper, instanceSignature, callSite)
         {
             _helper = helper;
-            _methodDesc = methodDesc;
+            _method = method;
+            _useInstantiatingStub = useInstantiatingStub;
             _delayLoadHelper = new ImportThunk(helper, factory, this);
+            _signatureContext = signatureContext;
         }
 
         public override void AppendMangledName(NameMangler nameMangler, Utf8StringBuilder sb)
@@ -47,9 +57,29 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
                 sb.Append(CallSite);
             }
         }
+        public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFactory factory)
+        {
+            foreach (DependencyListEntry baseEntry in base.GetStaticDependencies(factory))
+            {
+                yield return baseEntry;
+            }
+            if (_useInstantiatingStub)
+            {
+                // Require compilation of the canonical version for instantiating stubs
+                MethodDesc canonMethod = _method.Method.GetCanonMethodTarget(CanonicalFormKind.Specific);
+                ReadyToRunCodegenNodeFactory r2rFactory = (ReadyToRunCodegenNodeFactory)factory;
+                ISymbolNode canonMethodNode = r2rFactory.MethodEntrypoint(
+                    canonMethod,
+                    constrainedType: null,
+                    originalMethod: canonMethod,
+                    methodToken: _method.Token,
+                    signatureContext: _signatureContext);
+                yield return new DependencyListEntry(canonMethodNode, "Canonical method for instantiating stub");
+            }
+        }
 
         public override int ClassCode => 192837465;
 
-        public MethodDesc Method => _methodDesc;
+        public MethodDesc Method => _method.Method;
     }
 }

src/ILCompiler.ReadyToRun/src/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs

@@ -69,7 +69,20 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
         public override void AppendMangledName(NameMangler nameMangler, Utf8StringBuilder sb)
         {
             sb.Append(nameMangler.CompilationUnitPrefix);
-            sb.Append($@"MethodFixupSignature({_fixupKind.ToString()}: {_methodDesc.Signature.ReturnType} {_methodDesc}");
+            sb.Append($@"MethodFixupSignature(");
+            sb.Append(_fixupKind.ToString());
+            if (_isUnboxingStub)
+            {
+                sb.Append(" [UNBOX]");
+            }
+            if (_isInstantiatingStub)
+            {
+                sb.Append(" [INST]");
+            }
+            sb.Append(": ");
+            sb.Append(_methodDesc.Signature.ReturnType.ToString());
+            sb.Append(" ");
+            sb.Append(_methodDesc.ToString());
             if (_constrainedType != null)
             {
                 sb.Append(" @ ");

src/ILCompiler.ReadyToRun/src/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs

@@ -521,10 +521,12 @@ namespace ILCompiler.DependencyAnalysis
             ISymbolNode result;
             if (!_dynamicHelperCellCache.TryGetValue(methodWithToken, out result))
             {
-                result = new DelayLoadHelperImport(
+                result = new DelayLoadHelperMethodImport(
                     this,
                     DispatchImports,
                     ILCompiler.DependencyAnalysis.ReadyToRun.ReadyToRunHelper.READYTORUN_HELPER_DelayLoad_Helper_Obj,
+                    methodWithToken,
+                    useInstantiatingStub: true,
                     MethodSignature(
                         ReadyToRunFixupKind.READYTORUN_FIXUP_VirtualEntry,
                         methodWithToken.Method,
@@ -532,7 +534,8 @@ namespace ILCompiler.DependencyAnalysis
                         methodWithToken.Token,
                         signatureContext: signatureContext,
                         isUnboxingStub: false,
-                        isInstantiatingStub: false));
+                        isInstantiatingStub: true),
+                    signatureContext);
                 _dynamicHelperCellCache.Add(methodWithToken, result);
             }
             return result;

src/ILCompiler.ReadyToRun/src/Compiler/DependencyAnalysis/ReadyToRunSymbolNodeFactory.cs

@@ -206,14 +206,16 @@ namespace ILCompiler.DependencyAnalysis
                 _codegenNodeFactory,
                 _codegenNodeFactory.DispatchImports,
                 ILCompiler.DependencyAnalysis.ReadyToRun.ReadyToRunHelper.READYTORUN_HELPER_DelayLoad_Helper_Obj,
-                methodWithToken.Method,
+                methodWithToken,
+                useInstantiatingStub: false,
                 _codegenNodeFactory.MethodSignature(
                     ReadyToRunFixupKind.READYTORUN_FIXUP_VirtualEntry, methodWithToken.Method,
                     constrainedType: null, 
                     methodWithToken.Token, 
                     signatureContext: signatureContext, 
                     isUnboxingStub: false, 
-                    isInstantiatingStub: false));
+                    isInstantiatingStub: false),
+                signatureContext);
         }
 
         private ISymbolNode CreateDelegateCtorHelper(DelegateCreationInfo info, SignatureContext signatureContext)
@@ -591,9 +593,11 @@ namespace ILCompiler.DependencyAnalysis
                     _codegenNodeFactory.DispatchImports,
                     ILCompiler.DependencyAnalysis.ReadyToRun.ReadyToRunHelper.READYTORUN_HELPER_DelayLoad_MethodCall |
                     ILCompiler.DependencyAnalysis.ReadyToRun.ReadyToRunHelper.READYTORUN_HELPER_FLAG_VSD,
-                    method,
+                    new MethodWithToken(method, methodToken),
+                    useInstantiatingStub: false,
                     _codegenNodeFactory.MethodSignature(ReadyToRunFixupKind.READYTORUN_FIXUP_VirtualEntry, method,
                         null, methodToken, signatureContext, isUnboxingStub, isInstantiatingStub: false),
+                    signatureContext,
                     callSite);
 
                 _interfaceDispatchCells.Add(cellKey, dispatchCell);
@@ -866,8 +870,10 @@ namespace ILCompiler.DependencyAnalysis
                     _codegenNodeFactory,
                     _codegenNodeFactory.HelperImports,
                     ILCompiler.DependencyAnalysis.ReadyToRun.ReadyToRunHelper.READYTORUN_HELPER_DelayLoad_Helper,
-                    methodArgument.Method,
-                    new GenericLookupSignature(runtimeLookupKind,  fixupKind,  typeArgument: null, methodArgument, fieldArgument: null, methodContext, signatureContext));
+                    methodArgument,
+                    useInstantiatingStub: false,
+                    new GenericLookupSignature(runtimeLookupKind,  fixupKind,  typeArgument: null, methodArgument, fieldArgument: null, methodContext, signatureContext),
+                    signatureContext);
                 _genericLookupHelpers.Add(key, node);
             }
             return node;

src/ILCompiler.ReadyToRun/src/JitInterface/CorInfoImpl.ReadyToRun.cs

@@ -865,6 +865,7 @@ namespace Internal.JitInterface
 
             bool directCall = false;
             bool resolvedCallVirt = false;
+            bool useInstantiatingStub = false;
 
             if ((flags & CORINFO_CALLINFO_FLAGS.CORINFO_CALLINFO_LDFTN) != 0)
             {
@@ -972,8 +973,10 @@ namespace Internal.JitInterface
 
                         if (pResult->kind == CORINFO_CALL_KIND.CORINFO_VIRTUALCALL_LDVIRTFTN)
                         {
+                            MethodDesc exactMethod = (MethodDesc)GetRuntimeDeterminedObjectForToken(ref pResolvedToken);
+                            useInstantiatingStub = true;
                             pResult->codePointerOrStubLookup.constLookup = CreateConstLookupToSymbol(_compilation.NodeFactory.DynamicHelperCell(
-                                new MethodWithToken(targetMethod, new ModuleToken(_tokenContext, pResolvedToken.token)), _signatureContext));
+                                new MethodWithToken(exactMethod, new ModuleToken(_tokenContext, pResolvedToken.token)), _signatureContext));
                         }
                         else
                         {
@@ -990,11 +993,14 @@ namespace Internal.JitInterface
             else if (method.HasInstantiation)
             {
                 // GVM Call Support
+                MethodDesc exactMethod = (MethodDesc)GetRuntimeDeterminedObjectForToken(ref pResolvedToken);
+
                 pResult->kind = CORINFO_CALL_KIND.CORINFO_VIRTUALCALL_LDVIRTFTN;
                 pResult->nullInstanceCheck = true;
                 pResult->codePointerOrStubLookup.constLookup = CreateConstLookupToSymbol(
                     _compilation.NodeFactory.DynamicHelperCell(
-                        new MethodWithToken(targetMethod, new ModuleToken(_tokenContext, pResolvedToken.token)), _signatureContext));
+                        new MethodWithToken(exactMethod, new ModuleToken(_tokenContext, pResolvedToken.token)), _signatureContext));
+                useInstantiatingStub = true;         
             }
             else
             // In ReadyToRun, we always use the dispatch stub to call virtual methods
@@ -1029,7 +1035,7 @@ namespace Internal.JitInterface
             pResult->classFlags = getClassAttribsInternal(targetMethod.OwningType);
 
             pResult->methodFlags = getMethodAttribsInternal(targetMethod);
-            Get_CORINFO_SIG_INFO(targetMethod, &pResult->sig);
+            Get_CORINFO_SIG_INFO(targetMethod, &pResult->sig, useInstantiatingStub);
 
             if ((flags & CORINFO_CALLINFO_FLAGS.CORINFO_CALLINFO_VERIFICATION) != 0)
             {

src/JitInterface/src/CorInfoImpl.cs

@@ -415,12 +415,12 @@ namespace Internal.JitInterface
             return methodIL;
         }
 
-        private void Get_CORINFO_SIG_INFO(MethodDesc method, CORINFO_SIG_INFO* sig, bool isFatFunctionPointer = false)
+        private void Get_CORINFO_SIG_INFO(MethodDesc method, CORINFO_SIG_INFO* sig, bool suppressHiddenArgument = false)
         {
             Get_CORINFO_SIG_INFO(method.Signature, sig);
 
             // Does the method have a hidden parameter?
-            bool hasHiddenParameter = method.RequiresInstArg() && !isFatFunctionPointer;
+            bool hasHiddenParameter = !suppressHiddenArgument && method.RequiresInstArg();
 
             if (method.IsIntrinsic)
             {

tests/src/Simple/ReadyToRunUnit/Program.cs

@@ -908,6 +908,34 @@ internal class Program
         return success;
     }
 
+    class ClassWithGVM
+    {
+        public virtual bool GVM<T>(string expectedTypeName)
+        {
+            string typeName = GetTypeName<T>();
+            Console.WriteLine("GVM<{0}> called ({1} expected)", typeName, expectedTypeName);
+            return typeName == expectedTypeName;
+        }
+    }
+
+    private static void GVMTestCase(Func<string, bool> gvm, string expectedTypeName, ref bool success)
+    {
+        if (!gvm(expectedTypeName))
+        {
+            success = false;
+        }
+    }
+
+    private static bool GVMTest()
+    {
+        ClassWithGVM gvmInstance = new ClassWithGVM();
+        bool success = true;
+        GVMTestCase(gvmInstance.GVM<int>, "System.Int32", ref success);
+        GVMTestCase(gvmInstance.GVM<object>, "System.Object", ref success);
+        GVMTestCase(gvmInstance.GVM<string>, "System.String", ref success);
+        return success;
+    }
+
     public static int Main(string[] args)
     {
         if (args.Length > 0)
@@ -961,6 +989,7 @@ internal class Program
         RunTest("SharedGenericTlsGcStaticTest", SharedGenericTlsGcStaticTest());
         RunTest("SharedGenericTlsNonGcStaticTest", SharedGenericTlsNonGcStaticTest());
         RunTest("VirtualDelegateLoadTest", VirtualDelegateLoadTest());
+        RunTest("GVMTest", GVMTest());
 
         Console.WriteLine($@"{_passedTests.Count} tests pass:");
         foreach (string testName in _passedTests)