From 11c7b8a8390fd694532b0298af67b2fb0ae95b49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sun, 20 Dec 2009 19:44:32 +0100 Subject: [PATCH] Execution Path: some refactorings of the checking --- lib/checkother.cpp | 107 +++++++++++++++++++++++++++++-------------- lib/executionpath.h | 8 +++- test/testmemleak.cpp | 28 +++++++++++ 3 files changed, 108 insertions(+), 35 deletions(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 14ebe7e95..3a2966543 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -1124,11 +1124,21 @@ class CheckNullpointer : public ExecutionPath { public: // Startup constructor - CheckNullpointer(unsigned int v) : ExecutionPath(), varId(v), null(false) + CheckNullpointer(CheckOther *c) : ExecutionPath(), varId(0), null(false), checkOther(c) { } private: + // Create checking of specific variable: + CheckNullpointer(CheckOther *c, const unsigned int id, const std::string &name) + : ExecutionPath(), + varId(id), + varname(name), + null(false), + checkOther(c) + { + } + ExecutionPath *copy() { return new CheckNullpointer(*this); @@ -1138,41 +1148,73 @@ private: void operator=(const CheckNullpointer &); const unsigned int varId; + const std::string varname; bool null; + CheckOther * const checkOther; - static void setnull(std::list &checks) - { - std::list::iterator it; - for (it = checks.begin(); it != checks.end(); ++it) - dynamic_cast(*it)->null = true; - } - - static void dereference(bool &foundError, std::list &checks) + static void setnull(std::list &checks, const unsigned int varid) { std::list::iterator it; for (it = checks.begin(); it != checks.end(); ++it) { - if (dynamic_cast(*it)->null) + CheckNullpointer *c = dynamic_cast(*it); + if (c && c->varId == varid) + c->null = true; + } + } + + static void dereference(bool &foundError, std::list &checks, const Token *tok) + { + const unsigned int varid(tok->varId()); + + std::list::iterator it; + for (it = checks.begin(); it != checks.end(); ++it) + { + CheckNullpointer *c = dynamic_cast(*it); + if (c && c->varId == varid && c->null) { foundError = true; + c->checkOther->nullPointerError(tok, c->varname); break; } } } + static void bailOutVar(std::list &checks, const unsigned int varid) + { + std::list::iterator it; + for (it = checks.begin(); it != checks.end(); ++it) + { + CheckNullpointer *c = dynamic_cast(*it); + if (c && c->varId == varid) + { + c->bailOut(true); + } + } + } + const Token *parse(const Token &tok, bool &foundError, std::list &checks) const { - if (tok.varId() == varId) + if (Token::Match(tok.previous(), "[;{}] %type% * %var% ;")) { - if (Token::Match(tok.previous(), "[;{}=] %varid% = 0 ;", varId)) - setnull(checks); - else if (Token::Match(tok.tokAt(-2), "[;{}=] * %varid%", varId)) - dereference(foundError, checks); - else if (Token::Match(tok.next(), ". %var%")) - dereference(foundError, checks); - else - bailOut(checks); + const Token * vartok = tok.tokAt(2); + if (vartok->varId() != 0) + checks.push_back(new CheckNullpointer(checkOther, vartok->varId(), vartok->str())); + return vartok->next(); } + + if (tok.varId() != 0) + { + if (Token::Match(tok.previous(), "[;{}=] %var% = 0 ;")) + setnull(checks, tok.varId()); + else if (Token::Match(tok.tokAt(-2), "[;{}=] * %var%")) + dereference(foundError, checks, &tok); + else if (Token::Match(tok.next(), ". %var%")) + dereference(foundError, checks, &tok); + else + bailOutVar(checks, tok.varId()); + } + return &tok; } }; @@ -1386,6 +1428,18 @@ void CheckOther::executionPaths() const Token *tok = _tokenizer->tokens(); while (0 != (tok = Token::findmatch(tok, ") const| {"))) { + // check for null pointer errors.. + { + std::list checks; + checks.push_back(new CheckNullpointer(this)); + checkExecutionPaths(tok->next(), checks); + while (!checks.empty()) + { + delete checks.back(); + checks.pop_back(); + } + } + // Scan through this scope and check all variables.. unsigned int indentlevel = 0; for (; tok; tok = tok->next()) @@ -1477,21 +1531,6 @@ void CheckOther::executionPaths() if (tokerr) uninitvarError(tokerr, tok->str()); } - - // check if variable is accessed uninitialized.. - if (pointer) - { - std::list checks; - checks.push_back(new CheckNullpointer(tok->varId())); - const Token *tokerr = checkExecutionPaths(tok->next(), checks); - while (!checks.empty()) - { - delete checks.back(); - checks.pop_back(); - } - if (tokerr) - nullPointerError(tokerr, tok->str()); - } } } } diff --git a/lib/executionpath.h b/lib/executionpath.h index 0321fff32..bda2eafb9 100644 --- a/lib/executionpath.h +++ b/lib/executionpath.h @@ -30,7 +30,13 @@ class Token; class ExecutionPath { private: - mutable bool bailout_; + bool bailout_; + +protected: + void bailOut(bool b) + { + bailout_ |= b; + } public: ExecutionPath() : bailout_(false) diff --git a/test/testmemleak.cpp b/test/testmemleak.cpp index 08e4467ad..4f4c9a329 100644 --- a/test/testmemleak.cpp +++ b/test/testmemleak.cpp @@ -40,6 +40,8 @@ private: { TEST_CASE(test1); TEST_CASE(test2); + TEST_CASE(test3); + TEST_CASE(test4); } void check(const char code[]) @@ -78,6 +80,32 @@ private: "}\n"); ASSERT_EQUALS("", errout.str()); } + + void test3() + { + check("void foo(int x)\n" + "{\n" + " char *p = 0;\n" + " if (x == 1)\n" + " p = new char[100];\n" + " if (x == 2)\n" + " delete [] p;\n" + "}\n"); + TODO_ASSERT_EQUALS("[test.cpp:8]: (error) Memory leak: p\n", errout.str()); + } + + void test4() + { + check("void foo(int x)\n" + "{\n" + " char *p = 0;\n" + " if (x == 1)\n" + " p = new char[100];\n" + " if (x == 1)\n" + " delete [] p;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } }; static TestLocalLeaks testLocalLeaks;