From a2f776b1b7dcafd00d043795a17b3d9e1929ef6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Tue, 5 Aug 2014 06:24:23 +0200 Subject: [PATCH] Dead pointer: Added checking for dead pointer usage when pointer alias local variable that has gone out of scope. --- lib/checkuninitvar.cpp | 35 +++++++++++++++++++++++++++++++++++ lib/checkuninitvar.h | 9 ++++++++- lib/token.cpp | 24 ++++++++++++++++++++++++ lib/token.h | 2 ++ lib/valueflow.cpp | 2 +- test/testuninitvar.cpp | 41 +++++++++++++++++++++++++++++++++++++++++ test/testvalueflow.cpp | 8 ++++++++ 7 files changed, 119 insertions(+), 2 deletions(-) diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp index c7a5b20d7..75a014304 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -1930,3 +1930,38 @@ void CheckUninitVar::uninitStructMemberError(const Token *tok, const std::string "uninitStructMember", "Uninitialized struct member: " + membername); } + +void CheckUninitVar::deadPointer() +{ + const bool cpp = _tokenizer->isCPP(); + 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; + // Dead pointers.. + for (const Token* tok = scope->classStart; tok != scope->classEnd; tok = tok->next()) { + if (tok->variable() && + tok->variable()->isPointer() && + isVariableUsage(tok, true, false, cpp)) { + const Token *alias = tok->getValueTokenDeadPointer(); + if (alias) { + deadPointerError(tok,alias); + } + } + } + } +} + +void CheckUninitVar::deadPointerError(const Token *pointer, const Token *alias) +{ + const std::string strpointer(pointer ? pointer->str() : std::string("pointer")); + const std::string stralias(alias ? alias->expressionString() : std::string("&x")); + + reportError(pointer, + Severity::error, + "deadpointer", + "Dead pointer usage. Pointer '" + strpointer + "' is dead if it has been assigned '" + stralias + "' at line " + MathLib::toString(alias ? alias->linenr() : 0U) + "."); +} diff --git a/lib/checkuninitvar.h b/lib/checkuninitvar.h index b461b4486..e994ae6df 100644 --- a/lib/checkuninitvar.h +++ b/lib/checkuninitvar.h @@ -50,6 +50,7 @@ public: CheckUninitVar checkUninitVar(tokenizer, settings, errorLogger); checkUninitVar.executionPaths(); checkUninitVar.check(); + checkUninitVar.deadPointer(); } /** Check for uninitialized variables */ @@ -64,6 +65,10 @@ public: static bool isMemberVariableAssignment(const Token *tok, const std::string &membervar); bool isMemberVariableUsage(const Token *tok, bool isPointer, bool alloc, const std::string &membervar) const; + /** ValueFlow-based checking for dead pointer usage */ + void deadPointer(); + void deadPointerError(const Token *pointer, const Token *alias); + /** * @brief Uninitialized variables: analyse functions to see how they work with uninitialized variables * @param tokens [in] the token list @@ -94,6 +99,7 @@ private: c.uninitdataError(0, "varname"); c.uninitvarError(0, "varname"); c.uninitStructMemberError(0, "a.b"); + c.deadPointerError(0,0); } static std::string myName() { @@ -102,7 +108,8 @@ private: std::string classInfo() const { return "Uninitialized variables\n" - "* using uninitialized variables and data\n"; + "* using uninitialized variables and data\n" + "* using dead pointer\n"; } }; /// @} diff --git a/lib/token.cpp b/lib/token.cpp index 4d4b443fa..b69e5545b 100644 --- a/lib/token.cpp +++ b/lib/token.cpp @@ -20,6 +20,7 @@ #include "errorlogger.h" #include "check.h" #include "settings.h" +#include "symboldatabase.h" #include #include #include @@ -1317,6 +1318,29 @@ const Token *Token::getValueTokenMaxStrLength() const return ret; } +const Token *Token::getValueTokenDeadPointer() const +{ + std::list::const_iterator it; + for (it = values.begin(); it != values.end(); ++it) { + // Is this a pointer alias? + if (!it->tokvalue || it->tokvalue->str() != "&") + continue; + // Get variable + const Token *vartok = it->tokvalue->astOperand1(); + if (!vartok || !vartok->isName() || !vartok->variable()) + continue; + const Variable * const var = vartok->variable(); + if (var->isStatic()) + continue; + const Scope *s = this->scope(); + while ((s != nullptr) && (s != var->scope())) + s = s->nestedIn; + if (!s) + return it->tokvalue; + } + return nullptr; +} + void Token::assignProgressValues(Token *tok) { unsigned int total_count = 0; diff --git a/lib/token.h b/lib/token.h index 35fcf9e46..309ce8bc8 100644 --- a/lib/token.h +++ b/lib/token.h @@ -673,6 +673,8 @@ public: const Token *getValueTokenMaxStrLength() const; const Token *getValueTokenMinStrSize() const; + const Token *getValueTokenDeadPointer() const; + private: void next(Token *nextToken) { diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 9f87e4c74..a3fb9d6db 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -364,7 +364,7 @@ static void valueFlowPointerAlias(TokenList *tokenlist) continue; // child should be some buffer or variable - if (!Token::Match(tok->astOperand1(), "%var%|.|[")) + if (!Token::Match(tok->astOperand1(), "%var%|.|[|;")) continue; ValueFlow::Value value; diff --git a/test/testuninitvar.cpp b/test/testuninitvar.cpp index 1229ec830..e8c454440 100644 --- a/test/testuninitvar.cpp +++ b/test/testuninitvar.cpp @@ -73,6 +73,9 @@ private: // Test that std.cfg is configured correctly TEST_CASE(stdcfg); + + // dead pointer + TEST_CASE(deadPointer); } void checkUninitVar(const char code[], const char filename[] = "test.cpp") { @@ -3701,6 +3704,44 @@ private: "}"); ASSERT_EQUALS("", errout.str()); } + + void checkDeadPointer(const char code[]) { + // Clear the error buffer.. + errout.str(""); + + // Tokenize.. + Tokenizer tokenizer(&settings, this); + std::istringstream istr(code); + tokenizer.tokenize(istr, "test.cpp"); + tokenizer.simplifyTokenList2(); + + // Check code.. + CheckUninitVar check(&tokenizer, &settings, this); + check.deadPointer(); + } + + void deadPointer() { + checkDeadPointer("void f() {\n" + " int *p = p1;\n" + " if (cond) {\n" + " int x;\n" + " p = &x;\n" + " }\n" + " *p = 0;\n" + "}"); + ASSERT_EQUALS("[test.cpp:7]: (error) Dead pointer usage. Pointer 'p' is dead if it has been assigned '&x' at line 5.\n", errout.str()); + + checkDeadPointer("void a(const int *p) {\n" + " *p = 0;\n" + "}\n" + "\n" + "void b() {\n" + " int x;\n" + " int *p = &x;" + " a(p);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + } }; REGISTER_TEST(TestUninitVar) diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 3d14ebcb9..63bdb964f 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -186,6 +186,14 @@ private: " return x;\n" "}"; ASSERT_EQUALS(true, testValueOfX(code, 5, "& ret [ 0 ]")); + + // dead pointer + code = "void f() {\n" + " int *x;\n" + " if (cond) { int i; x = &i; }\n" + " *x = 0;\n" // <- x can point at i + "}"; + ASSERT_EQUALS(true, testValueOfX(code, 4, "& i")); } void valueFlowCalculations() {