From 05618771827af5e6b6d3d49155af02f795f31977 Mon Sep 17 00:00:00 2001
From: Paul Fultz II <pfultz2@yahoo.com>
Date: Sun, 6 May 2018 01:35:29 -0500
Subject: [PATCH] Fix false positive with negative array index in issue 8536
 (#1202)

* Fix FP with negative array index in valueflow

* Remove values when valueflow fails

* Add valueflow test
---
 lib/valueflow.cpp          |  8 ++++----
 test/testbufferoverrun.cpp | 17 +++++++++++++++++
 test/testvalueflow.cpp     | 12 ++++++++++++
 3 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp
index 6c30b3052..e47a3e8d8 100644
--- a/lib/valueflow.cpp
+++ b/lib/valueflow.cpp
@@ -1722,7 +1722,7 @@ static bool valueFlowForward(Token * const               startToken,
                 // '{'
                 Token * const startToken1 = tok2->linkAt(1)->next();
 
-                valueFlowForward(startToken1->next(),
+                bool vfresult = valueFlowForward(startToken1->next(),
                                  startToken1->link(),
                                  var,
                                  varid,
@@ -1741,7 +1741,7 @@ static bool valueFlowForward(Token * const               startToken,
                 // goto '}'
                 tok2 = startToken1->link();
 
-                if (isReturnScope(tok2)) {
+                if (isReturnScope(tok2) || !vfresult) {
                     if (condAlwaysTrue)
                         return false;
                     removeValues(values, truevalues);
@@ -1750,7 +1750,7 @@ static bool valueFlowForward(Token * const               startToken,
                 if (Token::simpleMatch(tok2, "} else {")) {
                     Token * const startTokenElse = tok2->tokAt(2);
 
-                    valueFlowForward(startTokenElse->next(),
+                    vfresult = valueFlowForward(startTokenElse->next(),
                                      startTokenElse->link(),
                                      var,
                                      varid,
@@ -1769,7 +1769,7 @@ static bool valueFlowForward(Token * const               startToken,
                     // goto '}'
                     tok2 = startTokenElse->link();
 
-                    if (isReturnScope(tok2)) {
+                    if (isReturnScope(tok2) || !vfresult) {
                         if (condAlwaysFalse)
                             return false;
                         removeValues(values, falsevalues);
diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp
index b5306b80b..a8fb49666 100644
--- a/test/testbufferoverrun.cpp
+++ b/test/testbufferoverrun.cpp
@@ -134,6 +134,7 @@ private:
         TEST_CASE(array_index_calculation);
         TEST_CASE(array_index_negative1);
         TEST_CASE(array_index_negative2);    // ticket #3063
+        TEST_CASE(array_index_negative3);
         TEST_CASE(array_index_for_decr);
         TEST_CASE(array_index_varnames);     // FP: struct member. #1576
         TEST_CASE(array_index_for_continue); // for,continue
@@ -1778,6 +1779,22 @@ private:
         ASSERT_EQUALS("[test.cpp:4]: (error) Array 'test.a[10]' accessed at index -1, which is out of bounds.\n", errout.str());
     }
 
+    void array_index_negative3() {
+      check("int f(int i) {\n"
+            "    int p[2] = {0, 0};\n"
+            "    if(i >= 2)\n"
+            "        return 0;\n"
+            "    else if(i == 0)\n"
+            "        return 0;\n"
+            "    return p[i - 1];\n"
+            "}\n"
+            "void g(int i) {\n"
+            "    if( i == 0 )\n"
+            "        return f(i);\n"
+            "}");
+        ASSERT_EQUALS("", errout.str());
+    }
+
     void array_index_for_decr() {
         check("void f()\n"
               "{\n"
diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp
index 37b508241..821c7c918 100644
--- a/test/testvalueflow.cpp
+++ b/test/testvalueflow.cpp
@@ -2455,6 +2455,18 @@ private:
         ASSERT_EQUALS(false, testValueOfX(code, 3U, 0));
         ASSERT_EQUALS(true, testValueOfX(code, 3U, 4));
 
+        code = "int f(int i) {\n"
+               "  if(i >= 2)\n"
+               "    return 0;\n"
+               "  else if(i == 0)\n"
+               "    return 0;\n"
+               "  int a = i;\n"
+               "}\n"
+               "void g(int i) {\n"
+               "  return f(0);\n"
+               "}";
+        ASSERT_EQUALS(false, testValueOfX(code, 6U, 0));
+
         code = "void f1(int x) { a=x; }\n"
                "void f2(int y) { f1(y<123); }\n";
         ASSERT_EQUALS(true, testValueOfX(code, 1U, 0));