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() {