Fix issue 9646: False negative: Return reference to temporary with const reference (#2782)

This commit is contained in:
Paul Fultz II 2020-09-07 03:52:54 -05:00 committed by GitHub
parent 06c4b8897b
commit 362ab44c40
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 35 additions and 14 deletions

View File

@ -487,7 +487,7 @@ void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token
for (const Token *tok = start; tok && tok != end; tok = tok->next()) {
// Return reference from function
if (returnRef && Token::simpleMatch(tok->astParent(), "return")) {
for (const LifetimeToken& lt : getLifetimeTokens(tok)) {
for (const LifetimeToken& lt : getLifetimeTokens(tok, true)) {
const Variable* var = lt.token->variable();
if (var && !var->isGlobal() && !var->isStatic() && !var->isReference() && !var->isRValueReference() &&
isInScope(var->nameToken(), tok->scope())) {
@ -513,9 +513,10 @@ void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token
for (const ValueFlow::Value& val:tok->values()) {
if (!val.isLocalLifetimeValue())
continue;
for (const LifetimeToken& lt :getLifetimeTokens(getParentLifetime(val.tokvalue))) {
const bool escape = Token::Match(tok->astParent(), "return|throw");
for (const LifetimeToken& lt : getLifetimeTokens(getParentLifetime(val.tokvalue), escape)) {
const Token * tokvalue = lt.token;
if (Token::Match(tok->astParent(), "return|throw")) {
if (escape) {
if (getPointerDepth(tok) < getPointerDepth(tokvalue))
continue;
if (!isLifetimeBorrowed(tok, mSettings))
@ -529,7 +530,8 @@ void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token
} else if (tokvalue->variable() && isDeadScope(tokvalue->variable()->nameToken(), tok->scope())) {
errorInvalidLifetime(tok, &val);
break;
} else if (!tokvalue->variable() && isDeadTemporary(mTokenizer->isCPP(), tokvalue, tok, &mSettings->library)) {
} else if (!tokvalue->variable() &&
isDeadTemporary(mTokenizer->isCPP(), tokvalue, tok, &mSettings->library)) {
errorDanglingTemporaryLifetime(tok, &val);
break;
} else if (tokvalue->variable() && isInScope(tokvalue->variable()->nameToken(), tok->scope())) {

View File

@ -3002,7 +3002,7 @@ ValueFlow::Value getLifetimeObjValue(const Token *tok, bool inconclusive)
return result;
}
std::vector<LifetimeToken> getLifetimeTokens(const Token* tok, ValueFlow::Value::ErrorPath errorPath, int depth)
std::vector<LifetimeToken> getLifetimeTokens(const Token* tok, bool escape, ValueFlow::Value::ErrorPath errorPath, int depth)
{
if (!tok)
return std::vector<LifetimeToken> {};
@ -3019,10 +3019,10 @@ std::vector<LifetimeToken> getLifetimeTokens(const Token* tok, ValueFlow::Value:
} else if (Token::simpleMatch(var->declEndToken(), "=")) {
errorPath.emplace_back(var->declEndToken(), "Assigned to reference.");
const Token *vartok = var->declEndToken()->astOperand2();
if (vartok == tok || (var->isConst() && isTemporary(true, vartok, nullptr, true)))
if (vartok == tok || (!escape && var->isConst() && isTemporary(true, vartok, nullptr, true)))
return {{tok, true, std::move(errorPath)}};
if (vartok)
return getLifetimeTokens(vartok, std::move(errorPath), depth - 1);
return getLifetimeTokens(vartok, escape, std::move(errorPath), depth - 1);
} else if (Token::simpleMatch(var->nameToken()->astParent(), ":") &&
var->nameToken()->astParent()->astParent() &&
Token::simpleMatch(var->nameToken()->astParent()->astParent()->previous(), "for (")) {
@ -3032,7 +3032,7 @@ std::vector<LifetimeToken> getLifetimeTokens(const Token* tok, ValueFlow::Value:
return {{tok, true, std::move(errorPath)}};
const Token* contok = var->nameToken()->astParent()->astOperand2();
if (contok)
return getLifetimeTokens(contok, std::move(errorPath), depth - 1);
return getLifetimeTokens(contok, escape, std::move(errorPath), depth - 1);
} else {
return std::vector<LifetimeToken> {};
}
@ -3047,7 +3047,7 @@ std::vector<LifetimeToken> getLifetimeTokens(const Token* tok, ValueFlow::Value:
for (const Token* returnTok : returns) {
if (returnTok == tok)
continue;
for (LifetimeToken& lt : getLifetimeTokens(returnTok, std::move(errorPath), depth - 1)) {
for (LifetimeToken& lt : getLifetimeTokens(returnTok, escape, std::move(errorPath), depth - 1)) {
const Token* argvarTok = lt.token;
const Variable* argvar = argvarTok->variable();
if (!argvar)
@ -3064,7 +3064,7 @@ std::vector<LifetimeToken> getLifetimeTokens(const Token* tok, ValueFlow::Value:
lt.errorPath.emplace_back(returnTok, "Return reference.");
lt.errorPath.emplace_back(tok->previous(), "Called function passing '" + argTok->str() + "'.");
std::vector<LifetimeToken> arglts = LifetimeToken::setInconclusive(
getLifetimeTokens(argTok, std::move(lt.errorPath), depth - 1), returns.size() > 1);
getLifetimeTokens(argTok, escape, std::move(lt.errorPath), depth - 1), returns.size() > 1);
result.insert(result.end(), arglts.begin(), arglts.end());
}
}
@ -3076,7 +3076,7 @@ std::vector<LifetimeToken> getLifetimeTokens(const Token* tok, ValueFlow::Value:
if (y == Library::Container::Yield::AT_INDEX || y == Library::Container::Yield::ITEM) {
errorPath.emplace_back(tok->previous(), "Accessing container.");
return LifetimeToken::setAddressOf(
getLifetimeTokens(tok->tokAt(-2)->astOperand1(), std::move(errorPath), depth - 1), false);
getLifetimeTokens(tok->tokAt(-2)->astOperand1(), escape, std::move(errorPath), depth - 1), false);
}
}
} else if (Token::Match(tok, ".|::|[")) {
@ -3099,10 +3099,10 @@ std::vector<LifetimeToken> getLifetimeTokens(const Token* tok, ValueFlow::Value:
if (!v.isLocalLifetimeValue())
continue;
errorPath.insert(errorPath.end(), v.errorPath.begin(), v.errorPath.end());
return getLifetimeTokens(v.tokvalue, std::move(errorPath));
return getLifetimeTokens(v.tokvalue, escape, std::move(errorPath));
}
} else {
return LifetimeToken::setAddressOf(getLifetimeTokens(vartok, std::move(errorPath)),
return LifetimeToken::setAddressOf(getLifetimeTokens(vartok, escape, std::move(errorPath)),
!(astIsContainer(vartok) && Token::simpleMatch(vartok->astParent(), "[")));
}
}

View File

@ -364,7 +364,10 @@ struct LifetimeToken {
const Token *parseCompareInt(const Token *tok, ValueFlow::Value &true_value, ValueFlow::Value &false_value);
std::vector<LifetimeToken> getLifetimeTokens(const Token* tok, ValueFlow::Value::ErrorPath errorPath = ValueFlow::Value::ErrorPath{}, int depth = 20);
std::vector<LifetimeToken> getLifetimeTokens(const Token* tok,
bool escape = false,
ValueFlow::Value::ErrorPath errorPath = ValueFlow::Value::ErrorPath{},
int depth = 20);
const Variable* getLifetimeVariable(const Token* tok, ValueFlow::Value::ErrorPath& errorPath, bool* addressOf = nullptr);

View File

@ -2660,6 +2660,22 @@ private:
" return std::string(str.c_str(), 1);\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("int get_value();\n"
"const int &get_reference1() {\n"
" const int &x = get_value();\n"
" return x;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (error) Reference to temporary returned.\n", errout.str());
check("int get_value();\n"
"const int &get_reference2() {\n"
" const int &x1 = get_value();\n"
" const int &x2 = x1;\n"
" return x2;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:3] -> [test.cpp:5]: (error) Reference to temporary returned.\n",
errout.str());
}
void invalidLifetime() {