Skip to content

Commit

Permalink
Fix S1854 FP: Throw should visit finally (#9530)
Browse files Browse the repository at this point in the history
  • Loading branch information
@sebastien-marichal
sebastien-marichal authored Jul 19, 2024
1 parent 527abcb commit 19c58d5
Show file tree
Hide file tree
Showing 5 changed files with 506 additions and 63 deletions.
2 changes: 1 addition & 1 deletion analyzers/src/SonarAnalyzer.CFG/CfgSerializer/CfgSerializer.RoslynLvaWalker.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ protected override void WriteEdges(BasicBlock block)
{
foreach (var predecessor in lva.BlockPredecessors[block.Ordinal].Where(x => !block.Predecessors.Any(y => y.Source == x)))
{
writer.WriteEdge(BlockId(predecessor), BlockId(block), "LVA");
writer.WriteEdge(BlockId(predecessor), BlockId(block), "LVA\" fontcolor=\"blue\" penwidth=\"2\" color=\"blue");
}
base.WriteEdges(block);
}
Expand Down
15 changes: 10 additions & 5 deletions analyzers/src/SonarAnalyzer.CFG/LiveVariableAnalysis/RoslynLiveVariableAnalysis.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,9 @@ private void BuildBranches(BasicBlock block)
AddPredecessorsOutsideRegion(finallyBlock);
}
}
if (block.IsEnclosedIn(ControlFlowRegionKind.Catch) && block.Successors.Any(x => x.Semantics == ControlFlowBranchSemantics.Rethrow))
if (block.IsEnclosedIn(ControlFlowRegionKind.Catch) && block.Successors.Any(x => x.Semantics is ControlFlowBranchSemantics.Rethrow or ControlFlowBranchSemantics.Throw))
{
BuildBranchesNestedCatchRethrow(block);
BuildBranchesRethrow(block);
}

void AddPredecessorsOutsideRegion(BasicBlock destination)
Expand All @@ -168,13 +168,18 @@ private void BuildBranchesFinally(BasicBlock source, ControlFlowRegion finallyRe
}
}

private void BuildBranchesNestedCatchRethrow(BasicBlock block)
private void BuildBranchesRethrow(BasicBlock block)
{
var reachableHandlers = block.EnclosingRegion(ControlFlowRegionKind.TryAndCatch).NestedRegion(ControlFlowRegionKind.Try).ReachableHandlers();
foreach (var catchBlock in reachableHandlers.Where(x => x.Kind == ControlFlowRegionKind.Catch && x.FirstBlockOrdinal > block.Ordinal).SelectMany(x => x.Blocks(Cfg)))
var reachableHandlerRegions = block.EnclosingRegion(ControlFlowRegionKind.TryAndCatch).NestedRegion(ControlFlowRegionKind.Try).ReachableHandlers();
var reachableCatchAndFinallyBlocks = reachableHandlerRegions.Where(x => x.FirstBlockOrdinal > block.Ordinal).SelectMany(x => x.Blocks(Cfg));
foreach (var catchBlock in reachableCatchAndFinallyBlocks.Where(x => x.EnclosingRegion.Kind is ControlFlowRegionKind.Catch))
{
AddBranch(block, catchBlock);
}
if (reachableCatchAndFinallyBlocks.FirstOrDefault() is { EnclosingRegion.Kind: ControlFlowRegionKind.Finally } finallyBlock)
{
AddBranch(block, finallyBlock);
}
}

