From 8d7088aa24fcc57ffbca386d29575adc2fa13fc3 Mon Sep 17 00:00:00 2001 From: Paul Date: Tue, 8 Sep 2020 18:30:45 -0500 Subject: [PATCH] Fix issue 9835: False negative: Return reference to temporary with const reference --- lib/astutils.cpp | 3 +++ lib/checkautovariables.cpp | 17 +++++++++++++++++ lib/checkautovariables.h | 2 ++ lib/valueflow.cpp | 4 ++-- test/testautovariables.cpp | 24 +++++++++++++++++++++++- 5 files changed, 47 insertions(+), 3 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 1f75dd775..2f42dcfba 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -248,6 +248,9 @@ bool isTemporary(bool cpp, const Token* tok, const Library* library, bool unknow if (Token::Match(tok, "&|<<|>>") && isLikelyStream(cpp, tok->astOperand1())) return false; if (Token::Match(tok->previous(), ">|%name% (")) { + if (tok->valueType()) { + return tok->valueType()->reference == Reference::None; + } const Token* ftok = nullptr; if (tok->previous()->link()) ftok = tok->previous()->link()->previous(); diff --git a/lib/checkautovariables.cpp b/lib/checkautovariables.cpp index 4f9e41af6..29ce78379 100644 --- a/lib/checkautovariables.cpp +++ b/lib/checkautovariables.cpp @@ -509,6 +509,16 @@ void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token errorDanglingReference(tok, var, errorPath); continue; } + // Reference to temporary + } else if (tok->variable() && (tok->variable()->isReference() || tok->variable()->isRValueReference())) { + for (const LifetimeToken& lt : getLifetimeTokens(getParentLifetime(tok))) { + const Token * tokvalue = lt.token; + if (isDeadTemporary(mTokenizer->isCPP(), tokvalue, tok, &mSettings->library)) { + errorDanglingTempReference(tok, lt.errorPath, lt.inconclusive); + break; + } + } + } for (const ValueFlow::Value& val:tok->values()) { if (!val.isLocalLifetimeValue()) @@ -621,6 +631,13 @@ void CheckAutoVariables::errorDanglngLifetime(const Token *tok, const ValueFlow: reportError(errorPath, Severity::error, "danglingLifetime", msg + ".", CWE562, inconclusive); } +void CheckAutoVariables::errorDanglingTempReference(const Token* tok, ErrorPath errorPath, bool inconclusive) +{ + errorPath.emplace_back(tok, ""); + reportError( + errorPath, Severity::error, "danglingTempReference", "Using reference to dangling temporary.", CWE562, inconclusive); +} + void CheckAutoVariables::errorReturnReference(const Token* tok, ErrorPath errorPath, bool inconclusive) { errorPath.emplace_back(tok, ""); diff --git a/lib/checkautovariables.h b/lib/checkautovariables.h index 7087aacef..99356ca56 100644 --- a/lib/checkautovariables.h +++ b/lib/checkautovariables.h @@ -79,6 +79,7 @@ private: void errorDanglingTemporaryLifetime(const Token* tok, const ValueFlow::Value* val); void errorReturnReference(const Token* tok, ErrorPath errorPath, bool inconclusive); void errorDanglingReference(const Token *tok, const Variable *var, ErrorPath errorPath); + void errorDanglingTempReference(const Token* tok, ErrorPath errorPath, bool inconclusive); void errorReturnTempReference(const Token* tok, ErrorPath errorPath, bool inconclusive); void errorInvalidDeallocation(const Token *tok, const ValueFlow::Value *val); void errorReturnAddressOfFunctionParameter(const Token *tok, const std::string &varname); @@ -94,6 +95,7 @@ private: c.errorReturnReference(nullptr, errorPath, false); c.errorDanglingReference(nullptr, nullptr, errorPath); c.errorReturnTempReference(nullptr, errorPath, false); + c.errorDanglingTempReference(nullptr, errorPath, false); c.errorInvalidDeallocation(nullptr, nullptr); c.errorReturnAddressOfFunctionParameter(nullptr, "parameter"); c.errorUselessAssignmentArg(nullptr); diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index c91dc02ac..4487af4c0 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -3062,7 +3062,7 @@ std::vector getLifetimeTokens(const Token* tok, bool escape, Valu return std::vector {}; const Token* argTok = args[n]; lt.errorPath.emplace_back(returnTok, "Return reference."); - lt.errorPath.emplace_back(tok->previous(), "Called function passing '" + argTok->str() + "'."); + lt.errorPath.emplace_back(tok->previous(), "Called function passing '" + argTok->expressionString() + "'."); std::vector arglts = LifetimeToken::setInconclusive( getLifetimeTokens(argTok, escape, std::move(lt.errorPath), depth - 1), returns.size() > 1); result.insert(result.end(), arglts.begin(), arglts.end()); @@ -3422,7 +3422,7 @@ struct LifetimeStore { return LifetimeStore{}; } const Token *argtok2 = args[n]; - return LifetimeStore{argtok2, "Passed to '" + tok->str() + "'.", ValueFlow::Value::LifetimeKind::Object}; + return LifetimeStore{argtok2, "Passed to '" + tok->expressionString() + "'.", ValueFlow::Value::LifetimeKind::Object}; } template diff --git a/test/testautovariables.cpp b/test/testautovariables.cpp index 9bca1691a..36651d3c7 100644 --- a/test/testautovariables.cpp +++ b/test/testautovariables.cpp @@ -1638,7 +1638,8 @@ private: " int& x = h();\n" " g(&x);\n" "}\n"); - ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:5] -> [test.cpp:5]: (error) Using pointer to temporary.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:5] -> [test.cpp:5]: (error) Using pointer to temporary.\n" + "[test.cpp:4] -> [test.cpp:5]: (error) Using reference to dangling temporary.\n", errout.str()); check("void g(int*);\n" "int h();\n" @@ -1675,6 +1676,27 @@ private: ASSERT_EQUALS("", errout.str()); } + void danglingTempReference() { + check("const std::string& g(const std::string& str_cref) {\n" + " return str_cref;\n" + "}\n" + "void f() {\n" + " const auto& str_cref2 = g(std::string(\"hello\"));\n" + " std::cout << str_cref2 << std::endl;\n" + "}\n"); + ASSERT_EQUALS("error", errout.str()); + + // Lifetime extended + check("std::string g(const std::string& str_cref) {\n" + " return str_cref;\n" + "}\n" + "void f() {\n" + " const auto& str_cref2 = g(std::string(\"hello\"));\n" + " std::cout << str_cref2 << std::endl;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + void testglobalnamespace() { check("class SharedPtrHolder\n" "{\n"