Fix issue 10004: ValueFlow: pointer value, wrongly set known value (#2931)

lib/astutils.cpp

@@ -67,6 +67,19 @@ void visitAstNodes(Token *ast, std::function<ChildrenToVisit(Token *)> visitor)
     visitAstNodesGeneric(ast, std::move(visitor));
 }
 
+const Token* findAstNode(const Token* ast, const std::function<bool(const Token*)>& pred)
+{
+    const Token* result = nullptr;
+    visitAstNodes(ast, [&](const Token* tok) {
+        if (pred(tok)) {
+            result = tok;
+            return ChildrenToVisit::done;
+        }
+        return ChildrenToVisit::op1_and_op2;
+    });
+    return result;
+}
+
 static void astFlattenRecursive(const Token *tok, std::vector<const Token *> *result, const char* op, nonneg int depth = 0)
 {
     ++depth;
@@ -1777,6 +1790,27 @@ bool isThisChanged(const Token* start, const Token* end, int indirect, const Set
     return false;
 }
 
+bool isExpressionChanged(const Token* expr, const Token* start, const Token* end, const Settings* settings, bool cpp, int depth)
+{
+    const Token* result = findAstNode(expr, [&](const Token* tok) {
+        if (exprDependsOnThis(tok) && isThisChanged(start, end, false, settings, cpp)) {
+            return true;
+        }
+        bool global = false;
+        if (tok->variable()) {
+            if (tok->variable()->isConst())
+                return false;
+            global = !tok->variable()->isLocal() && !tok->variable()->isArgument();
+        }
+        if (tok->exprId() > 0 &&
+            isVariableChanged(
+                start, end, tok->valueType() ? tok->valueType()->pointer : 0, tok->exprId(), global, settings, cpp, depth))
+            return true;
+        return false;
+    });
+    return result;
+}
+
 int numberOfArguments(const Token *start)
 {
     int arguments=0;

lib/astutils.h

@@ -49,6 +49,8 @@ enum class ChildrenToVisit {
 void visitAstNodes(const Token *ast, std::function<ChildrenToVisit(const Token *)> visitor);
 void visitAstNodes(Token *ast, std::function<ChildrenToVisit(Token *)> visitor);
 
+const Token* findAstNode(const Token* ast, const std::function<bool(const Token*)>& pred);
+
 std::vector<const Token*> astFlatten(const Token* tok, const char* op);
 
 bool astHasToken(const Token* root, const Token * tok);
@@ -204,6 +206,13 @@ bool isThisChanged(const Token* start, const Token* end, int indirect, const Set
 const Token* findVariableChanged(const Token *start, const Token *end, int indirect, const nonneg int exprid, bool globalvar, const Settings *settings, bool cpp, int depth = 20);
 Token* findVariableChanged(Token *start, const Token *end, int indirect, const nonneg int exprid, bool globalvar, const Settings *settings, bool cpp, int depth = 20);
 
+bool isExpressionChanged(const Token* expr,
+                         const Token* start,
+                         const Token* end,
+                         const Settings* settings,
+                         bool cpp,
+                         int depth = 20);
+
 /// If token is an alias if another variable
 bool isAliasOf(const Token *tok, nonneg int varid, bool* inconclusive = nullptr);
 

lib/checkcondition.cpp

@@ -460,23 +460,8 @@ void CheckCondition::duplicateCondition()
         if (!cond2)
             continue;
 
-        bool modified = false;
-        visitAstNodes(cond1, [&](const Token* tok3) {
-            if (exprDependsOnThis(tok3)) {
-                if (isThisChanged(scope.classDef->next(), cond2, false, mSettings, mTokenizer->isCPP())) {
-                    modified = true;
-                    return ChildrenToVisit::done;
-                }
-            }
-            if (tok3->varId() > 0 &&
-                isVariableChanged(scope.classDef->next(), cond2, tok3->valueType() ? tok3->valueType()->pointer : 0, tok3->varId(), false, mSettings, mTokenizer->isCPP())) {
-                modified = true;
-                return ChildrenToVisit::done;
-            }
-            return ChildrenToVisit::op1_and_op2;
-        });
         ErrorPath errorPath;
-        if (!modified &&
+        if (!isExpressionChanged(cond1, scope.classDef->next(), cond2, mSettings, mTokenizer->isCPP()) &&
             isSameExpression(mTokenizer->isCPP(), true, cond1, cond2, mSettings->library, true, true, &errorPath))
             duplicateConditionError(cond1, cond2, errorPath);
     }

lib/valueflow.cpp

@@ -1353,28 +1353,7 @@ static void valueFlowTerminatingCondition(TokenList *tokenlist, SymbolDatabase*
             if (!isEscapeScope(blockTok, tokenlist))
                 continue;
             // Check if any variables are modified in scope
-            bool bail = false;
-            for (const Token * tok2=condTok->next(); tok2 != condTok->link(); tok2 = tok2->next()) {
-                const Variable * var = tok2->variable();
-                if (!var)
-                    continue;
-                if (!var->scope())
-                    continue;
-                const Token * endToken = var->scope()->bodyEnd;
-                if (!var->isLocal() && !var->isConst() && !var->isArgument()) {
-                    bail = true;
-                    break;
-                }
-                if (var->isStatic() && !var->isConst()) {
-                    bail = true;
-                    break;
-                }
-                if (!var->isConst() && var->declEndToken() && isVariableChanged(var->declEndToken()->next(), endToken, tok2->varId(), false, settings, cpp)) {
-                    bail = true;
-                    break;
-                }
-            }
-            if (bail)
+            if (isExpressionChanged(condTok->astOperand2(), blockTok->link(), scope->bodyEnd, settings, cpp))
                 continue;
             // TODO: Handle multiple conditions
             if (Token::Match(condTok->astOperand2(), "%oror%|%or%|&|&&"))