From 4d1b3e06c7cb0f67b50ca16b8d579c30dcfb9886 Mon Sep 17 00:00:00 2001
From: Paul <pfultz2@yahoo.com>
Date: Thu, 10 Sep 2020 17:06:49 -0500
Subject: [PATCH] Fix FPs

---
 lib/forwardanalyzer.cpp | 21 ++++++++++++++++-----
 test/testvalueflow.cpp  | 23 +++++++++++++++++++++++
 2 files changed, 39 insertions(+), 5 deletions(-)

diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp
index 410d38207..4fe7bd6fd 100644
--- a/lib/forwardanalyzer.cpp
+++ b/lib/forwardanalyzer.cpp
@@ -10,9 +10,19 @@
 
 struct ForwardTraversal {
     enum class Progress { Continue, Break, Skip };
+    ForwardTraversal(const ValuePtr<ForwardAnalyzer>& analyzer, const Settings* settings)
+    : analyzer(analyzer), settings(settings), actions(ForwardAnalyzer::Action::None), analyzeOnly(false)
+    {}
     ValuePtr<ForwardAnalyzer> analyzer;
     const Settings* settings;
     ForwardAnalyzer::Action actions;
+    bool analyzeOnly;
+
+    bool stopUpdates()
+    {
+        analyzeOnly = true;
+        return actions.isModified();
+    }
 
     std::pair<bool, bool> evalCond(const Token* tok) {
         std::vector<int> result = analyzer->evaluate(tok);
@@ -92,7 +102,7 @@ struct ForwardTraversal {
             std::tie(checkThen, checkElse) = evalCond(condTok);
             if (!checkThen && !checkElse) {
                 // Stop if the value is conditional
-                if (!traverseUnknown && analyzer->isConditional())
+                if (!traverseUnknown && analyzer->isConditional() && stopUpdates())
                     return Progress::Break;
                 checkThen = true;
                 checkElse = true;
@@ -117,7 +127,7 @@ struct ForwardTraversal {
     Progress update(Token* tok) {
         ForwardAnalyzer::Action action = analyzer->analyze(tok);
         actions |= action;
-        if (!action.isNone())
+        if (!action.isNone() && !analyzeOnly)
             analyzer->update(tok, action);
         if (action.isInconclusive() && !analyzer->lowerToInconclusive())
             return Progress::Break;
@@ -402,6 +412,7 @@ struct ForwardTraversal {
                     } else {
                         tok = endBlock;
                     }
+                    actions |= (thenAction | elseAction);
                     if (bail)
                         return Progress::Break;
                     if (returnThen && returnElse)
@@ -415,7 +426,7 @@ struct ForwardTraversal {
                         if (checkThen) {
                             return Progress::Break;
                         } else {
-                            if (analyzer->isConditional())
+                            if (analyzer->isConditional() && stopUpdates())
                                 return Progress::Break;
                             analyzer->assume(condTok, false);
                         }
@@ -424,8 +435,8 @@ struct ForwardTraversal {
                         if (!analyzer->lowerToInconclusive())
                             return Progress::Break;
                     } else if (thenAction.isModified() || elseAction.isModified()) {
-                        if (!hasElse && analyzer->isConditional())
-                            return Progress::Break;
+                        if (!hasElse && analyzer->isConditional() && stopUpdates())
+                                return Progress::Break;
                         if (!analyzer->lowerToPossible())
                             return Progress::Break;
                         analyzer->assume(condTok, elseAction.isModified());
diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp
index a149df581..4ed668e14 100644
--- a/test/testvalueflow.cpp
+++ b/test/testvalueflow.cpp
@@ -2424,6 +2424,29 @@ private:
                "    }"
                "}";
         ASSERT_EQUALS(false, testValueOfXKnown(code, 3U, 2));
+
+        code = "int f(int i, int j) {\n"
+               "    if (i == 0) {\n"
+               "        if (j < 0)\n"
+               "            return 0;\n"
+               "        i = j+1;\n"
+               "    }\n"
+               "    int x = i;\n"
+               "    return x;\n"
+               "}\n";
+        ASSERT_EQUALS(false, testValueOfX(code, 8U, 0));
+
+        code = "int f(int i, int j) {\n"
+               "    if (i == 0) {\n"
+               "        if (j < 0)\n"
+               "            return 0;\n"
+               "        if (j < 0)\n"
+               "            i = j+1;\n"
+               "    }\n"
+               "    int x = i;\n"
+               "    return x;\n"
+               "}\n";
+        ASSERT_EQUALS(true, testValueOfX(code, 9U, 0));
     }
 
     void valueFlowAfterConditionExpr() {