Fix #8482: Regression with valueflow (#1159)

* Fix regression with valueflow

* Simplify test case

lib/valueflow.cpp

@@ -2572,9 +2572,8 @@ static void valueFlowAfterCondition(TokenList *tokenlist, SymbolDatabase* symbol
                         continue;
                     }
 
-                    std::list<ValueFlow::Value> * values = nullptr;
+                    bool dead_if = isReturnScope(after);
-                    if (check_else || !isReturnScope(after))
+                    bool dead_else = false;
-                        values = &false_values;
 
                     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))
+                        dead_else = isReturnScope(after);
-                            values = &true_values;
                     }
 
+                    std::list<ValueFlow::Value> * 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

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"

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