.NET 8 ComWrappers source generator doesn't handle _VTblGap* methods

[smourier]

Description

The current .NET 8 ComWrappers source generator doesn't seem to support skipping vtable slots in COM interface declarations, as described and supported by the builtin interface marshaling https://learn.microsoft.com/en-us/dotnet/framework/unmanaged-api/metadata/imetadataemit-definemethod-method#slots-in-the-v-table

Reproduction Steps

This code works fine with .NET Framework or .NET 8 with built-in marshaling:

    internal class Program
    {
        static void Main()
        {
            CoCreateInstance(CLSID_WICImagingFactory, 0, CLSCTX_ALL, typeof(IWICImagingFactory).GUID, out var obj);
            var factory = (IWICImagingFactory)obj;
            factory.CreateQueryWriter(GUID_MetadataFormatApp1, 0, out var writer);
        }

        static readonly Guid GUID_MetadataFormatApp1 = new("8fd3dfc3-f951-492b-817f-69c2e6d9a5b0");
        static readonly Guid CLSID_WICImagingFactory = new("cacaf262-9370-4615-a13b-9f5539da4c0a");
        const int CLSCTX_ALL = 23;

        [PreserveSig, DllImport("ole32")]
        public static extern int CoCreateInstance([MarshalAs(UnmanagedType.LPStruct)] Guid rclsid, nint pUnkOuter, int dwClsContext, [MarshalAs(UnmanagedType.LPStruct)] Guid riid, [MarshalAs(UnmanagedType.IUnknown)] out object ppv);

        [ComImport, Guid("ec5ec8a9-c395-4314-9c77-54d7a935ff70"), InterfaceType(ComInterfaceType.InterfaceIsIUnknown)]
        public partial interface IWICImagingFactory
        {
            void _VtblGap1_23(); // skip 23 methods we don't need

            [PreserveSig]
            int CreateQueryWriter([MarshalAs(UnmanagedType.LPStruct)] Guid guidMetadataFormat, nint pguidVendor, out IWICMetadataQueryWriter ppIQueryWriter);
        }

        [ComImport, Guid("a721791a-0def-4d06-bd91-2118bf1db10b"), InterfaceType(ComInterfaceType.InterfaceIsIUnknown)]
        public partial interface IWICMetadataQueryWriter
        {
            // undefined yet
        }
    }

Here's an equivalent source-generated .NET 8 ComWrappers:

internal partial class Program
{
    static unsafe void Main(string[] args)
    {
        CoCreateInstance(CLSID_WICImagingFactory, 0, CLSCTX_ALL, typeof(IWICImagingFactory).GUID, out var obj);
        var factory = (IWICImagingFactory)obj;
        factory.CreateQueryWriter(GUID_MetadataFormatApp1, Unsafe.NullRef<Guid>(), out var writer);
    }

    static readonly Guid GUID_MetadataFormatApp1 = new("8fd3dfc3-f951-492b-817f-69c2e6d9a5b0");
    static readonly Guid CLSID_WICImagingFactory = new("cacaf262-9370-4615-a13b-9f5539da4c0a");
    const int CLSCTX_ALL = 23;

    [PreserveSig, LibraryImport("ole32")]
    public static partial int CoCreateInstance(in Guid rclsid, nint pUnkOuter, int dwClsContext, in Guid riid, [MarshalUsing(typeof(UniqueComInterfaceMarshaller<object>))] out object ppv);

    [GeneratedComInterface, Guid("ec5ec8a9-c395-4314-9c77-54d7a935ff70")]
    public partial interface IWICImagingFactory
    {
        void _VTblGap1_23(); // skip 23 methods we don't need

        [PreserveSig]
        int CreateQueryWriter(in Guid guidMetadataFormat, in Guid pguidVendor, out IWICMetadataQueryWriter ppIQueryWriter);
    }

    [GeneratedComInterface, Guid("a721791a-0def-4d06-bd91-2118bf1db10b")]
    public partial interface IWICMetadataQueryWriter
    {
    }
}

Expected behavior

It should work the same.

Actual behavior

.NET 8 source-generated ComWrappers version crashes with a "System.AccessViolationException: Attempted to read or write protected memory" because the code seems to consider _VtblGap1_23 as a regular method.

Note if you change _VTblGap1_23 by _VtblGap1_23 (note the t has a different case), the error is not the same, you get a strange

