From d9774fd92daa45a2f7904f88606f81ff182caa4b Mon Sep 17 00:00:00 2001
From: Pavel Mikula
 <57188685+pavel-mikula-sonarsource@users.noreply.github.com>
Date: Thu, 20 Jun 2024 21:37:39 +0200
Subject: [PATCH] Fix S1854 FP: Value used in finally should LiveIn for all try
 blocks (#9448)

---
 .../LiveVariableAnalysis/RoslynLiveVariableAnalysis.cs | 10 ++++++++--
 .../RoslynLiveVariableAnalysisTest.TryCatchFinally.cs  |  2 +-
 .../TestCases/DeadStores.RoslynCfg.cs                  |  8 ++++----
 3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/analyzers/src/SonarAnalyzer.CFG/LiveVariableAnalysis/RoslynLiveVariableAnalysis.cs b/analyzers/src/SonarAnalyzer.CFG/LiveVariableAnalysis/RoslynLiveVariableAnalysis.cs
index 70944b96a9b..7ad0b47f127 100644
--- a/analyzers/src/SonarAnalyzer.CFG/LiveVariableAnalysis/RoslynLiveVariableAnalysis.cs
+++ b/analyzers/src/SonarAnalyzer.CFG/LiveVariableAnalysis/RoslynLiveVariableAnalysis.cs
@@ -88,7 +88,7 @@ private void BuildBranches(BasicBlock block)
                 BuildBranchesFinally(successor.Source, finallyRegion);
             }
         }
-        if (block.IsEnclosedIn(ControlFlowRegionKind.Try))
+        if (block.EnclosingNonLocalLifetimeRegion() is { Kind: ControlFlowRegionKind.Try } tryRegion)
         {
             var catchesAll = false;
             foreach (var catchOrFilterRegion in block.Successors.SelectMany(CatchOrFilterRegions))
@@ -98,7 +98,13 @@ private void BuildBranches(BasicBlock block)
             }
             if (!catchesAll && block.EnclosingRegion(ControlFlowRegionKind.TryAndFinally)?.NestedRegion(ControlFlowRegionKind.Finally) is { } finallyRegion)
             {
-                AddBranch(block, Cfg.Blocks[finallyRegion.FirstBlockOrdinal]);
+                var finallyBlock = Cfg.Blocks[finallyRegion.FirstBlockOrdinal];
+                AddBranch(block, finallyBlock);
+                // We assume that this block can throw in its first operation. Therefore predecessors outside this tryRegion need to be redirected to finally
+                foreach (var predecessor in block.Predecessors.Where(x => x.Source.Ordinal < tryRegion.FirstBlockOrdinal || x.Source.Ordinal > tryRegion.LastBlockOrdinal))
+                {
+                    AddBranch(predecessor.Source, finallyBlock);
+                }
             }
         }
     }
diff --git a/analyzers/tests/SonarAnalyzer.Test/LiveVariableAnalysis/RoslynLiveVariableAnalysisTest.TryCatchFinally.cs b/analyzers/tests/SonarAnalyzer.Test/LiveVariableAnalysis/RoslynLiveVariableAnalysisTest.TryCatchFinally.cs
index 84a299f8673..677b4356a37 100644
--- a/analyzers/tests/SonarAnalyzer.Test/LiveVariableAnalysis/RoslynLiveVariableAnalysisTest.TryCatchFinally.cs
+++ b/analyzers/tests/SonarAnalyzer.Test/LiveVariableAnalysis/RoslynLiveVariableAnalysisTest.TryCatchFinally.cs
@@ -1031,7 +1031,7 @@ public void Finally_AssignedInTry_LiveOut()
             """;
         var context = CreateContextCS(code);
         context.ValidateEntry();
-        context.Validate("Method(0);"/*Should be:, LiveOut("variable")*/);
+        context.Validate("Method(0);", LiveOut("variable"));
         context.Validate("Method(1);", LiveOut("variable"));
         context.Validate("Method(variable);", LiveIn("variable"));
         context.Validate("Method(2);");
diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/DeadStores.RoslynCfg.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/DeadStores.RoslynCfg.cs
index 7a00743379e..04b6f8d46d6 100644
--- a/analyzers/tests/SonarAnalyzer.Test/TestCases/DeadStores.RoslynCfg.cs
+++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/DeadStores.RoslynCfg.cs
@@ -192,7 +192,7 @@ void calculateRate2(int a, int b)
                 x = 11; // FN, muted due to try/catch
                 x = 12;
                 Console.Write(x);
-                x = 13; // Compliant, Console.Write could throw and value is read fater try/catch
+                x = 13; // FN, muted due to try/catch
             }
             catch (Exception)
             {
@@ -1186,7 +1186,7 @@ public static void DeadStore(int[] array)
                 x = 11;     // FN, muted due to try/catch
                 x = 12;
                 Console.Write(x);
-                x = 13;     // Compliant, Console.Write can throw
+                x = 13;     // FN, muted due to try/catch
             }
             catch (Exception)
             {
@@ -1420,7 +1420,7 @@ public void UsedInFinally_NestedInLambda()
             {
                 Action a = () =>
                 {
-                    int value = 42; // Noncompliant FP
+                    int value = 42; // Compliant
                     try
                     {
                         SomethingThatCanThrow();
@@ -1436,7 +1436,7 @@ public void UsedInFinally_NestedInLambda()
             {
             }
         }
-
+        
         public void UsedInFinally_Throw()
         {
             var value = 42; // Compliant