Fix GCRefMap for generic method calls via Unboxing stub (dotnet#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.
Rheoz · Jan 5, 2020 · 0025f37 · 0025f37
1 parent a751093
commit 0025f37
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 9 deletions.
diff --git a/...crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GCRefMapBuilder.cs b/...crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GCRefMapBuilder.cs
@@ -66,7 +66,7 @@ public GCRefMapBuilder(TargetDetails target, bool relocsOnly)
             _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 @@ public void GetCallRefMap(MethodDesc method)
                 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 @@ public void GetCallRefMap(MethodDesc method)
             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 @@ public void GetCallRefMap(MethodDesc method)
         /// <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 @@ private void FakeGcScanRoots(MethodDesc method, ArgIterator argit, CORCOMPILE_GC
             // 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/...ls/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GCRefMapNode.cs b/...ls/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,15 +81,20 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false)
             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
                     builder.Flush();
                 }
                 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/...gen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs b/...gen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs
@@ -48,6 +48,8 @@ public MethodFixupSignature(
 
         public override int ClassCode => 150063499;
 
+        public bool IsUnboxingStub => _isUnboxingStub;
+
         public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false)
         {
             if (relocsOnly)