Skip to content

Commit f1dbd1a

Browse files
eriklaxDaniel Marjamäki
authored andcommitted
Fixed cppcheck-opensource#3518 (False negative: Possible null pointer dereference (in the same condition))
1 parent 4966c0c commit f1dbd1a

3 files changed

Lines changed: 88 additions & 9 deletions

File tree

AUTHORS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ The cppcheck team, in alphabetical order:
33
Bill Egert
44
Daniel Marjamäki
55
Edoardo Prezioso
6+
Erik Lax
67
Gianluca Scacco
78
Greg Hewgill
89
Hoang Tuan Su

lib/checknullpointer.cpp

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -839,31 +839,60 @@ void CheckNullPointer::nullPointerByCheckAndDeRef()
839839

840840
// vartok : token for the variable
841841
const Token *vartok = 0;
842-
if (Token::Match(tok, "( ! %var% )|&&"))
842+
const Token *checkConditionStart = 0;
843+
if (Token::Match(tok, "( ! %var% )|&&")) {
843844
vartok = tok->tokAt(2);
844-
else if (Token::Match(tok, "( %var% )|&&"))
845+
checkConditionStart = vartok->next();
846+
} else if (Token::Match(tok, "( %var% )|&&")) {
845847
vartok = tok->next();
846-
else if (Token::Match(tok, "( ! ( %var% ="))
848+
} else if (Token::Match(tok, "( ! ( %var% =")) {
847849
vartok = tok->tokAt(3);
848-
else
850+
if (Token::Match(tok->tokAt(2)->link(), ") &&"))
851+
checkConditionStart = tok->tokAt(2)->link();
852+
} else
849853
continue;
850854

851855
// variable id for pointer
852856
const unsigned int varid(vartok->varId());
853857
if (varid == 0)
854858
continue;
855859

856-
const Variable* var = _tokenizer->getSymbolDatabase()->getVariableFromVarId(varid);
857860
// Check if variable is a pointer
861+
const Variable* var = _tokenizer->getSymbolDatabase()->getVariableFromVarId(varid);
858862
if (!var || !var->isPointer())
859863
continue;
860864

861865
if (Token::Match(vartok->next(), "&& ( %varid% =", varid))
862866
continue;
863867

868+
// Name and line of the pointer
869+
const std::string &pointerName = vartok->str();
870+
const unsigned int linenr = vartok->linenr();
871+
864872
// if this is true then it is known that the pointer is null
865873
bool null = true;
866874

875+
// Check the condition (eg. ( !x && x->i )
876+
if (checkConditionStart) {
877+
const Token * const conditionEnd = tok->link();
878+
for (const Token *tok2 = checkConditionStart; tok2 != conditionEnd; tok2 = tok2->next()) {
879+
// If we hit a || operator, abort
880+
if (Token::Match(tok2, "%oror%", varid)) {
881+
break;
882+
}
883+
// Pointer is used
884+
else if (Token::Match(tok2, "* %varid%", varid)) {
885+
nullPointerError(tok2->tokAt(2), pointerName, linenr, false);
886+
break;
887+
}
888+
// Pointer is used
889+
else if (Token::Match(tok2, "%varid% .", varid)) {
890+
nullPointerError(tok2, pointerName, linenr, false);
891+
break;
892+
}
893+
}
894+
}
895+
867896
// start token = inside the if-body
868897
const Token *tok1 = i->classStart;
869898

@@ -877,10 +906,6 @@ void CheckNullPointer::nullPointerByCheckAndDeRef()
877906
continue;
878907
}
879908

880-
// Name and line of the pointer
881-
const std::string &pointerName = vartok->str();
882-
const unsigned int linenr = vartok->linenr();
883-
884909
unsigned int indentlevel = 0;
885910

886911
// Set to true if we would normally bail out the check.

test/testnullpointer.cpp

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1478,6 +1478,59 @@ class TestNullPointer : public TestFixture {
14781478
"}");
14791479
ASSERT_EQUALS("", errout.str());
14801480

1481+
// check, assign and use
1482+
check("void f() {\n"
1483+
" char *p;\n"
1484+
" if (p == 0 && (p = malloc(10)) != a && (*p = a)) {\n"
1485+
" *p = 0;\n"
1486+
" }\n"
1487+
"}");
1488+
ASSERT_EQUALS("", errout.str());
1489+
1490+
// check, and use
1491+
check("void f() {\n"
1492+
" char *p;\n"
1493+
" if (p == 0 && (*p = 0)) {\n"
1494+
" return;\n"
1495+
" }\n"
1496+
"}");
1497+
ASSERT_EQUALS("[test.cpp:3]: (error) Possible null pointer dereference: p - otherwise it is redundant to check if p is null at line 3\n", errout.str());
1498+
1499+
// check, and use
1500+
check("void f() {\n"
1501+
" struct foo *p;\n"
1502+
" if (p == 0 && p->x == 10) {\n"
1503+
" return;\n"
1504+
" }\n"
1505+
"}");
1506+
ASSERT_EQUALS("[test.cpp:3]: (error) Possible null pointer dereference: p - otherwise it is redundant to check if p is null at line 3\n", errout.str());
1507+
1508+
// check, and use
1509+
check("void f() {\n"
1510+
" struct foo *p;\n"
1511+
" if (p == 0 || p->x == 10) {\n"
1512+
" return;\n"
1513+
" }\n"
1514+
"}");
1515+
ASSERT_EQUALS("", errout.str());
1516+
1517+
// check, and use
1518+
check("void f() {\n"
1519+
" char *p;\n"
1520+
" if ((p = malloc(10)) == NULL && (*p = a)) {\n"
1521+
" return;\n"
1522+
" }\n"
1523+
"}");
1524+
ASSERT_EQUALS("[test.cpp:3]: (error) Possible null pointer dereference: p - otherwise it is redundant to check if p is null at line 3\n", errout.str());
1525+
1526+
// check, and use
1527+
check("void f(struct X *p, int x) {\n"
1528+
" if (!p && x==1 || p && p->x==0) {\n"
1529+
" return;\n"
1530+
" }\n"
1531+
"}");
1532+
ASSERT_EQUALS("", errout.str());
1533+
14811534
{
14821535
const char code[] = "void f(Fred *fred) {\n"
14831536
" if (fred == NULL) { }\n"

0 commit comments

Comments
 (0)