From 0c13f9ba5c3ea80a534c016078b73a283dbf5f2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Mon, 14 Dec 2009 20:30:31 +0100 Subject: [PATCH] Added TestLocalLeaks --- Makefile | 8 +- lib/checkmemoryleak.cpp | 167 +++++++++++++++++++++++++++++++++++ lib/checkmemoryleak.h | 2 + lib/checkother.cpp | 187 +--------------------------------------- lib/executionpath.cpp | 167 +++++++++++++++++++++++++++++++++++ lib/executionpath.h | 74 ++++++++++++++++ test/testmemleak.cpp | 57 ++++++++++++ 7 files changed, 474 insertions(+), 188 deletions(-) create mode 100644 lib/executionpath.cpp create mode 100644 lib/executionpath.h diff --git a/Makefile b/Makefile index 55a905f9b..f26fcefaa 100644 --- a/Makefile +++ b/Makefile @@ -22,6 +22,7 @@ LIBOBJ = lib/checkautovariables.o \ lib/checkunusedfunctions.o \ lib/cppcheck.o \ lib/errorlogger.o \ + lib/executionpath.o \ lib/filelister.o \ lib/mathlib.o \ lib/preprocessor.o \ @@ -109,10 +110,10 @@ lib/checkexceptionsafety.o: lib/checkexceptionsafety.cpp lib/checkexceptionsafet lib/checkheaders.o: lib/checkheaders.cpp lib/checkheaders.h lib/tokenize.h lib/classinfo.h lib/token.h lib/errorlogger.h lib/settings.h lib/filelister.h $(CXX) $(CXXFLAGS) -Ilib -c -o lib/checkheaders.o lib/checkheaders.cpp -lib/checkmemoryleak.o: lib/checkmemoryleak.cpp lib/checkmemoryleak.h lib/check.h lib/token.h lib/tokenize.h lib/classinfo.h lib/settings.h lib/errorlogger.h lib/mathlib.h +lib/checkmemoryleak.o: lib/checkmemoryleak.cpp lib/checkmemoryleak.h lib/check.h lib/token.h lib/tokenize.h lib/classinfo.h lib/settings.h lib/errorlogger.h lib/mathlib.h lib/executionpath.h $(CXX) $(CXXFLAGS) -Ilib -c -o lib/checkmemoryleak.o lib/checkmemoryleak.cpp -lib/checkother.o: lib/checkother.cpp lib/checkother.h lib/check.h lib/token.h lib/tokenize.h lib/classinfo.h lib/settings.h lib/errorlogger.h lib/mathlib.h +lib/checkother.o: lib/checkother.cpp lib/checkother.h lib/check.h lib/token.h lib/tokenize.h lib/classinfo.h lib/settings.h lib/errorlogger.h lib/mathlib.h lib/executionpath.h $(CXX) $(CXXFLAGS) -Ilib -c -o lib/checkother.o lib/checkother.cpp lib/checkstl.o: lib/checkstl.cpp lib/checkstl.h lib/check.h lib/token.h lib/tokenize.h lib/classinfo.h lib/settings.h lib/errorlogger.h @@ -127,6 +128,9 @@ lib/cppcheck.o: lib/cppcheck.cpp lib/cppcheck.h lib/settings.h lib/errorlogger.h lib/errorlogger.o: lib/errorlogger.cpp lib/errorlogger.h lib/settings.h lib/tokenize.h lib/classinfo.h lib/token.h $(CXX) $(CXXFLAGS) -Ilib -c -o lib/errorlogger.o lib/errorlogger.cpp +lib/executionpath.o: lib/executionpath.cpp lib/executionpath.h lib/token.h + $(CXX) $(CXXFLAGS) -Ilib -c -o lib/executionpath.o lib/executionpath.cpp + lib/filelister.o: lib/filelister.cpp lib/filelister.h $(CXX) $(CXXFLAGS) -Ilib -c -o lib/filelister.o lib/filelister.cpp diff --git a/lib/checkmemoryleak.cpp b/lib/checkmemoryleak.cpp index 7f85fd8d2..469082b22 100644 --- a/lib/checkmemoryleak.cpp +++ b/lib/checkmemoryleak.cpp @@ -20,6 +20,7 @@ #include "checkmemoryleak.h" #include "mathlib.h" #include "tokenize.h" +#include "executionpath.h" #include #include @@ -2513,3 +2514,169 @@ void CheckMemoryLeakStructMember::check() + + +class CheckLocalLeaks : public ExecutionPath +{ +public: + // Startup constructor + CheckLocalLeaks(CheckMemoryLeak *c) : ExecutionPath(), ownerCheck(c), varId(0), allocated(false) + { + } + + static void printOut(const std::list &checks) + { + std::ostringstream ostr; + ostr << "CheckLocalLeaks::printOut" << std::endl; + for (std::list::const_iterator it = checks.begin(); it != checks.end(); ++it) + { + CheckLocalLeaks *c = dynamic_cast(*it); + if (c) + { + ostr << std::hex << (int)c << ": varId=" << c->varId << " allocated=" << (c->allocated ? "true" : "false") << std::endl; + } + } + std::cout << ostr.str(); + } + +private: + ExecutionPath *copy() + { + return new CheckLocalLeaks(*this); + } + + /** start checking of given variable */ + CheckLocalLeaks(CheckMemoryLeak *c, unsigned int v, const std::string &s) : ExecutionPath(), ownerCheck(c), varId(v), allocated(false), varname(s) + { + } + + CheckMemoryLeak * const ownerCheck; + const unsigned int varId; + bool allocated; + const std::string varname; + + /* no implementation */ + void operator=(const CheckLocalLeaks &); + + static void alloc(std::list &checks, const unsigned int varid) + { + if (varid == 0) + return; + + std::list::iterator it; + for (it = checks.begin(); it != checks.end(); ++it) + { + CheckLocalLeaks *C = dynamic_cast(*it); + if (C && C->varId == varid) + C->allocated = true; + } + } + + static void dealloc(std::list &checks, const Token *tok) + { + if (tok->varId() == 0) + return; + + std::list::iterator it; + for (it = checks.begin(); it != checks.end(); ++it) + { + CheckLocalLeaks *C = dynamic_cast(*it); + if (C && C->varId == tok->varId()) + C->allocated = false; + } + } + + static void ret(const std::list &checks, const Token *tok) + { + std::list::const_iterator it; + for (it = checks.begin(); it != checks.end(); ++it) + { + CheckLocalLeaks *C = dynamic_cast(*it); + if (C && C->allocated) + { + C->ownerCheck->memleakError(tok, C->varname, false); + } + } + } + + const Token *parse(const Token &tok, bool &, std::list &checks) const + { + //std::cout << "CheckLocalLeaks::parse " << tok.str() << std::endl; + //printOut(checks); + + if (!Token::Match(tok.previous(), "[;{}]")) + return &tok; + + if (Token::Match(&tok, "%type% * %var% ;")) + { + const Token * vartok = tok.tokAt(2); + if (vartok->varId() != 0) + checks.push_back(new CheckLocalLeaks(ownerCheck, vartok->varId(), vartok->str())); + return vartok->next(); + } + + if (Token::Match(&tok, "%var% = new")) + { + alloc(checks, tok.varId()); + + // goto end of statement + const Token *tok2 = &tok; + while (tok2 && tok2->str() != ";") + tok2 = tok2->next(); + return tok2; + } + + if (Token::Match(&tok, "delete %var% ;")) + { + dealloc(checks, tok.next()); + return tok.tokAt(2); + } + + if (Token::Match(&tok, "delete [ ] %var% ;")) + { + dealloc(checks, tok.tokAt(3)); + return tok.tokAt(4); + } + + if (tok.str() == "return") + { + ret(checks, &tok); + } + + return &tok; + } +}; + + +void CheckMemoryLeakInFunction::localleaks() +{ + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) + { + if (tok->str() != ")") + continue; + + // Start of implementation.. + if (Token::Match(tok, ") const| {")) + { + // goto the "{" + tok = tok->next(); + if (tok->str() == "const") + tok = tok->next(); + + // Check this scope.. + std::list checks; + checks.push_back(new CheckLocalLeaks(this)); + checkExecutionPaths(tok->next(), checks); + while (!checks.empty()) + { + delete checks.back(); + checks.pop_back(); + } + + // skip this scope - it has been checked + tok = tok->link(); + } + } + +} + diff --git a/lib/checkmemoryleak.h b/lib/checkmemoryleak.h index 8f64f3c3e..abe7dec4d 100644 --- a/lib/checkmemoryleak.h +++ b/lib/checkmemoryleak.h @@ -161,6 +161,8 @@ public: void check(); + void localleaks(); + #ifndef _MSC_VER private: #endif diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 107c503ec..7086520dc 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -21,6 +21,7 @@ #include "checkother.h" #include "mathlib.h" #include "tokenize.h" +#include "executionpath.h" #include #include @@ -1117,50 +1118,6 @@ void CheckOther::nullPointer() } -/** - * Base class for Execution Paths checking - * An execution path is a linear list of statements. There are no "if"/.. to worry about. - **/ -class ExecutionPath -{ -private: - mutable bool bailout_; - -public: - ExecutionPath() : bailout_(false) - { } - - virtual ~ExecutionPath() - { } - - virtual ExecutionPath *copy() = 0; - - bool bailOut() const - { - return bailout_; - } - - /** - * bail out all execution paths - * @param checks the execution paths to bail out on - **/ - static void bailOut(const std::list &checks) - { - std::list::const_iterator it; - for (it = checks.begin(); it != checks.end(); ++it) - (*it)->bailout_ = true; - } - - /** - * Parse tokens at given location - * @param tok token to parse - * @param foundError If an error is found this is set to true and the return token is the error token - * @param checks The execution paths. All execution paths in the list are executed in the current scope. - * @return if error is found => error token. if you want to skip tokens, return the last skipped token. otherwise return tok. - **/ - virtual const Token *parse(const Token &tok, bool &foundError, std::list &checks) const = 0; -}; - class CheckNullpointer : public ExecutionPath @@ -1416,148 +1373,6 @@ private: }; -static const Token *checkExecutionPaths(const Token *tok, std::list &checks) -{ - const std::auto_ptr check(checks.front()->copy()); - - // Number of "if" blocks in current scope - unsigned int numberOfIf = 0; - - for (; tok; tok = tok->next()) - { - if (tok->str() == "}") - return 0; - - // todo: handle for/while - if (Token::Match(tok, "for|while|switch")) - { - ExecutionPath::bailOut(checks); - return 0; - } - - if (Token::Match(tok, "= {|(")) - { - tok = tok->next()->link(); - if (Token::simpleMatch(tok, ") {")) - tok = tok->next()->link(); - if (!tok) - { - ExecutionPath::bailOut(checks); - return 0; - } - continue; - } - - if (tok->str() == "if") - { - ++numberOfIf; - if (numberOfIf > 1) - return 0; - - std::list newchecks; - while (tok->str() == "if") - { - // goto "(" - tok = tok->next(); - - // goto ")" - tok = tok ? tok->link() : 0; - - // Check if the condition contains the variable - if (tok) - { - for (const Token *tok2 = tok->link(); tok2 && tok2 != tok; tok2 = tok2->next()) - { - // The variable may be initialized.. - // if (someFunction(&var)) .. - if (tok2->varId()) - { - ExecutionPath::bailOut(checks); - return 0; - } - } - } - - // goto "{" - tok = tok ? tok->next() : 0; - - if (!Token::simpleMatch(tok, "{")) - return 0; - - // Recursively check into the if .. - { - std::list c; - std::list::iterator it; - for (it = checks.begin(); it != checks.end(); ++it) - c.push_back((*it)->copy()); - const Token *tokerr = checkExecutionPaths(tok->next(), c); - if (tokerr) - return tokerr; - while (!c.empty()) - { - newchecks.push_back(c.back()); - c.pop_back(); - } - } - - // goto "}" - tok = tok->link(); - - // there is no else => break out - if (Token::Match(tok, "} !!else")) - break; - - // parse next "if".. - tok = tok->tokAt(2); - if (tok->str() == "if") - continue; - - // there is no "if".. - const Token *tokerr = checkExecutionPaths(tok->next(), checks); - if (tokerr) - return tokerr; - - tok = tok->link(); - if (!tok) - return 0; - } - - std::list::iterator it; - for (it = newchecks.begin(); it != newchecks.end(); ++it) - checks.push_back((*it)->copy()); - } - - - { - bool foundError = false; - tok = check->parse(*tok, foundError, checks); - std::list::iterator it; - for (it = checks.begin(); it != checks.end();) - { - if ((*it)->bailOut()) - { - delete *it; - it = checks.erase(it); - } - else - { - ++it; - } - } - if (checks.empty()) - return 0; - else if (foundError) - return tok; - } - - // return ends all execution paths - if (tok->str() == "return") - { - ExecutionPath::bailOut(checks); - } - } - return 0; -} void CheckOther::executionPaths() { diff --git a/lib/executionpath.cpp b/lib/executionpath.cpp new file mode 100644 index 000000000..409feb4a0 --- /dev/null +++ b/lib/executionpath.cpp @@ -0,0 +1,167 @@ +/* + * Cppcheck - A tool for static C/C++ code analysis + * Copyright (C) 2007-2009 Daniel Marjamäki and Cppcheck team. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + + +#include "executionpath.h" +#include "token.h" +#include + + +const Token *checkExecutionPaths(const Token *tok, std::list &checks) +{ + const std::auto_ptr check(checks.front()->copy()); + + // Number of "if" blocks in current scope + unsigned int numberOfIf = 0; + + for (; tok; tok = tok->next()) + { + if (tok->str() == "}") + return 0; + + // todo: handle for/while + if (Token::Match(tok, "for|while|switch")) + { + ExecutionPath::bailOut(checks); + return 0; + } + + if (Token::Match(tok, "= {|(")) + { + tok = tok->next()->link(); + if (Token::simpleMatch(tok, ") {")) + tok = tok->next()->link(); + if (!tok) + { + ExecutionPath::bailOut(checks); + return 0; + } + continue; + } + + if (tok->str() == "if") + { + ++numberOfIf; + if (numberOfIf > 1) + return 0; + + std::list newchecks; + while (tok->str() == "if") + { + // goto "(" + tok = tok->next(); + + // goto ")" + tok = tok ? tok->link() : 0; + + // Check if the condition contains the variable + if (tok) + { + for (const Token *tok2 = tok->link(); tok2 && tok2 != tok; tok2 = tok2->next()) + { + // The variable may be initialized.. + // if (someFunction(&var)) .. + if (tok2->varId()) + { + ExecutionPath::bailOut(checks); + return 0; + } + } + } + + // goto "{" + tok = tok ? tok->next() : 0; + + if (!Token::simpleMatch(tok, "{")) + return 0; + + // Recursively check into the if .. + { + std::list c; + std::list::iterator it; + for (it = checks.begin(); it != checks.end(); ++it) + c.push_back((*it)->copy()); + const Token *tokerr = checkExecutionPaths(tok->next(), c); + if (tokerr) + return tokerr; + while (!c.empty()) + { + newchecks.push_back(c.back()); + c.pop_back(); + } + } + + // goto "}" + tok = tok->link(); + + // there is no else => break out + if (Token::Match(tok, "} !!else")) + break; + + // parse next "if".. + tok = tok->tokAt(2); + if (tok->str() == "if") + continue; + + // there is no "if".. + const Token *tokerr = checkExecutionPaths(tok->next(), checks); + if (tokerr) + return tokerr; + + tok = tok->link(); + if (!tok) + return 0; + } + + std::list::iterator it; + for (it = newchecks.begin(); it != newchecks.end(); ++it) + checks.push_back((*it)->copy()); + } + + + { + bool foundError = false; + tok = check->parse(*tok, foundError, checks); + std::list::iterator it; + for (it = checks.begin(); it != checks.end();) + { + if ((*it)->bailOut()) + { + delete *it; + it = checks.erase(it); + } + else + { + ++it; + } + } + if (checks.empty()) + return 0; + else if (foundError) + return tok; + } + + // return ends all execution paths + if (tok->str() == "return") + { + ExecutionPath::bailOut(checks); + } + } + return 0; +} + diff --git a/lib/executionpath.h b/lib/executionpath.h new file mode 100644 index 000000000..0321fff32 --- /dev/null +++ b/lib/executionpath.h @@ -0,0 +1,74 @@ +/* + * Cppcheck - A tool for static C/C++ code analysis + * Copyright (C) 2007-2009 Daniel Marjamäki and Cppcheck team. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#ifndef executionpathH +#define executionpathH + +#include + +class Token; + +/** + * Base class for Execution Paths checking + * An execution path is a linear list of statements. There are no "if"/.. to worry about. + **/ +class ExecutionPath +{ +private: + mutable bool bailout_; + +public: + ExecutionPath() : bailout_(false) + { } + + virtual ~ExecutionPath() + { } + + virtual ExecutionPath *copy() = 0; + + bool bailOut() const + { + return bailout_; + } + + /** + * bail out all execution paths + * @param checks the execution paths to bail out on + **/ + static void bailOut(const std::list &checks) + { + std::list::const_iterator it; + for (it = checks.begin(); it != checks.end(); ++it) + (*it)->bailout_ = true; + } + + /** + * Parse tokens at given location + * @param tok token to parse + * @param foundError If an error is found this is set to true and the return token is the error token + * @param checks The execution paths. All execution paths in the list are executed in the current scope. + * @return if error is found => error token. if you want to skip tokens, return the last skipped token. otherwise return tok. + **/ + virtual const Token *parse(const Token &tok, bool &foundError, std::list &checks) const = 0; +}; + + +const Token *checkExecutionPaths(const Token *tok, std::list &checks); + + +#endif diff --git a/test/testmemleak.cpp b/test/testmemleak.cpp index a7df4468c..14538b8f5 100644 --- a/test/testmemleak.cpp +++ b/test/testmemleak.cpp @@ -29,6 +29,63 @@ extern std::ostringstream errout; +class TestLocalLeaks : private TestFixture +{ +public: + TestLocalLeaks() : TestFixture("TestLocalLeaks") + { } + +private: + void run() + { + TEST_CASE(test1); + TEST_CASE(test2); + } + + void check(const char code[]) + { + // Tokenize.. + Tokenizer tokenizer; + std::istringstream istr(code); + tokenizer.tokenize(istr, "test.cpp"); + tokenizer.setVarId(); + tokenizer.simplifyTokenList(); + + // Clear the error buffer.. + errout.str(""); + + // Check for memory leaks.. + Settings settings; + CheckMemoryLeakInFunction checkMemoryLeak(&tokenizer, &settings, this); + checkMemoryLeak.localleaks(); + } + + void test1() + { + check("void foo()\n" + "{\n" + " char *p = new char[100];\n" + " return;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (error) Memory leak: p\n", errout.str()); + } + + void test2() + { + check("void foo()\n" + "{\n" + " char *p = new char[100];\n" + " delete [] p;\n" + " return;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } +}; + +static TestLocalLeaks testLocalLeaks; + + + class TestMemleak : private TestFixture { public: