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

Extend lifetime analysis to pointer usage (#1477) 

* Use lifetime analysis for pointers as well

* Fix issue 1143: Pointer to local array

* Update message when using pointers

* Avoid infinite loop in tracing lifetimes
This commit is contained in:
Paul Fultz II 2018-11-12 03:08:17 -06:00 committed by Daniel Marjamäki
parent 717a2d370c
commit 0e11bb07c8
6 changed files with 101 additions and 104 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
lib/astutils.cpp
View File

@ -71,6 +71,11 @@ bool astIsBool(const Token *tok)
return tok && (tok->isBoolean() || (tok->valueType() && tok->valueType()->type == ValueType::Type::BOOL && !tok->valueType()->pointer));
}
bool astIsPointer(const Token *tok)
{
return tok && tok->valueType() && tok->valueType()->pointer;
}
std::string astCanonicalType(const Token *expr)
{
if (!expr)

2
lib/astutils.h
View File

@ -43,6 +43,8 @@ bool astIsFloat(const Token *tok, bool unknown);
/** Is expression of boolean type? */
bool astIsBool(const Token *tok);
bool astIsPointer(const Token *tok);
/**
* Get canonical type of expression. const/static/etc are not included and neither *&.
* For example:

119
lib/checkautovariables.cpp
View File

@ -281,37 +281,6 @@ void CheckAutoVariables::autoVariables()
if (checkRvalueExpression(varTok))
errorAutoVariableAssignment(tok->next(), false);
}
// Critical return
else if (Token::Match(tok, "return %var% ;") && isAutoVar(tok->next())) {
const std::list<ValueFlow::Value> &values = tok->next()->values();
const ValueFlow::Value *value = nullptr;
for (std::list<ValueFlow::Value>::const_iterator it = values.begin(); it != values.end(); ++it) {
if (!it->isTokValue())
continue;
if (!mSettings->inconclusive && it->isInconclusive())
continue;
if (!Token::Match(it->tokvalue->previous(), "= & %var%"))
continue;
if (!isAutoVar(it->tokvalue->next()))
continue;
if (!value || value->isInconclusive())
value = &(*it);
}
if (value)
errorReturnAddressToAutoVariable(tok, value);
}
else if (Token::Match(tok, "return & %var% ;")) {
const Token* varTok = tok->tokAt(2);
if (isAutoVar(varTok))
errorReturnAddressToAutoVariable(tok);
else if (varTok->varId()) {
const Variable * var1 = varTok->variable();
if (var1 && var1->isArgument() && var1->typeEndToken()->str() != "&")
errorReturnAddressOfFunctionParameter(tok, varTok->str());
}
}
// Invalid pointer deallocation
else if ((Token::Match(tok, "%name% ( %var% ) ;") && mSettings->library.dealloc(tok)) ||
(mTokenizer->isCPP() && Token::Match(tok, "delete [| ]| (| %var% !!["))) {
@ -338,28 +307,6 @@ void CheckAutoVariables::autoVariables()
//---------------------------------------------------------------------------
void CheckAutoVariables::returnPointerToLocalArray()
{
const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase();
for (const Scope * scope : symbolDatabase->functionScopes) {
if (!scope->function)
continue;
const Token *tok = scope->function->tokenDef;
// have we reached a function that returns a pointer
if (tok->previous() && tok->previous()->str() == "*") {
for (const Token *tok2 = scope->bodyStart->next(); tok2 && tok2 != scope->bodyEnd; tok2 = tok2->next()) {
// Return pointer to local array variable..
if (tok2 ->str() == "return" && isAutoVarArray(tok2->astOperand1())) {
errorReturnPointerToLocalArray(tok2);
}
}
}
}
}
void CheckAutoVariables::errorReturnAddressToAutoVariable(const Token *tok)
{
reportError(tok, Severity::error, "returnAddressOfAutoVariable", "Address of an auto-variable returned.", CWE562, false);
@ -643,14 +590,6 @@ void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token
if (scope->bodyStart != start)
return;
for (const Token *tok = start; tok && tok != end; tok = tok->next()) {
// Skip duplicate warning from dangling references
if (Token::Match(tok, "& %var%"))
continue;
if (tok->variable() && tok->variable()->isPointer())
continue;
if (std::any_of(tok->values().begin(), tok->values().end(), std::mem_fn(&ValueFlow::Value::isTokValue)))
continue;
for (const ValueFlow::Value& val:tok->values()) {
if (!val.isLifetimeValue())
continue;
@ -686,29 +625,44 @@ void CheckAutoVariables::checkVarLifetime()
}
}
void CheckAutoVariables::errorReturnDanglingLifetime(const Token *tok, const ValueFlow::Value* val)
static std::string lifetimeType(const Token *tok, const ValueFlow::Value* val)
{
const Token *vartok = val->tokvalue;
ErrorPath errorPath = val->errorPath;
std::string msg = "";
std::string result;
if(!val)
return "object";
switch (val->lifetimeKind) {
case ValueFlow::Value::Object:
msg = "Returning object";
break;
case ValueFlow::Value::Lambda:
msg = "Returning lambda";
result = "lambda";
break;
case ValueFlow::Value::Iterator:
msg = "Returning iterator";
result = "iterator";
break;
case ValueFlow::Value::Object:
if(astIsPointer(tok))
result = "pointer";
else
result = "object";
break;
}
return result;
}
void CheckAutoVariables::errorReturnDanglingLifetime(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 = "Returning " + type;
if (vartok) {
errorPath.emplace_back(vartok, "Variable created here.");
const Variable * var = vartok->variable();
if (var) {
switch (val->lifetimeKind) {
case ValueFlow::Value::Object:
msg += " that points to local variable";
if(type == "pointer")
msg += " to local variable";
else
msg += " that points to local variable";
break;
case ValueFlow::Value::Lambda:
msg += " that captures local variable";
@ -726,27 +680,20 @@ void CheckAutoVariables::errorReturnDanglingLifetime(const Token *tok, const Val
void CheckAutoVariables::errorInvalidLifetime(const Token *tok, const ValueFlow::Value* val)
{
const Token *vartok = val->tokvalue;
ErrorPath errorPath = val->errorPath;
std::string msg = "";
switch (val->lifetimeKind) {
case ValueFlow::Value::Object:
msg = "Using object";
break;
case ValueFlow::Value::Lambda:
msg = "Using lambda";
break;
case ValueFlow::Value::Iterator:
msg = "Using iterator";
break;
}
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:
msg += " that points to local variable";
if(type == "pointer")
msg += " to local variable";
else
msg += " that points to local variable";
break;
case ValueFlow::Value::Lambda:
msg += " that captures local variable";

6
lib/checkautovariables.h
View File

@ -59,7 +59,6 @@ public:
void runSimplifiedChecks(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger) override {
CheckAutoVariables checkAutoVariables(tokenizer, settings, errorLogger);
checkAutoVariables.autoVariables();
checkAutoVariables.returnPointerToLocalArray();
}
/** assign function argument */
@ -68,9 +67,6 @@ public:
/** Check auto variables */
void autoVariables();
/** Returning pointer to local array */
void returnPointerToLocalArray();
/** Returning reference to local/temporary variable */
void returnReference();
@ -113,6 +109,8 @@ private:
c.errorReturnAddressOfFunctionParameter(nullptr, "parameter");
c.errorUselessAssignmentArg(nullptr);
c.errorUselessAssignmentPtrArg(nullptr);
c.errorReturnDanglingLifetime(nullptr, nullptr);
c.errorInvalidLifetime(nullptr, nullptr);
}
static std::string myName() {

34
lib/valueflow.cpp
View File

@ -430,6 +430,16 @@ static void setTokenValue(Token* tok, const ValueFlow::Value &value, const Setti
return;
}
if(value.isLifetimeValue()) {
if(value.lifetimeKind == ValueFlow::Value::Iterator && parent->isArithmeticalOp()) {
setTokenValue(parent,value,settings);
}
else if(astIsPointer(tok) && astIsPointer(parent) && (parent->isArithmeticalOp() || Token::Match(parent, "( %type%"))) {
setTokenValue(parent,value,settings);
}
return;
}
if (parent->str() == "(" && !parent->astOperand2() && Token::Match(parent,"( %name%")) {
const ValueType &valueType = ValueType::parseDecl(parent->next(), settings);
if (valueType.pointer)
@ -2950,6 +2960,8 @@ static const Variable * getLifetimeVariable(const Token * tok, ErrorPath& errorP
for (const ValueFlow::Value& v:tok->values()) {
if (!v.isLifetimeValue() && !v.tokvalue)
continue;
if(v.tokvalue == tok)
continue;
errorPath.insert(errorPath.end(), v.errorPath.begin(), v.errorPath.end());
const Variable * var2 = getLifetimeVariable(v.tokvalue, errorPath);
if (var2)
@ -3071,7 +3083,9 @@ static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase*, ErrorLogger
if (!vartok)
continue;
const Variable * var = getLifetimeVariable(vartok, errorPath);
if (!(var && !var->isPointer()))
if(!var)
continue;
if(var->isPointer() && Token::Match(vartok->astParent(), "[|*"))
continue;
errorPath.emplace_back(tok, "Address of variable taken here.");
@ -3108,6 +3122,24 @@ static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase*, ErrorLogger
valueFlowForwardLifetime(tok->tokAt(3), tokenlist, errorLogger, settings);
}
// Check variables
else if(tok->variable()) {
ErrorPath errorPath;
const Variable * var = getLifetimeVariable(tok, errorPath);
if(!var)
continue;
if(var->isArray() && tok->astParent() && (astIsPointer(tok->astParent()) || Token::Match(tok->astParent(), "%assign%|return"))) {
errorPath.emplace_back(tok, "Array decayed to pointer here.");
ValueFlow::Value value;
value.valueType = ValueFlow::Value::LIFETIME;
value.tokvalue = var->nameToken();
value.errorPath = errorPath;
setTokenValue(tok, value, tokenlist->getSettings());
valueFlowForwardLifetime(tok, tokenlist, errorLogger, settings);
}
}
}
}

39
test/testautovariables.cpp
View File

@ -52,7 +52,6 @@ private:
// Check auto variables
checkAutoVariables.autoVariables();
checkAutoVariables.returnPointerToLocalArray();
}
}
@ -474,7 +473,7 @@ private:
" int num=2;"
" return &num;"
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) Address of an auto-variable returned.\n", errout.str());
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:3] -> [test.cpp:3]: (error) Returning pointer to local variable 'num' that will be invalid when returning.\n", errout.str());
}
void testautovar_return2() {
@ -486,7 +485,7 @@ private:
" int num=2;"
" return &num;"
"}");
ASSERT_EQUALS("[test.cpp:6]: (error) Address of an auto-variable returned.\n", errout.str());
ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:6] -> [test.cpp:6]: (error) Returning pointer to local variable 'num' that will be invalid when returning.\n", errout.str());
}
void testautovar_return3() {
@ -707,7 +706,7 @@ private:
" char str[100] = {0};\n"
" return str;\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Pointer to local array variable returned.\n", errout.str());
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());
check("char *foo()\n" // use ValueFlow
"{\n"
@ -715,7 +714,7 @@ private:
" char *p = str;\n"
" return p;\n"
"}");
ASSERT_EQUALS("[test.cpp:5]: (error) Pointer to local array variable returned.\n", errout.str());
ASSERT_EQUALS("[test.cpp:4] -> [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());
check("class Fred {\n"
" char *foo();\n"
@ -725,7 +724,7 @@ private:
" char str[100] = {0};\n"
" return str;\n"
"}");
ASSERT_EQUALS("[test.cpp:7]: (error) Pointer to local array variable returned.\n", errout.str());
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());
check("char * format_reg(char *outbuffer_start) {\n"
" return outbuffer_start;\n"
@ -764,7 +763,7 @@ private:
" char q[] = \"AAAAAAAAAAAA\";\n"
" return &q[1];\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) Pointer to local array variable returned.\n", errout.str());
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2] -> [test.cpp:3]: (error) Returning pointer to local variable 'q' that will be invalid when returning.\n", errout.str());
check("char *foo()\n"
"{\n"
@ -779,13 +778,13 @@ private:
" char x[10] = {0};\n"
" return x+5;\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) Pointer to local array variable returned.\n", errout.str());
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());
check("char *foo(int y) {\n"
" char x[10] = {0};\n"
" return (x+8)-y;\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) Pointer to local array variable returned.\n", errout.str());
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());
}
void returnLocalVariable5() { // cast
@ -793,7 +792,7 @@ private:
" int x[10] = {0};\n"
" return (char *)x;\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) Pointer to local array variable returned.\n", errout.str());
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());
}
void returnLocalVariable6() { // valueflow
@ -802,7 +801,7 @@ private:
" int p = &x;\n"
" return p;\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Address of auto-variable 'x' returned\n", errout.str());
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());
}
void returnReference1() {
@ -1177,14 +1176,14 @@ private:
" return &y;\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) Address of function parameter 'y' returned.\n", errout.str());
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:1] -> [test.cpp:3]: (error) Returning pointer to local variable 'y' that will be invalid when returning.\n", errout.str());
check("int ** foo(int * y)\n"
"{\n"
" return &y;\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) Address of function parameter 'y' returned.\n", errout.str());
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:1] -> [test.cpp:3]: (error) Returning pointer to local variable 'y' that will be invalid when returning.\n", errout.str());
check("const int * foo(const int & y)\n"
"{\n"
@ -1395,6 +1394,16 @@ private:
"}\n");
ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:4] -> [test.cpp:7]: (error) Using lambda that captures local variable 'b' that is out of scope.\n", errout.str());
check("void f(bool b) {\n"
" int* x;\n"
" if(b) {\n"
" int y[6] = {0,1,2,3,4,5};\n"
" x = y;\n"
" }\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());
check("void foo(int a) {\n"
" std::function<void()> f;\n"
" if (a > 0) {\n"
@ -1421,6 +1430,10 @@ private:
" }\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("int &a[];\n"
"void b(){int *c = a};\n");
ASSERT_EQUALS("", errout.str());
}
};