Another attempt at fixing #99198 (#111255)

The problem was always `// Note: We don't set the IsUnboxingStub flag on template methods (all template lookups performed at runtime are performed with this flag not set`, I tried working around it in the original fix, but looks like I actually need the function pointer in #111178 (the tests are not failing but I have a local test that does).

So trying an alternative approach that just deletes the weird code. It's possible this only had to be weird due to universal shared code.

Author: MichalStrehovsky

src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeBuilder.cs

@@ -296,11 +296,11 @@ internal void ParseNativeLayoutInfo(InstantiatedMethod method)
 
             InstantiatedMethod nonTemplateMethod = method;
 
-            // Templates are always non-unboxing stubs
-            if (method.UnboxingStub)
+            // Templates are always unboxing stubs for valuetype instance methods
+            if (!method.UnboxingStub && method.OwningType.IsValueType && !TypeLoaderEnvironment.IsStaticMethodSignature(method.NameAndSignature))
             {
-                // Strip unboxing stub, note the first parameter which is false
-                nonTemplateMethod = (InstantiatedMethod)method.Context.ResolveGenericMethodInstantiation(false, (DefType)method.OwningType, method.NameAndSignature, method.Instantiation);
+                // Make it an unboxing stub, note the first parameter which is true
+                nonTemplateMethod = (InstantiatedMethod)method.Context.ResolveGenericMethodInstantiation(true, (DefType)method.OwningType, method.NameAndSignature, method.Instantiation);
             }
 
             uint nativeLayoutInfoToken;
@@ -311,11 +311,15 @@ internal void ParseNativeLayoutInfo(InstantiatedMethod method)
                 throw new MissingTemplateException();
             }
 
-            // We might have a mismatch between unboxing/non-unboxing variants so only remember it for static methods
-            if (TypeLoaderEnvironment.IsStaticMethodSignature(templateMethod.NameAndSignature)
-                && templateMethod.FunctionPointer != IntPtr.Zero)
+            if (templateMethod.FunctionPointer != IntPtr.Zero)
             {
                 nonTemplateMethod.SetFunctionPointer(templateMethod.FunctionPointer);
+
+                // Compensate for the template being an unboxing stub
+                if (nonTemplateMethod != method)
+                {
+                    method.SetFunctionPointer(TypeLoaderEnvironment.ConvertUnboxingFunctionPointerToUnderlyingNonUnboxingPointer(templateMethod.FunctionPointer, templateMethod.OwningType.RuntimeTypeHandle));
+                }
             }
 
             // Ensure that if this method is non-shareable from a normal canonical perspective, then

src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeLoaderEnvironment.ConstructedGenericMethodsLookup.cs

@@ -258,8 +258,17 @@ public bool TryGetGenericVirtualMethodPointer(InstantiatedMethod method, out Int
                 }
             }
 
+            InstantiatedMethod nonTemplateMethod = method;
+
+            // Templates are always unboxing stubs for valuetype instance methods
+            if (!method.UnboxingStub && method.OwningType.IsValueType && !IsStaticMethodSignature(method.NameAndSignature))
+            {
+                // Make it an unboxing stub, note the first parameter which is true
+                nonTemplateMethod = (InstantiatedMethod)method.Context.ResolveGenericMethodInstantiation(true, (DefType)method.OwningType, method.NameAndSignature, method.Instantiation);
+            }
+
             // If we cannot find an exact method entry point, look for an equivalent template and compute the generic dictionary
-            InstantiatedMethod templateMethod = TemplateLocator.TryGetGenericMethodTemplate(method, out _, out _);
+            InstantiatedMethod templateMethod = TemplateLocator.TryGetGenericMethodTemplate(nonTemplateMethod, out _, out _);
             if (templateMethod == null)
             {
                 methodPointer = default;

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NativeLayoutVertexNode.cs

@@ -107,6 +107,8 @@ public enum MethodEntryFlags
 
         public MethodDesc Method => _method;
 
+        public virtual bool IsUnboxingStub => _method.OwningType.IsValueType && !_method.Signature.IsStatic;
+
         public NativeLayoutMethodEntryVertexNode(NodeFactory factory, MethodDesc method, MethodEntryFlags flags)
         {
             _method = method;
@@ -148,7 +150,7 @@ public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFacto
 
             if ((_flags & MethodEntryFlags.SaveEntryPoint) != 0)
             {
-                IMethodNode methodEntryPointNode = GetMethodEntrypointNode(context, out _);
+                IMethodNode methodEntryPointNode = GetMethodEntrypointNode(context);
                 dependencies.Add(new DependencyListEntry(methodEntryPointNode, "NativeLayoutMethodEntryVertexNode entrypoint"));
             }
 
@@ -187,17 +189,17 @@ public override Vertex WriteVertex(NodeFactory factory)
                 }
             }
 
