diff --git a/lib/checkautovariables.cpp b/lib/checkautovariables.cpp index c99d94232..e116be9e6 100644 --- a/lib/checkautovariables.cpp +++ b/lib/checkautovariables.cpp @@ -606,7 +606,8 @@ void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token if (returnRef && Token::simpleMatch(tok->astParent(), "return")) { ErrorPath errorPath; const Variable *var = getLifetimeVariable(tok, errorPath); - if (var && var->isLocal() && !var->isArgument() && isInScope(var->nameToken(), tok->scope())) { + if (var && !var->isGlobal() && !var->isStatic() && !var->isReference() && !var->isRValueReference() && + isInScope(var->nameToken(), tok->scope())) { errorReturnReference(tok, errorPath); continue; } diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index ac0220e15..0361dc197 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2603,21 +2603,75 @@ static bool valueFlowForward(Token * const startToken, return true; } +static const Token *findSimpleReturn(const Function *f) +{ + const Scope *scope = f->functionScope; + if (!scope) + return nullptr; + const Token *returnTok = nullptr; + for (const Token *tok = scope->bodyStart; 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")) + continue; + // Multiple returns + if (returnTok) + return nullptr; + returnTok = tok; + } + return returnTok; +} + const Variable *getLifetimeVariable(const Token *tok, ValueFlow::Value::ErrorPath &errorPath) { if (!tok) return nullptr; const Variable *var = tok->variable(); - if (!var) - return nullptr; - if (var->isReference() || var->isRValueReference()) { - if (!var->declEndToken()) + if (var && var->declarationId() == tok->varId()) { + if (var->isReference() || var->isRValueReference()) { + if (!var->declEndToken()) + return nullptr; + if (var->isArgument()) { + errorPath.emplace_back(var->declEndToken(), "Passed to reference."); + return var; + } else if (Token::simpleMatch(var->declEndToken(), "=")) { + errorPath.emplace_back(var->declEndToken(), "Assigned to reference."); + const Token *vartok = var->declEndToken()->astOperand2(); + if (vartok == tok) + return nullptr; + if (vartok) + return getLifetimeVariable(vartok, errorPath); + } else { + return nullptr; + } + } + } else if (Token::Match(tok->previous(), "%name% (")) { + const Function *f = tok->previous()->function(); + if (!f) return nullptr; - errorPath.emplace_back(var->declEndToken(), "Assigned to reference."); - const Token *vartok = var->declEndToken()->astOperand2(); - if (vartok == tok) + if (!Function::returnsReference(f)) return nullptr; - return getLifetimeVariable(vartok, errorPath); + const Token *returnTok = findSimpleReturn(f); + if (!returnTok) + return nullptr; + const Variable *argvar = getLifetimeVariable(returnTok, errorPath); + if (!argvar) + return nullptr; + if (argvar->isArgument() && (argvar->isReference() || argvar->isRValueReference())) { + auto arg_it = std::find_if(f->argumentList.begin(), f->argumentList.end(), [&](const Variable &v) { + return v.nameToken() == argvar->nameToken(); + }); + if (arg_it == f->argumentList.end()) + return nullptr; + std::size_t n = std::distance(f->argumentList.begin(), arg_it); + const Token *argTok = getArguments(tok->previous()).at(n); + errorPath.emplace_back(returnTok, "Return reference."); + errorPath.emplace_back(tok->previous(), "Called function passing '" + argTok->str() + "'."); + return getLifetimeVariable(argTok, errorPath); + } } return var; } diff --git a/test/testautovariables.cpp b/test/testautovariables.cpp index c00bff926..48b0f3b73 100644 --- a/test/testautovariables.cpp +++ b/test/testautovariables.cpp @@ -107,6 +107,7 @@ private: TEST_CASE(returnReference5); TEST_CASE(returnReference6); TEST_CASE(returnReference7); + TEST_CASE(returnReferenceFunction); TEST_CASE(returnReferenceLiteral); TEST_CASE(returnReferenceCalculation); TEST_CASE(returnReferenceLambda); @@ -1094,6 +1095,53 @@ private: ASSERT_EQUALS("", errout.str()); } + void returnReferenceFunction() { + check("int& f(int& a) {\n" + " return a;\n" + "}\n" + "int& hello() {\n" + " int x = 0;\n" + " return f(x);\n" + "}\n"); + ASSERT_EQUALS( + "[test.cpp:1] -> [test.cpp:2] -> [test.cpp:6] -> [test.cpp:6]: (error) Reference to local variable returned.\n", + errout.str()); + + check("int f(int& a) {\n" + " return a;\n" + "}\n" + "int& hello() {\n" + " int x = 0;\n" + " return f(x);\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("int& f(int a) {\n" + " return a;\n" + "}\n" + "int& hello() {\n" + " int x = 0;\n" + " return f(x);\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2]: (error) Reference to local variable returned.\n", errout.str()); + + check("int f(int a) {\n" + " return a;\n" + "}\n" + "int& hello() {\n" + " int x = 0;\n" + " return f(x);\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("template\n" + "int& f(int& x, T y) {\n" + " x += y;\n" + " return x;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + void returnReferenceLiteral() { check("const std::string &a() {\n" " return \"foo\";\n" @@ -1199,7 +1247,7 @@ private: } void danglingReference() { - check("int &f( int k )\n" + check("int f( int k )\n" "{\n" " static int &r = k;\n" " return r;\n" @@ -1678,6 +1726,13 @@ private: " };\n" "}\n"); ASSERT_EQUALS("", errout.str()); + + check("int * f(std::vector& v) {\n" + " for(int & x : v)\n" + " return &x;\n" + " return nullptr;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); } void danglingLifetimeFunction() {