Lifetime: Follow functions that return references

This will now warn for cases like this:

```cpp
int& f(int& a) {
    return a;
}
int& hello() {
    int x = 0;
    return f(x);
}
```
This commit is contained in:
Paul Fultz II 2019-01-26 11:03:57 +01:00 committed by Daniel Marjamäki
parent 68bbe15116
commit d6aaf401df
3 changed files with 120 additions and 10 deletions

View File

@ -606,7 +606,8 @@ void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token
if (returnRef && Token::simpleMatch(tok->astParent(), "return")) { if (returnRef && Token::simpleMatch(tok->astParent(), "return")) {
ErrorPath errorPath; ErrorPath errorPath;
const Variable *var = getLifetimeVariable(tok, 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); errorReturnReference(tok, errorPath);
continue; continue;
} }

View File

@ -2603,21 +2603,75 @@ static bool valueFlowForward(Token * const startToken,
return true; 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) const Variable *getLifetimeVariable(const Token *tok, ValueFlow::Value::ErrorPath &errorPath)
{ {
if (!tok) if (!tok)
return nullptr; return nullptr;
const Variable *var = tok->variable(); const Variable *var = tok->variable();
if (!var) if (var && var->declarationId() == tok->varId()) {
return nullptr; if (var->isReference() || var->isRValueReference()) {
if (var->isReference() || var->isRValueReference()) { if (!var->declEndToken())
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; return nullptr;
errorPath.emplace_back(var->declEndToken(), "Assigned to reference."); if (!Function::returnsReference(f))
const Token *vartok = var->declEndToken()->astOperand2();
if (vartok == tok)
return nullptr; 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; return var;
} }

View File

@ -107,6 +107,7 @@ private:
TEST_CASE(returnReference5); TEST_CASE(returnReference5);
TEST_CASE(returnReference6); TEST_CASE(returnReference6);
TEST_CASE(returnReference7); TEST_CASE(returnReference7);
TEST_CASE(returnReferenceFunction);
TEST_CASE(returnReferenceLiteral); TEST_CASE(returnReferenceLiteral);
TEST_CASE(returnReferenceCalculation); TEST_CASE(returnReferenceCalculation);
TEST_CASE(returnReferenceLambda); TEST_CASE(returnReferenceLambda);
@ -1094,6 +1095,53 @@ private:
ASSERT_EQUALS("", errout.str()); 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<class T>\n"
"int& f(int& x, T y) {\n"
" x += y;\n"
" return x;\n"
"}\n");
ASSERT_EQUALS("", errout.str());
}
void returnReferenceLiteral() { void returnReferenceLiteral() {
check("const std::string &a() {\n" check("const std::string &a() {\n"
" return \"foo\";\n" " return \"foo\";\n"
@ -1199,7 +1247,7 @@ private:
} }
void danglingReference() { void danglingReference() {
check("int &f( int k )\n" check("int f( int k )\n"
"{\n" "{\n"
" static int &r = k;\n" " static int &r = k;\n"
" return r;\n" " return r;\n"
@ -1678,6 +1726,13 @@ private:
" };\n" " };\n"
"}\n"); "}\n");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
check("int * f(std::vector<int>& v) {\n"
" for(int & x : v)\n"
" return &x;\n"
" return nullptr;\n"
"}\n");
ASSERT_EQUALS("", errout.str());
} }
void danglingLifetimeFunction() { void danglingLifetimeFunction() {