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

[ComInterfaceGenerator] PreserveSig with collection return type will not properly marshal back to caller #89893

Closed
jtschuster opened this issue Aug 2, 2023 · 1 comment · Fixed by #90056

Comments

@jtschuster
Copy link
Member

This is probably not a realistic situation, but we may want to fix it or warn when the [PreserveSig] return value unmarshaller doesn't take an int as the unmanaged type.

This method definition

        [PreserveSig]
        [return: MarshalUsing(CountElementName = nameof(size))]
        StatelessCollection<StatelessType> ReturnPreserveSig(int size);

will generate this unmanaged to managed method

    internal static global::SharedTypes.ComInterfaces.NativeCollection<global::SharedTypes.ComInterfaces.StatelessType> ABI_ReturnPreserveSig(System.Runtime.InteropServices.ComWrappers.ComInterfaceDispatch* __this_native, int size)
    {
        global::SharedTypes.ComInterfaces.IStatelessCollectionStatelessElement @this = default;
        global::SharedTypes.ComInterfaces.StatelessCollection<global::SharedTypes.ComInterfaces.StatelessType> __retVal = default;
        global::SharedTypes.ComInterfaces.NativeCollection<global::SharedTypes.ComInterfaces.StatelessType> __retVal_native = default;
        // Setup - Perform required setup.
        int __retVal_native__numElements;
        System.Runtime.CompilerServices.Unsafe.SkipInit(out __retVal_native__numElements);
        try
        {
            // Unmarshal - Convert native data to managed data.
            @this = System.Runtime.InteropServices.ComWrappers.ComInterfaceDispatch.GetInstance<global::SharedTypes.ComInterfaces.IStatelessCollectionStatelessElement>(__this_native);
            __retVal = @this.ReturnPreserveSig(size);
            // Marshal - Convert managed data to native data.
            __retVal_native = global::SharedTypes.ComInterfaces.StatelessCollectionMarshaller<global::SharedTypes.ComInterfaces.StatelessType, nint>.ManagedToUnmanaged.AllocateContainerForUnmanagedElements(__retVal, out __retVal_native__numElements);
            {
                System.ReadOnlySpan<global::SharedTypes.ComInterfaces.StatelessType> __retVal_native__managedSpan = global::SharedTypes.ComInterfaces.StatelessCollectionMarshaller<global::SharedTypes.ComInterfaces.StatelessType, nint>.ManagedToUnmanaged.GetManagedValuesSource(__retVal);
                System.Span<nint> __retVal_native__nativeSpan = global::SharedTypes.ComInterfaces.StatelessCollectionMarshaller<global::SharedTypes.ComInterfaces.StatelessType, nint>.ManagedToUnmanaged.GetUnmanagedValuesDestination(__retVal_native, __retVal_native__numElements);
                __retVal_native__nativeSpan.Clear();
                ; // Should be a for loop here that marshals elements
            }
        }
        catch (System.Exception __exception)
        {
            __retVal_native = System.Runtime.InteropServices.Marshalling.ExceptionAsDefaultMarshaller<global::SharedTypes.ComInterfaces.NativeCollection<global::SharedTypes.ComInterfaces.StatelessType>>.ConvertToUnmanaged(__exception);
        }

        return __retVal_native;
    }

For comparison, a regular managed return value

        [return: MarshalUsing(CountElementName = nameof(size))]
        StatelessCollection<StatelessType> Return(int size);

