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); - } - } - } }