Allow [UnmanagedCallersOnly] on generic methods, when possible

[Sergio0694]

Background and Motivation

I recently discovered that [UnmanagedCallersOnly] isn't actually supported in generic methods (or methods within a generic type), even though this doesn't actually seem to be documented in the API docs. Reading from https://docs.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.unmanagedcallersonlyattribute (as well as the original blog post) I only see remarks about the feature being restricted to static methods, only called by native code, and with all params/returns being blittable. I'm not really seeing a limitation here about generic types, and I'm not actually sure why it's there (was that to simplify the implementation?). My question is: assuming there isn't some inherent technical limitation that makes it impossible to enable this, couldn't the restriction be lifted? I'm thinking the VM could just validate the signature at JIT time anyway, and then throw in case a T was used as a parameter, and then it was substituted by a non-blittable type. I could see at least the restriction being reduced so that all generic type parameters involved would have to be unmanaged (so that the VM would have to verify that they're blittable if used at the ABI boundary, but they'd at least be guaranteed not to be ref types, making the thing somewhat less error prone).

Consider this use case scenario:

unsafe struct IAction<T>
    where T : unmanaged
{
    // NOTE: simplified code just to illustrate the [UnmanagedCallersOnly] limitation.

    public void** lpVtbl;
    public GCHandle handle;
    public T state;

    [UnmanagedCallersOnly]
    public static void Invoke(Foo<T>* @this)
    {
        Unsafe.As<Action<T>>(@this->handle.Target)(@this->state);
    }
}

In this case the method signature is blittable and the other requirements are respected as well, shouldn't [UnmanagedCallersOnly] be allowed? Same for cases where T also partecipates directly in the signature, but is substituted for a blittable type anyway, which should make the whole thing still valid anyway.

Right now there's technically a way to work around this, but it involves a ton more complexity as you need to setup a whole stub to then jump to the right generic method from a non-generic one, and you also need to store the extra stub function pointer in the type, as that's needed to be carried around in the native object for the whole thing to work. Here's an example of what is needed today to work around this limitation (wouldn't actually recommend doing this ever, way too clunky):

Generic stub workaround (click to expand):

public static unsafe class IAction
{
    [UnmanagedCallersOnly]
    public static void Invoke(void* @this)
    {
        ((delegate*<void*, void>)((void**)@this)[1])(@this);
    }
}

public unsafe struct IAction<T>
    where T : unmanaged
{
    public static readonly void** Vtbl = InitVtbl();

    private static void** InitVtbl()
    {
        void** lpVtbl = (void**)RuntimeHelpers.AllocateTypeAssociatedMemory(typeof(IAction<T>), sizeof(void*));

        lpVtbl[0] = (delegate* unmanaged<void*, void>)&IAction.Invoke;

        return lpVtbl;
    }

    public static IAction<T>* Create(Action<T> callback, T state)
    {
        IAction<T>* action = (IAction<T>*)NativeMemory.Alloc((nuint)sizeof(IAction<T>));

        action->lpVtbl = Vtbl;
        action->stub = (delegate*<void*, void>)&IAction<T>.Stub;
        action->handle = GCHandle.Alloc(callback);
        action->state = state;

        return action;
    }

    public void** lpVtbl;
    public void* stub;
    public GCHandle handle;
    public T state;

    public static void Stub(void* @this)
    {
        Unsafe.As<Action<T>>(((IAction<T>*)@this)->handle.Target!)(((IAction<T>*)@this)->state);
    }
}

Proposal

No new APIs, just lifting the restrictions from the runtime and Roslyn (CS8895) that prevents [UnmanagedCallersOnly] from being used on generic methods or methods in generic types. The same other restrictions should remain, just that they'd be fully validated at runtime in case the method was generic and one or more type parameters were used in the signature.

Risks

Not really seeing any other than the possible runtime crash if a user tried to use this on a generic type that was then substituted for an incompatible type. But the whole feature is incredibly niche and advanced anyway, so wouldn't really consider that a problem (there's plenty of other things that could go wrong when dealing with code like this already anyway).

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[jkotas]

This is a doc bug. The original C# 9 function pointer spec has the limitation listed: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-9.0/function-pointers#systemruntimeinteropservicesunmanagedcallersonlyattribute

