diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp index 72748ca35..6b836df3e 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -25,6 +25,7 @@ #include "symboldatabase.h" #include #include +#include //--------------------------------------------------------------------------- // Register this check class (by creating a static instance of it) @@ -1288,8 +1289,8 @@ bool CheckUninitVar::checkScopeForVariable(const Scope* scope, const Token *tok, const Token *tok2 = tok->next()->link()->next(); if (tok2 && tok2->str() == "{") { - bool possibleinit = true; - bool init = checkScopeForVariable(scope, tok2->next(), var, &possibleinit, NULL, membervar); + bool possibleinit = false; + bool init = checkLoopBody(scope, tok2, var, membervar); // variable is initialized in the loop.. if (possibleinit || init) @@ -1350,21 +1351,12 @@ bool CheckUninitVar::checkScopeForVariable(const Scope* scope, const Token *tok, // variable is seen.. if (tok->varId() == var.varId()) { if (!membervar.empty()) { - if (Token::Match(tok, "%var% . %var%") && tok->strAt(2) == membervar) { - if (Token::Match(tok->tokAt(3), "[=.[]")) - return true; - else if (Token::Match(tok->tokAt(-2), "[(,=] &")) - return true; - else if (Token::Match(tok->previous(), "%op%") || Token::Match(tok->previous(), "[|=")) - uninitStructMemberError(tok, tok->str() + "." + membervar); - else - return true; - } else if (tok->strAt(1) == "=") + if (isMemberVariableAssignment(tok, membervar)) return true; - else if (tok->strAt(-1) == "&") - return true; - else if (Token::Match(tok->previous(), "[(,] %var% [,)]") && isVariableUsage(scope, tok, var.isPointer())) + + if (isMemberVariableUsage(scope, tok, var.isPointer(), membervar)) uninitStructMemberError(tok, tok->str() + "." + membervar); + } else { // Use variable if (!suppressErrors && isVariableUsage(scope, tok, var.isPointer())) @@ -1407,6 +1399,40 @@ bool CheckUninitVar::checkIfForWhileHead(const Scope *scope, const Token *startp return false; } +bool CheckUninitVar::checkLoopBody(const Scope* scope, const Token *tok, const Variable& var, const std::string &membervar) +{ + const Token *usetok = NULL; + + assert(tok->str() == "{"); + + for (const Token * const end = tok->link(); tok != end; tok = tok->next()) { + if (tok->varId() == var.varId()) { + if (!membervar.empty()) { + if (isMemberVariableAssignment(tok, membervar)) + return true; + + if (isMemberVariableUsage(scope, tok, var.isPointer(), membervar)) + usetok = tok; + } else { + if (isVariableUsage(scope, tok, var.isPointer())) + usetok = tok; + else + return true; + } + } + } + + if (usetok) { + if (membervar.empty()) + uninitvarError(usetok, usetok->str()); + else + uninitStructMemberError(usetok, usetok->str() + "." + membervar); + return true; + } + + return false; +} + bool CheckUninitVar::isVariableUsage(const Scope* scope, const Token *vartok, bool pointer) const { if (vartok->previous()->str() == "return") @@ -1519,6 +1545,36 @@ bool CheckUninitVar::isVariableUsage(const Scope* scope, const Token *vartok, bo return false; } +bool CheckUninitVar::isMemberVariableAssignment(const Token *tok, const std::string &membervar) const +{ + if (Token::Match(tok, "%var% . %var%") && tok->strAt(2) == membervar) { + if (Token::Match(tok->tokAt(3), "[=.[]")) + return true; + else if (Token::Match(tok->tokAt(-2), "[(,=] &")) + return true; + else if (Token::Match(tok->previous(), "%op%") || Token::Match(tok->previous(), "[|=")) + ; // member variable usage + else + return true; + } else if (tok->strAt(1) == "=") + return true; + else if (tok->strAt(-1) == "&") + return true; + return false; +} + +bool CheckUninitVar::isMemberVariableUsage(const Scope *scope, const Token *tok, bool isPointer, const std::string &membervar) const +{ + if (isMemberVariableAssignment(tok, membervar)) + return false; + + if (Token::Match(tok, "%var% . %var%") && tok->strAt(2) == membervar) + return true; + else if (Token::Match(tok->previous(), "[(,] %var% [,)]") && isVariableUsage(scope, tok, isPointer)) + return true; + + return false; +} void CheckUninitVar::uninitstringError(const Token *tok, const std::string &varname, bool strncpy_) { diff --git a/lib/checkuninitvar.h b/lib/checkuninitvar.h index 00055f579..36e31bcaa 100644 --- a/lib/checkuninitvar.h +++ b/lib/checkuninitvar.h @@ -66,8 +66,10 @@ public: 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 checkIfForWhileHead(const Scope *scope, const Token *startparentheses, const Variable& var, bool suppressErrors, bool isuninit, const std::string &membervar); + bool checkLoopBody(const Scope* scope, const Token *tok, const Variable& var, const std::string &membervar); bool isVariableUsage(const Scope* scope, const Token *vartok, bool ispointer) const; - + bool isMemberVariableAssignment(const Token *tok, const std::string &membervar) const; + bool isMemberVariableUsage(const Scope *scope, const Token *tok, bool isPointer, const std::string &membervar) const; /** * @brief Uninitialized variables: analyse functions to see how they work with uninitialized variables diff --git a/test/testuninitvar.cpp b/test/testuninitvar.cpp index d65c54313..4b65d8ef4 100644 --- a/test/testuninitvar.cpp +++ b/test/testuninitvar.cpp @@ -2151,41 +2151,6 @@ private: "}\n"); ASSERT_EQUALS("[test.cpp:5]: (error) Uninitialized variable: x\n", errout.str()); - // for, while - checkUninitVar2("void f() {\n" - " int x;\n" - " while (a) {\n" - " if (b) x++;\n" - " else x = 0;\n" - " }\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - checkUninitVar2("void f() {\n" - " int x;\n" - " for (int i = 0; i < 10; i += x) {\n" - " x = y;\n" - " }\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - checkUninitVar2("void f() {\n" - " int x;\n" - " for (int i = 0; i < 10; i += x) { }\n" - "}\n"); - ASSERT_EQUALS("[test.cpp:3]: (error) Uninitialized variable: x\n", errout.str()); - - checkUninitVar2("int f() {\n" - " int i;\n" - " for (i=0;i<9;++i)\n" - " if (foo()) goto out;\n" - "out:\n" - " return i;\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - checkUninitVar2(">{ x while (y) z int = }"); // #4175 : don't crash - // try checkUninitVar2("void f() {\n" " int i, *p = &i;\n" @@ -2674,6 +2639,41 @@ private: } void uninitvar2_while() { + // for, while + checkUninitVar2("void f() {\n" + " int x;\n" + " while (a) {\n" + " if (b) x++;\n" + " else x = 0;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + checkUninitVar2("void f() {\n" + " int x;\n" + " for (int i = 0; i < 10; i += x) {\n" + " x = y;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + checkUninitVar2("void f() {\n" + " int x;\n" + " for (int i = 0; i < 10; i += x) { }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (error) Uninitialized variable: x\n", errout.str()); + + checkUninitVar2("int f() {\n" + " int i;\n" + " for (i=0;i<9;++i)\n" + " if (foo()) goto out;\n" + "out:\n" + " return i;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + checkUninitVar2(">{ x while (y) z int = }"); // #4175 : don't crash + checkUninitVar2("int f(void) {\n" " int x;\n" " while (a()) {\n" // <- condition must always be true or there will be problem @@ -2685,6 +2685,26 @@ private: " return x;\n" "}\n", "test.c", true); TODO_ASSERT_EQUALS("error", "", errout.str()); + + checkUninitVar2("void f(void) {\n" + " int x;\n" + " for (;;) {\n" + " int a = x+1;\n" + " do_something(a);\n" + " }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (error) Uninitialized variable: x\n", errout.str()); + + checkUninitVar2("struct AB {int a; int b;};\n" + "void f(void) {\n" + " struct AB ab;\n" + " while (true) {\n" + " int a = 1+ab.a;\n" + " do_something(a);\n" + " }\n" + "}\n", "test.c"); + ASSERT_EQUALS("[test.c:5]: (error) Uninitialized variable: ab\n" + "[test.c:5]: (error) Uninitialized struct member: ab.a\n", errout.str()); } void uninitvar2_4494() {