fixed #7998 (uninitialized variable is not found when used with switch/case)

This commit is contained in:
Daniel Marjamäki 2017-04-23 18:05:14 +02:00
parent 3f069d9e44
commit 8a738eefab
8 changed files with 114 additions and 8 deletions

View File

@ -423,11 +423,17 @@ bool isVariableChanged(const Token *start, const Token *end, const unsigned int
if (Token::Match(tok->previous(), "++|-- %name%"))
return true;
bool inconclusive = false;
bool isChanged = isVariableChangedByFunctionCall(tok, settings, &inconclusive);
isChanged |= inconclusive;
if (isChanged)
return true;
const Token *ftok = tok;
while (ftok && !Token::Match(ftok, "[({[]"))
ftok = ftok->astParent();
if (ftok && Token::Match(ftok->link(), ") !!{")) {
bool inconclusive = false;
bool isChanged = isVariableChangedByFunctionCall(tok, settings, &inconclusive);
isChanged |= inconclusive;
if (isChanged)
return true;
}
const Token *parent = tok->astParent();
while (Token::Match(parent, ".|::"))

View File

@ -71,7 +71,7 @@ void CheckUninitVar::checkScope(const Scope* scope, const std::set<std::string>
if (i->isThrow())
continue;
if (i->nameToken()->strAt(1) == "(" || i->nameToken()->strAt(1) == "{" || i->nameToken()->strAt(1) == ":")
if (Token::Match(i->nameToken()->next(), "[({:]"))
continue;
if (Token::Match(i->nameToken(), "%name% =")) { // Variable is initialized, but Rhs might be not
@ -1034,6 +1034,9 @@ int CheckUninitVar::isFunctionParUsage(const Token *vartok, bool pointer, Alloc
start = start->previous();
}
if (Token::Match(start->link(), ") {"))
return (!pointer || alloc == NO_ALLOC);
// is this a function call?
if (start && Token::Match(start->previous(), "%name% (")) {
const bool address(vartok->previous()->str() == "&");
@ -1195,6 +1198,25 @@ void CheckUninitVar::uninitStructMemberError(const Token *tok, const std::string
"Uninitialized struct member: " + membername, CWE908, false);
}
void CheckUninitVar::valueFlowUninit()
{
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
std::list<Scope>::const_iterator scope;
// check every executable scope
for (scope = symbolDatabase->scopeList.begin(); scope != symbolDatabase->scopeList.end(); ++scope) {
if (!scope->isExecutable())
continue;
for (const Token* tok = scope->classStart; tok != scope->classEnd; tok = tok->next()) {
if (tok->values().size() != 1U || tok->values().front().valueType != ValueFlow::Value::UNINIT)
continue;
if (!isVariableUsage(tok, tok->variable()->isPointer(), NO_ALLOC))
continue;
uninitvarError(tok,tok->str());
}
}
}
void CheckUninitVar::deadPointer()
{
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();

View File

@ -50,6 +50,7 @@ public:
CheckUninitVar checkUninitVar(tokenizer, settings, errorLogger);
checkUninitVar.check();
checkUninitVar.deadPointer();
checkUninitVar.valueFlowUninit();
}
/** Check for uninitialized variables */
@ -70,6 +71,9 @@ public:
void deadPointer();
void deadPointerError(const Token *pointer, const Token *alias);
/** ValueFlow-based checking for uninitialized variables */
void valueFlowUninit();
/* data for multifile checking */
class MyFileInfo : public Check::FileInfo {
public:

View File

@ -1356,6 +1356,9 @@ void Token::printValueFlow(bool xml, std::ostream &out) const
case ValueFlow::Value::MOVED:
out << "movedvalue=\"" << ValueFlow::Value::toString(it->moveKind) << '\"';
break;
case ValueFlow::Value::UNINIT:
out << "uninit=\"1\"";
break;
}
if (it->condition)
out << " condition-line=\"" << it->condition->linenr() << '\"';
@ -1382,6 +1385,9 @@ void Token::printValueFlow(bool xml, std::ostream &out) const
case ValueFlow::Value::MOVED:
out << ValueFlow::Value::toString(it->moveKind);
break;
case ValueFlow::Value::UNINIT:
out << "Uninit";
break;
}
}
}

View File

