From 3c85d8a8acf1e0b4bf4f06507e91915f56c3bd70 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= <daniel.marjamaki@gmail.com>
Date: Sun, 17 Mar 2019 19:02:36 +0100
Subject: [PATCH] ValueFlow: Better info for buffer size values

---
 lib/checkbufferoverrun.cpp | 53 ++++++++++++++++++--------------------
 lib/valueflow.cpp          |  1 +
 test/testbufferoverrun.cpp |  3 ++-
 3 files changed, 28 insertions(+), 29 deletions(-)

diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp
index 54e724610..f1e39968a 100644
--- a/lib/checkbufferoverrun.cpp
+++ b/lib/checkbufferoverrun.cpp
@@ -60,25 +60,13 @@ static const CWE CWE788(788U);  // Access of Memory Location After End of Buffer
 
 //---------------------------------------------------------------------------
 
-static std::vector<Dimension> getDynamicDimensions(const Token *tok, MathLib::bigint typeSize)
+static const ValueFlow::Value *getBufferSizeValue(const Token *tok)
 {
-    if (typeSize == 0) {
-        const std::vector<Dimension> dimensions;
-        return dimensions;
-    }
     for (const ValueFlow::Value &value : tok->values()) {
-        if (!value.isBufferSizeValue())
-            continue;
-        Dimension dim;
-        dim.tok = nullptr;
-        dim.num = value.intvalue / typeSize;
-        dim.known = value.isKnown();
-        const std::vector<Dimension> dimensions{dim};
-        return dimensions;
+        if (value.isBufferSizeValue())
+            return &value;
     }
-
-    const std::vector<Dimension> dimensions;
-    return dimensions;
+    return nullptr;
 }
 
 static size_t getMinFormatStringOutputLength(const std::vector<const Token*> &parameters, unsigned int formatStringArgNr)
@@ -218,6 +206,7 @@ void CheckBufferOverrun::arrayIndex()
             continue;
 
         std::vector<Dimension> dimensions;
+        ErrorPath errorPath;
 
         bool mightBeLarger;
 
@@ -232,7 +221,15 @@ void CheckBufferOverrun::arrayIndex()
             dimensions.emplace_back(dim);
             mightBeLarger = false;
         } else if (array->valueType() && array->valueType()->pointer >= 1 && array->valueType()->isIntegral()) {
-            dimensions = getDynamicDimensions(array, array->valueType()->typeSize(*mSettings));
+            const ValueFlow::Value *value = getBufferSizeValue(array);
+            if (!value)
+                continue;
+            errorPath = value->errorPath;
+            Dimension dim;
+            dim.known = value->isKnown();
+            dim.tok = nullptr;
+            dim.num = value->intvalue / array->valueType()->typeSize(*mSettings);
+            dimensions.emplace_back(dim);
             mightBeLarger = false;
         }
 
@@ -326,22 +323,24 @@ size_t CheckBufferOverrun::getBufferSize(const Token *bufTok) const
     if (!bufTok->valueType())
         return 0;
     const Variable *var = bufTok->variable();
+
+    if (!var || var->dimensions().empty()) {
+        const ValueFlow::Value *value = getBufferSizeValue(bufTok);
+        if (value)
+            return value->intvalue;
+    }
+
     if (!var)
         return 0;
-    const MathLib::bigint typeSize = bufTok->valueType()->typeSize(*mSettings);
-    std::vector<Dimension> dimensions;
-    if (!var->dimensions().empty())
-        dimensions = var->dimensions();
-    else
-        dimensions = getDynamicDimensions(bufTok, typeSize);
-    if (dimensions.empty())
-        return 0;
 
     MathLib::bigint dim = 1;
-    for (const Dimension &d : dimensions)
+    for (const Dimension &d : var->dimensions())
         dim *= d.num;
+
     if (var->isPointerArray())
         return dim * mSettings->sizeof_pointer;
+
+    const MathLib::bigint typeSize = bufTok->valueType()->typeSize(*mSettings);
     return dim * typeSize;
 }
 //---------------------------------------------------------------------------
@@ -372,8 +371,6 @@ static bool checkBufferSize(const Token *ftok, const Library::ArgumentChecks::Mi
         if (arg && arg2 && arg->hasKnownIntValue() && arg2->hasKnownIntValue())
             return (arg->getKnownIntValue() * arg2->getKnownIntValue()) <= bufferSize;
         break;
-    case Library::ArgumentChecks::MinSize::Type::VALUE:
-        return minsize.value <= bufferSize;
     case Library::ArgumentChecks::MinSize::Type::NONE:
         break;
     };
diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp
index 1c1bcd67b..1f981ab37 100644
--- a/lib/valueflow.cpp
+++ b/lib/valueflow.cpp
@@ -5038,6 +5038,7 @@ static void valueFlowDynamicBufferSize(TokenList *tokenlist, SymbolDatabase *sym
                 continue;
 
             ValueFlow::Value value(sizeArg->getKnownIntValue());
+            value.errorPath.emplace_back(tok->tokAt(2), "Assign " + tok->strAt(1) + ", buffer with size " + MathLib::toString(value.intvalue));
             value.valueType = ValueFlow::Value::ValueType::BUFFER_SIZE;
             value.setKnown();
             const std::list<ValueFlow::Value> values{value};
diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp
index 8ab37e044..a9ff6d9b5 100644
--- a/test/testbufferoverrun.cpp
+++ b/test/testbufferoverrun.cpp
@@ -3601,7 +3601,8 @@ private:
               "void bar(char *p) {\n"
               "    strncpy(p, str, 100);\n"
               "}\n", false);
-        ASSERT_EQUALS("[test.cpp:4]: (warning, inconclusive) The buffer 'str' may not be null-terminated after the call to strncpy().\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:4]: (warning, inconclusive) The buffer 'str' may not be null-terminated after the call to strncpy().\n"
+                      "[test.cpp:8]: (warning, inconclusive) The buffer 'p' may not be null-terminated after the call to strncpy().\n", errout.str());
     }
 
     void terminateStrncpy4() {