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

Improved checkautovariables: 

- Added support for checking a few more code patterns
- Simplified code by using more information from the symboldatabase
- Moved redundant part of c_str-check to checkstl
Two fixes according to output of pvs studio in testsimplifytokens.cpp
This commit is contained in:
PKEuS 2012-03-01 18:38:20 +01:00
parent c61762f454
commit 1ef99e2f21
6 changed files with 85 additions and 107 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

106
lib/checkautovariables.cpp
View File

@ -35,23 +35,25 @@ namespace {
}
bool CheckAutoVariables::errorAv(const Token* left, const Token* right)
bool CheckAutoVariables::isRefArg(unsigned int varId)
{
const Variable *var = _tokenizer->getSymbolDatabase()->getVariableFromVarId(left->varId());
const Variable *var = _tokenizer->getSymbolDatabase()->getVariableFromVarId(varId);
if (!var || !var->isArgument() ||
(!var->isArray() && !Token::Match(var->nameToken()->tokAt(-3), "%type% * *")) ||
(var->isArray() && !Token::Match(var->nameToken()->tokAt(-2), "%type% *")))
return false;
return(var && var->isArgument() && var->isReference());
}
return isAutoVar(right->varId());
bool CheckAutoVariables::isPtrArg(unsigned int varId)
{
const Variable *var = _tokenizer->getSymbolDatabase()->getVariableFromVarId(varId);
return(var && var->isArgument() && var->isPointer());
}
bool CheckAutoVariables::isAutoVar(unsigned int varId)
{
const Variable *var = _tokenizer->getSymbolDatabase()->getVariableFromVarId(varId);
if (!var || !var->isLocal() || var->isStatic() || var->isArray() || var->isPointer())
if (!var || !var->isLocal() || var->isStatic())
return false;
if (var->isReference()) {
@ -68,10 +70,13 @@ bool CheckAutoVariables::isAutoVarArray(unsigned int varId)
{
const Variable *var = _tokenizer->getSymbolDatabase()->getVariableFromVarId(varId);
if (!var || !var->isLocal() || var->isStatic() || !var->isArray())
return false;
return (var && var->isLocal() && !var->isStatic() && var->isArray());
}
return true;
// Verification that we really take the address of a local variable
static bool checkRvalueExpression(const Variable* var, const Token* next)
{
return((next->str() != "." || (!var->isPointer() && (!var->isClass() || var->type()))) && next->strAt(2) != ".");
}
void CheckAutoVariables::autoVariables()
@ -86,19 +91,23 @@ void CheckAutoVariables::autoVariables()
continue;
for (const Token *tok = scope->classStart; tok && tok != scope->classEnd; tok = tok->next()) {
//Critical assignment
if (Token::Match(tok, "[;{}] * %var% = & %var%") && errorAv(tok->tokAt(2), tok->tokAt(5))) {
// Critical assignment
if (Token::Match(tok, "[;{}] %var% = & %var%") && isRefArg(tok->next()->varId()) && isAutoVar(tok->tokAt(4)->varId())) {
const Variable * var = symbolDatabase->getVariableFromVarId(tok->tokAt(4)->varId());
if (checkRvalueExpression(var, tok->tokAt(5)))
errorAutoVariableAssignment(tok->next(), false);
} else if (Token::Match(tok, "[;{}] * %var% = & %var%") && isPtrArg(tok->tokAt(2)->varId()) && isAutoVar(tok->tokAt(5)->varId())) {
const Variable * var = symbolDatabase->getVariableFromVarId(tok->tokAt(5)->varId());
if (var && (!var->isClass() || var->type()))
if (checkRvalueExpression(var, tok->tokAt(6)))
errorAutoVariableAssignment(tok->next(), false);
} else if (Token::Match(tok, "[;{}] %var% . %var% = & %var%")) {
// TODO: check if the parameter is only changed temporarily (#2969)
if (_settings->inconclusive) {
const Variable * var1 = symbolDatabase->getVariableFromVarId(tok->next()->varId());
if (var1 && var1->isArgument() && Token::Match(var1->nameToken()->tokAt(-2), "%type% *")) {
if (var1 && var1->isArgument() && var1->isPointer()) {
const Variable * var2 = symbolDatabase->getVariableFromVarId(tok->tokAt(6)->varId());
if (var2 && var2->isLocal() && !var2->isStatic() && !Token::simpleMatch(var2->typeEndToken(), "*"))
errorAutoVariableAssignment(tok->next(), _settings->inconclusive);
if (isAutoVar(tok->tokAt(6)->varId()) && checkRvalueExpression(var2, tok->tokAt(7)))
errorAutoVariableAssignment(tok->next(), true);
}
}
tok = tok->tokAt(6);
@ -106,23 +115,24 @@ void CheckAutoVariables::autoVariables()
// TODO: check if the parameter is only changed temporarily (#2969)
if (_settings->inconclusive) {
const Variable * var1 = symbolDatabase->getVariableFromVarId(tok->next()->varId());
if (var1 && var1->isArgument() && Token::Match(var1->nameToken()->tokAt(-2), "%type% *")) {
const Variable * var2 = symbolDatabase->getVariableFromVarId(tok->tokAt(5)->varId());
if (var2 && var2->isLocal() && var2->isArray() && !var2->isStatic())
errorAutoVariableAssignment(tok->next(), _settings->inconclusive);
if (var1 && var1->isArgument() && var1->isPointer()) {
if (isAutoVarArray(tok->tokAt(5)->varId()))
errorAutoVariableAssignment(tok->next(), true);
}
}
tok = tok->tokAt(5);
} else if (Token::Match(tok, "[;{}] * %var% = %var% ;")) {
const Variable * var1 = symbolDatabase->getVariableFromVarId(tok->tokAt(2)->varId());
if (var1 && var1->isArgument() && Token::Match(var1->nameToken()->tokAt(-3), "%type% * *")) {
const Variable * var2 = symbolDatabase->getVariableFromVarId(tok->tokAt(4)->varId());
if (var2 && var2->isLocal() && var2->isArray() && !var2->isStatic())
if (isAutoVarArray(tok->tokAt(4)->varId()))
errorAutoVariableAssignment(tok->next(), false);
}
tok = tok->tokAt(4);
} else if (Token::Match(tok, "[;{}] %var% [ %any% ] = & %var%") && errorAv(tok->next(), tok->tokAt(7))) {
errorAutoVariableAssignment(tok->next(), false);
} else if (Token::Match(tok, "[;{}] %var% [") && Token::Match(tok->linkAt(2), "] = & %var%") && isPtrArg(tok->next()->varId()) && isAutoVar(tok->linkAt(2)->tokAt(3)->varId())) {
const Token* const varTok = tok->linkAt(2)->tokAt(3);
const Variable * var = symbolDatabase->getVariableFromVarId(varTok->varId());
if (checkRvalueExpression(var, varTok->next()))
errorAutoVariableAssignment(tok->next(), false);
}
// Critical return
else if (Token::Match(tok, "return & %var% ;") && isAutoVar(tok->tokAt(2)->varId())) {
@ -154,14 +164,10 @@ void CheckAutoVariables::returnPointerToLocalArray()
for (scope = symbolDatabase->scopeList.begin(); scope != symbolDatabase->scopeList.end(); ++scope) {
// only check functions
if (scope->type != Scope::eFunction)
if (scope->type != Scope::eFunction || !scope->function)
continue;
const Token *tok = scope->classDef;
// skip any qualification
while (Token::Match(tok->tokAt(-2), "%type% ::"))
tok = tok->tokAt(-2);
const Token *tok = scope->function->tokenDef;
// have we reached a function that returns a pointer
if (tok->previous() && tok->previous()->str() == "*") {
@ -224,6 +230,7 @@ bool CheckAutoVariables::returnTemporary(const Token *tok) const
{
if (!Token::Match(tok, "return %var% ("))
return false;
// TODO: Find all functions that return objects by value
return bool(NULL != Token::findmatch(_tokenizer->tokens(), ("std :: string " + tok->next()->str() + " (").c_str()));
}
@ -237,14 +244,10 @@ void CheckAutoVariables::returnReference()
for (scope = symbolDatabase->scopeList.begin(); scope != symbolDatabase->scopeList.end(); ++scope) {
// only check functions
if (scope->type != Scope::eFunction)
if (scope->type != Scope::eFunction || !scope->function)
continue;
const Token *tok = scope->classDef;
// skip any qualification
while (Token::Match(tok->tokAt(-2), "%type% ::"))
tok = tok->tokAt(-2);
const Token *tok = scope->function->tokenDef;
// have we reached a function that returns a reference?
if (tok->previous() && tok->previous()->str() == "&") {
@ -310,6 +313,8 @@ void CheckAutoVariables::errorInvalidDeallocation(const Token *tok)
// Return c_str
void CheckAutoVariables::returncstr()
{
// TODO: Move this to CheckStl::string_c_str
// locate function that returns a const char *..
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
@ -317,32 +322,16 @@ void CheckAutoVariables::returncstr()
for (scope = symbolDatabase->scopeList.begin(); scope != symbolDatabase->scopeList.end(); ++scope) {
// only check functions
if (scope->type != Scope::eFunction)
if (scope->type != Scope::eFunction || !scope->function)
continue;
const Token *tok = scope->classDef;
// skip any qualification
while (Token::Match(tok->tokAt(-2), "%type% ::"))
tok = tok->tokAt(-2);
const Token *tok = scope->function->tokenDef;
// have we reached a function that returns a const char *
if (Token::simpleMatch(tok->tokAt(-3), "const char *")) {
for (const Token *tok2 = scope->classStart; tok2 && tok2 != scope->classEnd; tok2 = tok2->next()) {
// return..
if (Token::Match(tok2, "return %var% . c_str ( ) ;")) {
// is the returned variable a local variable?
const unsigned int varid = tok2->next()->varId();
const Variable *var = symbolDatabase->getVariableFromVarId(varid);
if (var && var->isLocal() && !var->isStatic()) {
// report error..
errorReturnAutocstr(tok2);
}
}
// return pointer to temporary..
else if (returnTemporary(tok2)) {
if (returnTemporary(tok2)) {
// report error..
errorReturnTempPointer(tok2);
}
@ -351,11 +340,6 @@ void CheckAutoVariables::returncstr()
}
}
void CheckAutoVariables::errorReturnAutocstr(const Token *tok)
{
reportError(tok, Severity::error, "returnAutocstr", "Returning pointer to auto variable");
}
void CheckAutoVariables::errorReturnTempPointer(const Token *tok)
{
reportError(tok, Severity::error, "returnTempPointer", "Returning pointer to temporary");

5
lib/checkautovariables.h
View File

@ -66,7 +66,8 @@ public:
void returncstr();
private:
bool errorAv(const Token* left, const Token* right);
bool isRefArg(unsigned int varId);
bool isPtrArg(unsigned int varId);
bool isAutoVar(unsigned int varId);
bool isAutoVarArray(unsigned int varId);
@ -82,7 +83,6 @@ private:
void errorAutoVariableAssignment(const Token *tok, bool inconclusive);
void errorReturnReference(const Token *tok);
void errorReturnTempReference(const Token *tok);
void errorReturnAutocstr(const Token *tok);
void errorReturnTempPointer(const Token *tok);
void errorInvalidDeallocation(const Token *tok);
void errorReturnAddressOfFunctionParameter(const Token *tok, const std::string &varname);
@ -94,7 +94,6 @@ private:
c.errorReturnPointerToLocalArray(0);
c.errorReturnReference(0);
c.errorReturnTempReference(0);
c.errorReturnAutocstr(0);
c.errorReturnTempPointer(0);
c.errorInvalidDeallocation(0);
c.errorReturnAddressOfFunctionParameter(0, "parameter");

2
lib/checkother.cpp
View File

@ -328,7 +328,7 @@ void CheckOther::cstyleCastError(const Token *tok)
static std::string analyzeType(const Token* tok)
{
if (tok->str() == "double"){
if (tok->str() == "double") {
if (tok->isLong())
return "long double";
else

66
test/testautovariables.cpp
View File

@ -47,7 +47,7 @@ private:
tokenizer.tokenize(istr, "test.cpp");
CheckAutoVariables checkAutoVariables(&tokenizer, &settings, this);
checkAutoVariables.runChecks(&tokenizer, &settings, this);
checkAutoVariables.returnReference();
tokenizer.simplifyTokenList();
@ -65,6 +65,8 @@ private:
TEST_CASE(testautovar5); // ticket #2926
TEST_CASE(testautovar6); // ticket #2931
TEST_CASE(testautovar7); // ticket #3066
TEST_CASE(testautovar8);
TEST_CASE(testautovar9);
TEST_CASE(testautovar_array1);
TEST_CASE(testautovar_array2);
TEST_CASE(testautovar_return1);
@ -87,8 +89,7 @@ private:
TEST_CASE(returnReference5);
// return c_str()..
TEST_CASE(returncstr1);
TEST_CASE(returncstr2);
TEST_CASE(returncstr);
// global namespace
TEST_CASE(testglobalnamespace);
@ -213,6 +214,27 @@ private:
ASSERT_EQUALS("", errout.str());
}
void testautovar8() {
check("void foo(int*& p) {\n"
" int i = 0;\n"
" p = &i;\n"
"}", false);
ASSERT_EQUALS("[test.cpp:3]: (error) Assigning address of local auto-variable to a function parameter.\n", errout.str());
}
void testautovar9() {
check("struct FN {int i;};\n"
"struct FP {FN* f};\n"
"void foo(int*& p, FN* p_fp) {\n"
" FN fn;\n"
" FP fp;\n"
" p = &fn.i;\n"
" p = &p_fp->i;\n"
" p = &fp.f->i;\n"
"}", false);
ASSERT_EQUALS("[test.cpp:6]: (error) Assigning address of local auto-variable to a function parameter.\n", errout.str());
}
void testautovar_array1() {
check("void func1(int* arr[2])\n"
"{\n"
@ -498,21 +520,7 @@ private:
ASSERT_EQUALS("", errout.str());
}
void returncstr1() {
check("const char *foo()\n"
"{\n"
" std::string s;\n"
" return s.c_str();\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (error) Returning pointer to auto variable\n", errout.str());
check("const char *Foo::f()\n"
"{\n"
" std::string s;\n"
" return s.c_str();\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (error) Returning pointer to auto variable\n", errout.str());
void returncstr() {
check("std::string hello()\n"
"{\n"
" return \"hello\";\n"
@ -523,28 +531,6 @@ private:
" return hello().c_str();\n"
"}\n");
ASSERT_EQUALS("[test.cpp:8]: (error) Returning pointer to temporary\n", errout.str());
}
void returncstr2() {
check("class Fred {\n"
" const char *foo();\n"
"};\n"
"const char *Fred::foo()\n"
"{\n"
" std::string s;\n"
" return s.c_str();\n"
"}\n");
ASSERT_EQUALS("[test.cpp:7]: (error) Returning pointer to auto variable\n", errout.str());
check("class Fred {\n"
" const char *foo();\n"
"};\n"
"const char *Foo::f()\n"
"{\n"
" std::string s;\n"
" return s.c_str();\n"
"}\n");
ASSERT_EQUALS("[test.cpp:7]: (error) Returning pointer to auto variable\n", errout.str());
check("class Fred {\n"
" std::string hello();\n"

4
test/testsimplifytokens.cpp
View File

@ -2857,7 +2857,7 @@ private:
"9: c ( ) ;\n"
"10: }\n";
ASSERT_EQUALS(expect, tokenizer.tokens()->stringifyList(""));
ASSERT_EQUALS(expect, tokenizer.tokens()->stringifyList());
}
{
@ -2893,7 +2893,7 @@ private:
"8: d ( ) ; }\n"
"9: }\n";
ASSERT_EQUALS(expect, tokenizer.tokens()->stringifyList(""));
ASSERT_EQUALS(expect, tokenizer.tokens()->stringifyList());
}
{

9
test/teststl.cpp
View File

@ -1498,6 +1498,15 @@ private:
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Dangerous usage of c_str(). The returned value by c_str() is invalid after this call.\n", errout.str());
check("class Foo {\n"
" const char *f();\n"
"};\n"
"const char *Foo::f() {\n"
" std::string s;\n"
" return s.c_str();\n"
"}");
ASSERT_EQUALS("[test.cpp:6]: (error) Dangerous usage of c_str(). The returned value by c_str() is invalid after this call.\n", errout.str());
check("const char* foo() {\n"
" static std::string text;\n"
" text = \"hello world\n\";\n"