From e21ad7c3e456b922d976c9b8b4d1d99d403023eb Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Mon, 20 Jun 2016 20:30:08 +0200 Subject: [PATCH] DataFlowVisitor: ignore MayThrow and consider every instruction to potentially throw. This brings the IL definite assignment analysis more in line with the C# analysis. It's arguably more correct, because there are async exceptions (ThreadAbortException). It also means we call JoinWith() less often, so the analysis should be faster. --- .../FlowAnalysis/DataFlowVisitor.cs | 43 +++++++++++++------ .../FlowAnalysis/DefiniteAssignmentVisitor.cs | 5 +++ .../FlowAnalysis/ReachingDefinitions.cs | 11 ++++- 3 files changed, 44 insertions(+), 15 deletions(-) diff --git a/ICSharpCode.Decompiler/FlowAnalysis/DataFlowVisitor.cs b/ICSharpCode.Decompiler/FlowAnalysis/DataFlowVisitor.cs index 0052e540b..3776edafb 100644 --- a/ICSharpCode.Decompiler/FlowAnalysis/DataFlowVisitor.cs +++ b/ICSharpCode.Decompiler/FlowAnalysis/DataFlowVisitor.cs @@ -204,7 +204,8 @@ namespace ICSharpCode.Decompiler.FlowAnalysis /// /// Within a try block, currentStateOnException == stateOnException[tryBlock.Parent]. /// - State currentStateOnException; + /// + protected State currentStateOnException; /// /// Creates a new DataFlowVisitor. @@ -216,7 +217,7 @@ namespace ICSharpCode.Decompiler.FlowAnalysis this.bottomState = initialState.Clone(); this.bottomState.ReplaceWithBottom(); Debug.Assert(bottomState.IsBottom); - this.currentStateOnException = bottomState.Clone(); + this.currentStateOnException = state.Clone(); } #if DEBUG @@ -236,6 +237,10 @@ namespace ICSharpCode.Decompiler.FlowAnalysis debugDict.Add(inst, state.Clone()); } } + + // currentStateOnException should be all states within the try block joined together + // -> state should already have been joined into currentStateOnException. + Debug.Assert(state.LessThanOrEqual(currentStateOnException)); #endif } #endif @@ -271,10 +276,6 @@ namespace ICSharpCode.Decompiler.FlowAnalysis "Unreachable code must be in the bottom state."); } - // If this instruction can throw an exception, handle the exceptional control flow edge. - if ((inst.DirectFlags & InstructionFlags.MayThrow) != 0) { - MayThrow(); - } DebugEndPoint(inst); } @@ -282,7 +283,18 @@ namespace ICSharpCode.Decompiler.FlowAnalysis /// Handle control flow when the current instruction throws an exception: /// joins the current state into the "exception state" of the current try block. /// - protected void MayThrow() + /// + /// This should not only be called for instructions that may throw an exception, + /// but for all instructions (due to async exceptions like ThreadAbortException)! + /// + /// To avoid redundant calls, every Visit() call may assume that the current state + /// is already propagated, and has to guarantee the same at the end. + /// This means this method should be called after every state change. + /// Alternatively, derived classes may directly modify both state + /// and currentStateOnException, so that a full JoinWith() call + /// is not necessary. + /// + protected void PropagateStateOnException() { currentStateOnException.JoinWith(state); } @@ -409,13 +421,12 @@ namespace ICSharpCode.Decompiler.FlowAnalysis protected internal override void VisitThrow(Throw inst) { inst.Argument.AcceptVisitor(this); - MayThrow(); MarkUnreachable(); } protected internal override void VisitRethrow(Rethrow inst) { - MayThrow(); + PropagateStateOnException(); MarkUnreachable(); } @@ -434,14 +445,20 @@ namespace ICSharpCode.Decompiler.FlowAnalysis State oldStateOnException = currentStateOnException; State newStateOnException; if (!stateOnException.TryGetValue(inst, out newStateOnException)) { - newStateOnException = bottomState.Clone(); + newStateOnException = state.Clone(); stateOnException.Add(inst, newStateOnException); } currentStateOnException = newStateOnException; inst.TryBlock.AcceptVisitor(this); + // swap back to the old object instance currentStateOnException = oldStateOnException; + // No matter what kind of try-instruction this is, it's possible + // that an async exception is thrown immediately in the handler block, + // so propagate the state: + oldStateOnException.JoinWith(newStateOnException); + return newStateOnException; } @@ -450,8 +467,6 @@ namespace ICSharpCode.Decompiler.FlowAnalysis DebugStartPoint(inst); State onException = HandleTryBlock(inst); State endpoint = state.Clone(); - // The exception might get propagated if no handler matches the type: - currentStateOnException.JoinWith(onException); foreach (var handler in inst.Handlers) { state.ReplaceWith(onException); BeginTryCatchHandler(handler); @@ -489,7 +504,7 @@ namespace ICSharpCode.Decompiler.FlowAnalysis State onSuccess = state.Clone(); state.JoinWith(onException); inst.FinallyBlock.AcceptVisitor(this); - MayThrow(); + PropagateStateOnException(); // Use MeetWith() to ensure points after the try-finally are reachable only if both the // try and the finally endpoints are reachable. state.MeetWith(onSuccess); @@ -505,7 +520,7 @@ namespace ICSharpCode.Decompiler.FlowAnalysis State onSuccess = state; state = onException; inst.FaultBlock.AcceptVisitor(this); - MayThrow(); // rethrow the exception after the fault block + PropagateStateOnException(); // rethrow the exception after the fault block // try-fault exits normally only if no exception occurred state = onSuccess; diff --git a/ICSharpCode.Decompiler/FlowAnalysis/DefiniteAssignmentVisitor.cs b/ICSharpCode.Decompiler/FlowAnalysis/DefiniteAssignmentVisitor.cs index 26dcfdf93..b938808ff 100644 --- a/ICSharpCode.Decompiler/FlowAnalysis/DefiniteAssignmentVisitor.cs +++ b/ICSharpCode.Decompiler/FlowAnalysis/DefiniteAssignmentVisitor.cs @@ -123,6 +123,11 @@ namespace ICSharpCode.Decompiler.FlowAnalysis state.MarkVariableInitialized(v.IndexInScope); // Note that this gets called even if the store is in unreachable code, // but that's OK because bottomState.MarkVariableInitialized() has no effect. + + // After the state change, we have to call + // PropagateStateOnException() = currentStateOnException.JoinWith(state); + // but because MarkVariableInitialized() only clears a bit, + // this is guaranteed to be a no-op. } } diff --git a/ICSharpCode.Decompiler/FlowAnalysis/ReachingDefinitions.cs b/ICSharpCode.Decompiler/FlowAnalysis/ReachingDefinitions.cs index 39849bf7c..c4e8d6c6e 100644 --- a/ICSharpCode.Decompiler/FlowAnalysis/ReachingDefinitions.cs +++ b/ICSharpCode.Decompiler/FlowAnalysis/ReachingDefinitions.cs @@ -318,7 +318,16 @@ namespace ICSharpCode.Decompiler.FlowAnalysis // Clear the set of stores for this variable: state.KillStores(rd.firstStoreIndexForVariable[v.IndexInScope], rd.firstStoreIndexForVariable[v.IndexInScope + 1]); // And replace it with this store: - state.SetStore(rd.storeIndexMap[inst]); + int si = rd.storeIndexMap[inst]; + state.SetStore(si); + + // We should call PropagateStateOnException() here because we changed the state. + // But that's equal to: currentStateOnException.UnionWith(state); + + // Because we're already guaranteed that state.LessThanOrEqual(currentStateOnException) + // when entering HandleStore(), all we really need to do to achieve what PropagateStateOnException() does + // is to add the single additional store to the exceptional state as well: + currentStateOnException.SetStore(si); } }