Fixed #4929 (False positive: possible null pointer deref (checks dont handle && and || well))

This commit is contained in:
Daniel Marjamäki 2013-10-22 10:47:48 +02:00
parent b15eeb0aa8
commit fd0f2d7900
2 changed files with 25 additions and 5 deletions

View File

@ -580,8 +580,8 @@ void CheckNullPointer::nullPointerStructByDeRefAndChec()
tok1 = tok1->next(); tok1 = tok1->next();
skipvar.insert(tok1->varId()); skipvar.insert(tok1->varId());
continue; continue;
} else if (Token::Match(tok1, "( ! %var% %oror%") || } else if (Token::Match(tok1, "(|%oror% ! %var% %oror%") ||
Token::Match(tok1, "( %var% &&")) { Token::Match(tok1, "(|&& %var% &&")) {
// TODO: there are false negatives caused by this. The // TODO: there are false negatives caused by this. The
// variable should be removed from skipvar after the // variable should be removed from skipvar after the
// condition // condition
@ -631,6 +631,8 @@ void CheckNullPointer::nullPointerStructByDeRefAndChec()
const unsigned int varid1(tok1->varId()); const unsigned int varid1(tok1->varId());
if (varid1 == 0) if (varid1 == 0)
continue; continue;
if (skipvar.find(varid1) != skipvar.end())
continue;
const Token *tok2 = tok1->previous(); const Token *tok2 = tok1->previous();
while (tok2 && !Token::Match(tok2, "[;{}]")) { while (tok2 && !Token::Match(tok2, "[;{}]")) {
if (Token::Match(tok2, "%varid% =", varid1)) { if (Token::Match(tok2, "%varid% =", varid1)) {
@ -850,7 +852,7 @@ void CheckNullPointer::nullPointerByDeRefAndChec()
// Don't write warning if the dereferencing is // Don't write warning if the dereferencing is
// guarded by ?: or && // guarded by ?: or &&
const Token *tok2 = tok1->previous(); const Token *tok2 = tok1->previous();
if (tok2 && (tok2->isArithmeticalOp() || tok2->str() == "(")) { if (tok2 && (tok2->isArithmeticalOp() || Token::Match(tok2, "[(,]"))) {
while (tok2 && !Token::Match(tok2, "[;{}?:]")) { while (tok2 && !Token::Match(tok2, "[;{}?:]")) {
if (tok2->str() == ")") { if (tok2->str() == ")") {
tok2 = tok2->link(); tok2 = tok2->link();
@ -859,8 +861,8 @@ void CheckNullPointer::nullPointerByDeRefAndChec()
break; break;
} }
} }
// guarded by && // guarded by && or ||
if (tok2->varId() == varid && tok2->next()->str() == "&&") if (Token::Match(tok2, "%varid% &&|%oror%",varid))
break; break;
tok2 = tok2->previous(); tok2 = tok2->previous();
} }

View File

@ -468,6 +468,24 @@ private:
"}"); "}");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
check("void f(ABC *abc) {\n"
" x(def || !abc || y(def, abc->a));\n"
" if (abc) {}\n"
"}");
ASSERT_EQUALS("", errout.str());
check("void f(ABC *abc) {\n"
" x(abc && y(def, abc->a));\n"
" if (abc) {}\n"
"}");
ASSERT_EQUALS("", errout.str());
check("void f(ABC *abc) {\n"
" x(def && abc && y(def, abc->a));\n"
" if (abc) {}\n"
"}");
ASSERT_EQUALS("", errout.str());
// #3228 - calling function with null object // #3228 - calling function with null object
{ {
const char code[] = "void f(Fred *fred) {\n" const char code[] = "void f(Fred *fred) {\n"