diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 39ea80259..031a6f124 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -344,6 +344,22 @@ const Token* getParentMember(const Token * tok) return tok; } +const Token* getParentLifetime(const Token* tok) +{ + if (!tok) + return tok; + const Variable* var = tok->variable(); + // TODO: Call getLifetimeVariable for deeper analysis + if (!var) + return tok; + if (var->isLocal() || var->isArgument()) + return tok; + const Token* parent = getParentMember(tok); + if (parent != tok) + return getParentLifetime(parent); + return tok; +} + bool astIsLHS(const Token* tok) { if (!tok) @@ -448,8 +464,6 @@ bool isAliasOf(const Token *tok, nonneg int varid) { if (tok->varId() == varid) return false; - if (tok->varId() == 0) - return false; for (const ValueFlow::Value &val : tok->values()) { if (!val.isLocalLifetimeValue()) continue; diff --git a/lib/astutils.h b/lib/astutils.h index a2af32a43..eb83e2bc9 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -97,6 +97,8 @@ const Token* astParentSkipParens(const Token* tok); const Token* getParentMember(const Token * tok); +const Token* getParentLifetime(const Token* tok); + bool astIsLHS(const Token* tok); bool astIsRHS(const Token* tok); diff --git a/lib/checkautovariables.cpp b/lib/checkautovariables.cpp index 88521f0e1..c7d5508f6 100644 --- a/lib/checkautovariables.cpp +++ b/lib/checkautovariables.cpp @@ -432,22 +432,6 @@ static bool isDeadScope(const Token * tok, const Scope * scope) return false; } -static const Token * getParentLifetime(const Token *tok) -{ - if (!tok) - return tok; - const Variable * var = tok->variable(); - // TODO: Call getLifetimeVariable for deeper analysis - if (!var) - return tok; - if (var->isLocal()) - return tok; - const Token * parent = getParentMember(tok); - if (parent != tok) - return getParentLifetime(parent); - return tok; -} - static int getPointerDepth(const Token *tok) { if (!tok) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 9d5250e15..82a078ce2 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -31,6 +31,7 @@ #include "token.h" #include "tokenize.h" #include "utils.h" +#include "valueflow.h" #include // find_if() #include @@ -1363,9 +1364,20 @@ void CheckOther::checkConstVariable() continue; if (isVariableChanged(var, mSettings, mTokenizer->isCPP())) continue; - if (Function::returnsReference(function) && - Token::findmatch(var->nameToken(), "return %varid% ;|[|.", scope->bodyEnd, var->declarationId())) - continue; + if (Function::returnsReference(function)) { + std::vector returns = Function::findReturns(function); + if (std::any_of(returns.begin(), returns.end(), [&](const Token* retTok) { + if (retTok->varId() == var->declarationId()) + return true; + while (Token::simpleMatch(retTok, ".")) + retTok = retTok->astOperand2(); + const Variable* retVar = getLifetimeVariable(getParentLifetime(retTok)); + if (!retVar) + return false; + return retVar->declarationId() == var->declarationId(); + })) + continue; + } // Skip if address is taken if (Token::findmatch(var->nameToken(), "& %varid%", scope->bodyEnd, var->declarationId())) continue; diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 6164cd39b..64a7acb02 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -2439,6 +2439,32 @@ bool Function::returnsReference(const Function* function, bool unknown) return false; } +std::vector Function::findReturns(const Function* f) +{ + std::vector result; + if (!f) + return result; + const Scope* scope = f->functionScope; + if (!scope) + return result; + for (const Token* tok = scope->bodyStart->next(); tok && tok != scope->bodyEnd; tok = tok->next()) { + if (tok->str() == "{" && tok->scope() && + (tok->scope()->type == Scope::eLambda || tok->scope()->type == Scope::eClass)) { + tok = tok->link(); + continue; + } + if (Token::simpleMatch(tok->astParent(), "return")) { + result.push_back(tok); + } + // Skip lambda functions since the scope may not be set correctly + const Token* lambdaEndToken = findLambdaEndToken(tok); + if (lambdaEndToken) { + tok = lambdaEndToken; + } + } + return result; +} + const Token * Function::constructorMemberInitialization() const { if (!isConstructor() || !functionScope || !functionScope->bodyStart) diff --git a/lib/symboldatabase.h b/lib/symboldatabase.h index f4a5162dd..0d4ba92ff 100644 --- a/lib/symboldatabase.h +++ b/lib/symboldatabase.h @@ -896,6 +896,8 @@ public: static bool returnsReference(const Function* function, bool unknown = false); + static std::vector findReturns(const Function* f); + const Token* returnDefEnd() const { if (this->hasTrailingReturnType()) { return Token::findmatch(retDef, "{|;"); diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 53d070705..3c756829d 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2718,30 +2718,6 @@ static void valueFlowForward(Token* startToken, } } -static std::vector findReturns(const Function* f) -{ - std::vector result; - const Scope* scope = f->functionScope; - if (!scope) - return result; - for (const Token* tok = scope->bodyStart->next(); tok && tok != scope->bodyEnd; tok = tok->next()) { - if (tok->str() == "{" && tok->scope() && - (tok->scope()->type == Scope::eLambda || tok->scope()->type == Scope::eClass)) { - tok = tok->link(); - continue; - } - if (Token::simpleMatch(tok->astParent(), "return")) { - result.push_back(tok); - } - // Skip lambda functions since the scope may not be set correctly - const Token* lambdaEndToken = findLambdaEndToken(tok); - if (lambdaEndToken) { - tok = lambdaEndToken; - } - } - return result; -} - static int getArgumentPos(const Variable *var, const Function *f) { auto arg_it = std::find_if(f->argumentList.begin(), f->argumentList.end(), [&](const Variable &v) { @@ -2850,6 +2826,16 @@ std::vector getLifetimeTokens(const Token* tok, ValueFlow::Value: return {{tok, true, std::move(errorPath)}}; if (vartok) return getLifetimeTokens(vartok, std::move(errorPath), depth - 1); + } else if (Token::simpleMatch(var->nameToken()->astParent(), ":") && + var->nameToken()->astParent()->astParent() && + Token::simpleMatch(var->nameToken()->astParent()->astParent()->previous(), "for (")) { + errorPath.emplace_back(var->nameToken(), "Assigned to reference."); + const Token* vartok = var->nameToken(); + if (vartok == tok) + return {{tok, true, std::move(errorPath)}}; + const Token* contok = var->nameToken()->astParent()->astOperand2(); + if (contok) + return getLifetimeTokens(contok, std::move(errorPath), depth - 1); } else { return std::vector {}; } @@ -2860,7 +2846,7 @@ std::vector getLifetimeTokens(const Token* tok, ValueFlow::Value: if (!Function::returnsReference(f)) return {{tok, std::move(errorPath)}}; std::vector result; - std::vector returns = findReturns(f); + std::vector returns = Function::findReturns(f); for (const Token* returnTok : returns) { if (returnTok == tok) continue; @@ -2947,6 +2933,12 @@ const Variable* getLifetimeVariable(const Token* tok, ValueFlow::Value::ErrorPat return nullptr; } +const Variable* getLifetimeVariable(const Token* tok) +{ + ValueFlow::Value::ErrorPath errorPath; + return getLifetimeVariable(tok, errorPath, nullptr); +} + static bool isNotLifetimeValue(const ValueFlow::Value& val) { return !val.isLifetimeValue(); @@ -3394,7 +3386,7 @@ static void valueFlowLifetimeFunction(Token *tok, TokenList *tokenlist, ErrorLog const Function *f = tok->function(); if (Function::returnsReference(f)) return; - std::vector returns = findReturns(f); + std::vector returns = Function::findReturns(f); const bool inconclusive = returns.size() > 1; for (const Token* returnTok : returns) { if (returnTok == tok) diff --git a/lib/valueflow.h b/lib/valueflow.h index b20d26e3a..ba8c60cfc 100644 --- a/lib/valueflow.h +++ b/lib/valueflow.h @@ -359,6 +359,8 @@ std::vector getLifetimeTokens(const Token* tok, ValueFlow::Value: const Variable* getLifetimeVariable(const Token* tok, ValueFlow::Value::ErrorPath& errorPath, bool* addressOf = nullptr); +const Variable* getLifetimeVariable(const Token* tok); + bool isLifetimeBorrowed(const Token *tok, const Settings *settings); std::string lifetimeType(const Token *tok, const ValueFlow::Value *val); diff --git a/test/testother.cpp b/test/testother.cpp index fd9012281..654cf6734 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -1959,6 +1959,24 @@ private: "}\n"); ASSERT_EQUALS("", errout.str()); + check("int* f(std::list& x, unsigned int y) {\n" + " for (int& m : x) {\n" + " if (m == y)\n" + " return &m;\n" + " }\n" + " return nullptr;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("int& f(std::list& x, int& y) {\n" + " for (int& m : x) {\n" + " if (m == y)\n" + " return m;\n" + " }\n" + " return y;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + check("void e();\n" "void g(void);\n" "void h(void);\n"