Fix GCRefMap for generic method calls via Unboxing stub (#1304)

Crossgen2 was generating GCRefMap for calls to generic methods via
unboxing stubs incorrectly. With unboxing stub, the caller doesn't pass
in an extra generic parameter since the reference to the boxed value
type passed to the stub serves that purpose. But the GetCallRefMap method
was generating GCRefMap entry for the extra generic argument, which was
leading to a crash when GC was scanning the arguments.

diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GCRefMapBuilder.cs b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GCRefMapBuilder.cs
index 9c7ef19..2b54e33 100644 (file)
--- a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GCRefMapBuilder.cs
+++ b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GCRefMapBuilder.cs
@@ -66,7 +66,7 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
             _transitionBlock = TransitionBlock.FromTarget(target);
         }
 
-        public void GetCallRefMap(MethodDesc method)
+        public void GetCallRefMap(MethodDesc method, bool isUnboxingStub)
         {
             TransitionBlock transitionBlock = TransitionBlock.FromTarget(method.Context.Target);
 
@@ -79,7 +79,7 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
                 parameterTypes[parameterIndex] = new TypeHandle(method.Signature[parameterIndex]);
             }
             CallingConventions callingConventions = (hasThis ? CallingConventions.ManagedInstance : CallingConventions.ManagedStatic);
-            bool hasParamType = method.GetCanonMethodTarget(CanonicalFormKind.Specific).RequiresInstArg();
+            bool hasParamType = method.RequiresInstArg() && !isUnboxingStub;
             bool extraFunctionPointerArg = false;
             bool[] forcedByRefParams = new bool[parameterTypes.Length];
             bool skipFirstArg = false;
@@ -102,7 +102,7 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
             CORCOMPILE_GCREFMAP_TOKENS[] fakeStack = new CORCOMPILE_GCREFMAP_TOKENS[transitionBlock.SizeOfTransitionBlock + nStackBytes];
 
             // Fill it in
-            FakeGcScanRoots(method, argit, fakeStack);
+            FakeGcScanRoots(method, argit, fakeStack, isUnboxingStub);
 
             // Encode the ref map
             uint nStackSlots;
@@ -147,7 +147,7 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
         /// <summary>
         /// Fill in the GC-relevant stack frame locations.
         /// </summary>
-        private void FakeGcScanRoots(MethodDesc method, ArgIterator argit, CORCOMPILE_GCREFMAP_TOKENS[] frame)
+        private void FakeGcScanRoots(MethodDesc method, ArgIterator argit, CORCOMPILE_GCREFMAP_TOKENS[] frame, bool isUnboxingStub)
         {
             // Encode generic instantiation arg
             if (argit.HasParamType)
@@ -165,7 +165,6 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
             // If the function has a this pointer, add it to the mask
             if (argit.HasThis)
             {
-                bool isUnboxingStub = false; // TODO: is this correct?
                 bool interior = method.OwningType.IsValueType && !isUnboxingStub;
 
                 frame[_transitionBlock.ThisOffset] = (interior ? CORCOMPILE_GCREFMAP_TOKENS.GCREFMAP_INTERIOR : CORCOMPILE_GCREFMAP_TOKENS.GCREFMAP_REF);

diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GCRefMapNode.cs b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GCRefMapNode.cs
index 07dfd94..5a4147f 100644 (file)
--- a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GCRefMapNode.cs
+++ b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GCRefMapNode.cs
@@ -5,9 +5,7 @@
 using System;
 using System.Collections.Generic;
 using System.Diagnostics;
-
 using Internal.Text;
-using Internal.TypeSystem;
 
 namespace ILCompiler.DependencyAnalysis.ReadyToRun
 {
@@ -83,7 +81,7 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
             for (int methodIndex = 0; methodIndex < _methods.Count; methodIndex++)
             {
                 IMethodNode methodNode = _methods[methodIndex];
-                if (methodNode == null || (methodNode is LocalMethodImport localMethod && localMethod.MethodCodeNode.IsEmpty))
+                if (methodNode == null)
                 {
                     // Flush an empty GC ref map block to prevent
                     // the indexed records from falling out of sync with methods
@@ -91,7 +89,12 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
                 }
                 else
                 {
-                    builder.GetCallRefMap(methodNode.Method);
+                    bool isUnboxingStub = false;
+                    if (methodNode is DelayLoadHelperImport methodImport)
+                    {
+                        isUnboxingStub = ((MethodFixupSignature)methodImport.ImportSignature.Target).IsUnboxingStub;
+                    }
+                    builder.GetCallRefMap(methodNode.Method, isUnboxingStub);
                 }
                 if (methodIndex >= nextMethodIndex)
                 {

diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs
index d5f645a..10937f8 100644 (file)
--- a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs
+++ b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs
@@ -48,6 +48,8 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
 
         public override int ClassCode => 150063499;
 
+        public bool IsUnboxingStub => _isUnboxingStub;
+
         public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false)
         {
             if (relocsOnly)