Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

COM interface derivation chain for 3+ interfaces doesn't generate shadowing and vtables correctly #86662

Closed
jkoritzinsky opened this issue May 23, 2023 · 2 comments
Labels
area-System.Runtime.InteropServices bug source-generator Indicates an issue with a source generator feature
Milestone

Comments

@jkoritzinsky
Copy link
Member

Given the following interfaces:

    [GeneratedComInterface]
    [Guid(_guid)]
    internal partial interface IGetAndSetInt
    {
        int GetInt();

        public void SetInt(int x);

        public const string _guid = "2c3f9903-b586-46b1-881b-adfce9af47b1";
    }
    [GeneratedComInterface]
    [Guid(_guid)]
    internal partial interface IDerived : IGetAndSetInt
    {
        void SetName([MarshalUsing(typeof(Utf16StringMarshaller))] string name);

        [return: MarshalUsing(typeof(Utf16StringMarshaller))]
        string GetName();

        internal new const string _guid = "7F0DB364-3C04-4487-9193-4BB05DC7B654";
    }
    [GeneratedComInterface]
    [Guid(_guid)]
    internal partial interface IDerived2 : IDerived
    {
        void Foo([MarshalUsing(typeof(Utf16StringMarshaller))] string name);

        internal new const string _guid = "60B86AC9-72FC-4BA3-9C83-4A3726FFBEF8";
    }

We generate the following code for IDerived2:

