From cda166acac1577f3f64bd2e4e962eb19595cc6c6 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Fri, 30 Jan 2026 16:12:33 -0800 Subject: [PATCH 1/9] Respect protected regions when unpinning Adjust string and Type marshalling rewrites to handle unpinning correctly when earlier parameters introduce protected regions. The RewriteBodyForTypeOfString and RewriteBodyForTypeOfType helpers now take the MethodDefinition and emit either an inline unpin sequence or a separate finally-protected unpin region (matching Roslyn) depending on whether earlier parameters require a protected region. Added a nop try-start label, conditional exception handler emission, and the HasProtectedRegionBeforeParameterIndex helper to detect when a dedicated finally is needed. This prevents invalid IL layout when other parameters use leave.s and ensures pinned locals are unpinned safely. --- .../InteropMethodRewriter.NativeParameter.cs | 135 ++++++++++++++++-- 1 file changed, 125 insertions(+), 10 deletions(-) diff --git a/src/WinRT.Interop.Generator/Rewriters/InteropMethodRewriter.NativeParameter.cs b/src/WinRT.Interop.Generator/Rewriters/InteropMethodRewriter.NativeParameter.cs index 117fbf3d3..b0d01e63d 100644 --- a/src/WinRT.Interop.Generator/Rewriters/InteropMethodRewriter.NativeParameter.cs +++ b/src/WinRT.Interop.Generator/Rewriters/InteropMethodRewriter.NativeParameter.cs @@ -136,6 +136,7 @@ public static void RewriteMethod( else if (parameterType.IsTypeOfString()) { RewriteBodyForTypeOfString( + method: method, body: body, tryMarker: tryMarker, loadMarker: loadMarker, @@ -147,6 +148,7 @@ public static void RewriteMethod( else if (parameterType.IsTypeOfType(interopReferences)) { RewriteBodyForTypeOfType( + method: method, body: body, tryMarker: tryMarker, loadMarker: loadMarker, @@ -270,8 +272,10 @@ private static void RewriteBody( } /// + /// The target method to perform two-pass code generation on. /// The target body to perform two-pass code generation on. private static void RewriteBodyForTypeOfString( + MethodDefinition method, CilMethodBody body, CilInstruction tryMarker, CilInstruction loadMarker, @@ -293,6 +297,7 @@ private static void RewriteBodyForTypeOfString( body.LocalVariables.Add(loc_2_length); // Prepare the jump labels + CilInstruction nop_tryStart = new(Nop); CilInstruction ldarg_pinning = CilInstruction.CreateLdarg(parameterIndex); CilInstruction ldarg_lengthNullCheck = CilInstruction.CreateLdarg(parameterIndex); CilInstruction ldarg_getLength = CilInstruction.CreateLdarg(parameterIndex); @@ -302,6 +307,7 @@ private static void RewriteBodyForTypeOfString( body.Instructions.ReferenceReplaceRange(tryMarker, [ // fixed (char* p = value) { } + nop_tryStart, CilInstruction.CreateLdarg(parameterIndex), new CilInstruction(Brtrue_S, ldarg_pinning.CreateLabel()), new CilInstruction(Ldc_I4_0), @@ -333,16 +339,57 @@ private static void RewriteBodyForTypeOfString( new CilInstruction(Ldloca_S, loc_1_hstringReference), new CilInstruction(Call, interopReferences.HStringReferenceget_HString.Import(module))]); - // Unpin the local (just assign 'null' to it) - body.Instructions.ReferenceReplaceRange(finallyMarker, [ - new CilInstruction(Ldc_I4_0), - new CilInstruction(Conv_U), - CilInstruction.CreateStloc(loc_0_pinnedString, body)]); + // We need to emit code to unpin the local (matching what Roslyn does), but we need to consider whether other parameters + // in this same method will be using a protected region. If there aren't any, we can just emit the instructions to unpin + // the local inline, without a protected region (we just want to unpin, we don't have any resources to release). However, + // if there's a parameter before this one that has a protected region, it means that the inner-most protected region would + // have a 'leave.s' instruction to jump to the 'ret' at the end of the method. If that's the case, we can't just emit some + // additional instructions here, as they would end up being in that same protected region, but after the 'leave.s', which + // is not valid. So to work around that (like Roslyn does), we use a protected region for to unpin as well. With that change, + // the 'leave.s' will remain the last instruction in the inner-most protected region, and then following that there will + // be the instructions to unpin, in their own 'finally' handler, which then have their own 'endfinally' after them. + if (HasProtectedRegionBeforeParameterIndex(method, parameterIndex, interopReferences)) + { + // Additional jump labels just for this protected region + CilInstruction ldc_i4_0_finallyStart = new(Ldc_I4_0); + CilInstruction nop_finallyEnd = new(Nop); + + // Unpin the local (just assign 'null' to it) + body.Instructions.ReferenceReplaceRange(finallyMarker, [ + ldc_i4_0_finallyStart, + new CilInstruction(Conv_U), + CilInstruction.CreateStloc(loc_0_pinnedString, body), + new CilInstruction(Endfinally), + nop_finallyEnd]); + + // Setup the protected region to unpin the local + body.ExceptionHandlers.Add(new CilExceptionHandler + { + HandlerType = CilExceptionHandlerType.Finally, + TryStart = nop_tryStart.CreateLabel(), + TryEnd = ldc_i4_0_finallyStart.CreateLabel(), + HandlerStart = ldc_i4_0_finallyStart.CreateLabel(), + HandlerEnd = nop_finallyEnd.CreateLabel() + }); + } + else + { + // We don't be needing this 'nop', so we can remove it + _ = body.Instructions.ReferenceRemove(nop_tryStart); + + // Unpin the local (same as above, but without the protected region) + body.Instructions.ReferenceReplaceRange(finallyMarker, [ + new CilInstruction(Ldc_I4_0), + new CilInstruction(Conv_U), + CilInstruction.CreateStloc(loc_0_pinnedString, body)]); + } } /// + /// The target method to perform two-pass code generation on. /// The target body to perform two-pass code generation on. private static void RewriteBodyForTypeOfType( + MethodDefinition method, CilMethodBody body, CilInstruction tryMarker, CilInstruction loadMarker, @@ -360,8 +407,11 @@ private static void RewriteBodyForTypeOfType( body.LocalVariables.Add(loc_0_typeReference); body.LocalVariables.Add(loc_1_pinnedTypeReference); + CilInstruction nop_tryStart = new(Nop); + // Get the 'TypeReference' value and pin it body.Instructions.ReferenceReplaceRange(tryMarker, [ + nop_tryStart, CilInstruction.CreateLdarg(parameterIndex), new CilInstruction(Ldloca_S, loc_0_typeReference), new CilInstruction(Call, interopReferences.TypeMarshallerConvertToUnmanagedUnsafe.Import(module)), @@ -374,11 +424,76 @@ private static void RewriteBodyForTypeOfType( new CilInstruction(Ldloca_S, loc_0_typeReference), new CilInstruction(Call, interopReferences.TypeReferenceConvertToUnmanagedUnsafe.Import(module))]); - // Unpin the local (just assign 'null' to it) - body.Instructions.ReferenceReplaceRange(finallyMarker, [ - new CilInstruction(Ldc_I4_0), - new CilInstruction(Conv_U), - CilInstruction.CreateStloc(loc_1_pinnedTypeReference, body)]); + // Same code as for 'string' marshalling above (see additional comments there) + if (HasProtectedRegionBeforeParameterIndex(method, parameterIndex, interopReferences)) + { + CilInstruction ldc_i4_0_finallyStart = new(Ldc_I4_0); + CilInstruction nop_finallyEnd = new(Nop); + + body.Instructions.ReferenceReplaceRange(finallyMarker, [ + ldc_i4_0_finallyStart, + new CilInstruction(Conv_U), + CilInstruction.CreateStloc(loc_1_pinnedTypeReference, body), + new CilInstruction(Endfinally), + nop_finallyEnd]); + + body.ExceptionHandlers.Add(new CilExceptionHandler + { + HandlerType = CilExceptionHandlerType.Finally, + TryStart = nop_tryStart.CreateLabel(), + TryEnd = ldc_i4_0_finallyStart.CreateLabel(), + HandlerStart = ldc_i4_0_finallyStart.CreateLabel(), + HandlerEnd = nop_finallyEnd.CreateLabel() + }); + } + else + { + _ = body.Instructions.ReferenceRemove(nop_tryStart); + + body.Instructions.ReferenceReplaceRange(finallyMarker, [ + new CilInstruction(Ldc_I4_0), + new CilInstruction(Conv_U), + CilInstruction.CreateStloc(loc_1_pinnedTypeReference, body)]); + } + } + + /// + /// Checks whether a given method has any parameters before the specified index that need a protected region. + /// + /// The target method to perform two-pass code generation on. + /// The index of the parameter to marshal. + /// The instance to use. + /// Whether has any parameters before that need a protected region. + private static bool HasProtectedRegionBeforeParameterIndex( + MethodDefinition method, + int parameterIndex, + InteropReferences interopReferences) + { + for (int i = parameterIndex - 1; i >= 1; i--) + { + TypeSignature parameterType = method.Parameters[i].ParameterType; + + // If the parameter type is a value type, it needs a protected region if it is a + // managed type, as that means its ABI value would need some kind of disposal. + if (parameterType.IsValueType) + { + return !parameterType.IsManagedValueType(interopReferences); + } + + // Otherwise, it always needs a protected region, unless it's one of the special + // reference types that don't need one when marshalling to a native method. This + // is because 'string' and 'Type' just do pinning, and 'Exception' has an ABI type + // that is just the blittable 'HResult' struct type, so no disposal is needed. + return + !parameterType.IsTypeOfString() && + !parameterType.IsTypeOfType(interopReferences) && + !parameterType.IsTypeOfException(interopReferences); + } + + // If we reached here, there are no parameters before the target index. We are + // intentionally skipping the parameter at index '0', as that is just the 'this' + // parameter, which is passed as a 'WindowsRuntimeObjectReference' object. + return false; } } } \ No newline at end of file From 75b52d6eea20a5149b1611ffa049189ce9cbfd97 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Fri, 30 Jan 2026 16:16:13 -0800 Subject: [PATCH 2/9] Always use finally for unpinning locals Simplify unpinning logic in InteropMethodRewriter.NativeParameter.cs by always emitting a protected finally region for unpinning pinned locals. Introduces shared ldc_i4_0_finallyStart and nop_finallyEnd labels and moves the ExceptionHandler.Add call outside the previous conditional paths. Removes the HasProtectedRegionBeforeParameterIndex check and associated branching, as the rewriter assumes a preceding protected region (e.g. the 'this' parameter) is always present. This consolidates string and TypeReference marshalling cleanup into a single consistent flow. --- .../InteropMethodRewriter.NativeParameter.cs | 150 +++++------------- 1 file changed, 42 insertions(+), 108 deletions(-) diff --git a/src/WinRT.Interop.Generator/Rewriters/InteropMethodRewriter.NativeParameter.cs b/src/WinRT.Interop.Generator/Rewriters/InteropMethodRewriter.NativeParameter.cs index b0d01e63d..0788d84b0 100644 --- a/src/WinRT.Interop.Generator/Rewriters/InteropMethodRewriter.NativeParameter.cs +++ b/src/WinRT.Interop.Generator/Rewriters/InteropMethodRewriter.NativeParameter.cs @@ -302,6 +302,8 @@ private static void RewriteBodyForTypeOfString( CilInstruction ldarg_lengthNullCheck = CilInstruction.CreateLdarg(parameterIndex); CilInstruction ldarg_getLength = CilInstruction.CreateLdarg(parameterIndex); CilInstruction ldloca_s_getHStringReference = new(Ldloca_S, loc_1_hstringReference); + CilInstruction ldc_i4_0_finallyStart = new(Ldc_I4_0); + CilInstruction nop_finallyEnd = new(Nop); // Pin the input 'string' value, get the (possibly 'null') length, and create the 'HStringReference' value body.Instructions.ReferenceReplaceRange(tryMarker, [ @@ -340,49 +342,31 @@ private static void RewriteBodyForTypeOfString( new CilInstruction(Call, interopReferences.HStringReferenceget_HString.Import(module))]); // We need to emit code to unpin the local (matching what Roslyn does), but we need to consider whether other parameters - // in this same method will be using a protected region. If there aren't any, we can just emit the instructions to unpin - // the local inline, without a protected region (we just want to unpin, we don't have any resources to release). However, - // if there's a parameter before this one that has a protected region, it means that the inner-most protected region would - // have a 'leave.s' instruction to jump to the 'ret' at the end of the method. If that's the case, we can't just emit some - // additional instructions here, as they would end up being in that same protected region, but after the 'leave.s', which - // is not valid. So to work around that (like Roslyn does), we use a protected region for to unpin as well. With that change, - // the 'leave.s' will remain the last instruction in the inner-most protected region, and then following that there will - // be the instructions to unpin, in their own 'finally' handler, which then have their own 'endfinally' after them. - if (HasProtectedRegionBeforeParameterIndex(method, parameterIndex, interopReferences)) - { - // Additional jump labels just for this protected region - CilInstruction ldc_i4_0_finallyStart = new(Ldc_I4_0); - CilInstruction nop_finallyEnd = new(Nop); - - // Unpin the local (just assign 'null' to it) - body.Instructions.ReferenceReplaceRange(finallyMarker, [ - ldc_i4_0_finallyStart, - new CilInstruction(Conv_U), - CilInstruction.CreateStloc(loc_0_pinnedString, body), - new CilInstruction(Endfinally), - nop_finallyEnd]); + // in this same method will be using a protected region. In the scenarios where this rewriter will be used, that will + // always be the case, because each method will always have the 'this' parameter as the first argument, being an object + // reference for the native object to invoke the method on. And that object reference needs its own protected region. + // This means that the inner-most protected region in the method would always have a 'leave.s' instruction to jump to the + // 'ret' at the end of the method. Because of this, we can't just emit some additional instructions here, as they would + // end up being in that same protected region, but after the 'leave.s', which is not valid. So to work around that (like + // Roslyn does), we use a protected region for to unpin as well. With that change, the 'leave.s' will remain the last + // instruction in the inner-most protected region, and then following that there will be the instructions to unpin, in + // their own 'finally' handler, which then have their own 'endfinally' after them. + body.Instructions.ReferenceReplaceRange(finallyMarker, [ + ldc_i4_0_finallyStart, + new CilInstruction(Conv_U), + CilInstruction.CreateStloc(loc_0_pinnedString, body), + new CilInstruction(Endfinally), + nop_finallyEnd]); - // Setup the protected region to unpin the local - body.ExceptionHandlers.Add(new CilExceptionHandler - { - HandlerType = CilExceptionHandlerType.Finally, - TryStart = nop_tryStart.CreateLabel(), - TryEnd = ldc_i4_0_finallyStart.CreateLabel(), - HandlerStart = ldc_i4_0_finallyStart.CreateLabel(), - HandlerEnd = nop_finallyEnd.CreateLabel() - }); - } - else + // Setup the protected region to unpin the local + body.ExceptionHandlers.Add(new CilExceptionHandler { - // We don't be needing this 'nop', so we can remove it - _ = body.Instructions.ReferenceRemove(nop_tryStart); - - // Unpin the local (same as above, but without the protected region) - body.Instructions.ReferenceReplaceRange(finallyMarker, [ - new CilInstruction(Ldc_I4_0), - new CilInstruction(Conv_U), - CilInstruction.CreateStloc(loc_0_pinnedString, body)]); - } + HandlerType = CilExceptionHandlerType.Finally, + TryStart = nop_tryStart.CreateLabel(), + TryEnd = ldc_i4_0_finallyStart.CreateLabel(), + HandlerStart = ldc_i4_0_finallyStart.CreateLabel(), + HandlerEnd = nop_finallyEnd.CreateLabel() + }); } /// @@ -407,7 +391,10 @@ private static void RewriteBodyForTypeOfType( body.LocalVariables.Add(loc_0_typeReference); body.LocalVariables.Add(loc_1_pinnedTypeReference); + // Prepare the jump labels CilInstruction nop_tryStart = new(Nop); + CilInstruction ldc_i4_0_finallyStart = new(Ldc_I4_0); + CilInstruction nop_finallyEnd = new(Nop); // Get the 'TypeReference' value and pin it body.Instructions.ReferenceReplaceRange(tryMarker, [ @@ -425,75 +412,22 @@ private static void RewriteBodyForTypeOfType( new CilInstruction(Call, interopReferences.TypeReferenceConvertToUnmanagedUnsafe.Import(module))]); // Same code as for 'string' marshalling above (see additional comments there) - if (HasProtectedRegionBeforeParameterIndex(method, parameterIndex, interopReferences)) - { - CilInstruction ldc_i4_0_finallyStart = new(Ldc_I4_0); - CilInstruction nop_finallyEnd = new(Nop); - - body.Instructions.ReferenceReplaceRange(finallyMarker, [ - ldc_i4_0_finallyStart, - new CilInstruction(Conv_U), - CilInstruction.CreateStloc(loc_1_pinnedTypeReference, body), - new CilInstruction(Endfinally), - nop_finallyEnd]); - - body.ExceptionHandlers.Add(new CilExceptionHandler - { - HandlerType = CilExceptionHandlerType.Finally, - TryStart = nop_tryStart.CreateLabel(), - TryEnd = ldc_i4_0_finallyStart.CreateLabel(), - HandlerStart = ldc_i4_0_finallyStart.CreateLabel(), - HandlerEnd = nop_finallyEnd.CreateLabel() - }); - } - else - { - _ = body.Instructions.ReferenceRemove(nop_tryStart); - - body.Instructions.ReferenceReplaceRange(finallyMarker, [ - new CilInstruction(Ldc_I4_0), - new CilInstruction(Conv_U), - CilInstruction.CreateStloc(loc_1_pinnedTypeReference, body)]); - } - } + body.Instructions.ReferenceReplaceRange(finallyMarker, [ + ldc_i4_0_finallyStart, + new CilInstruction(Conv_U), + CilInstruction.CreateStloc(loc_1_pinnedTypeReference, body), + new CilInstruction(Endfinally), + nop_finallyEnd]); - /// - /// Checks whether a given method has any parameters before the specified index that need a protected region. - /// - /// The target method to perform two-pass code generation on. - /// The index of the parameter to marshal. - /// The instance to use. - /// Whether has any parameters before that need a protected region. - private static bool HasProtectedRegionBeforeParameterIndex( - MethodDefinition method, - int parameterIndex, - InteropReferences interopReferences) - { - for (int i = parameterIndex - 1; i >= 1; i--) + // Same as above for the handler for the unpinning + body.ExceptionHandlers.Add(new CilExceptionHandler { - TypeSignature parameterType = method.Parameters[i].ParameterType; - - // If the parameter type is a value type, it needs a protected region if it is a - // managed type, as that means its ABI value would need some kind of disposal. - if (parameterType.IsValueType) - { - return !parameterType.IsManagedValueType(interopReferences); - } - - // Otherwise, it always needs a protected region, unless it's one of the special - // reference types that don't need one when marshalling to a native method. This - // is because 'string' and 'Type' just do pinning, and 'Exception' has an ABI type - // that is just the blittable 'HResult' struct type, so no disposal is needed. - return - !parameterType.IsTypeOfString() && - !parameterType.IsTypeOfType(interopReferences) && - !parameterType.IsTypeOfException(interopReferences); - } - - // If we reached here, there are no parameters before the target index. We are - // intentionally skipping the parameter at index '0', as that is just the 'this' - // parameter, which is passed as a 'WindowsRuntimeObjectReference' object. - return false; + HandlerType = CilExceptionHandlerType.Finally, + TryStart = nop_tryStart.CreateLabel(), + TryEnd = ldc_i4_0_finallyStart.CreateLabel(), + HandlerStart = ldc_i4_0_finallyStart.CreateLabel(), + HandlerEnd = nop_finallyEnd.CreateLabel() + }); } } } \ No newline at end of file From bbc1699155224939f7d3e1e5ff0010987ec8f3ea Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Fri, 30 Jan 2026 16:26:45 -0800 Subject: [PATCH 3/9] Handle unmanaged types and KeyValuePair params Add explicit handling for KeyValuePair<> parameter types by directly looking up the marshaller from emit state (no custom disposal needed). Change the previous default branch to explicitly detect managed value types. Add a new branch for unmanaged value types that only need marshalling (no disposal): remove the try/finally ranges and replace the load sequence to call the marshaller's ConvertToUnmanaged method. These changes reduce unnecessary disposal overhead and ensure correct marshalling paths for different value-type categories. --- .../InteropMethodRewriter.NativeParameter.cs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/WinRT.Interop.Generator/Rewriters/InteropMethodRewriter.NativeParameter.cs b/src/WinRT.Interop.Generator/Rewriters/InteropMethodRewriter.NativeParameter.cs index 0788d84b0..f81af2f86 100644 --- a/src/WinRT.Interop.Generator/Rewriters/InteropMethodRewriter.NativeParameter.cs +++ b/src/WinRT.Interop.Generator/Rewriters/InteropMethodRewriter.NativeParameter.cs @@ -87,6 +87,8 @@ public static void RewriteMethod( } else if (parameterType.IsConstructedKeyValuePairType(interopReferences)) { + // For 'KeyValuePair<,>' types, we can always directly lookup the marshaller from the emit state. + // We don't need to pass a disposal method, as they will use the generic one for all COM objects. RewriteBody( parameterType: parameterType, body: body, @@ -115,9 +117,9 @@ public static void RewriteMethod( interopReferences: interopReferences, module: module); } - else + else if (parameterType.IsManagedValueType(interopReferences)) { - // The last case handles all other value types, which need explicit disposal for their ABI values + // Handle all managed value types, which need explicit disposal for their ABI values InteropMarshallerType marshallerType = InteropMarshallerTypeResolver.GetMarshallerType(parameterType, interopReferences, emitState); RewriteBody( @@ -132,6 +134,16 @@ public static void RewriteMethod( interopReferences: interopReferences, module: module); } + else + { + // The last case is for unmanaged value types, which just need marshalling but no disposal + InteropMarshallerType marshallerType = InteropMarshallerTypeResolver.GetMarshallerType(parameterType, interopReferences, emitState); + + body.Instructions.ReferenceRemoveRange(tryMarker, finallyMarker); + body.Instructions.ReferenceReplaceRange(loadMarker, [ + CilInstruction.CreateLdarg(parameterIndex), + new CilInstruction(Call, marshallerType.ConvertToUnmanaged().Import(module))]); + } } else if (parameterType.IsTypeOfString()) { From 0795b832daa00d63f82872ff7bc50459dba044e6 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Fri, 30 Jan 2026 16:29:11 -0800 Subject: [PATCH 4/9] Remove unused 'method' parameter Remove the unused MethodDefinition parameter from RewriteBodyForTypeOfString and RewriteBodyForTypeOfType in src/WinRT.Interop.Generator/Rewriters/InteropMethodRewriter.NativeParameter.cs. Update call sites to stop passing the argument and remove the related XML doc lines. Minor cleanup to simplify helper signatures and remove dead parameter usage. --- .../Rewriters/InteropMethodRewriter.NativeParameter.cs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/WinRT.Interop.Generator/Rewriters/InteropMethodRewriter.NativeParameter.cs b/src/WinRT.Interop.Generator/Rewriters/InteropMethodRewriter.NativeParameter.cs index f81af2f86..352d2a989 100644 --- a/src/WinRT.Interop.Generator/Rewriters/InteropMethodRewriter.NativeParameter.cs +++ b/src/WinRT.Interop.Generator/Rewriters/InteropMethodRewriter.NativeParameter.cs @@ -148,7 +148,6 @@ public static void RewriteMethod( else if (parameterType.IsTypeOfString()) { RewriteBodyForTypeOfString( - method: method, body: body, tryMarker: tryMarker, loadMarker: loadMarker, @@ -160,7 +159,6 @@ public static void RewriteMethod( else if (parameterType.IsTypeOfType(interopReferences)) { RewriteBodyForTypeOfType( - method: method, body: body, tryMarker: tryMarker, loadMarker: loadMarker, @@ -284,10 +282,8 @@ private static void RewriteBody( } /// - /// The target method to perform two-pass code generation on. /// The target body to perform two-pass code generation on. private static void RewriteBodyForTypeOfString( - MethodDefinition method, CilMethodBody body, CilInstruction tryMarker, CilInstruction loadMarker, @@ -382,10 +378,8 @@ private static void RewriteBodyForTypeOfString( } /// - /// The target method to perform two-pass code generation on. /// The target body to perform two-pass code generation on. private static void RewriteBodyForTypeOfType( - MethodDefinition method, CilMethodBody body, CilInstruction tryMarker, CilInstruction loadMarker, From 880fad920dcf9afdb8af1c33f9b68e23cba3bfb0 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Fri, 30 Jan 2026 16:42:10 -0800 Subject: [PATCH 5/9] Fix typos in InteropMethodRewriter comments Corrected and clarified comment text in InteropMethodRewriter.NativeParameter.cs around HString handling and protected regions. Removed garbled words and rephrased the explanation of using a protected region to unpin the local so the 'leave.s' remains the last instruction in the inner-most region and the unpin instructions are emitted in their own finally handler. No functional code changes. --- .../Rewriters/InteropMethodRewriter.NativeParameter.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/WinRT.Interop.Generator/Rewriters/InteropMethodRewriter.NativeParameter.cs b/src/WinRT.Interop.Generator/Rewriters/InteropMethodRewriter.NativeParameter.cs index 352d2a989..2d90661d8 100644 --- a/src/WinRT.Interop.Generator/Rewriters/InteropMethodRewriter.NativeParameter.cs +++ b/src/WinRT.Interop.Generator/Rewriters/InteropMethodRewriter.NativeParameter.cs @@ -344,7 +344,7 @@ private static void RewriteBodyForTypeOfString( ldloca_s_getHStringReference, new CilInstruction(Call, interopReferences.HStringMarshallerConvertToUnmanagedUnsafe.Import(module))]); - // Get the 'HString' value from the reference and pass it as a parameter + // Get the 'HString' value from the reference and pass it as a parause a protected region for to unpinmeter body.Instructions.ReferenceReplaceRange(loadMarker, [ new CilInstruction(Ldloca_S, loc_1_hstringReference), new CilInstruction(Call, interopReferences.HStringReferenceget_HString.Import(module))]); @@ -356,9 +356,9 @@ private static void RewriteBodyForTypeOfString( // This means that the inner-most protected region in the method would always have a 'leave.s' instruction to jump to the // 'ret' at the end of the method. Because of this, we can't just emit some additional instructions here, as they would // end up being in that same protected region, but after the 'leave.s', which is not valid. So to work around that (like - // Roslyn does), we use a protected region for to unpin as well. With that change, the 'leave.s' will remain the last - // instruction in the inner-most protected region, and then following that there will be the instructions to unpin, in - // their own 'finally' handler, which then have their own 'endfinally' after them. + // Roslyn does), we use a protected region to unpin the local as well. With that change, the 'leave.s' will remain the + // last instruction in the inner-most protected region, and then following that there will be the instructions to unpin, + // in their own 'finally' handler, which then have their own 'endfinally' after them. body.Instructions.ReferenceReplaceRange(finallyMarker, [ ldc_i4_0_finallyStart, new CilInstruction(Conv_U), From 5bcc7c08535b817ec7f364d0e3ed8abf76106b68 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Fri, 30 Jan 2026 19:24:23 -0800 Subject: [PATCH 6/9] Make InteropMethodRewriter non-static Remove the static modifier from the partial InteropMethodRewriter class (change from 'internal static partial class' to 'internal partial class') to allow instance members. Replace the file-level XML summary with an inheritdoc () so the file reuses the main type's documentation. --- .../Rewriters/InteropMethodRewriter.ManagedValue.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/WinRT.Interop.Generator/Rewriters/InteropMethodRewriter.ManagedValue.cs b/src/WinRT.Interop.Generator/Rewriters/InteropMethodRewriter.ManagedValue.cs index 1777ea719..6b77fe5a8 100644 --- a/src/WinRT.Interop.Generator/Rewriters/InteropMethodRewriter.ManagedValue.cs +++ b/src/WinRT.Interop.Generator/Rewriters/InteropMethodRewriter.ManagedValue.cs @@ -15,10 +15,8 @@ namespace WindowsRuntime.InteropGenerator.Rewriters; -/// -/// A rewritef for interop method definitons, that can add marshalling code as needed. -/// -internal static partial class InteropMethodRewriter +/// +internal partial class InteropMethodRewriter { /// /// Contains the logic for marshalling managed values (i.e. parameters that are passed to managed methods, already on the stack). From fbf3711f978771a0094fbba03ff93d4f626d8a2d Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Fri, 30 Jan 2026 19:25:54 -0800 Subject: [PATCH 7/9] Add InteropMethodFixup abstract class Introduce InteropMethodFixup, an abstract type for applying custom fixups to generated methods as a final processing step. The class resides in WindowsRuntime.InteropGenerator.Fixups and exposes an abstract Apply(MethodDefinition method) method for implementers. Includes MIT license header and reference to AsmResolver.DotNet. --- .../Fixups/InteropMethodFixup.cs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 src/WinRT.Interop.Generator/Fixups/InteropMethodFixup.cs diff --git a/src/WinRT.Interop.Generator/Fixups/InteropMethodFixup.cs b/src/WinRT.Interop.Generator/Fixups/InteropMethodFixup.cs new file mode 100644 index 000000000..6870be135 --- /dev/null +++ b/src/WinRT.Interop.Generator/Fixups/InteropMethodFixup.cs @@ -0,0 +1,18 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using AsmResolver.DotNet; + +namespace WindowsRuntime.InteropGenerator.Fixups; + +/// +/// A type that can apply custom fixups to generated methods, as a last processing step. +/// +internal abstract class InteropMethodFixup +{ + /// + /// Applies the current fixup to a target method. + /// + /// The target method to apply the fixup to. + public abstract void Apply(MethodDefinition method); +} From 2e6d7602370ebd317d2ca8cb2437812e83f4cdfe Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Fri, 30 Jan 2026 23:43:28 -0800 Subject: [PATCH 8/9] Add RemoveLeftoverNopAfterLeave fixup and exceptions Introduce a new InteropMethodFixup.RemoveLeftoverNopAfterLeave implementation that removes invalid nop instructions inside protected regions and redirects labels accordingly. Add validation helpers for exception handler and branch labels, and helper methods to detect non-fall-through instructions and ranges. Add new WellKnownInteropExceptions for fixup-related errors (IDs 75-77) and clarify the existing MethodRewriteMissingBodyError message. Make InteropMethodFixup partial to accommodate the new fixup implementation. --- .../Errors/WellKnownInteropExceptions.cs | 26 +- ...MethodFixup.RemoveLeftoverNopAfterLeave.cs | 293 ++++++++++++++++++ .../Fixups/InteropMethodFixup.cs | 2 +- 3 files changed, 319 insertions(+), 2 deletions(-) create mode 100644 src/WinRT.Interop.Generator/Fixups/InteropMethodFixup.RemoveLeftoverNopAfterLeave.cs diff --git a/src/WinRT.Interop.Generator/Errors/WellKnownInteropExceptions.cs b/src/WinRT.Interop.Generator/Errors/WellKnownInteropExceptions.cs index a61dee3e5..0e9ab1cb7 100644 --- a/src/WinRT.Interop.Generator/Errors/WellKnownInteropExceptions.cs +++ b/src/WinRT.Interop.Generator/Errors/WellKnownInteropExceptions.cs @@ -498,7 +498,7 @@ public static WellKnownInteropException MethodRewriteError(TypeSignature type, M /// public static WellKnownInteropException MethodRewriteMissingBodyError(MethodDefinition method) { - return Exception(57, $"Generated interop method '{method}' is missing an IL method body."); + return Exception(57, $"Generated interop method '{method}' is missing an IL method body, two-pass rewrite cannot be performed."); } /// @@ -641,6 +641,30 @@ public static WellKnownInteropException EmitMetadataAssemblyAttributesError(Exce return Exception(74, "Failed to emit the metadata assembly attributes for the interop assembly.", exception); } + /// + /// A generated interop method is missing an IL method body. + /// + public static WellKnownInteropException MethodFixupMissingBodyError(MethodDefinition method) + { + return Exception(75, $"Generated interop method '{method}' is missing an IL method body, fixups cannot be applied."); + } + + /// + /// A generated interop method has invalid exception handler labels. + /// + public static WellKnownInteropException MethodFixupInvalidExceptionHandlerLabels(MethodDefinition method) + { + return Exception(76, $"Generated interop method '{method}' has invalid exception handler labels, fixups cannot be applied."); + } + + /// + /// A generated interop method has invalid branch instruction labels. + /// + public static WellKnownInteropException MethodFixupInvalidBranchInstructionLabels(MethodDefinition method) + { + return Exception(77, $"Generated interop method '{method}' has invalid branch instruction labels, fixups cannot be applied."); + } + /// /// Creates a new exception with the specified id and message. /// diff --git a/src/WinRT.Interop.Generator/Fixups/InteropMethodFixup.RemoveLeftoverNopAfterLeave.cs b/src/WinRT.Interop.Generator/Fixups/InteropMethodFixup.RemoveLeftoverNopAfterLeave.cs new file mode 100644 index 000000000..952d526d7 --- /dev/null +++ b/src/WinRT.Interop.Generator/Fixups/InteropMethodFixup.RemoveLeftoverNopAfterLeave.cs @@ -0,0 +1,293 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Collections.Generic; +using AsmResolver.DotNet; +using AsmResolver.DotNet.Code.Cil; +using AsmResolver.PE.DotNet.Cil; +using WindowsRuntime.InteropGenerator.Errors; + +namespace WindowsRuntime.InteropGenerator.Fixups; + +/// +internal partial class InteropMethodFixup +{ + /// + /// A fixup that removes invalid instructions within protected regions. + /// + /// + /// + /// This fixup finds and removes instructions that are located within a protected region + /// and that cause the method body to be invalid. This happens when they follow an instructions that does not + /// fall through (i.e. , , , or + /// ), within the same protected region. According to ECMA-335 rules, instructions cannot + /// follow those non fall through instructions that delimit a protected region. + /// + /// + /// When a instruction is removed, all labels pointing to it (from exception handlers + /// or branch instructions) are adjusted to point to the instruction immediately following the removed one. + /// + /// + public sealed class RemoveLeftoverNopAfterLeave : InteropMethodFixup + { + /// + public override void Apply(MethodDefinition method) + { + // Validate that we do have some IL body for the input method (this should always be the case) + if (method.CilMethodBody is not CilMethodBody body) + { + throw WellKnownInteropExceptions.MethodFixupMissingBodyError(method); + } + + CilInstructionCollection instructions = body.Instructions; + + // Ignore empty methods (they're invalid and will fail on emit anyway) + if (instructions.Count == 0) + { + return; + } + + bool areExceptionHandlersValid = false; + bool areJumpTargetsValid = false; + + // Process instructions in reverse order to avoid index shifting issues when removing + for (int i = instructions.Count - 1; i >= 0; i--) + { + CilInstruction instruction = instructions[i]; + + // Check if this is a 'nop' instruction, otherwise ignore it + if (instruction.OpCode != CilOpCodes.Nop) + { + continue; + } + + // Make sure that we do have some instruction before this. If that's not the case, + // we can't possibly follow a problematic instruction, so there's nothing to do. + if (i == 0) + { + continue; + } + + // Validate exception handlers only once, before checking for problematic 'nop'-s + if (!areExceptionHandlersValid) + { + ValidateExceptionHandlerLabels(method, body); + + areExceptionHandlersValid = true; + } + + // Check if this 'nop' is within a protected region ('try' block or handler) + if (!IsWithinProtectedRegion(body, instruction)) + { + continue; + } + + // Check if the previous instruction does not fall through + if (!IsNonFallThroughInstruction(instructions[i - 1])) + { + continue; + } + + // Validate branch instructions only once, before patching their labels below + if (!areJumpTargetsValid) + { + ValidateBranchInstructionLabels(method, body); + + areJumpTargetsValid = true; + } + + // Get the next instruction to redirect labels to (if any). There should always + // be one, otherwise the method would be invalid. If that's the case, we don't + // need to validate here, we can just let the emitter fail later. + CilInstruction? nextInstruction = i + 1 < instructions.Count ? instructions[i + 1] : null; + + // Redirect all labels pointing to this 'nop' to the next instruction + if (nextInstruction is not null) + { + RedirectLabels(body, instruction, nextInstruction); + } + + // Remove the invalid 'nop' instruction + instructions.RemoveAt(i); + } + } + + /// + /// Validates the exception handler labels for the specified method. + /// + /// The method to validate. + /// The method body to validate. + private static void ValidateExceptionHandlerLabels(MethodDefinition method, CilMethodBody body) + { + // We only support handlers that use instruction labels. We never manually patch + // labels that use explicit offsets, as those haven't been computed at this point. + // Additionally, filters should never be used by any generated interop methods. + foreach (CilExceptionHandler handler in body.ExceptionHandlers) + { + if (handler is not + { + TryStart: CilInstructionLabel { Instruction: not null }, + TryEnd: CilInstructionLabel { Instruction: not null }, + HandlerStart: CilInstructionLabel { Instruction: not null }, + HandlerEnd: CilInstructionLabel { Instruction: not null }, + FilterStart: null + }) + { + throw WellKnownInteropExceptions.MethodFixupInvalidExceptionHandlerLabels(method); + } + } + } + + /// + /// Validates the exception handler labels for the specified method. + /// + /// The method to validate. + /// The method body to validate. + private static void ValidateBranchInstructionLabels(MethodDefinition method, CilMethodBody body) + { + foreach (CilInstruction instruction in body.Instructions) + { + // Make sure that the operand of all branch instruction is a valid instruction label. + // This is the same exact validation we also did above for all exception handlers. + if (instruction.Operand is ICilLabel and not CilInstructionLabel { Instruction: not null }) + { + throw WellKnownInteropExceptions.MethodFixupInvalidBranchInstructionLabels(method); + } + + // Handle 'switch' instructions too (their operands are multiple branch targets) + if (instruction.Operand is IEnumerable labels) + { + foreach (ICilLabel label in labels) + { + if (label is not CilInstructionLabel { Instruction: not null }) + { + throw WellKnownInteropExceptions.MethodFixupInvalidBranchInstructionLabels(method); + } + } + } + } + } + + /// + /// Checks whether the specified instruction does not fall through to the next instruction. + /// + /// The instruction to check. + /// Whether does not fall through. + private static bool IsNonFallThroughInstruction(CilInstruction instruction) + { + CilOpCode opCode = instruction.OpCode; + + // Note: we only care about checking for problematic instructions for protected regions + return opCode == CilOpCodes.Leave || + opCode == CilOpCodes.Leave_S || + opCode == CilOpCodes.Endfinally || + opCode == CilOpCodes.Endfilter || + opCode == CilOpCodes.Throw || + opCode == CilOpCodes.Rethrow; + } + + /// + /// Checks whether the specified instruction is within a protected region (e.g. a block). + /// + /// The method body containing the instruction. + /// The instruction to check. + /// Whether is within a protected region. + private static bool IsWithinProtectedRegion(CilMethodBody body, CilInstruction instruction) + { + foreach (CilExceptionHandler handler in body.ExceptionHandlers) + { + // Check if instruction is within the 'try' block. Note that we can directly cast + // to 'CilInstructionLabel' here, as we've already validated the exception handlers. + if (IsInstructionInRange(body, instruction, (CilInstructionLabel)handler.TryStart!, (CilInstructionLabel)handler.TryEnd!)) + { + return true; + } + + // Check if instruction is within the handler block + if (IsInstructionInRange(body, instruction, (CilInstructionLabel)handler.HandlerStart!, (CilInstructionLabel)handler.HandlerEnd!)) + { + return true; + } + } + + return false; + } + + /// + /// Checks whether an instruction falls within a specified range. + /// + /// The method body containing the instruction. + /// The instruction to check. + /// The start label of the range. + /// The end label of the range (exclusive). + /// Whether is between the and labels. + private static bool IsInstructionInRange( + CilMethodBody body, + CilInstruction instruction, + CilInstructionLabel start, + CilInstructionLabel end) + { + int instructionIndex = body.Instructions.ReferenceIndexOf(instruction); + int startIndex = body.Instructions.ReferenceIndexOf(start.Instruction!); + int endIndex = body.Instructions.ReferenceIndexOf(end.Instruction!); + + return instructionIndex >= startIndex && instructionIndex < endIndex; + } + + /// + /// Redirects all labels pointing to the old instruction to point to the new instruction instead. + /// + /// The method body to update. + /// The instruction that labels currently point to. + /// The instruction that labels should point to after redirection. + private static void RedirectLabels(CilMethodBody body, CilInstruction oldInstruction, CilInstruction newInstruction) + { + ICilLabel newLabel = newInstruction.CreateLabel(); + + // Update exception handler labels (they've been validated before already) + foreach (CilExceptionHandler handler in body.ExceptionHandlers) + { + if (((CilInstructionLabel)handler.TryStart!).Instruction == oldInstruction) + { + handler.TryStart = newLabel; + } + + if (((CilInstructionLabel)handler.TryEnd!).Instruction == oldInstruction) + { + handler.TryEnd = newLabel; + } + + if (((CilInstructionLabel)handler.HandlerStart!).Instruction == oldInstruction) + { + handler.HandlerStart = newLabel; + } + + if (((CilInstructionLabel)handler.HandlerEnd!).Instruction == oldInstruction) + { + handler.HandlerEnd = newLabel; + } + } + + // Update branch instruction operands + foreach (CilInstruction instruction in body.Instructions) + { + // Handle single branch target + if (instruction.Operand is CilInstructionLabel label && label.Instruction == oldInstruction) + { + instruction.Operand = newLabel; + } + // Handle switch instruction (multiple branch targets) + else if (instruction.Operand is System.Collections.Generic.IList labels) + { + for (int i = 0; i < labels.Count; i++) + { + if (labels[i] is CilInstructionLabel switchLabel && switchLabel.Instruction == oldInstruction) + { + labels[i] = newLabel; + } + } + } + } + } + } +} diff --git a/src/WinRT.Interop.Generator/Fixups/InteropMethodFixup.cs b/src/WinRT.Interop.Generator/Fixups/InteropMethodFixup.cs index 6870be135..88aac1fff 100644 --- a/src/WinRT.Interop.Generator/Fixups/InteropMethodFixup.cs +++ b/src/WinRT.Interop.Generator/Fixups/InteropMethodFixup.cs @@ -8,7 +8,7 @@ namespace WindowsRuntime.InteropGenerator.Fixups; /// /// A type that can apply custom fixups to generated methods, as a last processing step. /// -internal abstract class InteropMethodFixup +internal abstract partial class InteropMethodFixup { /// /// Applies the current fixup to a target method. From 4927723934d0962f62bfebcbd9b7112508163f8e Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sat, 31 Jan 2026 00:03:16 -0800 Subject: [PATCH 9/9] Apply IL fixups to generated interop methods Add a two-pass IL fixup step that walks all generated types/methods and applies available InteropMethodFixup instances. Introduce FixupMethodDefinitions and call it during Emit, applying fixes (currently RemoveLeftoverNopAfterLeave) and reporting failures via a new WellKnownInteropExceptions.MethodFixupError helper. Also add necessary using directives. --- .../Errors/WellKnownInteropExceptions.cs | 9 +++++ ...MethodFixup.RemoveLeftoverNopAfterLeave.cs | 5 +++ .../Generation/InteropGenerator.Emit.cs | 37 +++++++++++++++++++ 3 files changed, 51 insertions(+) diff --git a/src/WinRT.Interop.Generator/Errors/WellKnownInteropExceptions.cs b/src/WinRT.Interop.Generator/Errors/WellKnownInteropExceptions.cs index 0e9ab1cb7..ada994168 100644 --- a/src/WinRT.Interop.Generator/Errors/WellKnownInteropExceptions.cs +++ b/src/WinRT.Interop.Generator/Errors/WellKnownInteropExceptions.cs @@ -8,6 +8,7 @@ using AsmResolver.DotNet.Code.Cil; using AsmResolver.DotNet.Signatures; using AsmResolver.PE.DotNet.Cil; +using WindowsRuntime.InteropGenerator.Fixups; namespace WindowsRuntime.InteropGenerator.Errors; @@ -665,6 +666,14 @@ public static WellKnownInteropException MethodFixupInvalidBranchInstructionLabel return Exception(77, $"Generated interop method '{method}' has invalid branch instruction labels, fixups cannot be applied."); } + /// + /// Failed to apply a fixup to a marshalling method. + /// + public static WellKnownInteropException MethodFixupError(InteropMethodFixup fixup, MethodDefinition method, Exception exception) + { + return Exception(78, $"Failed to apply fixup '{fixup.GetType()}' to method '{method}'.", exception); + } + /// /// Creates a new exception with the specified id and message. /// diff --git a/src/WinRT.Interop.Generator/Fixups/InteropMethodFixup.RemoveLeftoverNopAfterLeave.cs b/src/WinRT.Interop.Generator/Fixups/InteropMethodFixup.RemoveLeftoverNopAfterLeave.cs index 952d526d7..c401fe31a 100644 --- a/src/WinRT.Interop.Generator/Fixups/InteropMethodFixup.RemoveLeftoverNopAfterLeave.cs +++ b/src/WinRT.Interop.Generator/Fixups/InteropMethodFixup.RemoveLeftoverNopAfterLeave.cs @@ -30,6 +30,11 @@ internal partial class InteropMethodFixup /// public sealed class RemoveLeftoverNopAfterLeave : InteropMethodFixup { + /// + /// The singleton instance. + /// + public static readonly RemoveLeftoverNopAfterLeave Instance = new(); + /// public override void Apply(MethodDefinition method) { diff --git a/src/WinRT.Interop.Generator/Generation/InteropGenerator.Emit.cs b/src/WinRT.Interop.Generator/Generation/InteropGenerator.Emit.cs index 2c39f23c7..79f958d21 100644 --- a/src/WinRT.Interop.Generator/Generation/InteropGenerator.Emit.cs +++ b/src/WinRT.Interop.Generator/Generation/InteropGenerator.Emit.cs @@ -11,6 +11,7 @@ using WindowsRuntime.InteropGenerator.Builders; using WindowsRuntime.InteropGenerator.Errors; using WindowsRuntime.InteropGenerator.Factories; +using WindowsRuntime.InteropGenerator.Fixups; using WindowsRuntime.InteropGenerator.Helpers; using WindowsRuntime.InteropGenerator.Models; using WindowsRuntime.InteropGenerator.References; @@ -155,6 +156,11 @@ private static void Emit(InteropGeneratorArgs args, InteropGeneratorDiscoverySta args.Token.ThrowIfCancellationRequested(); + // Apply fixups to all generated interop methods + FixupMethodDefinitions(args, module); + + args.Token.ThrowIfCancellationRequested(); + // Emit interop types for user-defined array types DefineUserDefinedTypes(args, discoveryState, emitState, interopDefinitions, interopReferences, module); @@ -2336,6 +2342,37 @@ private static void RewriteMethodDefinitions( } } + /// + /// Applies fixups to IL method bodies for marshalling stubs as part of two-pass IL generation. + /// + /// + /// The interop module being built. + private static void FixupMethodDefinitions(InteropGeneratorArgs args, ModuleDefinition module) + { + ReadOnlySpan fixups = [InteropMethodFixup.RemoveLeftoverNopAfterLeave.Instance]; + + // Applies all available fixups in order, to all methods across all generated types in the module + foreach (TypeDefinition type in module.GetAllTypes()) + { + args.Token.ThrowIfCancellationRequested(); + + foreach (MethodDefinition method in type.Methods) + { + foreach (InteropMethodFixup fixup in fixups) + { + try + { + fixup.Apply(method); + } + catch (Exception e) + { + WellKnownInteropExceptions.MethodFixupError(fixup, method, e).ThrowOrAttach(e); + } + } + } + } + } + /// /// Defines the interop types for user-defined types. ///