[ComInterfaceGenerator] ManagedToUnmanagedStubs with 'out' or return value size may not have correct size or array pointer when cleaning up

[jtschuster]

In the example below, if the COM method returns a failure, the stub will throw before assigning the numElements, and the cleanup stage will create a span with an undefined pointer and undefined size.

    int[] global::SharedTypes.ComInterfaces.MarshallingFails.ICollectionMarshallingFails.GetConstSize()
    {
        var(__this, __vtable_native) = ((System.Runtime.InteropServices.Marshalling.IUnmanagedVirtualMethodTableProvider)this).GetVirtualMethodTableInfoForKey(typeof(global::SharedTypes.ComInterfaces.MarshallingFails.ICollectionMarshallingFails));
        int[] __retVal = default;
        nint* __retVal_native = default;
        int __invokeRetVal = default;
        // Setup - Perform required setup.
        int __retVal_native__numElements;
        System.Runtime.CompilerServices.Unsafe.SkipInit(out __retVal_native__numElements);
        try
        {
            {
                __invokeRetVal = ((delegate* unmanaged[MemberFunction]<void*, nint**, int> )__vtable_native[3])(__this, &__retVal_native);
            }

            System.GC.KeepAlive(this);
            // Unmarshal - Convert native data to managed data.
            System.Runtime.InteropServices.Marshal.ThrowExceptionForHR(__invokeRetVal);
            __retVal_native__numElements = 10;
            __retVal = global::System.Runtime.InteropServices.Marshalling.ArrayMarshaller<int, nint>.AllocateContainerForManagedElements(__retVal_native, __retVal_native__numElements);
            {
                System.ReadOnlySpan<nint> __retVal_native__nativeSpan = global::System.Runtime.InteropServices.Marshalling.ArrayMarshaller<int, nint>.GetUnmanagedValuesSource(__retVal_native, __retVal_native__numElements);
                System.Span<int> __retVal_native__managedSpan = global::System.Runtime.InteropServices.Marshalling.ArrayMarshaller<int, nint>.GetManagedValuesDestination(__retVal);
                for (int __i0 = 0; __i0 < __retVal_native__numElements; ++__i0)
                {
                    __retVal_native__managedSpan[__i0] = global::SharedTypes.ComInterfaces.MarshallingFails.ThrowOn4thElementMarshalled.ConvertToManaged(__retVal_native__nativeSpan[__i0]);
                }
            }
        }
        finally
        {
            // Cleanup - Perform required cleanup.
            {
                System.ReadOnlySpan<nint> __retVal_native__nativeSpan = global::System.Runtime.InteropServices.Marshalling.ArrayMarshaller<int, nint>.GetUnmanagedValuesDestination(__retVal_native, __retVal_native__numElements);
                for (int __i0 = 0; __i0 < __retVal_native__nativeSpan.Length; ++__i0)
                {
                    global::SharedTypes.ComInterfaces.MarshallingFails.ThrowOn4thElementMarshalled.Free(__retVal_native__nativeSpan[__i0]);
                }
            }

            __retVal_native__numElements = 10;
            global::System.Runtime.InteropServices.Marshalling.ArrayMarshaller<int, nint>.Free(__retVal_native);
        }

        return __retVal;
    }

[ghost]

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

Issue Details

---

In the example below, if the COM method returns a failure, the stub will throw before assigning the numElements, and the cleanup stage will create a span with an undefined pointer and undefined size.

    int[] global::SharedTypes.ComInterfaces.MarshallingFails.ICollectionMarshallingFails.GetConstSize()
    {
        var(__this, __vtable_native) = ((System.Runtime.InteropServices.Marshalling.IUnmanagedVirtualMethodTableProvider)this).GetVirtualMethodTableInfoForKey(typeof(global::SharedTypes.ComInterfaces.MarshallingFails.ICollectionMarshallingFails));
        int[] __retVal = default;
        nint* __retVal_native = default;
        int __invokeRetVal = default;
        // Setup - Perform required setup.
        int __retVal_native__numElements;
        System.Runtime.CompilerServices.Unsafe.SkipInit(out __retVal_native__numElements);
        try
        {
            {
                __invokeRetVal = ((delegate* unmanaged[MemberFunction]<void*, nint**, int> )__vtable_native[3])(__this, &__retVal_native);
            }

            System.GC.KeepAlive(this);
            // Unmarshal - Convert native data to managed data.
            System.Runtime.InteropServices.Marshal.ThrowExceptionForHR(__invokeRetVal);
            __retVal_native__numElements = 10;
            __retVal = global::System.Runtime.InteropServices.Marshalling.ArrayMarshaller<int, nint>.AllocateContainerForManagedElements(__retVal_native, __retVal_native__numElements);
            {
                System.ReadOnlySpan<nint> __retVal_native__nativeSpan = global::System.Runtime.InteropServices.Marshalling.ArrayMarshaller<int, nint>.GetUnmanagedValuesSource(__retVal_native, __retVal_native__numElements);
                System.Span<int> __retVal_native__managedSpan = global::System.Runtime.InteropServices.Marshalling.ArrayMarshaller<int, nint>.GetManagedValuesDestination(__retVal);
                for (int __i0 = 0; __i0 < __retVal_native__numElements; ++__i0)
                {
                    __retVal_native__managedSpan[__i0] = global::SharedTypes.ComInterfaces.MarshallingFails.ThrowOn4thElementMarshalled.ConvertToManaged(__retVal_native__nativeSpan[__i0]);
                }
            }
        }
        finally
        {
            // Cleanup - Perform required cleanup.
            {
                System.ReadOnlySpan<nint> __retVal_native__nativeSpan = global::System.Runtime.InteropServices.Marshalling.ArrayMarshaller<int, nint>.GetUnmanagedValuesDestination(__retVal_native, __retVal_native__numElements);
                for (int __i0 = 0; __i0 < __retVal_native__nativeSpan.Length; ++__i0)
                {
                    global::SharedTypes.ComInterfaces.MarshallingFails.ThrowOn4thElementMarshalled.Free(__retVal_native__nativeSpan[__i0]);
                }
            }

            __retVal_native__numElements = 10;
            global::System.Runtime.InteropServices.Marshalling.ArrayMarshaller<int, nint>.Free(__retVal_native);
        }

        return __retVal;
    }

Author:	jtschuster
Assignees:	-
Labels:	area-System.Runtime.InteropServices
Milestone:	-

[jkoritzinsky]

It looks like we assign the constant "numElements" after the loop. If we move that before the cleanup loop, then this should work okay.

[jkoritzinsky]

For non-constant scenarios, this is more difficult though, but I think we should be able to get it to work.