diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 0d6f3ccc3..3baedd81a 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2572,9 +2572,8 @@ static void valueFlowAfterCondition(TokenList *tokenlist, SymbolDatabase* symbol continue; } - std::list * values = nullptr; - if (check_else || !isReturnScope(after)) - values = &false_values; + bool dead_if = isReturnScope(after); + bool dead_else = false; if (Token::simpleMatch(after, "} else {")) { after = after->linkAt(2); @@ -2583,10 +2582,15 @@ static void valueFlowAfterCondition(TokenList *tokenlist, SymbolDatabase* symbol bailout(tokenlist, errorLogger, after, "possible noreturn scope"); continue; } - if (check_if || !isReturnScope(after)) - values = &true_values; + dead_else = isReturnScope(after); } + std::list * values = nullptr; + if(!dead_if && check_if) + values = &true_values; + else if(!dead_else && check_else) + values = &false_values; + if (values) { // TODO: constValue could be true if there are no assignments in the conditional blocks and // perhaps if there are no && and no || in the condition diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index 955229f5a..22519bfca 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -81,6 +81,7 @@ private: TEST_CASE(nullpointer27); // #6568 TEST_CASE(nullpointer28); // #6491 TEST_CASE(nullpointer30); // #6392 + TEST_CASE(nullpointer31); // #8482 TEST_CASE(nullpointer_addressOf); // address of TEST_CASE(nullpointerSwitch); // #2626 TEST_CASE(nullpointer_cast); // #4692 @@ -1360,6 +1361,21 @@ private: ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:3]: (warning, inconclusive) Either the condition 'if(values)' is redundant or there is possible null pointer dereference: values.\n", errout.str()); } + void nullpointer31() { // #8482 + check("struct F\n" + "{\n" + " int x;\n" + "};\n" + " \n" + "static void foo(F* f)\n" + "{\n" + " if( f ) {}\n" + " else { return; }\n" + " (void)f->x;\n" + "}\n", true); + ASSERT_EQUALS("", errout.str()); + } + void nullpointer_addressOf() { // address of check("void f() {\n" " struct X *x = 0;\n" diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index d9f0b7ccd..4eeb8f5f1 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -1812,6 +1812,19 @@ private: "}"; ASSERT_EQUALS(true, testValueOfX(code, 3U, 123)); + code = "void f(int x) {\n" + " if (x != 123) { }\n" + " else { throw ""; }\n" + " a = x;\n" + "}"; + ASSERT_EQUALS(false, testValueOfX(code, 4U, 123)); + code = "void f(int x) {\n" + " if (x == 123) { }\n" + " else { throw ""; }\n" + " a = x;\n" + "}"; + ASSERT_EQUALS(true, testValueOfX(code, 4U, 123)); + code = "void f(int x) {\n" " if (x < 123) { }\n" @@ -1825,6 +1838,42 @@ private: "}"; ASSERT_EQUALS(true, testValueOfX(code, 3U, 123)); + code = "void f(int x) {\n" + " if (x < 123) { }\n" + " else { throw \"\"; }\n" + " a = x;\n" + "}"; + ASSERT_EQUALS(true, testValueOfX(code, 4U, 122)); + ASSERT_EQUALS(false, testValueOfX(code, 4U, 123)); + + code = "void f(int x) {\n" + " if (x > 123) { }\n" + " else { a = x; }\n" + "}"; + ASSERT_EQUALS(true, testValueOfX(code, 3U, 123)); + + code = "void f(int x) {\n" + " if (x > 123) { throw \"\"; }\n" + " a = x;\n" + "}"; + ASSERT_EQUALS(true, testValueOfX(code, 3U, 123)); + + code = "void f(int x) {\n" + " if (x > 123) { }\n" + " else { throw \"\"; }\n" + " a = x;\n" + "}"; + ASSERT_EQUALS(true, testValueOfX(code, 4U, 124)); + ASSERT_EQUALS(false, testValueOfX(code, 4U, 123)); + + code = "void f(int x) {\n" + " if (x < 123) { return; }\n" + " else { return; }\n" + " a = x;\n" + "}"; + ASSERT_EQUALS(false, testValueOfX(code, 4U, 124)); + ASSERT_EQUALS(false, testValueOfX(code, 4U, 123)); + // if (var) code = "void f(int x) {\n" " if (x) { a = x; }\n" // <- x is not 0