Use blittable signatures for Platform P/Invoke

[dongle-the-gadget]

Closes #1317.

cc @Sergio0694

[Sergio0694]

Did a first quick pass and left some notes 🙂

[Sergio0694]

This is extremely inefficient, and it will allocate and throw away a buffer for the name every time.

[dongle-the-gadget]

What do you think would be a better way to do this? From looking at LibraryImport, it seems to stack allocate the UTF-8 string if it's small and heap allocate it otherwise.

[Sergio0694]

That's a reasonable approach, since most method names would be small enough anyway. Could pick a given threshold (maybe 0x100 like LibraryImport) and use that with a stackalloc if the name is small enough, or just allocate a new array if not, which would realistically never happen.

[dongle-the-gadget]

Does this seem reasonable?

internal static unsafe void* TryGetProcAddress(IntPtr moduleHandle, string functionName)
{
    bool allocated = false;
    Span<byte> buffer = stackalloc byte[0x100];
    if (functionName.Length * 3 >= 0x100) // Maximum of 3 bytes per UTF-8 character, stack allocation limit of 256 bytes (including the null terminator)
    {
        // Calculate accurate byte count when the provided stack-allocated buffer is not sufficient
        int exactByteCount = checked(Encoding.UTF8.GetByteCount(functionName) + 1); // + 1 for null terminator
        if (exactByteCount > 0x100)
        {
#if NET6_0_OR_GREATER
            buffer = new((byte*)NativeMemory.Alloc((nuint)exactByteCount), exactByteCount);
#else
            buffer = (byte*)Marshal.AllocHGlobal(exactByteCount);
#endif
            allocated = true;
        }
    }

    var rawByte = (byte*)Unsafe.AsPointer(ref MemoryMarshal.GetReference(buffer));

    int byteCount = Encoding.UTF8.GetBytes(functionName, buffer);
    buffer[byteCount] = 0;

    void* functionPtr = TryGetProcAddress(moduleHandle, (sbyte*)rawByte);

    if (allocated)
#if NET6_0_OR_GREATER
        NativeMemory.Free(rawByte);
#else
        Marshal.FreeHGlobal((IntPtr)rawByte);
#endif

    return functionPtr;
}

[manodasanW]

This looks reasonable but might want to confirm that the version of Encoding.UTF8.GetBytes you are using is accessible in NET Standard 2.0. It is fine if the .NET Standard version is not blittable / AOT complaint if there is no equivalent.

[dongle-the-gadget]

I think what I'll do is pin the string on .NET Standard 2.0 and use the GetBytes(Char*, Int32, Byte*, Int32) function.

[Sergio0694]

If this overload is only for downlevel, we should conditionally only include this when the other one is not.
We shouldn't just always include both of these, as only one of them is used at a time.

[dongle-the-gadget]

So we should have like two P/Invoke declarations for GetProcAddress? One blittable and the other not?

[Sergio0694]

I think we should evaluate with @manodasanW what approach to take here with the constraints we have, and possibly reevaluate some of them. For instance, I'm not sure the split for C# 11 is worth the complexity. If the embedded source files have to support downlevel to .NET Standard 2.0, then their current C# 9 language level is already officially not supported anyway. So at that point why not just bump that to C# 11 directly and say if you want to embed CsWinRT, you need to set your language version to C# 11. They'd have to do the same anyway for C# 9, and both are equally "not supported". Doing this would at least let us stop worrying about the language, and just check the TFM.

[dongle-the-gadget]

Well the TFM check isn't really needed (after all, UTF-8 string literals are only language-dependent rather than runtime-dependent). It's mostly to make sure that we are on the "correct" language version for the feature (since .NET 7 uses C#11 by default). Actually checking for the C# version is... not a very easy task to do in MSBuild.

[Sergio0694]

Yes but my point is even if we kept the limit of C# 9, that'd still be technically unsupported already. So what I'm saying is maybe we might as well just say you need C# 11 for embed support, and make the code simpler and more consistent. I can't make a decision on this though, I'm just suggesting what I think a possible approach could be 🙂

[manodasanW]

We have folks with embedded projections and regular projections that are going to be on the .NET 6 SDK due to that is the LTS build. So that is why we will still need the non C# 11 fallback.

[Sergio0694]

Is this something we could reevaluate after .NET 8 hits RTM? As in, could we just say consumers of the embedded projections would be expected to be using a language version matching whatever the latest LTS is?

[manodasanW]

Yea we will revisit this a couple months after .NET 8 comes out.

[manodasanW]

I had ran the CI pipeline and it looks like it ran into the following errors:

  src\WinRT.Runtime\ExceptionHelpers.cs(57,67): warning CS0649: Field 'ExceptionHelpers.getRestrictedErrorInfo' is never assigned to, and will always have its default value [src\WinRT.Runtime\WinRT.Runtime.csproj]
  src\WinRT.Runtime\ExceptionHelpers.cs(58,66): warning CS0649: Field 'ExceptionHelpers.setRestrictedErrorInfo' is never assigned to, and will always have its default value [src\WinRT.Runtime\WinRT.Runtime.csproj]

  src\cswinrt\strings\WinRT.cs(80,30): error CS0029: Cannot implicitly convert type 'byte*' to 'System.Span<byte>' [src\WinRT.Runtime\WinRT.Runtime.csproj]
  src\WinRT.Runtime\ExceptionHelpers.cs(94,42): error CS0029: Cannot implicitly convert type 'delegate* unmanaged[Stdcall]<System.IntPtr*, int>' to 'System.ReadOnlySpan<byte>' [src\WinRT.Runtime\WinRT.Runtime.csproj]
  src\WinRT.Runtime\ExceptionHelpers.cs(95,42): error CS0029: Cannot implicitly convert type 'delegate* unmanaged[Stdcall]<System.IntPtr, int>' to 'System.ReadOnlySpan<byte>' [src\WinRT.Runtime\WinRT.Runtime.csproj]

  src\WinRT.Runtime\ExceptionHelpers.cs(94,42): error CS0029: Cannot implicitly convert type 'delegate* unmanaged[Stdcall]<nint*, int>' to 'System.ReadOnlySpan<byte>' [src\WinRT.Runtime\WinRT.Runtime.csproj]
  src\WinRT.Runtime\ExceptionHelpers.cs(95,42): error CS0029: Cannot implicitly convert type 'delegate* unmanaged[Stdcall]<nint, int>' to 'System.ReadOnlySpan<byte>' [src\WinRT.Runtime\WinRT.Runtime.csproj]

[dongle-the-gadget]

@manodasanW the errors should be fixed now. Can you try running the CI again?

[manodasanW]

@dongle-the-gadget the latest CI run passed. Thanks for your changes and working through the feedback.