Refactoring: Removed CheckNullPointer::nullPointerByCheckAndDeRef and implemented needed analysis in ValueFlow instead.
This commit is contained in:
Daniel Marjamäki
parent
8d796519d9
commit
f78cbda2db
|
|
@ -560,218 +560,12 @@ void CheckNullPointer::nullPointerByDeRefAndChec()
|
|||
}
|
||||
}
|
||||
|
||||
void CheckNullPointer::nullPointerByCheckAndDeRef()
|
||||
{
|
||||
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
|
||||
|
||||
// Check if pointer is NULL and then dereference it..
|
||||
for (std::list<Scope>::const_iterator i = symbolDatabase->scopeList.begin(); i != symbolDatabase->scopeList.end(); ++i) {
|
||||
if (i->type != Scope::eIf && i->type != Scope::eElseIf && i->type != Scope::eWhile)
|
||||
continue;
|
||||
if (!i->classDef || i->classDef->isExpandedMacro())
|
||||
continue;
|
||||
|
||||
const Token* const tok = i->type != Scope::eElseIf ? i->classDef->next() : i->classDef->tokAt(2);
|
||||
// TODO: investigate false negatives:
|
||||
// - handle "while"?
|
||||
// - if there are logical operators
|
||||
// - if (x) { } else { ... }
|
||||
|
||||
// If the if-body ends with a unknown macro then bailout
|
||||
if (Token::Match(i->classEnd->tokAt(-3), "[;{}] %var% ;") && i->classEnd->tokAt(-2)->isUpperCaseName())
|
||||
continue;
|
||||
|
||||
// vartok : token for the variable
|
||||
const Token *vartok = nullptr;
|
||||
const Token *checkConditionStart = nullptr;
|
||||
if (Token::Match(tok, "( ! %var% )|&&")) {
|
||||
vartok = tok->tokAt(2);
|
||||
checkConditionStart = vartok->next();
|
||||
} else if (Token::Match(tok, "( %var% )|&&")) {
|
||||
vartok = tok->next();
|
||||
} else if (Token::Match(tok, "( ! ( %var% =")) {
|
||||
vartok = tok->tokAt(3);
|
||||
if (Token::simpleMatch(tok->linkAt(2), ") &&"))
|
||||
checkConditionStart = tok->linkAt(2);
|
||||
} else
|
||||
continue;
|
||||
|
||||
// Check if variable is a pointer
|
||||
const Variable *var = vartok->variable();
|
||||
if (!var || !var->isPointer())
|
||||
continue;
|
||||
|
||||
// variable id for pointer
|
||||
const unsigned int varid(vartok->varId());
|
||||
|
||||
const Scope* declScope = &*i;
|
||||
while (declScope->nestedIn && var->scope() != declScope && declScope->type != Scope::eFunction)
|
||||
declScope = declScope->nestedIn;
|
||||
|
||||
if (Token::Match(vartok->next(), "&& ( %varid% =", varid))
|
||||
continue;
|
||||
|
||||
// Name and line of the pointer
|
||||
const std::string &pointerName = vartok->str();
|
||||
|
||||
// Check the condition (eg. ( !x && x->i )
|
||||
if (checkConditionStart) {
|
||||
const Token * const conditionEnd = tok->link();
|
||||
for (const Token *tok2 = checkConditionStart; tok2 != conditionEnd; tok2 = tok2->next()) {
|
||||
// If we hit a || operator, abort
|
||||
if (tok2->str() == "||")
|
||||
break;
|
||||
|
||||
// Pointer is used
|
||||
bool unknown = _settings->inconclusive;
|
||||
if (tok2->varId() == varid && (isPointerDeRef(tok2, unknown) || unknown)) {
|
||||
nullPointerError(tok2, pointerName, vartok, unknown);
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// start token = inside the if-body
|
||||
const Token *tok1 = i->classStart;
|
||||
|
||||
if (Token::Match(tok, "( %var% )|&&")) {
|
||||
// start token = first token after the if/while body
|
||||
tok1 = i->classEnd->next();
|
||||
if (!tok1)
|
||||
continue;
|
||||
}
|
||||
|
||||
int indentlevel = 0;
|
||||
|
||||
// Set to true if we would normally bail out the check.
|
||||
bool inconclusive = false;
|
||||
|
||||
// Count { and } for tok2
|
||||
for (const Token *tok2 = tok1; tok2 != declScope->classEnd; tok2 = tok2->next()) {
|
||||
if (tok2->str() == "{")
|
||||
++indentlevel;
|
||||
else if (tok2->str() == "}") {
|
||||
if (indentlevel == 0) {
|
||||
if (_settings->inconclusive)
|
||||
inconclusive = true;
|
||||
else
|
||||
break;
|
||||
}
|
||||
--indentlevel;
|
||||
|
||||
// calling exit function?
|
||||
bool unknown = false;
|
||||
if (_tokenizer->IsScopeNoReturn(tok2, &unknown)) {
|
||||
if (_settings->inconclusive && unknown)
|
||||
inconclusive = true;
|
||||
else
|
||||
break;
|
||||
}
|
||||
|
||||
if (indentlevel <= 0) {
|
||||
// skip all "else" blocks because they are not executed in this execution path
|
||||
while (Token::simpleMatch(tok2, "} else if ("))
|
||||
tok2 = tok2->linkAt(3)->linkAt(1);
|
||||
if (Token::simpleMatch(tok2, "} else {"))
|
||||
tok2 = tok2->linkAt(2);
|
||||
}
|
||||
}
|
||||
|
||||
if (tok2->str() == "return" || tok2->str() == "throw") {
|
||||
bool unknown = _settings->inconclusive;
|
||||
for (; tok2 && tok2->str() != ";"; tok2 = tok2->next()) {
|
||||
if (tok2->varId() == varid) {
|
||||
if (CheckNullPointer::isPointerDeRef(tok2, unknown))
|
||||
nullPointerError(tok2, pointerName, vartok, inconclusive);
|
||||
else if (unknown)
|
||||
nullPointerError(tok2, pointerName, vartok, true);
|
||||
if (Token::Match(tok2, "%var% %oror%|&&|?"))
|
||||
break;
|
||||
}
|
||||
}
|
||||
break;
|
||||
}
|
||||
|
||||
// Bailout for "if".
|
||||
if (tok2->str() == "if") {
|
||||
if (_settings->inconclusive)
|
||||
inconclusive = true;
|
||||
else
|
||||
break;
|
||||
}
|
||||
|
||||
if (Token::Match(tok2, "goto|continue|break|switch|for"))
|
||||
break;
|
||||
|
||||
// parameters to sizeof are not dereferenced
|
||||
if (Token::Match(tok2, "decltype|sizeof|typeof")) {
|
||||
if (tok2->strAt(1) != "(")
|
||||
tok2 = tok2->next();
|
||||
else
|
||||
tok2 = tok2->next()->link();
|
||||
continue;
|
||||
}
|
||||
|
||||
// function call, check if pointer is dereferenced
|
||||
if (Token::Match(tok2, "%var% (") && !Token::Match(tok2, "if|while")) {
|
||||
std::list<const Token *> vars;
|
||||
parseFunctionCall(*tok2, vars, &_settings->library, 0);
|
||||
for (std::list<const Token *>::const_iterator it = vars.begin(); it != vars.end(); ++it) {
|
||||
if (Token::Match(*it, "%varid% [,)]", varid)) {
|
||||
nullPointerError(*it, pointerName, vartok, inconclusive);
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// calling unknown function (abort/init)..
|
||||
else if (Token::simpleMatch(tok2, ") ;") &&
|
||||
(Token::Match(tok2->link()->tokAt(-2), "[;{}.] %var% (") ||
|
||||
Token::Match(tok2->link()->tokAt(-5), "[;{}] ( * %var% ) ("))) {
|
||||
// noreturn function?
|
||||
bool unknown = false;
|
||||
if (_tokenizer->IsScopeNoReturn(tok2->tokAt(2), &unknown)) {
|
||||
if (!unknown || !_settings->inconclusive) {
|
||||
break;
|
||||
}
|
||||
inconclusive = _settings->inconclusive;
|
||||
}
|
||||
|
||||
// init function (global variables)
|
||||
if (!(var->isLocal() || var->isArgument()))
|
||||
break;
|
||||
}
|
||||
|
||||
if (tok2->varId() == varid) {
|
||||
// unknown: this is set to true by isPointerDeRef if
|
||||
// the function fails to determine if there
|
||||
// is a dereference or not
|
||||
bool unknown = _settings->inconclusive;
|
||||
|
||||
if (Token::Match(tok2->previous(), "[;{}=] %var% = 0 ;"))
|
||||
;
|
||||
|
||||
else if (CheckNullPointer::isPointerDeRef(tok2, unknown))
|
||||
nullPointerError(tok2, pointerName, vartok, inconclusive);
|
||||
|
||||
else if (unknown && _settings->inconclusive)
|
||||
nullPointerError(tok2, pointerName, vartok, true);
|
||||
|
||||
else
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
void CheckNullPointer::nullPointer()
|
||||
{
|
||||
nullPointerLinkedList();
|
||||
|
||||
if (_settings->isEnabled("warning")) {
|
||||
nullPointerByDeRefAndChec();
|
||||
nullPointerByCheckAndDeRef();
|
||||
nullPointerDefaultArgument();
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -82,12 +82,6 @@ public:
|
|||
/** @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();
|
||||
|
||||
|
|
|
|||
|
|
@ -868,9 +868,22 @@ static void valueFlowAfterCondition(TokenList *tokenlist, ErrorLogger *errorLogg
|
|||
if (ok && !scopeEnd.empty() && Token::simpleMatch(top->link(), ") {")) {
|
||||
Token *after = top->link()->linkAt(1);
|
||||
if (Token::simpleMatch(after->tokAt(-2), ") ; }")) {
|
||||
if (settings->debugwarnings)
|
||||
bailout(tokenlist, errorLogger, after, "possible noreturn scope");
|
||||
continue;
|
||||
const Token *funcname = after->linkAt(-2)->previous();
|
||||
const Token *start = funcname;
|
||||
if (funcname && Token::Match(funcname->tokAt(-3),"( * %var% )")) {
|
||||
funcname = funcname->previous();
|
||||
start = funcname->tokAt(-3);
|
||||
} else {
|
||||
while (Token::Match(start, "%var%|.|::"))
|
||||
start = start->previous();
|
||||
}
|
||||
if (Token::Match(start,"[;{}]") && Token::Match(funcname, "%var% )| (")) {
|
||||
if (!settings->library.isnotnoreturn(funcname->str())) {
|
||||
if (settings->debugwarnings)
|
||||
bailout(tokenlist, errorLogger, after, "possible noreturn scope");
|
||||
continue;
|
||||
}
|
||||
}
|
||||
}
|
||||
if (Token::simpleMatch(after, "} else {")) {
|
||||
after = after->linkAt(2);
|
||||
|
|
|
|||
|
|
@ -207,20 +207,13 @@ private:
|
|||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
// dereference in outer scope..
|
||||
{
|
||||
const char code[] = "void foo(int x, const Token *tok) {\n"
|
||||
" if (x == 123) {\n"
|
||||
" while (tok) tok = tok->next();\n"
|
||||
" }\n"
|
||||
" tok->str();\n"
|
||||
"}\n";
|
||||
|
||||
check(code, false);
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check(code, true);
|
||||
ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:3]: (warning, inconclusive) Possible null pointer dereference: tok - otherwise it is redundant to check it against null.\n", errout.str());
|
||||
}
|
||||
check("void foo(int x, const Token *tok) {\n"
|
||||
" if (x == 123) {\n"
|
||||
" while (tok) tok = tok->next();\n"
|
||||
" }\n"
|
||||
" tok->str();\n"
|
||||
"}\n");
|
||||
TODO_ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:3]: (warning, inconclusive) Possible null pointer dereference: tok - otherwise it is redundant to check it against null.\n", "", errout.str());
|
||||
|
||||
check("int foo(const Token *tok)\n"
|
||||
"{\n"
|
||||
|
|
@ -2109,10 +2102,13 @@ private:
|
|||
" std::cout << abc << p;\n"
|
||||
" }\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n"
|
||||
"[test.cpp:4] -> [test.cpp:2]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n"
|
||||
"[test.cpp:5] -> [test.cpp:2]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n"
|
||||
"[test.cpp:6] -> [test.cpp:2]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n", errout.str());
|
||||
TODO_ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n"
|
||||
"[test.cpp:4] -> [test.cpp:2]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n"
|
||||
"[test.cpp:5] -> [test.cpp:2]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n"
|
||||
"[test.cpp:6] -> [test.cpp:2]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n",
|
||||
"[test.cpp:3] -> [test.cpp:2]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n"
|
||||
"[test.cpp:4] -> [test.cpp:2]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n",
|
||||
errout.str());
|
||||
|
||||
check("void f() {\n"
|
||||
" void* p1 = 0;\n"
|
||||
|
|
|
|||
Loading…
Reference in New Issue