Guaranteed unmarshal before managed call in unmanaged to managed stubs

src/libraries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/UnmanagedToManagedStubGenerator.cs

@@ -5,7 +5,6 @@
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using Microsoft.CodeAnalysis;
-using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 using static Microsoft.CodeAnalysis.CSharp.SyntaxFactory;
 
@@ -14,7 +13,6 @@ namespace Microsoft.Interop
     internal sealed class UnmanagedToManagedStubGenerator
     {
         private const string ReturnIdentifier = "__retVal";
-        private const string InvokeSucceededIdentifier = "__invokeSucceeded";
 
         private readonly BoundGenerators _marshallers;
 
@@ -60,37 +58,22 @@ public BlockSyntax GenerateStubBody(ExpressionSyntax methodToInvoke)
                 || !statements.ManagedExceptionCatchClauses.IsEmpty;
             VariableDeclarations declarations = VariableDeclarations.GenerateDeclarationsForUnmanagedToManaged(_marshallers, _context, shouldInitializeVariables);
 
-            if (!statements.GuaranteedUnmarshal.IsEmpty)
-            {
-                setupStatements.Add(MarshallerHelpers.Declare(PredefinedType(Token(SyntaxKind.BoolKeyword)), InvokeSucceededIdentifier, initializeToDefault: true));
-            }
-
             setupStatements.AddRange(declarations.Initializations);
             setupStatements.AddRange(declarations.Variables);
             setupStatements.AddRange(statements.Setup);
 
             List<StatementSyntax> tryStatements = new();
+            tryStatements.AddRange(statements.GuaranteedUnmarshal);
             tryStatements.AddRange(statements.Unmarshal);
 
             tryStatements.Add(statements.InvokeStatement);
 
-            if (!statements.GuaranteedUnmarshal.IsEmpty)
-            {
-                tryStatements.Add(ExpressionStatement(AssignmentExpression(SyntaxKind.SimpleAssignmentExpression,
-                    IdentifierName(InvokeSucceededIdentifier),
-                    LiteralExpression(SyntaxKind.TrueLiteralExpression))));
-            }
-
             tryStatements.AddRange(statements.NotifyForSuccessfulInvoke);
             tryStatements.AddRange(statements.PinnedMarshal);
             tryStatements.AddRange(statements.Marshal);
 
             List<StatementSyntax> allStatements = setupStatements;
             List<StatementSyntax> finallyStatements = new();
-            if (!statements.GuaranteedUnmarshal.IsEmpty)
-            {
-                finallyStatements.Add(IfStatement(IdentifierName(InvokeSucceededIdentifier), Block(statements.GuaranteedUnmarshal)));
-            }
 
             SyntaxList<CatchClauseSyntax> catchClauses = List(statements.ManagedExceptionCatchClauses);
 

src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/MarshallerHelpers.cs

@@ -44,11 +44,21 @@ public static ForStatementSyntax GetForLoop(ExpressionSyntax lengthExpression, s
                         IdentifierName(indexerIdentifier))));
         }
 
+        /// <summary>
+        /// <code><paramref name="typeSyntax"/> <paramref name="identifier"/> = default;</code>
+        /// or
+        /// <code><paramref name="typeSyntax"/> <paramref name="identifier"/>;</code>
+        /// </summary>
         public static LocalDeclarationStatementSyntax Declare(TypeSyntax typeSyntax, string identifier, bool initializeToDefault)
         {
             return Declare(typeSyntax, identifier, initializeToDefault ? LiteralExpression(SyntaxKind.DefaultLiteralExpression) : null);
         }
 
