Fixed #1492 (false negatives: array index out of bounds)

This commit is contained in:
Robert Reif 2010-03-28 15:56:13 +02:00 committed by Daniel Marjamäki
parent 2dc4222c9a
commit 62d2845014
12 changed files with 395 additions and 10 deletions

View File

@ -31,6 +31,7 @@
#include <list> #include <list>
#include <cstring> #include <cstring>
#include <cctype> #include <cctype>
#include <climits>
#include <cassert> // <- assert #include <cassert> // <- assert
#include <cstdlib> // <- strtoul #include <cstdlib> // <- strtoul
@ -809,6 +810,63 @@ void CheckBufferOverrun::checkGlobalAndLocalVariable()
varid = tok->tokAt(varpos)->varId(); varid = tok->tokAt(varpos)->varId();
nextTok = varpos + 5; nextTok = varpos + 5;
} }
else if (Token::Match(tok, "%type% *| %var% [ %var% ] [;=]"))
{
unsigned int varpos = 1;
if (tok->next()->str() == "*")
++varpos;
// get maximum size from type
// find where this token is defined
const Token *index_type = Token::findmatch(_tokenizer->tokens(), "%varid%", tok->tokAt(varpos + 2)->varId());
index_type = index_type->previous();
if (index_type->str() == "char")
{
if (index_type->isUnsigned())
size = UCHAR_MAX + 1;
else if (index_type->isSigned())
size = SCHAR_MAX + 1;
else
size = CHAR_MAX + 1;
}
else if (index_type->str() == "short")
{
if (index_type->isUnsigned())
size = USHRT_MAX + 1;
else
size = SHRT_MAX + 1;
}
else if (index_type->str() == "int")
{
if (index_type->isUnsigned())
size = UINT_MAX; // really UINT_MAX + 1;
else
size = INT_MAX + 1U;
}
else if (index_type->str() == "long")
{
if (index_type->isUnsigned())
{
if (index_type->isLong())
size = ULONG_MAX; // really ULLONG_MAX + 1;
else
size = ULONG_MAX; // really ULONG_MAX + 1;
}
else
{
if (index_type->isLong())
size = ULONG_MAX; // really LLONG_MAX + 1;
else
size = LONG_MAX + 1U;
}
}
type = tok->strAt(varpos - 1);
varid = tok->tokAt(varpos)->varId();
nextTok = varpos + 5;
}
else if (indentlevel > 0 && Token::Match(tok, "[*;{}] %var% = new %type% [ %num% ]")) else if (indentlevel > 0 && Token::Match(tok, "[*;{}] %var% = new %type% [ %num% ]"))
{ {
size = MathLib::toLongNumber(tok->strAt(6)); size = MathLib::toLongNumber(tok->strAt(6));

View File

@ -1791,8 +1791,6 @@ bool CheckClass::isMemberFunc(const Token *tok)
return true; return true;
else if (Token::Match(tok, "operator %any% (")) else if (Token::Match(tok, "operator %any% ("))
return true; return true;
else if (Token::Match(tok, "%type%"))
tok = tok->next();
while (Token::Match(tok, ":: %type%")) while (Token::Match(tok, ":: %type%"))
tok = tok->tokAt(2); tok = tok->tokAt(2);

View File

@ -315,14 +315,12 @@ void CheckOther::checkUnsignedDivision()
{ {
if (Token::Match(tok, "[{};(,] %type% %var% [;=,)]")) if (Token::Match(tok, "[{};(,] %type% %var% [;=,)]"))
{ {
const std::string type = tok->strAt(1); if (tok->tokAt(1)->isUnsigned())
if (type == "char" || type == "short" || type == "int") varsign[tok->tokAt(2)->varId()] = 'u';
else
varsign[tok->tokAt(2)->varId()] = 's'; varsign[tok->tokAt(2)->varId()] = 's';
} }
else if (Token::Match(tok, "[{};(,] unsigned %type% %var% [;=,)]"))
varsign[tok->tokAt(3)->varId()] = 'u';
else if (!Token::Match(tok, "[).]") && else if (!Token::Match(tok, "[).]") &&
Token::Match(tok->next(), "%var% / %var%") && Token::Match(tok->next(), "%var% / %var%") &&
tok->tokAt(1)->varId() != 0 && tok->tokAt(1)->varId() != 0 &&
@ -906,8 +904,12 @@ void CheckOther::checkCharVariable()
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
{ {
// Declaring the variable.. // Declaring the variable..
if (Token::Match(tok, "[{};(,] signed| char %var% [;=,)]")) if (Token::Match(tok, "[{};(,] char %var% [;=,)]"))
{ {
// Check for unsigned char
if (tok->tokAt(1)->isUnsigned())
continue;
// Set tok to point to the variable name // Set tok to point to the variable name
tok = tok->tokAt(2); tok = tok->tokAt(2);
if (tok->str() == "char") if (tok->str() == "char")

View File

@ -32,6 +32,9 @@ Token::Token(Token **t) :
_isName(false), _isName(false),
_isNumber(false), _isNumber(false),
_isBoolean(false), _isBoolean(false),
_isUnsigned(false),
_isSigned(false),
_isLong(false),
_varId(0), _varId(0),
_next(0), _next(0),
_previous(0), _previous(0),
@ -557,7 +560,7 @@ size_t Token::getStrLength(const Token *tok)
bool Token::isStandardType() const bool Token::isStandardType() const
{ {
bool ret = false; bool ret = false;
const char *type[] = {"bool", "char", "short", "int", "long", "float", "double", "size_t", 0}; const char *type[] = {"bool", "char", "short", "int", "long", "float", "double", "size_t", "__int64", 0};
for (int i = 0; type[i]; i++) for (int i = 0; type[i]; i++)
ret |= (_str == type[i]); ret |= (_str == type[i]);
return ret; return ret;

View File

@ -149,6 +149,30 @@ public:
{ {
return _isBoolean; return _isBoolean;
} }
bool isUnsigned() const
{
return _isUnsigned;
}
void isUnsigned(bool sign)
{
_isUnsigned = sign;
}
bool isSigned() const
{
return _isSigned;
}
void isSigned(bool sign)
{
_isSigned = sign;
}
bool isLong() const
{
return _isLong;
}
void isLong(bool size)
{
_isLong = size;
}
bool isStandardType() const; bool isStandardType() const;
static const Token *findmatch(const Token *tok, const char pattern[], unsigned int varId = 0); static const Token *findmatch(const Token *tok, const char pattern[], unsigned int varId = 0);
@ -341,6 +365,9 @@ private:
bool _isName; bool _isName;
bool _isNumber; bool _isNumber;
bool _isBoolean; bool _isBoolean;
bool _isUnsigned;
bool _isSigned;
bool _isLong;
unsigned int _varId; unsigned int _varId;
Token *_next; Token *_next;
Token *_previous; Token *_previous;

View File

@ -134,6 +134,13 @@ unsigned int Tokenizer::sizeOfType(const Token *type) const
std::map<std::string, unsigned int>::const_iterator it = _typeSize.find(type->strAt(0)); std::map<std::string, unsigned int>::const_iterator it = _typeSize.find(type->strAt(0));
if (it == _typeSize.end()) if (it == _typeSize.end())
return 0; return 0;
else if (type->isLong())
{
if (type->str() == "double")
return sizeof(long double);
else if (type->str() == "long")
return sizeof(long long);
}
return it->second; return it->second;
} }
@ -152,6 +159,9 @@ void Tokenizer::insertTokens(Token *dest, const Token *src, unsigned int n)
dest->fileIndex(src->fileIndex()); dest->fileIndex(src->fileIndex());
dest->linenr(src->linenr()); dest->linenr(src->linenr());
dest->varId(src->varId()); dest->varId(src->varId());
dest->isUnsigned(src->isUnsigned());
dest->isSigned(src->isSigned());
dest->isLong(src->isLong());
src = src->next(); src = src->next();
--n; --n;
} }
@ -1231,6 +1241,10 @@ bool Tokenizer::tokenize(std::istream &code, const char FileName[], const std::s
// replace "unsigned i" with "unsigned int i" // replace "unsigned i" with "unsigned int i"
unsignedint(); unsignedint();
// colapse compound standard types into a single token
// unsigned long long int => long _isUnsigned=true,_isLong=true
simplifyStdType();
// Use "<" comparison instead of ">" // Use "<" comparison instead of ">"
simplifyComparisonOrder(); simplifyComparisonOrder();
@ -4305,6 +4319,59 @@ void Tokenizer::simplifyVarDecl()
} }
} }
void Tokenizer::simplifyStdType()
{
for (Token *tok = _tokens; tok; tok = tok->next())
{
// long unsigned => unsigned long
if (Token::Match(tok, "long|short unsigned|signed"))
{
std::string temp = tok->str();
tok->str(tok->next()->str());
tok->next()->str(temp);
}
if (!Token::Match(tok, "unsigned|signed|long|char|short|int|__int64"))
continue;
// check if signed or unsigned specified
if (Token::Match(tok, "unsigned|signed"))
{
bool isUnsigned = tok->str() == "unsigned";
tok->deleteThis();
tok->isUnsigned(isUnsigned);
tok->isSigned(!isUnsigned);
}
if (Token::Match(tok, "__int64"))
{
tok->str("long");
tok->isLong(true);
}
else if (Token::Match(tok, "long"))
{
if (Token::Match(tok->next(), "long"))
{
tok->isLong(true);
tok->deleteNext();
}
if (Token::Match(tok->next(), "int"))
tok->deleteNext();
else if (Token::Match(tok->next(), "double"))
{
tok->str("double");
tok->isLong(true);
tok->deleteNext();
}
}
else if (Token::Match(tok, "short"))
{
if (Token::Match(tok->next(), "int"))
tok->deleteNext();
}
}
}
void Tokenizer::unsignedint() void Tokenizer::unsignedint()
{ {

View File

@ -170,6 +170,12 @@ public:
*/ */
void unsignedint(); void unsignedint();
/**
* Colapse compound standard types into a single token.
* unsigned long long int => long _isUnsigned=true,_isLong=true
*/
void simplifyStdType();
/** /**
* Simplify question mark - colon operator * Simplify question mark - colon operator
* Example: 0 ? (2/0) : 0 => 0 * Example: 0 ? (2/0) : 0 => 0

View File

@ -93,6 +93,7 @@ private:
TEST_CASE(array_index_21); TEST_CASE(array_index_21);
TEST_CASE(array_index_22); TEST_CASE(array_index_22);
TEST_CASE(array_index_23); TEST_CASE(array_index_23);
TEST_CASE(array_index_24); // ticket #1492
TEST_CASE(array_index_multidim); TEST_CASE(array_index_multidim);
TEST_CASE(array_index_switch_in_for); TEST_CASE(array_index_switch_in_for);
TEST_CASE(array_index_calculation); TEST_CASE(array_index_calculation);
@ -757,6 +758,42 @@ private:
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'c[10]' index 8388608 out of bounds\n", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (error) Array 'c[10]' index 8388608 out of bounds\n", errout.str());
} }
void array_index_24()
{
// ticket #1492
check("void f(signed char n) {\n"
" int a[n];\n" // n <= SCHAR_MAX
" a[-1] = 0;\n" // negative index
" a[128] = 0;\n" // 128 > SCHAR_MAX
"}\n");
ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a[128]' index -1 out of bounds\n"
"[test.cpp:4]: (error) Array 'a[128]' index 128 out of bounds\n", errout.str());
check("void f(unsigned char n) {\n"
" int a[n];\n" // n <= UCHAR
" a[-1] = 0;\n" // negative index
" a[256] = 0;\n" // 256 > UCHAR_MAX
"}\n");
ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a[256]' index -1 out of bounds\n"
"[test.cpp:4]: (error) Array 'a[256]' index 256 out of bounds\n", errout.str());
check("void f(short n) {\n"
" int a[n];\n" // n <= SHRT_MAX
" a[-1] = 0;\n" // negative index
" a[32768] = 0;\n" // 32768 > SHRT_MAX
"}\n");
ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a[32768]' index -1 out of bounds\n"
"[test.cpp:4]: (error) Array 'a[32768]' index 32768 out of bounds\n", errout.str());
check("void f(unsigned short n) {\n"
" int a[n];\n" // n <= USHRT_MAX
" a[-1] = 0;\n" // negative index
" a[65536] = 0;\n" // 65536 > USHRT_MAX
"}\n");
ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a[65536]' index -1 out of bounds\n"
"[test.cpp:4]: (error) Array 'a[65536]' index 65536 out of bounds\n", errout.str());
}
void array_index_multidim() void array_index_multidim()
{ {
check("void f()\n" check("void f()\n"

View File

@ -102,6 +102,7 @@ private:
TEST_CASE(const12); // ticket #1552 TEST_CASE(const12); // ticket #1552
TEST_CASE(const13); // ticket #1519 TEST_CASE(const13); // ticket #1519
TEST_CASE(const14); TEST_CASE(const14);
TEST_CASE(const15);
TEST_CASE(constoperator); // operator< can often be const TEST_CASE(constoperator); // operator< can often be const
TEST_CASE(constincdec); // increment/decrement => non-const TEST_CASE(constincdec); // increment/decrement => non-const
TEST_CASE(constReturnReference); TEST_CASE(constReturnReference);
@ -2671,6 +2672,48 @@ private:
ASSERT_EQUALS("[test.cpp:3]: (style) The function 'A::foo' can be const\n", errout.str()); ASSERT_EQUALS("[test.cpp:3]: (style) The function 'A::foo' can be const\n", errout.str());
} }
void const15()
{
checkConst("class Fred {\n"
" unsigned long long int a;\n"
" unsigned long long int getA() { return a; }\n"
"};\n");
ASSERT_EQUALS("[test.cpp:3]: (style) The function 'Fred::getA' can be const\n", errout.str());
// constructors can't be const..
checkConst("class Fred {\n"
" unsigned long long int a;\n"
"public:\n"
" Fred() { }\n"
"};\n");
ASSERT_EQUALS("", errout.str());
// assignment through |=..
checkConst("class Fred {\n"
" unsigned long long int a;\n"
" unsigned long long int setA() { a |= true; }\n"
"};\n");
ASSERT_EQUALS("", errout.str());
// functions with a function call can't be const..
checkConst("class foo\n"
"{\n"
"public:\n"
" unsigned long long int x;\n"
" void b() { a(); }\n"
"};\n");
ASSERT_EQUALS("", errout.str());
// static functions can't be const..
checkConst("class foo\n"
"{\n"
"public:\n"
" static unsigned long long int get()\n"
" { return 0; }\n"
"};\n");
ASSERT_EQUALS("", errout.str());
}
// increment/decrement => not const // increment/decrement => not const
void constincdec() void constincdec()
{ {

View File

@ -72,6 +72,7 @@ private:
TEST_CASE(sizeof15); TEST_CASE(sizeof15);
TEST_CASE(sizeof16); TEST_CASE(sizeof16);
TEST_CASE(sizeof17); TEST_CASE(sizeof17);
TEST_CASE(sizeof18);
TEST_CASE(casting); TEST_CASE(casting);
TEST_CASE(strlen1); TEST_CASE(strlen1);
@ -249,6 +250,15 @@ private:
{ {
if (tok != tokenizer.tokens()) if (tok != tokenizer.tokens())
ret += " "; ret += " ";
if (!simplify)
{
if (tok->isUnsigned())
ret += "unsigned ";
else if (tok->isSigned())
ret += "signed ";
}
if (tok->isLong())
ret += "long ";
ret += tok->str(); ret += tok->str();
} }
@ -1080,6 +1090,123 @@ private:
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }
void sizeof18()
{
if (sizeof(short int) == 2)
{
{
const char code[] = "void f()\n"
"{\n"
" sizeof(short int);\n"
"}\n";
ASSERT_EQUALS("void f ( ) { 2 ; }", tok(code));
ASSERT_EQUALS("", errout.str());
}
{
const char code[] = "void f()\n"
"{\n"
" sizeof(unsigned short int);\n"
"}\n";
ASSERT_EQUALS("void f ( ) { 2 ; }", tok(code));
ASSERT_EQUALS("", errout.str());
}
{
const char code[] = "void f()\n"
"{\n"
" sizeof(short unsigned int);\n"
"}\n";
ASSERT_EQUALS("void f ( ) { 2 ; }", tok(code));
ASSERT_EQUALS("", errout.str());
}
{
const char code[] = "void f()\n"
"{\n"
" sizeof(signed short int);\n"
"}\n";
ASSERT_EQUALS("void f ( ) { 2 ; }", tok(code));
ASSERT_EQUALS("", errout.str());
}
}
if (sizeof(long long) == 8)
{
{
const char code[] = "void f()\n"
"{\n"
" sizeof(long long);\n"
"}\n";
ASSERT_EQUALS("void f ( ) { 8 ; }", tok(code));
ASSERT_EQUALS("", errout.str());
}
{
const char code[] = "void f()\n"
"{\n"
" sizeof(signed long long);\n"
"}\n";
ASSERT_EQUALS("void f ( ) { 8 ; }", tok(code));
ASSERT_EQUALS("", errout.str());
}
{
const char code[] = "void f()\n"
"{\n"
" sizeof(unsigned long long);\n"
"}\n";
ASSERT_EQUALS("void f ( ) { 8 ; }", tok(code));
ASSERT_EQUALS("", errout.str());
}
{
const char code[] = "void f()\n"
"{\n"
" sizeof(long unsigned long);\n"
"}\n";
ASSERT_EQUALS("void f ( ) { 8 ; }", tok(code));
ASSERT_EQUALS("", errout.str());
}
{
const char code[] = "void f()\n"
"{\n"
" sizeof(long long int);\n"
"}\n";
ASSERT_EQUALS("void f ( ) { 8 ; }", tok(code));
ASSERT_EQUALS("", errout.str());
}
{
const char code[] = "void f()\n"
"{\n"
" sizeof(signed long long int);\n"
"}\n";
ASSERT_EQUALS("void f ( ) { 8 ; }", tok(code));
ASSERT_EQUALS("", errout.str());
}
{
const char code[] = "void f()\n"
"{\n"
" sizeof(unsigned long long int);\n"
"}\n";
ASSERT_EQUALS("void f ( ) { 8 ; }", tok(code));
ASSERT_EQUALS("", errout.str());
}
{
const char code[] = "void f()\n"
"{\n"
" sizeof(long unsigned long int);\n"
"}\n";
ASSERT_EQUALS("void f ( ) { 8 ; }", tok(code));
ASSERT_EQUALS("", errout.str());
}
}
}
void casting() void casting()
{ {
{ {

View File

@ -231,6 +231,15 @@ private:
std::ostringstream ostr; std::ostringstream ostr;
for (const Token *tok = tokenizer.tokens(); tok; tok = tok->next()) for (const Token *tok = tokenizer.tokens(); tok; tok = tok->next())
{ {
if (!simplify)
{
if (tok->isUnsigned())
ostr << "unsigned ";
else if (tok->isSigned())
ostr << "signed ";
}
if (tok->isLong())
ostr << "long ";
ostr << tok->str(); ostr << tok->str();
// Append newlines // Append newlines
@ -2923,7 +2932,15 @@ private:
tokenizer.simplifyFunctionPointers(); tokenizer.simplifyFunctionPointers();
std::ostringstream ostr; std::ostringstream ostr;
for (const Token *tok = tokenizer.tokens(); tok; tok = tok->next()) for (const Token *tok = tokenizer.tokens(); tok; tok = tok->next())
{
if (tok->isUnsigned())
ostr << " unsigned";
else if (tok->isSigned())
ostr << " signed";
if (tok->isLong())
ostr << " long";
ostr << (tok->isName() ? " " : "") << tok->str(); ostr << (tok->isName() ? " " : "") << tok->str();
}
return ostr.str(); return ostr.str();
} }

View File

@ -318,7 +318,7 @@ private:
" func();\n" " func();\n"
" } while(a--);\n" " } while(a--);\n"
"}\n"); "}\n");
ASSERT_EQUALS(std::string(""), errout.str()); ASSERT_EQUALS(std::string("[test.cpp:2]: (style) Unused variable: z\n"), errout.str());
} }
void localvarStruct4() void localvarStruct4()