From 2dd5a38d0528919e012a07233b21833c32749482 Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Thu, 12 Mar 2015 22:50:23 +0100 Subject: [PATCH] Generalize inlining, and make it usable for symbolic phase-1 execution. --- .../ICSharpCode.Decompiler.csproj | 1 + ICSharpCode.Decompiler/IL/IInlineContext.cs | 55 ++++++++++++++++++ ICSharpCode.Decompiler/IL/InstructionFlags.cs | 4 ++ ICSharpCode.Decompiler/IL/Instructions.cs | 56 +++++++++---------- ICSharpCode.Decompiler/IL/Instructions.tt | 11 ++-- .../IL/Instructions/Block.cs | 24 ++------ .../IL/Instructions/BlockContainer.cs | 4 +- .../IL/Instructions/CallInstruction.cs | 7 +-- .../IL/Instructions/ILFunction.cs | 3 +- .../IL/Instructions/ILInstruction.cs | 16 ++++-- .../IL/Instructions/IfInstruction.cs | 19 +++++-- .../IL/Instructions/Return.cs | 6 +- .../IL/Instructions/SimpleInstruction.cs | 18 ++---- .../IL/Instructions/TryInstruction.cs | 17 +++--- .../IL/TransformingVisitor.cs | 56 +++++++++++++++---- doc/ILAst.txt | 3 +- 16 files changed, 191 insertions(+), 109 deletions(-) create mode 100644 ICSharpCode.Decompiler/IL/IInlineContext.cs diff --git a/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj b/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj index d4cdf3749..2f2197bcb 100644 --- a/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj +++ b/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj @@ -100,6 +100,7 @@ + diff --git a/ICSharpCode.Decompiler/IL/IInlineContext.cs b/ICSharpCode.Decompiler/IL/IInlineContext.cs new file mode 100644 index 000000000..917811180 --- /dev/null +++ b/ICSharpCode.Decompiler/IL/IInlineContext.cs @@ -0,0 +1,55 @@ +// Copyright (c) 2014 Daniel Grunwald +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. +using System; + +namespace ICSharpCode.Decompiler.IL +{ + /// + /// Execution context for phase 1: replace pop and peek instructions with evaluation stack values. + /// + interface IInlineContext + { + /// + /// Peeks at the top value on the evaluation stack, returning an instruction that represents + /// that value. + /// will replace instructions + /// with the value returned by this function. + /// + /// Combined instruction flags of the instructions + /// that the instruction getting inlined would get moved over. + /// + /// This method may return null when the evaluation stack is empty or the contents + /// are unknown. In this case, the peek instruction will not be replaced. + /// + ILInstruction Peek(InstructionFlags flagsBefore); + + /// + /// Pops the top value on the evaluation stack, returning an instruction that represents + /// that value. + /// will replace instructions + /// with the value returned by this function. + /// + /// Combined instruction flags of the instructions + /// that the instruction getting inlined would get moved over. + /// + /// This method may return null when the evaluation stack is empty or the contents + /// are unknown. In this case, the pop instruction will not be replaced. + /// + ILInstruction Pop(InstructionFlags flagsBefore); + } +} diff --git a/ICSharpCode.Decompiler/IL/InstructionFlags.cs b/ICSharpCode.Decompiler/IL/InstructionFlags.cs index ad7d3fd1c..331b303ef 100644 --- a/ICSharpCode.Decompiler/IL/InstructionFlags.cs +++ b/ICSharpCode.Decompiler/IL/InstructionFlags.cs @@ -64,6 +64,10 @@ namespace ICSharpCode.Decompiler.IL /// The instruction may have side effects, such as accessing heap memory, /// performing system calls, writing to local variables through pointers, etc. /// + /// + /// Throwing an exception or directly writing to local variables or the evaluation stack + /// is not considered a side effect, and is modeled by separate flags. + /// SideEffect = 0x40, /// diff --git a/ICSharpCode.Decompiler/IL/Instructions.cs b/ICSharpCode.Decompiler/IL/Instructions.cs index 49e9d5190..936ddcf93 100644 --- a/ICSharpCode.Decompiler/IL/Instructions.cs +++ b/ICSharpCode.Decompiler/IL/Instructions.cs @@ -196,9 +196,9 @@ namespace ICSharpCode.Decompiler.IL { this.Argument = this.argument.AcceptVisitor(visitor); } - internal override ILInstruction Inline(InstructionFlags flagsBefore, Stack instructionStack, out bool finished) + internal override ILInstruction Inline(InstructionFlags flagsBefore, IInlineContext context) { - this.Argument = this.argument.Inline(flagsBefore, instructionStack, out finished); + this.Argument = this.argument.Inline(flagsBefore, context); return this; } protected override InstructionFlags ComputeFlags() @@ -249,11 +249,10 @@ namespace ICSharpCode.Decompiler.IL this.Left = this.left.AcceptVisitor(visitor); this.Right = this.right.AcceptVisitor(visitor); } - internal override ILInstruction Inline(InstructionFlags flagsBefore, Stack instructionStack, out bool finished) + internal override ILInstruction Inline(InstructionFlags flagsBefore, IInlineContext context) { - this.Right = this.right.Inline(flagsBefore | ((this.left.Flags) & ~(InstructionFlags.MayPeek | InstructionFlags.MayPop)), instructionStack, out finished); - if (finished) - this.Left = this.left.Inline(flagsBefore, instructionStack, out finished); + this.Right = this.right.Inline(flagsBefore | ((this.left.Flags) & ~(InstructionFlags.MayPeek | InstructionFlags.MayPop)), context); + this.Left = this.left.Inline(flagsBefore, context); return this; } protected override InstructionFlags ComputeFlags() @@ -891,9 +890,9 @@ namespace ICSharpCode.Decompiler.IL { this.Value = this.value.AcceptVisitor(visitor); } - internal override ILInstruction Inline(InstructionFlags flagsBefore, Stack instructionStack, out bool finished) + internal override ILInstruction Inline(InstructionFlags flagsBefore, IInlineContext context) { - this.Value = this.value.Inline(flagsBefore, instructionStack, out finished); + this.Value = this.value.Inline(flagsBefore, context); return this; } readonly ILVariable variable; @@ -1170,9 +1169,9 @@ namespace ICSharpCode.Decompiler.IL { this.Target = this.target.AcceptVisitor(visitor); } - internal override ILInstruction Inline(InstructionFlags flagsBefore, Stack instructionStack, out bool finished) + internal override ILInstruction Inline(InstructionFlags flagsBefore, IInlineContext context) { - this.Target = this.target.Inline(flagsBefore, instructionStack, out finished); + this.Target = this.target.Inline(flagsBefore, context); return this; } /// Gets/Sets whether the memory access is volatile. @@ -1231,9 +1230,9 @@ namespace ICSharpCode.Decompiler.IL { this.Target = this.target.AcceptVisitor(visitor); } - internal override ILInstruction Inline(InstructionFlags flagsBefore, Stack instructionStack, out bool finished) + internal override ILInstruction Inline(InstructionFlags flagsBefore, IInlineContext context) { - this.Target = this.target.Inline(flagsBefore, instructionStack, out finished); + this.Target = this.target.Inline(flagsBefore, context); return this; } readonly IField field; @@ -1295,11 +1294,10 @@ namespace ICSharpCode.Decompiler.IL this.Target = this.target.AcceptVisitor(visitor); this.Value = this.value.AcceptVisitor(visitor); } - internal override ILInstruction Inline(InstructionFlags flagsBefore, Stack instructionStack, out bool finished) + internal override ILInstruction Inline(InstructionFlags flagsBefore, IInlineContext context) { - this.Value = this.value.Inline(flagsBefore | ((this.target.Flags) & ~(InstructionFlags.MayPeek | InstructionFlags.MayPop)), instructionStack, out finished); - if (finished) - this.Target = this.target.Inline(flagsBefore, instructionStack, out finished); + this.Value = this.value.Inline(flagsBefore | ((this.target.Flags) & ~(InstructionFlags.MayPeek | InstructionFlags.MayPop)), context); + this.Target = this.target.Inline(flagsBefore, context); return this; } /// Gets/Sets whether the memory access is volatile. @@ -1418,9 +1416,9 @@ namespace ICSharpCode.Decompiler.IL { this.Value = this.value.AcceptVisitor(visitor); } - internal override ILInstruction Inline(InstructionFlags flagsBefore, Stack instructionStack, out bool finished) + internal override ILInstruction Inline(InstructionFlags flagsBefore, IInlineContext context) { - this.Value = this.value.Inline(flagsBefore, instructionStack, out finished); + this.Value = this.value.Inline(flagsBefore, context); return this; } /// Gets/Sets whether the memory access is volatile. @@ -1535,9 +1533,9 @@ namespace ICSharpCode.Decompiler.IL { this.Target = this.target.AcceptVisitor(visitor); } - internal override ILInstruction Inline(InstructionFlags flagsBefore, Stack instructionStack, out bool finished) + internal override ILInstruction Inline(InstructionFlags flagsBefore, IInlineContext context) { - this.Target = this.target.Inline(flagsBefore, instructionStack, out finished); + this.Target = this.target.Inline(flagsBefore, context); return this; } readonly IType type; @@ -1607,11 +1605,10 @@ namespace ICSharpCode.Decompiler.IL this.Target = this.target.AcceptVisitor(visitor); this.Value = this.value.AcceptVisitor(visitor); } - internal override ILInstruction Inline(InstructionFlags flagsBefore, Stack instructionStack, out bool finished) + internal override ILInstruction Inline(InstructionFlags flagsBefore, IInlineContext context) { - this.Value = this.value.Inline(flagsBefore | ((this.target.Flags) & ~(InstructionFlags.MayPeek | InstructionFlags.MayPop)), instructionStack, out finished); - if (finished) - this.Target = this.target.Inline(flagsBefore, instructionStack, out finished); + this.Value = this.value.Inline(flagsBefore | ((this.target.Flags) & ~(InstructionFlags.MayPeek | InstructionFlags.MayPop)), context); + this.Target = this.target.Inline(flagsBefore, context); return this; } readonly IType type; @@ -1880,9 +1877,9 @@ namespace ICSharpCode.Decompiler.IL { this.Target = this.target.AcceptVisitor(visitor); } - internal override ILInstruction Inline(InstructionFlags flagsBefore, Stack instructionStack, out bool finished) + internal override ILInstruction Inline(InstructionFlags flagsBefore, IInlineContext context) { - this.Target = this.target.Inline(flagsBefore, instructionStack, out finished); + this.Target = this.target.Inline(flagsBefore, context); return this; } public override StackType ResultType { get { return StackType.I; } } @@ -1939,11 +1936,10 @@ namespace ICSharpCode.Decompiler.IL this.Array = this.array.AcceptVisitor(visitor); this.Index = this.index.AcceptVisitor(visitor); } - internal override ILInstruction Inline(InstructionFlags flagsBefore, Stack instructionStack, out bool finished) + internal override ILInstruction Inline(InstructionFlags flagsBefore, IInlineContext context) { - this.Index = this.index.Inline(flagsBefore | ((this.array.Flags) & ~(InstructionFlags.MayPeek | InstructionFlags.MayPop)), instructionStack, out finished); - if (finished) - this.Array = this.array.Inline(flagsBefore, instructionStack, out finished); + this.Index = this.index.Inline(flagsBefore | ((this.array.Flags) & ~(InstructionFlags.MayPeek | InstructionFlags.MayPop)), context); + this.Array = this.array.Inline(flagsBefore, context); return this; } readonly IType type; diff --git a/ICSharpCode.Decompiler/IL/Instructions.tt b/ICSharpCode.Decompiler/IL/Instructions.tt index 8dd5f7920..5380511ef 100644 --- a/ICSharpCode.Decompiler/IL/Instructions.tt +++ b/ICSharpCode.Decompiler/IL/Instructions.tt @@ -590,15 +590,12 @@ namespace ICSharpCode.Decompiler.IL opCode.Members.Add(b.ToString()); if (generateInline) { b = new StringBuilder(); - b.AppendLine("internal override ILInstruction Inline(InstructionFlags flagsBefore, Stack instructionStack, out bool finished)"); + b.AppendLine("internal override ILInstruction Inline(InstructionFlags flagsBefore, IInlineContext context)"); b.AppendLine("{"); for (int i = children.Length - 1; i >= 0; i--) { string arg = children[i].Name; - if (i < children.Length - 1) { - if (children[i].IsOptional) - b.AppendLine("\tif (finished && this." + arg + " != null)"); - else - b.AppendLine("\tif (finished)"); + if (children[i].IsOptional) { + b.AppendLine("\tif (this." + arg + " != null)"); b.Append("\t"); } b.Append("\tthis." + MakeName(arg) + " = this." + arg + ".Inline(flagsBefore"); @@ -607,7 +604,7 @@ namespace ICSharpCode.Decompiler.IL b.Append(string.Join(" | ", children.Take(i).Select(child2 => "this." + child2.Name + ".Flags"))); b.Append(") & ~(InstructionFlags.MayPeek | InstructionFlags.MayPop))"); } - b.AppendLine(", instructionStack, out finished);"); + b.AppendLine(", context);"); } b.AppendLine("\treturn this;"); b.Append("}"); diff --git a/ICSharpCode.Decompiler/IL/Instructions/Block.cs b/ICSharpCode.Decompiler/IL/Instructions/Block.cs index 55f07b6dd..97a638e71 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/Block.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/Block.cs @@ -41,15 +41,8 @@ namespace ICSharpCode.Decompiler.IL /// } /// /// - /// If the execution reaches the end of the block, one element is popped from the evaluation stack and - /// is used as the return value of the block. - /// This means it is impossible for a block to return void: any block with ResultType void must - /// perform an unconditional branch! + /// If the execution reaches the end of the block, the block returns the result value of the last instruction. /// - /// TODO: that seems like a bad idea for the purposes of goto-removal, I think we'll want - /// separate classes for void-blocks and inline-blocks. - /// Or maybe let blocks evaluate to the return value of their last instruction, and use a final pop() - /// instruction for inline-block semantics. /// TODO: actually I think it's a good idea to implement a small ILAst interpreter /// public virtual ILInstruction Phase1(InterpreterState state); /// public virtual InterpreterResult ILInstruction Phase2(InterpreterState state); @@ -77,17 +70,13 @@ namespace ICSharpCode.Decompiler.IL public override StackType ResultType { get { - return StackType.Void; + if (Instructions.Count == 0) + return StackType.Void; + else + return Instructions.Last().ResultType; } } - internal override void CheckInvariant() - { - base.CheckInvariant(); - // if the end-point isn't unreachable, there's an implicit pop at the end of the block - Debug.Assert(this.HasFlag(InstructionFlags.EndPointUnreachable) || this.ResultType != StackType.Void); - } - /// /// Gets the name of this block. /// @@ -156,10 +145,9 @@ namespace ICSharpCode.Decompiler.IL return flags; } - internal override ILInstruction Inline(InstructionFlags flagsBefore, Stack instructionStack, out bool finished) + internal override ILInstruction Inline(InstructionFlags flagsBefore, IInlineContext context) { // an inline block has no phase-1 effects, so we're immediately done with inlining - finished = true; return this; } } diff --git a/ICSharpCode.Decompiler/IL/Instructions/BlockContainer.cs b/ICSharpCode.Decompiler/IL/Instructions/BlockContainer.cs index 0a2c9677e..3b5cd61f0 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/BlockContainer.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/BlockContainer.cs @@ -97,9 +97,9 @@ namespace ICSharpCode.Decompiler.IL | (flagsInAllBlocks & InstructionFlags.EndPointUnreachable); } - internal override ILInstruction Inline(InstructionFlags flagsBefore, Stack instructionStack, out bool finished) + internal override ILInstruction Inline(InstructionFlags flagsBefore, IInlineContext context) { - finished = false; + // Blocks are phase-1 boundaries return this; } } diff --git a/ICSharpCode.Decompiler/IL/Instructions/CallInstruction.cs b/ICSharpCode.Decompiler/IL/Instructions/CallInstruction.cs index c366208f0..2ca0592fc 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/CallInstruction.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/CallInstruction.cs @@ -112,18 +112,15 @@ namespace ICSharpCode.Decompiler.IL output.Write(')'); } - internal override ILInstruction Inline(InstructionFlags flagsBefore, Stack instructionStack, out bool finished) + internal override ILInstruction Inline(InstructionFlags flagsBefore, IInlineContext context) { InstructionFlags operandFlags = InstructionFlags.None; for (int i = 0; i < Arguments.Count - 1; i++) { operandFlags |= Arguments[i].Flags; } flagsBefore |= operandFlags & ~(InstructionFlags.MayPeek | InstructionFlags.MayPop); - finished = true; for (int i = Arguments.Count - 1; i >= 0; i--) { - Arguments[i] = Arguments[i].Inline(flagsBefore, instructionStack, out finished); - if (!finished) - break; + Arguments[i] = Arguments[i].Inline(flagsBefore, context); } return this; } diff --git a/ICSharpCode.Decompiler/IL/Instructions/ILFunction.cs b/ICSharpCode.Decompiler/IL/Instructions/ILFunction.cs index 986226abe..cbdf32491 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/ILFunction.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/ILFunction.cs @@ -61,10 +61,9 @@ namespace ICSharpCode.Decompiler.IL return InstructionFlags.MayThrow; } - internal override ILInstruction Inline(InstructionFlags flagsBefore, Stack instructionStack, out bool finished) + internal override ILInstruction Inline(InstructionFlags flagsBefore, IInlineContext context) { // To the outside, lambda creation looks like a constant - finished = true; return this; } } diff --git a/ICSharpCode.Decompiler/IL/Instructions/ILInstruction.cs b/ICSharpCode.Decompiler/IL/Instructions/ILInstruction.cs index e5aa14df4..46d6ff80f 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/ILInstruction.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/ILInstruction.cs @@ -153,13 +153,21 @@ namespace ICSharpCode.Decompiler.IL public abstract void TransformChildren(ILVisitor visitor); /// - /// Attempts inlining from the instruction stack into this instruction. + /// Attempts inlining from the inline context into this instruction. /// /// Combined instruction flags of the instructions /// that the instructions getting inlined would get moved over. - /// The instruction stack. - /// Receives 'true' if all open 'pop' or 'peek' placeholders were inlined into; false otherwise. - internal abstract ILInstruction Inline(InstructionFlags flagsBefore, Stack instructionStack, out bool finished); + /// The inline context providing the values on the evaluation stack. + /// + /// Returns the modified ILInstruction after inlining is complete. + /// Note that inlining modifies the AST in-place, so this method usually returns this + /// (unless this should be replaced by another node) + /// + /// + /// Inlining from an inline context representing the actual evaluation stack + /// is equivalent to phase-1 execution of the instruction. + /// + internal abstract ILInstruction Inline(InstructionFlags flagsBefore, IInlineContext context); /// /// Number of parents that refer to this instruction and are connected to the root. diff --git a/ICSharpCode.Decompiler/IL/Instructions/IfInstruction.cs b/ICSharpCode.Decompiler/IL/Instructions/IfInstruction.cs index 833746d3c..abba07b84 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/IfInstruction.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/IfInstruction.cs @@ -22,6 +22,15 @@ using System.Diagnostics; namespace ICSharpCode.Decompiler.IL { + /// If statement / conditional expression. if (condition) trueExpr else falseExpr + /// + /// The condition must return StackType.I4, use comparison instructions like Ceq to check if other types are non-zero. + /// Phase-1 execution of an IfInstruction consists of phase-1 execution of the condition. + /// Phase-2 execution of an IfInstruction will phase-2-execute the condition. + /// If the condition evaluates to a non-zero, the TrueInst is executed (both phase-1 and phase-2). + /// If the condition evaluates to zero, the FalseInst is executed (both phase-1 and phase-2). + /// The return value of the IfInstruction is the return value of the TrueInst or FalseInst. + /// partial class IfInstruction : ILInstruction { public IfInstruction(ILInstruction condition, ILInstruction trueInst, ILInstruction falseInst = null) : base(OpCode.IfInstruction) @@ -43,18 +52,16 @@ namespace ICSharpCode.Decompiler.IL } } - internal override ILInstruction Inline(InstructionFlags flagsBefore, Stack instructionStack, out bool finished) + internal override ILInstruction Inline(InstructionFlags flagsBefore, IInlineContext context) { - this.Condition = condition.Inline(flagsBefore, instructionStack, out finished); - // don't continue inlining if this instruction still contains peek/pop instructions - if (HasFlag(InstructionFlags.MayPeek | InstructionFlags.MayPop)) - finished = false; + this.Condition = condition.Inline(flagsBefore, context); + // note: we skip TrueInst and FalseInst because there's a phase-1-boundary around them return this; } protected override InstructionFlags ComputeFlags() { - return condition.Flags | CombineFlags(trueInst.Flags, falseInst.Flags); + return condition.Flags | Block.Phase1Boundary(CombineFlags(trueInst.Flags, falseInst.Flags)); } internal static InstructionFlags CombineFlags(InstructionFlags trueFlags, InstructionFlags falseFlags) diff --git a/ICSharpCode.Decompiler/IL/Instructions/Return.cs b/ICSharpCode.Decompiler/IL/Instructions/Return.cs index 1b8b9f912..bc18e48b1 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/Return.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/Return.cs @@ -79,12 +79,10 @@ namespace ICSharpCode.Decompiler.IL this.ReturnValue = returnValue.AcceptVisitor(visitor); } - internal override ILInstruction Inline(InstructionFlags flagsBefore, Stack instructionStack, out bool finished) + internal override ILInstruction Inline(InstructionFlags flagsBefore, IInlineContext context) { if (returnValue != null) - this.ReturnValue = returnValue.Inline(flagsBefore, instructionStack, out finished); - else - finished = true; + this.ReturnValue = returnValue.Inline(flagsBefore, context); return this; } } diff --git a/ICSharpCode.Decompiler/IL/Instructions/SimpleInstruction.cs b/ICSharpCode.Decompiler/IL/Instructions/SimpleInstruction.cs index 43a20d2ea..c03292cb8 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/SimpleInstruction.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/SimpleInstruction.cs @@ -54,32 +54,26 @@ namespace ICSharpCode.Decompiler.IL return InstructionFlags.None; } - internal override ILInstruction Inline(InstructionFlags flagsBefore, Stack instructionStack, out bool finished) + internal override ILInstruction Inline(InstructionFlags flagsBefore, IInlineContext context) { - finished = true; // Nothing to do, since we don't have arguments. + // Nothing to do, since we don't have arguments. return this; } } partial class Pop { - internal override ILInstruction Inline(InstructionFlags flagsBefore, Stack instructionStack, out bool finished) + internal override ILInstruction Inline(InstructionFlags flagsBefore, IInlineContext context) { - if (instructionStack.Count > 0 && SemanticHelper.MayReorder(flagsBefore, instructionStack.Peek().Flags)) { - finished = true; - return instructionStack.Pop(); - } - finished = false; - return this; + return context.Pop(flagsBefore) ?? this; } } partial class Peek { - internal override ILInstruction Inline(InstructionFlags flagsBefore, Stack instructionStack, out bool finished) + internal override ILInstruction Inline(InstructionFlags flagsBefore, IInlineContext context) { - finished = false; - return this; + return context.Peek(flagsBefore) ?? this; } } } diff --git a/ICSharpCode.Decompiler/IL/Instructions/TryInstruction.cs b/ICSharpCode.Decompiler/IL/Instructions/TryInstruction.cs index 76b18681d..b68e937b5 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/TryInstruction.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/TryInstruction.cs @@ -38,10 +38,11 @@ namespace ICSharpCode.Decompiler.IL } } - internal override ILInstruction Inline(InstructionFlags flagsBefore, Stack instructionStack, out bool finished) + internal override ILInstruction Inline(InstructionFlags flagsBefore, IInlineContext context) { - // Cannot inline into try instructions because moving code into the try block would change semantics - finished = false; + // Inlining into exception-handling constructs would be madness. + // To keep phase-1 execution semantics consistent with inlining, there's a + // phase-1-boundary around every try/catch/finally/fault block. return this; } } @@ -79,7 +80,7 @@ namespace ICSharpCode.Decompiler.IL protected override InstructionFlags ComputeFlags() { - var flags = TryBlock.Flags; + var flags = Block.Phase1Boundary(TryBlock.Flags); foreach (var handler in Handlers) flags = IfInstruction.CombineFlags(flags, handler.Flags); return flags; @@ -117,7 +118,7 @@ namespace ICSharpCode.Decompiler.IL /// partial class TryCatchHandler { - internal override ILInstruction Inline(InstructionFlags flagsBefore, Stack instructionStack, out bool finished) + internal override ILInstruction Inline(InstructionFlags flagsBefore, IInlineContext context) { // should never happen as TryCatchHandler only appears within TryCatch instructions throw new InvalidOperationException(); @@ -136,7 +137,7 @@ namespace ICSharpCode.Decompiler.IL protected override InstructionFlags ComputeFlags() { - return Block.Phase1Boundary(filter.Flags | body.Flags); + return Block.Phase1Boundary(filter.Flags) | Block.Phase1Boundary(body.Flags); } public override void WriteTo(ITextOutput output) @@ -200,7 +201,7 @@ namespace ICSharpCode.Decompiler.IL protected override InstructionFlags ComputeFlags() { // if the endpoint of either the try or the finally is unreachable, the endpoint of the try-finally will be unreachable - return TryBlock.Flags | finallyBlock.Flags; + return Block.Phase1Boundary(TryBlock.Flags) | Block.Phase1Boundary(finallyBlock.Flags); } public override IEnumerable Children { @@ -248,7 +249,7 @@ namespace ICSharpCode.Decompiler.IL protected override InstructionFlags ComputeFlags() { // The endpoint of the try-fault is unreachable only if both endpoints are unreachable - return IfInstruction.CombineFlags(TryBlock.Flags, faultBlock.Flags); + return IfInstruction.CombineFlags(Block.Phase1Boundary(TryBlock.Flags), Block.Phase1Boundary(faultBlock.Flags)); } public override IEnumerable Children { diff --git a/ICSharpCode.Decompiler/IL/TransformingVisitor.cs b/ICSharpCode.Decompiler/IL/TransformingVisitor.cs index 29d8adaa6..4644250b5 100644 --- a/ICSharpCode.Decompiler/IL/TransformingVisitor.cs +++ b/ICSharpCode.Decompiler/IL/TransformingVisitor.cs @@ -54,20 +54,52 @@ namespace ICSharpCode.Decompiler.IL protected bool removeNops; + sealed class InliningStack : Stack, IInlineContext + { + /// + /// Indicates whether inlining was success for at least one + /// peek or pop instruction. + /// + internal bool didInline; + + /// + /// Indicates whether inlining encountered a peek or pop instruction + /// that could not be inlined. + /// + internal bool error; + + ILInstruction IInlineContext.Peek(InstructionFlags flagsBefore) + { + error = true; + return null; + } + + ILInstruction IInlineContext.Pop(InstructionFlags flagsBefore) + { + if (error) + return null; + if (base.Count > 0 && SemanticHelper.MayReorder(flagsBefore, base.Peek().Flags)) { + didInline = true; + return base.Pop(); + } + error = true; + return null; + } + } + protected internal override ILInstruction VisitBlock(Block block) { - Stack stack = new Stack(); + var stack = new InliningStack(); List output = new List(); for (int i = 0; i < block.Instructions.Count; i++) { var inst = block.Instructions[i]; - int stackCountBefore; - bool finished; do { inst = inst.AcceptVisitor(this); - stackCountBefore = stack.Count; - inst = inst.Inline(InstructionFlags.None, stack, out finished); - } while (stack.Count != stackCountBefore); // repeat transformations when something was inlined - if (inst.HasFlag(InstructionFlags.MayBranch) || !finished) { + stack.didInline = false; + stack.error = false; + inst = inst.Inline(InstructionFlags.None, stack); + } while (stack.didInline); // repeat transformations when something was inlined + if (stack.error || inst.HasFlag(InstructionFlags.MayBranch)) { // 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 some occurrence of 'pop' or 'peek' remains in the current instruction. @@ -76,9 +108,11 @@ namespace ICSharpCode.Decompiler.IL if (inst.ResultType == StackType.Void) { // We cannot directly push instructions onto the stack if they don't produce // a result. - if (finished && stack.Count > 0) { + 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 { Instructions = { headInst }, @@ -126,8 +160,10 @@ namespace ICSharpCode.Decompiler.IL // we can remove the container. if (container.Blocks.Count == 1 && container.EntryPoint.IncomingEdgeCount == 1) { // If the block has only one instruction, we can remove the block too - if (container.EntryPoint.Instructions.Count == 1) - return container.EntryPoint.Instructions[0]; + // (but only if this doesn't change the pop-order in the phase 1 evaluation of the parent block) + var instructions = container.EntryPoint.Instructions; + if (instructions.Count == 1 && !instructions[0].HasFlag(InstructionFlags.MayPeek | InstructionFlags.MayPop)) + return instructions[0]; return container.EntryPoint; } return container; diff --git a/doc/ILAst.txt b/doc/ILAst.txt index 8d8893388..0049717b6 100644 --- a/doc/ILAst.txt +++ b/doc/ILAst.txt @@ -62,7 +62,8 @@ For example, consider the ILAst for 'new List { 1 }.Length': call get_Length( { newobj List() call Add(peek, ldc.i4 1) - }) // inline blocks evaluate to the value they pushed onto the stack + pop + }) // inline blocks evaluate to the value of their last instruction When evaluating the 'call get_Length' instruction, in phase 1 we cannot completely replace all 'peek' and 'pop' instructions with values from the stack, because the List object is not yet pushed to the stack.