Generates this unmanaged to managed stub

    internal static int ABI_Return(System.Runtime.InteropServices.ComWrappers.ComInterfaceDispatch* __this_native, int size, global::SharedTypes.ComInterfaces.NativeCollection<global::SharedTypes.ComInterfaces.StatelessType>* __invokeRetValUnmanaged__param)
    {
        global::SharedTypes.ComInterfaces.IStatelessCollectionStatelessElement @this = default;
        ref global::SharedTypes.ComInterfaces.NativeCollection<global::SharedTypes.ComInterfaces.StatelessType> __invokeRetValUnmanaged = ref *__invokeRetValUnmanaged__param;
        global::SharedTypes.ComInterfaces.StatelessCollection<global::SharedTypes.ComInterfaces.StatelessType> __invokeRetVal = default;
        int __retVal = default;
        // Setup - Perform required setup.
        int __invokeRetValUnmanaged__numElements;
        System.Runtime.CompilerServices.Unsafe.SkipInit(out __invokeRetValUnmanaged__numElements);
        try
        {
            // Unmarshal - Convert native data to managed data.
            __retVal = 0; // S_OK
            @this = System.Runtime.InteropServices.ComWrappers.ComInterfaceDispatch.GetInstance<global::SharedTypes.ComInterfaces.IStatelessCollectionStatelessElement>(__this_native);
            __invokeRetVal = @this.Return(size);
            // Marshal - Convert managed data to native data.
            __invokeRetValUnmanaged = global::SharedTypes.ComInterfaces.StatelessCollectionMarshaller<global::SharedTypes.ComInterfaces.StatelessType, nint>.ManagedToUnmanaged.AllocateContainerForUnmanagedElements(__invokeRetVal, out __invokeRetValUnmanaged__numElements);
            {
                System.ReadOnlySpan<global::SharedTypes.ComInterfaces.StatelessType> __invokeRetValUnmanaged__managedSpan = global::SharedTypes.ComInterfaces.StatelessCollectionMarshaller<global::SharedTypes.ComInterfaces.StatelessType, nint>.ManagedToUnmanaged.GetManagedValuesSource(__invokeRetVal);
                System.Span<nint> __invokeRetValUnmanaged__nativeSpan = global::SharedTypes.ComInterfaces.StatelessCollectionMarshaller<global::SharedTypes.ComInterfaces.StatelessType, nint>.ManagedToUnmanaged.GetUnmanagedValuesDestination(__invokeRetValUnmanaged, __invokeRetValUnmanaged__numElements);
                __invokeRetValUnmanaged__nativeSpan.Clear();
                for (int __i0 = 0; __i0 < __invokeRetValUnmanaged__managedSpan.Length; ++__i0)
                {
                    __invokeRetValUnmanaged__nativeSpan[__i0] = global::SharedTypes.ComInterfaces.StatelessTypeMarshaller.ManagedToUnmanaged.ConvertToUnmanaged(__invokeRetValUnmanaged__managedSpan[__i0]);
                }
            }
        }
        catch (System.Exception __exception)
        {
            __retVal = System.Runtime.InteropServices.Marshalling.ExceptionAsHResultMarshaller<int>.ConvertToUnmanaged(__exception);
        }

        return __retVal;
    }
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Aug 2, 2023
@ghost
Copy link

ghost commented Aug 2, 2023

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

Issue Details

This is probably not a realistic situation, but we may want to fix it or warn when the [PreserveSig] return value unmarshaller doesn't take an int as the unmanaged type.

This method definition

        [PreserveSig]
        [return: MarshalUsing(CountElementName = nameof(size))]
        StatelessCollection<StatelessType> ReturnPreserveSig(int size);

will generate this unmanaged to managed method

    internal static global::SharedTypes.ComInterfaces.NativeCollection<global::SharedTypes.ComInterfaces.StatelessType> ABI_ReturnPreserveSig(System.Runtime.InteropServices.ComWrappers.ComInterfaceDispatch* __this_native, int size)
    {
        global::SharedTypes.ComInterfaces.IStatelessCollectionStatelessElement @this = default;
        global::SharedTypes.ComInterfaces.StatelessCollection<global::SharedTypes.ComInterfaces.StatelessType> __retVal = default;
        global::SharedTypes.ComInterfaces.NativeCollection<global::SharedTypes.ComInterfaces.StatelessType> __retVal_native = default;
        // Setup - Perform required setup.
        int __retVal_native__numElements;
        System.Runtime.CompilerServices.Unsafe.SkipInit(out __retVal_native__numElements);
        try
        {
            // Unmarshal - Convert native data to managed data.
            @this = System.Runtime.InteropServices.ComWrappers.ComInterfaceDispatch.GetInstance<global::SharedTypes.ComInterfaces.IStatelessCollectionStatelessElement>(__this_native);
            __retVal = @this.ReturnPreserveSig(size);
            // Marshal - Convert managed data to native data.
            __retVal_native = global::SharedTypes.ComInterfaces.StatelessCollectionMarshaller<global::SharedTypes.ComInterfaces.StatelessType, nint>.ManagedToUnmanaged.AllocateContainerForUnmanagedElements(__retVal, out __retVal_native__numElements);
            {
                System.ReadOnlySpan<global::SharedTypes.ComInterfaces.StatelessType> __retVal_native__managedSpan = global::SharedTypes.ComInterfaces.StatelessCollectionMarshaller<global::SharedTypes.ComInterfaces.StatelessType, nint>.ManagedToUnmanaged.GetManagedValuesSource(__retVal);
                System.Span<nint> __retVal_native__nativeSpan = global::SharedTypes.ComInterfaces.StatelessCollectionMarshaller<global::SharedTypes.ComInterfaces.StatelessType, nint>.ManagedToUnmanaged.GetUnmanagedValuesDestination(__retVal_native, __retVal_native__numElements);
                __retVal_native__nativeSpan.Clear();
                ; // Should be a for loop here that marshals elements
            }
        }
        catch (System.Exception __exception)
        {
            __retVal_native = System.Runtime.InteropServices.Marshalling.ExceptionAsDefaultMarshaller<global::SharedTypes.ComInterfaces.NativeCollection<global::SharedTypes.ComInterfaces.StatelessType>>.ConvertToUnmanaged(__exception);
        }

        return __retVal_native;
    }

