Ticket #3876: Improved check by only bailing of loops that contain break or continue

This commit is contained in:
Zachary Blair 2012-06-16 13:03:51 -07:00
parent 2c6c61fb0d
commit fa2bca1e09
2 changed files with 63 additions and 6 deletions

View File

@ -2253,6 +2253,7 @@ void CheckOther::checkDoubleFree()
std::set<unsigned int> closeDirVariables;
for (const Token* tok = _tokenizer->tokens(); tok; tok = tok->next()) {
bool isUnknown = true;
// Keep track of any variables passed to "free()", "g_free()" or "closedir()",
// and report an error if the same variable is passed twice.
@ -2287,15 +2288,19 @@ void CheckOther::checkDoubleFree()
}
// If this scope doesn't return, clear the set of previously freed variables
else if (tok->str() == "}" && _tokenizer->IsScopeNoReturn(tok)) {
else if (tok->str() == "}" && _tokenizer->IsScopeNoReturn(tok, &isUnknown) && !isUnknown) {
freedVariables.clear();
closeDirVariables.clear();
}
// If this scope is a "for" or "while" loop, give up on trying to figure
// out the flow of execution and just clear the set of previously freed variables
else if (tok->str() == "}" && tok->link() && tok->link()->previous() && tok->link()->previous()->link() &&
Token::Match(tok->link()->previous()->link()->previous(), "while|for")) {
// If this scope is a "for" or "while" loop that contains "break" or "continue",
// give up on trying to figure out the flow of execution and just clear the set
// of previously freed variables.
// TODO: There are false negatives. This bailout is only needed when the
// loop will exit without free()'ing the memory on the last iteration.
else if (tok->str() == "}" && tok->link() && tok->link()->linkAt(-1) &&
Token::Match(tok->link()->linkAt(-1)->previous(), "while|for") &&
Token::findmatch(tok->link()->linkAt(-1), "break|continue ;", tok) != NULL) {
freedVariables.clear();
closeDirVariables.clear();
}

View File

@ -4925,7 +4925,7 @@ private:
" char *p = malloc(100);\n"
" if (x) {\n"
" free(p);\n"
" bailout();\n"
" exit();\n"
" }\n"
" free(p);\n"
"}");
@ -5103,6 +5103,45 @@ private:
);
ASSERT_EQUALS("", errout.str());
check(
"void foo(int y)\n"
"{\n"
" char * x = NULL;\n"
" for (int i = 0; i < 10000; i++) {\n"
" x = new char[100];\n"
" delete[] x;\n"
" }\n"
" delete[] x;\n"
"}"
);
ASSERT_EQUALS("[test.cpp:8]: (error) Memory pointed to by 'x' is freed twice.\n", errout.str());
check(
"void foo(int y)\n"
"{\n"
" char * x = NULL;\n"
" while (isRunning()) {\n"
" x = new char[100];\n"
" delete[] x;\n"
" }\n"
" delete[] x;\n"
"}"
);
ASSERT_EQUALS("[test.cpp:8]: (error) Memory pointed to by 'x' is freed twice.\n", errout.str());
check(
"void foo(int y)\n"
"{\n"
" char * x = NULL;\n"
" while (isRunning()) {\n"
" x = malloc(100);\n"
" free(x);\n"
" }\n"
" free(x);\n"
"}"
);
ASSERT_EQUALS("[test.cpp:8]: (error) Memory pointed to by 'x' is freed twice.\n", errout.str());
check(
"void foo(int y)\n"
"{\n"
@ -5132,6 +5171,19 @@ private:
"}"
);
ASSERT_EQUALS("", errout.str());
check(
"void f()\n"
"{\n"
" char *p = 0;\n"
" if (x < 100) {\n"
" p = malloc(10);\n"
" free(p);\n"
" }\n"
" free(p);\n"
"}"
);
ASSERT_EQUALS("[test.cpp:8]: (error) Memory pointed to by 'p' is freed twice.\n", errout.str());
}
};