Fix verification of open delegates (#65425)

Fixes #65010

src/coreclr/tools/ILVerification/ILImporter.StackValue.cs

@@ -77,7 +77,7 @@ public bool IsMethod
 
         public bool IsBoxedValueType
         {
-            get { return Kind == StackValueKind.ObjRef && Type.IsValueType; }
+            get { return Kind == StackValueKind.ObjRef && Type != null && Type.IsValueType; }
         }
 
         public StackValue DereferenceByRef()

src/coreclr/tools/ILVerification/ILImporter.Verify.cs

@@ -1119,37 +1119,108 @@ void CheckDelegateCreation(StackValue ftn, StackValue obj)
                 VerificationError(VerifierError.DelegatePattern);
         }
 
-        void CheckIsDelegateAssignable(MethodDesc ftn, TypeDesc delegateType)
-        {
-            if (!IsDelegateAssignable(ftn, delegateType))
-                VerificationError(VerifierError.DelegateCtor);
-        }
-
-        bool IsDelegateAssignable(MethodDesc ftn, TypeDesc delegateType)
+        bool IsDelegateAssignable(MethodDesc targetMethod, TypeDesc delegateType, TypeDesc firstArg)
         {
             var invokeMethod = delegateType.GetMethod("Invoke", null);
             if (invokeMethod == null)
                 return false;
 
-            var ftnSignature = ftn.Signature;
-            var delegateSignature = invokeMethod.Signature;
+            var targetSignature = targetMethod.Signature;
+            var invokeSignature = invokeMethod.Signature;
 
             // Compare calling convention ignoring distinction between static and instance
-            if ((ftnSignature.Flags & ~MethodSignatureFlags.Static) != (delegateSignature.Flags & ~MethodSignatureFlags.Static))
+            if ((targetSignature.Flags & ~MethodSignatureFlags.Static) != (invokeSignature.Flags & ~MethodSignatureFlags.Static))
+                return false;
+
+            // Compare return type
+            if (!IsAssignable(targetSignature.ReturnType, invokeSignature.ReturnType))
                 return false;
 
+            int totalTargetArgs = targetSignature.Length + (targetSignature.IsStatic ? 0 : 1);
+
             // Compare signature parameters
-            if (ftnSignature.Length != delegateSignature.Length)
+
+            bool isOpenDelegate;
+            if (totalTargetArgs == invokeSignature.Length)
+            {
+                // All arguments provided by invoke, delegate must be open.
+                isOpenDelegate = true;
+            }
+            else if (totalTargetArgs == invokeSignature.Length + 1)
+            {
+                // One too few arguments provided by invoke, delegate must be closed.
+                isOpenDelegate = false;
+            }
+            else
+            {
                 return false;
+            }
 
-            for (int i = 0; i < ftnSignature.Length; i++)
+            // An open static delegate which takes no arguments. In that case we're done.
+            if (totalTargetArgs == 0)
             {
-                if (!IsAssignable(delegateSignature[i], ftnSignature[i]))
+                Debug.Assert(isOpenDelegate);
+                return true;
+            }
+
+            int consumedArgs = 0;
+
+            TypeDesc firstInvokeArg;
+            if (isOpenDelegate)
+            {
+                // If we're looking at an open delegate but the caller has provided a target it's not a match.
+                if (firstArg != null)
                     return false;
+
+                firstInvokeArg = invokeSignature[0];
+                consumedArgs++;
             }
+            else
+            {
+                // If we're looking at a closed delegate but the caller has not provided a target it's not a match.
+                if (firstArg == null)
+                    return false;
 
-            // Compare return type
-            return IsAssignable(ftnSignature.ReturnType, delegateSignature.ReturnType);
+                firstInvokeArg = firstArg;
+            }
+
+            TypeDesc firstTargetArg;
+            if (targetSignature.IsStatic)
+            {
+                // Checked above
+                Debug.Assert(targetSignature.Length != 0);
+
+                // The first argument for a static method is the first fixed arg.
+                firstTargetArg = targetSignature[0];
+                consumedArgs--;
+            }
+            else
+            {
+                // The type of the first argument to an instance method is from the method type.
+                firstTargetArg = targetMethod.OwningType;
+
+                // If the delegate is open and the target method is on a value type or primitive then the first argument of the invoke
+                // method must be a reference to that type. So make the type we got from the reference to a ref. (We don't need to
+                // do this for the closed instance case because there we got the invocation side type from the first arg passed in, i.e.
+                // ref was stripped from it implicitly).
+                if (isOpenDelegate && firstTargetArg.IsValueType)
+                    firstTargetArg = firstTargetArg.MakeByRefType();
+            }
+
+            if (!IsAssignable(firstInvokeArg, firstTargetArg))
+                return false;
+
+            // We better have same number of remaining args
+            if (invokeSignature.Length - consumedArgs != targetSignature.Length)
+                return false;
+
+            for (int i = isOpenDelegate ? 1 : 0; i < invokeSignature.Length; i++)
+            {
+                if (!IsAssignable(invokeSignature[i], targetSignature[i - consumedArgs]))
+                    return false;
+            }
+
+            return true;
         }
 
         ILOpcode GetOpcodeAt(int instructionOffset)
