diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 30dbfc824..5778dc215 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -955,16 +955,9 @@ static bool isScopeBracket(const Token *tok) return false; } -bool isVariableChangedByFunctionCall(const Token *tok, int indirect, const Settings *settings, bool *inconclusive) +const Token * getTokenArgumentFunction(const Token * tok, int& argn) { - if (!tok) - return false; - - const Token * const tok1 = tok; - - // address of variable - const bool addressOf = tok->astParent() && tok->astParent()->isUnaryOp("&"); - + argn = -1; { const Token *parent = tok->astParent(); if (parent && parent->isUnaryOp("&")) @@ -981,16 +974,16 @@ bool isVariableChangedByFunctionCall(const Token *tok, int indirect, const Setti while (Token::simpleMatch(parent, ",")) parent = parent->astParent(); if (!parent || parent->str() != "(") - return false; + return nullptr; } else - return false; + return nullptr; } - // goto start of function call and get argnr - int argnr = 0; + // goto start of function call and get argn + argn = 0; while (tok && !Token::simpleMatch(tok, ";") && !isScopeBracket(tok)) { if (tok->str() == ",") - ++argnr; + ++argn; else if (Token::Match(tok, ")|}")) tok = tok->link(); else if (Token::Match(tok->previous(), "%name% (|{")) @@ -1000,13 +993,33 @@ bool isVariableChangedByFunctionCall(const Token *tok, int indirect, const Setti tok = tok->previous(); } if (!Token::Match(tok, "{|(")) - return false; - const bool possiblyPassedByReference = (tok->next() == tok1 || Token::Match(tok1->previous(), ", %name% [,)}]")); + return nullptr; tok = tok->previous(); if (tok && tok->link() && tok->str() == ">") tok = tok->link()->previous(); if (!Token::Match(tok, "%name% [({<]")) + return nullptr; + return tok; +} + +bool isVariableChangedByFunctionCall(const Token *tok, int indirect, const Settings *settings, bool *inconclusive) +{ + if (!tok) + return false; + + const Token * const tok1 = tok; + + // address of variable + const bool addressOf = tok->astParent() && tok->astParent()->isUnaryOp("&"); + + int argnr; + tok = getTokenArgumentFunction(tok, argnr); + if (!tok) return false; // not a function => variable not changed + const Token * parenTok = tok->next(); + if (Token::simpleMatch(parenTok, "<") && parenTok->link()) + parenTok = parenTok->link()->next(); + const bool possiblyPassedByReference = (parenTok->next() == tok1 || Token::Match(tok1->previous(), ", %name% [,)}]")); // Constructor call if (tok->variable() && tok->variable()->nameToken() == tok) { diff --git a/lib/astutils.h b/lib/astutils.h index 5e40dd475..89f10afe6 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -120,6 +120,9 @@ bool isUniqueExpression(const Token* tok); /** Is scope a return scope (scope will unconditionally return) */ bool isReturnScope(const Token *endToken, const Settings * settings = nullptr, bool functionScope=false); +/// Return the token to the function and the argument number +const Token * getTokenArgumentFunction(const Token * tok, int& argn); + /** Is variable changed by function call? * In case the answer of the question is inconclusive, e.g. because the function declaration is not known * the return value is false and the output parameter inconclusive is set to true diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 89ae044fa..7b3a3b92a 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -5297,23 +5297,48 @@ static bool isContainerEmpty(const Token* tok) return false; } -static bool isContainerSizeChanged(nonneg int varId, const Token *start, const Token *end); +static bool isContainerSizeChanged(nonneg int varId, const Token *start, const Token *end, int depth = 20); -static bool isContainerSizeChangedByFunction(const Token *tok) +static bool isContainerSizeChangedByFunction(const Token *tok, int depth = 20) { - const Token *parent = tok->astParent(); - if (parent && parent->str() == "&") - parent = parent->astParent(); - while (parent && parent->str() == ",") - parent = parent->astParent(); - if (!parent) + if (!tok->valueType() || !tok->valueType()->container) return false; - if (Token::Match(parent->previous(), "%name% (")) - return true; - // some unsimplified template function, assume it modifies the container. - if (Token::simpleMatch(parent->previous(), ">") && parent->linkAt(-1)) - return true; - return false; + // If we are accessing an element then we are not changing the container size + if (Token::Match(tok, "%name% . %name% (")) { + Library::Container::Yield yield = tok->valueType()->container->getYield(tok->strAt(2)); + if (yield != Library::Container::Yield::NO_YIELD) + return false; + } + if (Token::simpleMatch(tok->astParent(), "[")) + return false; + + // address of variable + const bool addressOf = tok->astParent() && tok->astParent()->isUnaryOp("&"); + + int narg; + const Token * ftok = getTokenArgumentFunction(tok, narg); + if (!ftok) + return false; // not a function => variable not changed + const Function * fun = ftok->function(); + if (fun) { + const Variable *arg = fun->getArgumentVar(narg); + if (!arg->isReference() && !addressOf) + return false; + if (arg->isConst()) + return false; + const Scope * scope = fun->functionScope; + if (scope) { + // Argument not used + if (!arg->nameToken()) + return false; + if (depth > 0) + return isContainerSizeChanged(arg->declarationId(), scope->bodyStart, scope->bodyEnd, depth - 1); + } + } + + bool inconclusive = false; + const bool isChanged = isVariableChangedByFunctionCall(tok, 0, nullptr, &inconclusive); + return (isChanged || inconclusive); } static void valueFlowContainerReverse(Token *tok, nonneg int containerId, const ValueFlow::Value &value, const Settings *settings) @@ -5398,7 +5423,7 @@ static void valueFlowContainerForward(Token *tok, nonneg int containerId, ValueF } } -static bool isContainerSizeChanged(nonneg int varId, const Token *start, const Token *end) +static bool isContainerSizeChanged(nonneg int varId, const Token *start, const Token *end, int depth) { for (const Token *tok = start; tok != end; tok = tok->next()) { if (tok->varId() != varId) @@ -5426,7 +5451,7 @@ static bool isContainerSizeChanged(nonneg int varId, const Token *start, const T break; }; } - if (isContainerSizeChangedByFunction(tok)) + if (isContainerSizeChangedByFunction(tok, depth)) return true; } return false; diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 55f5c9d3a..87c0955af 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -3937,6 +3937,113 @@ private: "}"; ASSERT(tokenValues(code, "x . front").empty()); + code = "void g(std::list&);\n" + "void f() {\n" + " std::list x;\n" + " g(x);\n" + " x.front();\n" + "}"; + ASSERT(tokenValues(code, "x . front").empty()); + + code = "void g(std::list*);\n" + "void f() {\n" + " std::list x;\n" + " g(&x);\n" + " x.front();\n" + "}"; + ASSERT(tokenValues(code, "x . front").empty()); + + code = "void g(const std::list&);\n" + "void f() {\n" + " std::list x;\n" + " g(x);\n" + " x.front();\n" + "}"; + ASSERT_EQUALS("", isKnownContainerSizeValue(tokenValues(code, "x . front"), 0)); + + code = "void g(std::list);\n" + "void f() {\n" + " std::list x;\n" + " g(x);\n" + " x.front();\n" + "}"; + ASSERT_EQUALS("", isKnownContainerSizeValue(tokenValues(code, "x . front"), 0)); + + code = "void g(int&);\n" + "void f() {\n" + " std::list x;\n" + " g(x[0]);\n" + " x.front();\n" + "}"; + ASSERT_EQUALS("", isKnownContainerSizeValue(tokenValues(code, "x . front"), 0)); + + code = "void g(int&);\n" + "void f() {\n" + " std::list x;\n" + " g(x.back());\n" + " x.front();\n" + "}"; + ASSERT_EQUALS("", isKnownContainerSizeValue(tokenValues(code, "x . front"), 0)); + + code = "void g(std::list&) {}\n" + "void f() {\n" + " std::list x;\n" + " g(x);\n" + " x.front();\n" + "}"; + ASSERT_EQUALS("", isKnownContainerSizeValue(tokenValues(code, "x . front"), 0)); + + code = "void g(std::list& y) { y.push_back(1); }\n" + "void f() {\n" + " std::list x;\n" + " g(x);\n" + " x.front();\n" + "}"; + ASSERT(tokenValues(code, "x . front").empty()); + + code = "void g(std::list*) {}\n" + "void f() {\n" + " std::list x;\n" + " g(&x);\n" + " x.front();\n" + "}"; + ASSERT_EQUALS("", isKnownContainerSizeValue(tokenValues(code, "x . front"), 0)); + + code = "void g(std::list* y) { y->push_back(1); }\n" + "void f() {\n" + " std::list x;\n" + " g(&x);\n" + " x.front();\n" + "}"; + ASSERT(tokenValues(code, "x . front").empty()); + + code = "void h(std::list&);\n" + "void g(std::list& y) { h(y); }\n" + "void f() {\n" + " std::list x;\n" + " g(x);\n" + " x.front();\n" + "}"; + ASSERT(tokenValues(code, "x . front").empty()); + + code = "void h(const std::list&);\n" + "void g(std::list& y) { h(y); }\n" + "void f() {\n" + " std::list x;\n" + " g(x);\n" + " x.front();\n" + "}"; + ASSERT_EQUALS("", isKnownContainerSizeValue(tokenValues(code, "x . front"), 0)); + + code = "void h(const std::list&);\n" + "void g(std::list& y) { h(y); y.push_back(1); }\n" + "void f() {\n" + " std::list x;\n" + " g(x);\n" + " x.front();\n" + "}"; + ASSERT(tokenValues(code, "x . front").empty()); + code = "void f(std::vector ints) {\n" // #8697 " if (ints.empty())\n" " abort() << 123;\n"