From 9aa97cbb95cda0908df634298dd950353ba3c2fd Mon Sep 17 00:00:00 2001
From: Paul Fultz II <pfultz2@yahoo.com>
Date: Sun, 11 Aug 2019 08:39:37 -0500
Subject: [PATCH] Fix issue 8296: ValueFlow: value not set in conditional scope
 in subfunction (#2071)

* Fix issue 8296: ValueFlow: value not set in conditional scope in subfunction

* Refactor condition checkingg

* Make test case TODO
---
 lib/valueflow.cpp        | 17 +++++++++--------
 test/testnullpointer.cpp | 16 +++++++++++++++-
 test/testvalueflow.cpp   | 23 ++++++++++++++++++++++-
 3 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp
index c7d68ec7c..a7bdf6e9b 100644
--- a/lib/valueflow.cpp
+++ b/lib/valueflow.cpp
@@ -2145,16 +2145,16 @@ static bool valueFlowForward(Token * const               startToken,
                     continue;
                 }
                 const ProgramMemory &programMemory = getProgramMemory(tok2, varid, v);
-                if (subFunction && conditionIsTrue(condTok, programMemory))
+                const bool isTrue = conditionIsTrue(condTok, programMemory);
+                const bool isFalse = conditionIsFalse(condTok, programMemory);
+
+                if (isTrue)
                     truevalues.push_back(v);
-                else if (!conditionIsFalse(condTok, programMemory))
-                    truevalues.push_back(v);
-                if (conditionIsFalse(condTok, programMemory))
-                    falsevalues.push_back(v);
-                else if (!subFunction && !conditionIsTrue(condTok, programMemory))
+                if (isFalse)
                     falsevalues.push_back(v);
+
             }
-            if (truevalues.size() != values.size() || condAlwaysTrue) {
+            if (!truevalues.empty() || !falsevalues.empty()) {
                 // '{'
                 const Token * const startToken1 = tok2->linkAt(1)->next();
 
@@ -2211,7 +2211,8 @@ static bool valueFlowForward(Token * const               startToken,
                         removeValues(values, falsevalues);
                     }
                 }
-
+                if (values.empty())
+                    return false;
                 continue;
             }
 
diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp
index 38b82b0bb..08979b54a 100644
--- a/test/testnullpointer.cpp
+++ b/test/testnullpointer.cpp
@@ -74,6 +74,7 @@ private:
         TEST_CASE(nullpointer32); // #8460
         TEST_CASE(nullpointer33);
         TEST_CASE(nullpointer34);
+        TEST_CASE(nullpointer35);
         TEST_CASE(nullpointer_addressOf); // address of
         TEST_CASE(nullpointerSwitch); // #2626
         TEST_CASE(nullpointer_cast); // #4692
@@ -1413,6 +1414,19 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
+    void nullpointer35() {
+        check("bool f(int*);\n"
+              "void g(int* x) {\n"
+              "    if (f(x)) {\n"
+              "        *x = 1;\n"
+              "    }\n"
+              "}\n"
+              "void h() {\n"
+              "    g(0);\n"
+              "}\n", true);
+        ASSERT_EQUALS("", errout.str());
+    }
+
     void nullpointer_addressOf() { // address of
         check("void f() {\n"
               "  struct X *x = 0;\n"
@@ -2574,7 +2588,7 @@ private:
               "}");
         ASSERT_EQUALS("", errout.str());
 
-        check("void f(int *p = 0) {\n"
+        check("void f(int a, int *p = 0) {\n"
               "    if (a != 0)\n"
               "      *p = 0;\n"
               "}", true);
diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp
index 9c08227bd..924fc5493 100644
--- a/test/testvalueflow.cpp
+++ b/test/testvalueflow.cpp
@@ -94,6 +94,7 @@ private:
         TEST_CASE(valueFlowSwitchVariable);
 
         TEST_CASE(valueFlowForLoop);
+        TEST_CASE(valueFlowSubFunction);
         TEST_CASE(valueFlowFunctionReturn);
 
         TEST_CASE(valueFlowFunctionDefaultParameter);
@@ -2203,7 +2204,7 @@ private:
                "  else\n"
                "    b = x;\n"
                "}";
-        TODO_ASSERT_EQUALS(true, false, testValueOfX(code, 4U, 2));
+        ASSERT_EQUALS(true, testValueOfX(code, 4U, 2));
         ASSERT_EQUALS(false, testValueOfX(code, 6U, 2));
 
         // condition with 2nd variable
@@ -2925,6 +2926,26 @@ private:
         ASSERT_EQUALS(true, testValueOfX(code, 5U, 3));
     }
 
+    void valueFlowSubFunction() {
+        const char *code;
+
+        code = "int f(int size) {\n"
+               "    int x = 0;\n"
+               "    if(size>16) {\n"
+               "        x = size;\n"
+               "        int a = x;\n"
+               "    }\n"
+               "    return x;\n"
+               "}\n"
+               "void g(){\n"
+               "    f(42);\n"
+               "}";
+        ASSERT_EQUALS(true, testValueOfX(code, 5U, 17));
+        ASSERT_EQUALS(true, testValueOfX(code, 5U, 42));
+        ASSERT_EQUALS(true, testValueOfX(code, 7U, 0));
+        ASSERT_EQUALS(true, testValueOfX(code, 7U, 17));
+        ASSERT_EQUALS(true, testValueOfX(code, 7U, 42));
+    }
     void valueFlowFunctionReturn() {
         const char *code;