walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Robert Reif: improve check: array index out of bounds, show name of array, array size and array index

This commit is contained in:
Daniel Marjamäki 2009-12-25 15:25:58 +01:00
parent 1a25e40180
commit 5925b88b38
3 changed files with 89 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

66
lib/checkbufferoverrun.cpp
View File

@ -45,19 +45,19 @@ CheckBufferOverrun instance;
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------
void CheckBufferOverrun::arrayIndexOutOfBounds(const Token *tok, int size) void CheckBufferOverrun::arrayIndexOutOfBounds(const Token *tok, int size, int index)
{ {
if (!tok) if (!tok)
arrayIndexOutOfBounds(size); arrayIndexOutOfBounds(size, index);
else else
{ {
_callStack.push_back(tok); _callStack.push_back(tok);
arrayIndexOutOfBounds(size); arrayIndexOutOfBounds(size, index);
_callStack.pop_back(); _callStack.pop_back();
} }
} }
void CheckBufferOverrun::arrayIndexOutOfBounds(int size) void CheckBufferOverrun::arrayIndexOutOfBounds(int size, int index)
{ {
Severity::e severity; Severity::e severity;
if (size <= 1 || _callStack.size() > 1) if (size <= 1 || _callStack.size() > 1)
@ -71,7 +71,14 @@ void CheckBufferOverrun::arrayIndexOutOfBounds(int size)
severity = Severity::error; severity = Severity::error;
} }
reportError(_callStack, severity, "arrayIndexOutOfBounds", "Array index out of bounds"); if (_callStack.size() == 1)
{
std::ostringstream oss;
oss << "Array '" << (*_callStack.begin())->str() << "[" << size << "]' index " << index << " out of bounds";
reportError(_callStack, severity, "arrayIndexOutOfBounds", oss.str().c_str());
}
else
reportError(_callStack, severity, "arrayIndexOutOfBounds", "Array index out of bounds");
} }
void CheckBufferOverrun::bufferOverrun(const Token *tok) void CheckBufferOverrun::bufferOverrun(const Token *tok)
@ -159,19 +166,19 @@ void CheckBufferOverrun::checkScope(const Token *tok, const char *varname[], con
{ {
if (Token::Match(tok, "%varid% [ %num% ]", varid)) if (Token::Match(tok, "%varid% [ %num% ]", varid))
{ {
const char *num = tok->strAt(2); int index = std::strtol(tok->strAt(2), NULL, 10);
if (std::strtol(num, NULL, 10) >= size) if (index >= size)
{ {
arrayIndexOutOfBounds(tok->next(), size); arrayIndexOutOfBounds(tok, size, index);
} }
} }
} }
else if (Token::Match(tok, std::string(varnames + " [ %num% ]").c_str())) else if (Token::Match(tok, std::string(varnames + " [ %num% ]").c_str()))
{ {
const char *num = tok->strAt(2 + varc); int index = std::strtol(tok->strAt(2 + varc), NULL, 10);
if (std::strtol(num, NULL, 10) >= size) if (index >= size)
{ {
arrayIndexOutOfBounds(tok->next(), size); arrayIndexOutOfBounds(tok->tokAt(varc), size, index);
} }
} }
@ -195,22 +202,22 @@ void CheckBufferOverrun::checkScope(const Token *tok, const char *varname[], con
{ {
if (!tok->isName() && !Token::Match(tok, "[.&]") && Token::Match(tok->next(), "%varid% [ %num% ]", varid)) if (!tok->isName() && !Token::Match(tok, "[.&]") && Token::Match(tok->next(), "%varid% [ %num% ]", varid))
{ {
const char *num = tok->strAt(3); int index = std::strtol(tok->strAt(3), NULL, 10);
if (std::strtol(num, NULL, 10) >= size) if (index >= size)
{ {
if (std::strtol(num, NULL, 10) > size || !Token::Match(tok->previous(), "& (")) if (index > size || !Token::Match(tok->previous(), "& ("))
{ {
arrayIndexOutOfBounds(tok->next(), size); arrayIndexOutOfBounds(tok->next(), size, index);
} }
} }
} }
} }
else if (!tok->isName() && !Token::Match(tok, "[.&]") && Token::Match(tok->next(), std::string(varnames + " [ %num% ]").c_str())) else if (!tok->isName() && !Token::Match(tok, "[.&]") && Token::Match(tok->next(), std::string(varnames + " [ %num% ]").c_str()))
{ {
const char *num = tok->next()->strAt(2 + varc); int index = std::strtol(tok->strAt(3 + varc), NULL, 10);
if (std::strtol(num, NULL, 10) >= size) if (index >= size)
{ {
arrayIndexOutOfBounds(tok->next(), size); arrayIndexOutOfBounds(tok->tokAt(1 + varc), size, index);
} }
tok = tok->tokAt(4); tok = tok->tokAt(4);
continue; continue;
@ -480,7 +487,7 @@ void CheckBufferOverrun::checkScope(const Token *tok, const char *varname[], con
//printf("min_index = %d, max_index = %d, size = %d\n", min_index, max_index, size); //printf("min_index = %d, max_index = %d, size = %d\n", min_index, max_index, size);
if (min_index >= size || max_index >= size) if (min_index >= size || max_index >= size)
{ {
arrayIndexOutOfBounds(tok2->next(), size); arrayIndexOutOfBounds(tok2, size, min_index > max_index ? min_index : max_index);
} }
} }
@ -587,15 +594,12 @@ void CheckBufferOverrun::checkScope(const Token *tok, const char *varname[], con
// sent as the parameter, that is checked separately anyway. // sent as the parameter, that is checked separately anyway.
if (Token::Match(tok, "%var% (")) if (Token::Match(tok, "%var% ("))
{ {
// Don't make recursive checking..
if (std::find(_callStack.begin(), _callStack.end(), tok) != _callStack.end())
continue;
// Only perform this checking if showAll setting is enabled.. // Only perform this checking if showAll setting is enabled..
if (!_settings->_showAll) if (!_settings->_showAll)
continue; continue;
unsigned int parlevel = 0, par = 0; unsigned int parlevel = 0, par = 0;
const Token * tok1 = tok;
for (const Token *tok2 = tok; tok2; tok2 = tok2->next()) for (const Token *tok2 = tok; tok2; tok2 = tok2->next())
{ {
if (tok2->str() == "(") if (tok2->str() == "(")
@ -620,10 +624,16 @@ void CheckBufferOverrun::checkScope(const Token *tok, const char *varname[], con
if (parlevel == 1) if (parlevel == 1)
{ {
if ((varid > 0 && Token::Match(tok2, std::string("[(,] %varid% [,)]").c_str(), varid)) || if (varid > 0 && Token::Match(tok2, std::string("[(,] %varid% [,)]").c_str(), varid))
(varid == 0 && Token::Match(tok2, std::string("[(,] " + varnames + " [,)]").c_str())))
{ {
++par; ++par;
tok1 = tok2->next();
break;
}
else if (varid == 0 && Token::Match(tok2, std::string("[(,] " + varnames + " [,)]").c_str()))
{
++par;
tok1 = tok2->tokAt(varc + 1);
break; break;
} }
} }
@ -663,8 +673,12 @@ void CheckBufferOverrun::checkScope(const Token *tok, const char *varname[], con
ftok = ftok->next(); ftok = ftok->next();
ftok = ftok ? ftok->next() : 0; ftok = ftok ? ftok->next() : 0;
// Don't make recursive checking..
if (std::find(_callStack.begin(), _callStack.end(), tok1) != _callStack.end())
continue;
// Check variable usage in the function.. // Check variable usage in the function..
_callStack.push_back(tok); _callStack.push_back(tok1);
checkScope(ftok, parname, size, total_size, 0); checkScope(ftok, parname, size, total_size, 0);
_callStack.pop_back(); _callStack.pop_back();

6
lib/checkbufferoverrun.h
View File

@ -79,8 +79,8 @@ private:
/** callstack - used during intra-function checking */ /** callstack - used during intra-function checking */
std::list<const Token *> _callStack; std::list<const Token *> _callStack;
void arrayIndexOutOfBounds(const Token *tok, int size); void arrayIndexOutOfBounds(const Token *tok, int size, int index);
void arrayIndexOutOfBounds(int size); void arrayIndexOutOfBounds(int size, int index);
void bufferOverrun(const Token *tok); void bufferOverrun(const Token *tok);
void dangerousStdCin(const Token *tok); void dangerousStdCin(const Token *tok);
void strncatUsage(const Token *tok); void strncatUsage(const Token *tok);
@ -90,7 +90,7 @@ private:
void getErrorMessages() void getErrorMessages()
{ {
arrayIndexOutOfBounds(0, 2); arrayIndexOutOfBounds(0, 2, 2);
bufferOverrun(0); bufferOverrun(0);
dangerousStdCin(0); dangerousStdCin(0);
strncatUsage(0); strncatUsage(0);

68
test/testbufferoverrun.cpp
View File

@ -236,7 +236,7 @@ private:
" int data[2];\n" " int data[2];\n"
" data[ sizeof(data[0]) ] = 0;\n" " data[ sizeof(data[0]) ] = 0;\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:4]: (error) Array index out of bounds\n", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (error) Array 'data[2]' index 4 out of bounds\n", errout.str());
} }
void sizeof3() void sizeof3()
@ -260,7 +260,7 @@ private:
" str[15] = 0;\n" " str[15] = 0;\n"
" str[16] = 0;\n" " str[16] = 0;\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:5]: (error) Array index out of bounds\n", errout.str()); ASSERT_EQUALS("[test.cpp:5]: (error) Array 'str[16]' index 16 out of bounds\n", errout.str());
} }
@ -272,7 +272,7 @@ private:
" str[15] = 0;\n" " str[15] = 0;\n"
" str[16] = 0;\n" " str[16] = 0;\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:5]: (error) Array index out of bounds\n", errout.str()); ASSERT_EQUALS("[test.cpp:5]: (error) Array 'str[16]' index 16 out of bounds\n", errout.str());
} }
@ -334,7 +334,7 @@ private:
" int i[SIZE];\n" " int i[SIZE];\n"
" i[SIZE] = 0;\n" " i[SIZE] = 0;\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:5]: (error) Array index out of bounds\n", errout.str()); ASSERT_EQUALS("[test.cpp:5]: (error) Array 'i[10]' index 10 out of bounds\n", errout.str());
} }
@ -345,7 +345,7 @@ private:
" int i[10];\n" " int i[10];\n"
" i[ sizeof(i) - 1 ] = 0;\n" " i[ sizeof(i) - 1 ] = 0;\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:4]: (error) Array index out of bounds\n", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (error) Array 'i[10]' index 39 out of bounds\n", errout.str());
} }
@ -361,7 +361,7 @@ private:
" struct ABC abc;\n" " struct ABC abc;\n"
" abc.str[10] = 0;\n" " abc.str[10] = 0;\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:9]: (error) Array index out of bounds\n", errout.str()); ASSERT_EQUALS("[test.cpp:9]: (error) Array 'str[10]' index 10 out of bounds\n", errout.str());
// This is not out of bounds // This is not out of bounds
check("struct ABC\n" check("struct ABC\n"
@ -375,7 +375,7 @@ private:
" struct ABC* x = (struct ABC *)malloc(sizeof(struct ABC) + datasize - 1);\n" " struct ABC* x = (struct ABC *)malloc(sizeof(struct ABC) + datasize - 1);\n"
" x->str[1] = 0;" " x->str[1] = 0;"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:10]: (possible error) Array index out of bounds\n", errout.str()); ASSERT_EQUALS("[test.cpp:10]: (possible error) Array 'str[1]' index 1 out of bounds\n", errout.str());
TODO_ASSERT_EQUALS("", errout.str()); TODO_ASSERT_EQUALS("", errout.str());
} }
@ -391,7 +391,7 @@ private:
"{\n" "{\n"
" abc->str[10] = 0;\n" " abc->str[10] = 0;\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:8]: (error) Array index out of bounds\n", errout.str()); ASSERT_EQUALS("[test.cpp:8]: (error) Array 'str[10]' index 10 out of bounds\n", errout.str());
} }
@ -409,7 +409,7 @@ private:
" struct ABC abc;\n" " struct ABC abc;\n"
" abc.str[SIZE] = 0;\n" " abc.str[SIZE] = 0;\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:11]: (error) Array index out of bounds\n", errout.str()); ASSERT_EQUALS("[test.cpp:11]: (error) Array 'str[10]' index 10 out of bounds\n", errout.str());
} }
void array_index_9() void array_index_9()
@ -426,6 +426,30 @@ private:
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:9] -> [test.cpp:3]: (possible error) Array index out of bounds\n", errout.str()); ASSERT_EQUALS("[test.cpp:9] -> [test.cpp:3]: (possible error) Array index out of bounds\n", errout.str());
check("static void memclr( int i, char *data )\n"
"{\n"
" data[10] = 0;\n"
"}\n"
"\n"
"static void f()\n"
"{\n"
" char str[5];\n"
" memclr( 0, str ); // ERROR\n"
"}\n");
ASSERT_EQUALS("[test.cpp:9] -> [test.cpp:3]: (possible error) Array index out of bounds\n", errout.str());
check("static void memclr( int i, char *data )\n"
"{\n"
" data[i] = 0;\n"
"}\n"
"\n"
"static void f()\n"
"{\n"
" char str[5];\n"
" memclr( 10, str ); // ERROR\n"
"}\n");
TODO_ASSERT_EQUALS("[test.cpp:9] -> [test.cpp:3]: (possible error) Array index out of bounds\n", errout.str());
// This is not an error // This is not an error
check("static void memclr( char *data, int size )\n" check("static void memclr( char *data, int size )\n"
"{\n" "{\n"
@ -481,7 +505,7 @@ private:
" abc->str[10] = 0;\n" " abc->str[10] = 0;\n"
" }\n" " }\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:13]: (error) Array index out of bounds\n", errout.str()); ASSERT_EQUALS("[test.cpp:13]: (error) Array 'str[10]' index 10 out of bounds\n", errout.str());
} }
@ -498,7 +522,7 @@ private:
"{\n" "{\n"
" str[10] = 0;\n" " str[10] = 0;\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:10]: (error) Array index out of bounds\n", errout.str()); ASSERT_EQUALS("[test.cpp:10]: (error) Array 'str[10]' index 10 out of bounds\n", errout.str());
} }
void array_index_13() void array_index_13()
@ -523,7 +547,7 @@ private:
" for (int i = 0; i < 10; i++)\n" " for (int i = 0; i < 10; i++)\n"
" a[i+10] = i;\n" " a[i+10] = i;\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:5]: (error) Array index out of bounds\n", errout.str()); ASSERT_EQUALS("[test.cpp:5]: (error) Array 'a[10]' index 19 out of bounds\n", errout.str());
} }
void array_index_15() void array_index_15()
@ -534,7 +558,7 @@ private:
" for (int i = 0; i < 10; i++)\n" " for (int i = 0; i < 10; i++)\n"
" a[10+i] = i;\n" " a[10+i] = i;\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:5]: (error) Array index out of bounds\n", errout.str()); ASSERT_EQUALS("[test.cpp:5]: (error) Array 'a[10]' index 19 out of bounds\n", errout.str());
} }
void array_index_16() void array_index_16()
@ -545,7 +569,7 @@ private:
" for (int i = 0; i < 10; i++)\n" " for (int i = 0; i < 10; i++)\n"
" a[i+1] = i;\n" " a[i+1] = i;\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:5]: (error) Array index out of bounds\n", errout.str()); ASSERT_EQUALS("[test.cpp:5]: (error) Array 'a[10]' index 10 out of bounds\n", errout.str());
} }
void array_index_17() void array_index_17()
@ -556,7 +580,7 @@ private:
" for (int i = 0; i < 10; i++)\n" " for (int i = 0; i < 10; i++)\n"
" a[i*2] = i;\n" " a[i*2] = i;\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:5]: (error) Array index out of bounds\n", errout.str()); ASSERT_EQUALS("[test.cpp:5]: (error) Array 'a[10]' index 18 out of bounds\n", errout.str());
check("void f()\n" check("void f()\n"
"{\n" "{\n"
@ -572,7 +596,7 @@ private:
" for (int i = 0; i < 12; i+=6)\n" " for (int i = 0; i < 12; i+=6)\n"
" a[i+6] = i;\n" " a[i+6] = i;\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:5]: (error) Array index out of bounds\n", errout.str()); ASSERT_EQUALS("[test.cpp:5]: (error) Array 'a[12]' index 12 out of bounds\n", errout.str());
} }
void array_index_18() void array_index_18()
@ -649,7 +673,7 @@ private:
" char a[2];\n" " char a[2];\n"
" char *end = &(a[3]);\n" " char *end = &(a[3]);\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:4]: (error) Array index out of bounds\n", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[2]' index 3 out of bounds\n", errout.str());
} }
void array_index_20() void array_index_20()
@ -686,7 +710,7 @@ private:
" size_t indices[2];\n" " size_t indices[2];\n"
" int b = indices[2];\n" " int b = indices[2];\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:4]: (error) Array index out of bounds\n", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (error) Array 'indices[2]' index 2 out of bounds\n", errout.str());
} }
void array_index_23() void array_index_23()
@ -697,7 +721,7 @@ private:
" tab4[20] = 0;\n" " tab4[20] = 0;\n"
" free(tab4);\n" " free(tab4);\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:3]: (error) Array index out of bounds\n", errout.str()); ASSERT_EQUALS("[test.cpp:3]: (error) Array 'tab4[20]' index 20 out of bounds\n", errout.str());
} }
void array_index_multidim() void array_index_multidim()
@ -1271,7 +1295,7 @@ private:
"{\n" "{\n"
" str[3] = 0;\n" " str[3] = 0;\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:5]: (error) Array index out of bounds\n", errout.str()); ASSERT_EQUALS("[test.cpp:5]: (error) Array 'str[3]' index 3 out of bounds\n", errout.str());
} }
void alloc() void alloc()
@ -1281,14 +1305,14 @@ private:
" char *s = new char[10];\n" " char *s = new char[10];\n"
" s[10] = 0;\n" " s[10] = 0;\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:4]: (error) Array index out of bounds\n", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (error) Array 's[10]' index 10 out of bounds\n", errout.str());
check("void foo()\n" check("void foo()\n"
"{\n" "{\n"
" char *s = (char *)malloc(10);\n" " char *s = (char *)malloc(10);\n"
" s[10] = 0;\n" " s[10] = 0;\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:4]: (error) Array index out of bounds\n", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (error) Array 's[10]' index 10 out of bounds\n", errout.str());
} }