Add back support for Delegate field marshaling (#34620)

* Block route trip of function pointer as Delegate field.
Add tests for scenario.

* Update MarshalStructAsParamDLL.cpp

diff --git a/src/coreclr/src/vm/comdelegate.cpp b/src/coreclr/src/vm/comdelegate.cpp
index 958a220..9964e30 100644 (file)
--- a/src/coreclr/src/vm/comdelegate.cpp
+++ b/src/coreclr/src/vm/comdelegate.cpp
@@ -1315,14 +1315,11 @@ OBJECTREF COMDelegate::ConvertToDelegate(LPVOID pCallback, MethodTable* pMT)
         THROWS;
         GC_TRIGGERS;
         MODE_COOPERATIVE;
+        PRECONDITION(pCallback != NULL);
+        PRECONDITION(pMT != NULL);
     }
     CONTRACTL_END;
 
-    if (!pCallback)
-    {
-        return NULL;
-    }
-
     //////////////////////////////////////////////////////////////////////////////////////////////////////////
     // Check if this callback was originally a managed method passed out to unmanaged code.
     //
@@ -1358,6 +1355,9 @@ OBJECTREF COMDelegate::ConvertToDelegate(LPVOID pCallback, MethodTable* pMT)
         return pDelegate;
     }
 
+    // Validate the MethodTable is a delegate type
+    if (!pMT->IsDelegate())
+        COMPlusThrow(kArgumentException);
 
     //////////////////////////////////////////////////////////////////////////////////////////////////////////
     // This is an unmanaged callsite. We need to create a new delegate.

diff --git a/src/coreclr/src/vm/ilmarshalers.cpp b/src/coreclr/src/vm/ilmarshalers.cpp
index ec846aa..242d090 100644 (file)
--- a/src/coreclr/src/vm/ilmarshalers.cpp
+++ b/src/coreclr/src/vm/ilmarshalers.cpp
@@ -139,11 +139,17 @@ void ILDelegateMarshaler::EmitConvertContentsNativeToCLR(ILCodeStream* pslILEmit
     EmitLoadNativeValue(pslILEmit);
     pslILEmit->EmitLDTOKEN(pslILEmit->GetToken(m_pargs->m_pMT));
     pslILEmit->EmitCALL(METHOD__TYPE__GET_TYPE_FROM_HANDLE, 1, 1); // Type System.Type.GetTypeFromHandle(RuntimeTypeHandle handle)
-    pslILEmit->EmitCALL(METHOD__MARSHAL__GET_DELEGATE_FOR_FUNCTION_POINTER, 2, 1); // Delegate System.Marshal.GetDelegateForFunctionPointer(IntPtr p, Type t)
-    EmitStoreManagedValue(pslILEmit);
 
+    // COMPAT: There is a subtle difference between argument and field marshaling with Delegate types.
+    // During field marshaling, the plain Delegate type can be used even though that type doesn't
+    // represent a concrete type. Argument marshaling doesn't permit this so we use the public
+    // API which will validate that Delegate and MulticastDelegate aren't directly used.
     if (IsFieldMarshal(m_dwMarshalFlags))
     {
+        // Delegate System.Marshal.GetDelegateForFunctionPointerInternal(IntPtr p, Type t)
+        pslILEmit->EmitCALL(METHOD__MARSHAL__GET_DELEGATE_FOR_FUNCTION_POINTER_INTERNAL, 2, 1);
+        EmitStoreManagedValue(pslILEmit);
+
         // Field marshalling of delegates supports marshalling back null from native code.
         // Parameter and return value marshalling does not.
         ILCodeLabel* pFinishedLabel = pslILEmit->NewCodeLabel();
@@ -155,6 +161,9 @@ void ILDelegateMarshaler::EmitConvertContentsNativeToCLR(ILCodeStream* pslILEmit
     }
     else
     {
+        // Delegate System.Marshal.GetDelegateForFunctionPointer(IntPtr p, Type t)
+        pslILEmit->EmitCALL(METHOD__MARSHAL__GET_DELEGATE_FOR_FUNCTION_POINTER, 2, 1);
+        EmitStoreManagedValue(pslILEmit);
         pslILEmit->EmitLabel(pNullLabel);
     }
 

diff --git a/src/coreclr/src/vm/mscorlib.h b/src/coreclr/src/vm/mscorlib.h
index 12ec6d8..d2057ff 100644 (file)
--- a/src/coreclr/src/vm/mscorlib.h
+++ b/src/coreclr/src/vm/mscorlib.h
@@ -497,8 +497,10 @@ DEFINE_CLASS(MARSHAL,               Interop,                Marshal)
 #ifdef FEATURE_COMINTEROP
 DEFINE_METHOD(MARSHAL,              GET_HR_FOR_EXCEPTION,              GetHRForException,             SM_Exception_RetInt)
 #endif // FEATURE_COMINTEROP
-DEFINE_METHOD(MARSHAL,              GET_FUNCTION_POINTER_FOR_DELEGATE, GetFunctionPointerForDelegate, SM_Delegate_RetIntPtr)
-DEFINE_METHOD(MARSHAL,              GET_DELEGATE_FOR_FUNCTION_POINTER, GetDelegateForFunctionPointer, SM_IntPtr_Type_RetDelegate)
+DEFINE_METHOD(MARSHAL,              GET_FUNCTION_POINTER_FOR_DELEGATE,          GetFunctionPointerForDelegate,          SM_Delegate_RetIntPtr)
+DEFINE_METHOD(MARSHAL,              GET_DELEGATE_FOR_FUNCTION_POINTER,          GetDelegateForFunctionPointer,          SM_IntPtr_Type_RetDelegate)
+DEFINE_METHOD(MARSHAL,              GET_DELEGATE_FOR_FUNCTION_POINTER_INTERNAL, GetDelegateForFunctionPointerInternal,  SM_IntPtr_Type_RetDelegate)
+
 DEFINE_METHOD(MARSHAL,              ALLOC_CO_TASK_MEM,                 AllocCoTaskMem,                SM_Int_RetIntPtr)
 DEFINE_METHOD(MARSHAL,              FREE_CO_TASK_MEM,                  FreeCoTaskMem,                 SM_IntPtr_RetVoid)
 DEFINE_FIELD(MARSHAL,               SYSTEM_MAX_DBCS_CHAR_SIZE,         SystemMaxDBCSCharSize)

diff --git a/src/coreclr/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsLayoutSeq.cs b/src/coreclr/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsLayoutSeq.cs
index c917b0a..b096b13 100644 (file)
--- a/src/coreclr/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsLayoutSeq.cs
+++ b/src/coreclr/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsLayoutSeq.cs
@@ -65,7 +65,8 @@ public class Managed
         RunMarshalSeqStructAsParamByValInOut();
         RunMarshalSeqStructAsParamByRefInOut();
         RunMarshalSeqStructAsReturn();
-        
+        RunMarshalSeqStructDelegateField();
+
         if (failures > 0)
         {
             Console.WriteLine("\nTEST FAILED!");
@@ -347,6 +348,12 @@ public class Managed
     [DllImport("MarshalStructAsParam")]
     static extern MultipleBool GetBools(bool b1, bool b2);
 
+    [DllImport("MarshalStructAsParam")]
+    static extern IntPtr GetNativeIntIntFunction();
+
+    [DllImport("MarshalStructAsParam")]
+    static extern void MarshalStructAsParam_DelegateFieldMarshaling(ref DelegateFieldMarshaling dfm);
+
     #region Marshal struct method in PInvoke
     [SecuritySafeCritical]
     unsafe private static void MarshalStructAsParam_AsSeqByVal(StructID id)
@@ -2555,7 +2562,53 @@ public class Managed
             Console.WriteLine("Structure of two bools marshalled to BOOLs returned from native to managed failed");
         }
     }
-}
 
