diff --git a/ICSharpCode.Decompiler/IL/Instructions.cs b/ICSharpCode.Decompiler/IL/Instructions.cs index 7eb435221..99b70df14 100644 --- a/ICSharpCode.Decompiler/IL/Instructions.cs +++ b/ICSharpCode.Decompiler/IL/Instructions.cs @@ -528,10 +528,6 @@ namespace ICSharpCode.Decompiler.IL public sealed partial class Branch : SimpleInstruction { public override StackType ResultType { get { return StackType.Void; } } - protected override InstructionFlags ComputeFlags() - { - return InstructionFlags.EndPointUnreachable | InstructionFlags.MayBranch; - } public override T AcceptVisitor(ILVisitor visitor) { return visitor.VisitBranch(this); diff --git a/ICSharpCode.Decompiler/IL/Instructions.tt b/ICSharpCode.Decompiler/IL/Instructions.tt index acf021417..a7d79d88e 100644 --- a/ICSharpCode.Decompiler/IL/Instructions.tt +++ b/ICSharpCode.Decompiler/IL/Instructions.tt @@ -60,7 +60,7 @@ new OpCode("bit.not", "Bitwise NOT", Unary), new OpCode("arglist", "Retrieves the RuntimeArgumentHandle.", NoArguments, ResultType("O")), new OpCode("br", "Unconditional branch. goto target;", - CustomClassName("Branch"), NoArguments, CustomConstructor, UnconditionalBranch, MayBranch), + CustomClassName("Branch"), NoArguments, CustomConstructor, UnconditionalBranch, MayBranch, CustomComputeFlags), // TODO: get rid of endfinally, we can represent those with a normal branch to the end of the filter/finally block instead new OpCode("endfinally", "Marks the end of an finally, fault or exception filter block.", CustomClassName("EndFinally"), NoArguments, UnconditionalBranch, MayBranch), diff --git a/ICSharpCode.Decompiler/IL/Instructions/Branch.cs b/ICSharpCode.Decompiler/IL/Instructions/Branch.cs index 44fb6ec41..8ecd004d9 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/Branch.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/Branch.cs @@ -25,6 +25,14 @@ using System.Threading.Tasks; namespace ICSharpCode.Decompiler.IL { + /// + /// Unconditional branch. goto target; + /// + /// + /// Phase-1 execution of a branch is a no-op. + /// Phase-2 execution removes PopCount elements from the evaluation stack + /// and jumps to the target block. + /// partial class Branch : SimpleInstruction { readonly int targetILOffset; @@ -32,8 +40,6 @@ namespace ICSharpCode.Decompiler.IL /// /// Pops the specified number of arguments from the evaluation stack during the branching operation. - /// Note that the Branch instruction does not set InstructionFlags.MayPop -- the pop instead is considered - /// to happen after the branch was taken. /// public int PopCount; @@ -42,6 +48,16 @@ namespace ICSharpCode.Decompiler.IL this.targetILOffset = targetILOffset; } + protected override InstructionFlags ComputeFlags() + { + var flags = InstructionFlags.MayBranch | InstructionFlags.EndPointUnreachable; + if (PopCount > 0) { + // the branch pop happens during phase-2, so don't use MayPop + flags |= InstructionFlags.MayWriteEvaluationStack; + } + return flags; + } + public int TargetILOffset { get { return targetBlock != null ? targetBlock.ILRange.Start : targetILOffset; } } diff --git a/ICSharpCode.Decompiler/IL/Instructions/ILFunction.cs b/ICSharpCode.Decompiler/IL/Instructions/ILFunction.cs index cbdf32491..344462d20 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/ILFunction.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/ILFunction.cs @@ -37,8 +37,10 @@ namespace ICSharpCode.Decompiler.IL public override void WriteTo(ITextOutput output) { output.Write(OpCode); - output.Write(' '); - Method.WriteTo(output); + if (Method != null) { + output.Write(' '); + Method.WriteTo(output); + } output.WriteLine(" {"); output.Indent(); diff --git a/ICSharpCode.Decompiler/IL/TransformingVisitor.cs b/ICSharpCode.Decompiler/IL/TransformingVisitor.cs index b4018f8fc..70d43d8be 100644 --- a/ICSharpCode.Decompiler/IL/TransformingVisitor.cs +++ b/ICSharpCode.Decompiler/IL/TransformingVisitor.cs @@ -94,7 +94,11 @@ namespace ICSharpCode.Decompiler.IL stack.didInline = false; stack.error = false; inst = inst.Inline(InstructionFlags.None, stack); - Debug.Assert(stack.error == inst.HasFlag(InstructionFlags.MayPeek | InstructionFlags.MayPop)); + // An error implies that a peek or pop instruction wasn't replaced + // But even if we replaced all peek/pop instructions, we might have replaced them with + // another peek or pop instruction, so MayPeek/MayPop might still be set after + // we finish without error! + Debug.Assert(!stack.error || inst.HasFlag(InstructionFlags.MayPeek | InstructionFlags.MayPop)); } while (stack.didInline); // repeat transformations when something was inlined return inst; } @@ -105,44 +109,51 @@ namespace ICSharpCode.Decompiler.IL List output = new List(); for (int i = 0; i < block.Instructions.Count; i++) { var inst = block.Instructions[i]; - inst = DoInline(stack, inst); - if (inst.HasFlag(InstructionFlags.MayBranch - | InstructionFlags.MayPeek | InstructionFlags.MayPop + inst = DoInline(stack, inst); + if (inst.HasFlag(InstructionFlags.MayBranch | InstructionFlags.MayPop | InstructionFlags.MayReadEvaluationStack | InstructionFlags.MayWriteEvaluationStack)) { // Values currently on the stack might be used on both sides of the branch, // so we can't inline them. // We also have to flush the stack if the instruction still accesses the evaluation stack, // no matter whether in phase-1 or phase-2. FlushInstructionStack(stack, output); - } - if (inst.ResultType == StackType.Void) { - // We cannot directly push instructions onto the stack if they don't produce - // a result. - if (!stack.error && stack.Count > 0) { - // Wrap the instruction on top of the stack into an inline block, - // and append our void-typed instruction to the end of that block. - // TODO: I think this is wrong now that we changed the inline block semantics; - // we need to re-think how to build inline blocks. - var headInst = stack.Pop(); - var nestedBlock = headInst as Block ?? new Block { + } else if (inst.ResultType == StackType.Void && stack.Count > 0) { + // For void instructions on non-empty stack, we can create a new inline block (or add to an existing one) + // This works even when inst involves Peek. + ILInstruction headInst = stack.Pop(); + Block inlineBlock = headInst as Block; + if (inlineBlock == null || inlineBlock.FinalInstruction.OpCode != OpCode.Pop) { + inlineBlock = new Block { Instructions = { headInst }, - ILRange = headInst.ILRange + ILRange = new Interval(headInst.ILRange.Start, headInst.ILRange.Start), + FinalInstruction = new Pop(headInst.ResultType) }; - nestedBlock.Instructions.Add(inst); - stack.Push(nestedBlock); - } else { - // We can't move incomplete instructions into a nested block - // or the instruction stack was empty - FlushInstructionStack(stack, output); - output.Add(inst); } + inlineBlock.Instructions.Add(inst); + inst = inlineBlock; + } + if (inst.HasFlag(InstructionFlags.MayPeek)) { + // Prevent instruction from being inlined if it was peeked at. + FlushInstructionStack(stack, output); + } + if (inst.ResultType == StackType.Void) { + // We can't add void instructions to the stack, so flush the stack + // and directly add the instruction to the output. + FlushInstructionStack(stack, output); + output.Add(inst); } else { // Instruction has a result, so we can push it on the stack normally stack.Push(inst); } } // Allow inlining into the final instruction - block.FinalInstruction = DoInline(stack, block.FinalInstruction); + if (block.FinalInstruction.OpCode == OpCode.Pop && stack.Count > 0 && IsInlineBlock(stack.Peek())) { + // Don't inline an inline block into the final pop instruction: + // doing so would result in infinite recursion. + } else { + // regular inlining into the final instruction + block.FinalInstruction = DoInline(stack, block.FinalInstruction); + } FlushInstructionStack(stack, output); block.Instructions.ReplaceList(output); if (!(block.Parent is BlockContainer)) { @@ -151,12 +162,32 @@ namespace ICSharpCode.Decompiler.IL return block; } + bool IsInlineBlock(ILInstruction inst) + { + Block block = inst as Block; + return block != null && block.FinalInstruction.OpCode == OpCode.Pop; + } + void FlushInstructionStack(Stack stack, List output) { - output.AddRange(stack.Reverse()); + foreach (var inst in stack.Reverse()) { + AddToOutput(inst, output); + } stack.Clear(); } + void AddToOutput(ILInstruction inst, List output) + { + // Unpack inline blocks that would become direct children of the parent block + if (IsInlineBlock(inst)) { + foreach (var nestedInst in ((Block)inst).Instructions) { + AddToOutput(nestedInst, output); + } + } else { + output.Add(inst); + } + } + protected internal override ILInstruction VisitBlockContainer(BlockContainer container) { container.TransformChildren(this); diff --git a/ICSharpCode.Decompiler/Tests/ILTransforms/Inlining.cs b/ICSharpCode.Decompiler/Tests/ILTransforms/Inlining.cs index 36c1a0de4..5fc1add6b 100644 --- a/ICSharpCode.Decompiler/Tests/ILTransforms/Inlining.cs +++ b/ICSharpCode.Decompiler/Tests/ILTransforms/Inlining.cs @@ -16,6 +16,7 @@ // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER // DEALINGS IN THE SOFTWARE. using System; +using System.Diagnostics; using ICSharpCode.Decompiler.IL; using NUnit.Framework; using ICSharpCode.Decompiler.Tests.Helpers; @@ -91,5 +92,38 @@ namespace ICSharpCode.Decompiler.Tests.ILTransforms f.Body.ToString() ); } + + [Test] + public void BuildInlineBlock() + { + var f = MakeFunction( + new Block { + Instructions = { + new LdcI4(1), + new LdcI4(2), + new Call(TypeSystem.Action()) { Arguments = { new Peek(StackType.I4) } }, + new Call(TypeSystem.Action()) { Arguments = { new Peek(StackType.I4) } }, + new Call(TypeSystem.Action()) { Arguments = { new Pop(StackType.I4), new Pop(StackType.I4) } } + } + }); + f.AcceptVisitor(new TransformingVisitor()); + Debug.WriteLine(f.ToString()); + Assert.AreEqual( + new Call(TypeSystem.Action()) { + Arguments = { + new LdcI4(1), + new Block { + Instructions = { + new LdcI4(2), + new Call(TypeSystem.Action()) { Arguments = { new Peek(StackType.I4) } }, + new Call(TypeSystem.Action()) { Arguments = { new Peek(StackType.I4) } }, + }, + FinalInstruction = new Pop(StackType.I4) + } + } + }.ToString(), + f.Body.ToString() + ); + } } }