From 7443883b9c0d776096edf7d4e2366604a775c1e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Mon, 15 Jul 2013 18:55:40 +0200 Subject: [PATCH] Library: Improved handling in CheckNullPointer::parseFunctionCall for Library data --- lib/checknullpointer.cpp | 33 ++++++++++++++++-------- lib/checknullpointer.h | 2 ++ lib/checkuninitvar.cpp | 6 ++--- lib/library.h | 21 ++++++++++++++++ test/testnullpointer.cpp | 54 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 102 insertions(+), 14 deletions(-) diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp index 2d4f7255b..39e72f8ad 100644 --- a/lib/checknullpointer.cpp +++ b/lib/checknullpointer.cpp @@ -36,10 +36,11 @@ namespace { * @brief parse a function call and extract information about variable usage * @param tok first token * @param var variables that the function read / write. + * @param library --library files data * @param value 0 => invalid with null pointers as parameter. * 1-.. => only invalid with uninitialized data. */ -void CheckNullPointer::parseFunctionCall(const Token &tok, std::list &var, unsigned char value) +void CheckNullPointer::parseFunctionCall(const Token &tok, std::list &var, const Library *library, unsigned char value) { // standard functions that dereference first parameter.. static std::set functionNames1_all; // used no matter what 'value' is @@ -237,7 +238,11 @@ void CheckNullPointer::parseFunctionCall(const Token &tok, std::liststr() != "0") // Only if length (second parameter) is not zero var.push_back(firstParam); - } + else if (value == 0 && library != NULL && library->isnullargbad(tok.str(),1)) + var.push_back(firstParam); + else if (value == 1 && library != NULL && library->isuninitargbad(tok.str(),1)) + var.push_back(firstParam); + } // 2nd parameter.. if ((value == 0 && Token::Match(secondParam, "0|NULL ,|)")) || (secondParam && secondParam->varId() > 0)) { @@ -245,6 +250,10 @@ void CheckNullPointer::parseFunctionCall(const Token &tok, std::listisnullargbad(tok.str(),2)) + var.push_back(secondParam); + else if (value == 1 && library != NULL && library->isuninitargbad(tok.str(),2)) + var.push_back(secondParam); } if (Token::Match(&tok, "printf|sprintf|snprintf|fprintf|fnprintf|scanf|sscanf|fscanf|wprintf|swprintf|fwprintf|wscanf|swscanf|fwscanf")) { @@ -808,7 +817,7 @@ void CheckNullPointer::nullPointerByDeRefAndChec() if (Token::Match(tok2->next(), "%var% ( %varid% ,", varid)) { std::list varlist; - parseFunctionCall(*(tok2->next()), varlist, 0); + parseFunctionCall(*(tok2->next()), varlist, &_settings->library, 0); if (!varlist.empty() && varlist.front() == tok2->tokAt(3)) { nullPointerError(tok2->tokAt(3), varname, tok, inconclusive); break; @@ -1069,7 +1078,7 @@ void CheckNullPointer::nullPointerByCheckAndDeRef() // function call, check if pointer is dereferenced if (Token::Match(tok2, "%var% (") && !Token::Match(tok2, "if|while")) { std::list vars; - parseFunctionCall(*tok2, vars, 0); + parseFunctionCall(*tok2, vars, &_settings->library, 0); for (std::list::const_iterator it = vars.begin(); it != vars.end(); ++it) { if (Token::Match(*it, "%varid% [,)]", varid)) { nullPointerError(*it, pointerName, vartok, inconclusive); @@ -1167,7 +1176,7 @@ void CheckNullPointer::nullConstantDereference() nullPointerError(tok); } else { // function call std::list var; - parseFunctionCall(*tok, var, 0); + parseFunctionCall(*tok, var, &_settings->library, 0); // is one of the var items a NULL pointer? for (std::list::const_iterator it = var.begin(); it != var.end(); ++it) { @@ -1351,16 +1360,18 @@ void CheckNullPointer::nullPointerDefaultArgument() class Nullpointer : public ExecutionPath { public: /** Startup constructor */ - Nullpointer(Check *c, const SymbolDatabase* symbolDatabase_) : ExecutionPath(c, 0), symbolDatabase(symbolDatabase_), null(false) { + Nullpointer(Check *c, const SymbolDatabase* symbolDatabase_, const Library *lib) : ExecutionPath(c, 0), symbolDatabase(symbolDatabase_), library(lib), null(false) { } private: const SymbolDatabase* symbolDatabase; + const Library *library; /** Create checking of specific variable: */ - Nullpointer(Check *c, const unsigned int id, const std::string &name, const SymbolDatabase* symbolDatabase_) + Nullpointer(Check *c, const unsigned int id, const std::string &name, const SymbolDatabase* symbolDatabase_, const Library *lib) : ExecutionPath(c, id), symbolDatabase(symbolDatabase_), + library(lib), varname(name), null(false) { } @@ -1430,7 +1441,7 @@ private: // Pointer declaration declaration? const Variable *var = tok.variable(); if (var && var->isPointer() && var->nameToken() == &tok) - checks.push_back(new Nullpointer(owner, var->varId(), var->name(), symbolDatabase)); + checks.push_back(new Nullpointer(owner, var->varId(), var->name(), symbolDatabase, library)); } if (Token::simpleMatch(&tok, "try {")) { @@ -1450,7 +1461,7 @@ private: // parse usage.. std::list var; - CheckNullPointer::parseFunctionCall(tok, var, 0); + CheckNullPointer::parseFunctionCall(tok, var, library, 0); for (std::list::const_iterator it = var.begin(); it != var.end(); ++it) dereference(checks, *it); } @@ -1518,7 +1529,7 @@ private: if (Token::Match(&tok, "!| %var% (")) { std::list var; - CheckNullPointer::parseFunctionCall(tok.str() == "!" ? *tok.next() : tok, var, 0); + CheckNullPointer::parseFunctionCall(tok.str() == "!" ? *tok.next() : tok, var, library, 0); for (std::list::const_iterator it = var.begin(); it != var.end(); ++it) dereference(checks, *it); } @@ -1544,7 +1555,7 @@ private: void CheckNullPointer::executionPaths() { // Check for null pointer errors.. - Nullpointer c(this, _tokenizer->getSymbolDatabase()); + Nullpointer c(this, _tokenizer->getSymbolDatabase(), &_settings->library); checkExecutionPaths(_tokenizer->getSymbolDatabase(), &c); } diff --git a/lib/checknullpointer.h b/lib/checknullpointer.h index 0fbfd0a2b..e8f02b46c 100644 --- a/lib/checknullpointer.h +++ b/lib/checknullpointer.h @@ -63,11 +63,13 @@ public: * @brief parse a function call and extract information about variable usage * @param tok first token * @param var variables that the function read / write. + * @param library --library files data * @param value 0 => invalid with null pointers as parameter. * non-zero => invalid with uninitialized data. */ static void parseFunctionCall(const Token &tok, std::list &var, + const Library *library, unsigned char value); /** diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp index ae7eee1e7..577d664cc 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -622,7 +622,7 @@ private: // parse usage.. { std::list var1; - CheckNullPointer::parseFunctionCall(tok, var1, 1); + CheckNullPointer::parseFunctionCall(tok, var1, library, 1); for (std::list::const_iterator it = var1.begin(); it != var1.end(); ++it) { // does iterator point at first function parameter? const bool firstPar(*it == tok.tokAt(2)); @@ -643,7 +643,7 @@ private: // Using uninitialized pointer is bad if using null pointer is bad std::list var2; - CheckNullPointer::parseFunctionCall(tok, var2, 0); + CheckNullPointer::parseFunctionCall(tok, var2, library, 0); for (std::list::const_iterator it = var2.begin(); it != var2.end(); ++it) { if (std::find(var1.begin(), var1.end(), *it) == var1.end()) use_dead_pointer(checks, *it); @@ -886,7 +886,7 @@ private: else if (Token::Match(&tok, "!| %var% (")) { const Token * const ftok = (tok.str() == "!") ? tok.next() : &tok; std::list var1; - CheckNullPointer::parseFunctionCall(*ftok, var1, 1); + CheckNullPointer::parseFunctionCall(*ftok, var1, library, 1); for (std::list::const_iterator it = var1.begin(); it != var1.end(); ++it) { // is function memset/memcpy/etc? if (ftok->str().compare(0,3,"mem") == 0) diff --git a/lib/library.h b/lib/library.h index cc86233af..c4087a82f 100644 --- a/lib/library.h +++ b/lib/library.h @@ -80,6 +80,16 @@ public: // function name, argument nr => argument data std::map > functionArgument; + bool isnullargbad(const std::string &functionName, int argnr) const { + const Argument *arg = getarg(functionName,argnr); + return arg && arg->nullpointer; + } + + bool isuninitargbad(const std::string &functionName, int argnr) const { + const Argument *arg = getarg(functionName,argnr); + return arg && arg->uninitdata; + } + std::set returnuninitdata; private: @@ -88,6 +98,17 @@ private: std::map _dealloc; // deallocation functions std::map _noreturn; // is function noreturn? + const Argument * getarg(const std::string &functionName, int argnr) const { + std::map >::const_iterator it1; + it1 = functionArgument.find(functionName); + if (it1 != functionArgument.end()) { + const std::map::const_iterator it2 = it1->second.find(argnr); + if (it2 != it1->second.end()) + return &it2->second; + } + return NULL; + } + int getid(const std::map &data, const std::string &name) const { const std::map::const_iterator it = data.find(name); return (it == data.end()) ? 0 : it->second; diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index f98674f15..eb16d3473 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -72,6 +72,7 @@ private: TEST_CASE(nullpointerStdString); TEST_CASE(nullpointerStdStream); TEST_CASE(functioncall); + TEST_CASE(functioncalllibrary); // use Library to parse function call TEST_CASE(crash1); TEST_CASE(functioncallDefaultArguments); } @@ -2105,6 +2106,59 @@ private: } } + void functioncalllibrary() { + Settings settings; + Tokenizer tokenizer(&settings,this); + std::istringstream code("void f() { int a,b; x(a,b); }"); + tokenizer.tokenize(code,"test.c"); + const Token *xtok = Token::findsimplematch(tokenizer.tokens(), "x"); + + // nothing bad.. + { + Library library; + Library::Argument arg = {0}; + library.functionArgument["x"][1] = arg; + library.functionArgument["x"][2] = arg; + + std::list null, uninit; + CheckNullPointer::parseFunctionCall(*xtok, null, &library, 0U); + CheckNullPointer::parseFunctionCall(*xtok, uninit, &library, 1U); + ASSERT_EQUALS(0U, null.size()); + ASSERT_EQUALS(0U, uninit.size()); + } + + // for 1st parameter null pointer is not ok.. + { + Library library; + Library::Argument arg = {0}; + library.functionArgument["x"][1] = arg; + library.functionArgument["x"][2] = arg; + library.functionArgument["x"][1].nullpointer = true; + + std::list null,uninit; + CheckNullPointer::parseFunctionCall(*xtok, null, &library, 0U); + CheckNullPointer::parseFunctionCall(*xtok, uninit, &library, 1U); + ASSERT_EQUALS(1U, null.size()); + ASSERT_EQUALS("a", null.front()->str()); + ASSERT_EQUALS(0U, uninit.size()); + } + + // for 2nd parameter uninit data is not ok.. + { + Library library; + Library::Argument arg = {0}; + library.functionArgument["x"][1] = arg; + library.functionArgument["x"][2] = arg; + library.functionArgument["x"][2].uninitdata = true; + + std::list null,uninit; + CheckNullPointer::parseFunctionCall(*xtok, null, &library, 0U); + CheckNullPointer::parseFunctionCall(*xtok, uninit, &library, 1U); + ASSERT_EQUALS(0U, null.size()); + ASSERT_EQUALS(1U, uninit.size()); + ASSERT_EQUALS("b", uninit.front()->str()); + } + } void functioncallDefaultArguments() {