@ -2836,6 +2836,45 @@ static void valueFlowFunctionReturn(TokenList *tokenlist, ErrorLogger *errorLogg
}
}
static void valueFlowUninit(TokenList *tokenlist, SymbolDatabase * /*symbolDatabase*/, ErrorLogger *errorLogger, const Settings *settings)
{
for (Token *tok = tokenlist->front(); tok; tok = tok->next()) {
if (!Token::Match(tok,"[;{}] %type%"))
continue;
if (!tok->scope()->isExecutable())
continue;
const Token *vardecl = tok->next();
bool stdtype = false;
while (Token::Match(vardecl, "%name%|::|*") && vardecl->varId() == 0) {
stdtype |= vardecl->isStandardType();
vardecl = vardecl->next();
}
if (!Token::Match(vardecl, "%var% ;"))
continue;
if (Token::Match(vardecl, "%varid% ; %varid% =", vardecl->varId()))
continue;
if (!tokenlist->isC() && !stdtype)
continue;
const Variable *var = vardecl->variable();
if (!var || var->nameToken() != vardecl)
continue;
if ((!var->isPointer() && var->type() && var->type()->needInitialization != Type::True) ||
var->isStatic() || var->isExtern() || var->isReference() || var->isThrow())
continue;
ValueFlow::Value uninitValue;
uninitValue.setKnown();
uninitValue.valueType = ValueFlow::Value::UNINIT;
std::list<ValueFlow::Value> values;
values.push_back(uninitValue);
const bool constValue = true;
const bool subFunction = false;
valueFlowForward(vardecl->next(), vardecl->scope()->classEnd, var, vardecl->varId(), values, constValue, subFunction, tokenlist, errorLogger, settings);
}
}
const ValueFlow::Value *ValueFlow::valueFlowConstantFoldAST(const Token *expr, const Settings *settings)
{
if (expr && expr->values().empty()) {
@ -2867,6 +2906,7 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase,
valueFlowForLoop(tokenlist, symboldatabase, errorLogger, settings);
valueFlowSubFunction(tokenlist, errorLogger, settings);
valueFlowFunctionDefaultParameter(tokenlist, symboldatabase, errorLogger, settings);
valueFlowUninit(tokenlist, symboldatabase, errorLogger, settings);
}

View File

@ -57,6 +57,8 @@ namespace ValueFlow {
if (moveKind != rhs.moveKind)
return false;
break;
case UNINIT:
break;
};
return varvalue == rhs.varvalue &&
@ -68,7 +70,7 @@ namespace ValueFlow {
valueKind == rhs.valueKind;
}
enum ValueType { INT, TOK, FLOAT, MOVED } valueType;
enum ValueType { INT, TOK, FLOAT, MOVED, UNINIT } valueType;
bool isIntValue() const {
return valueType == INT;
}
@ -81,6 +83,9 @@ namespace ValueFlow {
bool isMovedValue() const {
return valueType == MOVED;
}
bool isUninitValue() const {
return valueType == UNINIT;
}
/** int value */
long long intvalue;

View File

@ -72,6 +72,7 @@ private:
TEST_CASE(trac_4871);
TEST_CASE(syntax_error); // Ticket #5073
TEST_CASE(trac_5970);
TEST_CASE(valueFlowUninit);
TEST_CASE(isVariableUsageDeref); // *p
@ -2325,7 +2326,6 @@ private:
" ({ if (0); });\n"
" for_each(i) { }\n"
"}", "test.c", false);
ASSERT_EQUALS("", errout.str());
// if, if
checkUninitVar("void f(int a) {\n"
@ -3810,6 +3810,14 @@ private:
check.deadPointer();
}
void valueFlowUninit() {
checkUninitVar("void f() {\n"
" int x;\n"
" switch (x) {}\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) Uninitialized variable: x\n", errout.str());
}
void isVariableUsageDeref() {
// *p
checkUninitVar("void f() {\n"

View File

@ -84,6 +84,8 @@ private:
TEST_CASE(valueFlowGlobalVar);
TEST_CASE(valueFlowInlineAssembly);
TEST_CASE(valueFlowUninit);
}
bool testValueOfX(const char code[], unsigned int linenr, int value) {
@ -2373,6 +2375,19 @@ private:
"}";
ASSERT_EQUALS(false, testValueOfX(code, 5U, 42));
}
void valueFlowUninit() {
std::list<ValueFlow::Value> values;
const char* code = "void f() {\n"
" int x;\n"
" switch (x) {}\n"
"}";
values = tokenValues(code, "x )");
ASSERT_EQUALS(1U, values.size());
ASSERT_EQUALS(ValueFlow::Value::UNINIT, values.empty() ? 0 : values.front().valueType);
}
};
REGISTER_TEST(TestValueFlow)