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

Fix 9870: False negative: null pointer after duplicate conditions (#3481)

This commit is contained in:
Paul Fultz II 2021-10-05 01:29:23 -05:00 committed by GitHub
parent 8668d445c7
commit 9b6c7007d2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 26 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
lib/forwardanalyzer.cpp
View File

@ -48,6 +48,7 @@ struct ForwardTraversal {
bool check = false; bool check = false;
bool escape = false; bool escape = false;
bool escapeUnknown = false; bool escapeUnknown = false;
bool active = false;
bool isEscape() const { bool isEscape() const {
return escape || escapeUnknown; return escape || escapeUnknown;
} }
@ -628,9 +629,11 @@ struct ForwardTraversal {
// Traverse then block // Traverse then block
thenBranch.escape = isEscapeScope(endBlock, thenBranch.escapeUnknown); thenBranch.escape = isEscapeScope(endBlock, thenBranch.escapeUnknown);
if (thenBranch.check) { if (thenBranch.check) {
thenBranch.active = true;
if (updateRange(endCond->next(), endBlock, depth - 1) == Progress::Break) if (updateRange(endCond->next(), endBlock, depth - 1) == Progress::Break)
return Break(); return Break();
} else if (!elseBranch.check) { } else if (!elseBranch.check) {
thenBranch.active = true;
if (checkBranch(thenBranch)) if (checkBranch(thenBranch))
bail = true; bail = true;
} }
@ -638,10 +641,12 @@ struct ForwardTraversal {
if (hasElse) { if (hasElse) {
elseBranch.escape = isEscapeScope(endBlock->linkAt(2), elseBranch.escapeUnknown); elseBranch.escape = isEscapeScope(endBlock->linkAt(2), elseBranch.escapeUnknown);
if (elseBranch.check) { if (elseBranch.check) {
elseBranch.active = true;
Progress result = updateRange(endBlock->tokAt(2), endBlock->linkAt(2), depth - 1); Progress result = updateRange(endBlock->tokAt(2), endBlock->linkAt(2), depth - 1);
if (result == Progress::Break) if (result == Progress::Break)
return Break(); return Break();
} else if (!thenBranch.check) { } else if (!thenBranch.check) {
elseBranch.active = true;
if (checkBranch(elseBranch)) if (checkBranch(elseBranch))
bail = true; bail = true;
} }
@ -649,7 +654,10 @@ struct ForwardTraversal {
} else { } else {
tok = endBlock; tok = endBlock;
} }
actions |= (thenBranch.action | elseBranch.action); if (thenBranch.active)
actions |= thenBranch.action;
if (elseBranch.active)
actions |= elseBranch.action;
if (bail) if (bail)
return Break(Analyzer::Terminate::Bail); return Break(Analyzer::Terminate::Bail);
if (thenBranch.isDead() && elseBranch.isDead()) { if (thenBranch.isDead() && elseBranch.isDead()) {
@ -660,7 +668,7 @@ struct ForwardTraversal {
return Break(Analyzer::Terminate::Bail); return Break(Analyzer::Terminate::Bail);
} }
// Conditional return // Conditional return
if (thenBranch.isEscape() && !hasElse) { if (thenBranch.active && thenBranch.isEscape() && !hasElse) {
if (!thenBranch.isConclusiveEscape()) { if (!thenBranch.isConclusiveEscape()) {
if (!analyzer->lowerToInconclusive()) if (!analyzer->lowerToInconclusive())
return Break(Analyzer::Terminate::Bail); return Break(Analyzer::Terminate::Bail);

16
test/testnullpointer.cpp
View File

@ -122,6 +122,7 @@ private:
TEST_CASE(nullpointer80); // #10410 TEST_CASE(nullpointer80); // #10410
TEST_CASE(nullpointer81); // #8724 TEST_CASE(nullpointer81); // #8724
TEST_CASE(nullpointer82); // #10331 TEST_CASE(nullpointer82); // #10331
TEST_CASE(nullpointer83); // #9870
TEST_CASE(nullpointer_addressOf); // address of TEST_CASE(nullpointer_addressOf); // address of
TEST_CASE(nullpointerSwitch); // #2626 TEST_CASE(nullpointerSwitch); // #2626
TEST_CASE(nullpointer_cast); // #4692 TEST_CASE(nullpointer_cast); // #4692
@ -2495,6 +2496,21 @@ private:
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }
void nullpointer83() // #9870
{
check("int* qux();\n"
"int* f7c2(int *x) {\n"
" int* p = 0;\n"
" if (nullptr == x)\n"
" p = qux();\n"
" if (nullptr == x)\n"
" return x;\n"
" *p = 1;\n"
" return x;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:8]: (warning) Possible null pointer dereference: p\n", errout.str());
}
void nullpointer_addressOf() { // address of void nullpointer_addressOf() { // address of
check("void f() {\n" check("void f() {\n"
" struct X *x = 0;\n" " struct X *x = 0;\n"