@@ -1525,7 +1596,8 @@ void ImportCall(ILOpcode opcode, int token)
 
                 CheckDelegateCreation(actualFtn, actualObj);
 
-                CheckIsDelegateAssignable(actualFtn.Method, methodType);
+                if (!IsDelegateAssignable(actualFtn.Method, methodType, actualObj.Type))
+                    VerificationError(VerifierError.DelegateCtor);
             }
             else
             {

src/tests/ilverify/ILTests/FtnTests.il

@@ -93,6 +93,11 @@
         ret
     }
 
+    .method public hidebysig static void StaticMethodString(string param) cil managed 
+    {
+        ret
+    }
+
     .method public hidebysig static int32 StaticIntMethod() cil managed
     {
         ldc.i4.0
@@ -167,6 +172,18 @@
     .field public static literal valuetype ByteEnum B = uint8(0)
 }
 
+.class private auto ansi sealed RefIntDelegate
+       extends [System.Runtime]System.MulticastDelegate
+{
+    .method public hidebysig specialname rtspecialname instance void  .ctor(object 'object', native int 'method') runtime managed
+    {
+    }
+
+    .method public hidebysig newslot virtual instance int32  Invoke(int32& x) runtime managed
+    {
+    }
+}
+
 .class public auto ansi beforefieldinit FtnTestsType
        extends [System.Runtime]System.Object
 {
@@ -264,6 +281,54 @@
         ret
     }
 
+    .method static public hidebysig void LdvirtFtn.InstanceMethodClosed_Valid() cil managed
+    {
+        // static void StaticMethodString(this string x)
+        // var f = new Action("xyz".StaticMethodString);
+        // f()
+
+        ldstr       "xyz"
+        ldftn       void TestClass::StaticMethodString(string)
+        newobj      instance void [System.Runtime]System.Action::.ctor(object, native int)
+        callvirt    instance void [System.Runtime]System.Action::Invoke()
+        ret
+    }
+
+    .method static public hidebysig void LdvirtFtn.InstanceMethodOpen_Valid() cil managed
+    {
+        ldnull
+        ldftn       instance class [System.Runtime]System.Type [System.Runtime]System.Object::GetType()
+        newobj      instance void class [System.Runtime]System.Func`2<object, class [System.Runtime]System.Type>::.ctor(object, native int)
+        ldstr       "obj"
+        callvirt    instance !1 class [System.Runtime]System.Func`2<object, class [System.Runtime]System.Type>::Invoke(!0)
+        pop
+        ret
+    }
+
+    .method static public hidebysig void LdvirtFtn.InstanceValueTypeMethodOpen_Valid() cil managed
+    {
+        .locals init (int32 V_0)
+
+        ldnull
+        ldftn       instance int32 [System.Runtime]System.Int32::GetHashCode()
+        newobj      instance void RefIntDelegate::.ctor(object, native int)
+        ldloca.s    V_0
+        callvirt    instance int32 RefIntDelegate::Invoke(int32&)
+        pop
+        ret
+    }
+
+    .method static public hidebysig void LdvirtFtn.InstanceValueTypeMethodOpenObjRef_Invalid_DelegateCtor() cil managed
+    {
+        ldnull
+        ldftn       instance int32 [System.Runtime]System.Int32::GetHashCode()
+        newobj      instance void class [System.Runtime]System.Func`2<int32, int32>::.ctor(object, native int)
+        ldc.i4.0
+        callvirt    instance !1 class [System.Runtime]System.Func`2<int32, int32>::Invoke(!0)
+        pop
+        ret
+    }
+
     .method static public hidebysig void LdvirtFtn.StaticMethod_Invalid_LdvirtftnOnStatic() cil managed
     {
         ldnull
@@ -274,7 +339,7 @@
         ret
     }
 
-    .method static public hidebysig void LdvirtFtn.ValueTypeNoBox_Invalid_StackObjRef.StackUnexpected.StackUnexpected.DelegateCtorSigO(int32 i) cil managed
+    .method static public hidebysig void LdvirtFtn.ValueTypeNoBox_Invalid_StackObjRef.StackUnexpected.StackUnexpected.DelegateCtorSigO.DelegateCtor(int32 i) cil managed
     {
         // (int i)
         // var f = new System.Func<int, int>(i.CompareTo);
@@ -290,7 +355,7 @@
         ret
     }
 
-    .method static public hidebysig void LdvirtFtn.ValueTypeWrongBox_Invalid_StackUnexpected.DelegateCtorSigO.DelegatePattern(int32 i) cil managed
+    .method static public hidebysig void LdvirtFtn.ValueTypeWrongBox_Invalid_StackUnexpected.DelegateCtorSigO.DelegatePattern.DelegateCtor(int32 i) cil managed
     {
         // (int i)
         // var f = new System.Func<int, int>(i.CompareTo);
@@ -307,7 +372,7 @@
         ret
     }
 
-    .method static public hidebysig void LdvirtFtn.ObjectForTestClassInstance_Invalid_StackUnexpected(object c) cil managed
+    .method static public hidebysig void LdvirtFtn.ObjectForTestClassInstance_Invalid_StackUnexpected.DelegateCtor(object c) cil managed
     {
         // (object c)
         // var a = new System.Action(c.InstanceMethod);