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

Fixed #5207 (Struct uninitialized members useage is not giving error (malloc).)

This commit is contained in:
Daniel Marjamäki 2013-12-04 20:32:20 +01:00
parent 663f5a14b0
commit 14a00046a3
3 changed files with 126 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

103
lib/checkuninitvar.cpp
View File

@ -1088,9 +1088,11 @@ void CheckUninitVar::checkScope(const Scope* scope)
tok = tok->next(); tok = tok->next();
if (Token::findsimplematch(i->typeStartToken(), "=", tok)) if (Token::findsimplematch(i->typeStartToken(), "=", tok))
continue; continue;
if (stdtype || i->isPointer()) if (stdtype || i->isPointer()) {
checkScopeForVariable(scope, tok, *i, NULL, NULL, ""); bool alloc = false;
if (Token::Match(i->typeStartToken(), "struct %type% %var% ;")) { checkScopeForVariable(scope, tok, *i, NULL, NULL, &alloc, "");
}
if (Token::Match(i->typeStartToken(), "struct %type% *| %var% ;")) {
const std::string structname(i->typeStartToken()->next()->str()); const std::string structname(i->typeStartToken()->next()->str());
const SymbolDatabase * symbolDatabase = _tokenizer->getSymbolDatabase(); const SymbolDatabase * symbolDatabase = _tokenizer->getSymbolDatabase();
for (std::size_t j = 0U; j < symbolDatabase->classAndStructScopes.size(); ++j) { for (std::size_t j = 0U; j < symbolDatabase->classAndStructScopes.size(); ++j) {
@ -1113,8 +1115,10 @@ void CheckUninitVar::checkScope(const Scope* scope)
} }
} }
if (!innerunion) if (!innerunion) {
checkScopeForVariable(scope, tok, *i, NULL, NULL, var.name()); bool alloc = false;
checkScopeForVariable(scope, tok, *i, NULL, NULL, &alloc, var.name());
}
} }
} }
} }
@ -1164,7 +1168,7 @@ static void conditionAlwaysTrueOrFalse(const Token *tok, const std::map<unsigned
} }
} }
bool CheckUninitVar::checkScopeForVariable(const Scope* scope, const Token *tok, const Variable& var, bool * const possibleInit, bool * const noreturn, const std::string &membervar) bool CheckUninitVar::checkScopeForVariable(const Scope* scope, const Token *tok, const Variable& var, bool * const possibleInit, bool * const noreturn, bool * const alloc, const std::string &membervar)
{ {
const bool suppressErrors(possibleInit && *possibleInit); const bool suppressErrors(possibleInit && *possibleInit);
@ -1173,6 +1177,9 @@ bool CheckUninitVar::checkScopeForVariable(const Scope* scope, const Token *tok,
unsigned int number_of_if = 0; unsigned int number_of_if = 0;
if (var.declarationId() == 0U)
return true;
// variable values // variable values
std::map<unsigned int, int> variableValue; std::map<unsigned int, int> variableValue;
static const int NOT_ZERO = (1<<30); // special variable value static const int NOT_ZERO = (1<<30); // special variable value
@ -1195,7 +1202,7 @@ bool CheckUninitVar::checkScopeForVariable(const Scope* scope, const Token *tok,
// Unconditional inner scope.. // Unconditional inner scope..
if (tok->str() == "{" && Token::Match(tok->previous(), "[;{}]")) { if (tok->str() == "{" && Token::Match(tok->previous(), "[;{}]")) {
if (checkScopeForVariable(scope, tok->next(), var, possibleInit, NULL, membervar)) if (checkScopeForVariable(scope, tok->next(), var, possibleInit, NULL, alloc, membervar))
return true; return true;
tok = tok->link(); tok = tok->link();
continue; continue;
@ -1213,7 +1220,7 @@ bool CheckUninitVar::checkScopeForVariable(const Scope* scope, const Token *tok,
conditionAlwaysTrueOrFalse(tok, variableValue, &alwaysTrue, &alwaysFalse); conditionAlwaysTrueOrFalse(tok, variableValue, &alwaysTrue, &alwaysFalse);
// initialization / usage in condition.. // initialization / usage in condition..
if (!alwaysTrue && checkIfForWhileHead(tok->next(), var, suppressErrors, bool(number_of_if == 0), membervar)) if (!alwaysTrue && checkIfForWhileHead(tok->next(), var, suppressErrors, bool(number_of_if == 0), alloc && *alloc, membervar))
return true; return true;
// checking if a not-zero variable is zero => bail out // checking if a not-zero variable is zero => bail out
@ -1236,7 +1243,7 @@ bool CheckUninitVar::checkScopeForVariable(const Scope* scope, const Token *tok,
if (tok->str() == "{") { if (tok->str() == "{") {
bool possibleInitIf(number_of_if > 0 || suppressErrors); bool possibleInitIf(number_of_if > 0 || suppressErrors);
bool noreturnIf = false; bool noreturnIf = false;
const bool initif = !alwaysFalse && checkScopeForVariable(scope, tok->next(), var, &possibleInitIf, &noreturnIf, membervar); const bool initif = !alwaysFalse && checkScopeForVariable(scope, tok->next(), var, &possibleInitIf, &noreturnIf, alloc, membervar);
// bail out for such code: // bail out for such code:
// if (a) x=0; // conditional initialization // if (a) x=0; // conditional initialization
@ -1286,7 +1293,7 @@ bool CheckUninitVar::checkScopeForVariable(const Scope* scope, const Token *tok,
bool possibleInitElse(number_of_if > 0 || suppressErrors); bool possibleInitElse(number_of_if > 0 || suppressErrors);
bool noreturnElse = false; bool noreturnElse = false;
const bool initelse = !alwaysTrue && checkScopeForVariable(scope, tok->next(), var, &possibleInitElse, NULL, membervar); const bool initelse = !alwaysTrue && checkScopeForVariable(scope, tok->next(), var, &possibleInitElse, NULL, alloc, membervar);
std::map<unsigned int, int> varValueElse; std::map<unsigned int, int> varValueElse;
if (!alwaysTrue && !initelse && !noreturnElse) { if (!alwaysTrue && !initelse && !noreturnElse) {
@ -1340,14 +1347,14 @@ bool CheckUninitVar::checkScopeForVariable(const Scope* scope, const Token *tok,
const bool forwhile = Token::Match(tok, "for|while ("); const bool forwhile = Token::Match(tok, "for|while (");
// is variable initialized in for-head (don't report errors yet)? // is variable initialized in for-head (don't report errors yet)?
if (forwhile && checkIfForWhileHead(tok->next(), var, true, false, membervar)) if (forwhile && checkIfForWhileHead(tok->next(), var, true, false, alloc && *alloc, membervar))
return true; return true;
// goto the { // goto the {
const Token *tok2 = forwhile ? tok->next()->link()->next() : tok->next(); const Token *tok2 = forwhile ? tok->next()->link()->next() : tok->next();
if (tok2 && tok2->str() == "{") { if (tok2 && tok2->str() == "{") {
bool init = checkLoopBody(tok2, var, membervar, (number_of_if > 0) | suppressErrors); bool init = checkLoopBody(tok2, var, alloc && *alloc, membervar, (number_of_if > 0) | suppressErrors);
// variable is initialized in the loop.. // variable is initialized in the loop..
if (init) if (init)
@ -1356,7 +1363,7 @@ bool CheckUninitVar::checkScopeForVariable(const Scope* scope, const Token *tok,
// is variable used in for-head? // is variable used in for-head?
if (!suppressErrors) { if (!suppressErrors) {
const Token *startCond = forwhile ? tok->next() : tok->next()->link()->tokAt(2); const Token *startCond = forwhile ? tok->next() : tok->next()->link()->tokAt(2);
checkIfForWhileHead(startCond, var, false, bool(number_of_if == 0), membervar); checkIfForWhileHead(startCond, var, false, bool(number_of_if == 0), alloc && *alloc, membervar);
} }
// goto "}" // goto "}"
@ -1406,8 +1413,12 @@ bool CheckUninitVar::checkScopeForVariable(const Scope* scope, const Token *tok,
} }
// Use variable // Use variable
else if (!suppressErrors && isVariableUsage(tok, var.isPointer(), _tokenizer->isCPP())) else if (!suppressErrors && isVariableUsage(tok, var.isPointer(), alloc && *alloc, _tokenizer->isCPP())) {
uninitvarError(tok, tok->str()); if (alloc && *alloc)
uninitdataError(tok, tok->str());
else
uninitvarError(tok, tok->str());
}
else else
// assume that variable is assigned // assume that variable is assigned
@ -1431,13 +1442,24 @@ bool CheckUninitVar::checkScopeForVariable(const Scope* scope, const Token *tok,
// variable is seen.. // variable is seen..
if (tok->varId() == var.declarationId()) { if (tok->varId() == var.declarationId()) {
// calling function that returns uninit data through pointer..
if (var.isPointer() &&
Token::Match(tok->next(), "= %var% (") &&
Token::simpleMatch(tok->linkAt(3), ") ;") &&
_settings->library.returnuninitdata.count(tok->strAt(2)) > 0U) {
if (alloc)
*alloc = true;
continue;
}
if (!membervar.empty()) { if (!membervar.empty()) {
if (isMemberVariableAssignment(tok, membervar)) { if (isMemberVariableAssignment(tok, membervar)) {
checkRhs(tok, var, membervar); checkRhs(tok, var, alloc && *alloc, membervar);
return true; return true;
} }
if (isMemberVariableUsage(tok, var.isPointer(), membervar)) if (isMemberVariableUsage(tok, var.isPointer(), alloc && *alloc, membervar))
uninitStructMemberError(tok, tok->str() + "." + membervar); uninitStructMemberError(tok, tok->str() + "." + membervar);
else if (Token::Match(tok->previous(), "[(,] %var% [,)]")) else if (Token::Match(tok->previous(), "[(,] %var% [,)]"))
@ -1445,12 +1467,16 @@ bool CheckUninitVar::checkScopeForVariable(const Scope* scope, const Token *tok,
} else { } else {
// Use variable // Use variable
if (!suppressErrors && isVariableUsage(tok, var.isPointer(), _tokenizer->isCPP())) if (!suppressErrors && isVariableUsage(tok, var.isPointer(), alloc && *alloc, _tokenizer->isCPP())) {
uninitvarError(tok, tok->str()); if (alloc && *alloc)
uninitdataError(tok, tok->str());
else
uninitvarError(tok, tok->str());
}
else { else {
if (tok->strAt(1) == "=") if (tok->strAt(1) == "=")
checkRhs(tok, var, ""); checkRhs(tok, var, alloc && *alloc, "");
// assume that variable is assigned // assume that variable is assigned
return true; return true;
@ -1462,7 +1488,7 @@ bool CheckUninitVar::checkScopeForVariable(const Scope* scope, const Token *tok,
return false; return false;
} }
bool CheckUninitVar::checkIfForWhileHead(const Token *startparentheses, const Variable& var, bool suppressErrors, bool isuninit, const std::string &membervar) bool CheckUninitVar::checkIfForWhileHead(const Token *startparentheses, const Variable& var, bool suppressErrors, bool isuninit, bool alloc, const std::string &membervar)
{ {
const Token * const endpar = startparentheses->link(); const Token * const endpar = startparentheses->link();
for (const Token *tok = startparentheses->next(); tok && tok != endpar; tok = tok->next()) { for (const Token *tok = startparentheses->next(); tok && tok != endpar; tok = tok->next()) {
@ -1472,13 +1498,13 @@ bool CheckUninitVar::checkIfForWhileHead(const Token *startparentheses, const Va
if (isMemberVariableAssignment(tok, membervar)) if (isMemberVariableAssignment(tok, membervar))
return true; return true;
if (isMemberVariableUsage(tok, var.isPointer(), membervar)) if (!suppressErrors && isMemberVariableUsage(tok, var.isPointer(), alloc, membervar))
uninitStructMemberError(tok, tok->str() + "." + membervar); uninitStructMemberError(tok, tok->str() + "." + membervar);
} }
continue; continue;
} }
if (isVariableUsage(tok, var.isPointer(), _tokenizer->isCPP())) { if (isVariableUsage(tok, var.isPointer(), alloc, _tokenizer->isCPP())) {
if (!suppressErrors) if (!suppressErrors)
uninitvarError(tok, tok->str()); uninitvarError(tok, tok->str());
else else
@ -1488,13 +1514,13 @@ bool CheckUninitVar::checkIfForWhileHead(const Token *startparentheses, const Va
} }
if (Token::Match(tok, "sizeof|decltype|offsetof (")) if (Token::Match(tok, "sizeof|decltype|offsetof ("))
tok = tok->next()->link(); tok = tok->next()->link();
if (!isuninit && tok->str() == "&&") if ((!isuninit || !membervar.empty()) && tok->str() == "&&")
suppressErrors = true; suppressErrors = true;
} }
return false; return false;
} }
bool CheckUninitVar::checkLoopBody(const Token *tok, const Variable& var, const std::string &membervar, const bool suppressErrors) bool CheckUninitVar::checkLoopBody(const Token *tok, const Variable& var, const bool alloc, const std::string &membervar, const bool suppressErrors)
{ {
const Token *usetok = NULL; const Token *usetok = NULL;
@ -1511,7 +1537,7 @@ bool CheckUninitVar::checkLoopBody(const Token *tok, const Variable& var, const
rhs = true; rhs = true;
if (tok2->str() == ";") if (tok2->str() == ";")
break; break;
if (rhs && tok2->varId() == var.declarationId() && isMemberVariableUsage(tok2, var.isPointer(), membervar)) { if (rhs && tok2->varId() == var.declarationId() && isMemberVariableUsage(tok2, var.isPointer(), alloc, membervar)) {
assign = false; assign = false;
break; break;
} }
@ -1520,12 +1546,15 @@ bool CheckUninitVar::checkLoopBody(const Token *tok, const Variable& var, const
return true; return true;
} }
if (isMemberVariableUsage(tok, var.isPointer(), membervar)) if (Token::Match(tok, "%var% ="))
return true;
if (isMemberVariableUsage(tok, var.isPointer(), alloc, membervar))
usetok = tok; usetok = tok;
else if (Token::Match(tok->previous(), "[(,] %var% [,)]")) else if (Token::Match(tok->previous(), "[(,] %var% [,)]"))
return true; return true;
} else { } else {
if (isVariableUsage(tok, var.isPointer(), _tokenizer->isCPP())) if (isVariableUsage(tok, var.isPointer(), alloc, _tokenizer->isCPP()))
usetok = tok; usetok = tok;
else { else {
bool assign = true; bool assign = true;
@ -1568,7 +1597,7 @@ bool CheckUninitVar::checkLoopBody(const Token *tok, const Variable& var, const
return false; return false;
} }
void CheckUninitVar::checkRhs(const Token *tok, const Variable &var, const std::string &membervar) void CheckUninitVar::checkRhs(const Token *tok, const Variable &var, bool alloc, const std::string &membervar)
{ {
bool rhs = false; bool rhs = false;
unsigned int indent = 0; unsigned int indent = 0;
@ -1576,9 +1605,9 @@ void CheckUninitVar::checkRhs(const Token *tok, const Variable &var, const std::
if (tok->str() == "=") if (tok->str() == "=")
rhs = true; rhs = true;
else if (rhs && tok->varId() == var.declarationId()) { else if (rhs && tok->varId() == var.declarationId()) {
if (membervar.empty() && isVariableUsage(tok, var.isPointer(), _tokenizer->isCPP())) if (membervar.empty() && isVariableUsage(tok, var.isPointer(), alloc, _tokenizer->isCPP()))
uninitvarError(tok, tok->str()); uninitvarError(tok, tok->str());
else if (!membervar.empty() && isMemberVariableUsage(tok, var.isPointer(), membervar)) else if (!membervar.empty() && isMemberVariableUsage(tok, var.isPointer(), alloc, membervar))
uninitStructMemberError(tok, tok->str() + "." + membervar); uninitStructMemberError(tok, tok->str() + "." + membervar);
} else if (tok->str() == ";" || (indent==0 && tok->str() == ",")) } else if (tok->str() == ";" || (indent==0 && tok->str() == ","))
@ -1594,9 +1623,9 @@ void CheckUninitVar::checkRhs(const Token *tok, const Variable &var, const std::
} }
} }
bool CheckUninitVar::isVariableUsage(const Token *vartok, bool pointer, bool cpp) bool CheckUninitVar::isVariableUsage(const Token *vartok, bool pointer, bool alloc, bool cpp)
{ {
if (vartok->previous()->str() == "return") if (vartok->previous()->str() == "return" && !alloc)
return true; return true;
// Passing variable to typeof/__alignof__ // Passing variable to typeof/__alignof__
@ -1707,6 +1736,10 @@ bool CheckUninitVar::isVariableUsage(const Token *vartok, bool pointer, bool cpp
bool unknown = false; bool unknown = false;
if (pointer && CheckNullPointer::isPointerDeRef(vartok, unknown)) { if (pointer && CheckNullPointer::isPointerDeRef(vartok, unknown)) {
// pointer is allocated
if (alloc)
return false;
// function parameter? // function parameter?
bool functionParameter = false; bool functionParameter = false;
if (Token::Match(vartok->tokAt(-2), "%var% (") || vartok->previous()->str() == ",") if (Token::Match(vartok->tokAt(-2), "%var% (") || vartok->previous()->str() == ",")
@ -1760,14 +1793,14 @@ bool CheckUninitVar::isMemberVariableAssignment(const Token *tok, const std::str
return false; return false;
} }
bool CheckUninitVar::isMemberVariableUsage(const Token *tok, bool isPointer, const std::string &membervar) const bool CheckUninitVar::isMemberVariableUsage(const Token *tok, bool isPointer, bool alloc, const std::string &membervar) const
{ {
if (isMemberVariableAssignment(tok, membervar)) if (isMemberVariableAssignment(tok, membervar))
return false; return false;
if (Token::Match(tok, "%var% . %var%") && tok->strAt(2) == membervar) if (Token::Match(tok, "%var% . %var%") && tok->strAt(2) == membervar)
return true; return true;
else if (Token::Match(tok->previous(), "[(,] %var% [,)]") && isVariableUsage(tok, isPointer, _tokenizer->isCPP())) else if (Token::Match(tok->previous(), "[(,] %var% [,)]") && isVariableUsage(tok, isPointer, alloc, _tokenizer->isCPP()))
return true; return true;
return false; return false;

12
lib/checkuninitvar.h
View File

@ -57,13 +57,13 @@ public:
/** Check for uninitialized variables */ /** Check for uninitialized variables */
void check(); void check();
void checkScope(const Scope* scope); void checkScope(const Scope* scope);
bool checkScopeForVariable(const Scope* scope, const Token *tok, const Variable& var, bool * const possibleInit, bool * const noreturn, const std::string &membervar); bool checkScopeForVariable(const Scope* scope, const Token *tok, const Variable& var, bool * const possibleInit, bool * const noreturn, bool * const alloc, const std::string &membervar);
bool checkIfForWhileHead(const Token *startparentheses, const Variable& var, bool suppressErrors, bool isuninit, const std::string &membervar); bool checkIfForWhileHead(const Token *startparentheses, const Variable& var, bool suppressErrors, bool isuninit, bool alloc, const std::string &membervar);
bool checkLoopBody(const Token *tok, const Variable& var, const std::string &membervar, const bool suppressErrors); bool checkLoopBody(const Token *tok, const Variable& var, const bool alloc, const std::string &membervar, const bool suppressErrors);
void checkRhs(const Token *tok, const Variable &var, const std::string &membervar); void checkRhs(const Token *tok, const Variable &var, bool alloc, const std::string &membervar);
static bool isVariableUsage(const Token *vartok, bool ispointer, bool cpp); static bool isVariableUsage(const Token *vartok, bool ispointer, bool alloc, bool cpp);
static bool isMemberVariableAssignment(const Token *tok, const std::string &membervar); static bool isMemberVariableAssignment(const Token *tok, const std::string &membervar);
bool isMemberVariableUsage(const Token *tok, bool isPointer, const std::string &membervar) const; bool isMemberVariableUsage(const Token *tok, bool isPointer, bool alloc, const std::string &membervar) const;
/** /**
* @brief Uninitialized variables: analyse functions to see how they work with uninitialized variables * @brief Uninitialized variables: analyse functions to see how they work with uninitialized variables

52
test/testuninitvar.cpp
View File

@ -64,6 +64,7 @@ private:
TEST_CASE(uninitvar2_structmembers); // struct members TEST_CASE(uninitvar2_structmembers); // struct members
TEST_CASE(uninitvar2_while); TEST_CASE(uninitvar2_while);
TEST_CASE(uninitvar2_4494); // #4494 TEST_CASE(uninitvar2_4494); // #4494
TEST_CASE(uninitvar2_malloc); // malloc returns uninitialized data
TEST_CASE(syntax_error); // Ticket #5073 TEST_CASE(syntax_error); // Ticket #5073
} }
@ -2047,6 +2048,7 @@ private:
// Tokenize.. // Tokenize..
Settings settings; Settings settings;
settings.debugwarnings = debugwarnings; settings.debugwarnings = debugwarnings;
settings.library.returnuninitdata.insert("malloc");
Tokenizer tokenizer(&settings, this); Tokenizer tokenizer(&settings, this);
std::istringstream istr(code); std::istringstream istr(code);
tokenizer.tokenize(istr, fname); tokenizer.tokenize(istr, fname);
@ -2909,6 +2911,35 @@ private:
"}\n"); "}\n");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
checkUninitVar2("struct AB { int a; int b; };\n"
"int f() {\n"
" struct AB *ab;\n"
" for (i = 1; i < 10; i++) {\n"
" if (condition && (ab = getab()) != NULL) {\n"
" a = ab->a;\n"
" }\n"
" }\n"
"}");
ASSERT_EQUALS("", errout.str());
checkUninitVar2("struct AB { int a; int b; };\n"
"int f(int x) {\n"
" struct AB *ab;\n"
" if (x == 0) {\n"
" ab = getab();\n"
" }\n"
" if (x == 0 && (ab != NULL || ab->a == 0)) { }\n"
"}");
ASSERT_EQUALS("", errout.str());
checkUninitVar2("struct A { int *x; };\n" // declarationId is 0 for "delete"
"void foo(void *info, void*p);\n"
"void bar(void) {\n"
" struct A *delete = 0;\n"
" foo( info, NULL );\n"
"}");
ASSERT_EQUALS("", errout.str());
// return // return
checkUninitVar2("struct AB { int a; int b; };\n" checkUninitVar2("struct AB { int a; int b; };\n"
"void f(void) {\n" "void f(void) {\n"
@ -3204,6 +3235,27 @@ private:
ASSERT_EQUALS("[test.cpp:20]: (error) Uninitialized variable: p\n", errout.str()); ASSERT_EQUALS("[test.cpp:20]: (error) Uninitialized variable: p\n", errout.str());
} }
void uninitvar2_malloc() {
checkUninitVar2("int f() {\n"
" int *p = malloc(40);\n"
" return *p;\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) Memory is allocated but not initialized: p\n", errout.str());
checkUninitVar2("int f() {\n"
" int *p = malloc(40);\n"
" var = *p;\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) Memory is allocated but not initialized: p\n", errout.str());
checkUninitVar2("struct AB { int a; int b; };\n"
"int f() {\n"
" struct AB *ab = malloc(sizeof(struct AB));\n"
" return ab->a;\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Uninitialized struct member: ab.a\n", errout.str());
}
void syntax_error() { // Ticket #5073 void syntax_error() { // Ticket #5073
// Nominal mode => No output // Nominal mode => No output
checkUninitVar2("struct flex_array {};\n" checkUninitVar2("struct flex_array {};\n"