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

Improvement to lifetime tracking of addressof and derefencing 

This will now warn for cases like this:

```cpp
auto& f() {
    std::vector<int> x;
    return x[0];
}
```

It also improves the handling of address of operator, so it can now warn across some function calls, 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-02-22 06:38:56 +01:00 committed by Daniel Marjamäki
parent 715714f4de
commit 507c7a4388
5 changed files with 145 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
lib/checkautovariables.cpp
View File

@ -603,6 +603,7 @@ void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token
return;
bool returnRef = Function::returnsReference(scope->function);
for (const Token *tok = start; tok && tok != end; tok = tok->next()) {
// Return reference form function
if (returnRef && Token::simpleMatch(tok->astParent(), "return")) {
ErrorPath errorPath;
const Variable *var = getLifetimeVariable(tok, errorPath);
@ -611,6 +612,7 @@ void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token
errorReturnReference(tok, errorPath);
continue;
}
// Assign reference to non-local variable
} else if (Token::Match(tok->astParent(), "&|&&") && Token::simpleMatch(tok->astParent()->astParent(), "=") &&
tok->variable() && tok->variable()->declarationId() == tok->varId() && tok->variable()->isStatic() &&
!tok->variable()->isArgument()) {
@ -624,8 +626,7 @@ void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token
for (const ValueFlow::Value& val:tok->values()) {
if (!val.isLocalLifetimeValue())
continue;
// Skip temporaries for now
if (val.tokvalue == tok)
if (!val.tokvalue->variable())
continue;
if (Token::Match(tok->astParent(), "return|throw")) {
if (getPointerDepth(tok) < getPointerDepth(val.tokvalue))
@ -633,11 +634,11 @@ void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token
if (tok->astParent()->str() == "return" && !astIsContainer(tok) && scope->function &&
mSettings->library.detectContainer(scope->function->retDef))
continue;
if (isInScope(val.tokvalue, scope)) {
if (isInScope(val.tokvalue->variable()->nameToken(), scope)) {
errorReturnDanglingLifetime(tok, &val);
break;
}
} else if (isDeadScope(val.tokvalue, tok->scope())) {
} else if (isDeadScope(val.tokvalue->variable()->nameToken(), tok->scope())) {
errorInvalidLifetime(tok, &val);
break;
} else if (tok->variable() && tok->variable()->declarationId() == tok->varId() &&
@ -679,7 +680,9 @@ void CheckAutoVariables::checkVarLifetime()
static std::string lifetimeMessage(const Token *tok, const ValueFlow::Value *val, ErrorPath &errorPath)
{
const Token *vartok = val ? val->tokvalue : nullptr;
const Token *tokvalue = val ? val->tokvalue : nullptr;
const Variable *tokvar = tokvalue ? tokvalue->variable() : nullptr;
const Token *vartok = tokvar ? tokvar->nameToken() : nullptr;
std::string type = lifetimeType(tok, val);
std::string msg = type;
if (vartok) {

117
lib/valueflow.cpp
View File

@ -2662,27 +2662,27 @@ std::string lifetimeType(const Token *tok, const ValueFlow::Value *val)
return result;
}
const Variable *getLifetimeVariable(const Token *tok, ValueFlow::Value::ErrorPath &errorPath, int depth)
const Token *getLifetimeToken(const Token *tok, ValueFlow::Value::ErrorPath &errorPath, int depth = 20)
{
if (!tok)
return nullptr;
const Variable *var = tok->variable();
if (depth < 0)
return var;
return tok;
if (var && var->declarationId() == tok->varId()) {
if (var->isReference() || var->isRValueReference()) {
if (!var->declEndToken())
return nullptr;
return tok;
if (var->isArgument()) {
errorPath.emplace_back(var->declEndToken(), "Passed to reference.");
return var;
return var->nameToken();
} 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;
return tok;
if (vartok)
return getLifetimeVariable(vartok, errorPath, depth-1);
return getLifetimeToken(vartok, errorPath, depth - 1);
} else {
return nullptr;
}
@ -2690,17 +2690,20 @@ const Variable *getLifetimeVariable(const Token *tok, ValueFlow::Value::ErrorPat
} else if (Token::Match(tok->previous(), "%name% (")) {
const Function *f = tok->previous()->function();
if (!f)
return nullptr;
return tok;
if (!Function::returnsReference(f))
return nullptr;
return tok;
const Token *returnTok = findSimpleReturn(f);
if (!returnTok)
return nullptr;
return tok;
if (returnTok == tok)
return var;
const Variable *argvar = getLifetimeVariable(returnTok, errorPath, depth-1);
return tok;
const Token *argvarTok = getLifetimeToken(returnTok, errorPath, depth - 1);
if (!argvarTok)
return tok;
const Variable *argvar = argvarTok->variable();
if (!argvar)
return nullptr;
return tok;
if (argvar->isArgument() && (argvar->isReference() || argvar->isRValueReference())) {
int n = getArgumentPos(argvar, f);
if (n < 0)
@ -2708,10 +2711,43 @@ const Variable *getLifetimeVariable(const Token *tok, ValueFlow::Value::ErrorPat
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, depth-1);
return getLifetimeToken(argTok, errorPath, depth - 1);
}
} else if (Token::Match(tok, ".|::|[")) {
const Token *vartok = tok;
while (vartok) {
if (vartok->str() == "[" || vartok->originalName() == "->")
vartok = vartok->astOperand1();
else if (vartok->str() == "." || vartok->str() == "::")
vartok = vartok->astOperand2();
else
break;
}
if (!vartok)
return tok;
const Variable *tokvar = vartok->variable();
if (!astIsContainer(vartok) && !(tokvar && tokvar->isArray()) && Token::Match(vartok->astParent(), "[|*|.") &&
vartok->astParent()->originalName() != ".") {
for (const ValueFlow::Value &v : vartok->values()) {
if (!v.isLocalLifetimeValue())
continue;
errorPath.insert(errorPath.end(), v.errorPath.begin(), v.errorPath.end());
return getLifetimeToken(v.tokvalue, errorPath);
}
} else {
return getLifetimeToken(vartok, errorPath);
}
}
return var;
return tok;
}
const Variable *getLifetimeVariable(const Token *tok, ValueFlow::Value::ErrorPath &errorPath)
{
const Token *tok2 = getLifetimeToken(tok, errorPath);
if (tok2 && tok2->variable())
return tok2->variable();
return nullptr;
}
static bool isNotLifetimeValue(const ValueFlow::Value& val)
@ -2825,17 +2861,17 @@ struct LifetimeStore {
template <class Predicate>
void byRef(Token *tok, TokenList *tokenlist, ErrorLogger *errorLogger, const Settings *settings, Predicate pred) const {
ErrorPath er = errorPath;
const Variable *var = getLifetimeVariable(argtok, er);
if (!var)
const Token *lifeTok = getLifetimeToken(argtok, er);
if (!lifeTok)
return;
if (!pred(var))
if (!pred(lifeTok))
return;
er.emplace_back(argtok, message);
ValueFlow::Value value;
value.valueType = ValueFlow::Value::LIFETIME;
value.lifetimeScope = ValueFlow::Value::Local;
value.tokvalue = var->nameToken();
value.tokvalue = lifeTok;
value.errorPath = er;
value.lifetimeKind = type;
// Don't add the value a second time
@ -2846,7 +2882,7 @@ struct LifetimeStore {
}
void byRef(Token *tok, TokenList *tokenlist, ErrorLogger *errorLogger, const Settings *settings) const {
byRef(tok, tokenlist, errorLogger, settings, [](const Variable *) {
byRef(tok, tokenlist, errorLogger, settings, [](const Token *) {
return true;
});
}
@ -2876,10 +2912,10 @@ struct LifetimeStore {
continue;
const Token *tok3 = v.tokvalue;
ErrorPath er = v.errorPath;
const Variable *var = getLifetimeVariable(tok3, er);
if (!var)
continue;
if (!pred(var))
const Token *lifeTok = getLifetimeToken(tok3, er);
if (!lifeTok)
return;
if (!pred(lifeTok))
return;
er.emplace_back(argtok, message);
er.insert(er.end(), errorPath.begin(), errorPath.end());
@ -2887,7 +2923,7 @@ struct LifetimeStore {
ValueFlow::Value value;
value.valueType = ValueFlow::Value::LIFETIME;
value.lifetimeScope = v.lifetimeScope;
value.tokvalue = var->nameToken();
value.tokvalue = lifeTok;
value.errorPath = er;
value.lifetimeKind = type;
// Don't add the value a second time
@ -2899,7 +2935,7 @@ struct LifetimeStore {
}
void byVal(Token *tok, TokenList *tokenlist, ErrorLogger *errorLogger, const Settings *settings) const {
byVal(tok, tokenlist, errorLogger, settings, [](const Variable *) {
byVal(tok, tokenlist, errorLogger, settings, [](const Token *) {
return true;
});
}
@ -2925,7 +2961,7 @@ struct LifetimeStore {
}
void byDerefCopy(Token *tok, TokenList *tokenlist, ErrorLogger *errorLogger, const Settings *settings) const {
byDerefCopy(tok, tokenlist, errorLogger, settings, [](const Variable *) {
byDerefCopy(tok, tokenlist, errorLogger, settings, [](const Token *) {
return true;
});
}
@ -3083,7 +3119,10 @@ static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase*, ErrorLogger
std::set<const Scope *> scopes;
auto isCapturingVariable = [&](const Variable *var) {
auto isCapturingVariable = [&](const Token *varTok) {
const Variable *var = varTok->variable();
if (!var)
return false;
const Scope *scope = var->scope();
if (!scope)
return false;
@ -3113,23 +3152,8 @@ static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase*, ErrorLogger
// address of
else if (tok->isUnaryOp("&")) {
ErrorPath errorPath;
// child should be some buffer or variable
const Token *vartok = tok->astOperand1();
while (vartok) {
if (vartok->str() == "[")
vartok = vartok->astOperand1();
else if (vartok->str() == "." || vartok->str() == "::")
vartok = vartok->astOperand2();
else
break;
}
if (!vartok)
continue;
const Variable * var = getLifetimeVariable(vartok, errorPath);
if (!var)
continue;
if (var->isPointer() && Token::Match(vartok->astParent(), "[|*"))
const Token *lifeTok = getLifetimeToken(tok->astOperand1(), errorPath);
if (!lifeTok)
continue;
errorPath.emplace_back(tok, "Address of variable taken here.");
@ -3137,7 +3161,7 @@ static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase*, ErrorLogger
ValueFlow::Value value;
value.valueType = ValueFlow::Value::LIFETIME;
value.lifetimeScope = ValueFlow::Value::Local;
value.tokvalue = var->nameToken();
value.tokvalue = lifeTok;
value.errorPath = errorPath;
setTokenValue(tok, value, tokenlist->getSettings());
@ -3149,7 +3173,6 @@ static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase*, ErrorLogger
const Library::Container * container = settings->library.detectContainer(tok->variable()->typeStartToken());
if (!container)
continue;
const Variable * var = tok->variable();
bool isIterator = !Token::Match(tok->tokAt(2), "data|c_str");
if (isIterator)
@ -3160,7 +3183,7 @@ static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase*, ErrorLogger
ValueFlow::Value value;
value.valueType = ValueFlow::Value::LIFETIME;
value.lifetimeScope = ValueFlow::Value::Local;
value.tokvalue = var->nameToken();
value.tokvalue = tok;
value.errorPath = errorPath;
value.lifetimeKind = isIterator ? ValueFlow::Value::Iterator : ValueFlow::Value::Object;
setTokenValue(tok->tokAt(3), value, tokenlist->getSettings());

2
lib/valueflow.h
View File

@ -229,7 +229,7 @@ namespace ValueFlow {
std::string eitherTheConditionIsRedundant(const Token *condition);
}
const Variable *getLifetimeVariable(const Token *tok, ValueFlow::Value::ErrorPath &errorPath, int depth=20);
const Variable *getLifetimeVariable(const Token *tok, ValueFlow::Value::ErrorPath &errorPath);
std::string lifetimeType(const Token *tok, const ValueFlow::Value *val);

52
test/testautovariables.cpp
View File

@ -108,6 +108,7 @@ private:
TEST_CASE(returnReference6);
TEST_CASE(returnReference7);
TEST_CASE(returnReferenceFunction);
TEST_CASE(returnReferenceContainer);
TEST_CASE(returnReferenceLiteral);
TEST_CASE(returnReferenceCalculation);
TEST_CASE(returnReferenceLambda);
@ -1108,6 +1109,17 @@ private:
"[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(
"[test.cpp:1] -> [test.cpp:2] -> [test.cpp:6] -> [test.cpp:6] -> [test.cpp:5] -> [test.cpp:6]: (error) Returning pointer to local variable 'x' that will be invalid when returning.\n",
errout.str());
check("int f(int& a) {\n"
" return a;\n"
"}\n"
@ -1143,6 +1155,28 @@ private:
ASSERT_EQUALS("", errout.str());
}
void returnReferenceContainer() {
check("auto& f() {\n"
" std::vector<int> x;\n"
" return x[0];\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3]: (error) Reference to local variable returned.\n", errout.str());
check("struct A { int foo; };\n"
"int& f(std::vector<A> v) {\n"
" auto it = v.begin();\n"
" return it->foo;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (error) Reference to local variable returned.\n", errout.str());
check("struct A { int foo; };\n"
"int& f(std::vector<A>& v) {\n"
" auto it = v.begin();\n"
" return it->foo;\n"
"}\n");
ASSERT_EQUALS("", errout.str());
}
void returnReferenceLiteral() {
check("const std::string &a() {\n"
" return \"foo\";\n"
@ -1447,6 +1481,24 @@ private:
"}\n");
ASSERT_EQUALS("[test.cpp:3] -> [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("auto f() {\n"
" std::vector<int> x;\n"
" auto p = &x[0];\n"
" return p;\n"
"}\n");
ASSERT_EQUALS(
"[test.cpp:3] -> [test.cpp:2] -> [test.cpp:4]: (error) Returning pointer to local variable 'x' that will be invalid when returning.\n",
errout.str());
check("struct A { int foo; };\n"
"int* f(std::vector<A> v) {\n"
" auto it = v.begin();\n"
" return &it->foo;\n"
"}\n");
ASSERT_EQUALS(
"[test.cpp:3] -> [test.cpp:4] -> [test.cpp:2] -> [test.cpp:4]: (error) Returning object that points to local variable 'v' that will be invalid when returning.\n",
errout.str());
check("auto f(std::vector<int> x) {\n"
" auto it = x.begin();\n"
" return it;\n"

18
test/testvalueflow.cpp
View File

@ -115,6 +115,10 @@ private:
TEST_CASE(valueFlowContainerSize);
}
static bool isNotTokValue(const ValueFlow::Value &val) {
return !val.isTokValue();
}
bool testValueOfXKnown(const char code[], unsigned int linenr, int value) {
// Tokenize..
Tokenizer tokenizer(&settings, this);
@ -321,6 +325,7 @@ private:
void valueFlowPointerAlias() {
const char *code;
std::list<ValueFlow::Value> values;
code = "const char * f() {\n"
" static const char *x;\n"
@ -342,8 +347,13 @@ private:
" struct X *x;\n"
" x = &x[1];\n"
"}";
ASSERT_EQUALS(true, tokenValues(code, "&").empty());
ASSERT_EQUALS(true, tokenValues(code, "x [").empty());
values = tokenValues(code, "&");
values.remove_if(&isNotTokValue);
ASSERT_EQUALS(true, values.empty());
values = tokenValues(code, "x [");
values.remove_if(&isNotTokValue);
ASSERT_EQUALS(true, values.empty());
}
void valueFlowLifetime() {
@ -356,7 +366,7 @@ private:
" auto x = [&]() { return a + 1; };\n"
" auto b = x;\n"
"}\n";
ASSERT_EQUALS(true, testValueOfX(code, 4, "a ;", ValueFlow::Value::LIFETIME));
ASSERT_EQUALS(true, testValueOfX(code, 4, "a + 1", ValueFlow::Value::LIFETIME));
code = "void f() {\n"
" int a = 1;\n"
@ -378,7 +388,7 @@ private:
" auto x = v.begin();\n"
" auto it = x;\n"
"}\n";
ASSERT_EQUALS(true, testValueOfX(code, 4, "v ;", ValueFlow::Value::LIFETIME));
ASSERT_EQUALS(true, testValueOfX(code, 4, "v . begin", ValueFlow::Value::LIFETIME));
}
void valueFlowArrayElement() {