Refactorized CheckBufferOverrun::arrayIndexThenCheck() and fixed false negative

This commit is contained in:
PKEuS 2014-05-03 09:30:30 +02:00
parent 59418e605f
commit 04fbbdb5e8
2 changed files with 24 additions and 6 deletions

View File

@ -2101,10 +2101,15 @@ void CheckBufferOverrun::arrayIndexThenCheck()
const Scope * const scope = symbolDatabase->functionScopes[i]; const Scope * const scope = symbolDatabase->functionScopes[i];
for (const Token *tok = scope->classStart; tok && tok != scope->classEnd; tok = tok->next()) { for (const Token *tok = scope->classStart; tok && tok != scope->classEnd; tok = tok->next()) {
if (Token::Match(tok, "%var% [ %var% ]")) { if (Token::Match(tok, "%var% [ %var% ]")) {
const std::string& indexName(tok->strAt(2)); tok = tok->tokAt(2);
unsigned int indexID = tok->varId();
if (!indexID)
continue;
const std::string& indexName(tok->str());
// skip array index.. // skip array index..
tok = tok->tokAt(4); tok = tok->tokAt(2);
while (tok && tok->str() == "[") while (tok && tok->str() == "[")
tok = tok->link()->next(); tok = tok->link()->next();
@ -2117,13 +2122,14 @@ void CheckBufferOverrun::arrayIndexThenCheck()
tok = tok->tokAt(2); tok = tok->tokAt(2);
// skip close parenthesis // skip close parenthesis
if (tok->str() == ")") { if (tok->str() == ")")
tok = tok->next(); tok = tok->next();
}
// check if array index is ok // check if array index is ok
// statement can be closed in parentheses, so "(| " is using // statement can be closed in parentheses, so "(| " is using
if (Token::Match(tok, ("&& (| " + indexName + " <|<=").c_str())) if (Token::Match(tok, "&& (| %varid% <|<=", indexID))
arrayIndexThenCheckError(tok, indexName);
else if (Token::Match(tok, "&& (| %any% >|>= %varid%", indexID))
arrayIndexThenCheckError(tok, indexName); arrayIndexThenCheckError(tok, indexName);
} }
} }

View File

@ -3950,10 +3950,16 @@ private:
" if (s[i] == 'x' && i < y) {\n" " if (s[i] == 'x' && i < y) {\n"
" }" " }"
"}"); "}");
ASSERT_EQUALS("", errout.str()); // No message because i is unknown and thus gets no varid. Avoid an internalError here.
check("void f(const char s[], int i) {\n"
" if (s[i] == 'x' && i < y) {\n"
" }"
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) Array index 'i' is used before limits check.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Array index 'i' is used before limits check.\n", errout.str());
check("void f(const char s[]) {\n" check("void f(const char s[]) {\n"
" for (i = 0; s[i] == 'x' && i < y; ++i) {\n" " for (int i = 0; s[i] == 'x' && i < y; ++i) {\n"
" }" " }"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2]: (style) Array index 'i' is used before limits check.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Array index 'i' is used before limits check.\n", errout.str());
@ -3964,6 +3970,12 @@ private:
"}"); "}");
ASSERT_EQUALS("[test.cpp:2]: (style) Array index 'i' is used before limits check.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Array index 'i' is used before limits check.\n", errout.str());
check("void f(const int a[], unsigned i) {\n"
" if((a[i] < 2) && (42 >= i)) {\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) Array index 'i' is used before limits check.\n", errout.str());
// this one doesn't work for now, hopefully in the future // this one doesn't work for now, hopefully in the future
check("void f(const int a[], unsigned i) {\n" check("void f(const int a[], unsigned i) {\n"
" if(a[i] < func(i) && i <= 42) {\n" " if(a[i] < func(i) && i <= 42) {\n"