Skip to content

Commit 64a1e45

Browse files
committed
Perform exceptions path analysis for all flow analyses
Fixes #5199 Fixes #5160 We already had implementation and option based support to perform exception path analysis to handle all operations that can potentially throw an exception, and hence need flow analysis data at the point of operation to be merged with analysis data at start of reachable catch blocks from that operation. However, we were only performing this analysis post pass when flow analysis is looking at exceptions data. Now we do it for all analysis as this is required for more accurate input analysis data at start of catch and finally blocks. NOTE: Fixing this issue led to an assert firing in CodeAnalysis dataflow operation visitor for an existing unit test, which has also been fixed in this PR
1 parent d408d6e commit 64a1e45

File tree

3 files changed

+31
-7
lines changed

3 files changed

+31
-7
lines changed

src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/CopyAnalysis/CopyAnalysis.CopyDataFlowOperationVisitor.cs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -397,9 +397,9 @@ protected override void UpdateValuesForAnalysisData(CopyAnalysisData targetAnaly
397397
continue;
398398
}
399399

400+
var existingValue = targetAnalysisData[key];
400401
if (CurrentAnalysisData.TryGetValue(key, out var newValue))
401402
{
402-
var existingValue = targetAnalysisData[key];
403403
if (newValue.AnalysisEntities.Count == 1)
404404
{
405405
if (existingValue.AnalysisEntities.Count == 1)
@@ -423,6 +423,23 @@ protected override void UpdateValuesForAnalysisData(CopyAnalysisData targetAnaly
423423

424424
processedEntities.AddRange(newValue.AnalysisEntities);
425425
}
426+
else
427+
{
428+
var entitiesToExclude = existingValue.AnalysisEntities.Where(e => !CurrentAnalysisData.HasAbstractValue(e));
429+
var excludedCount = 0;
430+
foreach (var entity in entitiesToExclude)
431+
{
432+
targetAnalysisData.RemoveEntries(entity);
433+
processedEntities.Add(entity);
434+
excludedCount++;
435+
}
436+
437+
if (excludedCount < existingValue.AnalysisEntities.Count)
438+
{
439+
newValue = existingValue.WithEntitiesRemoved(entitiesToExclude);
440+
targetAnalysisData.SetAbstactValueForEntities(newValue, entityBeingAssigned: null);
441+
}
442+
}
426443
}
427444
}
428445

src/Utilities/FlowAnalysis/FlowAnalysis/Framework/DataFlow/DataFlowAnalysis.cs

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ protected DataFlowAnalysis(AbstractAnalysisDomain<TAnalysisData> analysisDomain,
8686
// i.e. for every '{key, value}' pair in the dictionary, 'key' is the destination of at least one back edge
8787
// and 'value' is the minimum ordinal such that there is no back edge to 'key' from any basic block with ordinal > 'value'.
8888
using var loopRangeMap = PooledDictionary<int, int>.GetInstance();
89-
ComputeLoopRangeMap(cfg, loopRangeMap);
89+
var hasAnyTryBlock = ComputeLoopRangeMap(cfg, loopRangeMap);
9090

9191
TAnalysisData? normalPathsExitBlockData = null, exceptionPathsExitBlockData = null;
9292

@@ -115,7 +115,12 @@ protected DataFlowAnalysis(AbstractAnalysisDomain<TAnalysisData> analysisDomain,
115115
blockToUniqueInputFlowMap, loopRangeMap, exceptionPathsAnalysisPostPass: false);
116116
normalPathsExitBlockData = resultBuilder.ExitBlockOutputData;
117117

118-
if (analysisContext.ExceptionPathsAnalysis)
118+
// If we are executing exception paths analysis OR have at least one try/catch/finally block
119+
// in the method, execute an exception path analysis post pass.
120+
// This post pass will handle all possible operations within the control flow graph that can
121+
// throw an exception and merge analysis data after all such operation analyses into the
122+
// catch blocks reachable from those operations.
123+
if (analysisContext.ExceptionPathsAnalysis || hasAnyTryBlock)
119124
{
120125
RoslynDebug.Assert(normalPathsExitBlockData != null);
121126

@@ -286,11 +291,9 @@ private void RunCore(
286291

287292
// Check if we are starting a try region which has one or more associated catch/filter regions.
288293
// If so, we conservatively merge the input data for try region into the input data for the associated catch/filter regions.
289-
#pragma warning disable CA1508 // Avoid dead conditional code - https://github.com/dotnet/roslyn-analyzers/issues/4408
290294
if (block.EnclosingRegion?.Kind == ControlFlowRegionKind.Try &&
291295
block.EnclosingRegion.EnclosingRegion?.Kind == ControlFlowRegionKind.TryAndCatch &&
292296
block.EnclosingRegion.EnclosingRegion.FirstBlockOrdinal == block.Ordinal)
293-
#pragma warning restore CA1508 // Avoid dead conditional code
294297
{
295298
MergeIntoCatchInputData(block.EnclosingRegion.EnclosingRegion, input, block);
296299
}
@@ -791,15 +794,20 @@ private void CloneAndUpdateOutputIfEntryOrExitBlock(DataFlowAnalysisResultBuilde
791794
}
792795
}
793796

794-
private static void ComputeLoopRangeMap(ControlFlowGraph cfg, PooledDictionary<int, int> loopRangeMap)
797+
private static bool ComputeLoopRangeMap(ControlFlowGraph cfg, PooledDictionary<int, int> loopRangeMap)
795798
{
799+
var hasAnyTryBlock = false;
796800
for (int i = cfg.Blocks.Length - 1; i > 0; i--)
797801
{
798802
var block = cfg.Blocks[i];
799803
HandleBranch(block.FallThroughSuccessor);
800804
HandleBranch(block.ConditionalSuccessor);
805+
806+
hasAnyTryBlock |= block.EnclosingRegion.Kind == ControlFlowRegionKind.Try;
801807
}
802808

809+
return hasAnyTryBlock;
810+
803811
void HandleBranch(ControlFlowBranch? branch)
804812
{
805813
if (branch?.Destination != null && branch.IsBackEdge() && !loopRangeMap.ContainsKey(branch.Destination.Ordinal))

src/Utilities/FlowAnalysis/FlowAnalysis/Framework/DataFlow/DataFlowOperationVisitor.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -844,7 +844,6 @@ private TAbstractAnalysisValue GetAbstractValueForReturnOperation(IOperation ret
844844

845845
protected virtual void HandlePossibleThrowingOperation(IOperation operation)
846846
{
847-
Debug.Assert(DataFlowAnalysisContext.ExceptionPathsAnalysis);
848847
Debug.Assert(ExecutingExceptionPathsAnalysisPostPass);
849848

850849
// Bail out if we are not analyzing an interprocedural call and there is no

0 commit comments

Comments
 (0)