From a6fc52a63f0c3fec66a4665480de214468156442 Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Mon, 21 Nov 2016 15:26:41 +0100 Subject: [PATCH] ConditionDetection: move blocks into switch sections --- .../CSharp/CSharpDecompiler.cs | 4 +- .../CSharp/StatementBuilder.cs | 24 ++- .../CSharp/Transforms/DeclareVariables.cs | 4 +- .../FlowAnalysis/DataFlowVisitor.cs | 3 +- .../IL/ControlFlow/ConditionDetection.cs | 160 +++++++++++------- .../IL/ControlFlow/SwitchAnalysis.cs | 1 + ICSharpCode.Decompiler/IL/Instructions.cs | 2 +- ICSharpCode.Decompiler/IL/Instructions.tt | 2 +- .../IL/Instructions/SwitchInstruction.cs | 37 +++- 9 files changed, 163 insertions(+), 74 deletions(-) diff --git a/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs b/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs index 748cd1666..cb39079ca 100644 --- a/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs +++ b/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs @@ -52,8 +52,10 @@ namespace ICSharpCode.Decompiler.CSharp public static List GetILTransforms() { return new List { - new SplitVariables(), new ControlFlowSimplification(), + // Run SplitVariables only after ControlFlowSimplification duplicates return blocks, + // so that the return variable is split and can be inlined. + new SplitVariables(), new ILInlining(), new DetectPinnedRegions(), // must run after inlining but before non-critical control flow transforms new SwitchDetection(), diff --git a/ICSharpCode.Decompiler/CSharp/StatementBuilder.cs b/ICSharpCode.Decompiler/CSharp/StatementBuilder.cs index 91e02b694..03159902f 100644 --- a/ICSharpCode.Decompiler/CSharp/StatementBuilder.cs +++ b/ICSharpCode.Decompiler/CSharp/StatementBuilder.cs @@ -93,13 +93,35 @@ namespace ICSharpCode.Decompiler.CSharp foreach (var section in inst.Sections) { var astSection = new ICSharpCode.NRefactory.CSharp.SwitchSection(); astSection.CaseLabels.AddRange(section.Labels.Values.Select(i => CreateTypedCaseLabel(i, value.Type))); - astSection.Statements.Add(Convert(section.Body)); + ConvertSwitchSectionBody(astSection, section.Body); stmt.SwitchSections.Add(astSection); } + if (inst.DefaultBody.OpCode != OpCode.Nop) { + var astSection = new ICSharpCode.NRefactory.CSharp.SwitchSection(); + astSection.CaseLabels.Add(new CaseLabel()); + ConvertSwitchSectionBody(astSection, inst.DefaultBody); + stmt.SwitchSections.Add(astSection); + } + breakTarget = oldBreakTarget; return stmt; } + + private void ConvertSwitchSectionBody(NRefactory.CSharp.SwitchSection astSection, ILInstruction bodyInst) + { + var body = Convert(bodyInst); + astSection.Statements.Add(body); + if (!bodyInst.HasFlag(InstructionFlags.EndPointUnreachable)) { + // we need to insert 'break;' + BlockStatement block = body as BlockStatement; + if (block != null) { + block.Add(new BreakStatement()); + } else { + astSection.Statements.Add(new BreakStatement()); + } + } + } /// Target block that a 'continue;' statement would jump to Block continueTarget; diff --git a/ICSharpCode.Decompiler/CSharp/Transforms/DeclareVariables.cs b/ICSharpCode.Decompiler/CSharp/Transforms/DeclareVariables.cs index 2901da3c8..3eb8cb09d 100644 --- a/ICSharpCode.Decompiler/CSharp/Transforms/DeclareVariables.cs +++ b/ICSharpCode.Decompiler/CSharp/Transforms/DeclareVariables.cs @@ -65,10 +65,11 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms { public readonly IType Type; public readonly string Name; + /// /// Whether the variable needs to be default-initialized. /// - public readonly bool DefaultInitialization; + public bool DefaultInitialization; /// /// Integer value that can be used to compare to VariableToDeclare instances @@ -237,6 +238,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms } else { v.InsertionPoint = point2; } + v.DefaultInitialization |= prev.DefaultInitialization; // I think we don't need to re-check the dict entries that we already checked earlier, // because the new v.InsertionPoint only collides with another point x if either // the old v.InsertionPoint or the old prev.InsertionPoint already collided with x. diff --git a/ICSharpCode.Decompiler/FlowAnalysis/DataFlowVisitor.cs b/ICSharpCode.Decompiler/FlowAnalysis/DataFlowVisitor.cs index 739378a73..30858be29 100644 --- a/ICSharpCode.Decompiler/FlowAnalysis/DataFlowVisitor.cs +++ b/ICSharpCode.Decompiler/FlowAnalysis/DataFlowVisitor.cs @@ -569,8 +569,7 @@ namespace ICSharpCode.Decompiler.FlowAnalysis DebugStartPoint(inst); inst.Value.AcceptVisitor(this); State beforeSections = state.Clone(); - // It's possible for none of the cases to match, - // so afterSections is reachable without executing any section + inst.DefaultBody.AcceptVisitor(this); State afterSections = state.Clone(); foreach (var section in inst.Sections) { state.ReplaceWith(beforeSections); diff --git a/ICSharpCode.Decompiler/IL/ControlFlow/ConditionDetection.cs b/ICSharpCode.Decompiler/IL/ControlFlow/ConditionDetection.cs index a1f365c10..c1bc6ff05 100644 --- a/ICSharpCode.Decompiler/IL/ControlFlow/ConditionDetection.cs +++ b/ICSharpCode.Decompiler/IL/ControlFlow/ConditionDetection.cs @@ -40,10 +40,10 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow Run(container, context); } } - + BlockContainer currentContainer; ControlFlowNode[] controlFlowGraph; - + void Run(BlockContainer container, ILTransformContext context) { currentContainer = container; @@ -73,65 +73,16 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow // (guaranteed by combination of BlockContainer and Block invariants) Debug.Assert(block.Instructions.Last().HasFlag(InstructionFlags.EndPointUnreachable)); ILInstruction exitInst = block.Instructions.Last(); - + // Previous-to-last instruction might have conditional control flow, // usually an IfInstruction with a branch: IfInstruction ifInst = block.Instructions.SecondToLastOrDefault() as IfInstruction; if (ifInst != null && ifInst.FalseInst.OpCode == OpCode.Nop) { - if (IsBranchToLaterTarget(ifInst.TrueInst, exitInst)) { - // "if (c) goto lateBlock; goto earlierBlock;" - // -> "if (!c)" goto earlierBlock; goto lateBlock; - // This reordering should make the if structure correspond more closely to the original C# source code - block.Instructions[block.Instructions.Count - 1] = ifInst.TrueInst; - ifInst.TrueInst = exitInst; - exitInst = block.Instructions.Last(); - ifInst.Condition = new LogicNot(ifInst.Condition); - } - - ILInstruction trueExitInst; - if (IsUsableBranchToChild(cfgNode, ifInst.TrueInst)) { - // "if (...) goto targetBlock; exitInst;" - // -> "if (...) { targetBlock } exitInst;" - var targetBlock = ((Branch)ifInst.TrueInst).TargetBlock; - // The targetBlock was already processed, we can embed it into the if statement: - ifInst.TrueInst = targetBlock; - trueExitInst = targetBlock.Instructions.LastOrDefault(); - if (CompatibleExitInstruction(exitInst, trueExitInst)) { - // "if (...) { ...; goto exitPoint } goto exitPoint;" - // -> "if (...) { ... } goto exitPoint;" - targetBlock.Instructions.RemoveAt(targetBlock.Instructions.Count - 1); - trueExitInst = null; - } - } else { - trueExitInst = ifInst.TrueInst; - } - if (IsUsableBranchToChild(cfgNode, exitInst)) { - var targetBlock = ((Branch)exitInst).TargetBlock; - var falseExitInst = targetBlock.Instructions.LastOrDefault(); - if (CompatibleExitInstruction(trueExitInst, falseExitInst)) { - // if (...) { ...; goto exitPoint; } goto nextBlock; nextBlock: ...; goto exitPoint; - // -> if (...) { ... } else { ... } goto exitPoint; - targetBlock.Instructions.RemoveAt(targetBlock.Instructions.Count - 1); - ifInst.FalseInst = targetBlock; - exitInst = block.Instructions[block.Instructions.Count - 1] = falseExitInst; - Block trueBlock = ifInst.TrueInst as Block; - if (trueBlock != null) { - Debug.Assert(trueExitInst == trueBlock.Instructions.Last()); - trueBlock.Instructions.RemoveAt(trueBlock.Instructions.Count - 1); - } else { - Debug.Assert(trueExitInst == ifInst.TrueInst); - ifInst.TrueInst = new Nop { ILRange = ifInst.TrueInst.ILRange }; - } - } - } - if (ifInst.FalseInst.OpCode != OpCode.Nop && ifInst.FalseInst.ILRange.Start < ifInst.TrueInst.ILRange.Start - || ifInst.TrueInst.OpCode == OpCode.Nop) - { - // swap true and false branches of if, to bring them in the same order as the IL code - var oldTrue = ifInst.TrueInst; - ifInst.TrueInst = ifInst.FalseInst; - ifInst.FalseInst = oldTrue; - ifInst.Condition = new LogicNot(ifInst.Condition); + HandleIfInstruction(cfgNode, block, ifInst, ref exitInst); + } else { + SwitchInstruction switchInst = block.Instructions.SecondToLastOrDefault() as SwitchInstruction; + if (switchInst != null) { + HandleSwitchInstruction(cfgNode, block, switchInst, ref exitInst); } } if (IsUsableBranchToChild(cfgNode, exitInst)) { @@ -145,6 +96,64 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow } } + private void HandleIfInstruction(ControlFlowNode cfgNode, Block block, IfInstruction ifInst, ref ILInstruction exitInst) + { + if (IsBranchToLaterTarget(ifInst.TrueInst, exitInst)) { + // "if (c) goto lateBlock; goto earlierBlock;" + // -> "if (!c)" goto earlierBlock; goto lateBlock; + // This reordering should make the if structure correspond more closely to the original C# source code + block.Instructions[block.Instructions.Count - 1] = ifInst.TrueInst; + ifInst.TrueInst = exitInst; + exitInst = block.Instructions.Last(); + ifInst.Condition = new LogicNot(ifInst.Condition); + } + + ILInstruction trueExitInst; + if (IsUsableBranchToChild(cfgNode, ifInst.TrueInst)) { + // "if (...) goto targetBlock; exitInst;" + // -> "if (...) { targetBlock } exitInst;" + var targetBlock = ((Branch)ifInst.TrueInst).TargetBlock; + // The targetBlock was already processed, we can embed it into the if statement: + ifInst.TrueInst = targetBlock; + trueExitInst = targetBlock.Instructions.LastOrDefault(); + if (CompatibleExitInstruction(exitInst, trueExitInst)) { + // "if (...) { ...; goto exitPoint } goto exitPoint;" + // -> "if (...) { ... } goto exitPoint;" + targetBlock.Instructions.RemoveAt(targetBlock.Instructions.Count - 1); + trueExitInst = null; + } + } else { + trueExitInst = ifInst.TrueInst; + } + if (IsUsableBranchToChild(cfgNode, exitInst)) { + var targetBlock = ((Branch)exitInst).TargetBlock; + var falseExitInst = targetBlock.Instructions.LastOrDefault(); + if (CompatibleExitInstruction(trueExitInst, falseExitInst)) { + // if (...) { ...; goto exitPoint; } goto nextBlock; nextBlock: ...; goto exitPoint; + // -> if (...) { ... } else { ... } goto exitPoint; + targetBlock.Instructions.RemoveAt(targetBlock.Instructions.Count - 1); + ifInst.FalseInst = targetBlock; + exitInst = block.Instructions[block.Instructions.Count - 1] = falseExitInst; + Block trueBlock = ifInst.TrueInst as Block; + if (trueBlock != null) { + Debug.Assert(trueExitInst == trueBlock.Instructions.Last()); + trueBlock.Instructions.RemoveAt(trueBlock.Instructions.Count - 1); + } else { + Debug.Assert(trueExitInst == ifInst.TrueInst); + ifInst.TrueInst = new Nop { ILRange = ifInst.TrueInst.ILRange }; + } + } + } + if (ifInst.FalseInst.OpCode != OpCode.Nop && ifInst.FalseInst.ILRange.Start < ifInst.TrueInst.ILRange.Start + || ifInst.TrueInst.OpCode == OpCode.Nop) { + // swap true and false branches of if, to bring them in the same order as the IL code + var oldTrue = ifInst.TrueInst; + ifInst.TrueInst = ifInst.FalseInst; + ifInst.FalseInst = oldTrue; + ifInst.Condition = new LogicNot(ifInst.Condition); + } + } + bool IsBranchToLaterTarget(ILInstruction inst1, ILInstruction inst2) { Block block1, block2; @@ -153,7 +162,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow } return false; } - + bool IsUsableBranchToChild(ControlFlowNode cfgNode, ILInstruction potentialBranchInstruction) { Branch br = potentialBranchInstruction as Branch; @@ -163,7 +172,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow return targetBlock.Parent == currentContainer && cfgNode.Dominates(controlFlowGraph[targetBlock.ChildIndex]) && targetBlock.IncomingEdgeCount == 1 && targetBlock.FinalInstruction.OpCode == OpCode.Nop; } - + internal static bool CompatibleExitInstruction(ILInstruction exit1, ILInstruction exit2) { if (exit1 == null || exit2 == null || exit1.OpCode != exit2.OpCode) @@ -185,5 +194,38 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow return false; } } + + private void HandleSwitchInstruction(ControlFlowNode cfgNode, Block block, SwitchInstruction sw, ref ILInstruction exitInst) + { + Debug.Assert(sw.DefaultBody is Nop); + foreach (var section in sw.Sections) { + if (IsUsableBranchToChild(cfgNode, section.Body)) { + // case ...: goto targetBlock; + var targetBlock = ((Branch)section.Body).TargetBlock; + section.Body = targetBlock; + } + } + if (IsUsableBranchToChild(cfgNode, exitInst)) { + // switch(...){} goto targetBlock; + // ---> switch(..) { default: { targetBlock } } + var targetBlock = ((Branch)exitInst).TargetBlock; + sw.DefaultBody = targetBlock; + if (targetBlock.Instructions.Last().OpCode == OpCode.Branch || targetBlock.Instructions.Last().OpCode == OpCode.Leave) { + exitInst = block.Instructions[block.Instructions.Count - 1] = targetBlock.Instructions.Last(); + targetBlock.Instructions.RemoveAt(targetBlock.Instructions.Count - 1); + } else { + exitInst = null; + block.Instructions.RemoveAt(block.Instructions.Count - 1); + } + } + // Remove compatible exitInsts from switch sections: + foreach (var section in sw.Sections) { + Block sectionBlock = section.Body as Block; + if (sectionBlock != null && CompatibleExitInstruction(exitInst, sectionBlock.Instructions.Last())) { + sectionBlock.Instructions.RemoveAt(sectionBlock.Instructions.Count - 1); + } + } + sw.Sections.ReplaceList(sw.Sections.OrderBy(s => s.Body.ILRange.Start)); + } } } diff --git a/ICSharpCode.Decompiler/IL/ControlFlow/SwitchAnalysis.cs b/ICSharpCode.Decompiler/IL/ControlFlow/SwitchAnalysis.cs index c2ccb9bbd..18b5e0e0c 100644 --- a/ICSharpCode.Decompiler/IL/ControlFlow/SwitchAnalysis.cs +++ b/ICSharpCode.Decompiler/IL/ControlFlow/SwitchAnalysis.cs @@ -131,6 +131,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow private bool AnalyzeSwitch(SwitchInstruction inst, LongSet inputValues, out LongSet anyMatchValues) { + Debug.Assert(inst.DefaultBody is Nop); anyMatchValues = LongSet.Empty; long offset; if (MatchSwitchVar(inst.Value)) { diff --git a/ICSharpCode.Decompiler/IL/Instructions.cs b/ICSharpCode.Decompiler/IL/Instructions.cs index 74414c2e5..c80ce9b5a 100644 --- a/ICSharpCode.Decompiler/IL/Instructions.cs +++ b/ICSharpCode.Decompiler/IL/Instructions.cs @@ -1103,7 +1103,7 @@ namespace ICSharpCode.Decompiler.IL protected internal override bool PerformMatch(ILInstruction other, ref Patterns.Match match) { var o = other as SwitchInstruction; - return o != null && Value.PerformMatch(o.Value, ref match) && Patterns.ListMatch.DoMatch(this.Sections, o.Sections, ref match); + return o != null && Value.PerformMatch(o.Value, ref match) && DefaultBody.PerformMatch(o.DefaultBody, ref match) && Patterns.ListMatch.DoMatch(this.Sections, o.Sections, ref match); } } diff --git a/ICSharpCode.Decompiler/IL/Instructions.tt b/ICSharpCode.Decompiler/IL/Instructions.tt index 768c60961..044986c1b 100644 --- a/ICSharpCode.Decompiler/IL/Instructions.tt +++ b/ICSharpCode.Decompiler/IL/Instructions.tt @@ -86,7 +86,7 @@ }), CustomConstructor, CustomComputeFlags, CustomWriteTo), new OpCode("switch", "Switch statement", CustomClassName("SwitchInstruction"), CustomConstructor, CustomComputeFlags, CustomWriteTo, ResultType("Void"), - MatchCondition("Value.PerformMatch(o.Value, ref match) && Patterns.ListMatch.DoMatch(this.Sections, o.Sections, ref match)")), + MatchCondition("Value.PerformMatch(o.Value, ref match) && DefaultBody.PerformMatch(o.DefaultBody, ref match) && Patterns.ListMatch.DoMatch(this.Sections, o.Sections, ref match)")), new OpCode("switch.section", "Switch section within a switch statement", CustomClassName("SwitchSection"), CustomChildren(new [] { new ChildInfo("body") }), CustomConstructor, CustomComputeFlags, CustomWriteTo, ResultType("Void"), diff --git a/ICSharpCode.Decompiler/IL/Instructions/SwitchInstruction.cs b/ICSharpCode.Decompiler/IL/Instructions/SwitchInstruction.cs index 92509b571..a8ebb79ee 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/SwitchInstruction.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/SwitchInstruction.cs @@ -33,13 +33,15 @@ namespace ICSharpCode.Decompiler.IL partial class SwitchInstruction { public static readonly SlotInfo ValueSlot = new SlotInfo("Value", canInlineInto: true); + public static readonly SlotInfo DefaultBodySlot = new SlotInfo("DefaultBody"); public static readonly SlotInfo SectionSlot = new SlotInfo("Section", isCollection: true); public SwitchInstruction(ILInstruction value) : base(OpCode.SwitchInstruction) { this.Value = value; - this.Sections = new InstructionCollection(this, 1); + this.DefaultBody = new Nop(); + this.Sections = new InstructionCollection(this, 2); } ILInstruction value; @@ -51,16 +53,25 @@ namespace ICSharpCode.Decompiler.IL } } + ILInstruction defaultBody; + + public ILInstruction DefaultBody { + get { return this.defaultBody; } + set { + ValidateChild(value); + SetChildInstruction(ref this.defaultBody, value, 1); + } + } + public readonly InstructionCollection Sections; - + protected override InstructionFlags ComputeFlags() { - var sectionFlags = InstructionFlags.ControlFlow; - // note: the initial sectionFlags also represent the implicit empty 'default' case + var sectionFlags = defaultBody.Flags; foreach (var section in Sections) { sectionFlags = SemanticHelper.CombineBranches(sectionFlags, section.Flags); } - return value.Flags | sectionFlags; + return value.Flags | InstructionFlags.ControlFlow | sectionFlags; } public override InstructionFlags DirectFlags { @@ -77,6 +88,9 @@ namespace ICSharpCode.Decompiler.IL output.MarkFoldStart("{...}"); output.WriteLine("{"); output.Indent(); + output.Write("default: "); + defaultBody.WriteTo(output); + output.WriteLine(); foreach (var section in this.Sections) { section.WriteTo(output); output.WriteLine(); @@ -88,28 +102,34 @@ namespace ICSharpCode.Decompiler.IL protected override int GetChildCount() { - return 1 + Sections.Count; + return 2 + Sections.Count; } protected override ILInstruction GetChild(int index) { if (index == 0) return value; - return Sections[index - 1]; + else if (index == 1) + return defaultBody; + return Sections[index - 2]; } protected override void SetChild(int index, ILInstruction value) { if (index == 0) Value = value; + else if (index == 1) + DefaultBody = value; else - Sections[index - 1] = (SwitchSection)value; + Sections[index - 2] = (SwitchSection)value; } protected override SlotInfo GetChildSlot(int index) { if (index == 0) return ValueSlot; + else if (index == 1) + return DefaultBodySlot; return SectionSlot; } @@ -118,6 +138,7 @@ namespace ICSharpCode.Decompiler.IL var clone = new SwitchInstruction(value.Clone()); clone.ILRange = this.ILRange; clone.Value = value.Clone(); + this.DefaultBody = defaultBody.Clone(); clone.Sections.AddRange(this.Sections.Select(h => (SwitchSection)h.Clone())); return clone; }