diff --git a/Makefile b/Makefile index 41c5e33f4..2abff5e45 100644 --- a/Makefile +++ b/Makefile @@ -27,6 +27,7 @@ LIBOBJ = lib/checkautovariables.o \ lib/checkclass.o \ lib/checkexceptionsafety.o \ lib/checkmemoryleak.o \ + lib/checknullpointer.o \ lib/checkobsoletefunctions.o \ lib/checkother.o \ lib/checkpostfixoperator.o \ @@ -64,6 +65,7 @@ TESTOBJ = test/options.o \ test/testincompletestatement.o \ test/testmathlib.o \ test/testmemleak.o \ + test/testnullpointer.o \ test/testobsoletefunctions.o \ test/testoptions.o \ test/testother.o \ @@ -140,10 +142,13 @@ lib/checkexceptionsafety.o: lib/checkexceptionsafety.cpp lib/checkexceptionsafet lib/checkmemoryleak.o: lib/checkmemoryleak.cpp lib/checkmemoryleak.h lib/check.h lib/token.h lib/tokenize.h lib/settings.h lib/errorlogger.h lib/mathlib.h lib/executionpath.h $(CXX) $(CXXFLAGS) -Ilib -c -o lib/checkmemoryleak.o lib/checkmemoryleak.cpp +lib/checknullpointer.o: lib/checknullpointer.cpp lib/checknullpointer.h lib/check.h lib/token.h lib/tokenize.h lib/settings.h lib/errorlogger.h lib/executionpath.h lib/mathlib.h + $(CXX) $(CXXFLAGS) -Ilib -c -o lib/checknullpointer.o lib/checknullpointer.cpp + lib/checkobsoletefunctions.o: lib/checkobsoletefunctions.cpp lib/checkobsoletefunctions.h lib/check.h lib/token.h lib/tokenize.h lib/settings.h lib/errorlogger.h $(CXX) $(CXXFLAGS) -Ilib -c -o lib/checkobsoletefunctions.o lib/checkobsoletefunctions.cpp -lib/checkother.o: lib/checkother.cpp lib/checkother.h lib/check.h lib/token.h lib/tokenize.h lib/settings.h lib/errorlogger.h lib/mathlib.h lib/executionpath.h +lib/checkother.o: lib/checkother.cpp lib/checkother.h lib/check.h lib/token.h lib/tokenize.h lib/settings.h lib/errorlogger.h lib/mathlib.h lib/executionpath.h lib/checknullpointer.h $(CXX) $(CXXFLAGS) -Ilib -c -o lib/checkother.o lib/checkother.cpp lib/checkpostfixoperator.o: lib/checkpostfixoperator.cpp lib/checkpostfixoperator.h lib/check.h lib/token.h lib/tokenize.h lib/settings.h lib/errorlogger.h @@ -245,6 +250,9 @@ test/testmathlib.o: test/testmathlib.cpp lib/mathlib.h test/testsuite.h lib/erro test/testmemleak.o: test/testmemleak.cpp lib/tokenize.h lib/checkmemoryleak.h lib/check.h lib/token.h lib/settings.h lib/errorlogger.h test/testsuite.h test/redirect.h $(CXX) $(CXXFLAGS) -Ilib -Icli -c -o test/testmemleak.o test/testmemleak.cpp +test/testnullpointer.o: test/testnullpointer.cpp lib/tokenize.h lib/checknullpointer.h lib/check.h lib/token.h lib/settings.h lib/errorlogger.h test/testsuite.h test/redirect.h + $(CXX) $(CXXFLAGS) -Ilib -Icli -c -o test/testnullpointer.o test/testnullpointer.cpp + test/testobsoletefunctions.o: test/testobsoletefunctions.cpp lib/tokenize.h lib/checkobsoletefunctions.h lib/check.h lib/token.h lib/settings.h lib/errorlogger.h test/testsuite.h test/redirect.h $(CXX) $(CXXFLAGS) -Ilib -Icli -c -o test/testobsoletefunctions.o test/testobsoletefunctions.cpp diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp new file mode 100644 index 000000000..b75349379 --- /dev/null +++ b/lib/checknullpointer.cpp @@ -0,0 +1,773 @@ +/* + * Cppcheck - A tool for static C/C++ code analysis + * Copyright (C) 2007-2010 Daniel Marjamäki and Cppcheck team. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + + +//--------------------------------------------------------------------------- +#include "checknullpointer.h" +#include "executionpath.h" +#include "mathlib.h" +//--------------------------------------------------------------------------- + +// Register this check class (by creating a static instance of it) +namespace +{ +CheckNullPointer instance; +} + +//--------------------------------------------------------------------------- + + + +/** + * @brief parse a function call and extract information about variable usage + * @param tok first token + * @param var variables that the function read / write. + * @param value 0 => invalid with null pointers as parameter. + * 1-.. => invalid with uninitialized data. + */ +void CheckNullPointer::parseFunctionCall(const Token &tok, std::list &var, unsigned char value) +{ + // standard functions that dereference first parameter.. + // both uninitialized data and null pointers are invalid. + static std::set functionNames1; + if (functionNames1.empty()) + { + functionNames1.insert("memchr"); + functionNames1.insert("memcmp"); + functionNames1.insert("strcat"); + functionNames1.insert("strncat"); + functionNames1.insert("strchr"); + functionNames1.insert("strrchr"); + functionNames1.insert("strcmp"); + functionNames1.insert("strncmp"); + functionNames1.insert("strdup"); + functionNames1.insert("strndup"); + functionNames1.insert("strlen"); + functionNames1.insert("strstr"); + } + + // standard functions that dereference second parameter.. + // both uninitialized data and null pointers are invalid. + static std::set functionNames2; + if (functionNames2.empty()) + { + functionNames2.insert("memcmp"); + functionNames2.insert("memcpy"); + functionNames2.insert("memmove"); + functionNames2.insert("strcat"); + functionNames2.insert("strncat"); + functionNames2.insert("strcmp"); + functionNames2.insert("strncmp"); + functionNames2.insert("strcpy"); + functionNames2.insert("strncpy"); + functionNames2.insert("strstr"); + } + + // 1st parameter.. + if (Token::Match(&tok, "%var% ( %var% ,|)") && tok.tokAt(2)->varId() > 0) + { + if (functionNames1.find(tok.str()) != functionNames1.end()) + var.push_back(tok.tokAt(2)); + else if (value == 0 && Token::Match(&tok, "memchr|memcmp|memcpy|memmove|memset|strcpy|printf|sprintf|snprintf")) + var.push_back(tok.tokAt(2)); + else if (Token::simpleMatch(&tok, "fflush")) + var.push_back(tok.tokAt(2)); + } + + // 2nd parameter.. + if (Token::Match(&tok, "%var% ( %any% , %var% ,|)") && tok.tokAt(4)->varId() > 0) + { + if (functionNames2.find(tok.str()) != functionNames2.end()) + var.push_back(tok.tokAt(4)); + } +} + + + +void CheckNullPointer::nullPointerAfterLoop() +{ + // Locate insufficient null-pointer handling after loop + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) + { + if (! Token::Match(tok, "while ( %var% )")) + continue; + + const unsigned int varid(tok->tokAt(2)->varId()); + if (varid == 0) + continue; + + const std::string varname(tok->strAt(2)); + + // Locate the end of the while loop.. + const Token *tok2 = tok->tokAt(4); + if (tok2->str() == "{") + tok2 = tok2->link(); + else + { + while (tok2 && tok2->str() != ";") + tok2 = tok2->next(); + } + + // Goto next token + if (tok2) + tok2 = tok2->next(); + + // Check if the variable is dereferenced.. + while (tok2) + { + if (tok2->str() == "{" || tok2->str() == "}" || tok2->str() == "break") + break; + + if (tok2->varId() == varid) + { + if (tok2->next()->str() == "." || Token::Match(tok2->next(), "= %varid% .", varid)) + { + // Is this variable a pointer? + const Token *tok3 = Token::findmatch(_tokenizer->tokens(), "%type% * %varid% [;)=]", varid); + if (!tok3) + break; + + if (!tok3->previous() || + Token::Match(tok3->previous(), "[({};]") || + tok3->previous()->isName()) + { + nullPointerError(tok2, varname); + } + } + break; + } + + tok2 = tok2->next(); + } + } +} + +void CheckNullPointer::nullPointerLinkedList() +{ + // looping through items in a linked list in a inner loop.. + for (const Token *tok1 = _tokenizer->tokens(); tok1; tok1 = tok1->next()) + { + // search for a "for" token.. + if (!Token::simpleMatch(tok1, "for (")) + continue; + + if (!Token::simpleMatch(tok1->next()->link(), ") {")) + continue; + + // is there any dereferencing occuring in the for statement.. + unsigned int parlevel2 = 1; + for (const Token *tok2 = tok1->tokAt(2); tok2; tok2 = tok2->next()) + { + // Parantheses.. + if (tok2->str() == "(") + ++parlevel2; + else if (tok2->str() == ")") + { + if (parlevel2 <= 1) + break; + --parlevel2; + } + + // Dereferencing a variable inside the "for" parantheses.. + else if (Token::Match(tok2, "%var% . %var%")) + { + const unsigned int varid(tok2->varId()); + if (varid == 0) + continue; + + if (Token::Match(tok2->tokAt(-2), "%varid% ?", varid)) + continue; + + const std::string varname(tok2->str()); + + // Check usage of dereferenced variable in the loop.. + unsigned int indentlevel3 = 0; + for (const Token *tok3 = tok1->next()->link(); tok3; tok3 = tok3->next()) + { + if (tok3->str() == "{") + ++indentlevel3; + else if (tok3->str() == "}") + { + if (indentlevel3 <= 1) + break; + --indentlevel3; + } + else if (Token::Match(tok3, "while ( %varid% &&|)", varid)) + { + // Make sure there is a "break" to prevent segmentation faults.. + unsigned int indentlevel4 = indentlevel3; + for (const Token *tok4 = tok3->next()->link(); tok4; tok4 = tok4->next()) + { + if (tok4->str() == "{") + ++indentlevel4; + else if (tok4->str() == "}") + { + if (indentlevel4 <= 1) + { + // Is this variable a pointer? + const Token *tempTok = Token::findmatch(_tokenizer->tokens(), "%type% * %varid% [;)=]", varid); + if (tempTok) + nullPointerError(tok1, varname, tok3->linenr()); + + break; + } + --indentlevel4; + } + else if (tok4->str() == "break" || tok4->str() == "return") + break; + } + } + } + } + } + } +} + +void CheckNullPointer::nullPointerStructByDeRefAndChec() +{ + // don't check vars that has been tested against null already + std::set skipvar; + skipvar.insert(0); + + // Dereferencing a struct pointer and then checking if it's NULL.. + for (const Token *tok1 = _tokenizer->tokens(); tok1; tok1 = tok1->next()) + { + if (Token::Match(tok1, "if|while ( !| %var% )")) + { + tok1 = tok1->tokAt(2); + if (tok1->str() == "!") + tok1 = tok1->next(); + skipvar.insert(tok1->varId()); + continue; + } + + // dereference in assignment + if (Token::Match(tok1, "[{};] %var% = %var% . %var%")) + { + if (std::string(tok1->strAt(1)) == tok1->strAt(3)) + continue; + tok1 = tok1->tokAt(3); + } + + // dereference in function call + else if (Token::Match(tok1->tokAt(-2), "%var% ( %var% . %var%") || + Token::Match(tok1->previous(), ", %var% . %var%")) + { + + } + + // 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; + + const std::string varname(tok1->str()); + + unsigned int indentlevel2 = 0; + for (const Token *tok2 = tok1->tokAt(3); tok2; tok2 = tok2->next()) + { + if (tok2->str() == "{") + ++indentlevel2; + + else if (tok2->str() == "}") + { + if (indentlevel2 <= 1) + break; + --indentlevel2; + } + + // goto destination.. + else if (tok2->isName() && Token::simpleMatch(tok2->next(), ":")) + break; + + // Reassignment of the struct + else if (tok2->varId() == varid1) + { + if (tok2->next()->str() == "=") + 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 at base level => stop checking + else if (indentlevel2 == 0 && tok2->str() == "return") + break; + + else if (Token::Match(tok2, "if ( !| %varid% )", varid1)) + { + // Is this variable a pointer? + const Token *tempTok = Token::findmatch(_tokenizer->tokens(), "%type% * %varid% [;)=]", varid1); + if (tempTok) + nullPointerError(tok1, varname, tok2->linenr()); + break; + } + } + } +} + +void CheckNullPointer::nullPointerByDeRefAndChec() +{ + // Dereferencing a pointer and then checking if it's NULL.. + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) + { + if (tok->str() == "if" && Token::Match(tok->previous(), "; if ( ! %var% )")) + { + const unsigned int varid(tok->tokAt(3)->varId()); + if (varid == 0) + continue; + + const std::string varname(tok->strAt(3)); + + // Check that variable is a pointer.. + const Token *decltok = Token::findmatch(_tokenizer->tokens(), "%varid%", varid); + if (!Token::Match(decltok->tokAt(-3), "[{};,(] %type% *")) + continue; + + for (const Token *tok1 = tok->previous(); tok1 && tok1 != decltok; tok1 = tok1->previous()) + { + if (tok1->varId() == varid) + { + if (Token::Match(tok1->tokAt(-2), "[=;{}] *")) + { + nullPointerError(tok1, varname, tok->linenr()); + break; + } + else if (Token::simpleMatch(tok1->previous(), "&")) + { + break; + } + else if (Token::simpleMatch(tok1->next(), "=")) + { + break; + } + // dereference in function call + else if (Token::Match(tok1->tokAt(-3), "!!sizeof [(,] *")) + { + nullPointerError(tok1, varname, tok->linenr()); + } + } + + else if (tok1->str() == "{" || + tok1->str() == "}") + break; + + // label.. + else if (Token::Match(tok1, "%type% :")) + break; + } + } + } +} + +void CheckNullPointer::nullPointerByCheckAndDeRef() +{ + // Check if pointer is NULL and then dereference it.. + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) + { + if (Token::Match(tok, "if ( ! %var% ) {")) + { + bool null = true; + const unsigned int varid(tok->tokAt(3)->varId()); + if (varid == 0) + continue; + unsigned int indentlevel = 1; + for (const Token *tok2 = tok->tokAt(6); tok2; tok2 = tok2->next()) + { + if (tok2->str() == "{") + ++indentlevel; + else if (tok2->str() == "}") + { + if (indentlevel == 0) + break; + --indentlevel; + if (null && indentlevel == 0) + { + // skip all "else" blocks because they are not executed in this execution path + while (Token::Match(tok2, "} else {")) + tok2 = tok2->tokAt(2)->link(); + null = false; + } + } + + if (Token::Match(tok2, "goto|return|continue|break|throw|if")) + { + if (Token::Match(tok2, "return * %var%")) + nullPointerError(tok2, tok->strAt(3)); + break; + } + + // abort function.. + if (Token::Match(tok2->previous(), "[;{}] %var% (") && + Token::simpleMatch(tok2->next()->link(), ") ; }")) + { + break; + } + + if (tok2->varId() == varid) + { + if (Token::Match(tok2->previous(), "[;{}=] %var% = 0 ;")) + ; + + else if (Token::Match(tok2->tokAt(-2), "[;{}=+-/(,] * %var%")) + nullPointerError(tok2, tok->strAt(3)); + + else if (!Token::simpleMatch(tok2->tokAt(-2), "& (") && Token::Match(tok2->next(), ". %var%")) + nullPointerError(tok2, tok->strAt(3)); + + else if (Token::Match(tok2->previous(), "[;{}=+-/(,] %var% [ %any% ]")) + nullPointerError(tok2, tok->strAt(3)); + + else if (Token::Match(tok2->previous(), "return %var% [ %any% ]")) + nullPointerError(tok2, tok->strAt(3)); + + else if (Token::Match(tok2, "%var% (")) + nullPointerError(tok2, tok->strAt(3)); + + else + break; + } + } + } + } +} + + +void CheckNullPointer::nullPointer() +{ + nullPointerAfterLoop(); + nullPointerLinkedList(); + nullPointerStructByDeRefAndChec(); + nullPointerByDeRefAndChec(); + nullPointerByCheckAndDeRef(); +} + +/** Derefencing null constant (simplified token list) */ +void CheckNullPointer::nullConstantDereference() +{ + // this is kept at 0 for all scopes that are not executing + unsigned int indentlevel = 0; + + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) + { + // start of executable scope.. + if (indentlevel == 0 && Token::Match(tok, ") const| {")) + indentlevel = 1; + + else if (indentlevel >= 1) + { + if (tok->str() == "{") + ++indentlevel; + + else if (tok->str() == "}") + { + if (indentlevel <= 2) + indentlevel = 0; + else + --indentlevel; + } + + if (tok->str() == "(" && Token::simpleMatch(tok->previous(), "sizeof")) + tok = tok->link(); + + else if (Token::simpleMatch(tok, "exit ( )")) + { + // Goto end of scope + while (tok && tok->str() != "}") + { + if (tok->str() == "{") + tok = tok->link(); + tok = tok->next(); + } + if (!tok) + break; + } + + else if (Token::simpleMatch(tok, "* 0")) + { + if (Token::Match(tok->previous(), "[<>;{}=+-*/(,]") || + Token::Match(tok->previous(), "return|<<")) + { + nullPointerError(tok); + } + } + } + } +} + + +/// @addtogroup Checks +/// @{ + + +/** + * @brief %Check for null pointer usage (using ExecutionPath) + */ + +class Nullpointer : public ExecutionPath +{ +public: + /** Startup constructor */ + Nullpointer(Check *c) : ExecutionPath(c, 0), null(false) + { + } + +private: + /** Create checking of specific variable: */ + Nullpointer(Check *c, const unsigned int id, const std::string &name) + : ExecutionPath(c, id), + varname(name), + null(false) + { + } + + /** Copy this check */ + ExecutionPath *copy() + { + return new Nullpointer(*this); + } + + /** no implementation => compiler error if used by accident */ + void operator=(const Nullpointer &); + + /** is other execution path equal? */ + bool is_equal(const ExecutionPath *e) const + { + const Nullpointer *c = static_cast(e); + return (varname == c->varname && null == c->null); + } + + /** variable name for this check (empty => dummy check) */ + const std::string varname; + + /** is this variable null? */ + bool null; + + /** variable is set to null */ + static void setnull(std::list &checks, const unsigned int varid) + { + std::list::iterator it; + for (it = checks.begin(); it != checks.end(); ++it) + { + Nullpointer *c = dynamic_cast(*it); + if (c && c->varId == varid) + c->null = true; + } + } + + /** + * Dereferencing variable. Check if it is safe (if the variable is null there's an error) + * @param checks Checks + * @param tok token where dereferencing happens + */ + static void dereference(std::list &checks, const Token *tok) + { + const unsigned int varid(tok->varId()); + + std::list::iterator it; + for (it = checks.begin(); it != checks.end(); ++it) + { + Nullpointer *c = dynamic_cast(*it); + if (c && c->varId == varid && c->null) + { + CheckNullPointer *checkNullPointer = dynamic_cast(c->owner); + if (checkNullPointer) + { + checkNullPointer->nullPointerError(tok, c->varname); + return; + } + } + } + } + + /** parse tokens */ + const Token *parse(const Token &tok, std::list &checks) const + { + if (Token::Match(tok.previous(), "[;{}] const| %type% * %var% ;")) + { + const Token * vartok = tok.tokAt(2); + + if (tok.str() == "const") + vartok = vartok->next(); + + if (vartok->varId() != 0) + checks.push_back(new Nullpointer(owner, vartok->varId(), vartok->str())); + return vartok->next(); + } + + // Template pointer variable.. + if (Token::Match(tok.previous(), "[;{}] %type% ::|<")) + { + const Token * vartok = &tok; + while (Token::Match(vartok, "%type% ::")) + vartok = vartok->tokAt(2); + if (Token::Match(vartok, "%type% < %type%")) + { + vartok = vartok->tokAt(3); + while (vartok && (vartok->str() == "*" || vartok->isName())) + vartok = vartok->next(); + } + if (vartok + && (vartok->str() == ">" || vartok->isName()) + && Token::Match(vartok->next(), "* %var% ;|=")) + { + vartok = vartok->tokAt(2); + checks.push_back(new Nullpointer(owner, vartok->varId(), vartok->str())); + if (Token::simpleMatch(vartok->next(), "= 0 ;")) + setnull(checks, vartok->varId()); + return vartok->next(); + } + } + + if (Token::simpleMatch(&tok, "try {")) + { + // Bail out all used variables + unsigned int indentlevel = 0; + for (const Token *tok2 = &tok; tok2; tok2 = tok2->next()) + { + if (tok2->str() == "{") + ++indentlevel; + else if (tok2->str() == "}") + { + if (indentlevel == 0) + break; + if (indentlevel == 1 && !Token::simpleMatch(tok2,"} catch (")) + return tok2; + --indentlevel; + } + else if (tok2->varId()) + bailOutVar(checks,tok2->varId()); + } + } + + if (Token::Match(&tok, "%var% (")) + { + if (tok.str() == "sizeof") + return tok.next()->link(); + + // parse usage.. + std::list var; + CheckNullPointer::parseFunctionCall(tok, var, 0); + for (std::list::const_iterator it = var.begin(); it != var.end(); ++it) + dereference(checks, *it); + } + + if (tok.varId() != 0) + { + if (Token::Match(tok.previous(), "[;{}=] %var% = 0 ;")) + setnull(checks, tok.varId()); + else if (Token::Match(tok.tokAt(-2), "[;{}=+-/(,] * %var%")) + dereference(checks, &tok); + else if (Token::Match(tok.tokAt(-2), "return * %var%")) + dereference(checks, &tok); + else if (!Token::simpleMatch(tok.tokAt(-2), "& (") && Token::Match(tok.next(), ". %var%")) + dereference(checks, &tok); + else if (Token::Match(tok.previous(), "[;{}=+-/(,] %var% [ %any% ]")) + dereference(checks, &tok); + else if (Token::Match(tok.previous(), "return %var% [ %any% ]")) + dereference(checks, &tok); + else if (Token::Match(&tok, "%var% (")) + dereference(checks, &tok); + else + bailOutVar(checks, tok.varId()); + } + + else if (tok.str() == "delete") + { + const Token *ret = tok.next(); + if (Token::simpleMatch(ret, "[ ]")) + ret = ret->tokAt(2); + if (Token::Match(ret, "%var% ;")) + return ret->next(); + } + + return &tok; + } + + /** parse condition. @sa ExecutionPath::parseCondition */ + bool parseCondition(const Token &tok, std::list &checks) + { + for (const Token *tok2 = &tok; tok2; tok2 = tok2->next()) + { + if (tok2->str() == "(" || tok2->str() == ")") + break; + if (Token::Match(tok2, "[<>=] * %var%")) + dereference(checks, tok2->tokAt(2)); + } + + if (Token::Match(&tok, "!| %var% (")) + { + std::list var; + CheckNullPointer::parseFunctionCall(tok.str() == "!" ? *tok.next() : tok, var, 0); + for (std::list::const_iterator it = var.begin(); it != var.end(); ++it) + dereference(checks, *it); + } + + return ExecutionPath::parseCondition(tok, checks); + } + + + void parseLoopBody(const Token *tok, std::list &checks) const + { + while (tok) + { + if (Token::Match(tok, "{|}|return|goto|break|if")) + return; + const Token *next = parse(*tok, checks); + if (next) + tok = tok->next(); + } + } + +}; +/// @} + + +void CheckNullPointer::executionPaths() +{ + // Check for null pointer errors.. + Nullpointer c(this); + checkExecutionPaths(_tokenizer->tokens(), &c); +} + +void CheckNullPointer::nullPointerError(const Token *tok) +{ + reportError(tok, Severity::error, "nullPointer", "Null pointer dereference"); +} + +void CheckNullPointer::nullPointerError(const Token *tok, const std::string &varname) +{ + reportError(tok, Severity::error, "nullPointer", "Possible null pointer dereference: " + varname); +} + +void CheckNullPointer::nullPointerError(const Token *tok, const std::string &varname, const unsigned int line) +{ + reportError(tok, Severity::error, "nullPointer", "Possible null pointer dereference: " + varname + " - otherwise it is redundant to check if " + varname + " is null at line " + MathLib::toString(line)); +} + diff --git a/lib/checknullpointer.h b/lib/checknullpointer.h new file mode 100644 index 000000000..a6628fac0 --- /dev/null +++ b/lib/checknullpointer.h @@ -0,0 +1,146 @@ +/* + * Cppcheck - A tool for static C/C++ code analysis + * Copyright (C) 2007-2010 Daniel Marjamäki and Cppcheck team. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + + +//--------------------------------------------------------------------------- +#ifndef checknullpointerH +#define checknullpointerH +//--------------------------------------------------------------------------- + +#include "check.h" +#include "settings.h" + +class Token; + +/// @addtogroup Checks +/// @{ + + +/** @brief check for null pointer dereferencing */ + +class CheckNullPointer : public Check +{ +public: + /** @brief This constructor is used when registering the CheckNullPointer */ + CheckNullPointer() : Check() + { } + + /** @brief This constructor is used when running checks. */ + CheckNullPointer(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger) + : Check(tokenizer, settings, errorLogger) + { } + + /** @brief Run checks against the normal token list */ + void runChecks(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger) + { + CheckNullPointer checkNullPointer(tokenizer, settings, errorLogger); + checkNullPointer.nullPointer(); + } + + /** @brief Run checks against the simplified token list */ + void runSimplifiedChecks(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger) + { + CheckNullPointer checkNullPointer(tokenizer, settings, errorLogger); + checkNullPointer.nullConstantDereference(); + checkNullPointer.executionPaths(); + } + + /** + * @brief parse a function call and extract information about variable usage + * @param tok first token + * @param var variables that the function read / write. + * @param value 0 => invalid with null pointers as parameter. + * non-zero => invalid with uninitialized data. + */ + static void parseFunctionCall(const Token &tok, + std::list &var, + unsigned char value); + + /** @brief possible null pointer dereference */ + void nullPointer(); + + /** + * @brief Does one part of the check for nullPointer(). + * Checking if pointer is NULL and then dereferencing it.. + */ + void nullPointerByCheckAndDeRef(); + + /** @brief dereferencing null constant (after Tokenizer::simplifyKnownVariables) */ + void nullConstantDereference(); + + /** @brief new type of check: check execution paths */ + void executionPaths(); + + void nullPointerError(const Token *tok); // variable name unknown / doesn't exist + void nullPointerError(const Token *tok, const std::string &varname); + void nullPointerError(const Token *tok, const std::string &varname, const unsigned int line); + + void getErrorMessages() + { + nullPointerError(0, "pointer"); + } + + std::string name() const + { + return "Null pointer"; + } + + std::string classInfo() const + { + return "Null pointers\n" + "* null pointer dereferencing\n"; + } + +private: + + /** + * @brief Does one part of the check for nullPointer(). + * Locate insufficient null-pointer handling after loop + */ + void nullPointerAfterLoop(); + + /** + * @brief Does one part of the check for nullPointer(). + * looping through items in a linked list in a inner loop.. + */ + void nullPointerLinkedList(); + + /** + * @brief Does one part of the check for nullPointer(). + * Dereferencing a struct pointer and then checking if it's NULL.. + */ + void nullPointerStructByDeRefAndChec(); + + /** + * @brief Does one part of the check for nullPointer(). + * Dereferencing a pointer and then checking if it's NULL.. + */ + void nullPointerByDeRefAndChec(); + + /** + * @brief Does one part of the check for nullPointer(). + * -# initialize pointer to 0 + * -# conditionally assign pointer + * -# dereference pointer + */ + void nullPointerConditionalAssignment(); +}; +/// @} +//--------------------------------------------------------------------------- +#endif + diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 648cc93bd..67f11c6ff 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -21,6 +21,7 @@ #include "checkother.h" #include "mathlib.h" #include "executionpath.h" +#include "checknullpointer.h" // CheckNullPointer::parseFunctionCall #include #include @@ -2249,719 +2250,9 @@ void CheckOther::strPlusChar() } -void CheckOther::nullPointerAfterLoop() -{ - // Locate insufficient null-pointer handling after loop - for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) - { - if (! Token::Match(tok, "while ( %var% )")) - continue; - - const unsigned int varid(tok->tokAt(2)->varId()); - if (varid == 0) - continue; - - const std::string varname(tok->strAt(2)); - - // Locate the end of the while loop.. - const Token *tok2 = tok->tokAt(4); - if (tok2->str() == "{") - tok2 = tok2->link(); - else - { - while (tok2 && tok2->str() != ";") - tok2 = tok2->next(); - } - - // Goto next token - if (tok2) - tok2 = tok2->next(); - - // Check if the variable is dereferenced.. - while (tok2) - { - if (tok2->str() == "{" || tok2->str() == "}" || tok2->str() == "break") - break; - - if (tok2->varId() == varid) - { - if (tok2->next()->str() == "." || Token::Match(tok2->next(), "= %varid% .", varid)) - { - // Is this variable a pointer? - const Token *tok3 = Token::findmatch(_tokenizer->tokens(), "%type% * %varid% [;)=]", varid); - if (!tok3) - break; - - if (!tok3->previous() || - Token::Match(tok3->previous(), "[({};]") || - tok3->previous()->isName()) - { - nullPointerError(tok2, varname); - } - } - break; - } - - tok2 = tok2->next(); - } - } -} - -void CheckOther::nullPointerLinkedList() -{ - // looping through items in a linked list in a inner loop.. - for (const Token *tok1 = _tokenizer->tokens(); tok1; tok1 = tok1->next()) - { - // search for a "for" token.. - if (!Token::simpleMatch(tok1, "for (")) - continue; - - if (!Token::simpleMatch(tok1->next()->link(), ") {")) - continue; - - // is there any dereferencing occuring in the for statement.. - unsigned int parlevel2 = 1; - for (const Token *tok2 = tok1->tokAt(2); tok2; tok2 = tok2->next()) - { - // Parantheses.. - if (tok2->str() == "(") - ++parlevel2; - else if (tok2->str() == ")") - { - if (parlevel2 <= 1) - break; - --parlevel2; - } - - // Dereferencing a variable inside the "for" parantheses.. - else if (Token::Match(tok2, "%var% . %var%")) - { - const unsigned int varid(tok2->varId()); - if (varid == 0) - continue; - - if (Token::Match(tok2->tokAt(-2), "%varid% ?", varid)) - continue; - - const std::string varname(tok2->str()); - - // Check usage of dereferenced variable in the loop.. - unsigned int indentlevel3 = 0; - for (const Token *tok3 = tok1->next()->link(); tok3; tok3 = tok3->next()) - { - if (tok3->str() == "{") - ++indentlevel3; - else if (tok3->str() == "}") - { - if (indentlevel3 <= 1) - break; - --indentlevel3; - } - else if (Token::Match(tok3, "while ( %varid% &&|)", varid)) - { - // Make sure there is a "break" to prevent segmentation faults.. - unsigned int indentlevel4 = indentlevel3; - for (const Token *tok4 = tok3->next()->link(); tok4; tok4 = tok4->next()) - { - if (tok4->str() == "{") - ++indentlevel4; - else if (tok4->str() == "}") - { - if (indentlevel4 <= 1) - { - // Is this variable a pointer? - const Token *tempTok = Token::findmatch(_tokenizer->tokens(), "%type% * %varid% [;)=]", varid); - if (tempTok) - nullPointerError(tok1, varname, tok3->linenr()); - - break; - } - --indentlevel4; - } - else if (tok4->str() == "break" || tok4->str() == "return") - break; - } - } - } - } - } - } -} - -void CheckOther::nullPointerStructByDeRefAndChec() -{ - // don't check vars that has been tested against null already - std::set skipvar; - skipvar.insert(0); - - // Dereferencing a struct pointer and then checking if it's NULL.. - for (const Token *tok1 = _tokenizer->tokens(); tok1; tok1 = tok1->next()) - { - if (Token::Match(tok1, "if|while ( !| %var% )")) - { - tok1 = tok1->tokAt(2); - if (tok1->str() == "!") - tok1 = tok1->next(); - skipvar.insert(tok1->varId()); - continue; - } - - // dereference in assignment - if (Token::Match(tok1, "[{};] %var% = %var% . %var%")) - { - if (std::string(tok1->strAt(1)) == tok1->strAt(3)) - continue; - tok1 = tok1->tokAt(3); - } - - // dereference in function call - else if (Token::Match(tok1->tokAt(-2), "%var% ( %var% . %var%") || - Token::Match(tok1->previous(), ", %var% . %var%")) - { - - } - - // 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; - - const std::string varname(tok1->str()); - - unsigned int indentlevel2 = 0; - for (const Token *tok2 = tok1->tokAt(3); tok2; tok2 = tok2->next()) - { - if (tok2->str() == "{") - ++indentlevel2; - - else if (tok2->str() == "}") - { - if (indentlevel2 <= 1) - break; - --indentlevel2; - } - - // goto destination.. - else if (tok2->isName() && Token::simpleMatch(tok2->next(), ":")) - break; - - // Reassignment of the struct - else if (tok2->varId() == varid1) - { - if (tok2->next()->str() == "=") - 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 at base level => stop checking - else if (indentlevel2 == 0 && tok2->str() == "return") - break; - - else if (Token::Match(tok2, "if ( !| %varid% )", varid1)) - { - // Is this variable a pointer? - const Token *tempTok = Token::findmatch(_tokenizer->tokens(), "%type% * %varid% [;)=]", varid1); - if (tempTok) - nullPointerError(tok1, varname, tok2->linenr()); - break; - } - } - } -} - -void CheckOther::nullPointerByDeRefAndChec() -{ - // Dereferencing a pointer and then checking if it's NULL.. - for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) - { - if (tok->str() == "if" && Token::Match(tok->previous(), "; if ( ! %var% )")) - { - const unsigned int varid(tok->tokAt(3)->varId()); - if (varid == 0) - continue; - - const std::string varname(tok->strAt(3)); - - // Check that variable is a pointer.. - const Token *decltok = Token::findmatch(_tokenizer->tokens(), "%varid%", varid); - if (!Token::Match(decltok->tokAt(-3), "[{};,(] %type% *")) - continue; - - for (const Token *tok1 = tok->previous(); tok1 && tok1 != decltok; tok1 = tok1->previous()) - { - if (tok1->varId() == varid) - { - if (Token::Match(tok1->tokAt(-2), "[=;{}] *")) - { - nullPointerError(tok1, varname, tok->linenr()); - break; - } - else if (Token::simpleMatch(tok1->previous(), "&")) - { - break; - } - else if (Token::simpleMatch(tok1->next(), "=")) - { - break; - } - // dereference in function call - else if (Token::Match(tok1->tokAt(-3), "!!sizeof [(,] *")) - { - nullPointerError(tok1, varname, tok->linenr()); - } - } - - else if (tok1->str() == "{" || - tok1->str() == "}") - break; - - // label.. - else if (Token::Match(tok1, "%type% :")) - break; - } - } - } -} - -void CheckOther::nullPointerByCheckAndDeRef() -{ - // Check if pointer is NULL and then dereference it.. - for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) - { - if (Token::Match(tok, "if ( ! %var% ) {")) - { - bool null = true; - const unsigned int varid(tok->tokAt(3)->varId()); - if (varid == 0) - continue; - unsigned int indentlevel = 1; - for (const Token *tok2 = tok->tokAt(6); tok2; tok2 = tok2->next()) - { - if (tok2->str() == "{") - ++indentlevel; - else if (tok2->str() == "}") - { - if (indentlevel == 0) - break; - --indentlevel; - if (null && indentlevel == 0) - { - // skip all "else" blocks because they are not executed in this execution path - while (Token::Match(tok2, "} else {")) - tok2 = tok2->tokAt(2)->link(); - null = false; - } - } - - if (Token::Match(tok2, "goto|return|continue|break|throw|if")) - { - if (Token::Match(tok2, "return * %var%")) - nullPointerError(tok2, tok->strAt(3)); - break; - } - - // abort function.. - if (Token::Match(tok2->previous(), "[;{}] %var% (") && - Token::simpleMatch(tok2->next()->link(), ") ; }")) - { - break; - } - - if (tok2->varId() == varid) - { - if (Token::Match(tok2->previous(), "[;{}=] %var% = 0 ;")) - ; - - else if (Token::Match(tok2->tokAt(-2), "[;{}=+-/(,] * %var%")) - nullPointerError(tok2, tok->strAt(3)); - - else if (!Token::simpleMatch(tok2->tokAt(-2), "& (") && Token::Match(tok2->next(), ". %var%")) - nullPointerError(tok2, tok->strAt(3)); - - else if (Token::Match(tok2->previous(), "[;{}=+-/(,] %var% [ %any% ]")) - nullPointerError(tok2, tok->strAt(3)); - - else if (Token::Match(tok2->previous(), "return %var% [ %any% ]")) - nullPointerError(tok2, tok->strAt(3)); - - else if (Token::Match(tok2, "%var% (")) - nullPointerError(tok2, tok->strAt(3)); - - else - break; - } - } - } - } -} - - -void CheckOther::nullPointer() -{ - nullPointerAfterLoop(); - nullPointerLinkedList(); - nullPointerStructByDeRefAndChec(); - nullPointerByDeRefAndChec(); - nullPointerByCheckAndDeRef(); -} - -/** Derefencing null constant (simplified token list) */ -void CheckOther::nullConstantDereference() -{ - // this is kept at 0 for all scopes that are not executing - unsigned int indentlevel = 0; - - for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) - { - // start of executable scope.. - if (indentlevel == 0 && Token::Match(tok, ") const| {")) - indentlevel = 1; - - else if (indentlevel >= 1) - { - if (tok->str() == "{") - ++indentlevel; - - else if (tok->str() == "}") - { - if (indentlevel <= 2) - indentlevel = 0; - else - --indentlevel; - } - - if (tok->str() == "(" && Token::simpleMatch(tok->previous(), "sizeof")) - tok = tok->link(); - - else if (Token::simpleMatch(tok, "exit ( )")) - { - // Goto end of scope - while (tok && tok->str() != "}") - { - if (tok->str() == "{") - tok = tok->link(); - tok = tok->next(); - } - if (!tok) - break; - } - - else if (Token::simpleMatch(tok, "* 0")) - { - if (Token::Match(tok->previous(), "[<>;{}=+-*/(,]") || - Token::Match(tok->previous(), "return|<<")) - { - nullPointerError(tok); - } - } - } - } -} - -/** - * \brief parse a function call and extract information about variable usage - * \param tok first token - * \param var variables that the function read / write. - * \param value 0 => invalid with null pointers as parameter. - * 1-.. => invalid with uninitialized data. - */ -static void parseFunctionCall(const Token &tok, std::list &var, unsigned char value) -{ - // standard functions that dereference first parameter.. - // both uninitialized data and null pointers are invalid. - static std::set functionNames1; - if (functionNames1.empty()) - { - functionNames1.insert("memchr"); - functionNames1.insert("memcmp"); - functionNames1.insert("strcat"); - functionNames1.insert("strncat"); - functionNames1.insert("strchr"); - functionNames1.insert("strrchr"); - functionNames1.insert("strcmp"); - functionNames1.insert("strncmp"); - functionNames1.insert("strdup"); - functionNames1.insert("strndup"); - functionNames1.insert("strlen"); - functionNames1.insert("strstr"); - } - - // standard functions that dereference second parameter.. - // both uninitialized data and null pointers are invalid. - static std::set functionNames2; - if (functionNames2.empty()) - { - functionNames2.insert("memcmp"); - functionNames2.insert("memcpy"); - functionNames2.insert("memmove"); - functionNames2.insert("strcat"); - functionNames2.insert("strncat"); - functionNames2.insert("strcmp"); - functionNames2.insert("strncmp"); - functionNames2.insert("strcpy"); - functionNames2.insert("strncpy"); - functionNames2.insert("strstr"); - } - - // 1st parameter.. - if (Token::Match(&tok, "%var% ( %var% ,|)") && tok.tokAt(2)->varId() > 0) - { - if (functionNames1.find(tok.str()) != functionNames1.end()) - var.push_back(tok.tokAt(2)); - else if (value == 0 && Token::Match(&tok, "memchr|memcmp|memcpy|memmove|memset|strcpy|printf|sprintf|snprintf")) - var.push_back(tok.tokAt(2)); - else if (Token::simpleMatch(&tok, "fflush")) - var.push_back(tok.tokAt(2)); - } - - // 2nd parameter.. - if (Token::Match(&tok, "%var% ( %any% , %var% ,|)") && tok.tokAt(4)->varId() > 0) - { - if (functionNames2.find(tok.str()) != functionNames2.end()) - var.push_back(tok.tokAt(4)); - } -} - - /// @addtogroup Checks /// @{ - -/** - * @brief %Check for null pointer usage (using ExecutionPath) - */ - -class CheckNullpointer : public ExecutionPath -{ -public: - /** Startup constructor */ - CheckNullpointer(Check *c) : ExecutionPath(c, 0), null(false) - { - } - -private: - /** Create checking of specific variable: */ - CheckNullpointer(Check *c, const unsigned int id, const std::string &name) - : ExecutionPath(c, id), - varname(name), - null(false) - { - } - - /** Copy this check */ - ExecutionPath *copy() - { - return new CheckNullpointer(*this); - } - - /** no implementation => compiler error if used by accident */ - void operator=(const CheckNullpointer &); - - /** is other execution path equal? */ - bool is_equal(const ExecutionPath *e) const - { - const CheckNullpointer *c = static_cast(e); - return (varname == c->varname && null == c->null); - } - - /** variable name for this check (empty => dummy check) */ - const std::string varname; - - /** is this variable null? */ - bool null; - - /** variable is set to null */ - static void setnull(std::list &checks, const unsigned int varid) - { - std::list::iterator it; - for (it = checks.begin(); it != checks.end(); ++it) - { - CheckNullpointer *c = dynamic_cast(*it); - if (c && c->varId == varid) - c->null = true; - } - } - - /** - * Dereferencing variable. Check if it is safe (if the variable is null there's an error) - * @param checks Checks - * @param tok token where dereferencing happens - */ - static void dereference(std::list &checks, const Token *tok) - { - const unsigned int varid(tok->varId()); - - std::list::iterator it; - for (it = checks.begin(); it != checks.end(); ++it) - { - CheckNullpointer *c = dynamic_cast(*it); - if (c && c->varId == varid && c->null) - { - CheckOther *checkOther = dynamic_cast(c->owner); - if (checkOther) - { - checkOther->nullPointerError(tok, c->varname); - return; - } - } - } - } - - /** parse tokens */ - const Token *parse(const Token &tok, std::list &checks) const - { - if (Token::Match(tok.previous(), "[;{}] const| %type% * %var% ;")) - { - const Token * vartok = tok.tokAt(2); - - if (tok.str() == "const") - vartok = vartok->next(); - - if (vartok->varId() != 0) - checks.push_back(new CheckNullpointer(owner, vartok->varId(), vartok->str())); - return vartok->next(); - } - - // Template pointer variable.. - if (Token::Match(tok.previous(), "[;{}] %type% ::|<")) - { - const Token * vartok = &tok; - while (Token::Match(vartok, "%type% ::")) - vartok = vartok->tokAt(2); - if (Token::Match(vartok, "%type% < %type%")) - { - vartok = vartok->tokAt(3); - while (vartok && (vartok->str() == "*" || vartok->isName())) - vartok = vartok->next(); - } - if (vartok - && (vartok->str() == ">" || vartok->isName()) - && Token::Match(vartok->next(), "* %var% ;|=")) - { - vartok = vartok->tokAt(2); - checks.push_back(new CheckNullpointer(owner, vartok->varId(), vartok->str())); - if (Token::simpleMatch(vartok->next(), "= 0 ;")) - setnull(checks, vartok->varId()); - return vartok->next(); - } - } - - if (Token::simpleMatch(&tok, "try {")) - { - // Bail out all used variables - unsigned int indentlevel = 0; - for (const Token *tok2 = &tok; tok2; tok2 = tok2->next()) - { - if (tok2->str() == "{") - ++indentlevel; - else if (tok2->str() == "}") - { - if (indentlevel == 0) - break; - if (indentlevel == 1 && !Token::simpleMatch(tok2,"} catch (")) - return tok2; - --indentlevel; - } - else if (tok2->varId()) - bailOutVar(checks,tok2->varId()); - } - } - - if (Token::Match(&tok, "%var% (")) - { - if (tok.str() == "sizeof") - return tok.next()->link(); - - // parse usage.. - std::list var; - parseFunctionCall(tok, var, 0); - for (std::list::const_iterator it = var.begin(); it != var.end(); ++it) - dereference(checks, *it); - } - - if (tok.varId() != 0) - { - if (Token::Match(tok.previous(), "[;{}=] %var% = 0 ;")) - setnull(checks, tok.varId()); - else if (Token::Match(tok.tokAt(-2), "[;{}=+-/(,] * %var%")) - dereference(checks, &tok); - else if (Token::Match(tok.tokAt(-2), "return * %var%")) - dereference(checks, &tok); - else if (!Token::simpleMatch(tok.tokAt(-2), "& (") && Token::Match(tok.next(), ". %var%")) - dereference(checks, &tok); - else if (Token::Match(tok.previous(), "[;{}=+-/(,] %var% [ %any% ]")) - dereference(checks, &tok); - else if (Token::Match(tok.previous(), "return %var% [ %any% ]")) - dereference(checks, &tok); - else if (Token::Match(&tok, "%var% (")) - dereference(checks, &tok); - else - bailOutVar(checks, tok.varId()); - } - - else if (tok.str() == "delete") - { - const Token *ret = tok.next(); - if (Token::simpleMatch(ret, "[ ]")) - ret = ret->tokAt(2); - if (Token::Match(ret, "%var% ;")) - return ret->next(); - } - - return &tok; - } - - /** parse condition. @sa ExecutionPath::parseCondition */ - bool parseCondition(const Token &tok, std::list &checks) - { - for (const Token *tok2 = &tok; tok2; tok2 = tok2->next()) - { - if (tok2->str() == "(" || tok2->str() == ")") - break; - if (Token::Match(tok2, "[<>=] * %var%")) - dereference(checks, tok2->tokAt(2)); - } - - if (Token::Match(&tok, "!| %var% (")) - { - std::list var; - parseFunctionCall(tok.str() == "!" ? *tok.next() : tok, var, 0); - for (std::list::const_iterator it = var.begin(); it != var.end(); ++it) - dereference(checks, *it); - } - - return ExecutionPath::parseCondition(tok, checks); - } - - - void parseLoopBody(const Token *tok, std::list &checks) const - { - while (tok) - { - if (Token::Match(tok, "{|}|return|goto|break|if")) - return; - const Token *next = parse(*tok, checks); - if (next) - tok = tok->next(); - } - } - -}; - - /** * @brief %Check that uninitialized variables aren't used (using ExecutionPath) * */ @@ -3483,13 +2774,13 @@ private: // parse usage.. { std::list var; - parseFunctionCall(tok, var, 1); + CheckNullPointer::parseFunctionCall(tok, var, 1); for (std::list::const_iterator it = var.begin(); it != var.end(); ++it) use_array(checks, *it); // Using uninitialized pointer is bad if using null pointer is bad std::list var2; - parseFunctionCall(tok, var2, 0); + CheckNullPointer::parseFunctionCall(tok, var2, 0); for (std::list::const_iterator it = var2.begin(); it != var2.end(); ++it) { if (std::find(var.begin(), var.end(), *it) == var.end()) @@ -3771,7 +3062,7 @@ private: else if (Token::Match(&tok, "!| %var% (")) { std::list var; - parseFunctionCall(tok.str() == "!" ? *tok.next() : tok, var, 1); + CheckNullPointer::parseFunctionCall(tok.str() == "!" ? *tok.next() : tok, var, 1); for (std::list::const_iterator it = var.begin(); it != var.end(); ++it) use_array(checks, *it); } @@ -3909,12 +3200,6 @@ void CheckOther::saveAnalysisData(const std::set &data) const void CheckOther::executionPaths() { - // Check for null pointer errors.. - { - CheckNullpointer c(this); - checkExecutionPaths(_tokenizer->tokens(), &c); - } - // check if variable is accessed uninitialized.. { // no writing if multiple threads are used (TODO: thread safe analysis?) @@ -4160,21 +3445,6 @@ void CheckOther::strPlusChar(const Token *tok) reportError(tok, Severity::error, "strPlusChar", "Unusual pointer arithmetic"); } -void CheckOther::nullPointerError(const Token *tok) -{ - reportError(tok, Severity::error, "nullPointer", "Null pointer dereference"); -} - -void CheckOther::nullPointerError(const Token *tok, const std::string &varname) -{ - reportError(tok, Severity::error, "nullPointer", "Possible null pointer dereference: " + varname); -} - -void CheckOther::nullPointerError(const Token *tok, const std::string &varname, const unsigned int line) -{ - reportError(tok, Severity::error, "nullPointer", "Possible null pointer dereference: " + varname + " - otherwise it is redundant to check if " + varname + " is null at line " + MathLib::toString(line)); -} - void CheckOther::uninitstringError(const Token *tok, const std::string &varname) { reportError(tok, Severity::error, "uninitstring", "Dangerous usage of '" + varname + "' (strncpy doesn't always 0-terminate it)"); diff --git a/lib/checkother.h b/lib/checkother.h index 53aa7a6cb..7aad92acb 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -50,8 +50,6 @@ public: { CheckOther checkOther(tokenizer, settings, errorLogger); - checkOther.nullPointer(); - // Coding style checks checkOther.warningOldStylePointerCast(); checkOther.checkUnsignedDivision(); @@ -82,7 +80,6 @@ public: checkOther.checkFflushOnInputStream(); checkOther.invalidScanf(); - checkOther.nullConstantDereference(); checkOther.checkSelfAssignment(); checkOther.checkIncorrectLogicOperator(); @@ -141,18 +138,6 @@ public: /** @brief str plus char (unusual pointer arithmetic) */ void strPlusChar(); - /** @brief possible null pointer dereference */ - void nullPointer(); - - /** - * @brief Does one part of the check for nullPointer(). - * Checking if pointer is NULL and then dereferencing it.. - */ - void nullPointerByCheckAndDeRef(); - - /** @brief dereferencing null constant (after Tokenizer::simplifyKnownVariables) */ - void nullConstantDereference(); - /** @brief new type of check: check execution paths */ void executionPaths(); @@ -210,9 +195,6 @@ public: void variableScopeError(const Token *tok, const std::string &varname); void conditionAlwaysTrueFalse(const Token *tok, const std::string &truefalse); void strPlusChar(const Token *tok); - void nullPointerError(const Token *tok); // variable name unknown / doesn't exist - void nullPointerError(const Token *tok, const std::string &varname); - void nullPointerError(const Token *tok, const std::string &varname, const unsigned int line); void uninitstringError(const Token *tok, const std::string &varname); void uninitdataError(const Token *tok, const std::string &varname); void uninitvarError(const Token *tok, const std::string &varname); @@ -231,7 +213,6 @@ public: // error sprintfOverlappingDataError(0, "varname"); udivError(0); - nullPointerError(0, "pointer"); uninitstringError(0, "varname"); uninitdataError(0, "varname"); uninitvarError(0, "varname"); @@ -278,7 +259,6 @@ public: // error "* [[OverlappingData|bad usage of the function 'sprintf' (overlapping data)]]\n" "* division with zero\n" - "* null pointer dereferencing\n" "* using uninitialized variables and data\n" "* using fflush() on an input stream\n" "* scoped object destroyed immediately after construction\n" @@ -310,38 +290,6 @@ public: private: - /** - * @brief Does one part of the check for nullPointer(). - * Locate insufficient null-pointer handling after loop - */ - void nullPointerAfterLoop(); - - /** - * @brief Does one part of the check for nullPointer(). - * looping through items in a linked list in a inner loop.. - */ - void nullPointerLinkedList(); - - /** - * @brief Does one part of the check for nullPointer(). - * Dereferencing a struct pointer and then checking if it's NULL.. - */ - void nullPointerStructByDeRefAndChec(); - - /** - * @brief Does one part of the check for nullPointer(). - * Dereferencing a pointer and then checking if it's NULL.. - */ - void nullPointerByDeRefAndChec(); - - /** - * @brief Does one part of the check for nullPointer(). - * -# initialize pointer to 0 - * -# conditionally assign pointer - * -# dereference pointer - */ - void nullPointerConditionalAssignment(); - /** * @brief Used in warningRedundantCode() * Iterates through the %var% tokens in a fully qualified name and concatenates them. diff --git a/lib/lib.pri b/lib/lib.pri index 10c4fb3be..608e130bc 100644 --- a/lib/lib.pri +++ b/lib/lib.pri @@ -6,6 +6,7 @@ HEADERS += $$PWD/check.h \ $$PWD/checkclass.h \ $$PWD/checkexceptionsafety.h \ $$PWD/checkmemoryleak.h \ + $$PWD/checknullpointer.h \ $$PWD/checkobsoletefunctions.h \ $$PWD/checkother.h \ $$PWD/checkpostfixoperator.h \ @@ -30,6 +31,7 @@ SOURCES += $$PWD/checkautovariables.cpp \ $$PWD/checkclass.cpp \ $$PWD/checkexceptionsafety.cpp \ $$PWD/checkmemoryleak.cpp \ + $$PWD/checknullpointer.cpp \ $$PWD/checkobsoletefunctions.cpp \ $$PWD/checkother.cpp \ $$PWD/checkpostfixoperator.cpp \ diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp new file mode 100644 index 000000000..76d409eb7 --- /dev/null +++ b/test/testnullpointer.cpp @@ -0,0 +1,677 @@ +/* + * Cppcheck - A tool for static C/C++ code analysis + * Copyright (C) 2007-2010 Daniel Marjamäki and Cppcheck team. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include "tokenize.h" +#include "checknullpointer.h" +#include "testsuite.h" +#include + +extern std::ostringstream errout; + +class TestNullPointer : public TestFixture +{ +public: + TestNullPointer() : TestFixture("TestNullPointer") + { } + +private: + + + void run() + { + TEST_CASE(nullpointer1); + TEST_CASE(nullpointer2); + TEST_CASE(nullpointer3); // dereferencing struct and then checking if it's null + TEST_CASE(nullpointer4); + TEST_CASE(nullpointer5); // References should not be checked + TEST_CASE(nullpointer6); + TEST_CASE(nullpointer7); + TEST_CASE(nullpointer8); + TEST_CASE(nullpointer9); + TEST_CASE(nullpointer10); // check if pointer is null and then dereference it + } + + void check(const char code[]) + { + // Tokenize.. + Tokenizer tokenizer; + std::istringstream istr(code); + tokenizer.tokenize(istr, "test.cpp"); + + // Clear the error buffer.. + errout.str(""); + + // Check for redundant code.. + Settings settings; + settings._checkCodingStyle = true; + CheckNullPointer checkNullPointer(&tokenizer, &settings, this); + checkNullPointer.nullPointer(); + tokenizer.simplifyTokenList(); + checkNullPointer.nullConstantDereference(); + checkNullPointer.executionPaths(); + } + + + void nullpointer1() + { + check("int foo(const Token *tok)\n" + "{\n" + " while (tok);\n" + " tok = tok->next();\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (error) Possible null pointer dereference: tok\n", errout.str()); + + check("void foo()\n" + "{\n" + " for (const Token *tok = tokens; tok; tok = tok->next())\n" + " {\n" + " while (tok && tok->str() != \";\")\n" + " tok = tok->next();\n" + " }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (error) Possible null pointer dereference: tok - otherwise it is redundant to check if tok is null at line 5\n", errout.str()); + + check("void foo(Token &tok)\n" + "{\n" + " for (int i = 0; i < tok.size(); i++ )\n" + " {\n" + " while (!tok)\n" + " char c = tok.read();\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void foo()\n" + "{\n" + " for (const Token *tok = tokens; tok; tok = tok->next())\n" + " {\n" + " while (tok && tok->str() != \";\")\n" + " tok = tok->next();\n" + " if( !tok ) break;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void foo()\n" + "{\n" + " for (const Token *tok = tokens; tok; tok = tok ? tok->next() : NULL)\n" + " {\n" + " while (tok && tok->str() != \";\")\n" + " tok = tok->next();\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void foo(A*a)\n" + "{\n" + " switch (a->b()) {\n" + " case 1:\n" + " while( a ){\n" + " a = a->next;\n" + " }\n" + " break;\n" + " case 2:\n" + " a->b();\n" + " break;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + // ticket #1923 - no false positive when using else if + check("void f(A *a)\n" + "{\n" + " if (a->x == 1)\n" + " {\n" + " a = a->next;\n" + " }\n" + " else if (a->x == 2) { }\n" + " if (a) { }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + // ticket #2134 - sizeof doesn't dereference + check("void f() {\n" + " int c = 1;\n" + " int *list = NULL;\n" + " sizeof(*list);\n" + " if (!list)\n" + " ;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + } + + void nullpointer2() + { + // Null pointer dereference can only happen with pointers + check("void foo()\n" + "{\n" + " Fred fred;\n" + " while (fred);\n" + " fred.hello();\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + + // Dereferencing a struct and then checking if it is null + // This is checked by this function: + // CheckOther::nullPointerStructByDeRefAndChec + void nullpointer3() + { + // errors.. + check("void foo(struct ABC *abc)\n" + "{\n" + " int a = abc->a;\n" + " if (!abc)\n" + " ;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (error) Possible null pointer dereference: abc - otherwise it is redundant to check if abc is null at line 4\n", errout.str()); + + check("void foo(struct ABC *abc)\n" + "{\n" + " bar(abc->a);\n" + " if (!abc)\n" + " ;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (error) Possible null pointer dereference: abc - otherwise it is redundant to check if abc is null at line 4\n", errout.str()); + + // ok dereferencing in a condition + check("void foo(struct ABC *abc)\n" + "{\n" + " if (abc && abc->a);\n" + " if (!abc)\n" + " ;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + // ok to use a linked list.. + check("void foo(struct ABC *abc)\n" + "{\n" + " abc = abc->next;\n" + " if (!abc)\n" + " ;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + // reassign struct.. + check("void foo(struct ABC *abc)\n" + "{\n" + " int a = abc->a;\n" + " abc = abc->next;\n" + " if (!abc)\n" + " ;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void foo(struct ABC *abc)\n" + "{\n" + " int a = abc->a;\n" + " f(&abc);\n" + " if (!abc)\n" + " ;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + // goto.. + check("void foo(struct ABC *abc)\n" + "{\n" + " int a;\n" + " if (!abc)\n" + " goto out;" + " a = abc->a;\n" + " return;\n" + "out:\n" + " if (!abc)\n" + " ;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + // loops.. + check("void freeAbc(struct ABC *abc)\n" + "{\n" + " while (abc)\n" + " {\n" + " struct ABC *next = abc->next;\n" + " if (abc) delete abc;\n" + " abc = next;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void foo(struct ABC *abc)\n" + "{\n" + " int a = abc->a;" + " do\n" + " {\n" + " if (abc)\n" + " abc = abc->next;\n" + " --a;\n" + " }\n" + " while (a > 0);\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void f()\n" + "{\n" + " for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())\n" + " {\n" + " while (tok && tok->str() != \"{\")\n" + " tok = tok->next();\n" + " if (!tok)\n" + " return;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + // dynamic_cast.. + check("void foo(ABC *abc)\n" + "{\n" + " int a = abc->a;\n" + " if (!dynamic_cast(abc))\n" + " ;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + + // Dereferencing a pointer and then checking if it is null + void nullpointer4() + { + // errors.. + check("void foo(int *p)\n" + "{\n" + " *p = 0;\n" + " if (!p)\n" + " ;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (error) Possible null pointer dereference: p - otherwise it is redundant to check if p is null at line 4\n", errout.str()); + + check("void foo(int *p)\n" + "{\n" + " bar(*p);\n" + " if (!p)\n" + " ;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (error) Possible null pointer dereference: p - otherwise it is redundant to check if p is null at line 4\n", errout.str()); + + // no error + check("void foo()\n" + "{\n" + " int *p;\n" + " f(&p);\n" + " if (!p)\n" + " ;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void foo()\n" + "{\n" + " int **p = f();\n" + " if (!p)\n" + " ;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void foo(int *p)\n" + "{\n" + " if (x)\n" + " p = 0;\n" + " else\n" + " *p = 0;\n" + " if (!p)\n" + " ;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void foo(int x)\n" + "{\n" + " int a = 2 * x;" + " if (x == 0)\n" + " ;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void foo(int *p)\n" + "{\n" + " int var1 = p ? *p : 0;\n" + " if (!p)\n" + " ;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void foo(P *p)\n" + "{\n" + " while (p)\n" + " if (p->check())\n" + " break;\n" + " else\n" + " p = p->next();\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + + void nullpointer5() + { + // errors.. + check("void foo(A &a)\n" + "{\n" + " char c = a.c();\n" + " if (!a)\n" + " return;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + + // Execution paths.. + void nullpointer6() + { + // errors.. + check("static void foo()\n" + "{\n" + " Foo *p = 0;\n" + " if (a == 1)\n" + " p = new FooBar;\n" + " else if (a == 2)\n" + " p = new FooCar;\n" + " p->abcd();\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:8]: (error) Possible null pointer dereference: p\n", errout.str()); + + check("static void foo()\n" + "{\n" + " int *p = 0;\n" + " int *q = p;\n" + " q[0] = 0;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:5]: (error) Possible null pointer dereference: q\n", errout.str()); + + check("static void foo()\n" + "{\n" + " int *p = 0;\n" + " int &r = *p;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (error) Null pointer dereference\n", errout.str()); + + check("static void foo(int x)\n" + "{\n" + " int *p = 0;\n" + " int y = 5 + *p;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (error) Null pointer dereference\n", errout.str()); + + check("static void foo(int x)\n" + "{\n" + " Foo *abc = 0;\n" + " abc->a();\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (error) Possible null pointer dereference: abc\n", errout.str()); + + check("static void foo()\n" + "{\n" + " int *p(0);\n" + " std::cout << *p;" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (error) Null pointer dereference\n", errout.str()); + + check("void f()\n" + "{\n" + " char *c = 0;\n" + " {\n" + " delete c;\n" + " }\n" + " c[0] = 0;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:7]: (error) Possible null pointer dereference: c\n", errout.str()); + + check("static void foo()\n" + "{\n" + " int *p = 0;\n" + " if (3 > *p);\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (error) Null pointer dereference\n", errout.str()); + + check("void f()\n" + "{\n" + " if (x) {\n" + " char *c = 0;\n" + " *c = 0;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:5]: (error) Null pointer dereference\n", errout.str()); + + // no false positive.. + check("static void foo()\n" + "{\n" + " Foo *p = 0;\n" + " p = new Foo;\n" + " p->abcd();\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("static void foo()\n" + "{\n" + " Foo *p = 0;\n" + " if (!p)\n" + " return;\n" + " p->abcd();\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("static void foo()\n" + "{\n" + " int *p = 0;\n" + " exit();\n" + " *p = 0;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("static void foo(int a)\n" + "{\n" + " Foo *p = 0;\n" + " if (a && p)\n" + " p->do_something();\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void foo()\n" + "{\n" + " int sz = sizeof((*(struct dummy *)0).x);\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void get_offset(long &offset)\n" + "{\n" + " mystruct * temp; temp = 0;\n" + " offset = (long)(&(temp->z));\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + // Ticket #1893 - try/catch inside else + check("int *test(int *Z)\n" + "{\n" + " int *Q=NULL;\n" + " if (Z) {\n" + " Q = Z;\n" + " }\n" + " else {\n" + " Z = new int;\n" + " try {\n" + " } catch(...) {\n" + " }\n" + " Q = Z;\n" + " }\n" + " *Q=1;\n" + " return Q;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("int *test(int *Z)\n" + "{\n" + " int *Q=NULL;\n" + " if (Z) {\n" + " Q = Z;\n" + " }\n" + " else {\n" + " try {\n" + " } catch(...) {\n" + " }\n" + " }\n" + " *Q=1;\n" + " return Q;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:12]: (error) Possible null pointer dereference: Q\n", errout.str()); + + // Ticket #2052 (false positive for 'else continue;') + check("void f() {\n" + " for (int x = 0; x < 5; ++x) {" + " int *p = 0;\n" + " if (a(x)) p=b(x);\n" + " else continue;\n" + " *p = 0;\n" + " }\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + // function pointer.. + check("void foo()\n" + "{\n" + " void (*f)();\n" + " f = 0;\n" + " f();\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:5]: (error) Possible null pointer dereference: f\n", errout.str()); + + check("static void foo()\n" + "{\n" + " int *p = 0;\n" + " int *p2 = 0;\n" + " int r = *p;\n" + " int r2 = *p2;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:5]: (error) Null pointer dereference\n" + "[test.cpp:6]: (error) Null pointer dereference\n", errout.str()); + + // loops.. + check("void f() {\n" + " int *p = 0;\n" + " for (int i = 0; i < 10; ++i) {\n" + " int x = *p + 1;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (error) Possible null pointer dereference: p\n", errout.str()); + + check("void f(int a) {\n" + " const char *p = 0;\n" + " if (a) {\n" + " p = \"abcd\";\n" + " }\n" + " for (int i = 0; i < 3; i++) {\n" + " if (a && (p[i] == '1'));\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + + void nullpointer7() + { + check("void foo()\n" + "{\n" + " wxLongLong x = 0;\n" + " int y = x.GetValue();\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + + void nullpointer8() + { + check("void foo()\n" + "{\n" + " const char * x = 0;\n" + " strdup(x);\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (error) Possible null pointer dereference: x\n", errout.str()); + check("void foo()\n" + "{\n" + " char const * x = 0;\n" + " strdup(x);\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (error) Possible null pointer dereference: x\n", errout.str()); + } + + void nullpointer9() //#ticket 1778 + { + check("void foo()\n" + "{\n" + " std::string * x = 0;\n" + " *x = \"test\";\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (error) Possible null pointer dereference: x\n", errout.str()); + } + + // Check if pointer is null and the dereference it + void nullpointer10() + { + check("void foo(char *p) {\n" + " if (!p) {\n" + " }\n" + " *p = 0;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (error) Possible null pointer dereference: p\n", errout.str()); + + check("void foo(abc *p) {\n" + " if (!p) {\n" + " }\n" + " else if (!p->x) {\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void foo(char *p) {\n" + " if (!p) {\n" + " abort();\n" + " }\n" + " *p = 0;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void foo(char *p) {\n" + " if (!p) {\n" + " throw x;\n" + " }\n" + " *p = 0;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void foo() {\n" + " if (!p) {\n" + " switch (x) { }\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + // This is why this check can't be used on the simplified token list + check("void f(Foo *foo) {\n" + " if (!dynamic_cast(foo)) {\n" + " *foo = 0;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } +}; + +REGISTER_TEST(TestNullPointer) + diff --git a/test/testother.cpp b/test/testother.cpp index c41394e0a..457accadf 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -59,17 +59,6 @@ private: TEST_CASE(varScope9); // classes may have extra side-effects TEST_CASE(varScope10); // Undefined macro FOR - TEST_CASE(nullpointer1); - TEST_CASE(nullpointer2); - TEST_CASE(nullpointer3); // dereferencing struct and then checking if it's null - TEST_CASE(nullpointer4); - TEST_CASE(nullpointer5); // References should not be checked - TEST_CASE(nullpointer6); - TEST_CASE(nullpointer7); - TEST_CASE(nullpointer8); - TEST_CASE(nullpointer9); - TEST_CASE(nullpointer10); // check if pointer is null and then dereference it - TEST_CASE(uninitvar1); TEST_CASE(uninitvar_alloc); // data is allocated but not initialized TEST_CASE(uninitvar_arrays); // arrays @@ -594,633 +583,6 @@ private: } - void checkNullPointer(const char code[]) - { - // Tokenize.. - Tokenizer tokenizer; - std::istringstream istr(code); - tokenizer.tokenize(istr, "test.cpp"); - - // Clear the error buffer.. - errout.str(""); - - // Check for redundant code.. - Settings settings; - settings._checkCodingStyle = true; - CheckOther checkOther(&tokenizer, &settings, this); - checkOther.nullPointer(); - - tokenizer.simplifyTokenList(); - checkOther.nullConstantDereference(); - checkOther.executionPaths(); - } - - - void nullpointer1() - { - checkNullPointer("int foo(const Token *tok)\n" - "{\n" - " while (tok);\n" - " tok = tok->next();\n" - "}\n"); - ASSERT_EQUALS("[test.cpp:4]: (error) Possible null pointer dereference: tok\n", errout.str()); - - checkNullPointer("void foo()\n" - "{\n" - " for (const Token *tok = tokens; tok; tok = tok->next())\n" - " {\n" - " while (tok && tok->str() != \";\")\n" - " tok = tok->next();\n" - " }\n" - "}\n"); - ASSERT_EQUALS("[test.cpp:3]: (error) Possible null pointer dereference: tok - otherwise it is redundant to check if tok is null at line 5\n", errout.str()); - - checkNullPointer("void foo(Token &tok)\n" - "{\n" - " for (int i = 0; i < tok.size(); i++ )\n" - " {\n" - " while (!tok)\n" - " char c = tok.read();\n" - " }\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - checkNullPointer("void foo()\n" - "{\n" - " for (const Token *tok = tokens; tok; tok = tok->next())\n" - " {\n" - " while (tok && tok->str() != \";\")\n" - " tok = tok->next();\n" - " if( !tok ) break;\n" - " }\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - checkNullPointer("void foo()\n" - "{\n" - " for (const Token *tok = tokens; tok; tok = tok ? tok->next() : NULL)\n" - " {\n" - " while (tok && tok->str() != \";\")\n" - " tok = tok->next();\n" - " }\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - checkNullPointer("void foo(A*a)\n" - "{\n" - " switch (a->b()) {\n" - " case 1:\n" - " while( a ){\n" - " a = a->next;\n" - " }\n" - " break;\n" - " case 2:\n" - " a->b();\n" - " break;\n" - " }\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - // ticket #1923 - no false positive when using else if - checkNullPointer("void f(A *a)\n" - "{\n" - " if (a->x == 1)\n" - " {\n" - " a = a->next;\n" - " }\n" - " else if (a->x == 2) { }\n" - " if (a) { }\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - // ticket #2134 - sizeof doesn't dereference - checkNullPointer("void f() {\n" - " int c = 1;\n" - " int *list = NULL;\n" - " sizeof(*list);\n" - " if (!list)\n" - " ;\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - } - - void nullpointer2() - { - // Null pointer dereference can only happen with pointers - checkNullPointer("void foo()\n" - "{\n" - " Fred fred;\n" - " while (fred);\n" - " fred.hello();\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - } - - // Dereferencing a struct and then checking if it is null - // This is checked by this function: - // CheckOther::nullPointerStructByDeRefAndChec - void nullpointer3() - { - // errors.. - checkNullPointer("void foo(struct ABC *abc)\n" - "{\n" - " int a = abc->a;\n" - " if (!abc)\n" - " ;\n" - "}\n"); - ASSERT_EQUALS("[test.cpp:3]: (error) Possible null pointer dereference: abc - otherwise it is redundant to check if abc is null at line 4\n", errout.str()); - - checkNullPointer("void foo(struct ABC *abc)\n" - "{\n" - " bar(abc->a);\n" - " if (!abc)\n" - " ;\n" - "}\n"); - ASSERT_EQUALS("[test.cpp:3]: (error) Possible null pointer dereference: abc - otherwise it is redundant to check if abc is null at line 4\n", errout.str()); - - // ok dereferencing in a condition - checkNullPointer("void foo(struct ABC *abc)\n" - "{\n" - " if (abc && abc->a);\n" - " if (!abc)\n" - " ;\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - // ok to use a linked list.. - checkNullPointer("void foo(struct ABC *abc)\n" - "{\n" - " abc = abc->next;\n" - " if (!abc)\n" - " ;\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - // reassign struct.. - checkNullPointer("void foo(struct ABC *abc)\n" - "{\n" - " int a = abc->a;\n" - " abc = abc->next;\n" - " if (!abc)\n" - " ;\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - checkNullPointer("void foo(struct ABC *abc)\n" - "{\n" - " int a = abc->a;\n" - " f(&abc);\n" - " if (!abc)\n" - " ;\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - // goto.. - checkNullPointer("void foo(struct ABC *abc)\n" - "{\n" - " int a;\n" - " if (!abc)\n" - " goto out;" - " a = abc->a;\n" - " return;\n" - "out:\n" - " if (!abc)\n" - " ;\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - // loops.. - checkNullPointer("void freeAbc(struct ABC *abc)\n" - "{\n" - " while (abc)\n" - " {\n" - " struct ABC *next = abc->next;\n" - " if (abc) delete abc;\n" - " abc = next;\n" - " }\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - checkNullPointer("void foo(struct ABC *abc)\n" - "{\n" - " int a = abc->a;" - " do\n" - " {\n" - " if (abc)\n" - " abc = abc->next;\n" - " --a;\n" - " }\n" - " while (a > 0);\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - checkNullPointer("void f()\n" - "{\n" - " for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())\n" - " {\n" - " while (tok && tok->str() != \"{\")\n" - " tok = tok->next();\n" - " if (!tok)\n" - " return;\n" - " }\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - // dynamic_cast.. - checkNullPointer("void foo(ABC *abc)\n" - "{\n" - " int a = abc->a;\n" - " if (!dynamic_cast(abc))\n" - " ;\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - } - - // Dereferencing a pointer and then checking if it is null - void nullpointer4() - { - // errors.. - checkNullPointer("void foo(int *p)\n" - "{\n" - " *p = 0;\n" - " if (!p)\n" - " ;\n" - "}\n"); - ASSERT_EQUALS("[test.cpp:3]: (error) Possible null pointer dereference: p - otherwise it is redundant to check if p is null at line 4\n", errout.str()); - - checkNullPointer("void foo(int *p)\n" - "{\n" - " bar(*p);\n" - " if (!p)\n" - " ;\n" - "}\n"); - ASSERT_EQUALS("[test.cpp:3]: (error) Possible null pointer dereference: p - otherwise it is redundant to check if p is null at line 4\n", errout.str()); - - // no error - checkNullPointer("void foo()\n" - "{\n" - " int *p;\n" - " f(&p);\n" - " if (!p)\n" - " ;\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - checkNullPointer("void foo()\n" - "{\n" - " int **p = f();\n" - " if (!p)\n" - " ;\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - checkNullPointer("void foo(int *p)\n" - "{\n" - " if (x)\n" - " p = 0;\n" - " else\n" - " *p = 0;\n" - " if (!p)\n" - " ;\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - checkNullPointer("void foo(int x)\n" - "{\n" - " int a = 2 * x;" - " if (x == 0)\n" - " ;\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - checkNullPointer("void foo(int *p)\n" - "{\n" - " int var1 = p ? *p : 0;\n" - " if (!p)\n" - " ;\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - checkNullPointer("void foo(P *p)\n" - "{\n" - " while (p)\n" - " if (p->check())\n" - " break;\n" - " else\n" - " p = p->next();\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - } - - void nullpointer5() - { - // errors.. - checkNullPointer("void foo(A &a)\n" - "{\n" - " char c = a.c();\n" - " if (!a)\n" - " return;\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - } - - // Execution paths.. - void nullpointer6() - { - // errors.. - checkNullPointer("static void foo()\n" - "{\n" - " Foo *p = 0;\n" - " if (a == 1)\n" - " p = new FooBar;\n" - " else if (a == 2)\n" - " p = new FooCar;\n" - " p->abcd();\n" - "}\n"); - ASSERT_EQUALS("[test.cpp:8]: (error) Possible null pointer dereference: p\n", errout.str()); - - checkNullPointer("static void foo()\n" - "{\n" - " int *p = 0;\n" - " int *q = p;\n" - " q[0] = 0;\n" - "}\n"); - ASSERT_EQUALS("[test.cpp:5]: (error) Possible null pointer dereference: q\n", errout.str()); - - checkNullPointer("static void foo()\n" - "{\n" - " int *p = 0;\n" - " int &r = *p;\n" - "}\n"); - ASSERT_EQUALS("[test.cpp:4]: (error) Null pointer dereference\n", errout.str()); - - checkNullPointer("static void foo(int x)\n" - "{\n" - " int *p = 0;\n" - " int y = 5 + *p;\n" - "}\n"); - ASSERT_EQUALS("[test.cpp:4]: (error) Null pointer dereference\n", errout.str()); - - checkNullPointer("static void foo(int x)\n" - "{\n" - " Foo *abc = 0;\n" - " abc->a();\n" - "}\n"); - ASSERT_EQUALS("[test.cpp:4]: (error) Possible null pointer dereference: abc\n", errout.str()); - - checkNullPointer("static void foo()\n" - "{\n" - " int *p(0);\n" - " std::cout << *p;" - "}\n"); - ASSERT_EQUALS("[test.cpp:4]: (error) Null pointer dereference\n", errout.str()); - - checkNullPointer("void f()\n" - "{\n" - " char *c = 0;\n" - " {\n" - " delete c;\n" - " }\n" - " c[0] = 0;\n" - "}\n"); - ASSERT_EQUALS("[test.cpp:7]: (error) Possible null pointer dereference: c\n", errout.str()); - - checkNullPointer("static void foo()\n" - "{\n" - " int *p = 0;\n" - " if (3 > *p);\n" - "}\n"); - ASSERT_EQUALS("[test.cpp:4]: (error) Null pointer dereference\n", errout.str()); - - checkNullPointer("void f()\n" - "{\n" - " if (x) {\n" - " char *c = 0;\n" - " *c = 0;\n" - " }\n" - "}\n"); - ASSERT_EQUALS("[test.cpp:5]: (error) Null pointer dereference\n", errout.str()); - - // no false positive.. - checkNullPointer("static void foo()\n" - "{\n" - " Foo *p = 0;\n" - " p = new Foo;\n" - " p->abcd();\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - checkNullPointer("static void foo()\n" - "{\n" - " Foo *p = 0;\n" - " if (!p)\n" - " return;\n" - " p->abcd();\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - checkNullPointer("static void foo()\n" - "{\n" - " int *p = 0;\n" - " exit();\n" - " *p = 0;\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - checkNullPointer("static void foo(int a)\n" - "{\n" - " Foo *p = 0;\n" - " if (a && p)\n" - " p->do_something();\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - checkNullPointer("void foo()\n" - "{\n" - " int sz = sizeof((*(struct dummy *)0).x);\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - checkNullPointer("void get_offset(long &offset)\n" - "{\n" - " mystruct * temp; temp = 0;\n" - " offset = (long)(&(temp->z));\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - // Ticket #1893 - try/catch inside else - checkNullPointer("int *test(int *Z)\n" - "{\n" - " int *Q=NULL;\n" - " if (Z) {\n" - " Q = Z;\n" - " }\n" - " else {\n" - " Z = new int;\n" - " try {\n" - " } catch(...) {\n" - " }\n" - " Q = Z;\n" - " }\n" - " *Q=1;\n" - " return Q;\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - checkNullPointer("int *test(int *Z)\n" - "{\n" - " int *Q=NULL;\n" - " if (Z) {\n" - " Q = Z;\n" - " }\n" - " else {\n" - " try {\n" - " } catch(...) {\n" - " }\n" - " }\n" - " *Q=1;\n" - " return Q;\n" - "}\n"); - ASSERT_EQUALS("[test.cpp:12]: (error) Possible null pointer dereference: Q\n", errout.str()); - - // Ticket #2052 (false positive for 'else continue;') - checkNullPointer("void f() {\n" - " for (int x = 0; x < 5; ++x) {" - " int *p = 0;\n" - " if (a(x)) p=b(x);\n" - " else continue;\n" - " *p = 0;\n" - " }\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - // function pointer.. - checkNullPointer("void foo()\n" - "{\n" - " void (*f)();\n" - " f = 0;\n" - " f();\n" - "}\n"); - ASSERT_EQUALS("[test.cpp:5]: (error) Possible null pointer dereference: f\n", errout.str()); - - checkNullPointer("static void foo()\n" - "{\n" - " int *p = 0;\n" - " int *p2 = 0;\n" - " int r = *p;\n" - " int r2 = *p2;\n" - "}\n"); - ASSERT_EQUALS("[test.cpp:5]: (error) Null pointer dereference\n" - "[test.cpp:6]: (error) Null pointer dereference\n", errout.str()); - - // loops.. - checkNullPointer("void f() {\n" - " int *p = 0;\n" - " for (int i = 0; i < 10; ++i) {\n" - " int x = *p + 1;\n" - " }\n" - "}\n"); - ASSERT_EQUALS("[test.cpp:4]: (error) Possible null pointer dereference: p\n", errout.str()); - - checkNullPointer("void f(int a) {\n" - " const char *p = 0;\n" - " if (a) {\n" - " p = \"abcd\";\n" - " }\n" - " for (int i = 0; i < 3; i++) {\n" - " if (a && (p[i] == '1'));\n" - " }\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - } - - void nullpointer7() - { - checkNullPointer("void foo()\n" - "{\n" - " wxLongLong x = 0;\n" - " int y = x.GetValue();\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - } - - void nullpointer8() - { - checkNullPointer("void foo()\n" - "{\n" - " const char * x = 0;\n" - " strdup(x);\n" - "}\n"); - ASSERT_EQUALS("[test.cpp:4]: (error) Possible null pointer dereference: x\n", errout.str()); - checkNullPointer("void foo()\n" - "{\n" - " char const * x = 0;\n" - " strdup(x);\n" - "}\n"); - ASSERT_EQUALS("[test.cpp:4]: (error) Possible null pointer dereference: x\n", errout.str()); - } - - void nullpointer9() //#ticket 1778 - { - checkNullPointer("void foo()\n" - "{\n" - " std::string * x = 0;\n" - " *x = \"test\";\n" - "}\n"); - ASSERT_EQUALS("[test.cpp:4]: (error) Possible null pointer dereference: x\n", errout.str()); - } - - // Check if pointer is null and the dereference it - void nullpointer10() - { - checkNullPointer("void foo(char *p) {\n" - " if (!p) {\n" - " }\n" - " *p = 0;\n" - "}\n"); - ASSERT_EQUALS("[test.cpp:4]: (error) Possible null pointer dereference: p\n", errout.str()); - - checkNullPointer("void foo(abc *p) {\n" - " if (!p) {\n" - " }\n" - " else if (!p->x) {\n" - " }\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - checkNullPointer("void foo(char *p) {\n" - " if (!p) {\n" - " abort();\n" - " }\n" - " *p = 0;\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - checkNullPointer("void foo(char *p) {\n" - " if (!p) {\n" - " throw x;\n" - " }\n" - " *p = 0;\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - checkNullPointer("void foo() {\n" - " if (!p) {\n" - " switch (x) { }\n" - " }\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - // This is why this check can't be used on the simplified token list - checkNullPointer("void f(Foo *foo) {\n" - " if (!dynamic_cast(foo)) {\n" - " *foo = 0;\n" - " }\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - } - void checkUninitVar(const char code[]) { // Tokenize.. @@ -1528,13 +890,13 @@ private: "}\n"); ASSERT_EQUALS("", errout.str()); - checkNullPointer("void f(int a)\n" - "{\n" - " if (a) {\n" - " char *p;\n" - " *p = 0;\n" - " }\n" - "}\n"); + checkUninitVar("void f(int a)\n" + "{\n" + " if (a) {\n" + " char *p;\n" + " *p = 0;\n" + " }\n" + "}\n"); ASSERT_EQUALS("[test.cpp:5]: (error) Uninitialized variable: p\n", errout.str()); // +=