Extend lifetime checking for references
This will use the lifetime checker for dangling references. It will find these cases for indirectly assigned reference: ```cpp int &foo() { int s = 0; int& x = s; return x; } ``` This will also fix issue 510 as well: ```cpp int &f( int k ) { static int &r = k; return r; } ```
This commit is contained in:
parent
db413d254a
commit
3975913637
|
@ -527,31 +527,8 @@ void CheckAutoVariables::returnReference()
|
||||||
if (tok2->str() != "return")
|
if (tok2->str() != "return")
|
||||||
continue;
|
continue;
|
||||||
|
|
||||||
// return..
|
|
||||||
if (Token::Match(tok2, "return %var% ;")) {
|
|
||||||
// is the returned variable a local variable?
|
|
||||||
if (isAutoVar(tok2->next())) {
|
|
||||||
const Variable *var1 = tok2->next()->variable();
|
|
||||||
// If reference variable is used, check what it references
|
|
||||||
if (Token::Match(var1->nameToken(), "%var% [=(]")) {
|
|
||||||
const Token *tok3 = var1->nameToken()->tokAt(2);
|
|
||||||
if (!Token::Match(tok3, "%var% [);.]"))
|
|
||||||
continue;
|
|
||||||
|
|
||||||
// Only report error if variable that is referenced is
|
|
||||||
// a auto variable
|
|
||||||
if (!isAutoVar(tok3))
|
|
||||||
continue;
|
|
||||||
}
|
|
||||||
|
|
||||||
// report error..
|
|
||||||
errorReturnReference(tok2);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// return reference to temporary..
|
// return reference to temporary..
|
||||||
else if (Token::Match(tok2, "return %name% (") &&
|
if (Token::Match(tok2, "return %name% (") && Token::simpleMatch(tok2->linkAt(2), ") ;")) {
|
||||||
Token::simpleMatch(tok2->linkAt(2), ") ;")) {
|
|
||||||
if (returnTemporary(tok2->next())) {
|
if (returnTemporary(tok2->next())) {
|
||||||
// report error..
|
// report error..
|
||||||
errorReturnTempReference(tok2);
|
errorReturnTempReference(tok2);
|
||||||
|
@ -559,7 +536,8 @@ void CheckAutoVariables::returnReference()
|
||||||
}
|
}
|
||||||
|
|
||||||
// Return reference to a literal or the result of a calculation
|
// Return reference to a literal or the result of a calculation
|
||||||
else if (tok2->astOperand1() && (tok2->astOperand1()->isCalculation() || tok2->next()->isLiteral()) && astHasAutoResult(tok2->astOperand1())) {
|
else if (tok2->astOperand1() && (tok2->astOperand1()->isCalculation() || tok2->next()->isLiteral()) &&
|
||||||
|
astHasAutoResult(tok2->astOperand1())) {
|
||||||
errorReturnTempReference(tok2);
|
errorReturnTempReference(tok2);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -623,7 +601,25 @@ void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token
|
||||||
// If the scope is not set correctly then skip checking it
|
// If the scope is not set correctly then skip checking it
|
||||||
if (scope->bodyStart != start)
|
if (scope->bodyStart != start)
|
||||||
return;
|
return;
|
||||||
|
bool returnRef = Function::returnsReference(scope->function);
|
||||||
for (const Token *tok = start; tok && tok != end; tok = tok->next()) {
|
for (const Token *tok = start; tok && tok != end; tok = tok->next()) {
|
||||||
|
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())) {
|
||||||
|
errorReturnReference(tok, errorPath);
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
} else if (Token::Match(tok->astParent(), "&|&&") && Token::simpleMatch(tok->astParent()->astParent(), "=") &&
|
||||||
|
tok->variable() && tok->variable()->declarationId() == tok->varId() && tok->variable()->isStatic() &&
|
||||||
|
!tok->variable()->isArgument()) {
|
||||||
|
ErrorPath errorPath;
|
||||||
|
const Variable *var = getLifetimeVariable(tok, errorPath);
|
||||||
|
if (var && isInScope(var->nameToken(), tok->scope())) {
|
||||||
|
errorDanglingReference(tok, var, errorPath);
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
}
|
||||||
for (const ValueFlow::Value& val:tok->values()) {
|
for (const ValueFlow::Value& val:tok->values()) {
|
||||||
if (!val.isLifetimeValue())
|
if (!val.isLifetimeValue())
|
||||||
continue;
|
continue;
|
||||||
|
@ -756,9 +752,19 @@ void CheckAutoVariables::errorDanglngLifetime(const Token *tok, const ValueFlow:
|
||||||
reportError(errorPath, Severity::error, "danglingLifetime", msg + ".", CWE562, false);
|
reportError(errorPath, Severity::error, "danglingLifetime", msg + ".", CWE562, false);
|
||||||
}
|
}
|
||||||
|
|
||||||
void CheckAutoVariables::errorReturnReference(const Token *tok)
|
void CheckAutoVariables::errorReturnReference(const Token *tok, ErrorPath errorPath)
|
||||||
{
|
{
|
||||||
reportError(tok, Severity::error, "returnReference", "Reference to auto variable returned.", CWE562, false);
|
errorPath.emplace_back(tok, "");
|
||||||
|
reportError(errorPath, Severity::error, "returnReference", "Reference to local variable returned.", CWE562, false);
|
||||||
|
}
|
||||||
|
|
||||||
|
void CheckAutoVariables::errorDanglingReference(const Token *tok, const Variable *var, ErrorPath errorPath)
|
||||||
|
{
|
||||||
|
std::string tokName = tok ? tok->str() : "x";
|
||||||
|
std::string varName = var ? var->name() : "y";
|
||||||
|
std::string msg = "Non-local reference variable '" + tokName + "' to local variable '" + varName + "'";
|
||||||
|
errorPath.emplace_back(tok, "");
|
||||||
|
reportError(errorPath, Severity::error, "danglingReference", msg, CWE562, false);
|
||||||
}
|
}
|
||||||
|
|
||||||
void CheckAutoVariables::errorReturnTempReference(const Token *tok)
|
void CheckAutoVariables::errorReturnTempReference(const Token *tok)
|
||||||
|
|
|
@ -91,7 +91,8 @@ private:
|
||||||
void errorReturnDanglingLifetime(const Token *tok, const ValueFlow::Value* val);
|
void errorReturnDanglingLifetime(const Token *tok, const ValueFlow::Value* val);
|
||||||
void errorInvalidLifetime(const Token *tok, const ValueFlow::Value* val);
|
void errorInvalidLifetime(const Token *tok, const ValueFlow::Value* val);
|
||||||
void errorDanglngLifetime(const Token *tok, const ValueFlow::Value *val);
|
void errorDanglngLifetime(const Token *tok, const ValueFlow::Value *val);
|
||||||
void errorReturnReference(const Token *tok);
|
void errorReturnReference(const Token *tok, ErrorPath errorPath);
|
||||||
|
void errorDanglingReference(const Token *tok, const Variable *var, ErrorPath errorPath);
|
||||||
void errorReturnTempReference(const Token *tok);
|
void errorReturnTempReference(const Token *tok);
|
||||||
void errorInvalidDeallocation(const Token *tok, const ValueFlow::Value *val);
|
void errorInvalidDeallocation(const Token *tok, const ValueFlow::Value *val);
|
||||||
void errorReturnAddressOfFunctionParameter(const Token *tok, const std::string &varname);
|
void errorReturnAddressOfFunctionParameter(const Token *tok, const std::string &varname);
|
||||||
|
@ -99,12 +100,14 @@ private:
|
||||||
void errorUselessAssignmentPtrArg(const Token *tok);
|
void errorUselessAssignmentPtrArg(const Token *tok);
|
||||||
|
|
||||||
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const OVERRIDE {
|
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const OVERRIDE {
|
||||||
|
ErrorPath errorPath;
|
||||||
CheckAutoVariables c(nullptr,settings,errorLogger);
|
CheckAutoVariables c(nullptr,settings,errorLogger);
|
||||||
c.errorAutoVariableAssignment(nullptr, false);
|
c.errorAutoVariableAssignment(nullptr, false);
|
||||||
c.errorReturnAddressToAutoVariable(nullptr);
|
c.errorReturnAddressToAutoVariable(nullptr);
|
||||||
c.errorAssignAddressOfLocalArrayToGlobalPointer(nullptr, nullptr);
|
c.errorAssignAddressOfLocalArrayToGlobalPointer(nullptr, nullptr);
|
||||||
c.errorReturnPointerToLocalArray(nullptr);
|
c.errorReturnPointerToLocalArray(nullptr);
|
||||||
c.errorReturnReference(nullptr);
|
c.errorReturnReference(nullptr, errorPath);
|
||||||
|
c.errorDanglingReference(nullptr, nullptr, errorPath);
|
||||||
c.errorReturnTempReference(nullptr);
|
c.errorReturnTempReference(nullptr);
|
||||||
c.errorInvalidDeallocation(nullptr, nullptr);
|
c.errorInvalidDeallocation(nullptr, nullptr);
|
||||||
c.errorReturnAddressOfFunctionParameter(nullptr, "parameter");
|
c.errorReturnAddressOfFunctionParameter(nullptr, "parameter");
|
||||||
|
|
|
@ -2049,6 +2049,15 @@ bool Function::argsMatch(const Scope *scope, const Token *first, const Token *se
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
bool Function::returnsReference(const Function *function)
|
||||||
|
{
|
||||||
|
if (!function)
|
||||||
|
return false;
|
||||||
|
if (function->type != Function::eFunction)
|
||||||
|
return false;
|
||||||
|
return function->tokenDef->strAt(-1) == "&";
|
||||||
|
}
|
||||||
|
|
||||||
const Token * Function::constructorMemberInitialization() const
|
const Token * Function::constructorMemberInitialization() const
|
||||||
{
|
{
|
||||||
if (!isConstructor() || !functionScope || !functionScope->bodyStart)
|
if (!isConstructor() || !functionScope || !functionScope->bodyStart)
|
||||||
|
|
|
@ -845,6 +845,8 @@ public:
|
||||||
|
|
||||||
static bool argsMatch(const Scope *scope, const Token *first, const Token *second, const std::string &path, unsigned int path_length);
|
static bool argsMatch(const Scope *scope, const Token *first, const Token *second, const std::string &path, unsigned int path_length);
|
||||||
|
|
||||||
|
static bool returnsReference(const Function *function);
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @return token to ":" if the function is a constructor
|
* @return token to ":" if the function is a constructor
|
||||||
* and it contains member initialization otherwise a nullptr is returned
|
* and it contains member initialization otherwise a nullptr is returned
|
||||||
|
|
|
@ -2603,23 +2603,21 @@ static bool valueFlowForward(Token * const startToken,
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
static const Variable *getLifetimeVariable(const Token *tok, ErrorPath &errorPath)
|
const Variable *getLifetimeVariable(const Token *tok, ValueFlow::Value::ErrorPath &errorPath)
|
||||||
{
|
{
|
||||||
|
if (!tok)
|
||||||
|
return nullptr;
|
||||||
const Variable *var = tok->variable();
|
const Variable *var = tok->variable();
|
||||||
if (!var)
|
if (!var)
|
||||||
return nullptr;
|
return nullptr;
|
||||||
if (var->isReference() || var->isRValueReference()) {
|
if (var->isReference() || var->isRValueReference()) {
|
||||||
for (const ValueFlow::Value &v : tok->values()) {
|
if (!var->declEndToken())
|
||||||
if (!v.isLifetimeValue())
|
return nullptr;
|
||||||
continue;
|
errorPath.emplace_back(var->declEndToken(), "Assigned to reference.");
|
||||||
if (v.tokvalue == tok)
|
const Token *vartok = var->declEndToken()->astOperand2();
|
||||||
continue;
|
if (vartok == tok)
|
||||||
errorPath.insert(errorPath.end(), v.errorPath.begin(), v.errorPath.end());
|
return nullptr;
|
||||||
const Variable *var2 = getLifetimeVariable(v.tokvalue, errorPath);
|
return getLifetimeVariable(vartok, errorPath);
|
||||||
if (var2)
|
|
||||||
return var2;
|
|
||||||
}
|
|
||||||
return nullptr;
|
|
||||||
}
|
}
|
||||||
return var;
|
return var;
|
||||||
}
|
}
|
||||||
|
|
|
@ -32,6 +32,7 @@ class Settings;
|
||||||
class SymbolDatabase;
|
class SymbolDatabase;
|
||||||
class Token;
|
class Token;
|
||||||
class TokenList;
|
class TokenList;
|
||||||
|
class Variable;
|
||||||
|
|
||||||
namespace ValueFlow {
|
namespace ValueFlow {
|
||||||
class CPPCHECKLIB Value {
|
class CPPCHECKLIB Value {
|
||||||
|
@ -204,4 +205,6 @@ namespace ValueFlow {
|
||||||
std::string eitherTheConditionIsRedundant(const Token *condition);
|
std::string eitherTheConditionIsRedundant(const Token *condition);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const Variable *getLifetimeVariable(const Token *tok, ValueFlow::Value::ErrorPath &errorPath);
|
||||||
|
|
||||||
#endif // valueflowH
|
#endif // valueflowH
|
||||||
|
|
|
@ -112,6 +112,8 @@ private:
|
||||||
TEST_CASE(returnReferenceLambda);
|
TEST_CASE(returnReferenceLambda);
|
||||||
TEST_CASE(returnReferenceInnerScope);
|
TEST_CASE(returnReferenceInnerScope);
|
||||||
|
|
||||||
|
TEST_CASE(danglingReference);
|
||||||
|
|
||||||
// global namespace
|
// global namespace
|
||||||
TEST_CASE(testglobalnamespace);
|
TEST_CASE(testglobalnamespace);
|
||||||
|
|
||||||
|
@ -837,19 +839,27 @@ private:
|
||||||
}
|
}
|
||||||
|
|
||||||
void returnReference1() {
|
void returnReference1() {
|
||||||
|
check("int &foo()\n"
|
||||||
|
"{\n"
|
||||||
|
" int s = 0;\n"
|
||||||
|
" int& x = s;\n"
|
||||||
|
" return x;\n"
|
||||||
|
"}\n");
|
||||||
|
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:5]: (error) Reference to local variable returned.\n", errout.str());
|
||||||
|
|
||||||
check("std::string &foo()\n"
|
check("std::string &foo()\n"
|
||||||
"{\n"
|
"{\n"
|
||||||
" std::string s;\n"
|
" std::string s;\n"
|
||||||
" return s;\n"
|
" return s;\n"
|
||||||
"}");
|
"}");
|
||||||
ASSERT_EQUALS("[test.cpp:4]: (error) Reference to auto variable returned.\n", errout.str());
|
ASSERT_EQUALS("[test.cpp:4]: (error) Reference to local variable returned.\n", errout.str());
|
||||||
|
|
||||||
check("std::vector<int> &foo()\n"
|
check("std::vector<int> &foo()\n"
|
||||||
"{\n"
|
"{\n"
|
||||||
" std::vector<int> v;\n"
|
" std::vector<int> v;\n"
|
||||||
" return v;\n"
|
" return v;\n"
|
||||||
"}");
|
"}");
|
||||||
ASSERT_EQUALS("[test.cpp:4]: (error) Reference to auto variable returned.\n", errout.str());
|
ASSERT_EQUALS("[test.cpp:4]: (error) Reference to local variable returned.\n", errout.str());
|
||||||
|
|
||||||
check("std::vector<int> &foo()\n"
|
check("std::vector<int> &foo()\n"
|
||||||
"{\n"
|
"{\n"
|
||||||
|
@ -942,7 +952,7 @@ private:
|
||||||
" std::string s;\n"
|
" std::string s;\n"
|
||||||
" return s;\n"
|
" return s;\n"
|
||||||
"}");
|
"}");
|
||||||
ASSERT_EQUALS("[test.cpp:7]: (error) Reference to auto variable returned.\n", errout.str());
|
ASSERT_EQUALS("[test.cpp:7]: (error) Reference to local variable returned.\n", errout.str());
|
||||||
|
|
||||||
check("class Fred {\n"
|
check("class Fred {\n"
|
||||||
" std::vector<int> &foo();\n"
|
" std::vector<int> &foo();\n"
|
||||||
|
@ -952,7 +962,7 @@ private:
|
||||||
" std::vector<int> v;\n"
|
" std::vector<int> v;\n"
|
||||||
" return v;\n"
|
" return v;\n"
|
||||||
"}");
|
"}");
|
||||||
ASSERT_EQUALS("[test.cpp:7]: (error) Reference to auto variable returned.\n", errout.str());
|
ASSERT_EQUALS("[test.cpp:7]: (error) Reference to local variable returned.\n", errout.str());
|
||||||
|
|
||||||
check("class Fred {\n"
|
check("class Fred {\n"
|
||||||
" std::vector<int> &foo();\n"
|
" std::vector<int> &foo();\n"
|
||||||
|
@ -1188,6 +1198,23 @@ private:
|
||||||
ASSERT_EQUALS("", errout.str());
|
ASSERT_EQUALS("", errout.str());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void danglingReference() {
|
||||||
|
check("int &f( int k )\n"
|
||||||
|
"{\n"
|
||||||
|
" static int &r = k;\n"
|
||||||
|
" return r;\n"
|
||||||
|
"}\n");
|
||||||
|
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:3]: (error) Non-local reference variable 'r' to local variable 'k'\n",
|
||||||
|
errout.str());
|
||||||
|
|
||||||
|
check("int &f( int & k )\n"
|
||||||
|
"{\n"
|
||||||
|
" static int &r = k;\n"
|
||||||
|
" return r;\n"
|
||||||
|
"}\n");
|
||||||
|
ASSERT_EQUALS("", errout.str());
|
||||||
|
}
|
||||||
|
|
||||||
void testglobalnamespace() {
|
void testglobalnamespace() {
|
||||||
check("class SharedPtrHolder\n"
|
check("class SharedPtrHolder\n"
|
||||||
"{\n"
|
"{\n"
|
||||||
|
|
Loading…
Reference in New Issue