+        /// <summary>
+        /// <code><paramref name="typeSyntax"/> <paramref name="identifier"/> = <paramref name="identifier"/>;</code>
+        /// or
+        /// <code><paramref name="typeSyntax"/> <paramref name="identifier"/>;</code>
+        /// </summary>
         public static LocalDeclarationStatementSyntax Declare(TypeSyntax typeSyntax, string identifier, ExpressionSyntax? initializer)
         {
             VariableDeclaratorSyntax decl = VariableDeclarator(identifier);

src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/StatefulMarshallingStrategy.cs

@@ -438,7 +438,7 @@ public IEnumerable<StatementSyntax> GenerateSetupStatements(TypePositionInfo inf
                 yield return elementsSetup;
             }
             // Use the numElements local to ensure the compiler doesn't give errors for using an uninitialized variable.
-            // The value will never be used unless it has been initialized, so this is safe.
+            // The value may be used in cleanup before it has been initialized, so this is not safe.
             yield return MarshallerHelpers.SkipInitOrDefaultInit(
                 new TypePositionInfo(SpecialTypeInfo.Int32, NoMarshallingInfo.Instance)
                 {

src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/StatelessMarshallingStrategy.cs

@@ -404,7 +404,7 @@ public IEnumerable<StatementSyntax> GenerateSetupStatements(TypePositionInfo inf
                     SingletonSeparatedList(
                         VariableDeclarator(numElementsIdentifier))));
             // Use the numElements local to ensure the compiler doesn't give errors for using an uninitialized variable.
-            // The value will never be used unless it has been initialized, so this is safe.
+            // The value may be used in cleanup before it has been initialized, so this is unsafe
             yield return MarshallerHelpers.SkipInitOrDefaultInit(
                 new TypePositionInfo(SpecialTypeInfo.Int32, NoMarshallingInfo.Instance)
                 {

src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Tests/ComInterfaceGenerator.Tests.csproj

@@ -19,6 +19,7 @@
 
   <ItemGroup>
     <ProjectReference Include="..\..\gen\ComInterfaceGenerator\ComInterfaceGenerator.csproj" ReferenceOutputAssembly="false" OutputItemType="Analyzer" />
+    <ProjectReference Include="..\..\gen\LibraryImportGenerator\LibraryImportGenerator.csproj" ReferenceOutputAssembly="false" OutputItemType="Analyzer" />
     <ProjectReference Include="..\Ancillary.Interop\Ancillary.Interop.csproj" />
     <ProjectReference Include="..\TestAssets\NativeExports\NativeExports.csproj" />
     <ProjectReference Include="..\TestAssets\SharedTypes\SharedTypes.csproj" />

src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Tests/RcwAroundCcwTests.cs

@@ -116,6 +116,32 @@ public void IInterface()
             obj.GetOut(out var _);
         }
 
+        [Fact]
+        public void IStatefulFinallyMarshalling()
+        {
+            var obj = CreateWrapper<StatefulFinallyMarshalling, IStatefulFinallyMarshalling>();
+            var data = new StatefulFinallyType() { i = -10 };
+            obj.Method(data);
+            obj.MethodIn(in data);
+            obj.MethodOut(out _);
+            obj.MethodRef(ref data);
+            _ = obj.Return();
+            _ = obj.ReturnPreserveSig();
+        }
+
+        [Fact]
+        public void IStatelessFinallyMarshalling()
+        {
+            var obj = CreateWrapper<StatelessFinallyMarshalling, IStatelessFinallyMarshalling>();
+            var data = new StatelessFinallyType() { I = -10 };
+            obj.Method(data);
+            obj.MethodIn(in data);
+            obj.MethodOut(out _);
+            obj.MethodRef(ref data);
+            _ = obj.Return();
+            _ = obj.ReturnPreserveSig();
+        }
+
         [Fact]
         [ActiveIssue("https://github.com/dotnet/runtime/issues/89747")]
         public void ICollectionMarshallingFails()

src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/ComInterfaces/IStatefulFinallyMarshalling.cs

@@ -19,38 +19,70 @@ internal partial interface IStatefulFinallyMarshalling
         [PreserveSig]
         StatefulFinallyType ReturnPreserveSig();
     }
+    [GeneratedComClass]
+    internal partial class StatefulFinallyMarshalling : IStatefulFinallyMarshalling
+    {
+        public void Method(StatefulFinallyType param)
+        {
+            _ = param.i;
+        }
+        public void MethodIn(in StatefulFinallyType param)
+        {
+            _ = param.i;
+        }
+        public void MethodOut(out StatefulFinallyType param)
+        {
+            param = new StatefulFinallyType() { i = 42 };
+        }
+        public void MethodRef(ref StatefulFinallyType param)
+        {
+            _ = param.i;
+            param = new StatefulFinallyType() { i = 99 };
+        }
+        public StatefulFinallyType Return()
+            => new StatefulFinallyType() { i = 8 };
+        public StatefulFinallyType ReturnPreserveSig()
+            => new StatefulFinallyType() { i = 3 };
+    }
 
     [NativeMarshalling(typeof(StatefulFinallyTypeMarshaller))]
     internal class StatefulFinallyType
     {
+        public int i;
+    }
+
+    internal struct StatefulFinallyNative
+    {
+        public int i;
     }
 
     [CustomMarshaller(typeof(StatefulFinallyType), MarshalMode.Default, typeof(StatefulFinallyTypeMarshaller))]
     internal struct StatefulFinallyTypeMarshaller
     {
+        int managed_i;
+        int unmanaged_i;
         public void FromManaged(StatefulFinallyType managed)
         {
-            throw new NotImplementedException();
+            managed_i = managed.i;
         }
 
-        public nint ToUnmanaged()
+        public StatefulFinallyNative ToUnmanaged()
         {
-            throw new NotImplementedException();
+            return new StatefulFinallyNative() { i = this.managed_i };
         }
 
-        public void FromUnmanaged(nint unmanaged)
+        public void FromUnmanaged(StatefulFinallyNative unmanaged)
         {
-            throw new NotImplementedException();
+            unmanaged_i = unmanaged.i;
         }
 
         public StatefulFinallyType ToManagedFinally()
         {
-            throw new NotImplementedException();
+            return new StatefulFinallyType() { i = unmanaged_i };
         }
 
         public void Free()
         {
-            throw new NotImplementedException();
         }
     }
 }

src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/ComInterfaces/IStatelessFinallyMarshalling.cs

@@ -11,10 +11,10 @@ namespace SharedTypes.ComInterfaces
     [Guid("4732FA5D-C105-4A26-87A7-58DCEDD4A9B3")]
     internal partial interface IStatelessFinallyMarshalling
     {
-        void Method([MarshalUsing(CountElementName = nameof(size))] StatelessFinallyType param, int size);
-        void MethodIn([MarshalUsing(CountElementName = nameof(size))] in StatelessFinallyType param, int size);
-        void MethodOut([MarshalUsing(CountElementName = nameof(size))] out StatelessFinallyType param, int size);
-        void MethodRef([MarshalUsing(CountElementName = nameof(size))] ref StatelessFinallyType param, int size);
+        void Method(StatelessFinallyType param);
+        void MethodIn(in StatelessFinallyType param);
+        void MethodOut(out StatelessFinallyType param);
+        void MethodRef(ref StatelessFinallyType param);
         StatelessFinallyType Return();
         [PreserveSig]
         StatelessFinallyType ReturnPreserveSig();
@@ -23,12 +23,12 @@ internal partial interface IStatelessFinallyMarshalling
     [GeneratedComClass]
     internal partial class StatelessFinallyMarshalling : IStatelessFinallyMarshalling
     {
-        public void Method([MarshalUsing(CountElementName = "size")] StatelessFinallyType param, int size) { }
-        public void MethodIn([MarshalUsing(CountElementName = "size")] in StatelessFinallyType param, int size) { }
-        public void MethodOut([MarshalUsing(CountElementName = "size")] out StatelessFinallyType param, int size) { param = new StatelessFinallyType { I = 42 }; }
-        public void MethodRef([MarshalUsing(CountElementName = "size")] ref StatelessFinallyType param, int size) { param = new StatelessFinallyType { I = 200 }; }
-        public StatelessFinallyType Return() => throw new NotImplementedException();
-        public StatelessFinallyType ReturnPreserveSig() => throw new NotImplementedException();
+        public void Method(StatelessFinallyType param) { _ = param.I; }
+        public void MethodIn(in StatelessFinallyType param) { _ = param.I; }
+        public void MethodOut(out StatelessFinallyType param) { param = new StatelessFinallyType { I = 42 }; }
+        public void MethodRef(ref StatelessFinallyType param) { _ = param.I; param = new StatelessFinallyType { I = 200 }; }
+        public StatelessFinallyType Return() => new StatelessFinallyType { I = 200 };
+        public StatelessFinallyType ReturnPreserveSig() => new StatelessFinallyType { I = 200 };
     }
 
     [NativeMarshalling(typeof(StatelessFinallyTypeMarshaller))]
@@ -37,14 +37,19 @@ internal class StatelessFinallyType
         public int I;
     }
 
+    internal struct StatelessFinallyNative
+    {
+        public int i;
+    }
+
     [CustomMarshaller(typeof(StatelessFinallyType), MarshalMode.Default, typeof(StatelessFinallyTypeMarshaller))]
     internal static class StatelessFinallyTypeMarshaller
     {
         public static int FreeCount { get; private set; }
-        public static nint ConvertToUnmanaged(StatelessFinallyType managed) => managed.I;
+        public static StatelessFinallyNative ConvertToUnmanaged(StatelessFinallyType managed) => new StatelessFinallyNative() { i = managed.I };
 
-        public static StatelessFinallyType ConvertToManagedFinally(nint unmanaged) => new StatelessFinallyType { I = (int)unmanaged };
+        public static StatelessFinallyType ConvertToManagedFinally(StatelessFinallyNative unmanaged) => new StatelessFinallyType { I = unmanaged.i };
 
-        public static void Free(nint unmanaged) => FreeCount++;
+        public static void Free(StatelessFinallyNative unmanaged) => FreeCount++;
     }
 }