Fix 11985: False positive: uninitvar (valueflow) (#5781)

Author: pfultz2

lib/analyzer.h

@@ -181,6 +181,8 @@ struct Analyzer {
     virtual bool stopOnCondition(const Token* condTok) const = 0;
     /// The condition that will be assumed during analysis
     virtual void assume(const Token* tok, bool state, unsigned int flags = 0) = 0;
+    /// Update the state of the program at the token
+    virtual void updateState(const Token* tok) = 0;
     /// Return analyzer for expression at token
     virtual ValuePtr<Analyzer> reanalyze(Token* tok, const std::string& msg = emptyString) const = 0;
     virtual bool invalid() const {

lib/forwardanalyzer.cpp

@@ -443,7 +443,7 @@ namespace {
             if (checkElse && isDoWhile &&
                 (condTok->hasKnownIntValue() ||
                  (!bodyAnalysis.isModified() && !condAnalysis.isModified() && condAnalysis.isRead()))) {
-                if (updateRange(endBlock->link(), endBlock) == Progress::Break)
+                if (updateScope(endBlock) == Progress::Break)
                     return Break();
                 return updateRecursive(condTok);
             }
@@ -519,8 +519,17 @@ namespace {
             return updateLoop(endToken, endBlock, condTok, initTok, stepTok, true);
         }
 
-        Progress updateScope(Token* endBlock) {
-            return updateRange(endBlock->link(), endBlock);
+        Progress updateScope(Token* endBlock, int depth = 20)
+        {
+            if (!endBlock)
+                return Break();
+            assert(endBlock->link());
+            Token* ctx = endBlock->link()->previous();
+            if (Token::simpleMatch(ctx, ")"))
+                ctx = ctx->link()->previous();
+            if (ctx)
+                analyzer->updateState(ctx);
+            return updateRange(endBlock->link(), endBlock, depth);
         }
 
         Progress updateRange(Token* start, const Token* end, int depth = 20) {
@@ -682,7 +691,7 @@ namespace {
                         thenBranch.escape = isEscapeScope(endBlock, thenBranch.escapeUnknown);
                         if (thenBranch.check) {
                             thenBranch.active = true;
-                            if (updateRange(endCond->next(), endBlock, depth - 1) == Progress::Break)
+                            if (updateScope(endBlock, depth - 1) == Progress::Break)
                                 return Break();
                         } else if (!elseBranch.check) {
                             thenBranch.active = true;
@@ -694,7 +703,7 @@ namespace {
                             elseBranch.escape = isEscapeScope(endBlock->linkAt(2), elseBranch.escapeUnknown);
                             if (elseBranch.check) {
                                 elseBranch.active = true;
-                                const Progress result = updateRange(endBlock->tokAt(2), endBlock->linkAt(2), depth - 1);
+                                const Progress result = updateScope(endBlock->linkAt(2), depth - 1);
                                 if (result == Progress::Break)
                                     return Break();
                             } else if (!thenBranch.check) {
@@ -746,7 +755,7 @@ namespace {
                 } else if (Token::simpleMatch(tok, "try {")) {
                     Token* endBlock = tok->next()->link();
                     ForwardTraversal tryTraversal = fork();
-                    tryTraversal.updateRange(tok->next(), endBlock, depth - 1);
+                    tryTraversal.updateScope(endBlock, depth - 1);
                     bool bail = tryTraversal.actions.isModified();
                     if (bail) {
                         actions = tryTraversal.actions;
@@ -760,7 +769,7 @@ namespace {
                             return Break();
                         endBlock = endCatch->linkAt(1);
                         ForwardTraversal ft = fork();
-                        ft.updateRange(endBlock->link(), endBlock, depth - 1);
+                        ft.updateScope(endBlock, depth - 1);
                         bail |= ft.terminate != Analyzer::Terminate::None || ft.actions.isModified();
                     }
                     if (bail)
@@ -880,6 +889,8 @@ Analyzer::Result valueFlowGenericForward(Token* start, const Token* end, const V
     if (a->invalid())
         return Analyzer::Result{Analyzer::Action::None, Analyzer::Terminate::Bail};
     ForwardTraversal ft{a, settings};
+    if (start)
+        ft.analyzer->updateState(start);
     ft.updateRange(start, end);
     return Analyzer::Result{ ft.actions, ft.terminate };
 }

lib/valueflow.cpp

@@ -2969,6 +2969,13 @@ struct ValueFlowAnalyzer : Analyzer {
             makeConditional();
     }
 
+    void updateState(const Token* tok) override
+    {
+        // Update program state
+        pms.removeModifiedVars(tok);
+        pms.addState(tok, getProgramState());
+    }
+
     virtual void internalUpdate(Token* /*tok*/, const ValueFlow::Value& /*v*/, Direction /*d*/)
     {
         assert(false && "Internal update unimplemented.");

test/testvalueflow.cpp

@@ -876,6 +876,23 @@ class TestValueFlow : public TestFixture {
                "   y=x;\n"
                "}";
         ASSERT_EQUALS(false, testValueOfX(code, 3U, ValueFlow::Value::MoveKind::MovedVariable));
+
+        code = "void g(std::string);\n"
+               "void foo(std::string x) {\n"
+               "  g(std::move(x));\n"
+               "  int counter = 0;\n"
+               "\n"
+               "  for (int i = 0; i < 5; i++) {\n"
+               "    if (i % 2 == 0) {\n"
+               "      x = std::to_string(i);\n"
+               "      counter++;\n"
+               "    }\n"
+               "  }\n"
+               "  for (int i = 0; i < counter; i++) {\n"
+               "      if(x > 5) {}\n"
+               "  }\n"
+               "}\n";
+        ASSERT_EQUALS(false, testValueOfX(code, 13U, ValueFlow::Value::MoveKind::MovedVariable));
     }
 
     void valueFlowCalculations() {
@@ -5671,6 +5688,22 @@ class TestValueFlow : public TestFixture {
                "}";
         values = tokenValues(code, "buflen >=", ValueFlow::Value::ValueType::UNINIT);
         ASSERT_EQUALS(1, values.size());
+
+        code = "void foo() {\n"
+               "  int counter = 0;\n"
+               "  int x;\n"
+               "  for (int i = 0; i < 5; i++) {\n"
+               "    if (i % 2 == 0) {\n"
+               "      x = i;\n"
+               "      counter++;\n"
+               "    }\n"
+               "  }\n"
+               "  for (int i = 0; i < counter; i++) {\n"
+               "      if(x > 5) {}\n"
+               "  }\n"
+               "}\n";
+        values = tokenValues(code, "x > 5", ValueFlow::Value::ValueType::UNINIT);
+        ASSERT_EQUALS(0, values.size());
     }
 
     void valueFlowConditionExpressions() {