diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 435cf6a91..035087ead 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -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::const_iterator i = symbolDatabase->scopeList.begin(); i != symbolDatabase->scopeList.end(); ++i) { + for (std::list::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); - } - - } - } } //--------------------------------------------------------------------------- diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp index 24842b3fd..f3789061a 100644 --- a/lib/checknullpointer.cpp +++ b/lib/checknullpointer.cpp @@ -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::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::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::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(); diff --git a/lib/checkother.cpp b/lib/checkother.cpp index ab12a39c1..8afd2b5c6 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -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::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::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 > 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::const_iterator scope; - - for (scope = symbolDatabase->scopeList.begin(); scope != symbolDatabase->scopeList.end(); ++scope) { - // only check functions - if (scope->type != Scope::eFunction) + for (std::list::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 expressionMap; + std::map 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::iterator it = expressionMap.find(expression); + // try to look up the expression to check for duplicates + std::map::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::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)); } } } diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index ff33f3485..b3de44a0f 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -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::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::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::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()) diff --git a/test/teststl.cpp b/test/teststl.cpp index 5f13c5971..0e1826202 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -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 &ints)\n"