Unhandled exception. System.MissingMethodException: Method not found: 'Void IWICImagingFactory._VtblGap1_23()'.
   at System.ModuleHandle.ResolveType(QCallModule module, Int32 typeToken, IntPtr* typeInstArgs, Int32 typeInstCount, IntPtr* methodInstArgs, Int32 methodInstCount, ObjectHandleOnStack type)
   at System.ModuleHandle.ResolveTypeHandle(Int32 typeToken, RuntimeTypeHandle[] typeInstantiationContext, RuntimeTypeHandle[] methodInstantiationContext)
   at System.Reflection.RuntimeModule.ResolveType(Int32 metadataToken, Type[] genericTypeArguments, Type[] genericMethodArguments)
   at System.Reflection.CustomAttribute.FilterCustomAttributeRecord(MetadataToken caCtorToken, MetadataImport& scope, RuntimeModule decoratedModule, MetadataToken decoratedToken, RuntimeType attributeFilterType, Boolean mustBeInheritable, ListBuilder`1& derivedAttributes, RuntimeType& attributeType, IRuntimeMethodInfo& ctorWithParameters, Boolean& isVarArg)
   at System.Reflection.CustomAttribute.AddCustomAttributes(ListBuilder`1& attributes, RuntimeModule decoratedModule, Int32 decoratedMetadataToken, RuntimeType attributeFilterType, Boolean mustBeInheritable, ListBuilder`1 derivedAttributes)
   at System.Reflection.CustomAttribute.GetCustomAttributes(RuntimeType type, RuntimeType caType, Boolean inherit)
   at System.Attribute.GetCustomAttributes(MemberInfo element, Type attributeType, Boolean inherit)
   at System.Attribute.GetCustomAttribute(MemberInfo element, Type attributeType, Boolean inherit)
   at System.Runtime.InteropServices.Marshalling.IIUnknownDerivedDetails.GetFromAttribute(RuntimeTypeHandle handle)
   at System.Runtime.InteropServices.Marshalling.ComObject.LookUpVTableInfo(RuntimeTypeHandle handle, TableInfo& result, Int32& qiHResult)
   at System.Runtime.InteropServices.Marshalling.ComObject.System.Runtime.InteropServices.IDynamicInterfaceCastable.IsInterfaceImplemented(RuntimeTypeHandle interfaceType, Boolean throwIfNotImplemented)

Regression?

It's a regression from built-in marshaling.

Known Workarounds

I don't know any but declare dummy methods.

Configuration

Latest .NET 8, Windows 11, 64-bit

Other information

No response

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/interop-contrib
See info in area-owners.md if you want to be subscribed.

[AaronRobinsonMSFT]

@smourier Thanks for trying out this scenario. I'm not sure we want to continue this behavior into the new system. This is an undocumented feature that is an implicit contract between .NET Framework/CoreCLR and the C# compiler. It is an oddity that I wish was done in a better way. The concept of creating a gap makes a lot of sense, but this mechanism seems to much magic. Is the pattern itself important here or just the ability to define a vtable gap?

[smourier]

It was actually documented in "CLI Partition II" (as "Microsoft Specific") back then, as well as for example in in Serge Lidin's "Expert .NET 2.0 IL Assembler" book (I'm an old COM programmer ...).

It does look a bit awkward and magic but I find the feature super useful when you just consume interfaces (obviously) with lots of members, for example in one of my answer here https://stackoverflow.com/a/60112084/403671

Any mechanism that would allow (possibly multiple) gaps would be fine I guess, since we already lost source compatibility anyway with source-generation.

[AaronRobinsonMSFT]

It was actually documented in "CLI Partition II" (as "Microsoft Specific") back then

In ECMA-335? I can't find this in the spec anywhere.

as well as for example in in Serge Lidin's "Expert .NET 2.0 IL Assembler" book (I'm an old COM programmer ...).

Yep. Serge's book is not authoritative though and discusses many implementation details that are best left avoided.

Any mechanism that would allow (possibly multiple) gaps would be fine I guess

Perfect.

@jkoritzinsky and @jtschuster Any preference here? Retaining the _VTblGap* mechanic or a new attribute? I dislike the naming convention approach, but I admit I've recommend this to people as well so I'm sympathetic to retaining it.

[smourier]

In ECMA-335? I can't find this in the spec anywhere.

Oh no, when I say old, I mean it :-) https://download.microsoft.com/download/7/3/3/733ad403-90b2-4064-a81e-01035a7fe13c/ms%20partition%20ii.pdf

[AaronRobinsonMSFT]

Ah, this is prior to standardization. Well, I am going to defer to @jkoritzinsky or @agocke here. I personally don't like this pattern at all, but it is niche enough and something I've used as well. Regardless of whether we continue to respect the _VTblGap* pattern or we come up with an attribute placeholder, it should be documented and a test added. Going with the _VTblGap* pattern makes documentation much simpler for all Microsoft-created conforming runtime implementations though and I'm not sure a new attribute is all that helpful given the implementation cost.

Regardless, appreciate the issue, this is a documentation area that should be improved.

[agocke]

Oh wow, that's quite subtle. Never seen it before. I'm personally not opposed to supporting it as it does seem useful -- but I would probably support the existing mechanism if it's not too much trouble. It's already esoteric enough that I wouldn't want to invent a new, incompatible way of doing the same thing.

[hamarb123]

We could also chuck it in the ECMA-335 Augments so that it doesn't get lost in the future.

[AaronRobinsonMSFT]

probably support the existing mechanism if it's not too much trouble

Definitely not too much trouble from the runtime side. Adding this specific support to the COM source generators might be tricky since the generator would then also need to parse the _VTblGap* pattern and that means creation of additional parsers. It isn't like this can be just a "simple pass through" though. The source generators need to create vtable slots and also decide how to respect this for CCWs.

We could also chuck it in the ECMA-335 Augments if we support it so that it doesn't get lost in the future.

This should remain outside the official ECMA-335. I appreciate the idea of adding documentation and that should happen in here. Adding this to ECMA-335 is elevating this pattern to a place I'd prefer to not clutter with niche historical artifacts.

[dongle-the-gadget]

I'm pretty sure there's another issue to propose using attributes to manually define VTables? It looks to be a more elegant solution than relying on gaps.

[hamarb123]

In addition, as you can only define a method signature once, you can't have multiple gaps that are the same size.

You're supposed to have the sequence number according to the spec that was linked at the top of the original description (and it's also in the example code provided in the original description) - so there shouldn't actually be any issue with that.

[colejohnson66]

I also think an attribute-based solution would be the best option. The _VTblGap_X method is a flaky hack. It's an implicit contract between the compiler and runtime.

I think a "VTable offset" attribute would be the cleanest:

[GeneratedComInterface, Guid("ec5ec8a9-c395-4314-9c77-54d7a935ff70")]
public partial interface IWICImagingFactory
{
    [ComVTableOffset(23)]
    int CreateQueryWriter(in Guid guidMetadataFormat, in Guid pguidVendor, out IWICMetadataQueryWriter ppIQueryWriter);
}

Methods after one annotated with [ComVTableOffset] would, as before, have incrementing offsets, similar to enumerations with explicit values. It would be an error to have overlapping V-Table offsets.

The generator would change the output:

 file unsafe partial interface InterfaceImplementation
 {
     internal static void** CreateManagedVirtualFunctionTable()
     {
-        void** vtable = (void**)global::System.Runtime.CompilerServices.RuntimeHelpers.AllocateTypeAssociatedMemory(typeof(global::Program.IWICImagingFactory), sizeof(void*) * 5);
+        void** vtable = (void**)global::System.Runtime.CompilerServices.RuntimeHelpers.AllocateTypeAssociatedMemory(typeof(global::Program.IWICImagingFactory), sizeof(void*) * 27);
         {
             nint v0, v1, v2;
             global::System.Runtime.InteropServices.ComWrappers.GetIUnknownImpl(out v0, out v1, out v2);
             vtable[0] = (void*)v0;
             vtable[1] = (void*)v1;
             vtable[2] = (void*)v2;
         }

         {
-            vtable[3] = (void*)(delegate* unmanaged[MemberFunction]<global::System.Runtime.InteropServices.ComWrappers.ComInterfaceDispatch*, int> )&ABI__VTblGap1_23;
-            vtable[4] = (void*)(delegate* unmanaged[MemberFunction]<global::System.Runtime.InteropServices.ComWrappers.ComInterfaceDispatch*, global::System.Guid*, global::System.Guid*, void**, int> )&ABI_CreateQueryWriter;
+            vtable[26] = (void*)(delegate* unmanaged[MemberFunction]<global::System.Runtime.InteropServices.ComWrappers.ComInterfaceDispatch*, global::System.Guid*, global::System.Guid*, void**, int> )&ABI_CreateQueryWriter;
         }

         return vtable;
     }
 }

An open question would be: should one be allowed to specify an offset lower than the current? Could I do this?

[GeneratedComInterface, Guid("ec5ec8a9-c395-4314-9c77-54d7a935ff70")]
public partial interface IWICImagingFactory
{
    [ComVTableOffset(23)]
    HRESULT CreateQueryWriter(in Guid guidMetadataFormat, ref readonly Guid pguidVendor, out IWICMetadataQueryWriter ppIQueryWriter);

    [ComVTableOffset(22)]
    HRESULT CreatePalette(out IWICPalette ppIPalette);
}

[dongle-the-gadget]

I think a VTable index option looks better.