From 644a2163945340d2df6f4390781730bb74238567 Mon Sep 17 00:00:00 2001
From: PKEuS <philipp.kloke@web.de>
Date: Thu, 7 Jul 2016 19:38:15 +0200
Subject: [PATCH] Fixed two false positives related to char arrays initialized
 by a literal: - Run check for writing to string literals on non-simplified
 token list (#7283) - Run buffer overrun checking for string literals on
 non-simplified token list
 (https://sourceforge.net/p/cppcheck/discussion/general/thread/2c33dfc5/)

---
 lib/checkbufferoverrun.cpp | 13 +------------
 lib/checkbufferoverrun.h   | 17 ++++++++++++-----
 lib/checkstring.h          |  2 +-
 test/testbufferoverrun.cpp | 14 ++++++++++----
 test/teststring.cpp        | 18 ++++++++++++++++++
 5 files changed, 42 insertions(+), 22 deletions(-)

diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp
index c3b64d644..10f9f1b29 100644
--- a/lib/checkbufferoverrun.cpp
+++ b/lib/checkbufferoverrun.cpp
@@ -1464,21 +1464,10 @@ void CheckBufferOverrun::checkStructVariable()
         }
     }
 }
+
 //---------------------------------------------------------------------------
 
 void CheckBufferOverrun::bufferOverrun()
-{
-    checkGlobalAndLocalVariable();
-    if (_tokenizer->isMaxTime())
-        return;
-    checkStructVariable();
-    checkBufferAllocatedWithStrlen();
-    checkStringArgument();
-    checkInsecureCmdLineArgs();
-}
-//---------------------------------------------------------------------------
-
-void CheckBufferOverrun::bufferOverrun2()
 {
     // singlepass checking using ast, symboldatabase and valueflow
     for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
diff --git a/lib/checkbufferoverrun.h b/lib/checkbufferoverrun.h
index 68abbfe11..95dae4b8a 100644
--- a/lib/checkbufferoverrun.h
+++ b/lib/checkbufferoverrun.h
@@ -61,17 +61,24 @@ public:
 
     void runSimplifiedChecks(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger) {
         CheckBufferOverrun checkBufferOverrun(tokenizer, settings, errorLogger);
+        checkBufferOverrun.checkGlobalAndLocalVariable();
+        if (_tokenizer->isMaxTime())
+            return;
+        checkBufferOverrun.checkStructVariable();
+        checkBufferOverrun.checkBufferAllocatedWithStrlen();
+        checkBufferOverrun.checkInsecureCmdLineArgs();
         checkBufferOverrun.bufferOverrun();
-        checkBufferOverrun.bufferOverrun2();
         checkBufferOverrun.arrayIndexThenCheck();
         checkBufferOverrun.negativeArraySize();
     }
 
-    /** @brief %Check for buffer overruns */
-    void bufferOverrun();
+    void runChecks(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger) {
+        CheckBufferOverrun checkBufferOverrun(tokenizer, settings, errorLogger);
+        checkBufferOverrun.checkStringArgument();
+    }
 
-    /** @brief %Check for buffer overruns #2 (single pass, use ast and valueflow) */
-    void bufferOverrun2();
+    /** @brief %Check for buffer overruns (single pass, use ast and valueflow) */
+    void bufferOverrun();
 
     /** @brief Using array index before bounds check */
     void arrayIndexThenCheck();
diff --git a/lib/checkstring.h b/lib/checkstring.h
index e82cab275..8529a4fd0 100644
--- a/lib/checkstring.h
+++ b/lib/checkstring.h
@@ -49,6 +49,7 @@ public:
         // Checks
         checkString.strPlusChar();
         checkString.checkSuspiciousStringCompare();
+        checkString.stringLiteralWrite();
     }
 
     /** @brief Run checks against the simplified token list */
@@ -59,7 +60,6 @@ public:
         checkString.checkIncorrectStringCompare();
         checkString.checkAlwaysTrueOrFalseStringCompare();
         checkString.sprintfOverlappingData();
-        checkString.stringLiteralWrite();
     }
 
     /** @brief undefined behaviour, writing string literal */
diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp
index df5fb440f..f606f74a7 100644
--- a/test/testbufferoverrun.cpp
+++ b/test/testbufferoverrun.cpp
@@ -54,16 +54,15 @@ private:
         Tokenizer tokenizer(&settings, this);
         std::istringstream istr(code);
         tokenizer.tokenize(istr, filename);
-        tokenizer.simplifyTokenList2();
 
         // Clear the error buffer..
         errout.str("");
 
         // Check for buffer overruns..
         CheckBufferOverrun checkBufferOverrun(&tokenizer, &settings, this);
-        checkBufferOverrun.bufferOverrun();
-        checkBufferOverrun.bufferOverrun2();
-        checkBufferOverrun.arrayIndexThenCheck();
+        checkBufferOverrun.runChecks(&tokenizer, &settings, this);
+        tokenizer.simplifyTokenList2();
+        checkBufferOverrun.runSimplifiedChecks(&tokenizer, &settings, this);
     }
 
     void run() {
@@ -2001,6 +2000,13 @@ private:
               "    if ( name[0] == 'U' ? name[1] : 0) {}\n"
               "}");
         ASSERT_EQUALS("", errout.str());
+
+        check("int main(int argc, char **argv) {\n"
+              "    char str[6] = \"\\0\";\n"
+              "    unsigned short port = 65535;\n"
+              "    snprintf(str, sizeof(str), \"%hu\", port);\n"
+              "}", settings0, "test.c");
+        ASSERT_EQUALS("", errout.str());
     }
 
     void array_index_same_struct_and_var_name() {
diff --git a/test/teststring.cpp b/test/teststring.cpp
index 767770356..bac904459 100644
--- a/test/teststring.cpp
+++ b/test/teststring.cpp
@@ -88,6 +88,24 @@ private:
               "  abc[0] = 'a';\n"
               "}");
         ASSERT_EQUALS("", errout.str());
+
+        check("void foo_FP1(char *p) {\n"
+              "  p[1] = 'B';\n"
+              "}\n"
+              "void foo_FP2(void) {\n"
+              "  char* s = \"Y\";\n"
+              "  foo_FP1(s);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:5]: (error) Modifying string literal \"Y\" directly or indirectly is undefined behaviour.\n", errout.str());
+
+        check("void foo_FP1(char *p) {\n"
+              "  p[1] = 'B';\n"
+              "}\n"
+              "void foo_FP2(void) {\n"
+              "  char s[10] = \"Y\";\n"
+              "  foo_FP1(s);\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
     }
 
     void alwaysTrueFalseStringCompare() {