From c6d7fad84f3d1d12e0a9f3d09b78c9d118cbe6ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Thu, 29 Oct 2009 21:34:43 +0100 Subject: [PATCH] uninitialized variables: added check --- lib/checkother.cpp | 130 +++++++++++++++++++++++++++++++++++++++++++++ lib/checkother.h | 10 +++- test/testother.cpp | 55 +++++++++++++++++++ 3 files changed, 194 insertions(+), 1 deletion(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 0c462ab89..ba9b3f6bd 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -1160,6 +1160,131 @@ void CheckOther::nullPointer() nullPointerConditionalAssignment(); } +static const Token *uninitvar_checkscope(const Token *tok, const unsigned int varid, bool &init) +{ + /* limit the checking in conditional code.. + * int x; + * if (y) + * x = 33; + * if (y) + * return x; + */ + bool limit = false; + + for (; tok; tok = tok->next()) + { + if (tok->str() == "}") + return 0; + + // todo: handle for/while + if (Token::Match(tok, "for|while")) + { + init = true; + return 0; + } + + if (tok->str() == "if") + { + bool canInit = false; + bool ifInit = true; + while (tok->str() == "if") + { + // goto "(" + tok = tok->next(); + + // goto ")" + tok = tok ? tok->link() : 0; + + // goto "{" + tok = tok ? tok->next() : 0; + + if (!Token::simpleMatch(tok, "{")) + return 0; + + // Recursively check into the if .. + bool init2 = false; + const Token *tokerr = uninitvar_checkscope(tok->next(), varid, init2); + if (!limit && tokerr) + return tokerr; + + // if the scope didn't initialize then this whole if-chain is treated as non-initializing + ifInit &= init2; + canInit |= init2; + + // goto "}" + tok = tok->link(); + + // there is no else => not initialized + if (Token::Match(tok, "} !!else")) + { + ifInit = false; + break; + } + + // parse next "if".. + tok = tok->tokAt(2); + if (tok->str() == "if") + continue; + + // there is no "if".. + init2 = false; + tokerr = uninitvar_checkscope(tok->next(), varid, init2); + if (!limit && tokerr) + return tokerr; + + ifInit &= init2; + canInit |= init2; + + tok = tok->link(); + if (!tok) + return 0; + } + + if (ifInit) + { + init = true; + return 0; + } + + if (canInit) + { + limit = true; + } + } + + if (Token::Match(tok, "%varid% =", varid)) + { + init = true; + return 0; + } + + if (Token::Match(tok, "return| %varid% .|", varid)) + return tok; + } + return 0; +} + +void CheckOther::uninitvar() +{ + const Token *tok = _tokenizer->tokens(); + while (0 != (tok = Token::findmatch(tok, "[{};] %type% *| %var% ;"))) + { + // goto the variable + tok = tok->tokAt(2); + if (tok->str() == "*") + tok = tok->next(); + + // check that the variable id is non-zero + if (tok->varId() == 0) + continue; + + bool init = false; + const Token *tokerr = uninitvar_checkscope(tok->next(), tok->varId(), init); + if (tokerr) + uninitvarError(tokerr, tok->str()); + } +} + void CheckOther::checkZeroDivision() { for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) @@ -1301,6 +1426,11 @@ void CheckOther::nullPointerError(const Token *tok, const std::string &varname, reportError(tok, Severity::error, "nullPointer", "Possible null pointer dereference: " + varname + " - otherwise it is redundant to check if " + varname + " is null at line " + MathLib::toString(line)); } +void CheckOther::uninitvarError(const Token *tok, const std::string &varname) +{ + reportError(tok, Severity::error, "uninitvar", "Uninitialized variable: " + varname); +} + void CheckOther::zerodivError(const Token *tok) { reportError(tok, Severity::error, "zerodiv", "Division by zero"); diff --git a/lib/checkother.h b/lib/checkother.h index 98300aaf6..d75a6134c 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -76,6 +76,7 @@ public: checkOther.strPlusChar(); checkOther.invalidFunctionUsage(); checkOther.checkZeroDivision(); + checkOther.uninitvar(); } // Casting @@ -111,6 +112,9 @@ public: /** possible null pointer dereference */ void nullPointer(); + /** reading uninitialized var */ + void uninitvar(); + /** Check zero division*/ void checkZeroDivision(); @@ -124,6 +128,7 @@ public: // haystack.remove(needle); void redundantCondition2(); + // Error messages.. void cstyleCastError(const Token *tok); void redundantIfDelete0Error(const Token *tok); @@ -142,6 +147,7 @@ public: void strPlusChar(const Token *tok); void nullPointerError(const Token *tok, const std::string &varname); void nullPointerError(const Token *tok, const std::string &varname, const int line); + void uninitvarError(const Token *tok, const std::string &varname); void zerodivError(const Token *tok); void postIncrementError(const Token *tok, const std::string &var_name, const bool isIncrement); @@ -151,6 +157,7 @@ public: sprintfOverlappingDataError(0, "varname"); udivError(0); nullPointerError(0, "pointer"); + uninitvarError(0, "varname"); zerodivError(0); // style @@ -186,7 +193,7 @@ public: " * division with zero\n" " * null pointer dereferencing\n" - // warning + // style " * C-style pointer cast in cpp file\n" " * redundant if\n" " * bad usage of the function 'strtol'\n" @@ -198,6 +205,7 @@ public: " * variable scope can be limited\n" " * condition that is always true/false\n" " * unusal pointer arithmetic. For example: \"abc\" + 'd'\n" + " * uninitialized variables\n" // optimisations " * optimisation: detect post increment/decrement\n"; diff --git a/test/testother.cpp b/test/testother.cpp index d7e469b27..5f100e160 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -68,6 +68,8 @@ private: TEST_CASE(nullpointer5); // References should not be checked TEST_CASE(nullpointer6); + TEST_CASE(uninitvar1); + TEST_CASE(oldStylePointerCast); TEST_CASE(postIncrementDecrementStl); @@ -910,6 +912,59 @@ private: ASSERT_EQUALS("", errout.str()); } + + + void checkUninitVar(const char code[]) + { + // Tokenize.. + Tokenizer tokenizer; + std::istringstream istr(code); + tokenizer.tokenize(istr, "test.cpp"); + + // Clear the error buffer.. + errout.str(""); + + // Check for redundant code.. + Settings settings; + settings._checkCodingStyle = true; + CheckOther checkOther(&tokenizer, &settings, this); + checkOther.uninitvar(); + } + + void uninitvar1() + { + checkUninitVar("static void foo()\n" + "{\n" + " Foo *p;\n" + " if (x)\n" + " p = new Foo;\n" + " p->abcd();\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:6]: (error) Uninitialized variable: p\n", errout.str()); + + checkUninitVar("int foo()\n" + "{\n" + " int i;\n" + " if (x)\n" + " i = 22;\n" + " else\n" + " i = 33;\n" + " return i;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + checkUninitVar("static void foo()\n" + "{\n" + " Foo *p;\n" + " if (x)\n" + " p = new Foo;\n" + " if (x)\n" + " p->abcd();\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + + void checkOldStylePointerCast(const char code[]) { // Tokenize..