Fix issue 10102: False positive: knownConditionTrueFalse in for loop (#3038)

lib/valueflow.cpp

@@ -1306,93 +1306,6 @@ static void valueFlowSameExpressions(TokenList *tokenlist)
     }
 }
 
-static void valueFlowTerminatingCondition(TokenList *tokenlist, SymbolDatabase* symboldatabase, ErrorLogger *errorLogger, const Settings *settings)
-{
-    const bool cpp = symboldatabase->isCPP();
-    using Condition = std::pair<const Token*, const Scope*>;
-    for (const Scope * scope : symboldatabase->functionScopes) {
-        bool skipFunction = false;
-        std::vector<Condition> conds;
-        for (const Token* tok = scope->bodyStart; tok != scope->bodyEnd; tok = tok->next()) {
-            if (tok->isIncompleteVar()) {
-                if (settings->debugwarnings)
-                    bailoutIncompleteVar(tokenlist, errorLogger, tok, "Skipping function due to incomplete variable " + tok->str());
-                skipFunction = true;
-                break;
-            }
-            if (!Token::simpleMatch(tok, "if ("))
-                continue;
-            // Skip known values
-            if (tok->next()->hasKnownValue())
-                continue;
-            const Token * condTok = tok->next();
-            if (!Token::simpleMatch(condTok->link(), ") {"))
-                continue;
-            const Token * blockTok = condTok->link()->tokAt(1);
-            // Check if the block terminates early
-            if (!isEscapeScope(blockTok, tokenlist))
-                continue;
-            // Check if any variables are modified in scope
-            if (isExpressionChanged(condTok->astOperand2(), blockTok->link(), scope->bodyEnd, settings, cpp))
-                continue;
-            // TODO: Handle multiple conditions
-            if (Token::Match(condTok->astOperand2(), "%oror%|%or%|&|&&"))
-                continue;
-            const Scope * condScope = nullptr;
-            for (const Scope * parent = condTok->scope(); parent; parent = parent->nestedIn) {
-                if (parent->type == Scope::eIf ||
-                    parent->type == Scope::eWhile ||
-                    parent->type == Scope::eSwitch) {
-                    condScope = parent;
-                    break;
-                }
-            }
-            conds.emplace_back(condTok->astOperand2(), condScope);
-        }
-        if (skipFunction)
-            break;
-        for (Condition cond:conds) {
-            if (!cond.first)
-                continue;
-            Token *const startToken = cond.first->findExpressionStartEndTokens().second->next();
-            for (Token* tok = startToken; tok != scope->bodyEnd; tok = tok->next()) {
-                if (!Token::Match(tok, "%comp%"))
-                    continue;
-                // Skip known values
-                if (tok->hasKnownValue())
-                    continue;
-                if (cond.second) {
-                    bool bail = true;
-                    for (const Scope * parent = tok->scope()->nestedIn; parent; parent = parent->nestedIn) {
-                        if (parent == cond.second) {
-                            bail = false;
-                            break;
-                        }
-                    }
-                    if (bail)
-                        continue;
-                }
-                ErrorPath errorPath;
-                if (isOppositeCond(true, cpp, tok, cond.first, settings->library, true, true, &errorPath)) {
-                    ValueFlow::Value val(1);
-                    val.setKnown();
-                    val.condition = cond.first;
-                    val.errorPath = errorPath;
-                    val.errorPath.emplace_back(cond.first, "Assuming condition '" + cond.first->expressionString() + "' is false");
-                    setTokenValue(tok, val, tokenlist->getSettings());
-                } else if (isSameExpression(cpp, true, tok, cond.first, settings->library, true, true, &errorPath)) {
-                    ValueFlow::Value val(0);
-                    val.setKnown();
-                    val.condition = cond.first;
-                    val.errorPath = errorPath;
-                    val.errorPath.emplace_back(cond.first, "Assuming condition '" + cond.first->expressionString() + "' is false");
-                    setTokenValue(tok, val, tokenlist->getSettings());
-                }
-            }
-        }
-    }
-}
-
 static bool getExpressionRange(const Token *expr, MathLib::bigint *minvalue, MathLib::bigint *maxvalue)
 {
     if (expr->hasKnownIntValue()) {
@@ -2392,6 +2305,20 @@ struct ExpressionAnalyzer : SingleValueFlowAnalyzer {
     }
 };
 
+struct OppositeExpressionAnalyzer : ExpressionAnalyzer {
+    bool isNot;
+
+    OppositeExpressionAnalyzer() : ExpressionAnalyzer(), isNot(false) {}
+
+    OppositeExpressionAnalyzer(bool pIsNot, const Token* e, const ValueFlow::Value& val, const TokenList* t)
+        : ExpressionAnalyzer(e, val, t), isNot(pIsNot) 
+        {}
+
+    virtual bool match(const Token* tok) const OVERRIDE {
+        return isOppositeCond(isNot, isCPP(), expr, tok, getSettings()->library, true, true);
+    }
+};
+
 static Analyzer::Action valueFlowForwardExpression(Token* startToken,
         const Token* endToken,
         const Token* exprTok,
@@ -3807,6 +3734,58 @@ static void valueFlowAfterMove(TokenList *tokenlist, SymbolDatabase* symboldatab
     }
 }
 
+static const Token* findIncompleteVar(const Token* start, const Token* end)
+{
+    for (const Token* tok = start; tok != end; tok = tok->next()) {
+        if (tok->isIncompleteVar())
+            return tok;
+    }
+    return nullptr;
+}
+
+static void valueFlowTerminatingCondition(TokenList *tokenlist, SymbolDatabase* symboldatabase, ErrorLogger *errorLogger, const Settings *settings)
+{
+    for (const Scope * scope : symboldatabase->functionScopes) {
+        if (const Token* incompleteTok = findIncompleteVar(scope->bodyStart, scope->bodyEnd)) {
+            if (incompleteTok->isIncompleteVar()) {
+                if (settings->debugwarnings)
+                    bailoutIncompleteVar(tokenlist, errorLogger, incompleteTok, "Skipping function due to incomplete variable " + incompleteTok->str());
+                break;
+            }
+        }
+
+        for (const Token* tok = scope->bodyStart; tok != scope->bodyEnd; tok = tok->next()) {
+            if (!Token::simpleMatch(tok, "if ("))
+                continue;
+            // Skip known values
+            if (tok->next()->hasKnownValue())
+                continue;
+            const Token * parenTok = tok->next();
+            if (!Token::simpleMatch(parenTok->link(), ") {"))
+                continue;
+            const Token * blockTok = parenTok->link()->tokAt(1);
+            // Check if the block terminates early
+            if (!isEscapeScope(blockTok, tokenlist))
+                continue;
+
+            const Token* condTok = parenTok->astOperand2();
+            ValueFlow::Value v1(0);
+            v1.setKnown();
+            v1.condition = condTok;
+            v1.errorPath.emplace_back(condTok, "Assuming condition '" + condTok->expressionString() + "' is true");
+            ExpressionAnalyzer a1(condTok, v1, tokenlist);
+            valueFlowGenericForward(blockTok->link()->next(), scope->bodyEnd, a1, settings);
+
+            ValueFlow::Value v2(1);
+            v2.setKnown();
+            v2.condition = condTok;
+            v2.errorPath.emplace_back(condTok, "Assuming condition '" + condTok->expressionString() + "' is false");
+            OppositeExpressionAnalyzer a2(true, condTok, v2, tokenlist);
+            valueFlowGenericForward(blockTok->link()->next(), scope->bodyEnd, a2, settings);
+        }
+    }
+}
+
 static void valueFlowForwardAssign(Token * const               tok,
                                    const Variable * const      var,
                                    std::list<ValueFlow::Value> values,

test/testvalueflow.cpp

@@ -4188,7 +4188,8 @@ private:
                "    }\n"
                "    if (i != j) {}\n"
                "}\n";
-        ASSERT_EQUALS(false, valueOfTok(code, "!=").intvalue == 1);
+        ASSERT_EQUALS(true, valueOfTok(code, "!=").intvalue == 1);
+        ASSERT_EQUALS(false, valueOfTok(code, "!=").isKnown());
 
         code = "void f(int i, int j, bool a) {\n"
                "    if (i != j) {}\n"
@@ -4243,6 +4244,20 @@ private:
                "    if ( this->FileIndex < 0 ) {}\n"
                "}";
         ASSERT_EQUALS(false, valueOfTok(code, "<").intvalue == 1);
+
+        code = "int f(int p) {\n"
+               "    int v = 0;\n"
+               "    for (int i = 0; i < 1; ++i) {\n"
+               "        if (p == 0)\n"
+               "            v = 1;\n"
+               "        if (v == 1)\n"
+               "            break;\n"
+               "    }\n"
+               "    int x = v;\n"
+               "    return x;\n"
+               "}\n";
+        ASSERT_EQUALS(false, testValueOfXKnown(code, 10U, 0));
+        ASSERT_EQUALS(false, testValueOfXKnown(code, 10U, 1));
     }
 
     static std::string isPossibleContainerSizeValue(const std::list<ValueFlow::Value> &values, MathLib::bigint i) {