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

Take symbol database into use or improve its usage in some checks.

This commit is contained in:
PKEuS 2011-12-09 23:28:10 +02:00 committed by Reijo Tomperi
parent 9b685ba3c3
commit 91a0a071d0
5 changed files with 150 additions and 148 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

102
lib/checkbufferoverrun.cpp
View File

@ -1747,56 +1747,62 @@ void CheckBufferOverrun::checkBufferAllocatedWithStrlen()
void CheckBufferOverrun::checkInsecureCmdLineArgs()
{
const char pattern[] = "main ( int %var% , char *";
for (const Token *tok = Token::findmatch(_tokenizer->tokens(), pattern); tok; tok = Token::findmatch(tok->next(),pattern)) {
// Get the name of the argv variable
unsigned int varid = 0;
if (Token::Match(tok, "main ( int %var% , char * %var% [ ] ,|)")) {
varid = tok->tokAt(7)->varId();
} else if (Token::Match(tok, "main ( int %var% , char * * %var% ,|)")) {
varid = tok->tokAt(8)->varId();
const SymbolDatabase* const symbolDatabase = _tokenizer->getSymbolDatabase();
for (std::list<Scope>::const_iterator i = symbolDatabase->scopeList.begin(); i != symbolDatabase->scopeList.end(); ++i) {
for (std::list<Function>::const_iterator j = i->functionList.begin(); j != i->functionList.end(); ++j) {
const Token* tok = j->token;
// Get the name of the argv variable
unsigned int varid = 0;
if (Token::Match(tok, "main ( int %var% , char * %var% [ ] ,|)")) {
varid = tok->tokAt(7)->varId();
} else if (Token::Match(tok, "main ( int %var% , char * * %var% ,|)")) {
varid = tok->tokAt(8)->varId();
}
if (varid == 0)
continue;
// Jump to the opening curly brace
tok = tok->next()->link();
if (!Token::simpleMatch(tok, ") {"))
continue;
tok = tok->next();
// Search within main() for possible buffer overruns involving argv
int indentlevel = -1;
for (; tok; tok = tok->next()) {
if (tok->str() == "{") {
++indentlevel;
}
else if (tok->str() == "}") {
--indentlevel;
if (indentlevel < 0)
return;
}
// If argv is modified or tested, its size may be being limited properly
if (tok->varId() == varid)
break;
// Match common patterns that can result in a buffer overrun
// e.g. strcpy(buffer, argv[0])
if (Token::Match(tok, "strcpy|strcat ( %var% , * %varid%", varid) ||
Token::Match(tok, "strcpy|strcat ( %var% , %varid% [", varid)) {
cmdLineArgsError(tok);
} else if (Token::Match(tok, "sprintf ( %var% , %str% , %varid% [", varid) &&
tok->strAt(4).find("%s") != std::string::npos) {
cmdLineArgsError(tok);
} else if (Token::Match(tok, "sprintf ( %var% , %str% , * %varid%", varid) &&
tok->strAt(4).find("%s") != std::string::npos) {
cmdLineArgsError(tok);
}
}
}
if (varid == 0)
continue;
// Jump to the opening curly brace
tok = tok->next()->link();
if (!Token::simpleMatch(tok, ") {"))
continue;
tok = tok->next();
// Search within main() for possible buffer overruns involving argv
int indentlevel = -1;
for (; tok; tok = tok->next()) {
if (tok->str() == "{") {
++indentlevel;
}
else if (tok->str() == "}") {
--indentlevel;
if (indentlevel < 0)
return;
}
// If argv is modified or tested, its size may be being limited properly
if (tok->varId() == varid)
break;
// Match common patterns that can result in a buffer overrun
// e.g. strcpy(buffer, argv[0])
if (Token::Match(tok, "strcpy|strcat ( %var% , * %varid%", varid) ||
Token::Match(tok, "strcpy|strcat ( %var% , %varid% [", varid)) {
cmdLineArgsError(tok);
} else if (Token::Match(tok, "sprintf ( %var% , %str% , %varid% [", varid) &&
tok->strAt(4).find("%s") != std::string::npos) {
cmdLineArgsError(tok);
} else if (Token::Match(tok, "sprintf ( %var% , %str% , * %varid%", varid) &&
tok->strAt(4).find("%s") != std::string::npos) {
cmdLineArgsError(tok);
}
}
}
}
//---------------------------------------------------------------------------

22
lib/checknullpointer.cpp
View File

@ -288,13 +288,15 @@ bool CheckNullPointer::isPointer(const unsigned int varid)
void CheckNullPointer::nullPointerAfterLoop()
{
const SymbolDatabase* const symbolDatabase = _tokenizer->getSymbolDatabase();
// Locate insufficient null-pointer handling after loop
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
for (std::list<Scope>::const_iterator i = symbolDatabase->scopeList.begin(); i != symbolDatabase->scopeList.end(); ++i) {
// only interested in while ( %var% )
// TODO: Aren't there false negatives. Shouldn't other loops be handled such as:
// - while ( ! %var% )
// - while ( %var% && .. )
if (! Token::Match(tok, "while ( %var% )|&&"))
const Token* const tok = i->classDef;
if (i->type != Scope::eWhile || !Token::Match(tok, "while ( %var% )|&&"))
continue;
// Get variable id for the loop variable
@ -363,15 +365,18 @@ void CheckNullPointer::nullPointerAfterLoop()
void CheckNullPointer::nullPointerLinkedList()
{
const SymbolDatabase* const symbolDatabase = _tokenizer->getSymbolDatabase();
// looping through items in a linked list in a inner loop.
// Here is an example:
// for (const Token *tok = tokens; tok; tok = tok->next) {
// if (tok->str() == "hello")
// tok = tok->next; // <- tok might become a null pointer!
// }
for (const Token *tok1 = _tokenizer->tokens(); tok1; tok1 = tok1->next()) {
// search for a "for" token..
if (tok1->str() != "for")
for (std::list<Scope>::const_iterator i = symbolDatabase->scopeList.begin(); i != symbolDatabase->scopeList.end(); ++i) {
const Token* const tok1 = i->classDef;
// search for a "for" scope..
if (i->type != Scope::eFor || !tok1)
continue;
// is there any dereferencing occurring in the for statement
@ -624,11 +629,12 @@ void CheckNullPointer::nullPointerByDeRefAndChec()
// Dereferencing a pointer and then checking if it's NULL..
// This check will first scan for the check. And then scan backwards
// from the check, searching for dereferencing.
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
for (std::list<Scope>::const_iterator i = symbolDatabase->scopeList.begin(); i != symbolDatabase->scopeList.end(); ++i) {
// TODO: false negatives.
// - logical operators
// - while
if (tok->str() == "if" && Token::Match(tok->previous(), "; if ( !| %var% )|%oror%|&&")) {
const Token* const tok = i->classDef;
if (i->type == Scope::eIf && tok && Token::Match(tok->previous(), "; if ( !| %var% )|%oror%|&&")) {
const Token * vartok = tok->tokAt(2);
if (vartok->str() == "!")
vartok = vartok->next();

129
lib/checkother.cpp
View File

@ -249,12 +249,15 @@ void CheckOther::checkSuspiciousSemicolon()
if (!_settings->inconclusive || !_settings->isEnabled("style"))
return;
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
// Look for "if(); {}", "for(); {}" or "while(); {}"
if (Token::Match(tok, "if|for|while (")) {
const Token *end = tok->next()->link();
if (!end)
const SymbolDatabase* const symbolDatabase = _tokenizer->getSymbolDatabase();
// Look for "if(); {}", "for(); {}" or "while(); {}"
for (std::list<Scope>::const_iterator i = symbolDatabase->scopeList.begin(); i != symbolDatabase->scopeList.end(); ++i) {
if (i->type == Scope::eIf || i->type == Scope::eElse || i->type == Scope::eElseIf || i->type == Scope::eWhile || i->type == Scope::eFor) {
const Token *tok = Token::findsimplematch(i->classDef, "(", i->classEnd);
if (!tok)
continue;
const Token *end = tok->link();
// Ensure the semicolon is at the same line number as the if/for/while statement
// and the {..} block follows it without an extra empty line.
@ -580,13 +583,14 @@ void CheckOther::checkSwitchCaseFallThrough()
if (!(_settings->isEnabled("style") && _settings->experimental))
return;
const char switchPattern[] = "switch (";
const SymbolDatabase* const symbolDatabase = _tokenizer->getSymbolDatabase();
const char breakPattern[] = "break|continue|return|exit|goto|throw";
// Find the beginning of a switch. E.g.:
// switch (var) { ...
const Token *tok = Token::findmatch(_tokenizer->tokens(), switchPattern);
while (tok) {
for (std::list<Scope>::const_iterator i = symbolDatabase->scopeList.begin(); i != symbolDatabase->scopeList.end(); ++i) {
const Token* const tok = i->classDef;
if (i->type != Scope::eSwitch || !tok) // Find the beginning of a switch
continue;
// Check the contents of the switch statement
std::stack<std::pair<Token *, bool> > ifnest;
@ -638,7 +642,7 @@ void CheckOther::checkSwitchCaseFallThrough()
}
loopnest.push(tok2->link());
justbreak = false;
} else if (Token::Match(tok2, switchPattern)) {
} else if (Token::simpleMatch(tok2, "switch (")) {
// skip over nested switch, we'll come to that soon
tok2 = tok2->next()->link()->next()->link();
} else if (Token::Match(tok2, breakPattern)) {
@ -699,8 +703,6 @@ void CheckOther::checkSwitchCaseFallThrough()
}
}
tok = Token::findmatch(tok->next(), switchPattern);
}
}
@ -2290,54 +2292,45 @@ void CheckOther::checkDuplicateIf()
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
std::list<Scope>::const_iterator scope;
for (scope = symbolDatabase->scopeList.begin(); scope != symbolDatabase->scopeList.end(); ++scope) {
// only check functions
if (scope->type != Scope::eFunction)
for (std::list<Scope>::const_iterator scope = symbolDatabase->scopeList.begin(); scope != symbolDatabase->scopeList.end(); ++scope) {
const Token* const tok = scope->classDef;
// only check if statements
if (scope->type != Scope::eIf || !tok)
continue;
// check all the code in the function for if (...) and else if (...) statements
for (const Token *tok = scope->classStart; tok && tok != scope->classStart->link(); tok = tok->next()) {
if (Token::simpleMatch(tok, "if (") && tok->strAt(-1) != "else" &&
Token::simpleMatch(tok->next()->link(), ") {")) {
std::map<std::string, const Token*> expressionMap;
std::map<std::string, const Token*> expressionMap;
// get the expression from the token stream
std::string expression = stringifyTokens(tok->tokAt(2), tok->next()->link()->previous());
// get the expression from the token stream
std::string expression = stringifyTokens(tok->tokAt(2), tok->next()->link()->previous());
// save the expression and its location
expressionMap.insert(std::make_pair(expression, tok));
// save the expression and its location
expressionMap.insert(std::make_pair(expression, tok));
// find the next else if (...) statement
const Token *tok1 = tok->next()->link()->next()->link();
// find the next else if (...) statement
const Token *tok1 = tok->next()->link()->next()->link();
// check all the else if (...) statements
while (Token::simpleMatch(tok1, "} else if (") &&
Token::simpleMatch(tok1->linkAt(3), ") {")) {
// get the expression from the token stream
expression = stringifyTokens(tok1->tokAt(4), tok1->linkAt(3)->previous());
// check all the else if (...) statements
while (Token::simpleMatch(tok1, "} else if (") &&
Token::simpleMatch(tok1->linkAt(3), ") {")) {
// get the expression from the token stream
expression = stringifyTokens(tok1->tokAt(4), tok1->linkAt(3)->previous());
// try to look up the expression to check for duplicates
std::map<std::string, const Token *>::iterator it = expressionMap.find(expression);
// try to look up the expression to check for duplicates
std::map<std::string, const Token *>::iterator it = expressionMap.find(expression);
// found a duplicate
if (it != expressionMap.end()) {
// check for expressions that have side effects and ignore them
if (!expressionHasSideEffects(tok1->tokAt(4), tok1->linkAt(3)->previous()))
duplicateIfError(it->second, tok1->next());
}
// not a duplicate expression so save it and its location
else
expressionMap.insert(std::make_pair(expression, tok1->next()));
// find the next else if (...) statement
tok1 = tok1->linkAt(3)->next()->link();
}
tok = tok->next()->link()->next();
// found a duplicate
if (it != expressionMap.end()) {
// check for expressions that have side effects and ignore them
if (!expressionHasSideEffects(tok1->tokAt(4), tok1->linkAt(3)->previous()))
duplicateIfError(it->second, tok1->next());
}
// not a duplicate expression so save it and its location
else
expressionMap.insert(std::make_pair(expression, tok1->next()));
// find the next else if (...) statement
tok1 = tok1->linkAt(3)->next()->link();
}
}
}
@ -2371,30 +2364,24 @@ void CheckOther::checkDuplicateBranch()
std::list<Scope>::const_iterator scope;
for (scope = symbolDatabase->scopeList.begin(); scope != symbolDatabase->scopeList.end(); ++scope) {
// only check functions
if (scope->type != Scope::eFunction)
continue;
const Token* const tok = scope->classDef;
// check all the code in the function for if (..) else
for (const Token *tok = scope->classStart; tok && tok != scope->classStart->link(); tok = tok->next()) {
if (Token::simpleMatch(tok, "if (") && tok->strAt(-1) != "else" &&
Token::simpleMatch(tok->next()->link(), ") {") &&
Token::simpleMatch(tok->next()->link()->next()->link(), "} else {")) {
// save if branch code
std::string branch1 = stringifyTokens(tok->next()->link()->tokAt(2), tok->next()->link()->next()->link()->previous());
if (scope->type == Scope::eIf && tok && tok->next() &&
Token::simpleMatch(tok->next()->link(), ") {") &&
Token::simpleMatch(tok->next()->link()->next()->link(), "} else {")) {
// save if branch code
std::string branch1 = stringifyTokens(tok->next()->link()->tokAt(2), tok->next()->link()->next()->link()->previous());
// find else branch
const Token *tok1 = tok->next()->link()->next()->link();
// find else branch
const Token *tok1 = tok->next()->link()->next()->link();
// save else branch code
std::string branch2 = stringifyTokens(tok1->tokAt(3), tok1->linkAt(2)->previous());
// save else branch code
std::string branch2 = stringifyTokens(tok1->tokAt(3), tok1->linkAt(2)->previous());
// check for duplicates
if (branch1 == branch2)
duplicateBranchError(tok, tok1->tokAt(2));
tok = tok->next()->link()->next();
}
// check for duplicates
if (branch1 == branch2)
duplicateBranchError(tok, tok1->tokAt(2));
}
}
}

27
lib/checkstl.cpp
View File

@ -238,10 +238,13 @@ void CheckStl::mismatchingContainers()
void CheckStl::stlOutOfBounds()
{
const SymbolDatabase* const symbolDatabase = _tokenizer->getSymbolDatabase();
// Scan through all tokens..
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
for (std::list<Scope>::const_iterator i = symbolDatabase->scopeList.begin(); i != symbolDatabase->scopeList.end(); ++i) {
const Token* const tok = i->classDef;
// only interested in "for" loops
if (!Token::simpleMatch(tok, "for ("))
if (i->type != Scope::eFor || !tok)
continue;
// check if the for loop condition is wrong
@ -432,13 +435,16 @@ private:
void CheckStl::erase()
{
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
if (Token::simpleMatch(tok, "for (")) {
const SymbolDatabase* const symbolDatabase = _tokenizer->getSymbolDatabase();
for (std::list<Scope>::const_iterator i = symbolDatabase->scopeList.begin(); i != symbolDatabase->scopeList.end(); ++i) {
const Token* const tok = i->classDef;
if (tok && i->type == Scope::eFor) {
for (const Token *tok2 = tok->tokAt(2); tok2; tok2 = tok2->next()) {
if (tok2->str() == ";") {
if (Token::Match(tok2, "; %var% !=")) {
// Get declaration token for var..
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
const Variable *variableInfo = symbolDatabase->getVariableFromVarId(tok2->next()->varId());
const Token *decltok = variableInfo ? variableInfo->typeEndToken() : NULL;
@ -463,7 +469,7 @@ void CheckStl::erase()
}
}
if (Token::Match(tok, "while ( %var% !=")) {
else if (i->type == Scope::eWhile && Token::Match(tok, "while ( %var% !=")) {
const unsigned int varid = tok->tokAt(2)->varId();
if (varid > 0 && Token::findmatch(_tokenizer->tokens(), "> :: iterator %varid%", varid))
EraseCheckLoop::checkScope(this, tok->tokAt(2));
@ -712,7 +718,12 @@ void CheckStl::if_find()
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
for (std::list<Scope>::const_iterator i = symbolDatabase->scopeList.begin(); i != symbolDatabase->scopeList.end(); ++i) {
if (i->type != Scope::eIf && i->type != Scope::eElseIf)
continue;
const Token* tok = i->classDef;
if (Token::Match(tok, "if ( !| %var% . find ( %any% ) )")) {
// goto %var%
tok = tok->tokAt(2);
@ -738,7 +749,7 @@ void CheckStl::if_find()
}
}
if (Token::Match(tok, "if ( !| std :: find|find_if (")) {
else if (Token::Match(tok, "if ( !| std :: find|find_if (")) {
// goto '(' for the find
tok = tok->tokAt(4);
if (tok->isName())

18
test/teststl.cpp
View File

@ -442,22 +442,14 @@ private:
void erase1() {
check("void f()\n"
"{\n"
" for (it = foo.begin(); it != foo.end(); ++it)\n"
" {\n"
" for (it = foo.begin(); it != foo.end(); ++it) {\n"
" foo.erase(it);\n"
" }\n"
" for (it = foo.begin(); it != foo.end(); ++it) {\n"
" foo.erase(it);\n"
" }\n"
"}\n");
ASSERT_EQUALS("[test.cpp:5]: (error) Dangerous iterator usage after erase()-method.\n", errout.str());
check("for (it = foo.begin(); it != foo.end(); ++it)\n"
"{\n"
" foo.erase(it);\n"
"}\n"
"for (it = foo.begin(); it != foo.end(); ++it)\n"
"{\n"
" foo.erase(it);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3]: (error) Dangerous iterator usage after erase()-method.\n"
ASSERT_EQUALS("[test.cpp:4]: (error) Dangerous iterator usage after erase()-method.\n"
"[test.cpp:7]: (error) Dangerous iterator usage after erase()-method.\n", errout.str());
check("void f(std::list<int> &ints)\n"