From 904d52acac3aa887f7137b09743b9a659fba2811 Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Thu, 3 Dec 2020 00:15:31 -0600 Subject: [PATCH] Fix issue 10004: ValueFlow: pointer value, wrongly set known value (#2931) --- lib/astutils.cpp | 34 ++++++++++++++++++++++++++++++++++ lib/astutils.h | 9 +++++++++ lib/checkcondition.cpp | 17 +---------------- lib/valueflow.cpp | 23 +---------------------- 4 files changed, 45 insertions(+), 38 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index a8cf1b09f..2e8ac921c 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -67,6 +67,19 @@ void visitAstNodes(Token *ast, std::function visitor) visitAstNodesGeneric(ast, std::move(visitor)); } +const Token* findAstNode(const Token* ast, const std::function& 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 *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; diff --git a/lib/astutils.h b/lib/astutils.h index ce15d33c3..a81d02613 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -49,6 +49,8 @@ enum class ChildrenToVisit { void visitAstNodes(const Token *ast, std::function visitor); void visitAstNodes(Token *ast, std::function visitor); +const Token* findAstNode(const Token* ast, const std::function& pred); + std::vector 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); diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index 145789a23..a5e24ca6e 100644 --- a/lib/checkcondition.cpp +++ b/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); } diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index a61a6411b..cc2a58485 100644 --- a/lib/valueflow.cpp +++ b/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%|&|&&"))