Fixed #1748 (Undefined Behavior: Modification of string literal)

lib/checkstring.cpp

@@ -29,6 +29,30 @@ namespace {
 }
 
 
+//---------------------------------------------------------------------------
+// Writing string literal is UB
+//---------------------------------------------------------------------------
+void CheckString::stringLiteralWrite()
+{
+    for (const Token* tok = _tokenizer->tokens(); tok; tok = tok->next()) {
+        if (!tok->variable())
+            continue;
+        const Token *str = tok->getValueTokenMinStrSize();
+        if (!str)
+            continue;
+        if (Token::Match(tok, "%var% [") && Token::Match(tok->linkAt(1), "] ="))
+            stringLiteralWriteError(tok);
+        else if (Token::Match(tok->previous(), "* %var% ="))
+            stringLiteralWriteError(tok);
+    }
+}
+
+void CheckString::stringLiteralWriteError(const Token *tok)
+{
+    reportError(tok, Severity::error, "stringLiteralWrite",
+                "Modifying string literal directly or indirectly is UB");
+}
+
 //---------------------------------------------------------------------------
 // Check for string comparison involving two static strings.
 // if(strcmp("00FF00","00FF00")==0) // <- statement is always true

lib/checkstring.h

@@ -59,8 +59,12 @@ public:
         checkString.checkIncorrectStringCompare();
         checkString.checkAlwaysTrueOrFalseStringCompare();
         checkString.sprintfOverlappingData();
+        checkString.stringLiteralWrite();
     }
 
+    /** @brief undefined behaviour, writing string literal */
+    void stringLiteralWrite();
+
     /** @brief str plus char (unusual pointer arithmetic) */
     void strPlusChar();
 
@@ -77,6 +81,7 @@ public:
     void sprintfOverlappingData();
 
 private:
+    void stringLiteralWriteError(const Token *tok);
     void sprintfOverlappingDataError(const Token *tok, const std::string &varname);
     void strPlusCharError(const Token *tok);
     void incorrectStringCompareError(const Token *tok, const std::string& func, const std::string &string);

test/teststring.cpp

@@ -32,6 +32,8 @@ public:
 private:
 
     void run() {
+        TEST_CASE(stringLiteralWrite);
+
         TEST_CASE(alwaysTrueFalseStringCompare);
         TEST_CASE(suspiciousStringCompare);
         TEST_CASE(suspiciousStringCompare_char);
@@ -95,6 +97,26 @@ private:
         checkString.checkAlwaysTrueOrFalseStringCompare();
     }
 
+    void stringLiteralWrite() {
+        check("void f() {\n"
+              "  char *abc = \"abc\";\n"
+              "  abc[0] = 'a';\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3]: (error) Modifying string literal directly or indirectly is UB\n", errout.str());
+
+        check("void f() {\n"
+              "  char *abc = \"abc\";\n"
+              "  *abc = 'a';\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3]: (error) Modifying string literal directly or indirectly is UB\n", errout.str());
+
+        check("void f() {\n"
+              "  char *abc = \"abc\";\n"
+              "  if (*abc == 'a'){}\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+    }
+
     void alwaysTrueFalseStringCompare() {
         check("void f() {\n"
               "  if (strcmp(\"A\",\"A\")){}\n"