diff --git a/src/WinRT.Interop.Generator/Errors/WellKnownInteropExceptions.cs b/src/WinRT.Interop.Generator/Errors/WellKnownInteropExceptions.cs index a61dee3e5..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; @@ -498,7 +499,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 +642,38 @@ 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."); + } + + /// + /// 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 new file mode 100644 index 000000000..c401fe31a --- /dev/null +++ b/src/WinRT.Interop.Generator/Fixups/InteropMethodFixup.RemoveLeftoverNopAfterLeave.cs @@ -0,0 +1,298 @@ +// 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 + { + /// + /// The singleton instance. + /// + public static readonly RemoveLeftoverNopAfterLeave Instance = new(); + + /// + 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 new file mode 100644 index 000000000..88aac1fff --- /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 partial class InteropMethodFixup +{ + /// + /// Applies the current fixup to a target method. + /// + /// The target method to apply the fixup to. + public abstract 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. /// 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). diff --git a/src/WinRT.Interop.Generator/Rewriters/InteropMethodRewriter.NativeParameter.cs b/src/WinRT.Interop.Generator/Rewriters/InteropMethodRewriter.NativeParameter.cs index 117fbf3d3..2d90661d8 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()) { @@ -293,15 +305,19 @@ 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); 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, [ // fixed (char* p = value) { } + nop_tryStart, CilInstruction.CreateLdarg(parameterIndex), new CilInstruction(Brtrue_S, ldarg_pinning.CreateLabel()), new CilInstruction(Ldc_I4_0), @@ -328,16 +344,37 @@ 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))]); - // Unpin the local (just assign 'null' to it) + // 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. 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 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, [ - new CilInstruction(Ldc_I4_0), + ldc_i4_0_finallyStart, new CilInstruction(Conv_U), - CilInstruction.CreateStloc(loc_0_pinnedString, body)]); + 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() + }); } /// @@ -360,8 +397,14 @@ 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, [ + nop_tryStart, CilInstruction.CreateLdarg(parameterIndex), new CilInstruction(Ldloca_S, loc_0_typeReference), new CilInstruction(Call, interopReferences.TypeMarshallerConvertToUnmanagedUnsafe.Import(module)), @@ -374,11 +417,23 @@ 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) + // Same code as for 'string' marshalling above (see additional comments there) body.Instructions.ReferenceReplaceRange(finallyMarker, [ - new CilInstruction(Ldc_I4_0), + ldc_i4_0_finallyStart, new CilInstruction(Conv_U), - CilInstruction.CreateStloc(loc_1_pinnedTypeReference, body)]); + CilInstruction.CreateStloc(loc_1_pinnedTypeReference, body), + new CilInstruction(Endfinally), + nop_finallyEnd]); + + // Same as above for the handler for the unpinning + 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() + }); } } } \ No newline at end of file