diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp index a7e978bfa..0ec0184a7 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -24,6 +24,7 @@ #include "checknullpointer.h" // CheckNullPointer::parseFunctionCall #include "symboldatabase.h" #include +#include //--------------------------------------------------------------------------- // Register this check class (by creating a static instance of it) @@ -1100,8 +1101,9 @@ bool CheckUninitVar::checkScopeForVariable(const Scope* scope, const Token *tok, unsigned int number_of_if = 0; - // variables that are known to be non-zero - std::set notzero; + // variable values + std::map variableValue; + static const int NOT_ZERO = (1<<30); // special variable value for (; tok; tok = tok->next()) { // End of scope.. @@ -1126,17 +1128,39 @@ bool CheckUninitVar::checkScopeForVariable(const Scope* scope, const Token *tok, // assignment with nonzero constant.. if (Token::Match(tok->previous(), "[;{}] %var% = - %var% ;") && tok->varId() > 0) - notzero.insert(tok->varId()); + variableValue[tok->varId()] = NOT_ZERO; // Inner scope.. if (Token::simpleMatch(tok, "if (")) { + bool alwaysTrue = false; + + // known variable in condition.. + if (Token::Match(tok, "if ( !| %var% %oror%")) { + const Token *vartok = tok->tokAt(2); + if (vartok->str() == "!") + vartok = vartok->next(); + std::map::const_iterator it = variableValue.find(vartok->varId()); + if (tok->strAt(2) == "!") + alwaysTrue = bool(it != variableValue.end() && it->second == 0); + else + alwaysTrue = bool(it != variableValue.end() && it->second != 0); + } + // initialization / usage in condition.. - if (checkIfForWhileHead(scope, tok->next(), var, suppressErrors, bool(number_of_if == 0))) + if (!alwaysTrue && checkIfForWhileHead(scope, tok->next(), var, suppressErrors, bool(number_of_if == 0))) return true; // checking if a not-zero variable is zero => bail out - if (Token::Match(tok, "if ( %var% )") && notzero.find(tok->tokAt(2)->varId()) != notzero.end()) - return true; // this scope is not fully analysed => return true + unsigned int condVarId = 0, condVarValue = 0; + if (Token::Match(tok, "if ( %var% )")) { + std::map::const_iterator it = variableValue.find(tok->tokAt(2)->varId()); + if (it != variableValue.end() && it->second == NOT_ZERO) + return true; // this scope is not fully analysed => return true + else { + condVarId = tok->tokAt(2)->varId(); + condVarValue = NOT_ZERO; + } + } // goto the { tok = tok->next()->link()->next(); @@ -1146,16 +1170,21 @@ bool CheckUninitVar::checkScopeForVariable(const Scope* scope, const Token *tok, if (tok->str() == "{") { bool possibleInitIf(number_of_if > 0 || suppressErrors); bool noreturnIf = false; - const bool initif = checkScopeForVariable(scope, tok->next(), var, &possibleInitIf, &noreturnIf); + const bool initif = !alwaysTrue && checkScopeForVariable(scope, tok->next(), var, &possibleInitIf, &noreturnIf); - std::set notzeroIf; + std::map varValueIf; if (!initif) { for (const Token *tok2 = tok; tok2 && tok2 != tok->link(); tok2 = tok2->next()) { if (Token::Match(tok2, "[;{}] %var% = - %var% ;")) - notzeroIf.insert(tok2->next()->varId()); + varValueIf[tok2->next()->varId()] = NOT_ZERO; + if (Token::Match(tok2, "[;{}] %var% = %num% ;")) + varValueIf[tok2->next()->varId()] = (int)MathLib::toLongNumber(tok2->strAt(3)); } } + if (initif && condVarId > 0U) + variableValue[condVarId] = condVarValue ^ NOT_ZERO; + // goto the } tok = tok->link(); @@ -1173,14 +1202,20 @@ bool CheckUninitVar::checkScopeForVariable(const Scope* scope, const Token *tok, bool noreturnElse = false; const bool initelse = checkScopeForVariable(scope, tok->next(), var, &possibleInitElse, NULL); - std::set notzeroElse; + std::map varValueElse; if (!initelse) { for (const Token *tok2 = tok; tok2 && tok2 != tok->link(); tok2 = tok2->next()) { if (Token::Match(tok2, "[;{}] %var% = - %var% ;")) - notzeroElse.insert(tok2->next()->varId()); + varValueElse[tok2->next()->varId()] = NOT_ZERO; + if (Token::Match(tok2, "[;{}] %var% = %num% ;")) + varValueElse[tok2->next()->varId()] = (int)MathLib::toLongNumber(tok2->strAt(3)); } } + + if (initelse && condVarId > 0U) + variableValue[condVarId] = condVarValue; + // goto the } tok = tok->link(); @@ -1192,8 +1227,8 @@ bool CheckUninitVar::checkScopeForVariable(const Scope* scope, const Token *tok, if (initif || initelse || possibleInitElse) { ++number_of_if; - notzero.insert(notzeroIf.begin(), notzeroIf.end()); - notzero.insert(notzeroElse.begin(), notzeroElse.end()); + variableValue.insert(varValueIf.begin(), varValueIf.end()); + variableValue.insert(varValueElse.begin(), varValueElse.end()); } } } diff --git a/test/testuninitvar.cpp b/test/testuninitvar.cpp index 52f0e18d0..a51a25cdb 100644 --- a/test/testuninitvar.cpp +++ b/test/testuninitvar.cpp @@ -58,6 +58,7 @@ private: TEST_CASE(uninitvar5); // #3861 TEST_CASE(uninitvar6); // handling unknown types in C and C++ files TEST_CASE(uninitvar2_func); // function calls + TEST_CASE(uninitvar2_value); // value flow } void checkUninitVar(const char code[], const char filename[] = "test.cpp") { @@ -2132,36 +2133,6 @@ private: "}\n"); ASSERT_EQUALS("[test.cpp:5]: (error) Uninitialized variable: x\n", errout.str()); - // if goto is simplified there might be conditions that are always true - checkUninitVar2("void f() {\n" - " int i;\n" - " if (x) {\n" - " int y = -ENOMEM;\n" - " if (y != 0) return;\n" - " i++;\n" - " }\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - checkUninitVar2("void f() {\n" - " int i, y;\n" - " if (x) {\n" - " y = -ENOMEM;\n" - " if (y != 0) return;\n" - " i++;\n" - " }\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - checkUninitVar2("void f() {\n" - " int i, y;\n" - " if (x) y = -ENOMEM;\n" - " else y = get_value(i);\n" - " if (y != 0) return;\n" // <- condition is always true if i is uninitialized - " i++;\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - // for, while checkUninitVar2("void f() {\n" " int x;\n" @@ -2470,6 +2441,66 @@ private: ASSERT_EQUALS("", errout.str()); } + void uninitvar2_value() { + checkUninitVar2("void f() {\n" + " int i;\n" + " if (x) {\n" + " int y = -ENOMEM;\n" // assume constant ENOMEM is nonzero since it's negated + " if (y != 0) return;\n" + " i++;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + checkUninitVar2("void f() {\n" + " int i, y;\n" + " if (x) {\n" + " y = -ENOMEM;\n" // assume constant ENOMEM is nonzero since it's negated + " if (y != 0) return;\n" + " i++;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + checkUninitVar2("void f() {\n" + " int i, y;\n" + " if (x) y = -ENOMEM;\n" // assume constant ENOMEM is nonzero since it's negated + " else y = get_value(i);\n" + " if (y != 0) return;\n" // <- condition is always true if i is uninitialized + " i++;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + checkUninitVar2("void f(int x) {\n" + " int i;\n" + " if (!x) i = 0;\n" + " if (!x || i>0) {}\n" // <- error + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (error) Uninitialized variable: i\n", errout.str()); + + checkUninitVar2("void f(int x) {\n" + " int i;\n" + " if (x) i = 0;\n" + " if (!x || i>0) {}\n" // <- no error + "}\n"); + ASSERT_EQUALS("", errout.str()); + + checkUninitVar2("void f(int x) {\n" + " int i;\n" + " if (!x) { }\n" + " else i = 0;\n" + " if (x || i>0) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:5]: (error) Uninitialized variable: i\n", errout.str()); + + checkUninitVar2("void f(int x) {\n" + " int i;\n" + " if (x) { }\n" + " else i = 0;\n" + " if (x || i>0) {}\n" // <- no error + "}\n"); + ASSERT_EQUALS("", errout.str()); + } }; REGISTER_TEST(TestUninitVar)