Fix 10851: False positive: known variable value below for loop (#3891)

* Fix 10851: False positive: known variable value below for loop

* Format

* Add test for 10863

* Format

lib/programmemory.cpp

@@ -241,11 +241,12 @@ void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, const Toke
             return;
         if (endTok && isExpressionChanged(vartok, tok->next(), endTok, settings, true))
             return;
-        bool impossible = (tok->str() == "==" && !then) || (tok->str() == "!=" && then);
+        const bool impossible = (tok->str() == "==" && !then) || (tok->str() == "!=" && then);
-        pm.setIntValue(vartok, then ? truevalue.intvalue : falsevalue.intvalue, impossible);
+        const ValueFlow::Value& v = then ? truevalue : falsevalue;
+        pm.setValue(vartok, impossible ? asImpossible(v) : v);
         const Token* containerTok = settings->library.getContainerFromYield(vartok, Library::Container::Yield::SIZE);
         if (containerTok)
-            pm.setContainerSizeValue(containerTok, then ? truevalue.intvalue : falsevalue.intvalue, !impossible);
+            pm.setContainerSizeValue(containerTok, v.intvalue, !impossible);
     } else if (Token::simpleMatch(tok, "!")) {
         programMemoryParseCondition(pm, tok->astOperand1(), endTok, settings, !then);
     } else if (then && Token::simpleMatch(tok, "&&")) {
@@ -760,6 +761,9 @@ static ValueFlow::Value execute(const Token* expr, ProgramMemory& pm, const Sett
     for (const ValueFlow::Value& value : expr->values()) {
         if (!value.isSymbolicValue())
             continue;
+        // TODO: Handle possible symbolic values
+        if (!value.isKnown())
+            continue;
         if (!pm.hasValue(value.tokvalue->exprId()))
             continue;
         ValueFlow::Value v2 = pm.at(value.tokvalue->exprId());

lib/valueflow.cpp

@@ -2111,7 +2111,10 @@ struct ValueFlowAnalyzer : Analyzer {
             // Check if its assigned to the same value
             if (value && !value->isImpossible() && Token::simpleMatch(tok->astParent(), "=") && astIsLHS(tok) &&
                 astIsIntegral(tok->astParent()->astOperand2(), false)) {
-                std::vector<MathLib::bigint> result = evaluate(Evaluate::Integral, tok->astParent()->astOperand2());
+                std::vector<MathLib::bigint> result =
+                    evaluateInt(tok->astParent()->astOperand2(), [&] {
+                    return ProgramMemory{getProgramState()};
+                });
                 if (!result.empty() && value->equalTo(result.front()))
                     return Action::Idempotent;
             }
@@ -2396,27 +2399,34 @@ struct ValueFlowAnalyzer : Analyzer {
         return Action::None;
     }
 
+    template<class F>
+    std::vector<MathLib::bigint> evaluateInt(const Token* tok, F getProgramMemory) const
+    {
+        if (tok->hasKnownIntValue())
+            return {static_cast<int>(tok->values().front().intvalue)};
+        std::vector<MathLib::bigint> result;
+        ProgramMemory pm = getProgramMemory();
+        if (Token::Match(tok, "&&|%oror%")) {
+            if (conditionIsTrue(tok, pm, getSettings()))
+                result.push_back(1);
+            if (conditionIsFalse(tok, pm, getSettings()))
+                result.push_back(0);
+        } else {
+            MathLib::bigint out = 0;
+            bool error = false;
+            execute(tok, &pm, &out, &error, getSettings());
+            if (!error)
+                result.push_back(out);
+        }
+        return result;
+    }
+
     virtual std::vector<MathLib::bigint> evaluate(Evaluate e, const Token* tok, const Token* ctx = nullptr) const override
     {
         if (e == Evaluate::Integral) {
-            if (tok->hasKnownIntValue())
+            return evaluateInt(tok, [&] {
-                return {static_cast<int>(tok->values().front().intvalue)};
+                return pms.get(tok, ctx, getProgramState());
-            std::vector<MathLib::bigint> result;
+            });
-            ProgramMemory pm = pms.get(tok, ctx, getProgramState());
-            if (Token::Match(tok, "&&|%oror%")) {
-                if (conditionIsTrue(tok, pm, getSettings()))
-                    result.push_back(1);
-                if (conditionIsFalse(tok, pm, getSettings()))
-                    result.push_back(0);
-            } else {
-                MathLib::bigint out = 0;
-                bool error = false;
-                execute(tok, &pm, &out, &error, getSettings());
-                if (!error)
-                    result.push_back(out);
-            }
-
-            return result;
         } else if (e == Evaluate::ContainerEmpty) {
             const ValueFlow::Value* value = ValueFlow::findValue(tok->values(), nullptr, [](const ValueFlow::Value& v) {
                 return v.isKnown() && v.isContainerSizeValue();
@@ -5387,7 +5397,7 @@ static bool isBreakScope(const Token* const endToken)
     return Token::findmatch(endToken->link(), "break|goto", endToken);
 }
 
-static ValueFlow::Value asImpossible(ValueFlow::Value v)
+ValueFlow::Value asImpossible(ValueFlow::Value v)
 {
     v.invertRange();
     v.setImpossible();

lib/valueflow.h

@@ -458,6 +458,8 @@ namespace ValueFlow {
     std::vector<ValueFlow::Value> isOutOfBounds(const Value& size, const Token* indexTok, bool possible = true);
 }
 
+ValueFlow::Value asImpossible(ValueFlow::Value v);
+
 bool isContainerSizeChanged(const Token* tok, const Settings* settings = nullptr, int depth = 20);
 
 struct LifetimeToken {

test/testcondition.cpp

@@ -4433,6 +4433,22 @@ private:
               "   if (pool) {}\n"
               "}\n");
         ASSERT_EQUALS("", errout.str());
+
+        // #10863
+        check("void f(const int A[], int Len) {\n"
+              "  if (Len <= 0)\n"
+              "    return;\n"
+              "  int I = 0;\n"
+              "  while (I < Len) {\n"
+              "    int K = I + 1;\n"
+              "    for (; K < Len; K++) {\n"
+              "      if (A[I] != A[K])\n"
+              "        break;\n"
+              "    } \n"
+              "    I = K;   \n"
+              "  }\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
     }
 
     void alwaysTrueTryCatch()

test/testvalueflow.cpp

@@ -6739,6 +6739,19 @@ private:
                "    bool result = x;\n"
                "}\n";
         ASSERT_EQUALS(false, testValueOfXKnown(code, 5U, 0));
+
+        code = "void foo() {\n"
+               "    int x = 0;\n"
+               "    for (int i = 0; i < 5; i++) {\n"
+               "        int y = 0;\n"
+               "        for (int j = 0; j < 10; j++)\n"
+               "            y++;\n"
+               "        if (y >= x)\n"
+               "            x = y;\n"
+               "    }\n"
+               "    return x;\n"
+               "}\n";
+        ASSERT_EQUALS(false, testValueOfXKnown(code, 10U, 0));
     }
 
     void valueFlowUnsigned() {