From 031adef6ea0933fbfec3ffb263f87c85edd2e9a4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= <daniel.marjamaki@gmail.com>
Date: Fri, 30 Nov 2012 09:01:15 +0100
Subject: [PATCH] Array index checking: Fixed TODO comment (false negatives
 when using ?:)

---
 lib/checkbufferoverrun.cpp | 30 ++++++++++++++++++++++++++----
 test/testbufferoverrun.cpp | 17 +++++++++++++++++
 2 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp
index 1c2e262bf..0918fccc3 100644
--- a/lib/checkbufferoverrun.cpp
+++ b/lib/checkbufferoverrun.cpp
@@ -431,10 +431,32 @@ void CheckBufferOverrun::parse_for_body(const Token *tok, const ArrayInfo &array
     const std::string pattern = (arrayInfo.varid() ? std::string("%varid%") : arrayInfo.varname()) + " [ " + strindex + " ]";
 
     for (const Token* tok2 = tok; tok2 && tok2 != tok->link(); tok2 = tok2->next()) {
-        // TODO: try to reduce false negatives. This is just a quick fix
-        // for TestBufferOverrun::array_index_for_question
-        if (tok2->str() == "?")
-            break;
+        // TestBufferOverrun::array_index_for_question
+        if (tok2->str() == "?") {
+            // does condition check counter variable?
+            bool usesCounter = false;
+            const Token *tok3 = tok2->previous();
+            while (Token::Match(tok3, "%var%|%num%|)|>=|>|<=|<|==|!=")) {
+                if (tok3->str() == strindex) {
+                    usesCounter = true;
+                    break;
+                }
+                tok3 = tok3->previous();
+            }
+
+            // If strindex is used in the condition then skip the
+            // conditional expressions
+            if (usesCounter) {
+                while (tok2 && !Token::Match(tok2, "[)],;]")) {
+                    if (tok2->str() == "(" || tok2->str() == "[")
+                        tok2 = tok2->link();
+                    tok2 = tok2->next();
+                }
+                if (!tok2)
+                    break;
+                continue;
+            }
+        }
 
         if (Token::simpleMatch(tok2, "for (") && Token::simpleMatch(tok2->next()->link(), ") {")) {
             const Token *endpar = tok2->next()->link();
diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp
index d32b48ef8..4dced8439 100644
--- a/test/testbufferoverrun.cpp
+++ b/test/testbufferoverrun.cpp
@@ -1872,6 +1872,23 @@ private:
               "    }\n"
               "}");
         ASSERT_EQUALS("", errout.str());
+
+        check("void f() {\n"
+              "    int a[10];\n"
+              "    for (int i = 0; i != 10; ++i) {\n"
+              "        some_condition ? 0 : a[i-1];\n"
+              "    }\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[10]' accessed at index -1, which is out of bounds.\n", errout.str());
+
+        check("void f() {\n"
+              "    int a[10];\n"
+              "    for (int i = 0; i != 10; ++i) {\n"
+              "        i==0 ? 0 : a[i-1];\n"
+              "        a[i-1] = 0;\n"
+              "    }\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:5]: (error) Array 'a[10]' accessed at index -1, which is out of bounds.\n", errout.str());
     }
 
     void array_index_for_varid0() { // #4228: No varid for counter variable