From baa1ae079ddb8e327bbe2b438f8014a5a00737de Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= <daniel.marjamaki@gmail.com>
Date: Sun, 3 May 2015 15:00:47 +0200
Subject: [PATCH] New check: negative size in array declaration. Ticket #1760

---
 lib/checkbufferoverrun.cpp | 27 +++++++++++++++++++++++++++
 lib/checkbufferoverrun.h   |  6 ++++++
 test/testbufferoverrun.cpp | 15 +++++++++++----
 3 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp
index 74b240c51..bbeedf3ed 100644
--- a/lib/checkbufferoverrun.cpp
+++ b/lib/checkbufferoverrun.cpp
@@ -970,6 +970,33 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo
     }
 }
 
+//---------------------------------------------------------------------------
+// Negative size in array declarations
+//---------------------------------------------------------------------------
+
+void CheckBufferOverrun::negativeArraySize()
+{
+    const SymbolDatabase* symbolDatabase = _tokenizer->getSymbolDatabase();
+    for (unsigned int i = 1; i <= _tokenizer->varIdCount(); i++) {
+        const Variable * const var = symbolDatabase->getVariableFromVarId(i);
+        if (!var || !var->isArray())
+            continue;
+        const Token * const nameToken = var->nameToken();
+        if (!Token::Match(nameToken, "%var% [") || !nameToken->next()->astOperand2())
+            continue;
+        const ValueFlow::Value *sz = nameToken->next()->astOperand2()->getValueLE(-1,_settings);
+        if (!sz)
+            continue;
+        negativeArraySizeError(nameToken);
+    }
+}
+
+void CheckBufferOverrun::negativeArraySizeError(const Token *tok)
+{
+    reportError(tok, Severity::error, "negativeArraySize",
+                "Declaration of array '" + (tok ? tok->str() : std::string()) + "' with negative size is undefined behaviour");
+}
+
 //---------------------------------------------------------------------------
 // Checking member variables of structs.
 //---------------------------------------------------------------------------
diff --git a/lib/checkbufferoverrun.h b/lib/checkbufferoverrun.h
index c8defb0d7..20e0ae250 100644
--- a/lib/checkbufferoverrun.h
+++ b/lib/checkbufferoverrun.h
@@ -60,6 +60,7 @@ public:
         checkBufferOverrun.bufferOverrun();
         checkBufferOverrun.bufferOverrun2();
         checkBufferOverrun.arrayIndexThenCheck();
+        checkBufferOverrun.negativeArraySize();
     }
 
     /** @brief %Check for buffer overruns */
@@ -71,6 +72,9 @@ public:
     /** @brief Using array index before bounds check */
     void arrayIndexThenCheck();
 
+    /** @brief negative size for array */
+    void negativeArraySize();
+
     /** @brief %Check for buffer overruns by inspecting execution paths */
     void executionPaths();
 
@@ -218,6 +222,7 @@ private:
     void bufferOverrunError(const std::list<const Token *> &callstack, const std::string &varnames = emptyString);
     void strncatUsageError(const Token *tok);
     void negativeMemoryAllocationSizeError(const Token *tok); // provide a negative value to memory allocation function
+    void negativeArraySizeError(const Token *tok);
     void outOfBoundsError(const Token *tok, const std::string &what, const bool show_size_info, const MathLib::bigint &supplied_size, const MathLib::bigint &actual_size);
     void sizeArgumentAsCharError(const Token *tok);
     void terminateStrncpyError(const Token *tok, const std::string &varname);
@@ -251,6 +256,7 @@ public:
         c.possibleBufferOverrunError(0, "source", "destination", false);
         c.argumentSizeError(0, "function", "array");
         c.negativeMemoryAllocationSizeError(0);
+        c.negativeArraySizeError(0);
         c.reportError(nullptr, Severity::warning, "arrayIndexOutOfBoundsCond", "Array 'x[10]' accessed at index 20, which is out of bounds. Otherwise condition 'y==20' is redundant.");
     }
 private:
diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp
index 3f80c4e84..165fbe517 100644
--- a/test/testbufferoverrun.cpp
+++ b/test/testbufferoverrun.cpp
@@ -60,10 +60,8 @@ private:
         }
 
         // Check for buffer overruns..
-        CheckBufferOverrun checkBufferOverrun(&tokenizer, &settings, this);
-        checkBufferOverrun.bufferOverrun();
-        checkBufferOverrun.bufferOverrun2();
-        checkBufferOverrun.arrayIndexThenCheck();
+        CheckBufferOverrun checkBufferOverrun;
+        checkBufferOverrun.runSimplifiedChecks(&tokenizer, &settings, this);
     }
 
     void check(const char code[], const Settings &settings, const char filename[] = "test.cpp") {
@@ -246,6 +244,7 @@ private:
         TEST_CASE(bufferNotZeroTerminated);
 
         TEST_CASE(negativeMemoryAllocationSizeError) // #389
+        TEST_CASE(negativeArraySize);
 
         TEST_CASE(garbage1) // #6303
     }
@@ -3806,6 +3805,14 @@ private:
         ASSERT_EQUALS("[test.cpp:4]: (error) Memory allocation size is negative.\n", errout.str());
     }
 
+    void negativeArraySize() {
+        check("void f(int sz) {\n" // #1760 - VLA
+              "   int a[sz];\n"
+              "}\n"
+              "void x() { f(-100); }");
+        ASSERT_EQUALS("[test.cpp:2]: (error) Declaration of array 'a' with negative size is undefined behaviour\n", errout.str());
+    }
+
     void garbage1() {
         check("void foo() {\n"
               "char *a = malloc(10);\n"