From d8244e347b79650775e20c2eb8713e4cd3896409 Mon Sep 17 00:00:00 2001 From: Chicken-Bones Date: Wed, 25 Jul 2018 23:16:14 +1000 Subject: [PATCH] Select outer-loop continue branches as break targets for switches in nested loops --- .../TestCases/Pretty/Switch.cs | 24 +++ .../TestCases/Pretty/Switch.il | 87 ++++++++++ .../TestCases/Pretty/Switch.opt.il | 69 ++++++++ .../TestCases/Pretty/Switch.opt.roslyn.il | 76 +++++++++ .../TestCases/Pretty/Switch.roslyn.il | 99 +++++++++++ .../IL/ControlFlow/LoopDetection.cs | 68 +++++--- .../IL/ControlFlow/SwitchDetection.cs | 155 +++++++++++------- 7 files changed, 496 insertions(+), 82 deletions(-) diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Switch.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Switch.cs index f16c18451..ed55a23ae 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Switch.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Switch.cs @@ -832,6 +832,30 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty Console.WriteLine("end"); } + public static void SwitchWithContinueInDoubleLoop() + { + bool value = false; + for (int i = 0; i < 10; i++) { + for (int j = 0; j < 10; j++) { + switch (i + j) { + case 1: + case 3: + case 5: + case 7: + case 11: + case 13: + case 17: + break; + default: + continue; + } + value = true; + break; + } + } + Console.WriteLine(value); + } + public static void SwitchLoopNesting() { for (int i = 0; i < 10; i++) { diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Switch.il b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Switch.il index 4a411c095..eea9abefe 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Switch.il +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Switch.il @@ -2674,6 +2674,93 @@ IL_0050: ret } // end of method Switch::SwitchWithContinue7 + .method public hidebysig static void SwitchWithContinueInDoubleLoop() cil managed + { + // Code size 128 (0x80) + .maxstack 2 + .locals init (bool V_0, + int32 V_1, + int32 V_2, + int32 V_3, + bool V_4) + IL_0000: nop + IL_0001: ldc.i4.0 + IL_0002: stloc.0 + IL_0003: ldc.i4.0 + IL_0004: stloc.1 + IL_0005: br.s IL_006d + + IL_0007: nop + IL_0008: ldc.i4.0 + IL_0009: stloc.2 + IL_000a: br.s IL_005d + + IL_000c: nop + IL_000d: ldloc.1 + IL_000e: ldloc.2 + IL_000f: add + IL_0010: stloc.3 + IL_0011: ldloc.3 + IL_0012: ldc.i4.1 + IL_0013: sub + IL_0014: switch ( + IL_0051, + IL_0053, + IL_0051, + IL_0053, + IL_0051, + IL_0053, + IL_0051) + IL_0035: ldloc.3 + IL_0036: ldc.i4.s 11 + IL_0038: sub + IL_0039: switch ( + IL_0051, + IL_0053, + IL_0051) + IL_004a: ldloc.3 + IL_004b: ldc.i4.s 17 + IL_004d: beq.s IL_0051 + + IL_004f: br.s IL_0053 + + IL_0051: br.s IL_0055 + + IL_0053: br.s IL_0059 + + IL_0055: ldc.i4.1 + IL_0056: stloc.0 + IL_0057: br.s IL_0068 + + IL_0059: ldloc.2 + IL_005a: ldc.i4.1 + IL_005b: add + IL_005c: stloc.2 + IL_005d: ldloc.2 + IL_005e: ldc.i4.s 10 + IL_0060: clt + IL_0062: stloc.s V_4 + IL_0064: ldloc.s V_4 + IL_0066: brtrue.s IL_000c + + IL_0068: nop + IL_0069: ldloc.1 + IL_006a: ldc.i4.1 + IL_006b: add + IL_006c: stloc.1 + IL_006d: ldloc.1 + IL_006e: ldc.i4.s 10 + IL_0070: clt + IL_0072: stloc.s V_4 + IL_0074: ldloc.s V_4 + IL_0076: brtrue.s IL_0007 + + IL_0078: ldloc.0 + IL_0079: call void [mscorlib]System.Console::WriteLine(bool) + IL_007e: nop + IL_007f: ret + } // end of method Switch::SwitchWithContinueInDoubleLoop + .method public hidebysig static void SwitchLoopNesting() cil managed { // Code size 153 (0x99) diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Switch.opt.il b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Switch.opt.il index 754475712..b9809e001 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Switch.opt.il +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Switch.opt.il @@ -2146,6 +2146,75 @@ IL_003c: ret } // end of method Switch::SwitchWithContinue7 + .method public hidebysig static void SwitchWithContinueInDoubleLoop() cil managed + { + // Code size 105 (0x69) + .maxstack 2 + .locals init (bool V_0, + int32 V_1, + int32 V_2, + int32 V_3) + IL_0000: ldc.i4.0 + IL_0001: stloc.0 + IL_0002: ldc.i4.0 + IL_0003: stloc.1 + IL_0004: br.s IL_005d + + IL_0006: ldc.i4.0 + IL_0007: stloc.2 + IL_0008: br.s IL_0054 + + IL_000a: ldloc.1 + IL_000b: ldloc.2 + IL_000c: add + IL_000d: stloc.3 + IL_000e: ldloc.3 + IL_000f: ldc.i4.1 + IL_0010: sub + IL_0011: switch ( + IL_004c, + IL_0050, + IL_004c, + IL_0050, + IL_004c, + IL_0050, + IL_004c) + IL_0032: ldloc.3 + IL_0033: ldc.i4.s 11 + IL_0035: sub + IL_0036: switch ( + IL_004c, + IL_0050, + IL_004c) + IL_0047: ldloc.3 + IL_0048: ldc.i4.s 17 + IL_004a: bne.un.s IL_0050 + + IL_004c: ldc.i4.1 + IL_004d: stloc.0 + IL_004e: br.s IL_0059 + + IL_0050: ldloc.2 + IL_0051: ldc.i4.1 + IL_0052: add + IL_0053: stloc.2 + IL_0054: ldloc.2 + IL_0055: ldc.i4.s 10 + IL_0057: blt.s IL_000a + + IL_0059: ldloc.1 + IL_005a: ldc.i4.1 + IL_005b: add + IL_005c: stloc.1 + IL_005d: ldloc.1 + IL_005e: ldc.i4.s 10 + IL_0060: blt.s IL_0006 + + IL_0062: ldloc.0 + IL_0063: call void [mscorlib]System.Console::WriteLine(bool) + IL_0068: ret + } // end of method Switch::SwitchWithContinueInDoubleLoop + .method public hidebysig static void SwitchLoopNesting() cil managed { // Code size 101 (0x65) diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Switch.opt.roslyn.il b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Switch.opt.roslyn.il index 964373488..526a58edd 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Switch.opt.roslyn.il +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Switch.opt.roslyn.il @@ -2305,6 +2305,82 @@ IL_0033: ret } // end of method Switch::SwitchWithContinue7 + .method public hidebysig static void SwitchWithContinueInDoubleLoop() cil managed + { + // Code size 101 (0x65) + .maxstack 2 + .locals init (bool V_0, + int32 V_1, + int32 V_2, + int32 V_3) + IL_0000: ldc.i4.0 + IL_0001: stloc.0 + IL_0002: ldc.i4.0 + IL_0003: stloc.1 + IL_0004: br.s IL_0059 + + IL_0006: ldc.i4.0 + IL_0007: stloc.2 + IL_0008: br.s IL_0050 + + IL_000a: ldloc.1 + IL_000b: ldloc.2 + IL_000c: add + IL_000d: stloc.3 + IL_000e: ldloc.3 + IL_000f: ldc.i4.s 11 + IL_0011: bgt.s IL_003e + + IL_0013: ldloc.3 + IL_0014: ldc.i4.1 + IL_0015: sub + IL_0016: switch ( + IL_0048, + IL_004c, + IL_0048, + IL_004c, + IL_0048, + IL_004c, + IL_0048) + IL_0037: ldloc.3 + IL_0038: ldc.i4.s 11 + IL_003a: beq.s IL_0048 + + IL_003c: br.s IL_004c + + IL_003e: ldloc.3 + IL_003f: ldc.i4.s 13 + IL_0041: beq.s IL_0048 + + IL_0043: ldloc.3 + IL_0044: ldc.i4.s 17 + IL_0046: bne.un.s IL_004c + + IL_0048: ldc.i4.1 + IL_0049: stloc.0 + IL_004a: br.s IL_0055 + + IL_004c: ldloc.2 + IL_004d: ldc.i4.1 + IL_004e: add + IL_004f: stloc.2 + IL_0050: ldloc.2 + IL_0051: ldc.i4.s 10 + IL_0053: blt.s IL_000a + + IL_0055: ldloc.1 + IL_0056: ldc.i4.1 + IL_0057: add + IL_0058: stloc.1 + IL_0059: ldloc.1 + IL_005a: ldc.i4.s 10 + IL_005c: blt.s IL_0006 + + IL_005e: ldloc.0 + IL_005f: call void [mscorlib]System.Console::WriteLine(bool) + IL_0064: ret + } // end of method Switch::SwitchWithContinueInDoubleLoop + .method public hidebysig static void SwitchLoopNesting() cil managed { // Code size 92 (0x5c) diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Switch.roslyn.il b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Switch.roslyn.il index aafd8f990..e5f3b8b50 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Switch.roslyn.il +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Switch.roslyn.il @@ -3019,6 +3019,105 @@ IL_004b: ret } // end of method Switch::SwitchWithContinue7 + .method public hidebysig static void SwitchWithContinueInDoubleLoop() cil managed + { + // Code size 128 (0x80) + .maxstack 2 + .locals init (bool V_0, + int32 V_1, + int32 V_2, + int32 V_3, + bool V_4, + bool V_5) + IL_0000: nop + IL_0001: ldc.i4.0 + IL_0002: stloc.0 + IL_0003: ldc.i4.0 + IL_0004: stloc.1 + IL_0005: br.s IL_006d + + IL_0007: nop + IL_0008: ldc.i4.0 + IL_0009: stloc.2 + IL_000a: br.s IL_005d + + IL_000c: nop + IL_000d: ldloc.1 + IL_000e: ldloc.2 + IL_000f: add + IL_0010: stloc.3 + IL_0011: ldloc.3 + IL_0012: ldc.i4.s 11 + IL_0014: bgt.s IL_0043 + + IL_0016: ldloc.3 + IL_0017: ldc.i4.1 + IL_0018: sub + IL_0019: switch ( + IL_0051, + IL_0053, + IL_0051, + IL_0053, + IL_0051, + IL_0053, + IL_0051) + IL_003a: br.s IL_003c + + IL_003c: ldloc.3 + IL_003d: ldc.i4.s 11 + IL_003f: beq.s IL_0051 + + IL_0041: br.s IL_0053 + + IL_0043: ldloc.3 + IL_0044: ldc.i4.s 13 + IL_0046: beq.s IL_0051 + + IL_0048: br.s IL_004a + + IL_004a: ldloc.3 + IL_004b: ldc.i4.s 17 + IL_004d: beq.s IL_0051 + + IL_004f: br.s IL_0053 + + IL_0051: br.s IL_0055 + + IL_0053: br.s IL_0059 + + IL_0055: ldc.i4.1 + IL_0056: stloc.0 + IL_0057: br.s IL_0068 + + IL_0059: ldloc.2 + IL_005a: ldc.i4.1 + IL_005b: add + IL_005c: stloc.2 + IL_005d: ldloc.2 + IL_005e: ldc.i4.s 10 + IL_0060: clt + IL_0062: stloc.s V_4 + IL_0064: ldloc.s V_4 + IL_0066: brtrue.s IL_000c + + IL_0068: nop + IL_0069: ldloc.1 + IL_006a: ldc.i4.1 + IL_006b: add + IL_006c: stloc.1 + IL_006d: ldloc.1 + IL_006e: ldc.i4.s 10 + IL_0070: clt + IL_0072: stloc.s V_5 + IL_0074: ldloc.s V_5 + IL_0076: brtrue.s IL_0007 + + IL_0078: ldloc.0 + IL_0079: call void [mscorlib]System.Console::WriteLine(bool) + IL_007e: nop + IL_007f: ret + } // end of method Switch::SwitchWithContinueInDoubleLoop + .method public hidebysig static void SwitchLoopNesting() cil managed { // Code size 140 (0x8c) diff --git a/ICSharpCode.Decompiler/IL/ControlFlow/LoopDetection.cs b/ICSharpCode.Decompiler/IL/ControlFlow/LoopDetection.cs index f80610834..9e0f4e0bc 100644 --- a/ICSharpCode.Decompiler/IL/ControlFlow/LoopDetection.cs +++ b/ICSharpCode.Decompiler/IL/ControlFlow/LoopDetection.cs @@ -42,6 +42,15 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow /// Block container corresponding to the current cfg. BlockContainer currentBlockContainer; + + /// + /// Enabled during DetectSwitchBody, used by ExtendLoop and children + /// + private bool isSwitch; + /// + /// Used when isSwitch == true, to determine appropriate exit points within loops + /// + private SwitchDetection.LoopContext loopContext; /// /// Check whether 'block' is a loop head; and construct a loop instruction @@ -234,16 +243,16 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow /// /// Requires and maintains the invariant that a node is marked as visited iff it is contained in the loop. /// - void ExtendLoop(ControlFlowNode loopHead, List loop, out ControlFlowNode exitPoint, bool isSwitch=false) + void ExtendLoop(ControlFlowNode loopHead, List loop, out ControlFlowNode exitPoint) { - exitPoint = FindExitPoint(loopHead, loop, isSwitch); + exitPoint = FindExitPoint(loopHead, loop); Debug.Assert(!loop.Contains(exitPoint), "Cannot pick an exit point that is part of the natural loop"); if (exitPoint != null) { // Either we are in case 1 and just picked an exit that maximizes the amount of code // outside the loop, or we are in case 2 and found an exit point via post-dominance. // Note that if exitPoint == NoExitPoint, we end up adding all dominated blocks to the loop. var ep = exitPoint; - foreach (var node in TreeTraversal.PreOrder(loopHead, n => DominatorTreeChildren(n, ep, isSwitch))) { + foreach (var node in TreeTraversal.PreOrder(loopHead, n => DominatorTreeChildren(n, ep))) { if (!node.Visited) { node.Visited = true; loop.Add(node); @@ -272,9 +281,9 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow /// 3) otherwise (exit point unknown, heuristically extend loop): null /// /// This method must not write to the Visited flags on the CFG. - internal ControlFlowNode FindExitPoint(ControlFlowNode loopHead, IReadOnlyList naturalLoop, bool isSwitch) + internal ControlFlowNode FindExitPoint(ControlFlowNode loopHead, IReadOnlyList naturalLoop) { - bool hasReachableExit = HasReachableExit(loopHead, isSwitch); + bool hasReachableExit = HasReachableExit(loopHead); if (!hasReachableExit) { // Case 1: // There are no nodes n so that loopHead dominates a predecessor of n but not n itself @@ -292,7 +301,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow ControlFlowNode exitPoint = null; int exitPointILOffset = -1; foreach (var node in loopHead.DominatorTreeChildren) { - PickExitPoint(node, isSwitch, ref exitPoint, ref exitPointILOffset); + PickExitPoint(node, ref exitPoint, ref exitPointILOffset); } return exitPoint; } else { @@ -300,7 +309,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow // We need to pick our exit point so that all paths from the loop head // to the reachable exits run through that exit point. var cfg = context.ControlFlowGraph.cfg; - var revCfg = PrepareReverseCFG(loopHead, isSwitch, out int exitNodeArity); + var revCfg = PrepareReverseCFG(loopHead, out int exitNodeArity); //ControlFlowNode.ExportGraph(cfg).Show("cfg"); //ControlFlowNode.ExportGraph(revCfg).Show("rev"); ControlFlowNode commonAncestor = revCfg[loopHead.UserIndex]; @@ -336,8 +345,10 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow if (exitNodeArity > 1) return null; + // Exit node is on the very edge of the tree, and isn't important for determining inclusion + // Still necessary for switch detection to insert correct leave statements if (exitNodeArity == 1 && isSwitch) - return SwitchDetection.GetBreakTargets(loopHead, loopHead).Distinct().Single(); + return loopContext.GetBreakTargets(loopHead).Distinct().Single(); // If exitNodeArity == 0, we should maybe look test if our exits out of the block container are all compatible? // but I don't think it hurts to have a bit too much code inside the loop in this rare case. @@ -382,12 +393,21 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow } } - bool HasReachableExit(ControlFlowNode node, bool isSwitch) => isSwitch - ? SwitchDetection.GetBreakTargets(context.ControlFlowNode, node).Any() + /// + /// Extension of ControlFlowGraph.HasReachableExit + /// Uses loopContext.GetBreakTargets().Any() when analyzing switches to avoid + /// classifying continue blocks as reachable exits. + /// + bool HasReachableExit(ControlFlowNode node) => isSwitch + ? loopContext.GetBreakTargets(node).Any() : context.ControlFlowGraph.HasReachableExit(node); - IEnumerable DominatorTreeChildren(ControlFlowNode n, ControlFlowNode exitPoint, bool isSwitch) => - n.DominatorTreeChildren.Where(c => c != exitPoint && (!isSwitch || !SwitchDetection.IsContinue(context.ControlFlowNode, c))); + /// + /// Returns the children in a loop dominator tree, with an optional exit point + /// Avoids returning continue statements when analysing switches (because increment blocks can be dominated) + /// + IEnumerable DominatorTreeChildren(ControlFlowNode n, ControlFlowNode exitPoint) => + n.DominatorTreeChildren.Where(c => c != exitPoint && (!isSwitch || !loopContext.MatchContinue(c))); /// /// Pick exit point by picking any node that has no reachable exits. @@ -399,14 +419,14 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow /// /// Code amount in and its dominated nodes. /// This method must not write to the Visited flags on the CFG. - void PickExitPoint(ControlFlowNode node, bool isSwitch, ref ControlFlowNode exitPoint, ref int exitPointILOffset) + void PickExitPoint(ControlFlowNode node, ref ControlFlowNode exitPoint, ref int exitPointILOffset) { - if (isSwitch && SwitchDetection.IsContinue(context.ControlFlowNode, node)) + if (isSwitch && loopContext.MatchContinue(node)) return; Block block = (Block)node.UserData; if (block.ILRange.Start > exitPointILOffset - && !HasReachableExit(node, isSwitch) + && !HasReachableExit(node) && ((Block)node.UserData).Parent == currentBlockContainer) { // HasReachableExit(node) == false @@ -425,7 +445,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow // moving almost all of the code into the loop. } foreach (var child in node.DominatorTreeChildren) { - PickExitPoint(child, isSwitch, ref exitPoint, ref exitPointILOffset); + PickExitPoint(child, ref exitPoint, ref exitPointILOffset); } } @@ -446,7 +466,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow /// 2 = more than one CFG node (not dominated by loopHead) was used as an exit node. /// /// - ControlFlowNode[] PrepareReverseCFG(ControlFlowNode loopHead, bool isSwitch, out int exitNodeArity) + ControlFlowNode[] PrepareReverseCFG(ControlFlowNode loopHead, out int exitNodeArity) { ControlFlowNode[] cfg = context.ControlFlowGraph.cfg; ControlFlowNode[] rev = new ControlFlowNode[cfg.Length + 1]; @@ -458,12 +478,13 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow ControlFlowNode exitNode = new ControlFlowNode { UserIndex = -1 }; rev[cfg.Length] = exitNode; for (int i = 0; i < cfg.Length; i++) { - if (!loopHead.Dominates(cfg[i]) || isSwitch && SwitchDetection.IsContinue(loopHead, cfg[i])) + if (!loopHead.Dominates(cfg[i]) || isSwitch && cfg[i] != loopHead && loopContext.MatchContinue(cfg[i])) continue; // Add reverse edges for all edges in cfg foreach (var succ in cfg[i].Successors) { - if (isSwitch && SwitchDetection.IsContinue(loopHead, succ)) + // edges to outer loops still count as exits (labelled continue not implemented) + if (isSwitch && loopContext.MatchContinue(succ, 1)) continue; if (loopHead.Dominates(succ)) { @@ -669,11 +690,14 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow Debug.Assert(h.UserData == block); Debug.Assert(!TreeTraversal.PreOrder(h, n => n.DominatorTreeChildren).Any(n => n.Visited)); + isSwitch = true; + loopContext = new SwitchDetection.LoopContext(context.ControlFlowGraph, h); + var nodesInSwitch = new List(); nodesInSwitch.Add(h); h.Visited = true; - ExtendLoop(h, nodesInSwitch, out var exitPoint, isSwitch: true); - if (exitPoint != null && exitPoint.Predecessors.Count == 1 && !HasReachableExit(exitPoint, true)) { + ExtendLoop(h, nodesInSwitch, out var exitPoint); + if (exitPoint != null && h.Dominates(exitPoint) && exitPoint.Predecessors.Count == 1 && !HasReachableExit(exitPoint)) { // If the exit point is reachable from just one single "break;", // it's better to move the code into the switch. // (unlike loops which should not be nested unless necessary, @@ -716,6 +740,8 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow branch.ReplaceWith(new Leave(switchContainer) { ILRange = branch.ILRange }); } } + + isSwitch = false; } } } diff --git a/ICSharpCode.Decompiler/IL/ControlFlow/SwitchDetection.cs b/ICSharpCode.Decompiler/IL/ControlFlow/SwitchDetection.cs index d826be31b..737387edd 100644 --- a/ICSharpCode.Decompiler/IL/ControlFlow/SwitchDetection.cs +++ b/ICSharpCode.Decompiler/IL/ControlFlow/SwitchDetection.cs @@ -36,11 +36,99 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow /// class SwitchDetection : IILTransform { + private readonly SwitchAnalysis analysis = new SwitchAnalysis(); + private ILTransformContext context; private BlockContainer currentContainer; private ControlFlowGraph controlFlowGraph; + private LoopContext loopContext; + + /// + /// When detecting a switch, it is important to distinguish Branch instructions which will + /// eventually decompile to continue; statements. + /// + /// A LoopContext is constructed for a node and its dominator tree, as for a Branch to be a continue; + /// statement, it must be contained within the target-loop + /// + /// This class also supplies the depth of the loop targetted by a continue; statement relative to the + /// context node, to avoid (or eventually support) labelled continues to outer loops + /// + public class LoopContext + { + private readonly IDictionary continueDepth = new Dictionary(); + + public LoopContext(ControlFlowGraph cfg, ControlFlowNode contextNode) + { + var loopHeads = new List(); + + void Analyze(ControlFlowNode n) + { + if (n.Visited) + return; + + n.Visited = true; + if (n.Dominates(contextNode)) + loopHeads.Add(n); + else + n.Successors.ForEach(Analyze); + } + contextNode.Successors.ForEach(Analyze); + + foreach (var node in cfg.cfg) + node.Visited = false; + + int l = 1; + foreach (var loopHead in loopHeads.OrderBy(n => n.PostOrderNumber)) + continueDepth[FindContinue(loopHead)] = l++; + } + + private static ControlFlowNode FindContinue(ControlFlowNode loopHead) + { + // potential continue target + var pred = loopHead.Predecessors.OnlyOrDefault(p => p != loopHead && loopHead.Dominates(p)); + if (pred == null) + return loopHead; + + // match for loop increment block + if (pred.Successors.Count == 1) { + if (HighLevelLoopTransform.MatchIncrementBlock((Block)pred.UserData, out var target) &&target == loopHead.UserData) + return pred; + } + + // match do-while condition + if (pred.Successors.Count <= 2) { + if (HighLevelLoopTransform.MatchDoWhileConditionBlock((Block)pred.UserData, out var t1, out var t2) && + (t1 == loopHead.UserData || t2 == loopHead.UserData)) + return pred; + } + + return loopHead; + } + + public bool MatchContinue(ControlFlowNode node) => MatchContinue(node, out var _); + + public bool MatchContinue(ControlFlowNode node, int depth) => + MatchContinue(node, out int _depth) && depth == _depth; + + public bool MatchContinue(ControlFlowNode node, out int depth) => continueDepth.TryGetValue(node, out depth); - SwitchAnalysis analysis = new SwitchAnalysis(); + public int GetContinueDepth(ControlFlowNode node) => MatchContinue(node, out var depth) ? depth : 0; + + /// + /// Lists all potential targets for break; statements from a domination tree, + /// assuming the domination tree must be exited via either break; or continue; + /// + /// First list all nodes in the dominator tree (excluding continue nodes) + /// Then return the all successors not contained within said tree. + /// + /// Note that node will be returned once for each outgoing edge. + /// Labelled continue statements (depth > 1) are counted as break targets + /// + internal IEnumerable GetBreakTargets(ControlFlowNode dominator) => + TreeTraversal.PreOrder(dominator, n => n.DominatorTreeChildren.Where(c => !MatchContinue(c))) + .SelectMany(n => n.Successors) + .Where(n => !dominator.Dominates(n) && !MatchContinue(n, 1)); + } public void Run(ILFunction function, ILTransformContext context) { @@ -248,8 +336,10 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow { if (controlFlowGraph == null) controlFlowGraph = new ControlFlowGraph(currentContainer, context.CancellationToken); - + var switchHead = controlFlowGraph.GetNode(analysis.RootBlock); + loopContext = new LoopContext(controlFlowGraph, switchHead); + // grab the control flow nodes for blocks targetted by each section var caseNodes = new List(); foreach (var s in analysis.Sections) { @@ -257,7 +347,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow continue; var node = controlFlowGraph.GetNode(block); - if (!IsContinue(switchHead, node)) + if (!loopContext.MatchContinue(node)) caseNodes.Add(node); } @@ -274,7 +364,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow return true; // cannot have more than one break case without gotos // check that case nodes flow through a single point - var breakTargets = caseNodes.Except(externalCases).SelectMany(n => GetBreakTargets(switchHead, n)).ToHashSet(); + var breakTargets = caseNodes.Except(externalCases).SelectMany(n => loopContext.GetBreakTargets(n)).ToHashSet(); // if there are multiple break targets, then gotos are required // if there are none, then the external case (if any) can be the break target @@ -318,62 +408,5 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow //add the null case logic to the incoming flow blocks flowBlocks.Add(nullableBlock); } - - internal static bool IsContinue(ControlFlowNode innerLoopHead, ControlFlowNode node) => - IsContinue(node, out var outerLoopHead) && outerLoopHead.Dominates(innerLoopHead); - - private static bool IsContinue(ControlFlowNode node, out ControlFlowNode loopHead) - { - bool IsLoopHead(ControlFlowNode n) => n.Predecessors.Any(n.Dominates); - ControlFlowNode OnlyInloopPred(ControlFlowNode n) => n.Predecessors.OnlyOrDefault(p => p != n && n.Dominates(p)); - - loopHead = null; - - // loop head - if (IsLoopHead(node)) { - var preBlock = OnlyInloopPred(node); - if (preBlock != null && IsContinue(preBlock, out var preHead) && preHead == node) - return false; - - loopHead = node; - return true; - } - - // match for loop increment block - if (node.Successors.Count == 1) { - // potential loop head - loopHead = node.Successors.SingleOrDefault(s => IsLoopHead(s) && OnlyInloopPred(s) == node); - if (loopHead != null && - HighLevelLoopTransform.MatchIncrementBlock((Block)node.UserData, out var target) && - target == loopHead.UserData) - return true; - } - - // match do-while condition - if (node.Successors.Count <= 2) { - // potential loop head - loopHead = node.Successors.OnlyOrDefault(s => IsLoopHead(s) && OnlyInloopPred(s) == node); - if (loopHead != null && - HighLevelLoopTransform.MatchDoWhileConditionBlock((Block)node.UserData, out var t1, out var t2) && - (t1 == loopHead.UserData || t2 == loopHead.UserData)) - return true; - } - - return false; - } - - /// - /// Lists all potential targets for break; statements from a domination tree, - /// assuming the domination tree must be exited via either break; or continue; - /// - /// First list all nodes in the dominator tree (excluding continue nodes) - /// Then return the all successors not contained within said tree. - /// - /// Note that node will be returned once for every outgoing edge - /// - internal static IEnumerable GetBreakTargets(ControlFlowNode loopHead, ControlFlowNode dominator) => - TreeTraversal.PreOrder(dominator, n => n.DominatorTreeChildren.Where(c => !IsContinue(loopHead, c))) - .SelectMany(n => n.Successors) - .Where(n => !dominator.Dominates(n) && !IsContinue(loopHead, n)); } }