Refactor LVA and fix S1854 FP

analyzers/src/SonarAnalyzer.CFG/LiveVariableAnalysis/RoslynLiveVariableAnalysis.cs

@@ -30,10 +30,24 @@ namespace SonarAnalyzer.CFG.LiveVariableAnalysis
 {
     public sealed class RoslynLiveVariableAnalysis : LiveVariableAnalysisBase<ControlFlowGraph, BasicBlock>
     {
+        private readonly Dictionary<int, List<BasicBlock>> blockPredecessors = new();
+        private readonly Dictionary<int, List<BasicBlock>> blockSuccessors = new();
+
         protected override BasicBlock ExitBlock => Cfg.ExitBlock;
 
-        public RoslynLiveVariableAnalysis(ControlFlowGraph cfg) : base(cfg, OriginalDeclaration(cfg.OriginalOperation)) =>
+        public RoslynLiveVariableAnalysis(ControlFlowGraph cfg) : base(cfg, OriginalDeclaration(cfg.OriginalOperation))
+        {
+            foreach (var ordinal in cfg.Blocks.Select(x => x.Ordinal))
+            {
+                blockPredecessors.Add(ordinal, new());
+                blockSuccessors.Add(ordinal, new());
+            }
+            foreach (var block in cfg.Blocks)
+            {
+                BuildBranches(block);
+            }
             Analyze();
+        }
 
         public ISymbol ParameterOrLocalSymbol(IOperation operation)
         {
@@ -49,84 +63,60 @@ var _ when ILocalReferenceOperationWrapper.IsInstance(operation) => ILocalRefere
         protected override IEnumerable<BasicBlock> ReversedBlocks() =>
             Cfg.Blocks.Reverse();
 
-        protected override IEnumerable<BasicBlock> Successors(BasicBlock block)
+        protected override IEnumerable<BasicBlock> Predecessors(BasicBlock block) =>
+            blockPredecessors[block.Ordinal];
+
+        protected override IEnumerable<BasicBlock> Successors(BasicBlock block) =>
+            blockSuccessors[block.Ordinal];
+
+        protected override State ProcessBlock(BasicBlock block)
+        {
+            var ret = new RoslynState(this);
+            ret.ProcessBlock(Cfg, block);
+            return ret;
+        }
+
+        public override bool IsLocal(ISymbol symbol) =>
+            originalDeclaration.Equals(symbol?.ContainingSymbol);
+
+        private void BuildBranches(BasicBlock block)
         {
             foreach (var successor in block.Successors)
             {
-                if (successor.Destination != null && successor.FinallyRegions.Any())
-                {   // When exiting finally region, redirect to finally instead of the normal destination
-                    foreach (var finallyRegion in successor.FinallyRegions)
-                    {
-                        yield return Cfg.Blocks[finallyRegion.FirstBlockOrdinal];
-                    }
-                }
-                else if (successor.Destination != null)
+                if (successor.Destination != null)
                 {
-                    yield return successor.Destination;
+                    // When exiting finally region, redirect to finally instead of the normal destination
+                    AddBranch(successor.Source, successor.FinallyRegions.Any() ? Cfg.Blocks[successor.FinallyRegions.First().FirstBlockOrdinal] : successor.Destination);
                 }
-                else if (successor.Source.EnclosingRegion.Kind == ControlFlowRegionKind.Finally)
-                {   // Redirect exit from throw and finally to following blocks.
-                    foreach (var trySuccessor in TryRegionSuccessors(block.EnclosingRegion))
-                    {
-                        yield return trySuccessor.Destination;
-                    }
+                else if (successor.Source.EnclosingRegion is { Kind: ControlFlowRegionKind.Finally } finallyRegion)
+                {
+                    BuildBranchesFinally(successor.Source, finallyRegion);
                 }
             }
             if (block.IsEnclosedIn(ControlFlowRegionKind.Try))
             {
                 foreach (var catchOrFilterRegion in block.Successors.SelectMany(CatchOrFilterRegions))
                 {
-                    yield return Cfg.Blocks[catchOrFilterRegion.FirstBlockOrdinal];
+                    AddBranch(block, Cfg.Blocks[catchOrFilterRegion.FirstBlockOrdinal]);
                 }
             }
         }
 
-        protected override IEnumerable<BasicBlock> Predecessors(BasicBlock block)
+        private void BuildBranchesFinally(BasicBlock source, ControlFlowRegion finallyRegion)
         {
-            if (block.Predecessors.Any())
+            // Redirect exit from throw and finally to following blocks.
+            foreach (var trySuccessor in TryRegionSuccessors(source.EnclosingRegion))
             {
-                foreach (var predecessor in block.Predecessors)
-                {
-                    // When exiting finally region, redirect predecessor to the source of StructuredEceptionHandling branches
-                    if (predecessor.FinallyRegions.Any())
-                    {
-                        foreach (var structuredExceptionHandling in StructuredExceptionHandlinBranches(predecessor.FinallyRegions))
-                        {
-                            yield return structuredExceptionHandling.Source;
-                        }
-                    }
-                    else
-                    {
-                        yield return predecessor.Source;
-                    }
-                }
+                AddBranch(source, trySuccessor.Destination);
             }
-            else if (block.EnclosingNonLocalLifetimeRegion() is var enclosingRegion
-                && enclosingRegion.Kind == ControlFlowRegionKind.Finally
-                && block.Ordinal == block.EnclosingRegion.FirstBlockOrdinal)
-            {
-                // Link first block of FinallyRegion to the source of all branches exiting that FinallyRegion
-                foreach (var trySuccessor in TryRegionSuccessors(enclosingRegion))
-                {
-                    yield return trySuccessor.Source;
-                }
-            }
-
-            IEnumerable<ControlFlowBranch> StructuredExceptionHandlinBranches(IEnumerable<ControlFlowRegion> finallyRegions) =>
-                finallyRegions.Select(x => Cfg.Blocks[x.LastBlockOrdinal].Successors.SingleOrDefault(x => x.Semantics == ControlFlowBranchSemantics.StructuredExceptionHandling))
-                    .Where(x => x != null);
         }
 
-        protected override State ProcessBlock(BasicBlock block)
+        private void AddBranch(BasicBlock source, BasicBlock destination)
         {
-            var ret = new RoslynState(this);
-            ret.ProcessBlock(Cfg, block);
-            return ret;
+            blockSuccessors[source.Ordinal].Add(destination);
+            blockPredecessors[destination.Ordinal].Add(source);
         }
 
-        public override bool IsLocal(ISymbol symbol) =>
-            originalDeclaration.Equals(symbol?.ContainingSymbol);
-
         private IEnumerable<ControlFlowBranch> TryRegionSuccessors(ControlFlowRegion finallyRegion)
         {
             var tryRegion = finallyRegion.EnclosingRegion.NestedRegions.Single(x => x.Kind == ControlFlowRegionKind.Try);

analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/LocksReleasedAllPathsBase.cs

@@ -134,7 +134,6 @@ ProgramState ProcessCondition(ISymbol lockSymbol)
                         : RemoveLock(context, lockSymbol);
                 }
             }
-
         }
 
         protected override ProgramState PostProcessSimple(SymbolicContext context)

analyzers/tests/SonarAnalyzer.UnitTest/LiveVariableAnalysis/RoslynLiveVariableAnalysisTest.TryCatchFinally.cs

@@ -559,8 +559,8 @@ public void Catch_Loop_Propagates_LiveIn()
             var context = CreateContextCS(code);
             context.ValidateEntry();
             context.Validate("Method(0);", new LiveOut("variable"));
-            context.Validate("Method(1);", new LiveIn("variable")/*Should be:, new LiveOut("variable")*/);
-            context.Validate("Method(2);");
+            context.Validate("Method(1);", new LiveIn("variable"), new LiveOut("variable"));
+            context.Validate("Method(2);", new LiveIn("variable"), new LiveOut("variable"));
             context.Validate("Method(3);", new LiveIn("variable"), new LiveOut("variable"));
             context.Validate("Method(4);", new LiveIn("variable"), new LiveOut("variable"));
             context.Validate("Method(5);");

analyzers/tests/SonarAnalyzer.UnitTest/TestCases/DeadStores.RoslynCfg.cs

@@ -1389,7 +1389,7 @@ public void TryReturns_Loop()
             var counter = 0;
             while (counter < 5)
             {
-                counter++;  // Noncompliant FP related to LVA
+                counter++;  // Compliant
                 try
                 {
                     SomethingThatCanThrow();