[System.Runtime.InteropServices.DynamicInterfaceCastableImplementationAttribute]
file unsafe partial interface InterfaceImplementation : global::SharedTypes.ComInterfaces.IDerived2
{
    [System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Interop.ComInterfaceGenerator", "42.42.42.42")]
    [System.Runtime.CompilerServices.SkipLocalsInitAttribute]
    void global::SharedTypes.ComInterfaces.IDerived2.Foo(string name)
    {
        var(__this, __vtable_native) = ((System.Runtime.InteropServices.Marshalling.IUnmanagedVirtualMethodTableProvider)this).GetVirtualMethodTableInfoForKey(typeof(global::SharedTypes.ComInterfaces.IDerived2));
        int __invokeRetVal;
        // Pin - Pin data in preparation for calling the P/Invoke.
        fixed (void* __name_native = &global::System.Runtime.InteropServices.Marshalling.Utf16StringMarshaller.GetPinnableReference(name))
        {
            __invokeRetVal = ((delegate* unmanaged[MemberFunction]<void*, ushort*, int> )__vtable_native[5])(__this, (ushort*)__name_native);
        }

        System.GC.KeepAlive(this);
        // Unmarshal - Convert native data to managed data.
        System.Runtime.InteropServices.Marshal.ThrowExceptionForHR(__invokeRetVal);
    }

    [System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Interop.ComInterfaceGenerator", "42.42.42.42")]
    [System.Runtime.CompilerServices.SkipLocalsInitAttribute]
    int global::SharedTypes.ComInterfaces.IDerived2.GetInt()
    {
        var(__this, __vtable_native) = ((System.Runtime.InteropServices.Marshalling.IUnmanagedVirtualMethodTableProvider)this).GetVirtualMethodTableInfoForKey(typeof(global::SharedTypes.ComInterfaces.IDerived2));
        int __retVal;
        int __invokeRetVal;
        {
            __invokeRetVal = ((delegate* unmanaged[MemberFunction]<void*, int*, int> )__vtable_native[3])(__this, &__retVal);
        }

        System.GC.KeepAlive(this);
        // Unmarshal - Convert native data to managed data.
        System.Runtime.InteropServices.Marshal.ThrowExceptionForHR(__invokeRetVal);
        return __retVal;
    }

    [System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Interop.ComInterfaceGenerator", "42.42.42.42")]
    [System.Runtime.CompilerServices.SkipLocalsInitAttribute]
    void global::SharedTypes.ComInterfaces.IDerived2.SetInt(int x)
    {
        var(__this, __vtable_native) = ((System.Runtime.InteropServices.Marshalling.IUnmanagedVirtualMethodTableProvider)this).GetVirtualMethodTableInfoForKey(typeof(global::SharedTypes.ComInterfaces.IDerived2));
        int __invokeRetVal;
        {
            __invokeRetVal = ((delegate* unmanaged[MemberFunction]<void*, int, int> )__vtable_native[4])(__this, x);
        }

        System.GC.KeepAlive(this);
        // Unmarshal - Convert native data to managed data.
        System.Runtime.InteropServices.Marshal.ThrowExceptionForHR(__invokeRetVal);
    }

    int global::SharedTypes.ComInterfaces.IGetAndSetInt.GetInt() => throw new System.Diagnostics.UnreachableException();
    void global::SharedTypes.ComInterfaces.IGetAndSetInt.SetInt(int x) => throw new System.Diagnostics.UnreachableException();
}

file unsafe partial interface InterfaceImplementation
{
    [System.Runtime.InteropServices.UnmanagedCallersOnlyAttribute(CallConvs = new[] { typeof(System.Runtime.CompilerServices.CallConvMemberFunction) })]
    internal static int ABI_Foo(System.Runtime.InteropServices.ComWrappers.ComInterfaceDispatch* __this_native, ushort* __name_native)
    {
        global::SharedTypes.ComInterfaces.IDerived2 @this = default;
        string name = default;
        int __retVal = default;
        try
        {
            // Unmarshal - Convert native data to managed data.
            __retVal = 0; // S_OK
            name = global::System.Runtime.InteropServices.Marshalling.Utf16StringMarshaller.ConvertToManaged(__name_native);
            @this = System.Runtime.InteropServices.ComWrappers.ComInterfaceDispatch.GetInstance<global::SharedTypes.ComInterfaces.IDerived2>(__this_native);
            @this.Foo(name);
        }
        catch (System.Exception __exception)
        {
            __retVal = System.Runtime.InteropServices.Marshalling.ExceptionAsHResultMarshaller<int>.ConvertToUnmanaged(__exception);
        }

        return __retVal;
    }
}

file unsafe partial interface InterfaceImplementation
{
    internal static void** CreateManagedVirtualFunctionTable()
    {
        void** vtable = (void**)System.Runtime.CompilerServices.RuntimeHelpers.AllocateTypeAssociatedMemory(typeof(global::SharedTypes.ComInterfaces.IDerived2), sizeof(void*) * 6);
        {
            System.Runtime.InteropServices.NativeMemory.Copy(System.Runtime.InteropServices.Marshalling.StrategyBasedComWrappers.DefaultIUnknownInterfaceDetailsStrategy.GetIUnknownDerivedDetails(typeof(global::SharedTypes.ComInterfaces.IGetAndSetInt).TypeHandle).ManagedVirtualMethodTable, vtable, (nuint)(sizeof(void*) * 5));
        }

        {
            vtable[5] = (void*)(delegate* unmanaged[MemberFunction]<System.Runtime.InteropServices.ComWrappers.ComInterfaceDispatch*, ushort*, int> )&ABI_Foo;
        }

        return vtable;
    }
}

We should be generating shadow methods for IGetAndSetInt as well as IDerived for IDerived2, not just IDerived. We should also copy the base vtable from IDerived instead of IGetAndSetInt.

@jkoritzinsky jkoritzinsky added bug area-System.Runtime.InteropServices source-generator Indicates an issue with a source generator feature labels May 23, 2023
@jkoritzinsky jkoritzinsky added this to the 8.0.0 milestone May 23, 2023
@ghost
Copy link

ghost commented May 23, 2023

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

Issue Details

Given the following interfaces:

    [GeneratedComInterface]
    [Guid(_guid)]
    internal partial interface IGetAndSetInt
    {
        int GetInt();

        public void SetInt(int x);

        public const string _guid = "2c3f9903-b586-46b1-881b-adfce9af47b1";
    }
    [GeneratedComInterface]
    [Guid(_guid)]
    internal partial interface IDerived : IGetAndSetInt
    {
        void SetName([MarshalUsing(typeof(Utf16StringMarshaller))] string name);

        [return: MarshalUsing(typeof(Utf16StringMarshaller))]
        string GetName();

        internal new const string _guid = "7F0DB364-3C04-4487-9193-4BB05DC7B654";
    }
    [GeneratedComInterface]
    [Guid(_guid)]
    internal partial interface IDerived2 : IDerived
    {
        void Foo([MarshalUsing(typeof(Utf16StringMarshaller))] string name);

        internal new const string _guid = "60B86AC9-72FC-4BA3-9C83-4A3726FFBEF8";
    }

We generate the following code for IDerived2:

[System.Runtime.InteropServices.DynamicInterfaceCastableImplementationAttribute]
file unsafe partial interface InterfaceImplementation : global::SharedTypes.ComInterfaces.IDerived2
{
    [System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Interop.ComInterfaceGenerator", "42.42.42.42")]
    [System.Runtime.CompilerServices.SkipLocalsInitAttribute]
    void global::SharedTypes.ComInterfaces.IDerived2.Foo(string name)
    {
        var(__this, __vtable_native) = ((System.Runtime.InteropServices.Marshalling.IUnmanagedVirtualMethodTableProvider)this).GetVirtualMethodTableInfoForKey(typeof(global::SharedTypes.ComInterfaces.IDerived2));
        int __invokeRetVal;
        // Pin - Pin data in preparation for calling the P/Invoke.
        fixed (void* __name_native = &global::System.Runtime.InteropServices.Marshalling.Utf16StringMarshaller.GetPinnableReference(name))
        {
            __invokeRetVal = ((delegate* unmanaged[MemberFunction]<void*, ushort*, int> )__vtable_native[5])(__this, (ushort*)__name_native);
        }

        System.GC.KeepAlive(this);
        // Unmarshal - Convert native data to managed data.
        System.Runtime.InteropServices.Marshal.ThrowExceptionForHR(__invokeRetVal);
    }

    [System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Interop.ComInterfaceGenerator", "42.42.42.42")]
    [System.Runtime.CompilerServices.SkipLocalsInitAttribute]
    int global::SharedTypes.ComInterfaces.IDerived2.GetInt()
    {
        var(__this, __vtable_native) = ((System.Runtime.InteropServices.Marshalling.IUnmanagedVirtualMethodTableProvider)this).GetVirtualMethodTableInfoForKey(typeof(global::SharedTypes.ComInterfaces.IDerived2));
        int __retVal;
        int __invokeRetVal;
        {
            __invokeRetVal = ((delegate* unmanaged[MemberFunction]<void*, int*, int> )__vtable_native[3])(__this, &__retVal);
        }

        System.GC.KeepAlive(this);
        // Unmarshal - Convert native data to managed data.
        System.Runtime.InteropServices.Marshal.ThrowExceptionForHR(__invokeRetVal);
        return __retVal;
    }

    [System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Interop.ComInterfaceGenerator", "42.42.42.42")]
    [System.Runtime.CompilerServices.SkipLocalsInitAttribute]
    void global::SharedTypes.ComInterfaces.IDerived2.SetInt(int x)
    {
        var(__this, __vtable_native) = ((System.Runtime.InteropServices.Marshalling.IUnmanagedVirtualMethodTableProvider)this).GetVirtualMethodTableInfoForKey(typeof(global::SharedTypes.ComInterfaces.IDerived2));
        int __invokeRetVal;
        {
            __invokeRetVal = ((delegate* unmanaged[MemberFunction]<void*, int, int> )__vtable_native[4])(__this, x);
        }

        System.GC.KeepAlive(this);
        // Unmarshal - Convert native data to managed data.
        System.Runtime.InteropServices.Marshal.ThrowExceptionForHR(__invokeRetVal);
    }

    int global::SharedTypes.ComInterfaces.IGetAndSetInt.GetInt() => throw new System.Diagnostics.UnreachableException();
    void global::SharedTypes.ComInterfaces.IGetAndSetInt.SetInt(int x) => throw new System.Diagnostics.UnreachableException();
}

file unsafe partial interface InterfaceImplementation
{
    [System.Runtime.InteropServices.UnmanagedCallersOnlyAttribute(CallConvs = new[] { typeof(System.Runtime.CompilerServices.CallConvMemberFunction) })]
    internal static int ABI_Foo(System.Runtime.InteropServices.ComWrappers.ComInterfaceDispatch* __this_native, ushort* __name_native)
    {
        global::SharedTypes.ComInterfaces.IDerived2 @this = default;
        string name = default;
        int __retVal = default;
        try
        {
            // Unmarshal - Convert native data to managed data.
            __retVal = 0; // S_OK
            name = global::System.Runtime.InteropServices.Marshalling.Utf16StringMarshaller.ConvertToManaged(__name_native);
            @this = System.Runtime.InteropServices.ComWrappers.ComInterfaceDispatch.GetInstance<global::SharedTypes.ComInterfaces.IDerived2>(__this_native);
            @this.Foo(name);
        }
        catch (System.Exception __exception)
        {
            __retVal = System.Runtime.InteropServices.Marshalling.ExceptionAsHResultMarshaller<int>.ConvertToUnmanaged(__exception);
        }

        return __retVal;
    }
}

file unsafe partial interface InterfaceImplementation
{
    internal static void** CreateManagedVirtualFunctionTable()
    {
        void** vtable = (void**)System.Runtime.CompilerServices.RuntimeHelpers.AllocateTypeAssociatedMemory(typeof(global::SharedTypes.ComInterfaces.IDerived2), sizeof(void*) * 6);
        {
            System.Runtime.InteropServices.NativeMemory.Copy(System.Runtime.InteropServices.Marshalling.StrategyBasedComWrappers.DefaultIUnknownInterfaceDetailsStrategy.GetIUnknownDerivedDetails(typeof(global::SharedTypes.ComInterfaces.IGetAndSetInt).TypeHandle).ManagedVirtualMethodTable, vtable, (nuint)(sizeof(void*) * 5));
        }

        {
            vtable[5] = (void*)(delegate* unmanaged[MemberFunction]<System.Runtime.InteropServices.ComWrappers.ComInterfaceDispatch*, ushort*, int> )&ABI_Foo;
        }

        return vtable;
    }
}

We should be generating shadow methods for IGetAndSetInt as well as IDerived for IDerived2, not just IDerived. We should also copy the base vtable from IDerived instead of IGetAndSetInt.

Author: jkoritzinsky
Assignees: -
Labels:

bug, area-System.Runtime.InteropServices, source-generator

Milestone: 8.0.0

@jkoritzinsky
Copy link
Member Author

Looks like this was fixed by #86467

@ghost ghost locked as resolved and limited conversation to collaborators Jun 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Runtime.InteropServices bug source-generator Indicates an issue with a source generator feature
Projects
None yet
Development

No branches or pull requests

1 participant