Fixed #4840 (false negative: buffer access out of bounds)

lib/checkbufferoverrun.cpp

@@ -319,6 +319,11 @@ static bool for_condition(const Token *tok2, unsigned int varid, std::string &mi
         maxMinFlipped = true;
         max_value = min_value;
         min_value = tok2->str();
+    }  else if (Token::Match(tok2, "%varid% -- ; )", varid) ||
+                Token::Match(tok2, "-- %varid% ; )", varid)) {
+        maxMinFlipped = true;
+        max_value = min_value;
+        min_value = (tok2->str() == "--") ? "1" : "0";
     } else {
         // parse condition
         while (tok2 && tok2->str() != ";") {
@@ -819,7 +824,8 @@ void CheckBufferOverrun::checkScopeForBody(const Token *tok, const ArrayInfo &ar
     }
     if (!tok2 || tok2->str() != ";")
         return;
-    if (!for3(tok2->next(), counter_varid, min_counter_value, max_counter_value, maxMinFlipped))
+    const bool hasFor3 = tok2->next()->str() != ")";
+    if (hasFor3 && !for3(tok2->next(), counter_varid, min_counter_value, max_counter_value, maxMinFlipped))
         return;
 
     if (Token::Match(tok2->next(), "%var% =") && MathLib::toLongNumber(max_counter_value) < size)

lib/tokenize.cpp

@@ -3382,15 +3382,22 @@ bool Tokenizer::simplifyTokenList()
 
     simplifyUndefinedSizeArray();
 
+    simplifyCasts();
+
+    // Simplify simple calculations before replace constants, this allows the replacement of constants that are calculated
+    // e.g. const static int value = sizeof(X)/sizeof(Y);
+    simplifyCalculations();
+
     // Replace constants..
     for (Token *tok = list.front(); tok; tok = tok->next()) {
-        if (Token::Match(tok, "const static| %type% %var% = %num% ;")) {
+        if (Token::Match(tok, "const static| %type% %var% = %num% ;") ||
+            Token::Match(tok, "const static| %type% %var% ( %num% ) ;")) {
             unsigned int offset = 0;
             if (tok->strAt(1) == "static")
                 offset = 1;
             const unsigned int varId(tok->tokAt(2 + offset)->varId());
             if (varId == 0) {
-                tok = tok->tokAt(5);
+                tok = tok->tokAt(5 + offset);
                 continue;
             }
 
@@ -3413,8 +3420,6 @@ bool Tokenizer::simplifyTokenList()
         }
     }
 
-    simplifyCasts();
-
     // Simplify simple calculations..
     simplifyCalculations();
 
@@ -5937,9 +5942,10 @@ bool Tokenizer::simplifyKnownVariables()
                 tok = tok->previous();
                 goback = false;
             }
-            if (tok->isName() && Token::Match(tok, "static| const| static| %type% const| %var% = %any% ;")) {
+            if (tok->isName() && (Token::Match(tok, "static| const| static| %type% const| %var% = %any% ;") ||
+                                  Token::Match(tok, "static| const| static| %type% const| %var% ( %any% ) ;"))) {
                 bool isconst = false;
-                for (const Token *tok2 = tok; tok2->str() != "="; tok2 = tok2->next()) {
+                for (const Token *tok2 = tok; (tok2->str() != "=") && (tok2->str() != "("); tok2 = tok2->next()) {
                     if (tok2->str() == "const") {
                         isconst = true;
                         break;
@@ -5962,7 +5968,7 @@ bool Tokenizer::simplifyKnownVariables()
 
                 const Token * const vartok = (tok->next() && tok->next()->str() == "const") ? tok->tokAt(2) : tok->next();
                 const Token * const valuetok = vartok->tokAt(2);
-                if (Token::Match(valuetok, "%bool%|%char%|%num%|%str% ;")) {
+                if (Token::Match(valuetok, "%bool%|%char%|%num%|%str% )| ;")) {
                     //check if there's not a reference usage inside the code
                     bool withreference = false;
                     for (const Token *tok2 = valuetok->tokAt(2); tok2; tok2 = tok2->next()) {

test/testbufferoverrun.cpp

@@ -121,6 +121,7 @@ private:
         TEST_CASE(array_index_43); // struct with array
         TEST_CASE(array_index_44); // #3979
         TEST_CASE(array_index_45); // #4207 - calling function with variable number of parameters (...)
+        TEST_CASE(array_index_46); // #4840 - two-statement for loop
         TEST_CASE(array_index_multidim);
         TEST_CASE(array_index_switch_in_for);
         TEST_CASE(array_index_for_in_for);   // FP: #2634
@@ -1506,6 +1507,36 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
+    // Two statement for-loop
+    void array_index_46() {
+        // #4840
+        check("void bufferAccessOutOfBounds2() {\n"
+              "    char *buffer[]={\"a\",\"b\",\"c\"};\n"
+              "    for(int i=3; i--;) {\n"
+              "        printf(\"files(%i): %s\n\", 3-i, buffer[3-i]);\n"
+              "    }\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:4]: (error) Array 'buffer[3]' accessed at index 3, which is out of bounds.\n", errout.str());
+
+        check("void f() {\n"
+              "    int buffer[9];\n"
+              "    long int i;\n"
+              "    for(i=10; i--;) {\n"
+              "        buffer[i] = i;\n"
+              "    }\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:5]: (error) Buffer is accessed out of bounds: buffer\n", errout.str());
+
+        // Correct access limits -> i from 9 to 0
+        check("void f() {\n"
+              "    int buffer[10];\n"
+              "    for(unsigned long int i=10; i--;) {\n"
+              "        buffer[i] = i;\n"
+              "    }\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+    }
+
     void array_index_multidim() {
         check("void f()\n"
               "{\n"

test/testtokenize.cpp

@@ -333,6 +333,7 @@ private:
         TEST_CASE(simplify_constants2);
         TEST_CASE(simplify_constants3);
         TEST_CASE(simplify_constants4);
+        TEST_CASE(simplify_constants5);
         TEST_CASE(simplify_null);
         TEST_CASE(simplifyMulAndParens);    // Ticket #2784 + #3184
 
@@ -5295,6 +5296,15 @@ private:
         ASSERT_EQUALS("x = 4 ;\ny = 50 ;", tokenizeAndStringify(code,true));
     }
 
+    void simplify_constants5() {
+        const char code[] = "int buffer[10];\n"
+                            "static const int NELEMS = sizeof(buffer)/sizeof(int);\n"
+                            "static const int NELEMS2(sizeof(buffer)/sizeof(int));\n"
+                            "x = NELEMS;\n"
+                            "y = NELEMS2;\n";
+        ASSERT_EQUALS("int buffer [ 10 ] ;\n\n\nx = 10 ;\ny = 10 ;", tokenizeAndStringify(code,true));
+    }
+
     void simplify_null() {
         {
             const char code[] =