From 102729cfde299da70bfc278bbaad1f4d992da72f Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Sun, 8 Oct 2017 23:06:38 +0200 Subject: [PATCH] Put switch instructions into their own BlockContainer. This removes the need for "goto case" in the ILAst as we can just branch to the appropriate block. It also means we can support "break;" within switch using the "leave" instruction (otherwise we'd have to introduce yet another special kind of jump). --- .../CSharp/StatementBuilder.cs | 35 ++++++++- .../FlowAnalysis/DataFlowVisitor.cs | 12 +--- .../IL/ControlFlow/ConditionDetection.cs | 22 +----- .../IL/ControlFlow/LoopDetection.cs | 72 ++++++++++++++++--- ICSharpCode.Decompiler/IL/Instructions.cs | 49 ------------- ICSharpCode.Decompiler/IL/Instructions.tt | 3 - .../IL/Instructions/Block.cs | 3 + .../IL/Instructions/BlockContainer.cs | 2 +- .../IL/Instructions/SwitchInstruction.cs | 33 --------- 9 files changed, 102 insertions(+), 129 deletions(-) diff --git a/ICSharpCode.Decompiler/CSharp/StatementBuilder.cs b/ICSharpCode.Decompiler/CSharp/StatementBuilder.cs index 3ddb00fe9..12ba474aa 100644 --- a/ICSharpCode.Decompiler/CSharp/StatementBuilder.cs +++ b/ICSharpCode.Decompiler/CSharp/StatementBuilder.cs @@ -98,11 +98,17 @@ namespace ICSharpCode.Decompiler.CSharp } return new CaseLabel(exprBuilder.ConvertConstantValue(new ConstantResolveResult(type, value), allowImplicitConversion: true)); } - + protected internal override Statement VisitSwitchInstruction(SwitchInstruction inst) { + return TranslateSwitch(null, inst); + } + + SwitchStatement TranslateSwitch(BlockContainer switchContainer, SwitchInstruction inst) + { + Debug.Assert(switchContainer.EntryPoint.IncomingEdgeCount == 1); var oldBreakTarget = breakTarget; - breakTarget = null; // 'break' within a switch would only leave the switch + breakTarget = switchContainer; // 'break' within a switch would only leave the switch TranslatedExpression value; var strToInt = inst.Value as StringToInt; @@ -134,7 +140,28 @@ namespace ICSharpCode.Decompiler.CSharp ConvertSwitchSectionBody(astSection, section.Body); stmt.SwitchSections.Add(astSection); } - + if (switchContainer != null) { + // Translate any remaining blocks: + var lastSectionStatements = stmt.SwitchSections.Last().Statements; + foreach (var block in switchContainer.Blocks.Skip(1)) { + lastSectionStatements.Add(new LabelStatement { Label = block.Label }); + foreach (var nestedInst in block.Instructions) { + var nestedStmt = Convert(nestedInst); + if (nestedStmt is BlockStatement b) { + foreach (var nested in b.Statements) + lastSectionStatements.Add(nested.Detach()); + } else { + lastSectionStatements.Add(nestedStmt); + } + } + Debug.Assert(block.FinalInstruction.OpCode == OpCode.Nop); + } + if (endContainerLabels.TryGetValue(switchContainer, out string label)) { + lastSectionStatements.Add(new LabelStatement { Label = label }); + lastSectionStatements.Add(new BreakStatement()); + } + } + breakTarget = oldBreakTarget; return stmt; } @@ -471,6 +498,8 @@ namespace ICSharpCode.Decompiler.CSharp continueCount = oldContinueCount; breakTarget = oldBreakTarget; return loop; + } else if (container.EntryPoint.Instructions.Count == 1 && container.EntryPoint.Instructions[0] is SwitchInstruction switchInst) { + return TranslateSwitch(container, switchInst); } else { return ConvertBlockContainer(container, false); } diff --git a/ICSharpCode.Decompiler/FlowAnalysis/DataFlowVisitor.cs b/ICSharpCode.Decompiler/FlowAnalysis/DataFlowVisitor.cs index 0632edc79..034e33798 100644 --- a/ICSharpCode.Decompiler/FlowAnalysis/DataFlowVisitor.cs +++ b/ICSharpCode.Decompiler/FlowAnalysis/DataFlowVisitor.cs @@ -624,17 +624,7 @@ namespace ICSharpCode.Decompiler.FlowAnalysis state = afterSections; DebugEndPoint(inst); } - - protected internal override void VisitGotoCase(GotoCase inst) - { - // Handling goto case here is tricky: - // We'll need a fixed-point iteration for SwitchInstruction similar to BlockContainer, - // and we'll need to handle GotoCase like Branch, including stuff like ProcessBranchesLeavingTryFinally(). - // Hopefully we won't need a data-flow analysis in the decompiler pipeline after 'goto case' instructions - // are already introduced. - throw new NotImplementedException(); - } - + protected internal override void VisitYieldReturn(YieldReturn inst) { DebugStartPoint(inst); diff --git a/ICSharpCode.Decompiler/IL/ControlFlow/ConditionDetection.cs b/ICSharpCode.Decompiler/IL/ControlFlow/ConditionDetection.cs index 21da91022..954120bbf 100644 --- a/ICSharpCode.Decompiler/IL/ControlFlow/ConditionDetection.cs +++ b/ICSharpCode.Decompiler/IL/ControlFlow/ConditionDetection.cs @@ -65,10 +65,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow Debug.Assert(block.Instructions.Last().HasFlag(InstructionFlags.EndPointUnreachable)); ILInstruction exitInst = block.Instructions.Last(); - if (exitInst is SwitchInstruction switchInst) { - HandleSwitchInstruction(cfgNode, block, switchInst); - } - + // Previous-to-last instruction might have conditional control flow, // usually an IfInstruction with a branch: IfInstruction ifInst = block.Instructions.SecondToLastOrDefault() as IfInstruction; @@ -287,22 +284,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow && targetBlock.IncomingEdgeCount == 1 && targetBlock.FinalInstruction.OpCode == OpCode.Nop && cfgNode.Dominates(context.ControlFlowGraph.GetNode(targetBlock)); } - - private void HandleSwitchInstruction(ControlFlowNode cfgNode, Block block, SwitchInstruction sw) - { - // First, move blocks into the switch section - foreach (var section in sw.Sections) { - if (IsUsableBranchToChild(cfgNode, section.Body)) { - // case ...: goto targetBlock; - var targetBlock = ((Branch)section.Body).TargetBlock; - targetBlock.Remove(); - section.Body = targetBlock; - } - } - // TODO: find a common exit point among the cases, and if possible inline that into this block. - sw.Sections.ReplaceList(sw.Sections.OrderBy(s => s.Body.ILRange.Start)); - } - + private bool IsBranchOrLeave(ILInstruction inst) { switch (inst) { diff --git a/ICSharpCode.Decompiler/IL/ControlFlow/LoopDetection.cs b/ICSharpCode.Decompiler/IL/ControlFlow/LoopDetection.cs index b8086bee0..322543b05 100644 --- a/ICSharpCode.Decompiler/IL/ControlFlow/LoopDetection.cs +++ b/ICSharpCode.Decompiler/IL/ControlFlow/LoopDetection.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.Collections.Generic; using System.Diagnostics; using System.Linq; @@ -57,6 +58,11 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow // Because this is a post-order block transform, we can assume that // any nested loops within this loop have already been constructed. + if (block.Instructions.Last() is SwitchInstruction switchInst) { + // Switch instructions support "break;" just like loops + DetectSwitchBody(block, switchInst); + } + ControlFlowNode h = context.ControlFlowNode; // CFG node for our potential loop head Debug.Assert(h.UserData == block); Debug.Assert(!TreeTraversal.PreOrder(h, n => n.DominatorTreeChildren).Any(n => n.Visited)); @@ -101,7 +107,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow ConstructLoop(loop, exitPoint); } } - + /// /// Recurse into the dominator tree and find back edges/natural loops. /// @@ -412,13 +418,25 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow // Move contents of oldEntryPoint to newEntryPoint // (we can't move the block itself because it might be the target of branch instructions outside the loop) newEntryPoint.Instructions.ReplaceList(oldEntryPoint.Instructions); - newEntryPoint.FinalInstruction = oldEntryPoint.FinalInstruction; newEntryPoint.ILRange = oldEntryPoint.ILRange; oldEntryPoint.Instructions.ReplaceList(new[] { loopContainer }); if (exitTargetBlock != null) oldEntryPoint.Instructions.Add(new Branch(exitTargetBlock)); - oldEntryPoint.FinalInstruction = new Nop(); - + + MoveBlocksIntoContainer(loop, loopContainer); + + // Rewrite branches within the loop from oldEntryPoint to newEntryPoint: + foreach (var branch in loopContainer.Descendants.OfType()) { + if (branch.TargetBlock == oldEntryPoint) { + branch.TargetBlock = newEntryPoint; + } else if (branch.TargetBlock == exitTargetBlock) { + branch.ReplaceWith(new Leave(loopContainer) { ILRange = branch.ILRange }); + } + } + } + + private void MoveBlocksIntoContainer(List loop, BlockContainer loopContainer) + { // Move other blocks into the loop body: they're all dominated by the loop header, // and thus cannot be the target of branch instructions outside the loop. for (int i = 1; i < loop.Count; i++) { @@ -440,12 +458,48 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow Block block = (Block)loop[i].UserData; Debug.Assert(block.IsDescendantOf(loopContainer)); } + } + + private void DetectSwitchBody(Block block, SwitchInstruction switchInst) + { + Debug.Assert(block.Instructions.Last() == switchInst); + ControlFlowNode h = context.ControlFlowNode; // CFG node for our potential loop head + Debug.Assert(h.UserData == block); + Debug.Assert(!TreeTraversal.PreOrder(h, n => n.DominatorTreeChildren).Any(n => n.Visited)); + + var nodesInSwitch = new List(); + nodesInSwitch.Add(h); + h.Visited = true; + ExtendLoop(h, nodesInSwitch, out var exitPoint); + + context.Step("Create BlockContainer for switch", switchInst); + // Sort blocks in the loop in reverse post-order to make the output look a bit nicer. + // (if the loop doesn't contain nested loops, this is a topological sort) + nodesInSwitch.Sort((a, b) => b.PostOrderNumber.CompareTo(a.PostOrderNumber)); + Debug.Assert(nodesInSwitch[0] == h); + foreach (var node in nodesInSwitch) { + node.Visited = false; // reset visited flag so that we can find outer loops + Debug.Assert(h.Dominates(node) || !node.IsReachable, "The switch body must be dominated by the switch head"); + } + + BlockContainer switchContainer = new BlockContainer(); + Block newEntryPoint = new Block(); + newEntryPoint.ILRange = switchInst.ILRange; + switchContainer.Blocks.Add(newEntryPoint); + newEntryPoint.Instructions.Add(switchInst); + block.Instructions[block.Instructions.Count - 1] = switchContainer; + + Block exitTargetBlock = (Block)exitPoint?.UserData; + if (exitTargetBlock != null) { + block.Instructions.Add(new Branch(exitTargetBlock)); + } + + MoveBlocksIntoContainer(nodesInSwitch, switchContainer); + // Rewrite branches within the loop from oldEntryPoint to newEntryPoint: - foreach (var branch in loopContainer.Descendants.OfType()) { - if (branch.TargetBlock == oldEntryPoint) { - branch.TargetBlock = newEntryPoint; - } else if (branch.TargetBlock == exitTargetBlock) { - branch.ReplaceWith(new Leave(loopContainer) { ILRange = branch.ILRange }); + foreach (var branch in switchContainer.Descendants.OfType()) { + if (branch.TargetBlock == exitTargetBlock) { + branch.ReplaceWith(new Leave(switchContainer) { ILRange = branch.ILRange }); } } } diff --git a/ICSharpCode.Decompiler/IL/Instructions.cs b/ICSharpCode.Decompiler/IL/Instructions.cs index ccb7bdedf..0ae8a6815 100644 --- a/ICSharpCode.Decompiler/IL/Instructions.cs +++ b/ICSharpCode.Decompiler/IL/Instructions.cs @@ -63,8 +63,6 @@ namespace ICSharpCode.Decompiler.IL SwitchInstruction, /// Switch section within a switch statement SwitchSection, - /// Unconditional branch to switch section. goto case target; - GotoCase, /// Try-catch statement. TryCatch, /// Catch handler within a try-catch statement. @@ -1407,40 +1405,6 @@ namespace ICSharpCode.Decompiler.IL } } namespace ICSharpCode.Decompiler.IL -{ - /// Unconditional branch to switch section. goto case target; - public sealed partial class GotoCase : SimpleInstruction - { - public override StackType ResultType { get { return StackType.Void; } } - protected override InstructionFlags ComputeFlags() - { - return InstructionFlags.EndPointUnreachable | InstructionFlags.MayBranch; - } - public override InstructionFlags DirectFlags { - get { - return InstructionFlags.EndPointUnreachable | InstructionFlags.MayBranch; - } - } - public override void AcceptVisitor(ILVisitor visitor) - { - visitor.VisitGotoCase(this); - } - public override T AcceptVisitor(ILVisitor visitor) - { - return visitor.VisitGotoCase(this); - } - public override T AcceptVisitor(ILVisitor visitor, C context) - { - return visitor.VisitGotoCase(this, context); - } - protected internal override bool PerformMatch(ILInstruction other, ref Patterns.Match match) - { - var o = other as GotoCase; - return o != null && this.TargetSection == o.TargetSection; - } - } -} -namespace ICSharpCode.Decompiler.IL { /// Try-catch statement. public sealed partial class TryCatch : TryInstruction @@ -4573,10 +4537,6 @@ namespace ICSharpCode.Decompiler.IL { Default(inst); } - protected internal virtual void VisitGotoCase(GotoCase inst) - { - Default(inst); - } protected internal virtual void VisitTryCatch(TryCatch inst) { Default(inst); @@ -4867,10 +4827,6 @@ namespace ICSharpCode.Decompiler.IL { return Default(inst); } - protected internal virtual T VisitGotoCase(GotoCase inst) - { - return Default(inst); - } protected internal virtual T VisitTryCatch(TryCatch inst) { return Default(inst); @@ -5161,10 +5117,6 @@ namespace ICSharpCode.Decompiler.IL { return Default(inst, context); } - protected internal virtual T VisitGotoCase(GotoCase inst, C context) - { - return Default(inst, context); - } protected internal virtual T VisitTryCatch(TryCatch inst, C context) { return Default(inst, context); @@ -5399,7 +5351,6 @@ namespace ICSharpCode.Decompiler.IL "if.notnull", "switch", "switch.section", - "goto.case", "try.catch", "try.catch.handler", "try.finally", diff --git a/ICSharpCode.Decompiler/IL/Instructions.tt b/ICSharpCode.Decompiler/IL/Instructions.tt index c75172ddf..ca7590811 100644 --- a/ICSharpCode.Decompiler/IL/Instructions.tt +++ b/ICSharpCode.Decompiler/IL/Instructions.tt @@ -98,9 +98,6 @@ CustomClassName("SwitchSection"), CustomChildren(new [] { new ChildInfo("body") }), CustomConstructor, CustomComputeFlags, CustomWriteTo, ResultType("Void"), MatchCondition("this.Labels.SetEquals(o.Labels) && this.HasNullLabel == o.HasNullLabel")), - new OpCode("goto.case", "Unconditional branch to switch section. goto case target;", - CustomClassName("GotoCase"), NoArguments, CustomConstructor, UnconditionalBranch, MayBranch, - MatchCondition("this.TargetSection == o.TargetSection")), new OpCode("try.catch", "Try-catch statement.", BaseClass("TryInstruction"), CustomConstructor, CustomComputeFlags, CustomWriteTo, MatchCondition("TryBlock.PerformMatch(o.TryBlock, ref match)"), diff --git a/ICSharpCode.Decompiler/IL/Instructions/Block.cs b/ICSharpCode.Decompiler/IL/Instructions/Block.cs index 3931beece..0c5f9b7cc 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/Block.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/Block.cs @@ -108,6 +108,9 @@ namespace ICSharpCode.Decompiler.IL // only the last instruction may have an unreachable endpoint Debug.Assert(!Instructions[i].HasFlag(InstructionFlags.EndPointUnreachable)); } + if (this.Type == BlockType.ControlFlow) { + Debug.Assert(finalInstruction.OpCode == OpCode.Nop); + } } public override StackType ResultType { diff --git a/ICSharpCode.Decompiler/IL/Instructions/BlockContainer.cs b/ICSharpCode.Decompiler/IL/Instructions/BlockContainer.cs index 5cf84221c..c48021ac7 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/BlockContainer.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/BlockContainer.cs @@ -154,7 +154,7 @@ namespace ICSharpCode.Decompiler.IL Debug.Assert(EntryPoint == Blocks[0]); Debug.Assert(!IsConnected || EntryPoint.IncomingEdgeCount >= 1); Debug.Assert(Blocks.All(b => b.HasFlag(InstructionFlags.EndPointUnreachable))); - Debug.Assert(Blocks.All(b => b.FinalInstruction.OpCode == OpCode.Nop)); + Debug.Assert(Blocks.All(b => b.Type == BlockType.ControlFlow)); // this also implies that the blocks don't use FinalInstruction } protected override InstructionFlags ComputeFlags() diff --git a/ICSharpCode.Decompiler/IL/Instructions/SwitchInstruction.cs b/ICSharpCode.Decompiler/IL/Instructions/SwitchInstruction.cs index 5de07f592..ae828983f 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/SwitchInstruction.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/SwitchInstruction.cs @@ -196,37 +196,4 @@ namespace ICSharpCode.Decompiler.IL body.WriteTo(output, options); } } - - /// - /// Unconditional branch to switch section. goto case target;/goto default; - /// - /// - /// Like normal branches, can trigger finally blocks. - /// - partial class GotoCase // : IBranchOrLeaveInstruction - { - public SwitchSection TargetSection { get; } - - public GotoCase(SwitchSection target) : base(OpCode.GotoCase) - { - this.TargetSection = target ?? throw new ArgumentNullException(nameof(target)); - } - - internal override void CheckInvariant(ILPhase phase) - { - base.CheckInvariant(phase); - var parentSwitch = this.Ancestors.OfType().First(); - Debug.Assert(parentSwitch == TargetSection.Parent); - } - - public override void WriteTo(ITextOutput output, ILAstWritingOptions options) - { - output.Write("goto.case "); - if (TargetSection.HasNullLabel) { - output.WriteReference("null", TargetSection, isLocal: true); - } else { - output.WriteReference(TargetSection.Labels.Values.First().ToString(), TargetSection, isLocal: true); - } - } - } }