+    private static void RunMarshalSeqStructDelegateField()
+    {
+        Console.WriteLine($"\nRunning {nameof(Managed.RunMarshalSeqStructDelegateField)}...");
+
+        var dfm = new DelegateFieldMarshaling();
+        try
+        {
+            MarshalStructAsParam_DelegateFieldMarshaling(ref dfm);
+        }
+        catch
+        {
+            Console.WriteLine($"Marshaling null function pointer as ${nameof(Delegate)} field should succeed");
+            failures++;
+        }
 
-   
+        dfm.IntIntFunction = new IntIntDelegate(IntIntImpl);
+        try
+        {
+            MarshalStructAsParam_DelegateFieldMarshaling(ref dfm);
+        }
+        catch
+        {
+            Console.WriteLine($"Marshaling function pointer as ${nameof(Delegate)} field should succeed");
+            failures++;
+        }
+
+        dfm.IntIntFunction = Marshal.GetDelegateForFunctionPointer<IntIntDelegate>(GetNativeIntIntFunction());
+        try
+        {
+            MarshalStructAsParam_DelegateFieldMarshaling(ref dfm);
+            Console.WriteLine("Marshaling native function pointer should fail");
+            failures++;
+        }
+        catch (ArgumentException)
+        {
+            // Should be ArgumentException for native function pointer round trip.
+        }
+        catch
+        {
+            Console.WriteLine($"Marshaling native function pointer should throw {nameof(ArgumentException)}");
+            failures++;
+        }
+
+        int IntIntImpl(int a)
+        {
+            return a;
+        }
+    }
+}

diff --git a/src/coreclr/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsParamDLL.cpp b/src/coreclr/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsParamDLL.cpp
index 45e9020..74ab359 100644 (file)
--- a/src/coreclr/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsParamDLL.cpp
+++ b/src/coreclr/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsParamDLL.cpp
@@ -1262,3 +1262,28 @@ extern "C" DLL_EXPORT MultipleBools STDMETHODCALLTYPE GetBools(BOOL b1, BOOL b2)
 {
     return {b1, b2};
 }
+
+using IntIntDelegate = int (STDMETHODCALLTYPE*)(int a);
+
+struct DelegateFieldMarshaling
+{
+    IntIntDelegate IntIntFunction;
+};
+
+namespace
+{
+    int STDMETHODCALLTYPE IntIntImpl(int a)
+    {
+        return a;
+    }
+}
+
+extern "C" DLL_EXPORT void* STDMETHODCALLTYPE GetNativeIntIntFunction()
+{
+    return (void*)&IntIntImpl;
+}
+
+extern "C" DLL_EXPORT void STDMETHODCALLTYPE MarshalStructAsParam_DelegateFieldMarshaling(DelegateFieldMarshaling* d)
+{
+    // Nothing to do
+}

diff --git a/src/coreclr/tests/src/Interop/StructMarshalling/PInvoke/Struct.cs b/src/coreclr/tests/src/Interop/StructMarshalling/PInvoke/Struct.cs
index 750548a..2932045 100644 (file)
--- a/src/coreclr/tests/src/Interop/StructMarshalling/PInvoke/Struct.cs
+++ b/src/coreclr/tests/src/Interop/StructMarshalling/PInvoke/Struct.cs
@@ -495,3 +495,11 @@ public struct UnicodeCharArrayClassification
     public char[] arr;
     public float f;
 }
+
+delegate int IntIntDelegate(int a);
+
+[StructLayout(LayoutKind.Sequential)]
+public struct DelegateFieldMarshaling
+{
+    public Delegate IntIntFunction;
+}