Skip to content

ComInterfaceGenerator bugs when 2 interfaces inherit from the same interface #111890

@m-celikba

Description

@m-celikba

Description

Consider the following code:

    [GeneratedComInterface, Guid("427CE02F-515A-4338-BE30-F70DA3ADAEC1")]
    public unsafe partial interface IRoot 
    {
        void MyMethod0();
    }

    [GeneratedComInterface, Guid("1AEE856E-6501-43C8-843B-8210B57DFD27")]
    public unsafe partial interface ItfA : IRoot
    {
        void MyMethod1();
    }

    [GeneratedComInterface, Guid("3264857C-BB23-418D-9528-1E4226F2FEA4")]
    public unsafe partial interface ItfB : IRoot
    {
        void MyMethod2();
    }

when IRoot is defined int the same assembly it works.

However when it is defined in another assembly, I get:

 CSC : warning CS8785: Generator 'ComInterfaceGenerator' failed to generate source. It will not contribute to the output and compilation errors may occur as a result. Exception was of type 'ArgumentException' with message 'An item with the same key has already been added.'.

Detailed msbuild output:

CSC : warning CS8785: Generator 'ComInterfaceGenerator' failed to generate source. It will not contribute to the output and compilation errors may occur as a result. Exception was of type 'ArgumentException' with message 'An item with the same key has already been added.'.
1>    System.ArgumentException: An item with the same key has already been added.
1>    at System.ThrowHelper.ThrowArgumentException(ExceptionResource resource)
1>    at System.Collections.Generic.Dictionary`2.Insert(TKey key, TValue value, Boolean add)
1>    at Microsoft.Interop.ComInterfaceContext.GetContexts(ImmutableArray`1 data, CancellationToken _)
1>    at Microsoft.CodeAnalysis.TransformNode`2.UpdateStateTable(Builder builder, NodeStateTable`1 previousTable, CancellationToken cancellationToken)

Debugging this shows:

Image

Reproduction Steps

see above

Expected behavior

no exception raised

Actual behavior

see description the following exception is raised System.ArgumentException: An item with the same key has already been added.

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

Activity

added
needs-area-labelAn area label is needed to ensure this gets routed to the appropriate area owners
on Jan 28, 2025
added
source-generatorIndicates an issue with a source generator feature
and removed
needs-area-labelAn area label is needed to ensure this gets routed to the appropriate area owners
on Jan 28, 2025
dotnet-policy-service

dotnet-policy-service commented on Jan 28, 2025

@dotnet-policy-service
Contributor

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

AaronRobinsonMSFT

AaronRobinsonMSFT commented on Jan 28, 2025

@AaronRobinsonMSFT
Member

@jkoritzinsky @jtschuster This seems odd. The number of interfaces that require other implementations shouldn't matter. Actually is this one of the issues we talked about where the base interface is in another assembly?

m-celikba

m-celikba commented on Jan 28, 2025

@m-celikba
Author

@AaronRobinsonMSFT
If the base interface is in another assembly and only one interface inherits from it, it works.
If the base interface is in another assembly and more than one interface inherits from it, it fails with ArgumentException

jkoritzinsky

jkoritzinsky commented on Jan 28, 2025

@jkoritzinsky
Member

This is a real bug in the "different aseembly" limited support we added.

Basically, that loop is the only place we import the base interfaces into our view. We should change that to do a try-add instead of a throwing Add.

m-celikba

m-celikba commented on Jan 28, 2025

@m-celikba
Author

@jkoritzinsky I have a local fix. I'd like to move on locally with this fix.
I can build the generator project but have 2 problems: the assembly version numbers are messed up and I'm wondering what's the best way to override one of the runtime dll (in this case the generator dll) in my projects.
Where can I learn more about these ?

jkoritzinsky

jkoritzinsky commented on Jan 28, 2025

@jkoritzinsky
Member

There's no good mechanism to override a generator dll shipped with the sdk with a local one. You would have to add some custom MSBuild targets to remove the one provided by the SDK and add your own.

self-assigned this
on Jan 28, 2025
removed
untriagedNew issue has not been triaged by the area owner
on Jan 28, 2025
added
in-prThere is an active PR which will close this issue when it is merged
on Jan 28, 2025
m-celikba

m-celikba commented on Jan 29, 2025

@m-celikba
Author

@jkoritzinsky @jtschuster there's another issue. Consider my example above.
when IRoot is in the same assembly as ItfA, the vtable length in CreateManagedVirtualFunctionTable() for ITfA has size 3+1+1=5.
when IRoot is in a different assembly than ItfA, the vtable length in CreateManagedVirtualFunctionTable() for ITfA has size 3+1=4.
It looks like the vtable is missing room for the base interface IRoot's methods.
(in the line void** vtable = (void**)global::System.Runtime.CompilerServices.RuntimeHelpers.AllocateTypeAssociatedMemory).
The vtable indices are correct though.
This causes a buffer overrun as the code tries to write to a smaller array.

Please let me know if you want me to file another more detailed bug.

m-celikba

m-celikba commented on Jan 30, 2025

@m-celikba
Author

I should have said that the previous buffer overflow issue happens when/if the original issue is fixed.

If inheriting source generated COM interfaces from interfaces in another assembly is supported (I know there's a warning but as long as you know what you are doing this is a proper use case), then it should just work.

void** vtable = (void**)global::System.Runtime.CompilerServices.RuntimeHelpers.AllocateTypeAssociatedMemory(typeof(<itf>), sizeof(void*) * <number_of_methods_on_this_interface_and_its_base>);
{
    global::System.Runtime.InteropServices.NativeMemory.Copy(
        global::System.Runtime.InteropServices.Marshalling.StrategyBasedComWrappers.DefaultIUnknownInterfaceDetailsStrategy.GetIUnknownDerivedDetails(typeof(global::test_build_lib.IRoot).TypeHandle).ManagedVirtualMethodTable, vtable, 
        (nuint)(sizeof(void*) * <number_of_methods_on_the_base_only>));
}

If this is not possible, then another simple way could be to allow passing a vtable offset.
[GeneratedComInterface(VTableOffset=<number_of_methods_on_the_base_only>)]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

area-System.Runtime.InteropServicesin-prThere is an active PR which will close this issue when it is mergedsource-generatorIndicates an issue with a source generator feature

Type

No type

Projects

Status

No status

Milestone

No milestone

Relationships

None yet

Participants

@jkoritzinsky@huoyaoyuan@AaronRobinsonMSFT@jtschuster@m-celikba

Issue actions

    ComInterfaceGenerator bugs when 2 interfaces inherit from the same interface · Issue #111890 · dotnet/runtime