From 0f4cdc9e87efbf01ac20254ae6264c7c27f76101 Mon Sep 17 00:00:00 2001 From: Edoardo Prezioso Date: Sun, 25 Nov 2012 14:55:31 +0100 Subject: [PATCH] More CheckNullPointer speed ups, related to #4266. --- lib/checknullpointer.cpp | 328 ++++++++++++++++++++------------------- 1 file changed, 168 insertions(+), 160 deletions(-) diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp index bbc6937f7..6e9d731d8 100644 --- a/lib/checknullpointer.cpp +++ b/lib/checknullpointer.cpp @@ -534,183 +534,189 @@ void CheckNullPointer::nullPointerLinkedList() void CheckNullPointer::nullPointerStructByDeRefAndChec() { // Dereferencing a struct pointer and then checking if it's NULL.. + const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase(); // skipvar: don't check vars that has been tested against null already std::set skipvar; skipvar.insert(0); - // Scan through all tokens - for (const Token *tok1 = _tokenizer->tokens(); tok1; tok1 = tok1->next()) { - // Checking if some pointer is null. - // then add the pointer to skipvar => is it known that it isn't NULL - if (Token::Match(tok1, "if|while ( !| %var% )")) { - tok1 = tok1->tokAt(2); - if (tok1->str() == "!") - tok1 = tok1->next(); - skipvar.insert(tok1->varId()); + const std::size_t functions = symbolDatabase->functionScopes.size(); + for (std::size_t i = 0; i < functions; ++i) { + const Scope * scope = symbolDatabase->functionScopes[i]; + if (scope->function == 0 || !scope->function->hasBody) // We only look for functions with a body continue; - } else if (Token::Match(tok1, "( ! %var% %oror%") || - Token::Match(tok1, "( %var% &&")) { - // TODO: there are false negatives caused by this. The - // variable should be removed from skipvar after the - // condition - tok1 = tok1->next(); - if (tok1->str() == "!") + for (const Token *tok1 = scope->classStart; tok1 != scope->classEnd; tok1 = tok1->next()) { + // Checking if some pointer is null. + // then add the pointer to skipvar => is it known that it isn't NULL + if (Token::Match(tok1, "if|while ( !| %var% )")) { + tok1 = tok1->tokAt(2); + if (tok1->str() == "!") + tok1 = tok1->next(); + skipvar.insert(tok1->varId()); + continue; + } else if (Token::Match(tok1, "( ! %var% %oror%") || + Token::Match(tok1, "( %var% &&")) { + // TODO: there are false negatives caused by this. The + // variable should be removed from skipvar after the + // condition tok1 = tok1->next(); - skipvar.insert(tok1->varId()); - continue; - } + if (tok1->str() == "!") + tok1 = tok1->next(); + skipvar.insert(tok1->varId()); + continue; + } - bool inconclusive = false; + bool inconclusive = false; - /** - * @todo There are lots of false negatives here. A dereference - * is only investigated if a few specific conditions are met. - */ + /** + * @todo There are lots of false negatives here. A dereference + * is only investigated if a few specific conditions are met. + */ - // dereference in assignment - if (Token::Match(tok1, "[;{}] %var% . %var%")) { - tok1 = tok1->next(); - if (tok1->strAt(3) == "(") { - if (!_settings->inconclusive) + // dereference in assignment + if (Token::Match(tok1, "[;{}] %var% . %var%")) { + tok1 = tok1->next(); + if (tok1->strAt(3) == "(") { + if (!_settings->inconclusive) + continue; + inconclusive = true; + } + } + + // dereference in assignment + else if (Token::Match(tok1, "[{};] %var% = %var% . %var%")) { + if (tok1->strAt(1) == tok1->strAt(3)) continue; - inconclusive = true; + tok1 = tok1->tokAt(3); } - } - // dereference in assignment - else if (Token::Match(tok1, "[{};] %var% = %var% . %var%")) { - if (tok1->strAt(1) == tok1->strAt(3)) + // dereference in condition + else if (Token::Match(tok1, "if ( !| %var% .")) { + tok1 = tok1->tokAt(2); + if (tok1->str() == "!") + tok1 = tok1->next(); + } + + // dereference in function call (but not sizeof|decltype) + else if ((Token::Match(tok1->tokAt(-2), "%var% ( %var% . %var%") && !Token::Match(tok1->tokAt(-2), "sizeof|decltype ( %var% . %var%")) || + Token::Match(tok1->previous(), ", %var% . %var%")) { + // Is the function return value taken by the pointer? + bool assignment = false; + const unsigned int varid1(tok1->varId()); + if (varid1 == 0) + continue; + const Token *tok2 = tok1->previous(); + while (tok2 && !Token::Match(tok2, "[;{}]")) { + if (Token::Match(tok2, "%varid% =", varid1)) { + assignment = true; + break; + } + tok2 = tok2->previous(); + } + if (assignment) + continue; + + // Is the dereference checked with a previous && + bool checked = false; + for (tok2 = tok1->tokAt(-2); tok2; tok2 = tok2->previous()) { + if (Token::Match(tok2, "[,(;{}]")) + break; + else if (tok2->str() == ")") + tok2 = tok2->link(); + else if (Token::Match(tok2, "%varid% &&", varid1)) { + checked = true; + break; + } + } + if (checked) + continue; + } + + // Goto next token + else { continue; - tok1 = tok1->tokAt(3); - } + } - // dereference in condition - else if (Token::Match(tok1, "if ( !| %var% .")) { - tok1 = tok1->tokAt(2); - if (tok1->str() == "!") - tok1 = tok1->next(); - } - - // dereference in function call (but not sizeof|decltype) - else if ((Token::Match(tok1->tokAt(-2), "%var% ( %var% . %var%") && !Token::Match(tok1->tokAt(-2), "sizeof|decltype ( %var% . %var%")) || - Token::Match(tok1->previous(), ", %var% . %var%")) { - // Is the function return value taken by the pointer? - bool assignment = false; + // struct dereference was found - investigate if it is later + // checked that it is not NULL const unsigned int varid1(tok1->varId()); - if (varid1 == 0) + if (skipvar.find(varid1) != skipvar.end()) continue; - const Token *tok2 = tok1->previous(); - while (tok2 && !Token::Match(tok2, "[;{}]")) { - if (Token::Match(tok2, "%varid% =", varid1)) { - assignment = true; + + // name of struct pointer + const std::string& varname(tok1->str()); + + // is pointer local? + bool isLocal = false; + const Variable * var = _tokenizer->getSymbolDatabase()->getVariableFromVarId(tok1->varId()); + if (!var) + continue; + if (var->isLocal() || var->isArgument()) + isLocal = true; + + // member function may or may not nullify the pointer if it's global (#2647) + if (!isLocal) { + const Token *tok2 = tok1; + while (Token::Match(tok2, "%var% .")) + tok2 = tok2->tokAt(2); + if (Token::Match(tok2,"%var% (")) + continue; + } + + // count { and } using tok2 + const Token* const end2 = tok1->scope()->classEnd; + for (const Token *tok2 = tok1->tokAt(3); tok2 != end2; tok2 = tok2->next()) { + bool unknown = false; + + // label / ?: + if (tok2->str() == ":") + break; + + // function call.. + else if (Token::Match(tok2, "[;{}] %var% (") && CanFunctionAssignPointer(tok2->next(), varid1, unknown)) { + if (!_settings->inconclusive || !unknown) + break; + inconclusive = true; + } + + // Reassignment of the struct + else if (tok2->varId() == varid1) { + if (tok2->next()->str() == "=") { + // Avoid false positives when there is 'else if' + // TODO: can this be handled better? + if (tok1->strAt(-2) == "if") + skipvar.insert(varid1); + break; + } + if (Token::Match(tok2->tokAt(-2), "[,(] &")) + break; + } + + // Loop.. + /** @todo don't bail out if the variable is not used in the loop */ + else if (tok2->str() == "do") + break; + + // return/break at base level => stop checking + else if (tok2->scope()->classEnd == end2 && (tok2->str() == "return" || tok2->str() == "break")) + break; + + // Function call: If the pointer is not a local variable it + // might be changed by the call. + else if (Token::Match(tok2, "[;{}] %var% (") && + Token::simpleMatch(tok2->linkAt(2), ") ;") && !isLocal) { break; } - tok2 = tok2->previous(); - } - if (assignment) - continue; - // Is the dereference checked with a previous && - bool checked = false; - for (tok2 = tok1->tokAt(-2); tok2; tok2 = tok2->previous()) { - if (Token::Match(tok2, "[,(;{}]")) - break; - else if (tok2->str() == ")") - tok2 = tok2->link(); - else if (Token::Match(tok2, "%varid% &&", varid1)) { - checked = true; + // Check if pointer is null. + // TODO: false negatives for "if (!p || .." + else if (!tok2->isExpandedMacro() && Token::Match(tok2, "if ( !| %varid% )|&&", varid1)) { + // Is this variable a pointer? + if (var->isPointer()) + nullPointerError(tok1, varname, tok2, inconclusive); break; } } - if (checked) - continue; - } - - // Goto next token - else { - continue; - } - - // struct dereference was found - investigate if it is later - // checked that it is not NULL - const unsigned int varid1(tok1->varId()); - if (skipvar.find(varid1) != skipvar.end()) - continue; - - // name of struct pointer - const std::string& varname(tok1->str()); - - // is pointer local? - bool isLocal = false; - const Variable * var = _tokenizer->getSymbolDatabase()->getVariableFromVarId(tok1->varId()); - if (!var) - continue; - if (var->isLocal() || var->isArgument()) - isLocal = true; - - // member function may or may not nullify the pointer if it's global (#2647) - if (!isLocal) { - const Token *tok2 = tok1; - while (Token::Match(tok2, "%var% .")) - tok2 = tok2->tokAt(2); - if (Token::Match(tok2,"%var% (")) - continue; - } - - // count { and } using tok2 - const Token* const end2 = tok1->scope()->classEnd; - for (const Token *tok2 = tok1->tokAt(3); tok2 != end2; tok2 = tok2->next()) { - bool unknown = false; - - // label / ?: - if (tok2->str() == ":") - break; - - // function call.. - else if (Token::Match(tok2, "[;{}] %var% (") && CanFunctionAssignPointer(tok2->next(), varid1, unknown)) { - if (!_settings->inconclusive || !unknown) - break; - inconclusive = true; - } - - // Reassignment of the struct - else if (tok2->varId() == varid1) { - if (tok2->next()->str() == "=") { - // Avoid false positives when there is 'else if' - // TODO: can this be handled better? - if (tok1->strAt(-2) == "if") - skipvar.insert(varid1); - break; - } - if (Token::Match(tok2->tokAt(-2), "[,(] &")) - break; - } - - // Loop.. - /** @todo don't bail out if the variable is not used in the loop */ - else if (tok2->str() == "do") - break; - - // return/break at base level => stop checking - else if (tok2->scope()->classEnd == end2 && (tok2->str() == "return" || tok2->str() == "break")) - break; - - // Function call: If the pointer is not a local variable it - // might be changed by the call. - else if (Token::Match(tok2, "[;{}] %var% (") && - Token::simpleMatch(tok2->linkAt(2), ") ;") && !isLocal) { - break; - } - - // Check if pointer is null. - // TODO: false negatives for "if (!p || .." - else if (!tok2->isExpandedMacro() && Token::Match(tok2, "if ( !| %varid% )|&&", varid1)) { - // Is this variable a pointer? - if (var->isPointer()) - nullPointerError(tok1, varname, tok2, inconclusive); - break; - } } } } @@ -1099,16 +1105,18 @@ void CheckNullPointer::nullConstantDereference() { const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase(); - for (std::list::const_iterator i = symbolDatabase->scopeList.begin(); i != symbolDatabase->scopeList.end(); ++i) { - if (i->type != Scope::eFunction || !i->classStart) + const std::size_t functions = symbolDatabase->functionScopes.size(); + for (std::size_t i = 0; i < functions; ++i) { + const Scope * scope = symbolDatabase->functionScopes[i]; + if (scope->function == 0 || !scope->function->hasBody) // We only look for functions with a body continue; - const Token *tok = i->classStart; + const Token *tok = scope->classStart; - if (i->function && (i->function->type == Function::eConstructor || i->function->type == Function::eCopyConstructor)) - tok = i->function->token; // Check initialization list + if (scope->function && (scope->function->type == Function::eConstructor || scope->function->type == Function::eCopyConstructor)) + tok = scope->function->token; // Check initialization list - for (; tok != i->classEnd; tok = tok->next()) { + for (; tok != scope->classEnd; tok = tok->next()) { if (Token::Match(tok, "sizeof|decltype|typeid (")) tok = tok->next()->link();