From 8a738eefabd45a2c5f110ced25f1bebf0e744700 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sun, 23 Apr 2017 18:05:14 +0200 Subject: [PATCH] fixed #7998 (uninitialized variable is not found when used with switch/case) --- lib/astutils.cpp | 16 +++++++++++----- lib/checkuninitvar.cpp | 24 +++++++++++++++++++++++- lib/checkuninitvar.h | 4 ++++ lib/token.cpp | 6 ++++++ lib/valueflow.cpp | 40 ++++++++++++++++++++++++++++++++++++++++ lib/valueflow.h | 7 ++++++- test/testuninitvar.cpp | 10 +++++++++- test/testvalueflow.cpp | 15 +++++++++++++++ 8 files changed, 114 insertions(+), 8 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 08b6d3dc8..e2ee56dda 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -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, ".|::")) diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp index 3fd2a3318..cb51cbbce 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -71,7 +71,7 @@ void CheckUninitVar::checkScope(const Scope* scope, const std::set 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::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(); diff --git a/lib/checkuninitvar.h b/lib/checkuninitvar.h index 9701d0bae..e22917eeb 100644 --- a/lib/checkuninitvar.h +++ b/lib/checkuninitvar.h @@ -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: diff --git a/lib/token.cpp b/lib/token.cpp index 21844a757..c05021c61 100644 --- a/lib/token.cpp +++ b/lib/token.cpp @@ -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; } } } diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 77697c898..bd9919aa0 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -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 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); } diff --git a/lib/valueflow.h b/lib/valueflow.h index d8516b56d..90163803f 100644 --- a/lib/valueflow.h +++ b/lib/valueflow.h @@ -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; diff --git a/test/testuninitvar.cpp b/test/testuninitvar.cpp index 44fc18394..c25971e19 100644 --- a/test/testuninitvar.cpp +++ b/test/testuninitvar.cpp @@ -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" diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index f9e2f65db..82ca33781 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -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 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)