diff --git a/ICSharpCode.Decompiler.Tests/DataFlowTest.cs b/ICSharpCode.Decompiler.Tests/DataFlowTest.cs new file mode 100644 index 000000000..e42850616 --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/DataFlowTest.cs @@ -0,0 +1,67 @@ +// Copyright (c) 2016 Daniel Grunwald +// +// 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; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading; +using System.Threading.Tasks; +using ICSharpCode.Decompiler.FlowAnalysis; +using ICSharpCode.Decompiler.IL; +using ICSharpCode.Decompiler.TypeSystem; +using NUnit.Framework; + +namespace ICSharpCode.Decompiler.Tests +{ + [TestFixture] + class DataFlowTest + { + class RDTest : ReachingDefinitionsVisitor + { + ILVariable v; + + public RDTest(ILFunction f, ILVariable v) : base(f, _ => true, CancellationToken.None) + { + this.v = v; + } + + protected internal override void VisitTryFinally(TryFinally inst) + { + Assert.IsTrue(IsPotentiallyUninitialized(state, v)); + base.VisitTryFinally(inst); + Assert.IsTrue(state.IsReachable); + Assert.AreEqual(1, GetStores(state, v).Count()); + Assert.IsFalse(IsPotentiallyUninitialized(state, v)); + } + } + + [Test] + public void TryFinallyWithAssignmentInFinally() + { + ILVariable v = new ILVariable(VariableKind.Local, SpecialType.UnknownType, 0); + ILFunction f = new ILFunction(null, new TryFinally( + new Nop(), + new StLoc(v, new LdcI4(0)) + )); + f.AddRef(); + f.Variables.Add(v); + f.Body.AcceptVisitor(new RDTest(f, v)); + } + } +} diff --git a/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj b/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj index e1624ad4f..742d197d8 100644 --- a/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj +++ b/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj @@ -49,6 +49,7 @@ + diff --git a/ICSharpCode.Decompiler/FlowAnalysis/DataFlowVisitor.cs b/ICSharpCode.Decompiler/FlowAnalysis/DataFlowVisitor.cs index defbde9d3..2ac18d313 100644 --- a/ICSharpCode.Decompiler/FlowAnalysis/DataFlowVisitor.cs +++ b/ICSharpCode.Decompiler/FlowAnalysis/DataFlowVisitor.cs @@ -110,23 +110,18 @@ namespace ICSharpCode.Decompiler.FlowAnalysis /// this.isReachable |= incomingState.isReachable; /// void JoinWith(Self incomingState); - + /// - /// The meet operation. + /// A special operation to merge the end-state of the finally-block with the end state of + /// a branch leaving the try-block. /// - /// If possible, this method sets this to the greatest state that is smaller than (or equal to) - /// both input states. - /// At a minimum, meeting with an unreachable state must result in an unreachable state. + /// If either input state is unreachable, this call must result in an unreachable state. /// - /// - /// MeetWith() is used when control flow passes out of a try-finally construct: the endpoint of the try-finally - /// is reachable only if both the endpoint of the try and the endpoint of the finally blocks are reachable. - /// /// - /// The simple state "bool isReachable", would implement MeetWith as: - /// this.isReachable &= incomingState.isReachable; + /// The simple state "bool isReachable", would implement TriggerFinally as: + /// this.isReachable &= finallyState.isReachable; /// - void MeetWith(Self incomingState); + void TriggerFinally(Self finallyState); /// /// Gets whether this is the bottom state. @@ -400,35 +395,56 @@ namespace ICSharpCode.Decompiler.FlowAnalysis DebugEndPoint(container); workLists.Remove(container); } - + + readonly List<(IBranchOrLeaveInstruction, State)> branchesTriggeringFinally = new List<(IBranchOrLeaveInstruction, State)>(); + protected internal override void VisitBranch(Branch inst) + { + if (inst.TriggersFinallyBlock) { + Debug.Assert(state.LessThanOrEqual(currentStateOnException)); + branchesTriggeringFinally.Add((inst, state.Clone())); + } else { + MergeBranchStateIntoTargetBlock(inst, state); + } + MarkUnreachable(); + } + + void MergeBranchStateIntoTargetBlock(Branch inst, State branchState) { var targetBlock = inst.TargetBlock; var targetState = GetBlockInputState(targetBlock); - if (!state.LessThanOrEqual(targetState)) { - targetState.JoinWith(state); - + if (!branchState.LessThanOrEqual(targetState)) { + targetState.JoinWith(branchState); + BlockContainer container = (BlockContainer)targetBlock.Parent; workLists[container].Add(targetBlock.ChildIndex); } - MarkUnreachable(); } protected internal override void VisitLeave(Leave inst) { inst.Value.AcceptVisitor(this); - State targetState; - if (stateOnLeave.TryGetValue(inst.TargetContainer, out targetState)) { - targetState.JoinWith(state); + if (inst.TriggersFinallyBlock) { + Debug.Assert(state.LessThanOrEqual(currentStateOnException)); + branchesTriggeringFinally.Add((inst, state.Clone())); + } else { + MergeBranchStateIntoStateOnLeave(inst, state); + } + MarkUnreachable(); + } + + void MergeBranchStateIntoStateOnLeave(Leave inst, State branchState) + { + if (stateOnLeave.TryGetValue(inst.TargetContainer, out State targetState)) { + targetState.JoinWith(branchState); } else { - stateOnLeave.Add(inst.TargetContainer, state.Clone()); + stateOnLeave.Add(inst.TargetContainer, branchState.Clone()); } // Note: We don't have to put the block container onto the work queue, // because it's an ancestor of the Leave instruction, and hence // we are currently somewhere within the VisitBlockContainer() call. - MarkUnreachable(); } - + protected internal override void VisitThrow(Throw inst) { inst.Argument.AcceptVisitor(this); @@ -512,22 +528,57 @@ namespace ICSharpCode.Decompiler.FlowAnalysis { throw new NotSupportedException(); } - + protected internal override void VisitTryFinally(TryFinally inst) { DebugStartPoint(inst); + int branchesTriggeringFinallyOldCount = branchesTriggeringFinally.Count; // At first, handle 'try { .. } finally { .. }' like 'try { .. } catch {} .. if (?) rethrow; }' State onException = HandleTryBlock(inst); State onSuccess = state.Clone(); state.JoinWith(onException); inst.FinallyBlock.AcceptVisitor(this); - PropagateStateOnException(); - // Use MeetWith() to ensure points after the try-finally are reachable only if both the + //PropagateStateOnException(); // rethrow the exception after the finally block -- should be redundant + Debug.Assert(state.LessThanOrEqual(currentStateOnException)); + + ProcessBranchesLeavingTryFinally(inst, branchesTriggeringFinallyOldCount); + + // Use TriggerFinally() to ensure points after the try-finally are reachable only if both the // try and the finally endpoints are reachable. - state.MeetWith(onSuccess); + onSuccess.TriggerFinally(state); + state = onSuccess; DebugEndPoint(inst); } + /// + /// Process branches leaving the try-finally, + /// * Calls TriggerFinally() on each branchesTriggeringFinally + /// * Removes entries from branchesTriggeringFinally if they won't trigger additional finally blocks. + /// * After all finallies are applied, the branch state is merged into the target block. + /// + void ProcessBranchesLeavingTryFinally(TryFinally tryFinally, int branchesTriggeringFinallyOldCount) + { + int outPos = branchesTriggeringFinallyOldCount; + for (int i = branchesTriggeringFinallyOldCount; i < branchesTriggeringFinally.Count; ++i) { + var (branch, stateOnBranch) = branchesTriggeringFinally[i]; + Debug.Assert(((ILInstruction)branch).IsDescendantOf(tryFinally)); + Debug.Assert(tryFinally.IsDescendantOf(branch.TargetContainer)); + stateOnBranch.TriggerFinally(state); + bool triggersAnotherFinally = Branch.GetExecutesFinallyBlock(tryFinally, branch.TargetContainer); + if (triggersAnotherFinally) { + branchesTriggeringFinally[outPos++] = (branch, stateOnBranch); + } else { + // Merge state into target block. + if (branch is Leave leave) { + MergeBranchStateIntoStateOnLeave((Leave)branch, stateOnBranch); + } else { + MergeBranchStateIntoTargetBlock((Branch)branch, stateOnBranch); + } + } + } + branchesTriggeringFinally.RemoveRange(outPos, branchesTriggeringFinally.Count - outPos); + } + protected internal override void VisitTryFault(TryFault inst) { DebugStartPoint(inst); @@ -537,8 +588,9 @@ namespace ICSharpCode.Decompiler.FlowAnalysis State onSuccess = state; state = onException; inst.FaultBlock.AcceptVisitor(this); - PropagateStateOnException(); // rethrow the exception after the fault block - + //PropagateStateOnException(); // rethrow the exception after the fault block + Debug.Assert(state.LessThanOrEqual(currentStateOnException)); + // try-fault exits normally only if no exception occurred state = onSuccess; DebugEndPoint(inst); @@ -579,5 +631,10 @@ namespace ICSharpCode.Decompiler.FlowAnalysis inst.Value.AcceptVisitor(this); DebugEndPoint(inst); } + + protected internal override void VisitILFunction(ILFunction function) + { + throw new NotImplementedException(); + } } } diff --git a/ICSharpCode.Decompiler/FlowAnalysis/DefiniteAssignmentVisitor.cs b/ICSharpCode.Decompiler/FlowAnalysis/DefiniteAssignmentVisitor.cs index 96ac2184e..5424a9011 100644 --- a/ICSharpCode.Decompiler/FlowAnalysis/DefiniteAssignmentVisitor.cs +++ b/ICSharpCode.Decompiler/FlowAnalysis/DefiniteAssignmentVisitor.cs @@ -37,8 +37,9 @@ namespace ICSharpCode.Decompiler.FlowAnalysis /// /// bits[i]: There is a code path from the entry point to this state's position /// that does not write to function.Variables[i]. + /// (i.e. the variable is not definitely assigned at the state's position) /// - /// Initial state: all bits set + /// Initial state: all bits set = nothing is definitely assigned /// Bottom state: all bits clear /// readonly BitSet bits; @@ -77,9 +78,20 @@ namespace ICSharpCode.Decompiler.FlowAnalysis bits.UnionWith(incomingState.bits); } - public void MeetWith(State incomingState) + public void TriggerFinally(State finallyState) { - bits.IntersectWith(incomingState.bits); + // If there is no path to the end of the try-block that leaves a variable v + // uninitialized, then there is no such path to the end of the whole try-finally either. + // (the try-finally cannot complete successfully unless the try block does the same) + // ==> any bits that are false in this.state must be false in the output state. + + // Or said otherwise: a variable is definitely assigned after try-finally if it is + // definitely assigned in either the try or the finally block. + // Given that the bits are the opposite of definite assignment, this gives us: + // !outputBits[i] == !bits[i] || !finallyState.bits[i]. + // and thus: + // outputBits[i] == bits[i] && finallyState.bits[i]. + bits.IntersectWith(finallyState.bits); } public void ReplaceWithBottom() diff --git a/ICSharpCode.Decompiler/FlowAnalysis/ReachingDefinitionsVisitor.cs b/ICSharpCode.Decompiler/FlowAnalysis/ReachingDefinitionsVisitor.cs index eb11dfab2..8b6d22a24 100644 --- a/ICSharpCode.Decompiler/FlowAnalysis/ReachingDefinitionsVisitor.cs +++ b/ICSharpCode.Decompiler/FlowAnalysis/ReachingDefinitionsVisitor.cs @@ -91,8 +91,8 @@ namespace ICSharpCode.Decompiler.FlowAnalysis /// /// Reaching store bit (bit si, where allStores[si] != null): /// There is a code path from the entry point to this state's position - /// that passes through through allStores[i] and does not pass through another - /// store to allStores[i].Variable. + /// that passes through through allStores[si] and does not pass through another + /// store to allStores[si].Variable. /// /// The indices for a variable's reaching store bits are between firstStoreIndexForVariable[v.IndexInScope] /// to firstStoreIndexForVariable[v.IndexInScope + 1] (both endpoints exclusive!). @@ -128,17 +128,29 @@ namespace ICSharpCode.Decompiler.FlowAnalysis public void JoinWith(State incomingState) { // When control flow is joined together, we can simply union our bitsets. - // (joined node is reachable iff either input is reachable) + // (a store is reachable iff it is reachable through either incoming path) bits.UnionWith(incomingState.bits); } - public void MeetWith(State incomingState) + public void TriggerFinally(State finallyState) { - // At the end of a try-finally construct, we intersect the try-bitset - // with the finally-bitset - // (the try-finally-endpoint is reachable if both the try-block-endpoint and - // the finally-block-endpoint are reachable) - bits.IntersectWith(incomingState.bits); + // Some cases to consider: + // try { v = 1; } finally { v = 2; } + // => only the store 2 is visible after the try-finally + // v = 1; try { v = 2; } finally { } + // => both stores are visible after the try-finally + // In general, we're looking for the post-state of the finally-block + // assume the finally-block was entered without throwing an exception. + // But we don't have that information (it would require analyzing the finally block twice), + // so the next best thing is to approximate it by just keeping the state after the finally + // (i.e. doing nothing at all). + // However, the DataFlowVisitor requires us to return bottom if the end-state of the + // try-block was unreachable, so let's so at least that. + // (note that in principle we could just AND the reachable and uninitialized bits, + // but we don't have a good solution for the normal store bits) + if (IsReachable) { + ReplaceWith(finallyState); + } } public bool IsBottom { @@ -148,7 +160,7 @@ namespace ICSharpCode.Decompiler.FlowAnalysis public void ReplaceWithBottom() { // We need to clear all bits, not just ReachableBit, so that - // the bottom state behaves as expected in joins/meets. + // the bottom state behaves as expected in joins. bits.ClearAll(); } diff --git a/ICSharpCode.Decompiler/IL/ControlFlow/AsyncAwaitDecompiler.cs b/ICSharpCode.Decompiler/IL/ControlFlow/AsyncAwaitDecompiler.cs index 3220f7c09..bc06c9c47 100644 --- a/ICSharpCode.Decompiler/IL/ControlFlow/AsyncAwaitDecompiler.cs +++ b/ICSharpCode.Decompiler/IL/ControlFlow/AsyncAwaitDecompiler.cs @@ -76,6 +76,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow // These fields are set by AnalyzeStateMachine(): int smallestAwaiterVarIndex; + HashSet moveNextLeaves = new HashSet(); // For each block containing an 'await', stores the awaiter variable, and the field storing the awaiter // across the yield point. @@ -89,6 +90,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow fieldToParameterMap.Clear(); cachedFieldToParameterMap.Clear(); awaitBlocks.Clear(); + moveNextLeaves.Clear(); if (!MatchTaskCreationPattern(function)) return; try { @@ -99,7 +101,9 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow } InlineBodyOfMoveNext(function); + function.CheckInvariant(ILPhase.InAsyncAwait); CleanUpBodyOfMoveNext(function); + function.CheckInvariant(ILPhase.InAsyncAwait); AnalyzeStateMachine(function); DetectAwaitPattern(function); @@ -469,6 +473,12 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow }); } } + foreach (var leave in function.Descendants.OfType()) { + if (leave.TargetContainer == moveNextFunction.Body) { + leave.TargetContainer = (BlockContainer)function.Body; + moveNextLeaves.Add(leave); + } + } function.Variables.AddRange(function.Descendants.OfType().Select(inst => inst.Variable).Distinct()); function.Variables.RemoveDead(); function.Variables.AddRange(fieldToParameterMap.Values); @@ -478,7 +488,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow { context.Step("FinalizeInlineMoveNext()", function); foreach (var leave in function.Descendants.OfType()) { - if (leave.TargetContainer == moveNextFunction.Body) { + if (moveNextLeaves.Contains(leave)) { leave.ReplaceWith(new InvalidBranch { Message = "leave MoveNext - await not detected correctly", ILRange = leave.ILRange @@ -508,7 +518,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow foreach (var block in container.Blocks) { context.CancellationToken.ThrowIfCancellationRequested(); - if (block.Instructions.Last().MatchLeave((BlockContainer)moveNextFunction.Body)) { + if (block.Instructions.Last() is Leave leave && moveNextLeaves.Contains(leave)) { // This is likely an 'await' block if (AnalyzeAwaitBlock(block, out var awaiterVar, out var awaiterField, out var state)) { block.Instructions.Add(new Await(new LdLoca(awaiterVar))); diff --git a/ICSharpCode.Decompiler/IL/Instructions/Branch.cs b/ICSharpCode.Decompiler/IL/Instructions/Branch.cs index fcd90d7d8..8cfab388b 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/Branch.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/Branch.cs @@ -27,7 +27,7 @@ namespace ICSharpCode.Decompiler.IL /// /// When jumping to the entrypoint of the current block container, the branch represents a continue statement. /// - partial class Branch : SimpleInstruction + partial class Branch : SimpleInstruction, IBranchOrLeaveInstruction { readonly int targetILOffset; Block targetBlock; @@ -95,7 +95,25 @@ namespace ICSharpCode.Decompiler.IL public string TargetLabel { get { return targetBlock != null ? targetBlock.Label : CecilExtensions.OffsetToString(TargetILOffset); } } - + + /// + /// Gets whether this branch executes at least one finally block before jumping to the target block. + /// + public bool TriggersFinallyBlock { + get { + return GetExecutesFinallyBlock(this, TargetContainer); + } + } + + internal static bool GetExecutesFinallyBlock(ILInstruction inst, BlockContainer container) + { + for (; inst != container; inst = inst.Parent) { + if (inst.Parent is TryFinally && inst.SlotInfo == TryFinally.TryBlockSlot) + return true; + } + return false; + } + internal override void CheckInvariant(ILPhase phase) { base.CheckInvariant(phase); @@ -113,4 +131,9 @@ namespace ICSharpCode.Decompiler.IL output.WriteReference(TargetLabel, (object)targetBlock ?? TargetILOffset, isLocal: true); } } + + interface IBranchOrLeaveInstruction + { + BlockContainer TargetContainer { get; } + } } diff --git a/ICSharpCode.Decompiler/IL/Instructions/ILInstruction.cs b/ICSharpCode.Decompiler/IL/Instructions/ILInstruction.cs index a4c8f66ca..b4042364e 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/ILInstruction.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/ILInstruction.cs @@ -37,6 +37,11 @@ namespace ICSharpCode.Decompiler.IL /// The usual invariants are established. /// Normal, + /// + /// Special phase within the async-await decompiler, where a few selected invariants + /// are temporarily suspended. (see Leave.CheckInvariant) + /// + InAsyncAwait } /// diff --git a/ICSharpCode.Decompiler/IL/Instructions/Leave.cs b/ICSharpCode.Decompiler/IL/Instructions/Leave.cs index d6ab063bd..7bdabcc26 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/Leave.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/Leave.cs @@ -30,7 +30,7 @@ namespace ICSharpCode.Decompiler.IL /// Phase-2 execution removes PopCount elements from the evaluation stack /// and jumps to the target block. /// - partial class Leave : ILInstruction + partial class Leave : ILInstruction, IBranchOrLeaveInstruction { BlockContainer targetContainer; @@ -93,11 +93,20 @@ namespace ICSharpCode.Decompiler.IL get { return targetContainer?.Parent is ILFunction; } } + /// + /// Gets whether this branch executes at least one finally block before jumping to the end of the target block container. + /// + public bool TriggersFinallyBlock { + get { + return Branch.GetExecutesFinallyBlock(this, TargetContainer); + } + } + internal override void CheckInvariant(ILPhase phase) { base.CheckInvariant(phase); Debug.Assert(phase <= ILPhase.InILReader || this.IsDescendantOf(targetContainer)); - Debug.Assert(phase <= ILPhase.InILReader || value.ResultType == targetContainer.ResultType); + Debug.Assert(phase <= ILPhase.InILReader || phase == ILPhase.InAsyncAwait || value.ResultType == targetContainer.ResultType); } public override void WriteTo(ITextOutput output, ILAstWritingOptions options)