diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp index da50a0e64..04112a264 100644 --- a/lib/checknullpointer.cpp +++ b/lib/checknullpointer.cpp @@ -115,28 +115,31 @@ void CheckNullPointer::parseFunctionCall(const Token &tok, std::listnextArgument(); + // 1st parameter.. - if ((Token::Match(&tok, "%var% ( %var% ,|)") && tok.tokAt(2)->varId() > 0) || - (value == 0 && Token::Match(&tok, "%var% ( 0 ,|)"))) { + if ((Token::Match(firstParam, "%var% ,|)") && firstParam->varId() > 0) || + (value == 0 && Token::Match(firstParam, "0 ,|)"))) { if (functionNames1_all.find(tok.str()) != functionNames1_all.end()) - var.push_back(tok.tokAt(2)); + var.push_back(firstParam); else if (value == 0 && functionNames1_nullptr.find(tok.str()) != functionNames1_nullptr.end()) - var.push_back(tok.tokAt(2)); - else if (value != 0 && Token::simpleMatch(&tok, "fflush")) - var.push_back(tok.tokAt(2)); - else if (value == 0 && Token::Match(&tok, "snprintf|vsnprintf|fnprintf|vfnprintf") && tok.strAt(4) != "0") // Only if length is not zero - var.push_back(tok.tokAt(2)); + var.push_back(firstParam); + else if (value != 0 && tok.str() == "fflush") + var.push_back(firstParam); + else if (value == 0 && Token::Match(&tok, "snprintf|vsnprintf|fnprintf|vfnprintf") && secondParam && secondParam->str() != "0") // Only if length (second parameter) is not zero + var.push_back(firstParam); } // 2nd parameter.. - if (Token::Match(&tok, "%var% ( !!)")) { - const Token* secondParameter = tok.tokAt(2)->nextArgument(); - if (secondParameter && ((value == 0 && secondParameter->str() == "0") || (Token::Match(secondParameter, "%var%") && secondParameter->varId() > 0))) { - if (functionNames2_all.find(tok.str()) != functionNames2_all.end()) - var.push_back(secondParameter); - else if (value == 0 && functionNames2_nullptr.find(tok.str()) != functionNames2_nullptr.end()) - var.push_back(secondParameter); - } + if (secondParam && ((value == 0 && secondParam->str() == "0") || (Token::Match(secondParam, "%var%") && secondParam->varId() > 0))) { + if (functionNames2_all.find(tok.str()) != functionNames2_all.end()) + var.push_back(secondParam); + else if (value == 0 && functionNames2_nullptr.find(tok.str()) != functionNames2_nullptr.end()) + var.push_back(secondParam); } if (Token::Match(&tok, "printf|sprintf|snprintf|fprintf|fnprintf|scanf|sscanf|fscanf")) { @@ -145,25 +148,22 @@ void CheckNullPointer::parseFunctionCall(const Token &tok, std::listnextArgument(); // Find second parameter (format string) - if (formatStringTok && Token::Match(formatStringTok, "%str%")) { + formatString = firstParam->strValue(); + argListTok = secondParam; + } else if (Token::Match(&tok, "sprintf|fprintf|sscanf|fscanf")) { + const Token* formatStringTok = secondParam; // Find second parameter (format string) + if (formatStringTok && formatStringTok->str()[0] == '"') { argListTok = formatStringTok->nextArgument(); // Find third parameter (first argument of va_args) - formatString = formatStringTok->str(); + formatString = formatStringTok->strValue(); } - } else if (Token::Match(&tok, "snprintf|fnprintf ( %any%")) { - const Token* formatStringTok = tok.tokAt(2); - for (int i = 0; i < 2 && formatStringTok; i++) { + } else if (Token::Match(&tok, "snprintf|fnprintf")) { + const Token* formatStringTok = secondParam; + for (int i = 0; i < 1 && formatStringTok; i++) { formatStringTok = formatStringTok->nextArgument(); // Find third parameter (format string) } - if (formatStringTok && Token::Match(formatStringTok, "%str%")) { + if (formatStringTok && formatStringTok->str()[0] == '"') { argListTok = formatStringTok->nextArgument(); // Find fourth parameter (first argument of va_args) - formatString = formatStringTok->str(); + formatString = formatStringTok->strValue(); } } @@ -191,7 +191,7 @@ void CheckNullPointer::parseFunctionCall(const Token &tok, std::liststr() == "0") || (Token::Match(argListTok, "%var%") && argListTok->varId() > 0)) { + if ((value == 0 && argListTok->str() == "0") || (argListTok->varId() > 0)) { var.push_back(argListTok); } } @@ -1026,7 +1026,13 @@ void CheckNullPointer::nullConstantDereference() else if (Token::Match(tok, "0 [") && (tok->previous()->str() != "&" || !Token::Match(tok->next()->link()->next(), "[.(]"))) nullPointerError(tok); - else if (Token::Match(tok->previous(), "[={};] %var% (")) { + else if (Token::Match(tok->previous(), "!!. %var% (") && (tok->previous()->str() != "::" || tok->strAt(-2) == "std")) { + if (Token::Match(tok->tokAt(2), "0 )")) { + const Variable* var = symbolDatabase->getVariableFromVarId(tok->varId()); + if (var && !var->isPointer() && !var->isArray() && Token::Match(var->typeStartToken(), "const| std :: string !!::")) + nullPointerError(tok); + } + std::list var; parseFunctionCall(*tok, var, 0); @@ -1038,11 +1044,6 @@ void CheckNullPointer::nullConstantDereference() } } else if (Token::simpleMatch(tok, "std :: string ( 0 )")) nullPointerError(tok); - else if (Token::Match(tok, "%var% ( 0 )")) { - const Variable* var = symbolDatabase->getVariableFromVarId(tok->varId()); - if (var && !var->isPointer() && !var->isArray() && Token::Match(var->typeStartToken(), "const| std :: string !!::")) - nullPointerError(tok); - } unsigned int ovarid = 0; if (Token::Match(tok, "0 ==|!= %var%")) diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 17f6a9ab1..1f33c2136 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -199,14 +199,14 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti scope->access = Private; else if (tok->str() == "protected") scope->access = Protected; - else if (tok->str() == "public") + else scope->access = Public; tok = tok->tokAt(2); } // class function? - else if (tok->previous()->str() != "::" && isFunction(tok, &funcStart, &argStart)) { + else if (tok->previous()->str() != "::" && isFunction(tok, scope, &funcStart, &argStart)) { Function function; // save the function definition argument start '(' @@ -347,12 +347,12 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti } // nested class or friend function? - else if (tok->previous()->str() == "::" && isFunction(tok, &funcStart, &argStart)) { + else if (tok->previous()->str() == "::" && isFunction(tok, scope, &funcStart, &argStart)) { /** @todo check entire qualification for match */ Scope * nested = scope->findInNestedListRecursive(tok->strAt(-2)); if (nested) - addFunction(&scope, &tok, argStart); + addClassFunction(&scope, &tok, argStart); else { /** @todo handle friend functions */ } @@ -373,54 +373,23 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti const Token *argStart = 0; // function? - if (isFunction(tok, &funcStart, &argStart)) { + if (isFunction(tok, scope, &funcStart, &argStart)) { // has body? if (Token::Match(argStart->link(), ") const| {|:")) { Scope *old_scope = scope; // class function if (tok->previous() && tok->previous()->str() == "::") - addFunction(&scope, &tok, argStart); + addClassFunction(&scope, &tok, argStart); // class destructor else if (tok->previous() && tok->previous()->str() == "~" && tok->tokAt(-2) && tok->strAt(-2) == "::") - addFunction(&scope, &tok, argStart); + addClassFunction(&scope, &tok, argStart); // regular function - else { - Function function; - - // save the function definition argument start '(' - function.argDef = argStart; - - // save the access type - function.access = Public; - - // save the function name location - function.tokenDef = funcStart; - function.token = funcStart; - - function.isInline = false; - function.hasBody = true; - function.arg = function.argDef; - function.type = Function::eFunction; - - // find start of function '{' - const Token *start = tok; - while (start && start->str() != "{") - start = start->next(); - - // save start of function - function.start = start; - - addNewFunction(&scope, &tok); - - if (scope) { - old_scope->functionList.push_back(function); - scope->function = &old_scope->functionList.back(); - } - } + else + addGlobalFunction(scope, tok, argStart, funcStart); // syntax error if (!scope) { @@ -432,47 +401,17 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti // function returning function pointer with body else if (Token::simpleMatch(argStart->link(), ") ) (") && Token::Match(argStart->link()->linkAt(2), ") const| {")) { - const Token *tok1 = funcStart; + tok = funcStart; Scope *old_scope = scope; // class function - if (tok1->previous()->str() == "::") - addFunction(&scope, &tok1, argStart); + if (tok->previous()->str() == "::") + addClassFunction(&scope, &tok, argStart); // regular function else { - Function function; - - // save the function definition argument start '(' - function.argDef = argStart; - - // save the access type - function.access = Public; - - // save the function name location - function.tokenDef = funcStart; - function.token = funcStart; - - function.isInline = false; - function.hasBody = true; - function.arg = function.argDef; - function.type = Function::eFunction; - function.retFuncPtr = true; - - // find start of function '{' - const Token *start = tok; - while (start && start->str() != "{") - start = start->next(); - - // save start of function - function.start = start; - - addNewFunction(&scope, &tok1); - - if (scope) { - old_scope->functionList.push_back(function); - scope->function = &old_scope->functionList.back(); - } + Function* function = addGlobalFunction(scope, tok, argStart, funcStart); + function->retFuncPtr = true; } // syntax error? @@ -480,8 +419,6 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti scope = old_scope; break; } - - tok = tok1; } // function prototype @@ -810,7 +747,7 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti } } -bool SymbolDatabase::isFunction(const Token *tok, const Token **funcStart, const Token **argStart) const +bool SymbolDatabase::isFunction(const Token *tok, const Scope* outerScope, const Token **funcStart, const Token **argStart) const { // function returning function pointer? '... ( ... %var% ( ... ))( ... ) {' if (tok->str() == "(" && @@ -825,7 +762,10 @@ bool SymbolDatabase::isFunction(const Token *tok, const Token **funcStart, const } // regular function? - else if (Token::Match(tok, "%var% (") && + else if (Token::Match(tok, "%var% (") && tok->previous() && + (tok->previous()->isName() || tok->previous()->str() == "&" || tok->previous()->str() == "*" || // Either a return type in front of tok + tok->previous()->str() == "::" || tok->previous()->str() == "~" || // or a scope qualifier in front of tok + outerScope->isClassOrStruct()) && // or a ctor/dtor (Token::Match(tok->next()->link(), ") const| ;|{|=") || Token::Match(tok->next()->link(), ") : %var% (|::"))) { *funcStart = tok; @@ -848,10 +788,18 @@ bool SymbolDatabase::argsMatch(const Scope *scope, const Token *first, const Tok // skip default value assignment else if (first->next()->str() == "=") { - first = first->tokAt(2); + first = first->nextArgument(); - if (second->next()->str() == "=") - second = second->tokAt(2); + if (second->next()->str() == "=") { + second = second->nextArgument(); + if (!first || !second) { // End of argument list (first or second) + match = !first && !second; + break; + } + } else if (!first) { // End of argument list (first) + match = second->next() && second->next()->str() == ")"; + break; + } } // definition missing variable name @@ -877,10 +825,6 @@ bool SymbolDatabase::argsMatch(const Scope *scope, const Token *first, const Tok // skip variable names first = first->next(); second = second->next(); - - // skip default value assignment - if (first->next()->str() == "=") - first = first->tokAt(2); } // variable with class path @@ -920,7 +864,45 @@ bool SymbolDatabase::argsMatch(const Scope *scope, const Token *first, const Tok return match; } -void SymbolDatabase::addFunction(Scope **scope, const Token **tok, const Token *argStart) +Function* SymbolDatabase::addGlobalFunction(Scope*& scope, const Token*& tok, const Token *argStart, const Token* funcStart) +{ + Function function; + + // save the function definition argument start '(' + function.argDef = argStart; + + // save the access type + function.access = Public; + + // save the function name location + function.tokenDef = funcStart; + function.token = funcStart; + + function.isInline = false; + function.hasBody = true; + function.arg = function.argDef; + function.type = Function::eFunction; + + // find start of function '{' + const Token *start = tok; + while (start && start->str() != "{") + start = start->next(); + + // save start of function + function.start = start; + + Scope* old_scope = scope; + addNewFunction(&scope, &tok); + + if (scope) { + old_scope->functionList.push_back(function); + scope->function = &old_scope->functionList.back(); + return scope->function; + } + return 0; +} + +void SymbolDatabase::addClassFunction(Scope **scope, const Token **tok, const Token *argStart) { int count = 0; bool added = false; @@ -1880,29 +1862,25 @@ const Variable *Scope::getVariable(const std::string &varname) const return NULL; } -inline const Token* skipScopeIdentifiers(const Token* tok) +static const Token* skipScopeIdentifiers(const Token* tok) { - const Token* ret = tok; - - if (Token::simpleMatch(ret, "::")) { - ret = ret->next(); + if (Token::simpleMatch(tok, "::")) { + tok = tok->next(); } - while (Token::Match(ret, "%type% ::")) { - ret = ret->tokAt(2); + while (Token::Match(tok, "%type% ::")) { + tok = tok->tokAt(2); } - return ret; + return tok; } -inline const Token* skipPointers(const Token* tok) +static const Token* skipPointers(const Token* tok) { - const Token* ret = tok; - - while (Token::Match(ret, "*|&")) { - ret = ret->next(); + while (Token::Match(tok, "*|&")) { + tok = tok->next(); } - return ret; + return tok; } bool Scope::isVariableDeclaration(const Token* tok, const Token*& vartok, const Token*& typetok, bool &isArray, bool &isPointer, bool &isReference) const @@ -1928,11 +1906,11 @@ bool Scope::isVariableDeclaration(const Token* tok, const Token*& vartok, const localVarTok = skipPointers(localTypeTok->next()); } - if (isSimpleVariable(localVarTok)) { + if (Token::Match(localVarTok, "%var% ;|=")) { vartok = localVarTok; typetok = localTypeTok; isArray = false; - } else if (isArrayVariable(localVarTok)) { + } else if (Token::Match(localVarTok, "%var% [") && localVarTok->str() != "operator") { vartok = localVarTok; typetok = localTypeTok; isArray = true; @@ -1956,16 +1934,6 @@ bool Scope::isVariableDeclaration(const Token* tok, const Token*& vartok, const return NULL != vartok; } -bool Scope::isSimpleVariable(const Token* tok) const -{ - return Token::Match(tok, "%var% ;|="); -} - -bool Scope::isArrayVariable(const Token* tok) const -{ - return Token::Match(tok, "%var% [") && tok->next()->str() != "operator"; -} - bool Scope::findClosingBracket(const Token* tok, const Token*& close) const { bool found = false; diff --git a/lib/symboldatabase.h b/lib/symboldatabase.h index ccdd113d7..1cf541281 100644 --- a/lib/symboldatabase.h +++ b/lib/symboldatabase.h @@ -530,8 +530,6 @@ private: * @return true if tok points to a variable declaration, false otherwise */ bool isVariableDeclaration(const Token* tok, const Token*& vartok, const Token*& typetok, bool &isArray, bool &isPointer, bool &isReference) const; - bool isSimpleVariable(const Token* tok) const; - bool isArrayVariable(const Token* tok) const; bool findClosingBracket(const Token* tok, const Token*& close) const; }; @@ -589,10 +587,11 @@ private: // Needed by Borland C++: friend class Scope; - void addFunction(Scope **info, const Token **tok, const Token *argStart); + void addClassFunction(Scope **info, const Token **tok, const Token *argStart); + Function* addGlobalFunction(Scope*& scope, const Token*& tok, const Token *argStart, const Token* funcStart); void addNewFunction(Scope **info, const Token **tok); const Token *initBaseInfo(Scope *info, const Token *tok); - bool isFunction(const Token *tok, const Token **funcStart, const Token **argStart) const; + bool isFunction(const Token *tok, const Scope* outerScope, const Token **funcStart, const Token **argStart) const; /** class/struct types */ std::set classAndStructTypes; diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index 4cda707f3..64acd1a87 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -1521,6 +1521,13 @@ private: "}\n"); ASSERT_EQUALS("[test.cpp:3]: (error) Null pointer dereference\n", errout.str()); + // Ticket #1171 + check("void foo(void* bar) {\n" + " if(strcmp(0, bar) == 0)\n" + " func();\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (error) Null pointer dereference\n", errout.str()); + // Ticket #2413 - it's ok to pass NULL to fflush check("void foo() {\n" " fflush(NULL);\n" diff --git a/test/testobsoletefunctions.cpp b/test/testobsoletefunctions.cpp index b6b4f1992..c98eeb29e 100644 --- a/test/testobsoletefunctions.cpp +++ b/test/testobsoletefunctions.cpp @@ -226,7 +226,7 @@ private: // ticket #3121 void test_declared_function() { - check("ftime ( int a )\n" + check("int ftime ( int a )\n" "{\n" " return a;\n" "}\n"