private void AddBranch(BasicBlock source, BasicBlock destination)
Expand Down
31 changes: 16 additions & 15 deletions analyzers/tests/SonarAnalyzer.Test/CFG/Roslyn/RoslynLvaSerializerTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,14 @@ void Use(int v) {}
cfg0_block0 [shape=record label="{ENTRY #0}"]
cfg0_block5 [shape=record label="{EXIT #5}"]
cfg0_block1 -> cfg0_block2
cfg0_block2 -> cfg0_block3 [label="LVA"]
cfg0_block1 -> cfg0_block3 [label="LVA"]
cfg0_block2 -> cfg0_block4 [label="LVA"]
cfg0_block3 -> cfg0_block4 [label="LVA"]
cfg0_block2 -> cfg0_block3 [label="LVA" fontcolor="blue" penwidth="2" color="blue"]
cfg0_block1 -> cfg0_block3 [label="LVA" fontcolor="blue" penwidth="2" color="blue"]
cfg0_block2 -> cfg0_block4 [label="LVA" fontcolor="blue" penwidth="2" color="blue"]
cfg0_block3 -> cfg0_block4 [label="LVA" fontcolor="blue" penwidth="2" color="blue"]
cfg0_block4 -> NoDestination_cfg0_block4 [label="StructuredExceptionHandling"]
cfg0_block0 -> cfg0_block1
cfg0_block4 -> cfg0_block5 [label="LVA"]
cfg0_block4 -> cfg0_block5 [label="LVA"]
cfg0_block4 -> cfg0_block5 [label="LVA" fontcolor="blue" penwidth="2" color="blue"]
cfg0_block4 -> cfg0_block5 [label="LVA" fontcolor="blue" penwidth="2" color="blue"]
cfg0_block2 -> cfg0_block5
cfg0_block3 -> cfg0_block5
}
Expand Down Expand Up @@ -160,13 +160,14 @@ void Use(int v) {}
cfg0_block0 [shape=record label="{ENTRY #0}"]
cfg0_block5 [shape=record label="{EXIT #5}"]
cfg0_block1 -> cfg0_block2
cfg0_block2 -> cfg0_block3 [label="LVA"]
cfg0_block1 -> cfg0_block3 [label="LVA"]
cfg0_block2 -> cfg0_block3 [label="LVA" fontcolor="blue" penwidth="2" color="blue"]
cfg0_block1 -> cfg0_block3 [label="LVA" fontcolor="blue" penwidth="2" color="blue"]
cfg0_block3 -> NoDestination_cfg0_block3 [label="Rethrow"]
cfg0_block2 -> cfg0_block4 [label="LVA"]
cfg0_block2 -> cfg0_block4 [label="LVA" fontcolor="blue" penwidth="2" color="blue"]
cfg0_block3 -> cfg0_block4 [label="LVA" fontcolor="blue" penwidth="2" color="blue"]
cfg0_block4 -> NoDestination_cfg0_block4 [label="StructuredExceptionHandling"]
cfg0_block0 -> cfg0_block1
cfg0_block4 -> cfg0_block5 [label="LVA"]
cfg0_block4 -> cfg0_block5 [label="LVA" fontcolor="blue" penwidth="2" color="blue"]
cfg0_block2 -> cfg0_block5
}

Expand Down Expand Up @@ -265,17 +266,17 @@ void Use(int v) {}
cfg0_block3 -> cfg0_block4 [label="Else"]
cfg0_block2 -> cfg0_block3
cfg0_block4 -> cfg0_block3
cfg0_block3 -> cfg0_block5 [label="LVA"]
cfg0_block3 -> cfg0_block5 [label="LVA"]
cfg0_block2 -> cfg0_block5 [label="LVA"]
cfg0_block4 -> cfg0_block5 [label="LVA"]
cfg0_block3 -> cfg0_block5 [label="LVA" fontcolor="blue" penwidth="2" color="blue"]
cfg0_block3 -> cfg0_block5 [label="LVA" fontcolor="blue" penwidth="2" color="blue"]
cfg0_block2 -> cfg0_block5 [label="LVA" fontcolor="blue" penwidth="2" color="blue"]
cfg0_block4 -> cfg0_block5 [label="LVA" fontcolor="blue" penwidth="2" color="blue"]
cfg0_block5 -> cfg0_block6 [label="Else"]
cfg0_block5 -> cfg0_block7 [label="WhenTrue"]
cfg0_block6 -> cfg0_block7
cfg0_block7 -> NoDestination_cfg0_block7 [label="StructuredExceptionHandling"]
cfg0_block1 -> cfg0_block2
cfg0_block0 -> cfg0_block1
cfg0_block7 -> cfg0_block8 [label="LVA"]
cfg0_block7 -> cfg0_block8 [label="LVA" fontcolor="blue" penwidth="2" color="blue"]
cfg0_block3 -> cfg0_block8 [label="WhenFalse"]
}

Expand Down
Loading

0 comments on commit 19c58d5

Please sign in to comment.