CheckBufferOverrun: Fixed FP when accessing string that contains '\0'. Refactoring address-of.

lib/checkbufferoverrun.cpp

@@ -259,6 +259,14 @@ void CheckBufferOverrun::negativeMemoryAllocationSizeError(const Token *tok)
 //---------------------------------------------------------------------------
 
 
+static bool isAddressOf(const Token *tok)
+{
+    const Token *tok2 = tok->astParent();
+    while (Token::Match(tok2, "%name%|.|::|["))
+        tok2 = tok2->astParent();
+    return tok2 && tok2->str() == "&" && !(tok2->astOperand1() && tok2->astOperand2());
+}
+
 /**
  * bailout if variable is used inside if/else/switch block or if there is "break"
  * @param tok token for "if" or "switch"
@@ -781,13 +789,7 @@ void CheckBufferOverrun::valueFlowCheckArrayIndex(const Token * const tok, const
     */
     const bool printInconclusive = _settings->inconclusive;
     // Taking address?
-    bool addressOf = false;
+    const bool addressOf = isAddressOf(tok);
-    {
-        const Token *tok2 = tok->astParent();
-        while (Token::Match(tok2, "%name%|.|::|["))
-            tok2 = tok2->astParent();
-        addressOf = tok2 && tok2->str() == "&" && !(tok2->astOperand1() && tok2->astOperand2());
-    }
 
     // Look for errors first
     for (int warn = 0; warn == 0 || warn == 1; ++warn) {
@@ -1068,9 +1070,9 @@ void CheckBufferOverrun::checkGlobalAndLocalVariable()
     // check string literals
     for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
         if (Token::Match(tok, "%str% [") && tok->next()->astOperand2()) {
-            const std::size_t strLen = Token::getStrLength(tok);
+            const std::size_t size = Token::getStrSize(tok);
             const ValueFlow::Value *value = tok->next()->astOperand2()->getMaxValue(false);
-            if (value && value->intvalue > strLen)
+            if (value && value->intvalue >= (isAddressOf(tok) ? size + 1U : size))
                 bufferOverrunError(tok, tok->str());
         }
 
@@ -1097,7 +1099,7 @@ void CheckBufferOverrun::checkGlobalAndLocalVariable()
                         continue;
 
                     const MathLib::bigint index = arrayInfo.totalIndex(indexes);
-                    if (index <= elements)
+                    if (index < (isAddressOf(tok) ? elements + 1U : elements))
                         continue;
 
                     std::list<const Token *> callstack;

test/testbufferoverrun.cpp

@@ -2070,6 +2070,23 @@ private:
               "}");
         ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (error) Array 'a[10]' accessed at index 20, which is out of bounds.\n", errout.str());
 
+        {
+            // address of
+            check("void f() {\n"
+                  "  int a[10];\n"
+                  "  int *p = a;\n"
+                  "  p[10] = 0;\n"
+                  "}");
+            ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (error) Array 'a[10]' accessed at index 10, which is out of bounds.\n", errout.str());
+
+            check("void f() {\n"
+                  "  int a[10];\n"
+                  "  int *p = a;\n"
+                  "  dostuff(&p[10]);\n"
+                  "}");
+            ASSERT_EQUALS("", errout.str());
+        }
+
         check("void f() {\n"
               "  int a[X];\n" // unknown size
               "  int *p = a;\n"
@@ -2447,6 +2464,12 @@ private:
     void buffer_overrun_28() {
         check("char c = \"abc\"[4];");
         ASSERT_EQUALS("[test.cpp:1]: (error) Buffer is accessed out of bounds: \"abc\"\n", errout.str());
+
+        check("p = &\"abc\"[4];");
+        ASSERT_EQUALS("", errout.str());
+
+        check("char c = \"\\0abc\"[2];");
+        ASSERT_EQUALS("", errout.str());
     }
 
     void buffer_overrun_bailoutIfSwitch() {