From 01a8890d6d702e8a927fef50679203c9ee82bf75 Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Sun, 2 Jan 2022 01:15:38 -0600 Subject: [PATCH] Fix 9760: False positive: constParameter on parameter used to take non-const pointer via array decaying (#3660) --- lib/astutils.cpp | 46 ++++++++++++++++++++++++++++++---------------- test/testother.cpp | 4 ++++ 2 files changed, 34 insertions(+), 16 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index fe3040ef9..04d032ae1 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -2326,28 +2326,40 @@ static bool isExpressionChangedAt(const F& getExprTok, bool cpp, int depth) { + if (depth < 0) + return true; if (tok->exprId() != exprid) { if (globalvar && Token::Match(tok, "%name% (")) // TODO: Is global variable really changed by function call? return true; - // Is aliased function call or alias passed to function - if ((Token::Match(tok, "%var% (") || isVariableChangedByFunctionCall(tok, 1, settings)) && - std::any_of(tok->values().begin(), tok->values().end(), std::mem_fn(&ValueFlow::Value::isLifetimeValue))) { - bool aliased = false; - // If we can't find the expression then assume it was modified - if (!getExprTok()) - return true; - visitAstNodes(getExprTok(), [&](const Token* childTok) { - if (childTok->varId() > 0 && isAliasOf(tok, childTok->varId())) { - aliased = true; - return ChildrenToVisit::done; + const bool pointer = astIsPointer(tok); + bool aliased = false; + // If we can't find the expression then assume it is an alias + if (!getExprTok()) + aliased = true; + if (!aliased) { + aliased = findAstNode(getExprTok(), [&](const Token* childTok) { + for (const ValueFlow::Value& val : tok->values()) { + if (val.isImpossible()) + continue; + if (val.isLocalLifetimeValue() || (pointer && val.isSymbolicValue() && val.intvalue == 0)) { + if (findAstNode(val.tokvalue, + [&](const Token* aliasTok) { + return aliasTok->exprId() == childTok->exprId(); + })) + return true; + } } - return ChildrenToVisit::op1_and_op2; + return false; }); - // TODO: Try to traverse the lambda function - if (aliased) - return true; } + if (!aliased) + return false; + if (isVariableChanged(tok, 1, settings, cpp, depth)) + return true; + // TODO: Try to traverse the lambda function + if (Token::Match(tok, "%var% (")) + return true; return false; } return (isVariableChanged(tok, indirect, settings, cpp, depth)); @@ -2398,7 +2410,7 @@ bool isVariableChanged(const Variable * var, const Settings *settings, bool cpp, return false; if (Token::Match(start, "; %varid% =", var->declarationId())) start = start->tokAt(2); - return isVariableChanged(start->next(), var->scope()->bodyEnd, var->declarationId(), var->isGlobal(), settings, cpp, depth); + return isExpressionChanged(var->nameToken(), start->next(), var->scope()->bodyEnd, settings, cpp, depth); } bool isVariablesChanged(const Token* start, @@ -2458,6 +2470,8 @@ bool isThisChanged(const Token* start, const Token* end, int indirect, const Set bool isExpressionChanged(const Token* expr, const Token* start, const Token* end, const Settings* settings, bool cpp, int depth) { + if (depth < 0) + return true; if (!precedes(start, end)) return false; const Token* result = findAstNode(expr, [&](const Token* tok) { diff --git a/test/testother.cpp b/test/testother.cpp index 0b4f13fdc..8806c8211 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -2623,6 +2623,10 @@ private: check("struct S { void f(); int i; };\n" "void call_f(S& s) { (s.*(&S::f))(); }\n"); ASSERT_EQUALS("", errout.str()); + + check("struct S { int a[1]; };\n" + "void f(S& s) { int* p = s.a; *p = 0; }\n"); + ASSERT_EQUALS("", errout.str()); } void constParameterCallback() {