This was intentional limitation to keep the runtime implementation simple. Generics and interop are not supported across the board (e.g. generic DllImport is not supported either).

[jkoritzinsky]

As @jkotas mentioned, we decided to limit the implementation for simplicity.

With generics specifically, there's a few additional issues. Specifically, UnmanagedCallersOnly is incompatible with shared-generics based on the current design. Since UnmanagedCallersOnly is designed to present the minimal overhead for calling a .NET function from native code, it does not provide any wrapper stub. As a result, if an UnmanagedCallersOnly method has a shared generic instantiation, then there's no mechanism to pass in the generic instantiation arg.

If we were to allow UnmanagedCallersOnly on generic methods, we'd have to be extra careful that we don't end up using a shared instantiation, otherwise users would get unexpected crashes and parameters would seem to not line up correctly. I encountered this while working on C#/WinRT and debugging it was quite messy.

Since shared instantiations is a performance optimization that we don't publicly document when it occurs and we currently don't provide a mechanism to opt-out a method from a shared generic experience, we've decided to explicitly not support generic UnmanagedCallersOnly methods as a whole, at least for the initial support.

Additionally, generic unmanaged function pointers are explicitly not supported across the board (see #9136), so it would be impossible to take an address of an UnmanagedCallersOnly method that's instantiated based on a generic parameter in the method it's in and cast it to its function pointer type, which would make the experience significantly worse than the usual UnmanagedCallersOnly experience.

I'll update the documentation for now, and in the future we can look at this again.

[Sergio0694]

"With generics specifically, there's a few additional issues. Specifically, UnmanagedCallersOnly is incompatible with shared-generics based on the current design. Since UnmanagedCallersOnly is designed to present the minimal overhead for calling a .NET function from native code, it does not provide any wrapper stub. As a result, if an UnmanagedCallersOnly method has a shared generic instantiation, then there's no mechanism to pass in the generic instantiation arg."

Hey @jkoritzinsky, yeah totally agree with that of course. As mentioned on Discord as well, my idea was in fact to only allow this when all substituted type parameters are unmanaged, so eg. the runtime could just throw a TypeLoadException or similar when the new type is instantiated (eg. I could see this being checked from MethodTableBuilder::BuildMethodTableThrowing maybe?), if it contains an [UnmanagedCallersOnly] method and any of the type arguments aren't unmanaged. This reminds me a bit of #43486, where we had basically a very similar proposal of removing the build-time enforcement of the explicit layout not being allowed on generics, and instead just checking it at runtime to again allow that when all type arguments were unmanaged.

Does that seem reasonable and potentially something that would make sense doing? 🙂

[jkotas]

It would make sense as end-to-end enablement of generics with interop. I do not think relaxing the generics restrictions for [UnmanagedCallersOnly] only while keeping it every else in interop makes sense.

It would help to have more concrete motivating example: The concrete unmanaged library that you trying to wrap, why source generators are not a viable solution to the problem, etc.

The counterexample is https://github.com/microsoft/CsWinRT. It has interop wrappers for WinRT generics and they were able to do without this. I guess that you often also need to marshal the generic types involved and that requires manual specialization based on T anyway.

[MichalStrehovsky]

my idea was in fact to only allow this when all substituted type parameters are unmanaged

This would basically hardcode/assume the current implementation details of generic sharing in CoreCLR. E.g. Mono sometimes does generic sharing over valuetypes as well, often when they would be considered unmanaged. Those would need a context argument as well. We shouldn't assume the current implementation of shared generic code in the design. It might change depending on scenario.

[Sergio0694]

"It would help to have more concrete motivating example: The concrete unmanaged library that you trying to wrap, why source generators are not a viable solution to the problem, etc."

So, I actually have a concrete example of a case where this would've been useful to have 😄

To very quickly give some context, I have a library, ComputeSharp, which allows developers to implement DX12 shaders in C# (there's a source generator that then transpiles them to HLSL and optionally precompiles them and embeds them in .data if needed too). Anyway, recently I've been working on adding support for D2D1 pixel shaders, and one thing I wanted to implement was also some API to easily create an ID2D1Effect instance from a given pixel shader (which is just an arbitrary C# struct for the user). Long story short, in order to register a D2D1 effect, you need to register it by passing a delegate* unmanaged<IUnknown**, HRESULT> factory to ID2D1Factory1::RegisterEffectFromString. Now, ideally I wanted something like this:

// Define some shader
[D2DInputCount(1)]
[D2DInputSimple(0)]
[D2DEmbeddedBytecode(D2D1ShaderProfile.PixelShader50)]
public partial struct MyShader : ID2D1PixelShader
{
    public float4 Execute()
    {
        return D2D1.GetInput(0); // Some arbitrary logic here...
    }
}

using ComPtr<ID2D1Effect> effect = default;

// Create the effect
D2D1InteropServices.CreateD2D1Effect<MyShader>(effect.GetAddressOf());

Behind the scenes, I had to implement the actual effect, which means rolling up an implementation of ID2D1EffectImpl and ID2D1DrawTransform. Ideally, you'd think you'd just do something like this to setup all the per-shader and per-effect state:

internal unsafe struct D2D1PixelShaderEffect<T>
    where T : unmanaged, ID2D1PixelShader
{
    // Random fields you might need...
    private static readonly Guid shaderId;
    private static readonly byte* bytecode;
    private static readonly int bytecodeSize;
    private static readonly int numberOfInputs;

    static D2D1PixelShaderEffect()
    {
        // Setup the shared state for all effects...
    }

    [UnmanagedCallersOnly]
    public static int CreateEffect(IUnknown** effectImpl)
    {
        // Create and initialize the new effect instance
    }
}

And then for registration you could just grab a pointer for that factory, use it to register, and go about your day. That could work given that T here is constrained to unmanaged, so every resolved function pointer would always point to a separate concrete method, but of course as discussed that's not allowed (small note: as @MichalStrehovsky said there's also an argument about value type generics never being shared being an implementation detail and not a spec guarantee, as a side note, but for this example let's just assume that's always the case). The solution I ended up at to solve this was to... Fallback to actually using a cached delegate, and then using Marshal.GetFunctionPointerForDelegate. Not really what I was expecting to find myself using on .NET 6+ for something like this, but alas it did solve the issue and I couldn't find any other ways around it 😄

So I basically ended up with something like this (see actual code here):

internal unsafe partial struct PixelShaderEffect
{
    [UnmanagedFunctionPointer(CallingConvention.Winapi)]
    private delegate int FactoryDelegate(IUnknown** effectImpl);

    public static class For<T>
        where T : unmanaged, ID2D1PixelShader
    {
        // Random fields you might need...
        private static readonly Guid shaderId;
        private static readonly byte* bytecode;
        private static readonly int bytecodeSize;
        private static readonly int numberOfInputs;

        private static readonly FactoryDelegate EffectFactory = CreateEffect;

        static For()
        {
            // Setup the shared state for all effects...
        }

        public static delegate* unmanaged<IUnknown**, HRESULT> Factory
        {
            get => (delegate* unmanaged<IUnknown**, HRESULT>)Marshal.GetFunctionPointerForDelegate(EffectFactory);
        }

        private static int CreateEffect(IUnknown** effectImpl)
        {
            // Create and initialize the new effect instance
        }
    }
}

This works, you'd just get that Factory function pointer and use it to register the effect, and you'd still be able to have it access all the T-specific shared static state for that shader instance. But, it's way clunkier and arguably more error prone to setup than just just being able to get a function pointer from that method in a generic type.

Anyway, I just figured I'd share my findings here since I ended up (by accident) bashing my head again against the current [UnmanagedCallersOnly] restriction, and thought this could be a valid (although niche) example of where it could be useful.

[DaZombieKiller]

This would basically hardcode/assume the current implementation details of generic sharing in CoreCLR. E.g. Mono sometimes does generic sharing over valuetypes as well, often when they would be considered unmanaged. Those would need a context argument as well. We shouldn't assume the current implementation of shared generic code in the design. It might change depending on scenario.

Would it be fine to enforce "no generic sharing" just for UnmanagedCallersOnly methods? It's comparable to a static field in a generic type, which cannot be shared:

static class C<T>
{
    public static int Value;
}

Alternatively, allowing UnmanagedCallConv on delegates would at least improve the situation. Right now, you cannot use extensible calling conventions with GetFunctionPointerForDelegate as you are limited to the CallingConvention enum (meaning you cannot use CallConvMemberFunction, for example).