Improved expression parsing in CheckNullPointer::isPointerDeRef() - fixed #4692
This commit is contained in:
parent
3e8eddc69e
commit
51685f24c5
|
@ -323,19 +323,29 @@ bool CheckNullPointer::isPointerDeRef(const Token *tok, bool &unknown)
|
||||||
|
|
||||||
unknown = false;
|
unknown = false;
|
||||||
|
|
||||||
|
const Token* prev = tok->previous();
|
||||||
|
while (prev->str() == "." || prev->str() == "::") { // Skip over previous member dereferences
|
||||||
|
prev = prev->previous();
|
||||||
|
while (prev->link() && (prev->str() == "]" || prev->str() == ">"))
|
||||||
|
prev = prev->link()->previous();
|
||||||
|
prev = prev->previous();
|
||||||
|
}
|
||||||
|
while (prev->str() == ")") // Skip over casts
|
||||||
|
prev = prev->link()->previous();
|
||||||
|
|
||||||
// Dereferencing pointer..
|
// Dereferencing pointer..
|
||||||
if (tok->strAt(-1) == "*" && (Token::Match(tok->tokAt(-2), "return|throw|;|{|}|:|[|(|,") || tok->tokAt(-2)->isOp()) && !Token::Match(tok->tokAt(-3), "sizeof|decltype"))
|
if (prev->str() == "*" && (Token::Match(prev->previous(), "return|throw|;|{|}|:|[|(|,") || prev->previous()->isOp()) && !Token::Match(prev->tokAt(-2), "sizeof|decltype"))
|
||||||
return true;
|
return true;
|
||||||
|
|
||||||
// read/write member variable
|
// read/write member variable
|
||||||
if (!Token::simpleMatch(tok->tokAt(-2), "& (") && !Token::Match(tok->tokAt(-2), "sizeof|decltype (") && tok->strAt(-1) != "&" && Token::Match(tok->next(), ". %var%")) {
|
if (!Token::simpleMatch(prev->previous(), "& (") && !Token::Match(prev->previous(), "sizeof|decltype (") && prev->str() != "&" && Token::Match(tok->next(), ". %var%")) {
|
||||||
if (tok->strAt(3) != "(")
|
if (tok->strAt(3) != "(")
|
||||||
return true;
|
return true;
|
||||||
unknown = true;
|
unknown = true;
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (Token::Match(tok, "%var% [") && (tok->previous()->str() != "&" || Token::Match(tok->next()->link()->next(), "[.(]")))
|
if (Token::Match(tok, "%var% [") && (prev->str() != "&" || Token::Match(tok->next()->link()->next(), "[.(]")))
|
||||||
return true;
|
return true;
|
||||||
|
|
||||||
if (Token::Match(tok, "%var% ("))
|
if (Token::Match(tok, "%var% ("))
|
||||||
|
@ -347,19 +357,19 @@ bool CheckNullPointer::isPointerDeRef(const Token *tok, bool &unknown)
|
||||||
return true;
|
return true;
|
||||||
|
|
||||||
// std::string dereferences nullpointers
|
// std::string dereferences nullpointers
|
||||||
if (Token::Match(tok->tokAt(-4), "std :: string|wstring ( %var% )"))
|
if (Token::Match(prev->tokAt(-3), "std :: string|wstring (") && tok->strAt(1) == ")")
|
||||||
return true;
|
return true;
|
||||||
if (Token::Match(tok->tokAt(-2), "%var% ( %var% )")) {
|
if (Token::Match(prev->previous(), "%var% (") && tok->strAt(1) == ")") {
|
||||||
const Variable* var = tok->tokAt(-2)->variable();
|
const Variable* var = tok->tokAt(-2)->variable();
|
||||||
if (var && !var->isPointer() && !var->isArray() && Token::Match(var->typeStartToken(), "std :: string|wstring !!::"))
|
if (var && !var->isPointer() && !var->isArray() && Token::Match(var->typeStartToken(), "std :: string|wstring !!::"))
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
// streams dereference nullpointers
|
// streams dereference nullpointers
|
||||||
if (Token::Match(tok->previous(), "<<|>> %var%")) {
|
if (Token::Match(prev, "<<|>>")) {
|
||||||
const Variable* var = tok->variable();
|
const Variable* var = tok->variable();
|
||||||
if (var && var->isPointer() && Token::Match(var->typeStartToken(), "char|wchar_t")) { // Only outputting or reading to char* can cause problems
|
if (var && var->isPointer() && Token::Match(var->typeStartToken(), "char|wchar_t")) { // Only outputting or reading to char* can cause problems
|
||||||
const Token* tok2 = tok->previous(); // Find start of statement
|
const Token* tok2 = prev; // Find start of statement
|
||||||
for (; tok2; tok2 = tok2->previous()) {
|
for (; tok2; tok2 = tok2->previous()) {
|
||||||
if (Token::Match(tok2->previous(), ";|{|}|:"))
|
if (Token::Match(tok2->previous(), ";|{|}|:"))
|
||||||
break;
|
break;
|
||||||
|
@ -377,9 +387,9 @@ bool CheckNullPointer::isPointerDeRef(const Token *tok, bool &unknown)
|
||||||
const Variable *ovar = NULL;
|
const Variable *ovar = NULL;
|
||||||
if (Token::Match(tok, "%var% ==|!= %var%"))
|
if (Token::Match(tok, "%var% ==|!= %var%"))
|
||||||
ovar = tok->tokAt(2)->variable();
|
ovar = tok->tokAt(2)->variable();
|
||||||
else if (Token::Match(tok->tokAt(-2), "%var% ==|!= %var%"))
|
else if (Token::Match(prev->previous(), "%var% ==|!="))
|
||||||
ovar = tok->tokAt(-2)->variable();
|
ovar = tok->tokAt(-2)->variable();
|
||||||
else if (Token::Match(tok->tokAt(-2), "%var% =|+ %var% )|]|,|;|+"))
|
else if (Token::Match(prev->previous(), "%var% =|+") && Token::Match(tok->next(), ")|]|,|;|+"))
|
||||||
ovar = tok->tokAt(-2)->variable();
|
ovar = tok->tokAt(-2)->variable();
|
||||||
if (ovar && !ovar->isPointer() && !ovar->isArray() && Token::Match(ovar->typeStartToken(), "std :: string|wstring !!::"))
|
if (ovar && !ovar->isPointer() && !ovar->isArray() && Token::Match(ovar->typeStartToken(), "std :: string|wstring !!::"))
|
||||||
return true;
|
return true;
|
||||||
|
@ -392,41 +402,41 @@ bool CheckNullPointer::isPointerDeRef(const Token *tok, bool &unknown)
|
||||||
return false;
|
return false;
|
||||||
|
|
||||||
// OK to delete a null
|
// OK to delete a null
|
||||||
if (Token::Match(tok->previous(), "delete %var%") || Token::Match(tok->tokAt(-3), "delete [ ] %var%"))
|
if (Token::Match(prev, "delete") || Token::Match(prev->tokAt(-2), "delete [ ]"))
|
||||||
return false;
|
return false;
|
||||||
|
|
||||||
// OK to check if pointer is null
|
// OK to check if pointer is null
|
||||||
// OK to take address of pointer
|
// OK to take address of pointer
|
||||||
if (Token::Match(tok->previous(), "!|& %var%"))
|
if (Token::Match(prev, "!|&"))
|
||||||
return false;
|
return false;
|
||||||
|
|
||||||
// OK to check pointer in "= p ? : "
|
// OK to check pointer in "= p ? : "
|
||||||
if (tok->next()->str() == "?" &&
|
if (tok->next()->str() == "?" &&
|
||||||
(Token::Match(tok->previous(), "return|throw|;|{|}|:|[|(|,") || tok->previous()->isAssignmentOp()))
|
(Token::Match(prev, "return|throw|;|{|}|:|[|(|,") || prev->isAssignmentOp()))
|
||||||
return false;
|
return false;
|
||||||
|
|
||||||
// OK to pass pointer to function
|
// OK to pass pointer to function
|
||||||
if (Token::Match(tok->previous(), "[(,] %var% [,)]") &&
|
if (Token::Match(prev, "[(,]") && Token::Match(tok->next(), "[,)]") &&
|
||||||
(!Token::Match(tok->previous(), "( %var%") ||
|
(prev->str() != "(" ||
|
||||||
Token::Match(tok->tokAt(-2), "%var% ( %var%")))
|
Token::Match(prev->previous(), "%var% (")))
|
||||||
return false;
|
return false;
|
||||||
|
|
||||||
// Compare pointer
|
// Compare pointer
|
||||||
if (Token::Match(tok->previous(), "(|&&|%oror%|==|!= %var%"))
|
if (Token::Match(prev, "(|&&|%oror%|==|!="))
|
||||||
return false;
|
return false;
|
||||||
if (Token::Match(tok, "%var% &&|%oror%|==|!=|)"))
|
if (Token::Match(tok, "%var% &&|%oror%|==|!=|)"))
|
||||||
return false;
|
return false;
|
||||||
|
|
||||||
// Taking address
|
// Taking address
|
||||||
if (Token::Match(tok->previous(), "return|= %var% ;"))
|
if (Token::Match(prev, "return|=") && tok->strAt(1) == ";")
|
||||||
return false;
|
return false;
|
||||||
|
|
||||||
// (void)var
|
// (void)var
|
||||||
if (Token::Match(tok->previous(), "[{;}] %var% ;"))
|
if (Token::Match(prev, "[{;}]") && tok->strAt(1) == ";")
|
||||||
return false;
|
return false;
|
||||||
|
|
||||||
// Shift pointer (e.g. to cout, but its no char* (see above))
|
// Shift pointer (e.g. to cout, but its no char* (see above))
|
||||||
if (Token::Match(tok->previous(), "<<|>> %var%"))
|
if (Token::Match(prev, "<<|>>"))
|
||||||
return false;
|
return false;
|
||||||
|
|
||||||
// unknown if it's a dereference
|
// unknown if it's a dereference
|
||||||
|
|
|
@ -57,6 +57,7 @@ private:
|
||||||
TEST_CASE(nullpointer21); // #4038 (fp: if (x) p=q; else return;)
|
TEST_CASE(nullpointer21); // #4038 (fp: if (x) p=q; else return;)
|
||||||
TEST_CASE(nullpointer22); // #4007 (fp: (uri != NULL) && (*uri == 0))
|
TEST_CASE(nullpointer22); // #4007 (fp: (uri != NULL) && (*uri == 0))
|
||||||
TEST_CASE(nullpointer23); // #4665 (false positive)
|
TEST_CASE(nullpointer23); // #4665 (false positive)
|
||||||
|
TEST_CASE(nullpointer_cast); // #4692
|
||||||
TEST_CASE(nullpointer_castToVoid); // #3771
|
TEST_CASE(nullpointer_castToVoid); // #3771
|
||||||
TEST_CASE(pointerCheckAndDeRef); // check if pointer is null and then dereference it
|
TEST_CASE(pointerCheckAndDeRef); // check if pointer is null and then dereference it
|
||||||
TEST_CASE(nullConstantDereference); // Dereference NULL constant
|
TEST_CASE(nullConstantDereference); // Dereference NULL constant
|
||||||
|
@ -1344,6 +1345,16 @@ private:
|
||||||
TODO_ASSERT_EQUALS("","[test.cpp:4]: (error) Possible null pointer dereference: c\n", errout.str());
|
TODO_ASSERT_EQUALS("","[test.cpp:4]: (error) Possible null pointer dereference: c\n", errout.str());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void nullpointer_cast() { // #4692
|
||||||
|
check("char *nasm_skip_spaces(const char *p) {\n"
|
||||||
|
" if (p)\n"
|
||||||
|
" while (*p && nasm_isspace(*p))\n"
|
||||||
|
" p++;\n"
|
||||||
|
" return (char *)p;\n"
|
||||||
|
"}");
|
||||||
|
ASSERT_EQUALS("", errout.str());
|
||||||
|
}
|
||||||
|
|
||||||
void nullpointer_castToVoid() { // #3771
|
void nullpointer_castToVoid() { // #3771
|
||||||
check("void f () {\n"
|
check("void f () {\n"
|
||||||
" int *buf = NULL;\n"
|
" int *buf = NULL;\n"
|
||||||
|
|
Loading…
Reference in New Issue