+            if (IsUnboxingStub)
+                flags |= MethodFlags.IsUnboxingStub;
+
             uint fptrReferenceId = 0;
             if ((_flags & MethodEntryFlags.SaveEntryPoint) != 0)
             {
                 flags |= MethodFlags.HasFunctionPointer;
 
-                bool unboxingStub;
-                IMethodNode methodEntryPointNode = GetMethodEntrypointNode(factory, out unboxingStub);
+                IMethodNode methodEntryPointNode = GetMethodEntrypointNode(factory);
                 fptrReferenceId = factory.MetadataManager.NativeLayoutInfo.ExternalReferences.GetIndex(methodEntryPointNode);
 
-                if (unboxingStub)
-                    flags |= MethodFlags.IsUnboxingStub;
                 if (methodEntryPointNode.Method.IsCanonicalMethod(CanonicalFormKind.Universal))
                     flags |= MethodFlags.FunctionPointerIsUSG;
             }
@@ -219,10 +221,9 @@ private Vertex GetContainingTypeVertex(NodeFactory factory)
             }
         }
 
-        protected virtual IMethodNode GetMethodEntrypointNode(NodeFactory factory, out bool unboxingStub)
+        protected virtual IMethodNode GetMethodEntrypointNode(NodeFactory factory)
         {
-            unboxingStub = _method.OwningType.IsValueType && !_method.Signature.IsStatic;
-            IMethodNode methodEntryPointNode = factory.MethodEntrypoint(_method, unboxingStub);
+            IMethodNode methodEntryPointNode = factory.MethodEntrypoint(_method, IsUnboxingStub);
             return methodEntryPointNode;
         }
     }
@@ -748,15 +749,11 @@ public override Vertex WriteVertex(NodeFactory factory)
             return SetSavedVertex(factory.MetadataManager.NativeLayoutInfo.TemplatesSection.Place(methodEntryVertex));
         }
 
-        protected override IMethodNode GetMethodEntrypointNode(NodeFactory factory, out bool unboxingStub)
+        protected override IMethodNode GetMethodEntrypointNode(NodeFactory factory)
         {
             Debug.Assert(NeedsEntrypoint(_method));
-            unboxingStub = _method.OwningType.IsValueType && !_method.Signature.IsStatic;
             // TODO-SIZE: this is only address taken if it's a target of a delegate
-            IMethodNode methodEntryPointNode = factory.AddressTakenMethodEntrypoint(_method, unboxingStub);
-            // Note: We don't set the IsUnboxingStub flag on template methods (all template lookups performed at runtime are performed with this flag not set,
-            // since it can't always be conveniently computed for a concrete method before looking up its template)
-            unboxingStub = false;
+            IMethodNode methodEntryPointNode = factory.AddressTakenMethodEntrypoint(_method, IsUnboxingStub);
             return methodEntryPointNode;
         }
 
@@ -1677,7 +1674,7 @@ public WrappedMethodDictionaryVertexNode(NodeFactory factory, MethodDesc method)
             {
             }
 
-            protected override IMethodNode GetMethodEntrypointNode(NodeFactory factory, out bool unboxingStub)
+            protected override IMethodNode GetMethodEntrypointNode(NodeFactory factory)
             {
                 throw new NotSupportedException();
             }
@@ -1903,16 +1900,17 @@ private sealed class WrappedMethodEntryVertexNode : NativeLayoutMethodEntryVerte
             public bool _unboxingStub;
             public IMethodNode _functionPointerNode;
 
+            public override bool IsUnboxingStub => _unboxingStub;
+
             public WrappedMethodEntryVertexNode(NodeFactory factory, MethodDesc method, bool unboxingStub, IMethodNode functionPointerNode) :
                 base(factory, method, functionPointerNode != null ? MethodEntryFlags.SaveEntryPoint : default(MethodEntryFlags))
             {
                 _unboxingStub = unboxingStub;
                 _functionPointerNode = functionPointerNode;
             }
 
-            protected override IMethodNode GetMethodEntrypointNode(NodeFactory factory, out bool unboxingStub)
+            protected override IMethodNode GetMethodEntrypointNode(NodeFactory factory)
             {
-                unboxingStub = _unboxingStub;
                 return _functionPointerNode;
             }