Fix issue 9890: ValueFlow: known value not found (variable is changed in path that returns) (#3010)

lib/forwardanalyzer.cpp

@@ -1,4 +1,5 @@
 #include "forwardanalyzer.h"
+#include "analyzer.h"
 #include "astutils.h"
 #include "settings.h"
 #include "symboldatabase.h"
@@ -18,6 +19,29 @@ struct ForwardTraversal {
     Analyzer::Action actions;
     bool analyzeOnly;
 
+    struct Branch {
+        Analyzer::Action action = Analyzer::Action::None;
+        bool check = false;
+        bool escape = false;
+        bool escapeUnknown = false;
+        const Token* endBlock = nullptr;
+        bool isEscape() const {
+            return escape || escapeUnknown;
+        }
+        bool isConclusiveEscape() const {
+            return escape && !escapeUnknown;
+        }
+        bool isModified() const {
+            return action.isModified() && !isConclusiveEscape();
+        }
+        bool isInconclusive() const {
+            return action.isInconclusive() && !isConclusiveEscape();
+        }
+        bool isDead() const {
+            return action.isModified() || action.isInconclusive() || isEscape();
+        }
+    };
+
     bool stopUpdates() {
         analyzeOnly = true;
         return actions.isModified();
@@ -186,6 +210,11 @@ struct ForwardTraversal {
         return result;
     }
 
+    void forkRange(Token* start, const Token* end) {
+        ForwardTraversal ft = *this;
+        ft.updateRange(start, end);
+    }
+
     void forkScope(Token* endBlock, bool isModified = false) {
         if (analyzer->updateScope(endBlock, isModified)) {
             ForwardTraversal ft = *this;
@@ -197,11 +226,11 @@ struct ForwardTraversal {
         return Token::findsimplematch(endBlock->link(), "goto", endBlock);
     }
 
-    bool isEscapeScope(const Token* endBlock, bool unknown = false) {
+    bool isEscapeScope(const Token* endBlock, bool& unknown) {
         const Token* ftok = nullptr;
         bool r = isReturnScope(endBlock, &settings->library, &ftok);
         if (!r && ftok)
-            return unknown;
+            unknown = true;
         return r;
     }
 
@@ -379,34 +408,32 @@ struct ForwardTraversal {
                     // Traverse condition
                     if (updateRecursive(condTok) == Progress::Break)
                         return Progress::Break;
+                    Branch thenBranch{};
+                    Branch elseBranch{};
                     // Check if condition is true or false
-                    bool checkThen, checkElse;
+                    std::tie(thenBranch.check, elseBranch.check) = evalCond(condTok);
-                    std::tie(checkThen, checkElse) = evalCond(condTok);
-                    Analyzer::Action thenAction = Analyzer::Action::None;
-                    Analyzer::Action elseAction = Analyzer::Action::None;
                     bool hasElse = Token::simpleMatch(endBlock, "} else {");
                     bool bail = false;
 
                     // Traverse then block
-                    bool returnThen = isEscapeScope(endBlock, true);
+                    thenBranch.escape = isEscapeScope(endBlock, thenBranch.escapeUnknown);
-                    bool returnElse = false;
+                    if (thenBranch.check) {
-                    if (checkThen) {
                         if (updateRange(endCond->next(), endBlock) == Progress::Break)
                             return Progress::Break;
-                    } else if (!checkElse) {
+                    } else if (!elseBranch.check) {
-                        thenAction = checkScope(endBlock);
+                        thenBranch.action = checkScope(endBlock);
                         if (hasGoto(endBlock))
                             bail = true;
                     }
                     // Traverse else block
                     if (hasElse) {
-                        returnElse = isEscapeScope(endBlock->linkAt(2), true);
+                        elseBranch.escape = isEscapeScope(endBlock->linkAt(2), elseBranch.escapeUnknown);
-                        if (checkElse) {
+                        if (elseBranch.check) {
                             Progress result = updateRange(endBlock->tokAt(2), endBlock->linkAt(2));
                             if (result == Progress::Break)
                                 return Progress::Break;
-                        } else if (!checkThen) {
+                        } else if (!thenBranch.check) {
-                            elseAction = checkScope(endBlock->linkAt(2));
+                            elseBranch.action = checkScope(endBlock->linkAt(2));
                             if (hasGoto(endBlock))
                                 bail = true;
                         }
@@ -414,18 +441,18 @@ struct ForwardTraversal {
                     } else {
                         tok = endBlock;
                     }
-                    actions |= (thenAction | elseAction);
+                    actions |= (thenBranch.action | elseBranch.action);
                     if (bail)
                         return Progress::Break;
-                    if (returnThen && returnElse)
+                    if (thenBranch.isDead() && elseBranch.isDead())
-                        return Progress::Break;
-                    else if (thenAction.isModified() && elseAction.isModified())
-                        return Progress::Break;
-                    else if ((returnThen || returnElse) && (thenAction.isModified() || elseAction.isModified()))
                         return Progress::Break;
                     // Conditional return
-                    if (returnThen && !hasElse) {
+                    if (thenBranch.isEscape() && !hasElse) {
-                        if (checkThen) {
+                        if (!thenBranch.isConclusiveEscape()) {
+                            if (!analyzer->lowerToInconclusive())
+                                return Progress::Break;
+                        }
+                        else if (thenBranch.check) {
                             return Progress::Break;
                         } else {
                             if (analyzer->isConditional() && stopUpdates())
@@ -433,15 +460,15 @@ struct ForwardTraversal {
                             analyzer->assume(condTok, false);
                         }
                     }
-                    if (thenAction.isInconclusive() || elseAction.isInconclusive()) {
+                    if (thenBranch.isInconclusive() || elseBranch.isInconclusive()) {
                         if (!analyzer->lowerToInconclusive())
                             return Progress::Break;
-                    } else if (thenAction.isModified() || elseAction.isModified()) {
+                    } else if (thenBranch.isModified() || elseBranch.isModified()) {
                         if (!hasElse && analyzer->isConditional() && stopUpdates())
                             return Progress::Break;
                         if (!analyzer->lowerToPossible())
                             return Progress::Break;
-                        analyzer->assume(condTok, elseAction.isModified());
+                        analyzer->assume(condTok, elseBranch.isModified());
                     }
                 }
             } else if (Token::simpleMatch(tok, "try {")) {

lib/templatesimplifier.cpp

@@ -445,7 +445,7 @@ unsigned int TemplateSimplifier::templateParameters(const Token *tok)
         }
 
         // skip std::
-        if (tok && tok->str() == "::")
+        if (tok->str() == "::")
             tok = tok->next();
         while (Token::Match(tok, "%name% ::")) {
             tok = tok->tokAt(2);

lib/valueflow.cpp

@@ -2314,7 +2314,7 @@ struct SingleValueFlowAnalyzer : ValueFlowAnalyzer {
         if (value.conditional)
             return true;
         if (value.condition)
-            return !value.isImpossible();
+            return !value.isKnown() && !value.isImpossible();
         return false;
     }
 

test/testcondition.cpp

@@ -3494,6 +3494,23 @@ private:
               "        if (p != (uint32_t) -1) {}\n"
               "}\n");
         ASSERT_EQUALS("", errout.str());
+
+        // #9890
+        check("int g(int);\n"
+              "bool h(int*);\n"
+              "int f(int *x) {\n"
+              "    int y = g(0);\n"
+              "    if (!y) {\n"
+              "        if (h(x)) {\n"
+              "            y = g(1);\n"
+              "            if (y) {}\n"
+              "            return 0;\n"
+              "        }\n"
+              "        if (!y) {}\n"
+              "    }\n"
+              "    return 0;\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:11]: (style) Condition '!y' is always true\n", errout.str());
     }
 
     void alwaysTrueInfer() {