walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Issue 6175: Check lifetime of a variables stored in containers and member variables 

Cppcheck will now warn for all cases here:

```cpp
#include <vector>
class CCluster {};
class MyClass
{ public:
    std::vector<CCluster*> m_cluster;
    void createCluster()
    {
        CCluster cl;
        CCluster* pcl=&cl;
        m_cluster.push_back(pcl);
    }
    void createCluster2()
    {
        CCluster cl;
        m_cluster.push_back(&cl);
    }
    CCluster* Cluster()
    {
        CCluster cl;
        CCluster* pcl=&cl;
        return pcl;
    }
    CCluster* Cluster2()
    {
        CCluster cl;
        return &cl;
    }
};

```
This commit is contained in:
Paul Fultz II 2018-11-21 08:43:57 +01:00 committed by Daniel Marjamäki
parent 081bd7660e
commit f16d9d7d90
5 changed files with 197 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

61
lib/checkautovariables.cpp
View File

@ -604,13 +604,22 @@ void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token
if (val.tokvalue == tok)
continue;
if (Token::Match(tok->astParent(), "return|throw")) {
if (getPointerDepth(tok) >= getPointerDepth(val.tokvalue) && isInScope(val.tokvalue, scope)) {
if (getPointerDepth(tok) < getPointerDepth(val.tokvalue))
continue;
if (tok->astParent()->str() == "return" && !astIsContainer(tok) && scope->function &&
mSettings->library.detectContainer(scope->function->retDef))
continue;
if (isInScope(val.tokvalue, scope)) {
errorReturnDanglingLifetime(tok, &val);
break;
}
} else if (isDeadScope(val.tokvalue, tok->scope())) {
errorInvalidLifetime(tok, &val);
break;
} else if (tok->variable() && !tok->variable()->isLocal() && !tok->variable()->isArgument() &&
isInScope(val.tokvalue, tok->scope())) {
errorDanglngLifetime(tok, &val);
break;
}
}
const Token *lambdaEndToken = findLambdaEndToken(tok);
@ -627,10 +636,6 @@ void CheckAutoVariables::checkVarLifetime()
for (const Scope * scope : symbolDatabase->functionScopes) {
if (!scope->function)
continue;
// Skip if returning a container
const Library::Container * container = mSettings->library.detectContainer(scope->function->retDef);
if (container)
continue;
checkVarLifetimeScope(scope->bodyStart, scope->bodyEnd);
}
}
@ -657,12 +662,11 @@ static std::string lifetimeType(const Token *tok, const ValueFlow::Value* val)
return result;
}
void CheckAutoVariables::errorReturnDanglingLifetime(const Token *tok, const ValueFlow::Value* val)
static std::string lifetimeMessage(const Token *tok, const ValueFlow::Value *val, ErrorPath &errorPath)
{
const Token *vartok = val ? val->tokvalue : nullptr;
ErrorPath errorPath = val ? val->errorPath : ErrorPath();
std::string type = lifetimeType(tok, val);
std::string msg = "Returning " + type;
std::string msg = type;
if (vartok) {
errorPath.emplace_back(vartok, "Variable created here.");
const Variable * var = vartok->variable();
@ -684,41 +688,34 @@ void CheckAutoVariables::errorReturnDanglingLifetime(const Token *tok, const Val
msg += " '" + var->name() + "'";
}
}
return msg;
}
void CheckAutoVariables::errorReturnDanglingLifetime(const Token *tok, const ValueFlow::Value *val)
{
ErrorPath errorPath = val ? val->errorPath : ErrorPath();
std::string msg = "Returning " + lifetimeMessage(tok, val, errorPath);
errorPath.emplace_back(tok, "");
reportError(errorPath, Severity::error, "returnDanglingLifetime", msg + " that will be invalid when returning.", CWE562, false);
}
void CheckAutoVariables::errorInvalidLifetime(const Token *tok, const ValueFlow::Value* val)
{
const Token *vartok = val ? val->tokvalue : nullptr;
ErrorPath errorPath = val ? val->errorPath : ErrorPath();
std::string type = lifetimeType(tok, val);
std::string msg = "Using " + type;
if (vartok) {
errorPath.emplace_back(vartok, "Variable created here.");
const Variable * var = vartok->variable();
if (var) {
switch (val->lifetimeKind) {
case ValueFlow::Value::Object:
if (type == "pointer")
msg += " to local variable";
else
msg += " that points to local variable";
break;
case ValueFlow::Value::Lambda:
msg += " that captures local variable";
break;
case ValueFlow::Value::Iterator:
msg += " to local container";
break;
}
msg += " '" + var->name() + "'";
}
}
std::string msg = "Using " + lifetimeMessage(tok, val, errorPath);
errorPath.emplace_back(tok, "");
reportError(errorPath, Severity::error, "invalidLifetime", msg + " that is out of scope.", CWE562, false);
}
void CheckAutoVariables::errorDanglngLifetime(const Token *tok, const ValueFlow::Value *val)
{
ErrorPath errorPath = val ? val->errorPath : ErrorPath();
std::string tokName = tok ? tok->str() : "x";
std::string msg = "Non-local variable '" + tokName + "' will use " + lifetimeMessage(tok, val, errorPath);
errorPath.emplace_back(tok, "");
reportError(errorPath, Severity::error, "danglingLifetime", msg + ".", CWE562, false);
}
void CheckAutoVariables::errorReturnReference(const Token *tok)
{
reportError(tok, Severity::error, "returnReference", "Reference to auto variable returned.", CWE562, false);

2
lib/checkautovariables.h
View File

@ -90,6 +90,7 @@ private:
void errorAutoVariableAssignment(const Token *tok, bool inconclusive);
void errorReturnDanglingLifetime(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 errorReturnReference(const Token *tok);
void errorReturnTempReference(const Token *tok);
void errorInvalidDeallocation(const Token *tok, const ValueFlow::Value *val);
@ -111,6 +112,7 @@ private:
c.errorUselessAssignmentPtrArg(nullptr);
c.errorReturnDanglingLifetime(nullptr, nullptr);
c.errorInvalidLifetime(nullptr, nullptr);
c.errorDanglngLifetime(nullptr, nullptr);
}
static std::string myName() {

83
lib/valueflow.cpp
View File

@ -2500,7 +2500,7 @@ static void valueFlowLifetimeFunction(Token *tok, TokenList *tokenlist, ErrorLog
static void valueFlowForwardLifetime(Token * tok, TokenList *tokenlist, ErrorLogger *errorLogger, const Settings *settings)
{
const Token *parent = tok->astParent();
while (parent && parent->isArithmeticalOp())
while (parent && (parent->isArithmeticalOp() || parent->str() == ","))
parent = parent->astParent();
if (!parent)
return;
@ -2547,6 +2547,27 @@ static void valueFlowForwardLifetime(Token * tok, TokenList *tokenlist, ErrorLog
// Function call
} else if (Token::Match(parent->previous(), "%name% (")) {
valueFlowLifetimeFunction(const_cast<Token *>(parent->previous()), tokenlist, errorLogger, settings);
// Variable
} else if (tok->variable()) {
const Variable *var = tok->variable();
if (!var->typeStartToken() && !var->typeStartToken()->scope())
return;
const Token *endOfVarScope = var->typeStartToken()->scope()->bodyEnd;
std::list<ValueFlow::Value> values = tok->values();
const Token *nextExpression = nextAfterAstRightmostLeaf(parent);
// Only forward lifetime values
values.remove_if(&isNotLifetimeValue);
valueFlowForward(const_cast<Token *>(nextExpression),
endOfVarScope,
var,
var->declarationId(),
values,
false,
false,
tokenlist,
errorLogger,
settings);
}
}
@ -2615,8 +2636,46 @@ struct LifetimeStore {
return true;
});
}
template <class Predicate>
void byDerefCopy(Token *tok, TokenList *tokenlist, ErrorLogger *errorLogger, const Settings *settings, Predicate pred) const {
for (const ValueFlow::Value &v : argtok->values()) {
if (!v.isLifetimeValue())
continue;
const Token *tok2 = v.tokvalue;
ErrorPath errorPath = v.errorPath;
const Variable *var = getLifetimeVariable(tok2, errorPath);
if (!var)
continue;
for (const Token *tok3 = tok; tok != var->declEndToken(); tok3 = tok3->previous()) {
if (tok3->varId() == var->declarationId()) {
LifetimeStore{tok3, message, type} .byVal(tok, tokenlist, errorLogger, settings, pred);
break;
}
}
}
}
void byDerefCopy(Token *tok, TokenList *tokenlist, ErrorLogger *errorLogger, const Settings *settings) const {
byDerefCopy(tok, tokenlist, errorLogger, settings, [](const Variable *) {
return true;
});
}
};
static const Token *endTemplateArgument(const Token *tok)
{
for (; tok; tok = tok->next()) {
if (Token::Match(tok, ">|,"))
return tok;
else if (tok->link() && Token::Match(tok, "(|{|[|<"))
tok = tok->link();
else if (Token::simpleMatch(tok, ";"))
return nullptr;
}
return nullptr;
}
static void valueFlowLifetimeFunction(Token *tok, TokenList *tokenlist, ErrorLogger *errorLogger, const Settings *settings)
{
if (!Token::Match(tok, "%name% ("))
@ -2631,6 +2690,21 @@ static void valueFlowLifetimeFunction(Token *tok, TokenList *tokenlist, ErrorLog
LifetimeStore{argtok, "Passed to '" + tok->str() + "'.", ValueFlow::Value::Object} .byVal(
tok->next(), tokenlist, errorLogger, settings);
}
} else if (Token::Match(tok->tokAt(-2), "%var% . push_back|push_front|insert|push|assign") &&
astIsContainer(tok->tokAt(-2))) {
const Token *containerTypeTok = tok->tokAt(-2)->valueType()->containerTypeToken;
const Token *endTypeTok = endTemplateArgument(containerTypeTok);
const bool isPointer = endTypeTok && Token::simpleMatch(endTypeTok->previous(), "*");
Token *vartok = tok->tokAt(-2);
std::vector<const Token *> args = getArguments(tok);
if (args.size() == 2 && astCanonicalType(args[0]) == astCanonicalType(args[1]) &&
(((astIsIterator(args[0]) && astIsIterator(args[1])) || (astIsPointer(args[0]) && astIsPointer(args[1]))))) {
LifetimeStore{args.back(), "Added to container '" + vartok->str() + "'.", ValueFlow::Value::Object} .byDerefCopy(
vartok, tokenlist, errorLogger, settings);
} else if (!args.empty() && astIsPointer(args.back()) == isPointer) {
LifetimeStore{args.back(), "Added to container '" + vartok->str() + "'.", ValueFlow::Value::Object} .byVal(
vartok, tokenlist, errorLogger, settings);
}
}
}
@ -3369,11 +3443,14 @@ static void execute(const Token *expr,
else if (expr->str() == "[" && expr->astOperand1() && expr->astOperand2()) {
const Token *tokvalue = nullptr;
if (!programMemory->getTokValue(expr->astOperand1()->varId(), &tokvalue)) {
if (expr->astOperand1()->values().size() != 1U || !expr->astOperand1()->values().front().isTokValue()) {
auto tokvalue_it = std::find_if(expr->astOperand1()->values().begin(),
expr->astOperand1()->values().end(),
std::mem_fn(&ValueFlow::Value::isTokValue));
if (tokvalue_it == expr->astOperand1()->values().end()) {
*error = true;
return;
}
tokvalue = expr->astOperand1()->values().front().tokvalue;
tokvalue = tokvalue_it->tokvalue;
}
if (!tokvalue || !tokvalue->isLiteral()) {
*error = true;

90
test/testautovariables.cpp
View File

@ -707,7 +707,9 @@ private:
" char str[100] = {0};\n"
" return str;\n"
"}");
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:3] -> [test.cpp:4]: (error) Returning pointer to local variable 'str' that will be invalid when returning.\n", errout.str());
ASSERT_EQUALS(
"[test.cpp:3] -> [test.cpp:3] -> [test.cpp:4]: (error) Returning pointer to local variable 'str' that will be invalid when returning.\n",
errout.str());
check("char *foo()\n" // use ValueFlow
"{\n"
@ -715,7 +717,9 @@ private:
" char *p = str;\n"
" return p;\n"
"}");
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:3] -> [test.cpp:5]: (error) Returning pointer to local variable 'str' that will be invalid when returning.\n", errout.str());
ASSERT_EQUALS(
"[test.cpp:3] -> [test.cpp:3] -> [test.cpp:5]: (error) Returning pointer to local variable 'str' that will be invalid when returning.\n",
errout.str());
check("class Fred {\n"
" char *foo();\n"
@ -725,7 +729,9 @@ private:
" char str[100] = {0};\n"
" return str;\n"
"}");
ASSERT_EQUALS("[test.cpp:7] -> [test.cpp:6] -> [test.cpp:7]: (error) Returning pointer to local variable 'str' that will be invalid when returning.\n", errout.str());
ASSERT_EQUALS(
"[test.cpp:6] -> [test.cpp:6] -> [test.cpp:7]: (error) Returning pointer to local variable 'str' that will be invalid when returning.\n",
errout.str());
check("char * format_reg(char *outbuffer_start) {\n"
" return outbuffer_start;\n"
@ -779,13 +785,17 @@ private:
" char x[10] = {0};\n"
" return x+5;\n"
"}");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2] -> [test.cpp:3]: (error) Returning pointer to local variable 'x' that will be invalid when returning.\n", errout.str());
ASSERT_EQUALS(
"[test.cpp:2] -> [test.cpp:2] -> [test.cpp:3]: (error) Returning pointer to local variable 'x' that will be invalid when returning.\n",
errout.str());
check("char *foo(int y) {\n"
" char x[10] = {0};\n"
" return (x+8)-y;\n"
"}");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2] -> [test.cpp:3]: (error) Returning pointer to local variable 'x' that will be invalid when returning.\n", errout.str());
ASSERT_EQUALS(
"[test.cpp:2] -> [test.cpp:2] -> [test.cpp:3]: (error) Returning pointer to local variable 'x' that will be invalid when returning.\n",
errout.str());
}
void returnLocalVariable5() { // cast
@ -793,7 +803,9 @@ private:
" int x[10] = {0};\n"
" return (char *)x;\n"
"}");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2] -> [test.cpp:3]: (error) Returning pointer to local variable 'x' that will be invalid when returning.\n", errout.str());
ASSERT_EQUALS(
"[test.cpp:2] -> [test.cpp:2] -> [test.cpp:3]: (error) Returning pointer to local variable 'x' that will be invalid when returning.\n",
errout.str());
}
void returnLocalVariable6() { // valueflow
@ -1354,6 +1366,68 @@ private:
"[test.cpp:3] -> [test.cpp:4] -> [test.cpp:2] -> [test.cpp:4]: (error) Returning object that points to local variable 'x' that will be invalid when returning.\n",
errout.str());
check("std::vector<int*> f() {\n"
" int i = 0;\n"
" std::vector<int*> v;\n"
" v.push_back(&i);\n"
" return v;\n"
"}\n");
ASSERT_EQUALS(
"[test.cpp:4] -> [test.cpp:4] -> [test.cpp:2] -> [test.cpp:5]: (error) Returning object that points to local variable 'i' that will be invalid when returning.\n",
errout.str());
check("std::vector<int*> f() {\n"
" std::vector<int*> r;\n"
" int i = 0;\n"
" std::vector<int*> v;\n"
" v.push_back(&i);\n"
" r.assign(v.begin(), v.end());\n"
" return r;\n"
"}\n");
ASSERT_EQUALS(
"[test.cpp:5] -> [test.cpp:5] -> [test.cpp:5] -> [test.cpp:3] -> [test.cpp:7]: (error) Returning object that points to local variable 'i' that will be invalid when returning.\n",
errout.str());
check("struct A {\n"
" std::vector<int*> v;\n"
" void f() {\n"
" int i;\n"
" v.push_back(&i);\n"
" }\n"
"};\n");
ASSERT_EQUALS(
"[test.cpp:5] -> [test.cpp:5] -> [test.cpp:4] -> [test.cpp:5]: (error) Non-local variable 'v' will use object that points to local variable 'i'.\n",
errout.str());
check("struct A {\n"
" std::vector<int*> v;\n"
" void f() {\n"
" int i;\n"
" int * p = &i;\n"
" v.push_back(p);\n"
" }\n"
"};\n");
ASSERT_EQUALS(
"[test.cpp:5] -> [test.cpp:6] -> [test.cpp:4] -> [test.cpp:6]: (error) Non-local variable 'v' will use object that points to local variable 'i'.\n",
errout.str());
check("struct A {\n"
" std::vector<std::string> v;\n"
" void f() {\n"
" char s[3];\n"
" v.push_back(s);\n"
" }\n"
"};\n");
ASSERT_EQUALS("", errout.str());
check("std::vector<std::string> f() {\n"
" const char * s = \"hello\";\n"
" std::vector<std::string> v;\n"
" v.push_back(s);\n"
" return v;\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("auto f() {\n"
" static std::vector<int> x;\n"
" return x.begin();\n"
@ -1488,7 +1562,9 @@ private:
" }\n"
" x[3];\n"
"}\n");
ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:4] -> [test.cpp:7]: (error) Using pointer to local variable 'y' that is out of scope.\n", errout.str());
ASSERT_EQUALS(
"[test.cpp:4] -> [test.cpp:4] -> [test.cpp:7]: (error) Using pointer to local variable 'y' that is out of scope.\n",
errout.str());
check("void foo(int a) {\n"
" std::function<void()> f;\n"

6
test/testsimplifytypedef.cpp
View File

@ -1604,7 +1604,7 @@ private:
// The expected tokens..
const char expected2[] = "void f ( ) { char a [ 256 ] ; a = { 0 } ; char b [ 256 ] ; b = { 0 } ; }";
ASSERT_EQUALS(expected2, tok(code2, false));
ASSERT_EQUALS(expected2, tok(code2, false, Settings::Native, false));
ASSERT_EQUALS("", errout.str());
const char code3[] = "typedef char TString[256];\n"
@ -1615,7 +1615,7 @@ private:
// The expected tokens..
const char expected3[] = "void f ( ) { char a [ 256 ] ; a = \"\" ; char b [ 256 ] ; b = \"\" ; }";
ASSERT_EQUALS(expected3, tok(code3, false));
ASSERT_EQUALS(expected3, tok(code3, false, Settings::Native, false));
ASSERT_EQUALS("", errout.str());
const char code4[] = "typedef char TString[256];\n"
@ -1626,7 +1626,7 @@ private:
// The expected tokens..
const char expected4[] = "void f ( ) { char a [ 256 ] ; a = \"1234\" ; char b [ 256 ] ; b = \"5678\" ; }";
ASSERT_EQUALS(expected4, tok(code4, false));
ASSERT_EQUALS(expected4, tok(code4, false, Settings::Native, false));
ASSERT_EQUALS("", errout.str());
}