CheckBufferOverrun: Added bufferOverrun2 that is based on ValueFlow/SymbolDatabase/Ast from the start. Replaced some old checking.

lib/checkbufferoverrun.cpp

@@ -1056,11 +1056,13 @@ void CheckBufferOverrun::checkScope(const Token *tok, const std::vector<std::str
 
         // Loop..
         if (Token::simpleMatch(tok, "for (")) {
-            const ArrayInfo arrayInfo1(declarationId, varnames, (unsigned int)size, (unsigned int)total_size);
-            bool bailout = false;
-            checkScopeForBody(tok, arrayInfo1, bailout);
-            if (bailout)
-                break;
+            /*
+                const ArrayInfo arrayInfo1(declarationId, varnames, (unsigned int)size, (unsigned int)total_size);
+                bool bailout = false;
+                checkScopeForBody(tok, arrayInfo1, bailout);
+                if (bailout)
+                    break;
+            */
             continue;
         }
 
@@ -1757,6 +1759,39 @@ void CheckBufferOverrun::bufferOverrun()
 }
 //---------------------------------------------------------------------------
 
+void CheckBufferOverrun::bufferOverrun2()
+{
+    // singlepass checking using ast, symboldatabase and valueflow
+    for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
+        if (Token::Match(tok, "%var% [")) {
+            const Variable *var = tok->variable();
+            if (!var || var->nameToken() == tok || !var->isArray())
+                continue;
+
+            // TODO: last array in struct..
+            if (var->dimension(0) <= 1 && Token::simpleMatch(var->nameToken()->linkAt(1),"] ; }"))
+                continue;
+
+            // TODO: what to do about negative index..
+            const Token *index = tok->next()->astOperand2();
+            if (index && index->getValueLE(-1LL,_settings))
+                continue;
+
+            ArrayInfo arrayInfo(var,_tokenizer);
+
+            // Set full varname..
+            if (tok->astParent() && tok->astParent()->str() == ".") {
+                const Token *parent = tok->astParent();
+                while (parent && parent->astParent() && parent->astParent()->str() == ".")
+                    parent = parent->astParent();
+                arrayInfo.varname(parent->expressionString());
+            }
+
+            valueFlowCheckArrayIndex(tok->next(), arrayInfo);
+        }
+    }
+}
+//---------------------------------------------------------------------------
 
 MathLib::bigint CheckBufferOverrun::countSprintfLength(const std::string &input_string, const std::list<const Token*> &parameters)
 {

lib/checkbufferoverrun.h

@@ -58,6 +58,7 @@ public:
     void runSimplifiedChecks(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger) {
         CheckBufferOverrun checkBufferOverrun(tokenizer, settings, errorLogger);
         checkBufferOverrun.bufferOverrun();
+        checkBufferOverrun.bufferOverrun2();
         checkBufferOverrun.arrayIndexThenCheck();
         checkBufferOverrun.writeOutsideBufferSize();
     }
@@ -65,6 +66,9 @@ public:
     /** @brief %Check for buffer overruns */
     void bufferOverrun();
 
+    /** @brief %Check for buffer overruns #2 (single pass, use ast and valueflow) */
+    void bufferOverrun2();
+
     /** @brief Using array index before bounds check */
     void arrayIndexThenCheck();
 

test/testbufferoverrun.cpp

@@ -67,6 +67,7 @@ private:
         // Check for buffer overruns..
         CheckBufferOverrun checkBufferOverrun(&tokenizer, &settings, this);
         checkBufferOverrun.bufferOverrun();
+        checkBufferOverrun.bufferOverrun2();
         checkBufferOverrun.arrayIndexThenCheck();
         checkBufferOverrun.writeOutsideBufferSize();
     }
@@ -610,7 +611,7 @@ private:
               "    for ( unsigned int i = 0; i < 64; ++i )\n"
               "        f.str[i] = 0;\n"
               "}");
-        ASSERT_EQUALS("[test.cpp:10]: (error) Buffer is accessed out of bounds: f.str\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:10]: (error) Array 'f.str[10]' accessed at index 63, which is out of bounds.\n", errout.str());
 
         check("struct AB { char a[NUM]; char b[NUM]; }\n"
               "void f(struct AB *ab) {\n"
@@ -1391,7 +1392,8 @@ private:
               "   y = var[ 0 ].arr[ 3 ];\n" // <-- array access out of bounds
               "   return y;\n"
               "}");
-        ASSERT_EQUALS("[test.cpp:10]: (error) Array 'var.arr[3]' accessed at index 3, which is out of bounds.\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:10]: (error) Array 'var.arr[3]' accessed at index 3, which is out of bounds.\n"
+                      "[test.cpp:10]: (error) Array 'var[0].arr[3]' accessed at index 3, which is out of bounds.\n", errout.str());
 
         check("int f( )\n"
               "{\n"
@@ -1435,7 +1437,8 @@ private:
               "var[0].var[ 2 ] = 2;\n"
               "var[0].var[ 4 ] = 4;\n" // <-- array access out of bounds
               "}");
-        ASSERT_EQUALS("[test.cpp:9]: (error) Array 'var.var[3]' accessed at index 4, which is out of bounds.\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:9]: (error) Array 'var.var[3]' accessed at index 4, which is out of bounds.\n"
+                      "[test.cpp:9]: (error) Array 'var[0].var[3]' accessed at index 4, which is out of bounds.\n", errout.str());
 
         check("void f( ) {\n"
               "struct S{\n"
@@ -3933,7 +3936,7 @@ private:
               "    A::X x;\n"
               "    x.buf[10] = 0;\n"
               "}");
-        TODO_ASSERT_EQUALS("[test.cpp:9]: (error) Array 'x.buf[10]' index 10 out of bounds.\n", "", errout.str());
+        ASSERT_EQUALS("[test.cpp:9]: (error) Array 'x.buf[10]' accessed at index 10, which is out of bounds.\n", errout.str());
     }
 
     void getErrorMessages() {