For comparison, a regular managed return value

        [return: MarshalUsing(CountElementName = nameof(size))]
        StatelessCollection<StatelessType> Return(int size);

Generates this unmanaged to managed stub

    internal static int ABI_Return(System.Runtime.InteropServices.ComWrappers.ComInterfaceDispatch* __this_native, int size, global::SharedTypes.ComInterfaces.NativeCollection<global::SharedTypes.ComInterfaces.StatelessType>* __invokeRetValUnmanaged__param)
    {
        global::SharedTypes.ComInterfaces.IStatelessCollectionStatelessElement @this = default;
        ref global::SharedTypes.ComInterfaces.NativeCollection<global::SharedTypes.ComInterfaces.StatelessType> __invokeRetValUnmanaged = ref *__invokeRetValUnmanaged__param;
        global::SharedTypes.ComInterfaces.StatelessCollection<global::SharedTypes.ComInterfaces.StatelessType> __invokeRetVal = default;
        int __retVal = default;
        // Setup - Perform required setup.
        int __invokeRetValUnmanaged__numElements;
        System.Runtime.CompilerServices.Unsafe.SkipInit(out __invokeRetValUnmanaged__numElements);
        try
        {
            // Unmarshal - Convert native data to managed data.
            __retVal = 0; // S_OK
            @this = System.Runtime.InteropServices.ComWrappers.ComInterfaceDispatch.GetInstance<global::SharedTypes.ComInterfaces.IStatelessCollectionStatelessElement>(__this_native);
            __invokeRetVal = @this.Return(size);
            // Marshal - Convert managed data to native data.
            __invokeRetValUnmanaged = global::SharedTypes.ComInterfaces.StatelessCollectionMarshaller<global::SharedTypes.ComInterfaces.StatelessType, nint>.ManagedToUnmanaged.AllocateContainerForUnmanagedElements(__invokeRetVal, out __invokeRetValUnmanaged__numElements);
            {
                System.ReadOnlySpan<global::SharedTypes.ComInterfaces.StatelessType> __invokeRetValUnmanaged__managedSpan = global::SharedTypes.ComInterfaces.StatelessCollectionMarshaller<global::SharedTypes.ComInterfaces.StatelessType, nint>.ManagedToUnmanaged.GetManagedValuesSource(__invokeRetVal);
                System.Span<nint> __invokeRetValUnmanaged__nativeSpan = global::SharedTypes.ComInterfaces.StatelessCollectionMarshaller<global::SharedTypes.ComInterfaces.StatelessType, nint>.ManagedToUnmanaged.GetUnmanagedValuesDestination(__invokeRetValUnmanaged, __invokeRetValUnmanaged__numElements);
                __invokeRetValUnmanaged__nativeSpan.Clear();
                for (int __i0 = 0; __i0 < __invokeRetValUnmanaged__managedSpan.Length; ++__i0)
                {
                    __invokeRetValUnmanaged__nativeSpan[__i0] = global::SharedTypes.ComInterfaces.StatelessTypeMarshaller.ManagedToUnmanaged.ConvertToUnmanaged(__invokeRetValUnmanaged__managedSpan[__i0]);
                }
            }
        }
        catch (System.Exception __exception)
        {
            __retVal = System.Runtime.InteropServices.Marshalling.ExceptionAsHResultMarshaller<int>.ConvertToUnmanaged(__exception);
        }

        return __retVal;
    }
Author: jtschuster
Assignees: -
Labels:

area-System.Runtime.InteropServices

Milestone: -

@jtschuster jtschuster removed the untriaged New issue has not been triaged by the area owner label Aug 2, 2023
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Aug 4, 2023
jkoritzinsky added a commit to jkoritzinsky/runtime that referenced this issue Aug 7, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 8, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Sep 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant