Support generic arguments for calli in CoreCLR

[hez2010]

Adding generic arguments support for calli in CoreCLR.

We don't need to handle generics in DynamicMethodDesc as it won't be a generic method definition and the generic arguments here can only be blittable types (so no generic sharing).

Note that this has already been supported by NativeAOT.

Console.WriteLine(Call<int, int>((delegate* unmanaged<int, int>)&TestII, 42));
Console.WriteLine(Call<float, float>((delegate* unmanaged<float, float>)&TestFF, 42.42f));
Console.WriteLine(Call<float, int>((delegate* unmanaged<float, int>)&TestFI, 42.42f));

Console.WriteLine(Foo<int>.Call<int>((delegate* unmanaged<int, int>)&TestII, 42));

unsafe static U Call<T, U>(void* pfn, T arg)
{
    return ((delegate* unmanaged<T, U>)pfn)(arg);
}

[UnmanagedCallersOnly]
static int TestII(int arg) => arg;

[UnmanagedCallersOnly]
static float TestFF(float arg) => arg;

[UnmanagedCallersOnly]
static int TestFI(float arg) => (int)arg;

class Foo<T>
{
    public unsafe static U Call<U>(void* pfn, T arg)
    {
        return ((delegate* unmanaged<T, U>)pfn)(arg);
    }
}

Output:

42
42.42
42
42

Resolves #9136

[jkotas]

here can only be blittable types (so no generic sharing).

We still need a reasonable failure mode for the unsupported cases. Please make sure to add tests for them.

[jkotas]

Also #9136 (comment) : Would it make sense to enable this only when runtime marshalling is disabled for the assembly?

[hez2010]

Would it make sense to enable this only when runtime marshalling is disabled for the assembly?

We already disallow any runtime marshalling against UnmanagedCallerOnly methods, so I assumes we only need an error mode when the signature is unexpected?

[MichalPetryka]

Would it make sense to enable this only when runtime marshalling is disabled for the assembly?

We already disallow any runtime marshalling against UnmanagedCallerOnly methods, so I assumes we only need an error mode when the signature is unexpected?

Your change applies to all unmanaged pointers though, not just UnmanagedCallersaOnly.

[hez2010]

Also #9136 (comment) : Would it make sense to enable this only when runtime marshalling is disabled for the assembly?

I added a guard to make sure no marshaling is required for calli.

[jkotas]

This needs tests and fixes for bugs discovered by the tests.

[jkotas]

How hard is it to implement generic unmanaged calli support in Mono?

cc @vargaz @lambdageek

[AaronRobinsonMSFT]

Based on my conversations with @lambdageek I think generics support at some of these interop margins are a bit tricky in mono.

[jkotas]

Hmm, that's unfortunate.

We try to maintain feature parity between all runtimes. We need to figure out what we are going to do about Mono support before enabling it for CoreCLR.

[lambdageek]

The example from the issue descriptions works in net8.0 Mono already with JIT and interpreter. I haven't tried the examples from the rest of the added test.

---

The thing that Aaron is talking about is that mono doesn't have great support for creating wrapper methods that are themselves generic (ie: they have generic method params). We would need that for the generic UnsafeAccessor work - the issue there is that we need to generate a generic method. Likewise if it was legal to do something like this:

void *pGenericFn = &SomeGenericUnsafeCallersOnlyMethod;

[UnsafeCallersOnly]
unsafe void SomeGenericUnsafeCallersOnlyMethod<T>(T *) where T : unmanaged { ... }

we would need to generate a generic wrapper.

---

For unmanaged function pointers we create wrappers around the &Func - ldftn SomeUnmanagedCallersOnlyMethod actually returns the address of the wrapper that has the native, not managed, calling convention.

At the calli, we don't need to do anything special almost all the time.

The one place where we might get into trouble is if we need to do a call out using generic sharing, or gsharedvt (universal generic sharing): That is Foo<int>.Call<int>(...) from the issue description, but compiled for ios, for example. I haven't tried it.

[vargaz]

The failures are probably going to be with generic sharing and aot. The JIT should work fine.

[hez2010]

Hmm, that's unfortunate.

We try to maintain feature parity between all runtimes. We need to figure out what we are going to do about Mono support before enabling it for CoreCLR.

The tests were actually passing on mono, except mono-llvm-aot failed to build the negative tests (RunInvalidGenericFunctionPointerTest).

[jkotas]

Could you please factor out the negative cases into separate test and only disable those for Mono?

Handling of negative cases by llvm-aot should not be blocking.

[hez2010]

Seems that even if we removed negative cases, mono-aot-llvm still fail to compile (but with a different error message).

[hez2010]

Is there anything I can do to make this PR get merged? Maybe I should disable the generic calli tests until mono impl ready to unblock the CI?

[jkotas]

until mono impl ready

Do you plan to work on the mono impl?

[hez2010]

until mono impl ready

Do you plan to work on the mono impl?

I can try but I don't have experience with the mono codebase. Maybe need some help when implement this for mono.

[jkotas]

I can try but I don't have experience with the mono codebase. Maybe need some help when implement this for mono.

@vargaz @lambdageek Do you have any advice for how to implement this for Mono?

(If you prefer to merge this PR as is and deal with the mono implementation separately, that's fine too.)

[vargaz]

Would be better to merge this and disable the failing tests on mono.

[jkotas]

Would be better to merge this and disable the failing tests on mono.

@hez2010 Could you please do that?

[hez2010]

Done. CI failures are unrelated.

[jkotas]

I think that the conditional compilation like this is violating invariants of the test builds. From https://github.com/dotnet/runtime/blob/main/docs/workflow/testing/coreclr/test-configuration.md#adding-test-guidelines: "Therefore the managed portion of each test must not contain - Target platform dependent conditionally included files"

The pattern to disable tests for Mono is <CLRTestTargetUnsupported Condition="'$(RuntimeFlavor)' == 'mono'">true</CLRTestTargetUnsupported>.

[jkoritzinsky]

We do build the tests separately for Mono and CoreCLR builds (and we depend on this in the test source generator today).

I think this is fine for now (and is better than splitting out these tests into a separate process just to avoid running these tests on Mono).

[jkotas]

This should be a link to an issue that tracks implementing this feature in Mono.

[jkotas]

LGTM.

@dotnet/interop-contrib Could you please review this as well?

[jkotas]

Thank you!