walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Fixed false negative: nullpointer passed as std::string argument (#7927) 

Refactorization: Removed dead code from CheckNullPointer::parseFunctionCall()
This commit is contained in:
PKEuS 2017-02-24 19:10:34 +01:00
parent fef52f2ea1
commit b08f99a082
3 changed files with 28 additions and 64 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
lib/checknullpointer.cpp
View File

@ -48,7 +48,7 @@ static bool checkNullpointerFunctionCallPlausibility(const Function* func, unsig
* @param value 0 => invalid with null pointers as parameter. * @param value 0 => invalid with null pointers as parameter.
* 1-.. => only invalid with uninitialized data. * 1-.. => only invalid with uninitialized data.
*/ */
void CheckNullPointer::parseFunctionCall(const Token &tok, std::list<const Token *> &var, const Library *library, unsigned char value) void CheckNullPointer::parseFunctionCall(const Token &tok, std::list<const Token *> &var, const Library *library)
{ {
if (Token::Match(&tok, "%name% ( )") || !tok.tokAt(2)) if (Token::Match(&tok, "%name% ( )") || !tok.tokAt(2))
return; return;
@ -57,21 +57,18 @@ void CheckNullPointer::parseFunctionCall(const Token &tok, std::list<const Token
const Token* secondParam = firstParam->nextArgument(); const Token* secondParam = firstParam->nextArgument();
// 1st parameter.. // 1st parameter..
if (Token::Match(firstParam, "%var% ,|)") || if (Token::Match(&tok, "snprintf|vsnprintf|fnprintf|vfnprintf") && secondParam && secondParam->str() != "0") // Only if length (second parameter) is not zero
(value == 0 && Token::Match(firstParam, "0|NULL ,|)"))) { var.push_back(firstParam);
if (value == 0 && Token::Match(&tok, "snprintf|vsnprintf|fnprintf|vfnprintf") && secondParam && secondParam->str() != "0") // Only if length (second parameter) is not zero
var.push_back(firstParam);
}
// Library if (library || tok.function() != nullptr) {
if (library) {
const Token *param = firstParam; const Token *param = firstParam;
int argnr = 1; int argnr = 1;
while (param) { while (param) {
if (Token::Match(param, "%var% ,|)") || (value==0 && Token::Match(param, "0|NULL ,|)"))) { if (library && library->isnullargbad(&tok, argnr) && checkNullpointerFunctionCallPlausibility(tok.function(), argnr))
if (value == 0 && library->isnullargbad(&tok, argnr) && checkNullpointerFunctionCallPlausibility(tok.function(), argnr)) var.push_back(param);
var.push_back(param); else if (tok.function()) {
else if (value == 1 && library->isuninitargbad(&tok, argnr)) const Variable* argVar = tok.function()->getArgumentVar(argnr-1);
if (argVar && argVar->isStlStringType() && !argVar->isArrayOrPointer())
var.push_back(param); var.push_back(param);
} }
param = param->nextArgument(); param = param->nextArgument();
@ -124,10 +121,8 @@ void CheckNullPointer::parseFunctionCall(const Token &tok, std::list<const Token
if (_continue) if (_continue)
continue; continue;
if ((*i == 'n' || *i == 's' || scan) && (!scan || value == 0)) { if ((*i == 'n' || *i == 's' || scan)) {
if ((value == 0 && argListTok->str() == "0") || Token::Match(argListTok, "%var% [,)]")) { var.push_back(argListTok);
var.push_back(argListTok);
}
} }
if (*i != 'm') // %m is a non-standard glibc extension that requires no parameter if (*i != 'm') // %m is a non-standard glibc extension that requires no parameter
@ -345,7 +340,7 @@ void CheckNullPointer::nullPointerByDeRefAndChec()
if (!ftok || !ftok->previous()) if (!ftok || !ftok->previous())
continue; continue;
std::list<const Token *> varlist; std::list<const Token *> varlist;
parseFunctionCall(*ftok->previous(), varlist, &_settings->library, 0); parseFunctionCall(*ftok->previous(), varlist, &_settings->library);
if (std::find(varlist.begin(), varlist.end(), tok) != varlist.end()) { if (std::find(varlist.begin(), varlist.end(), tok) != varlist.end()) {
if (value->condition == nullptr) if (value->condition == nullptr)
nullPointerError(tok, tok->str(), false, value->defaultArg, !value->isKnown()); nullPointerError(tok, tok->str(), false, value->defaultArg, !value->isKnown());
@ -422,7 +417,7 @@ void CheckNullPointer::nullConstantDereference()
nullPointerError(tok); nullPointerError(tok);
} else { // function call } else { // function call
std::list<const Token *> var; std::list<const Token *> var;
parseFunctionCall(*tok, var, &_settings->library, 0); parseFunctionCall(*tok, var, &_settings->library);
// is one of the var items a NULL pointer? // is one of the var items a NULL pointer?
for (std::list<const Token *>::const_iterator it = var.begin(); it != var.end(); ++it) { for (std::list<const Token *>::const_iterator it = var.begin(); it != var.end(); ++it) {

5
lib/checknullpointer.h
View File

@ -61,13 +61,10 @@ public:
* @param tok first token * @param tok first token
* @param var variables that the function read / write. * @param var variables that the function read / write.
* @param library --library files data * @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, static void parseFunctionCall(const Token &tok,
std::list<const Token *> &var, std::list<const Token *> &var,
const Library *library, const Library *library);
unsigned char value);
/** /**
* Is there a pointer dereference? Everything that should result in * Is there a pointer dereference? Everything that should result in

56
test/testnullpointer.cpp
View File

@ -2075,6 +2075,16 @@ private:
" ASSERT_MESSAGE(\"Error on s\", 0 == s.compare(\"Some text\"));\n" " ASSERT_MESSAGE(\"Error on s\", 0 == s.compare(\"Some text\"));\n"
"}"); "}");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
check("void foo(int i, std::string s);\n"
"void bar() {\n"
" foo(0, \"\");\n"
" foo(0, 0);\n"
" foo(var, 0);\n"
" foo(0, var);\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Null pointer dereference\n"
"[test.cpp:5]: (error) Null pointer dereference\n", errout.str());
} }
void nullpointerStdStream() { void nullpointerStdStream() {
@ -2258,11 +2268,9 @@ private:
library.functions["x"].argumentChecks[2] = arg; library.functions["x"].argumentChecks[2] = arg;
library.functions["x"].argumentChecks[3] = arg; library.functions["x"].argumentChecks[3] = arg;
std::list<const Token *> null, uninit; std::list<const Token *> null;
CheckNullPointer::parseFunctionCall(*xtok, null, &library, 0U); CheckNullPointer::parseFunctionCall(*xtok, null, &library);
CheckNullPointer::parseFunctionCall(*xtok, uninit, &library, 1U);
ASSERT_EQUALS(0U, null.size()); ASSERT_EQUALS(0U, null.size());
ASSERT_EQUALS(0U, uninit.size());
} }
// for 1st parameter null pointer is not ok.. // for 1st parameter null pointer is not ok..
@ -2274,46 +2282,10 @@ private:
library.functions["x"].argumentChecks[3] = arg; library.functions["x"].argumentChecks[3] = arg;
library.functions["x"].argumentChecks[1].notnull = true; library.functions["x"].argumentChecks[1].notnull = true;
std::list<const Token *> null,uninit; std::list<const Token *> null;
CheckNullPointer::parseFunctionCall(*xtok, null, &library, 0U); CheckNullPointer::parseFunctionCall(*xtok, null, &library);
CheckNullPointer::parseFunctionCall(*xtok, uninit, &library, 1U);
ASSERT_EQUALS(1U, null.size()); ASSERT_EQUALS(1U, null.size());
ASSERT_EQUALS("a", null.front()->str()); ASSERT_EQUALS("a", null.front()->str());
ASSERT_EQUALS(0U, uninit.size());
}
// for 2nd parameter uninit data is not ok..
{
Library library;
Library::ArgumentChecks arg;
library.functions["x"].argumentChecks[1] = arg;
library.functions["x"].argumentChecks[2] = arg;
library.functions["x"].argumentChecks[3] = arg;
library.functions["x"].argumentChecks[2].notuninit = true;
std::list<const Token *> 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());
}
// for 3rd parameter uninit data is not ok..
{
Library library;
Library::ArgumentChecks arg;
library.functions["x"].argumentChecks[1] = arg;
library.functions["x"].argumentChecks[2] = arg;
library.functions["x"].argumentChecks[3] = arg;
library.functions["x"].argumentChecks[3].notuninit = true;
std::list<const Token *> 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("c", uninit.front()->str());
} }
} }