From dd485b971d10e081518016bba9ada8bb3529b1cf Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Thu, 24 Nov 2016 11:13:49 +0100 Subject: [PATCH] Eliminate goto in conditional return in try block. --- .../IL/ControlFlow/ConditionDetection.cs | 16 +++++++- .../ControlFlow/ControlFlowSimplification.cs | 1 + .../IL/Instructions/BlockContainer.cs | 3 ++ .../Tests/ICSharpCode.Decompiler.Tests.csproj | 1 + .../Tests/PrettyTestRunner.cs | 6 +++ .../TestCases/Pretty/ExceptionHandling.cs | 40 +++++++++++++++++++ NRefactory | 2 +- 7 files changed, 66 insertions(+), 3 deletions(-) create mode 100644 ICSharpCode.Decompiler/Tests/TestCases/Pretty/ExceptionHandling.cs diff --git a/ICSharpCode.Decompiler/IL/ControlFlow/ConditionDetection.cs b/ICSharpCode.Decompiler/IL/ControlFlow/ConditionDetection.cs index 36f63dbec..f51314780 100644 --- a/ICSharpCode.Decompiler/IL/ControlFlow/ConditionDetection.cs +++ b/ICSharpCode.Decompiler/IL/ControlFlow/ConditionDetection.cs @@ -190,7 +190,8 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow } 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 + // swap true and false branches of if/else construct, + // to bring them in the same order as the IL code var oldTrue = ifInst.TrueInst; ifInst.TrueInst = ifInst.FalseInst; ifInst.FalseInst = oldTrue; @@ -210,10 +211,21 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow bool IsBranchToLaterTarget(ILInstruction inst1, ILInstruction inst2) { - Block block1, block2; + Block block1 = null, block2 = null; if (inst1.MatchBranch(out block1) && inst2.MatchBranch(out block2)) { return block1.ILRange.Start > block2.ILRange.Start; } + BlockContainer container1, container2; + if (inst1.MatchLeave(out container1) && container1.Parent is TryInstruction) { + // 'leave tryBlock' is considered to have a later target than + // any branch within the container, and also a later target + // than a return instruction. + // This is necessary to avoid "goto" statements in the + // ExceptionHandling.ConditionalReturnInThrow test. + if (!inst2.MatchLeave(out container2)) + container2 = block2?.Parent as BlockContainer; + return container2 == null || container2.IsDescendantOf(container1); + } return false; } diff --git a/ICSharpCode.Decompiler/IL/ControlFlow/ControlFlowSimplification.cs b/ICSharpCode.Decompiler/IL/ControlFlow/ControlFlowSimplification.cs index 327c241a6..3f92dd864 100644 --- a/ICSharpCode.Decompiler/IL/ControlFlow/ControlFlowSimplification.cs +++ b/ICSharpCode.Decompiler/IL/ControlFlow/ControlFlowSimplification.cs @@ -90,6 +90,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow } var nextBranch = (Branch)targetBlock.Instructions[0]; branch.TargetBlock = nextBranch.TargetBlock; + branch.AddILRange(nextBranch.ILRange); if (targetBlock.IncomingEdgeCount == 0) targetBlock.Instructions.Clear(); // mark the block for deletion targetBlock = branch.TargetBlock; diff --git a/ICSharpCode.Decompiler/IL/Instructions/BlockContainer.cs b/ICSharpCode.Decompiler/IL/Instructions/BlockContainer.cs index cde5d2cb0..552b6ea06 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/BlockContainer.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/BlockContainer.cs @@ -181,6 +181,9 @@ namespace ICSharpCode.Decompiler.IL /// public void SortBlocks(bool deleteUnreachableBlocks = false) { + if (Blocks.Count < 2) + return; + // Visit blocks in post-order BitSet visited = new BitSet(Blocks.Count); List postOrder = new List(); diff --git a/ICSharpCode.Decompiler/Tests/ICSharpCode.Decompiler.Tests.csproj b/ICSharpCode.Decompiler/Tests/ICSharpCode.Decompiler.Tests.csproj index e9704298e..a2d02bb44 100644 --- a/ICSharpCode.Decompiler/Tests/ICSharpCode.Decompiler.Tests.csproj +++ b/ICSharpCode.Decompiler/Tests/ICSharpCode.Decompiler.Tests.csproj @@ -151,6 +151,7 @@ + diff --git a/ICSharpCode.Decompiler/Tests/PrettyTestRunner.cs b/ICSharpCode.Decompiler/Tests/PrettyTestRunner.cs index c71b6e019..c3aceb3d0 100644 --- a/ICSharpCode.Decompiler/Tests/PrettyTestRunner.cs +++ b/ICSharpCode.Decompiler/Tests/PrettyTestRunner.cs @@ -88,6 +88,12 @@ namespace ICSharpCode.Decompiler.Tests Run(cscOptions: cscOptions); } + [Test] + public void ExceptionHandling([Values(CompilerOptions.None, CompilerOptions.Optimize)] CompilerOptions cscOptions) + { + Run(cscOptions: cscOptions); + } + void Run([CallerMemberName] string testName = null, AssemblerOptions asmOptions = AssemblerOptions.None, CompilerOptions cscOptions = CompilerOptions.None) { var ilFile = Path.Combine(TestCasePath, testName); diff --git a/ICSharpCode.Decompiler/Tests/TestCases/Pretty/ExceptionHandling.cs b/ICSharpCode.Decompiler/Tests/TestCases/Pretty/ExceptionHandling.cs new file mode 100644 index 000000000..f6a264e8f --- /dev/null +++ b/ICSharpCode.Decompiler/Tests/TestCases/Pretty/ExceptionHandling.cs @@ -0,0 +1,40 @@ +// Copyright (c) AlphaSierraPapa for the SharpDevelop Team +// +// 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.Tests.TestCases.Pretty +{ + public abstract class ExceptionHandling + { + public abstract bool B(int i); + public abstract void M(int i); + + public bool ConditionalReturnInThrow() + { + try { + if (this.B(0)) { + return this.B(1); + } + } catch { + } + return false; + } + + } +} diff --git a/NRefactory b/NRefactory index 85cf91fe5..2b10193ea 160000 --- a/NRefactory +++ b/NRefactory @@ -1 +1 @@ -Subproject commit 85cf91fe5138252a44b8e1d14a10366f90a5a10d +Subproject commit 2b10193ea20a26ae9c8db21a79c60f3be8d8cca8