From 9d31afb6631b73f420074d8a3bf2351bad32cea2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sun, 3 May 2015 10:44:40 +0200 Subject: [PATCH] Fixed #1748 (Undefined Behavior: Modification of string literal) --- lib/checkstring.cpp | 24 ++++++++++++++++++++++++ lib/checkstring.h | 5 +++++ test/teststring.cpp | 22 ++++++++++++++++++++++ 3 files changed, 51 insertions(+) diff --git a/lib/checkstring.cpp b/lib/checkstring.cpp index 936dce4ab..090dbeb33 100644 --- a/lib/checkstring.cpp +++ b/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 diff --git a/lib/checkstring.h b/lib/checkstring.h index e3f46ce52..eab7e20fd 100644 --- a/lib/checkstring.h +++ b/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); diff --git a/test/teststring.cpp b/test/teststring.cpp index 96db7cc6a..80d2dd1a9 100644 --- a/test/teststring.cpp +++ b/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"