Skip to content

Commit 644a216

Browse files
PKEuSPKEuS
committed
Fixed two false positives related to char arrays initialized by a literal:
- Run check for writing to string literals on non-simplified token list (cppcheck-opensource#7283) - Run buffer overrun checking for string literals on non-simplified token list (https://sourceforge.net/p/cppcheck/discussion/general/thread/2c33dfc5/)
1 parent 3bdcf68 commit 644a216

5 files changed

Lines changed: 42 additions & 22 deletions

File tree

‎lib/checkbufferoverrun.cpp‎

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1464,21 +1464,10 @@ void CheckBufferOverrun::checkStructVariable()
14641464
}
14651465
}
14661466
}
1467-
//---------------------------------------------------------------------------
14681467

1469-
void CheckBufferOverrun::bufferOverrun()
1470-
{
1471-
checkGlobalAndLocalVariable();
1472-
if (_tokenizer->isMaxTime())
1473-
return;
1474-
checkStructVariable();
1475-
checkBufferAllocatedWithStrlen();
1476-
checkStringArgument();
1477-
checkInsecureCmdLineArgs();
1478-
}
14791468
//---------------------------------------------------------------------------
14801469

1481-
void CheckBufferOverrun::bufferOverrun2()
1470+
void CheckBufferOverrun::bufferOverrun()
14821471
{
14831472
// singlepass checking using ast, symboldatabase and valueflow
14841473
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {

‎lib/checkbufferoverrun.h‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -61,17 +61,24 @@ class CPPCHECKLIB CheckBufferOverrun : public Check {
6161

6262
void runSimplifiedChecks(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger) {
6363
CheckBufferOverrun checkBufferOverrun(tokenizer, settings, errorLogger);
64+
checkBufferOverrun.checkGlobalAndLocalVariable();
65+
if (_tokenizer->isMaxTime())
66+
return;
67+
checkBufferOverrun.checkStructVariable();
68+
checkBufferOverrun.checkBufferAllocatedWithStrlen();
69+
checkBufferOverrun.checkInsecureCmdLineArgs();
6470
checkBufferOverrun.bufferOverrun();
65-
checkBufferOverrun.bufferOverrun2();
6671
checkBufferOverrun.arrayIndexThenCheck();
6772
checkBufferOverrun.negativeArraySize();
6873
}
6974

70-
/** @brief %Check for buffer overruns */
71-
void bufferOverrun();
75+
void runChecks(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger) {
76+
CheckBufferOverrun checkBufferOverrun(tokenizer, settings, errorLogger);
77+
checkBufferOverrun.checkStringArgument();
78+
}
7279

73-
/** @brief %Check for buffer overruns #2 (single pass, use ast and valueflow) */
74-
void bufferOverrun2();
80+
/** @brief %Check for buffer overruns (single pass, use ast and valueflow) */
81+
void bufferOverrun();
7582

7683
/** @brief Using array index before bounds check */
7784
void arrayIndexThenCheck();

‎lib/checkstring.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ class CPPCHECKLIB CheckString : public Check {
4949
// Checks
5050
checkString.strPlusChar();
5151
checkString.checkSuspiciousStringCompare();
52+
checkString.stringLiteralWrite();
5253
}
5354

5455
/** @brief Run checks against the simplified token list */
@@ -59,7 +60,6 @@ class CPPCHECKLIB CheckString : public Check {
5960
checkString.checkIncorrectStringCompare();
6061
checkString.checkAlwaysTrueOrFalseStringCompare();
6162
checkString.sprintfOverlappingData();
62-
checkString.stringLiteralWrite();
6363
}
6464

6565
/** @brief undefined behaviour, writing string literal */

‎test/testbufferoverrun.cpp‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,16 +54,15 @@ class TestBufferOverrun : public TestFixture {
5454
Tokenizer tokenizer(&settings, this);
5555
std::istringstream istr(code);
5656
tokenizer.tokenize(istr, filename);
57-
tokenizer.simplifyTokenList2();
5857

5958
// Clear the error buffer..
6059
errout.str("");
6160

6261
// Check for buffer overruns..
6362
CheckBufferOverrun checkBufferOverrun(&tokenizer, &settings, this);
64-
checkBufferOverrun.bufferOverrun();
65-
checkBufferOverrun.bufferOverrun2();
66-
checkBufferOverrun.arrayIndexThenCheck();
63+
checkBufferOverrun.runChecks(&tokenizer, &settings, this);
64+
tokenizer.simplifyTokenList2();
65+
checkBufferOverrun.runSimplifiedChecks(&tokenizer, &settings, this);
6766
}
6867

6968
void run() {
@@ -2001,6 +2000,13 @@ class TestBufferOverrun : public TestFixture {
20012000
" if ( name[0] == 'U' ? name[1] : 0) {}\n"
20022001
"}");
20032002
ASSERT_EQUALS("", errout.str());
2003+
2004+
check("int main(int argc, char **argv) {\n"
2005+
" char str[6] = \"\\0\";\n"
2006+
" unsigned short port = 65535;\n"
2007+
" snprintf(str, sizeof(str), \"%hu\", port);\n"
2008+
"}", settings0, "test.c");
2009+
ASSERT_EQUALS("", errout.str());
20042010
}
20052011

20062012
void array_index_same_struct_and_var_name() {

‎test/teststring.cpp‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,24 @@ class TestString : public TestFixture {
8888
" abc[0] = 'a';\n"
8989
"}");
9090
ASSERT_EQUALS("", errout.str());
91+
92+
check("void foo_FP1(char *p) {\n"
93+
" p[1] = 'B';\n"
94+
"}\n"
95+
"void foo_FP2(void) {\n"
96+
" char* s = \"Y\";\n"
97+
" foo_FP1(s);\n"
98+
"}");
99+
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:5]: (error) Modifying string literal \"Y\" directly or indirectly is undefined behaviour.\n", errout.str());
100+
101+
check("void foo_FP1(char *p) {\n"
102+
" p[1] = 'B';\n"
103+
"}\n"
104+
"void foo_FP2(void) {\n"
105+
" char s[10] = \"Y\";\n"
106+
" foo_FP1(s);\n"
107+
"}");
108+
ASSERT_EQUALS("", errout.str());
91109
}
92110

93111
void alwaysTrueFalseStringCompare() {

0 commit comments

Comments
 (0)