Check for generic structs in "needs marshalling" check (#90836)

Co-authored-by: Aaron Robinson <[email protected]>
Co-authored-by: Jeremy Koritzinsky <[email protected]>

src/coreclr/vm/dllimport.cpp

@@ -3316,8 +3316,12 @@ BOOL NDirect::MarshalingRequired(
                 FALLTHROUGH;
 
             case ELEMENT_TYPE_VALUETYPE:
+            case ELEMENT_TYPE_GENERICINST:
             {
                 TypeHandle hndArgType = arg.GetTypeHandleThrowing(pModule, &emptyTypeContext);
+                bool isValidGeneric = IsValidForGenericMarshalling(hndArgType.GetMethodTable(), false, runtimeMarshallingEnabled);
+                if(!hndArgType.IsValueType() ||  !isValidGeneric)
+                    return true;
 
                 if (hndArgType.GetMethodTable()->IsInt128OrHasInt128Fields())
                 {

src/coreclr/vm/mlinfo.cpp

@@ -1067,43 +1067,43 @@ OleColorMarshalingInfo *EEMarshalingData::GetOleColorMarshalingInfo()
 }
 #endif // FEATURE_COMINTEROP
 
-namespace
+bool IsValidForGenericMarshalling(MethodTable* pMT, bool isFieldScenario, bool builtInMarshallingEnabled)
 {
-    bool IsValidForGenericMarshalling(MethodTable* pMT, bool isFieldScenario, bool builtInMarshallingEnabled = true)
-    {
-        _ASSERTE(pMT != NULL);
+    _ASSERTE(pMT != NULL);
 
-        // Not generic, so passes "generic" test
-        if (!pMT->HasInstantiation())
-            return true;
+    // Not generic, so passes "generic" test
+    if (!pMT->HasInstantiation())
+        return true;
 
-        // We can't block generic types for field scenarios for back-compat reasons.
-        if (isFieldScenario)
-            return true;
+    // We can't block generic types for field scenarios for back-compat reasons.
+    if (isFieldScenario)
+        return true;
 
-        // Built-in marshalling considers the blittability for a generic type.
-        if (builtInMarshallingEnabled && !pMT->IsBlittable())
-            return false;
-
-        // Generics (blittable when built-in is enabled) are allowed to be marshalled with the following exceptions:
-        // * Nullable<T>: We don't want to be locked into the default behavior as we may want special handling later
-        // * Span<T>: Not supported by built-in marshalling
-        // * ReadOnlySpan<T>: Not supported by built-in marshalling
-        // * Vector64<T>: Represents the __m64 ABI primitive which requires currently unimplemented handling
-        // * Vector128<T>: Represents the __m128 ABI primitive which requires currently unimplemented handling
-        // * Vector256<T>: Represents the __m256 ABI primitive which requires currently unimplemented handling
-        // * Vector512<T>: Represents the __m512 ABI primitive which requires currently unimplemented handling
-        // * Vector<T>: Has a variable size (either __m128 or __m256) and isn't readily usable for interop scenarios
-        return !pMT->HasSameTypeDefAs(g_pNullableClass)
-            && !pMT->HasSameTypeDefAs(CoreLibBinder::GetClass(CLASS__SPAN))
-            && !pMT->HasSameTypeDefAs(CoreLibBinder::GetClass(CLASS__READONLY_SPAN))
-            && !pMT->HasSameTypeDefAs(CoreLibBinder::GetClass(CLASS__VECTOR64T))
-            && !pMT->HasSameTypeDefAs(CoreLibBinder::GetClass(CLASS__VECTOR128T))
-            && !pMT->HasSameTypeDefAs(CoreLibBinder::GetClass(CLASS__VECTOR256T))
-            && !pMT->HasSameTypeDefAs(CoreLibBinder::GetClass(CLASS__VECTOR512T))
-            && !pMT->HasSameTypeDefAs(CoreLibBinder::GetClass(CLASS__VECTORT));
-    }
+    // Built-in marshalling considers the blittability for a generic type.
+    if (builtInMarshallingEnabled && !pMT->IsBlittable())
+        return false;
+
+    // Generics (blittable when built-in is enabled) are allowed to be marshalled with the following exceptions:
+    // * Nullable<T>: We don't want to be locked into the default behavior as we may want special handling later
+    // * Span<T>: Not supported by built-in marshalling
+    // * ReadOnlySpan<T>: Not supported by built-in marshalling
+    // * Vector64<T>: Represents the __m64 ABI primitive which requires currently unimplemented handling
+    // * Vector128<T>: Represents the __m128 ABI primitive which requires currently unimplemented handling
+    // * Vector256<T>: Represents the __m256 ABI primitive which requires currently unimplemented handling
+    // * Vector512<T>: Represents the __m512 ABI primitive which requires currently unimplemented handling
+    // * Vector<T>: Has a variable size (either __m128 or __m256) and isn't readily usable for interop scenarios
+    return !pMT->HasSameTypeDefAs(g_pNullableClass)
+        && !pMT->HasSameTypeDefAs(CoreLibBinder::GetClass(CLASS__SPAN))
+        && !pMT->HasSameTypeDefAs(CoreLibBinder::GetClass(CLASS__READONLY_SPAN))
+        && !pMT->HasSameTypeDefAs(CoreLibBinder::GetClass(CLASS__VECTOR64T))
+        && !pMT->HasSameTypeDefAs(CoreLibBinder::GetClass(CLASS__VECTOR128T))
+        && !pMT->HasSameTypeDefAs(CoreLibBinder::GetClass(CLASS__VECTOR256T))
+        && !pMT->HasSameTypeDefAs(CoreLibBinder::GetClass(CLASS__VECTOR512T))
+        && !pMT->HasSameTypeDefAs(CoreLibBinder::GetClass(CLASS__VECTORT));
+}
 
+namespace
+{
     MarshalInfo::MarshalType GetDisabledMarshallerType(
         Module* pModule,
         SigPointer sig,

src/coreclr/vm/mlinfo.h

@@ -287,6 +287,8 @@ class EEMarshalingData
 
 struct ItfMarshalInfo;
 
+bool IsValidForGenericMarshalling(MethodTable* pMT, bool isFieldScenario, bool builtInMarshallingEnabled = true);
+
 class MarshalInfo
 {
 public:

src/tests/Interop/UnmanagedCallersOnly/InvalidCallbacks.il

@@ -240,4 +240,68 @@
         call instance void [System.Runtime]System.Object::.ctor()
         ret
     }
-}
+}
+
+.class public sequential ansi sealed beforefieldinit InvalidCSharp.MaybeBlittable`1<T>
+	extends [System.Runtime]System.ValueType
+{
+	// Fields
+	.field private !T Value
+}
+
+.class public auto ansi sealed beforefieldinit InvalidCSharp.NotBlittable`1<T>
+	extends [System.Runtime]System.Object
+{
+	// Fields
+	.field private !T Value
+    // Methods
+    .method public hidebysig specialname rtspecialname
+        instance void .ctor () cil managed
+    {
+        .maxstack 8
+        IL_0000: ldarg.0
+        IL_0001: call instance void [System.Runtime]System.Object::.ctor()
+        IL_0006: ret
+    }
+}
+
+.class public auto ansi beforefieldinit InvalidCSharp.InvalidGenericUnmanagedCallersOnlyParameters
+    extends [System.Runtime]System.Object
+{
+    .method public hidebysig specialname rtspecialname
+        instance void .ctor () cil managed
+    {
+        .maxstack 8
+        ldarg.0
+        call instance void [System.Runtime]System.Object::.ctor()
+        ret
+    }
+
+    .method public hidebysig static
+        int32 GenericClass (
+            class InvalidCSharp.NotBlittable`1<int32> param
+        ) cil managed
+    {
+        .custom instance void [System.Runtime.InteropServices]System.Runtime.InteropServices.UnmanagedCallersOnlyAttribute::.ctor() = (
+            01 00 00 00
+        )
+        .maxstack 8
+
+        IL_0000: ldc.i4.0
+        IL_0001: ret
+    }
+
+    .method public hidebysig static
+        int32 GenericStructWithObjectField (
+            valuetype InvalidCSharp.MaybeBlittable`1<object> param
+        ) cil managed
+    {
+        .custom instance void [System.Runtime.InteropServices]System.Runtime.InteropServices.UnmanagedCallersOnlyAttribute::.ctor() = (
+            01 00 00 00
+        )
+        .maxstack 8
+
+        IL_0000: ldc.i4.0
+        IL_0001: ret
+    }
+}

src/tests/Interop/UnmanagedCallersOnly/UnmanagedCallersOnlyTest.cs

@@ -37,6 +37,7 @@ public static int Main(string[] args)
             NegativeTest_FromInstantiatedGenericClass();
             TestUnmanagedCallersOnlyViaUnmanagedCalli();
             TestPInvokeMarkedWithUnmanagedCallersOnly();
+            TestUnmanagedCallersOnlyWithGeneric();
 
             // Exception handling is only supported on CoreCLR Windows.
             if (TestLibrary.Utilities.IsWindows && !TestLibrary.Utilities.IsMonoRuntime)
@@ -217,4 +218,30 @@ public static void TestPInvokeMarkedWithUnmanagedCallersOnly()
         int n = 1234;
         Assert.Throws<NotSupportedException>(() => ((delegate* unmanaged<int, int>)&CallingUnmanagedCallersOnlyDirectly.PInvokeMarkedWithUnmanagedCallersOnly)(n));
     }
+
+    public static void TestUnmanagedCallersOnlyWithGeneric()
+    {
+        Assert.Equal(0, ((delegate* unmanaged<Blittable<nint>, int>)&BlittableGenericStruct)(new Blittable<nint>()));
+
+        Assert.Equal(0, ((delegate* unmanaged<MaybeBlittable<nint>, int>)&MaybeBlittableGenericStruct)(new MaybeBlittable<nint>()));
+
+
+        Assert.Throws<InvalidProgramException>(()
+            => ((delegate* unmanaged<nint, int>)(void*)(delegate* unmanaged<NotBlittable<int>, int>)&InvalidGenericUnmanagedCallersOnlyParameters.GenericClass)((nint)1));
+
+        Assert.Throws<InvalidProgramException>(()
+            => ((delegate* unmanaged<nint, int>)(void*)(delegate* unmanaged<MaybeBlittable<object>, int>)&InvalidGenericUnmanagedCallersOnlyParameters.GenericStructWithObjectField)((nint)1));
+    }
+
+    internal struct Blittable<T> where T : unmanaged
+    {
+        T Value;
+    }
+
+
+    [UnmanagedCallersOnly]
+    internal static int BlittableGenericStruct(Blittable<nint> param) => 0;
+
+    [UnmanagedCallersOnly]
+    internal static int MaybeBlittableGenericStruct(MaybeBlittable<nint> param) => 0;
 }