diff --git a/lib/checkleakautovar.cpp b/lib/checkleakautovar.cpp index d9209897f..aa83bcca0 100644 --- a/lib/checkleakautovar.cpp +++ b/lib/checkleakautovar.cpp @@ -21,10 +21,7 @@ //--------------------------------------------------------------------------- #include "checkleakautovar.h" - #include "checkmemoryleak.h" // <- CheckMemoryLeak::memoryLeak -#include "checkother.h" // <- doubleFreeError - #include "tokenize.h" #include "symboldatabase.h" @@ -34,9 +31,6 @@ // Register this check class (by creating a static instance of it) namespace { CheckLeakAutoVar instance; - - const int DEALLOC = -1; - const int NOALLOC = 0; } //--------------------------------------------------------------------------- @@ -44,14 +38,14 @@ namespace { void VarInfo::print() { std::cout << "size=" << alloctype.size() << std::endl; - std::map::const_iterator it; + std::map::const_iterator it; for (it = alloctype.begin(); it != alloctype.end(); ++it) { std::string strusage; std::map::const_iterator use = possibleUsage.find(it->first); if (use != possibleUsage.end()) strusage = use->second; - std::cout << "alloctype='" << it->second << "' " + std::cout << "alloctype='" << it->second.type << "' " << "possibleUsage='" << strusage << "'" << std::endl; } } @@ -59,7 +53,7 @@ void VarInfo::print() void VarInfo::possibleUsageAll(const std::string &functionName) { possibleUsage.clear(); - std::map::const_iterator it; + std::map::const_iterator it; for (it = alloctype.begin(); it != alloctype.end(); ++it) possibleUsage[it->first] = functionName; } @@ -102,10 +96,22 @@ void CheckLeakAutoVar::configurationInfo(const Token* tok, const std::string &fu } } +void CheckLeakAutoVar::doubleFreeError(const Token *tok, const std::string &varname, int type) +{ + if (_settings->library.isresource(type)) + reportError(tok, Severity::error, "doubleFree", "Resource handle '" + varname + "' freed twice."); + else + reportError(tok, Severity::error, "doubleFree", "Memory pointed to by '" + varname + "' is freed twice."); +} + + void CheckLeakAutoVar::check() { const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase(); + // Local variables that are known to be non-zero. + const std::set notzero; + // Check function scopes const std::size_t functions = symbolDatabase->functionScopes.size(); for (std::size_t i = 0; i < functions; ++i) { @@ -113,15 +119,12 @@ void CheckLeakAutoVar::check() // Empty variable info VarInfo varInfo; - // Local variables that are known to be non-zero. - const std::set notzero; - checkScope(scope->classStart, &varInfo, notzero); varInfo.conditionalAlloc.clear(); // Clear reference arguments from varInfo.. - std::map::iterator it = varInfo.alloctype.begin(); + std::map::iterator it = varInfo.alloctype.begin(); while (it != varInfo.alloctype.end()) { const Variable *var = symbolDatabase->getVariableFromVarId(it->first); if (!var || @@ -140,7 +143,7 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken, VarInfo *varInfo, std::set notzero) { - std::map &alloctype = varInfo->alloctype; + std::map &alloctype = varInfo->alloctype; std::map &possibleUsage = varInfo->possibleUsage; const std::set conditionalAlloc(varInfo->conditionalAlloc); @@ -149,9 +152,9 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken, for (const Token *tok = startToken; tok && tok != endToken; tok = tok->next()) { // Deallocation and then dereferencing pointer.. if (tok->varId() > 0) { - const std::map::iterator var = alloctype.find(tok->varId()); + const std::map::iterator var = alloctype.find(tok->varId()); if (var != alloctype.end()) { - if (var->second == DEALLOC && !Token::Match(tok->previous(), "[;{},=] %var% =")) { + if (var->second.status == VarInfo::DEALLOC && !Token::Match(tok->previous(), "[;{},=] %var% =")) { deallocUseError(tok, tok->str()); } else if (Token::simpleMatch(tok->tokAt(-2), "= &")) { varInfo->erase(tok->varId()); @@ -164,7 +167,8 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken, } if (tok->str() == "(" && tok->previous()->isName()) { - functionCall(tok->previous(), varInfo, NOALLOC); + VarInfo::AllocInfo allocation(0, VarInfo::NOALLOC); + functionCall(tok->previous(), varInfo, allocation); tok = tok->link(); continue; } @@ -172,6 +176,7 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken, // look for end of statement if (!Token::Match(tok, "[;{}]") || Token::Match(tok->next(), "[;{}]")) continue; + tok = tok->next(); if (!tok || tok == endToken) break; @@ -236,8 +241,12 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken, if (Token::Match(tok->tokAt(2), "%type% (")) { int i = _settings->library.alloc(tok->tokAt(2)); if (i > 0) { - alloctype[tok->varId()] = i; + alloctype[tok->varId()].type = i; + alloctype[tok->varId()].status = VarInfo::ALLOC; } + } else if (_tokenizer->isCPP() && tok->strAt(2) == "new") { + alloctype[tok->varId()].type = -1; + alloctype[tok->varId()].status = VarInfo::ALLOC; } // Assigning non-zero value variable. It might be used to @@ -257,8 +266,10 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken, if (innerTok->str() == ")") break; if (innerTok->str() == "(" && innerTok->previous()->isName()) { - const int deallocId = _settings->library.dealloc(tok); - functionCall(innerTok->previous(), varInfo, deallocId); + VarInfo::AllocInfo allocation(_settings->library.dealloc(tok), VarInfo::DEALLOC); + if (allocation.type == 0) + allocation.status = VarInfo::NOALLOC; + functionCall(innerTok->previous(), varInfo, allocation); innerTok = innerTok->link(); } } @@ -303,7 +314,7 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken, old.swap(*varInfo); // Conditional allocation in varInfo1 - std::map::const_iterator it; + std::map::const_iterator it; for (it = varInfo1.alloctype.begin(); it != varInfo1.alloctype.end(); ++it) { if (varInfo2.alloctype.find(it->first) == varInfo2.alloctype.end() && old.alloctype.find(it->first) == old.alloctype.end()) { @@ -321,13 +332,13 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken, // Conditional allocation/deallocation for (it = varInfo1.alloctype.begin(); it != varInfo1.alloctype.end(); ++it) { - if (it->second == DEALLOC && conditionalAlloc.find(it->first) != conditionalAlloc.end()) { + if (it->second.status == VarInfo::DEALLOC && conditionalAlloc.find(it->first) != conditionalAlloc.end()) { varInfo->conditionalAlloc.erase(it->first); varInfo2.erase(it->first); } } for (it = varInfo2.alloctype.begin(); it != varInfo2.alloctype.end(); ++it) { - if (it->second == DEALLOC && conditionalAlloc.find(it->first) != conditionalAlloc.end()) { + if (it->second.status == VarInfo::DEALLOC && conditionalAlloc.find(it->first) != conditionalAlloc.end()) { varInfo->conditionalAlloc.erase(it->first); varInfo1.erase(it->first); } @@ -341,22 +352,23 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken, } } - // unknown control.. - else if (Token::Match(tok, "%type% (") && Token::simpleMatch(tok->linkAt(1), ") {")) { + // unknown control.. (TODO: handle loops) + else if ((Token::Match(tok, "%type% (") && Token::simpleMatch(tok->linkAt(1), ") {")) || Token::Match(tok, "do {")) { varInfo->clear(); break; } // Function call.. - else if (Token::Match(tok, "%type% (") && tok->str() != "return") { - const int dealloc = _settings->library.dealloc(tok); - - functionCall(tok, varInfo, dealloc); + else if (Token::Match(tok, "%type% (") && tok->str() != "return" && tok->str() != "throw") { + VarInfo::AllocInfo allocation(_settings->library.dealloc(tok), VarInfo::DEALLOC); + if (allocation.type == 0) + allocation.status = VarInfo::NOALLOC; + functionCall(tok, varInfo, allocation); tok = tok->next()->link(); // Handle scopes that might be noreturn - if (dealloc == NOALLOC && Token::simpleMatch(tok, ") ; }")) { + if (allocation.status == VarInfo::NOALLOC && Token::simpleMatch(tok, ") ; }")) { const std::string &functionName(tok->link()->previous()->str()); bool unknown = false; if (_tokenizer->IsScopeNoReturn(tok->tokAt(2), &unknown)) { @@ -371,6 +383,20 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken, continue; } + // delete + else if (_tokenizer->isCPP() && tok->str() == "delete") { + if (tok->strAt(1) == "[") + tok = tok->tokAt(3); + else + tok = tok->next(); + while (Token::Match(tok, "%var% ::|.")) + tok = tok->tokAt(2); + if (tok->varId()) { + VarInfo::AllocInfo allocation(-1, VarInfo::DEALLOC); + changeAllocStatus(varInfo, allocation, tok, tok); + } + } + // return else if (tok->str() == "return") { ret(tok, *varInfo); @@ -404,16 +430,40 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken, } } -void CheckLeakAutoVar::functionCall(const Token *tok, VarInfo *varInfo, const int dealloc) +void CheckLeakAutoVar::changeAllocStatus(VarInfo *varInfo, const VarInfo::AllocInfo& allocation, const Token* tok, const Token* arg) +{ + std::map &alloctype = varInfo->alloctype; + std::map &possibleUsage = varInfo->possibleUsage; + const std::map::iterator var = alloctype.find(arg->varId()); + if (var != alloctype.end()) { + if (allocation.status == VarInfo::NOALLOC) { + // possible usage + possibleUsage[arg->varId()] = tok->str(); + if (var->second.status == VarInfo::DEALLOC && arg->previous()->str() == "&") + varInfo->erase(arg->varId()); + } else if (var->second.status == VarInfo::DEALLOC) { + doubleFreeError(tok, arg->str(), allocation.type); + } else if (var->second.type != allocation.type) { + // mismatching allocation and deallocation + mismatchError(tok, arg->str()); + varInfo->erase(arg->varId()); + } else { + // deallocation + var->second.status = VarInfo::DEALLOC; + var->second.type = allocation.type; + } + } else if (allocation.status != VarInfo::NOALLOC) { + alloctype[arg->varId()].status = VarInfo::DEALLOC; + } +} + +void CheckLeakAutoVar::functionCall(const Token *tok, VarInfo *varInfo, const VarInfo::AllocInfo& allocation) { // Ignore function call? const bool ignore = bool(_settings->library.leakignore.find(tok->str()) != _settings->library.leakignore.end()); if (ignore) return; - std::map &alloctype = varInfo->alloctype; - std::map &possibleUsage = varInfo->possibleUsage; - for (const Token *arg = tok->tokAt(2); arg; arg = arg->nextArgument()) { if (arg->str() == "new") arg = arg->next(); @@ -426,29 +476,9 @@ void CheckLeakAutoVar::functionCall(const Token *tok, VarInfo *varInfo, const in arg = arg->next(); // Is variable allocated? - const std::map::iterator var = alloctype.find(arg->varId()); - if (var != alloctype.end()) { - if (dealloc == NOALLOC) { - // possible usage - possibleUsage[arg->varId()] = tok->str(); - if (var->second == DEALLOC && arg->previous()->str() == "&") - varInfo->erase(arg->varId()); - } else if (var->second == DEALLOC) { - CheckOther checkOther(_tokenizer, _settings, _errorLogger); - checkOther.doubleFreeError(tok, arg->str()); - } else if (var->second != dealloc) { - // mismatching allocation and deallocation - mismatchError(tok, arg->str()); - varInfo->erase(arg->varId()); - } else { - // deallocation - var->second = DEALLOC; - } - } else if (dealloc != NOALLOC) { - alloctype[arg->varId()] = DEALLOC; - } + changeAllocStatus(varInfo, allocation, tok, arg); } else if (Token::Match(arg, "%var% (")) { - functionCall(arg, varInfo, dealloc); + functionCall(arg, varInfo, allocation); } } } @@ -457,14 +487,14 @@ void CheckLeakAutoVar::functionCall(const Token *tok, VarInfo *varInfo, const in void CheckLeakAutoVar::leakIfAllocated(const Token *vartok, const VarInfo &varInfo) { - const std::map &alloctype = varInfo.alloctype; + const std::map &alloctype = varInfo.alloctype; const std::map &possibleUsage = varInfo.possibleUsage; - const std::map::const_iterator var = alloctype.find(vartok->varId()); - if (var != alloctype.end() && var->second != DEALLOC) { + const std::map::const_iterator var = alloctype.find(vartok->varId()); + if (var != alloctype.end() && var->second.status != VarInfo::DEALLOC) { const std::map::const_iterator use = possibleUsage.find(vartok->varId()); if (use == possibleUsage.end()) { - leakError(vartok, vartok->str(), var->second); + leakError(vartok, vartok->str(), var->second.type); } else { configurationInfo(vartok, use->second); } @@ -473,13 +503,13 @@ void CheckLeakAutoVar::leakIfAllocated(const Token *vartok, void CheckLeakAutoVar::ret(const Token *tok, const VarInfo &varInfo) { - const std::map &alloctype = varInfo.alloctype; + const std::map &alloctype = varInfo.alloctype; const std::map &possibleUsage = varInfo.possibleUsage; const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase(); - for (std::map::const_iterator it = alloctype.begin(); it != alloctype.end(); ++it) { + for (std::map::const_iterator it = alloctype.begin(); it != alloctype.end(); ++it) { // don't warn if variable is conditionally allocated - if (it->second != DEALLOC && varInfo.conditionalAlloc.find(it->first) != varInfo.conditionalAlloc.end()) + if (it->second.status != VarInfo::DEALLOC && varInfo.conditionalAlloc.find(it->first) != varInfo.conditionalAlloc.end()) continue; // don't warn if there is a reference of the variable @@ -504,14 +534,13 @@ void CheckLeakAutoVar::ret(const Token *tok, const VarInfo &varInfo) } // return deallocated pointer - if (used && it->second == DEALLOC) + if (used && it->second.status == VarInfo::DEALLOC) deallocReturnError(tok, var->name()); - else if (!used && it->second != DEALLOC) { - + else if (!used && it->second.status != VarInfo::DEALLOC) { const std::map::const_iterator use = possibleUsage.find(varid); if (use == possibleUsage.end()) { - leakError(tok, var->name(), it->second); + leakError(tok, var->name(), it->second.type); } else { configurationInfo(tok, use->second); } diff --git a/lib/checkleakautovar.h b/lib/checkleakautovar.h index 086730211..cfb218155 100644 --- a/lib/checkleakautovar.h +++ b/lib/checkleakautovar.h @@ -31,7 +31,13 @@ class CPPCHECKLIB VarInfo { public: - std::map alloctype; + enum AllocStatus { DEALLOC = -1, NOALLOC = 0, ALLOC = 1 }; + struct AllocInfo { + AllocStatus status; + int type; + AllocInfo(int type_ = 0, AllocStatus status_ = NOALLOC) : status(status_), type(type_) {} + }; + std::map alloctype; std::map possibleUsage; std::set conditionalAlloc; std::set referenced; @@ -98,7 +104,10 @@ private: std::set notzero); /** parse function call */ - void functionCall(const Token *tok, VarInfo *varInfo, const int dealloc); + void functionCall(const Token *tok, VarInfo *varInfo, const VarInfo::AllocInfo& allocation); + + /** parse changes in allocation status */ + void changeAllocStatus(VarInfo *varInfo, const VarInfo::AllocInfo& allocation, const Token* tok, const Token* arg); /** return. either "return" or end of variable scope is seen */ void ret(const Token *tok, const VarInfo &varInfo); @@ -110,6 +119,7 @@ private: void mismatchError(const Token* tok, const std::string &varname); void deallocUseError(const Token *tok, const std::string &varname); void deallocReturnError(const Token *tok, const std::string &varname); + void doubleFreeError(const Token *tok, const std::string &varname, int type); /** message: user configuration is needed to complete analysis */ void configurationInfo(const Token* tok, const std::string &functionName); @@ -118,6 +128,7 @@ private: CheckLeakAutoVar c(0, settings, errorLogger); c.deallocReturnError(0, "p"); c.configurationInfo(0, "f"); // user configuration is needed to complete analysis + c.doubleFreeError(0, "varname", 0); } static std::string myName() { @@ -125,7 +136,7 @@ private: } std::string classInfo() const { - return "Detect when a auto variable is allocated but not deallocated.\n"; + return "Detect when a auto variable is allocated but not deallocated or deallocated twice.\n"; } }; /// @} diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 3f86553da..f97445c7a 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -2127,122 +2127,6 @@ void CheckOther::invalidFreeError(const Token *tok, bool inconclusive) } -//----------------------------------------------------------------------------- -// Check for double free -// free(p); free(p); -//----------------------------------------------------------------------------- -void CheckOther::checkDoubleFree() -{ - std::set freedVariables; - std::set closeDirVariables; - - const SymbolDatabase* symbolDatabase = _tokenizer->getSymbolDatabase(); - const std::size_t functions = symbolDatabase->functionScopes.size(); - for (std::size_t i = 0; i < functions; ++i) { - freedVariables.clear(); - closeDirVariables.clear(); - const Scope * scope = symbolDatabase->functionScopes[i]; - for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) { - // Keep track of any variables passed to "free()", "g_free()" or "closedir()", - // and report an error if the same variable is passed twice. - if (Token::Match(tok, "free|g_free|closedir ( %var% )")) { - const unsigned int varId = tok->tokAt(2)->varId(); - if (varId) { - if (Token::Match(tok, "free|g_free")) { - if (freedVariables.find(varId) != freedVariables.end()) - doubleFreeError(tok, tok->strAt(2)); - else - freedVariables.insert(varId); - } else if (tok->str() == "closedir") { - if (closeDirVariables.find(varId) != closeDirVariables.end()) - doubleCloseDirError(tok, tok->strAt(2)); - else - closeDirVariables.insert(varId); - } - } - } - - // Keep track of any variables operated on by "delete" or "delete[]" - // and report an error if the same variable is delete'd twice. - else if (Token::Match(tok, "delete %var% ;") || Token::Match(tok, "delete [ ] %var% ;")) { - const int varIndex = (tok->strAt(1) == "[") ? 3 : 1; - const unsigned int varId = tok->tokAt(varIndex)->varId(); - if (varId) { - if (freedVariables.find(varId) != freedVariables.end()) - doubleFreeError(tok, tok->strAt(varIndex)); - else - freedVariables.insert(varId); - } - } - - // If this scope doesn't return, clear the set of previously freed variables - else if (tok->str() == "}" && _tokenizer->IsScopeNoReturn(tok)) { - freedVariables.clear(); - closeDirVariables.clear(); - } - - // If this scope is a "for" or "while" loop that contains "break" or "continue", - // give up on trying to figure out the flow of execution and just clear the set - // of previously freed variables. - // TODO: There are false negatives. This bailout is only needed when the - // loop will exit without free()'ing the memory on the last iteration. - else if (tok->str() == "}" && tok->link() && tok->link()->previous() && - tok->link()->linkAt(-1) && - Token::Match(tok->link()->linkAt(-1)->previous(), "while|for") && - Token::findmatch(tok->link()->linkAt(-1), "break|continue ;", tok) != nullptr) { - freedVariables.clear(); - closeDirVariables.clear(); - } - - // If a variable is passed to a function, remove it from the set of previously freed variables - else if (Token::Match(tok, "%var% (") && _settings->library.leakignore.find(tok->str()) == _settings->library.leakignore.end()) { - - // If this is a new function definition, clear all variables - if (Token::simpleMatch(tok->next()->link(), ") {")) { - freedVariables.clear(); - closeDirVariables.clear(); - } - // If it is a function call, then clear those variables in its argument list - else if (Token::simpleMatch(tok->next()->link(), ") ;")) { - for (const Token* tok2 = tok->tokAt(2); tok2 != tok->linkAt(1); tok2 = tok2->next()) { - const unsigned int varId = tok2->varId(); - if (varId) { - freedVariables.erase(varId); - closeDirVariables.erase(varId); - } - } - } - } - - // If a pointer is assigned a new value, remove it from the set of previously freed variables - else if (Token::Match(tok, "%var% =")) { - const unsigned int varId = tok->varId(); - if (varId) { - freedVariables.erase(varId); - closeDirVariables.erase(varId); - } - } - - // Any control statements in-between delete, free() or closedir() statements - // makes it unclear whether any subsequent statements would be redundant. - if (Token::Match(tok, "if|else|for|while|break|continue|goto|return|throw|switch")) { - freedVariables.clear(); - closeDirVariables.clear(); - } - } - } -} - -void CheckOther::doubleFreeError(const Token *tok, const std::string &varname) -{ - reportError(tok, Severity::error, "doubleFree", "Memory pointed to by '" + varname +"' is freed twice."); -} - -void CheckOther::doubleCloseDirError(const Token *tok, const std::string &varname) -{ - reportError(tok, Severity::error, "doubleCloseDir", "Directory handle '" + varname +"' closed twice."); -} - //--------------------------------------------------------------------------- // check for the same expression on both sides of an operator // (x == x), (x && x), (x || x) diff --git a/lib/checkother.h b/lib/checkother.h index 69b25e4ae..a8527ec2d 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -103,7 +103,6 @@ public: checkOther.checkPipeParameterSize(); checkOther.checkInvalidFree(); - checkOther.checkDoubleFree(); checkOther.checkRedundantCopy(); checkOther.checkNegativeBitwiseShift(); checkOther.checkSuspiciousEqualityComparison(); @@ -209,10 +208,6 @@ public: void checkInvalidFree(); void invalidFreeError(const Token *tok, bool inconclusive); - /** @brief %Check for double free or double close operations */ - void checkDoubleFree(); - void doubleFreeError(const Token *tok, const std::string &varname); - /** @brief %Check for code creating redundant copies */ void checkRedundantCopy(); @@ -291,7 +286,6 @@ private: void unsignedPositiveError(const Token *tok, const std::string &varname, bool inconclusive); void pointerPositiveError(const Token *tok, bool inconclusive); void SuspiciousSemicolonError(const Token *tok); - void doubleCloseDirError(const Token *tok, const std::string &varname); void negativeBitwiseShiftError(const Token *tok); void redundantCopyError(const Token *tok, const std::string &varname); void incompleteArrayFillError(const Token* tok, const std::string& buffer, const std::string& function, bool boolean); @@ -309,7 +303,6 @@ private: c.zerodivError(0, false); c.zerodivcondError(0,0,false); c.misusedScopeObjectError(NULL, "varname"); - c.doubleFreeError(0, "varname"); c.invalidPointerCastError(0, "float", "double", false); c.negativeBitwiseShiftError(0); c.checkPipeParameterSizeError(0, "varname", "dimension"); @@ -371,7 +364,6 @@ private: "- scoped object destroyed immediately after construction\n" "- assignment in an assert statement\n" "- free() or delete of an invalid memory location\n" - "- double free() or double closedir()\n" "- bitwise operation with negative right operand\n" "- provide wrong dimensioned array to pipe() system command (--std=posix)\n" "- cast the return values of getc(),fgetc() and getchar() to character and compare it to EOF\n" diff --git a/test/testleakautovar.cpp b/test/testleakautovar.cpp index 4060b4ebb..74733ad1a 100644 --- a/test/testleakautovar.cpp +++ b/test/testleakautovar.cpp @@ -113,7 +113,7 @@ private: TEST_CASE(nestedAllocation); } - void check(const char code[]) { + void check(const char code[], bool cpp = false) { // Clear the error buffer.. errout.str(""); @@ -121,39 +121,14 @@ private: Settings settings; int id = 0; while (!settings.library.ismemory(++id)); - settings.library.setalloc("malloc",id); - settings.library.setdealloc("free",id); + settings.library.setalloc("malloc", id); + settings.library.setdealloc("free", id); while (!settings.library.isresource(++id)); - settings.library.setalloc("fopen",id); - settings.library.setdealloc("fclose",id); + settings.library.setalloc("fopen", id); + settings.library.setdealloc("fclose", id); Tokenizer tokenizer(&settings, this); std::istringstream istr(code); - tokenizer.tokenize(istr, "test.c"); - tokenizer.simplifyTokenList2(); - - // Check for leaks.. - CheckLeakAutoVar c; - settings.checkLibrary = true; - settings.addEnabled("information"); - c.runSimplifiedChecks(&tokenizer, &settings, this); - } - - void checkcpp(const char code[]) { - // Clear the error buffer.. - errout.str(""); - - // Tokenize.. - Settings settings; - int id = 0; - while (!settings.library.ismemory(++id)); - settings.library.setalloc("malloc",id); - settings.library.setdealloc("free",id); - while (!settings.library.isresource(++id)); - settings.library.setalloc("fopen",id); - settings.library.setdealloc("fclose",id); - Tokenizer tokenizer(&settings, this); - std::istringstream istr(code); - tokenizer.tokenize(istr, "test.c.cpp"); + tokenizer.tokenize(istr, cpp?"test.cpp":"test.c"); tokenizer.simplifyTokenList2(); // Check for leaks.. @@ -349,6 +324,391 @@ private: " free(p);\n" "}"); ASSERT_EQUALS("[test.c:6]: (error) Memory pointed to by 'p' is freed twice.\n", errout.str()); + + check( + "void foo(char *p) {\n" + " free(p);\n" + " free(p);\n" + "}"); + ASSERT_EQUALS("[test.c:3]: (error) Memory pointed to by 'p' is freed twice.\n", errout.str()); + + check( + "void foo(char *p, char *r) {\n" + " free(p);\n" + " free(r);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check( + "void foo() {\n" + " free(p);\n" + " free(r);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check( + "void foo(char *p) {\n" + " if (x < 3) free(p);\n" + " else { if (x > 9) free(p); }\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check( + "void foo(char *p) {\n" + " free(p);\n" + " getNext(&p);\n" + " free(p);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check( + "void foo(char *p) {\n" + " free(p);\n" + " bar();\n" + " free(p);\n" + "}"); + ASSERT_EQUALS("[test.c:4]: (error) Memory pointed to by 'p' is freed twice.\n", errout.str()); + + check( + "void foo(char *p) {\n" + " free(p);\n" + " printf(\"Freed memory at location %x\", p);\n" + " free(p);\n" + "}"); + ASSERT_EQUALS("[test.c:4]: (error) Memory pointed to by 'p' is freed twice.\n", errout.str()); + + check( + "void foo(FILE *p) {\n" + " fclose(p);\n" + " fclose(p);\n" + "}"); + ASSERT_EQUALS("[test.c:3]: (error) Resource handle 'p' freed twice.\n", errout.str()); + + check( + "void foo(FILE *p, FILE *r) {\n" + " fclose(p);\n" + " fclose(r);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check( + "void foo(FILE *p) {\n" + " if (x < 3) fclose(p);\n" + " else { if (x > 9) fclose(p); }\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check( + "void foo(FILE *p) {\n" + " fclose(p);\n" + " gethandle(&p);\n" + " fclose(p);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check( + "void foo(FILE *p) {\n" + " fclose(p);\n" + " gethandle();\n" + " fclose(p);\n" + "}"); + ASSERT_EQUALS("[test.c:4]: (error) Resource handle 'p' freed twice.\n", errout.str()); + + check( + "void foo(Data* p) {\n" + " free(p->a);\n" + " free(p->b);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check( + "void f() {\n" + " char *p; p = malloc(100);\n" + " if (x) {\n" + " free(p);\n" + " exit();\n" + " }\n" + " free(p);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check( + "void f() {\n" + " char *p; p = malloc(100);\n" + " if (x) {\n" + " free(p);\n" + " x = 0;\n" + " }\n" + " free(p);\n" + "}"); + ASSERT_EQUALS("[test.c:7]: (error) Memory pointed to by 'p' is freed twice.\n", errout.str()); + + check( + "void f() {\n" + " char *p; p = do_something();\n" + " free(p);\n" + " p = do_something();\n" + " free(p);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check( + "void foo(char *p) {\n" + " delete p;\n" + " delete p;\n" + "}", true); + ASSERT_EQUALS("[test.cpp:3]: (error) Memory pointed to by 'p' is freed twice.\n", errout.str()); + + check( + "void foo(char *p, char *r) {\n" + " delete p;\n" + " delete r;\n" + "}", true); + ASSERT_EQUALS("", errout.str()); + + check( + "void foo(P p) {\n" + " delete p.x;\n" + " delete p;\n" + "}", true); + ASSERT_EQUALS("", errout.str()); + + check( + "void foo(char *p) {\n" + " delete p;\n" + " getNext(&p);\n" + " delete p;\n" + "}", true); + ASSERT_EQUALS("", errout.str()); + + check( + "void foo(char *p) {\n" + " delete p;\n" + " bar();\n" + " delete p;\n" + "}", true); + ASSERT_EQUALS("[test.cpp:4]: (error) Memory pointed to by 'p' is freed twice.\n", errout.str()); + + check( + "void foo(char *p) {\n" + " delete[] p;\n" + " delete[] p;\n" + "}", true); + ASSERT_EQUALS("[test.cpp:3]: (error) Memory pointed to by 'p' is freed twice.\n", errout.str()); + + check( + "void foo(char *p, char *r) {\n" + " delete[] p;\n" + " delete[] r;\n" + "}", true); + ASSERT_EQUALS("", errout.str()); + + check( + "void foo(char *p) {\n" + " delete[] p;\n" + " getNext(&p);\n" + " delete[] p;\n" + "}", true); + ASSERT_EQUALS("", errout.str()); + + check( + "void foo(char *p) {\n" + " delete[] p;\n" + " bar();\n" + " delete[] p;\n" + "}", true); + ASSERT_EQUALS("[test.cpp:4]: (error) Memory pointed to by 'p' is freed twice.\n", errout.str()); + + check( + "~LineMarker() {\n" + " delete pxpm;\n" + "}\n" + "LineMarker &operator=(const LineMarker &) {\n" + " delete pxpm;\n" + " pxpm = NULL;\n" + " return *this;\n" + "}", true); + ASSERT_EQUALS("", errout.str()); + + check( + "void foo()\n" + "{\n" + " int* ptr; ptr = NULL;\n" + " try\n" + " {\n" + " ptr = new int(4);\n" + " }\n" + " catch(...)\n" + " {\n" + " delete ptr;\n" + " throw;\n" + " }\n" + " delete ptr;\n" + "}", true); + ASSERT_EQUALS("", errout.str()); + + check( + "int foo()\n" + "{\n" + " int* a; a = new int;\n" + " bool doDelete; doDelete = true;\n" + " if (a != 0)\n" + " {\n" + " doDelete = false;\n" + " delete a;\n" + " }\n" + " if(doDelete)\n" + " delete a;\n" + " return 0;\n" + "}", true); + TODO_ASSERT_EQUALS("", "[test.cpp:11]: (error) Memory pointed to by 'a' is freed twice.\n", errout.str()); + + check( + "void foo(int y)\n" + "{\n" + " char * x; x = NULL;\n" + " while(true) {\n" + " x = new char[100];\n" + " if (y++ > 100)\n" + " break;\n" + " delete[] x;\n" + " }\n" + " delete[] x;\n" + "}", true); + ASSERT_EQUALS("", errout.str()); + + check( + "void foo(int y)\n" + "{\n" + " char * x; x = NULL;\n" + " for (int i = 0; i < 10000; i++) {\n" + " x = new char[100];\n" + " delete[] x;\n" + " }\n" + " delete[] x;\n" + "}", true); + TODO_ASSERT_EQUALS("[test.cpp:8]: (error) Memory pointed to by 'x' is freed twice.\n", "", errout.str()); + + check( + "void foo(int y)\n" + "{\n" + " char * x; x = NULL;\n" + " while (isRunning()) {\n" + " x = new char[100];\n" + " delete[] x;\n" + " }\n" + " delete[] x;\n" + "}", true); + TODO_ASSERT_EQUALS("[test.cpp:8]: (error) Memory pointed to by 'x' is freed twice.\n", "", errout.str()); + + check( + "void foo(int y)\n" + "{\n" + " char * x; x = NULL;\n" + " while (isRunning()) {\n" + " x = malloc(100);\n" + " free(x);\n" + " }\n" + " free(x);\n" + "}"); + TODO_ASSERT_EQUALS("[test.c:8]: (error) Memory pointed to by 'x' is freed twice.\n", "", errout.str()); + + check( + "void foo(int y)\n" + "{\n" + " char * x; x = NULL;\n" + " for (;;) {\n" + " x = new char[100];\n" + " if (y++ > 100)\n" + " break;\n" + " delete[] x;\n" + " }\n" + " delete[] x;\n" + "}", true); + ASSERT_EQUALS("", errout.str()); + + check( + "void foo(int y)\n" + "{\n" + " char * x; x = NULL;\n" + " do {\n" + " x = new char[100];\n" + " if (y++ > 100)\n" + " break;\n" + " delete[] x;\n" + " } while (true);\n" + " delete[] x;\n" + "}", true); + ASSERT_EQUALS("", errout.str()); + + check( + "void f()\n" + "{\n" + " char *p; p = 0;\n" + " if (x < 100) {\n" + " p = malloc(10);\n" + " free(p);\n" + " }\n" + " free(p);\n" + "}"); + ASSERT_EQUALS("[test.c:8]: (error) Memory pointed to by 'p' is freed twice.\n", errout.str()); + + check( + "void MyFunction()\n" + "{\n" + " char* data; data = new char[100];\n" + " try\n" + " {\n" + " }\n" + " catch(err)\n" + " {\n" + " delete[] data;\n" + " MyThrow(err);\n" + " }\n" + " delete[] data;\n" + "}\n" + + "void MyThrow(err)\n" + "{\n" + " throw(err);\n" + "}", true); + ASSERT_EQUALS("", errout.str()); + + check( + "void MyFunction()\n" + "{\n" + " char* data; data = new char[100];\n" + " try\n" + " {\n" + " }\n" + " catch(err)\n" + " {\n" + " delete[] data;\n" + " MyExit(err);\n" + " }\n" + " delete[] data;\n" + "}\n" + + "void MyExit(err)\n" + "{\n" + " exit(err);\n" + "}", true); + ASSERT_EQUALS("", errout.str()); + + check( // #6252 + "struct Wrapper {\n" + " Thing* m_thing;\n" + " Wrapper() : m_thing(0) {\n" + " }\n" + " ~Wrapper() {\n" + " delete m_thing;\n" + " }\n" + " void changeThing() {\n" + " delete m_thing;\n" + " m_thing = new Thing;\n" + " }\n" + "};", true); + ASSERT_EQUALS("", errout.str()); } void doublefree2() { // #3891 @@ -649,10 +1009,10 @@ private: } void test5() { // unknown type - checkcpp("void f() { Fred *p = malloc(10); }"); + check("void f() { Fred *p = malloc(10); }", true); ASSERT_EQUALS("", errout.str()); - check("void f() { Fred *p = malloc(10); }"); + check("void f() { Fred *p = malloc(10); }", false); ASSERT_EQUALS("[test.c:1]: (error) Memory leak: p\n", errout.str()); } diff --git a/test/testother.cpp b/test/testother.cpp index d3622ce59..6293ca71b 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -148,7 +148,6 @@ private: TEST_CASE(checkForSuspiciousSemicolon1); TEST_CASE(checkForSuspiciousSemicolon2); - TEST_CASE(checkDoubleFree); TEST_CASE(checkInvalidFree); TEST_CASE(checkRedundantCopy); @@ -4842,429 +4841,6 @@ private: ASSERT_EQUALS("", errout.str()); } - void checkDoubleFree() { - check( - "void foo(char *p) {\n" - " free(p);\n" - " free(p);\n" - "}"); - ASSERT_EQUALS("[test.cpp:3]: (error) Memory pointed to by 'p' is freed twice.\n", errout.str()); - - check( - "void foo(char *p, char *r) {\n" - " free(p);\n" - " free(r);\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - check( - "void foo() {\n" - " free(p);\n" - " free(r);\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - check( - "void foo(char *p) {\n" - " if (x < 3) free(p);\n" - " else { if (x > 9) free(p); }\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - check( - "void foo(char *p) {\n" - " free(p);\n" - " getNext(&p);\n" - " free(p);\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - check( - "void foo(char *p) {\n" - " free(p);\n" - " bar();\n" - " free(p);\n" - "}"); - ASSERT_EQUALS("[test.cpp:4]: (error) Memory pointed to by 'p' is freed twice.\n", errout.str()); - - check( - "void foo(char *p) {\n" - " free(p);\n" - " printf(\"Freed memory at location %x\", p);\n" - " free(p);\n" - "}", nullptr, false, false, false, true, &settings_std); - ASSERT_EQUALS("[test.cpp:4]: (error) Memory pointed to by 'p' is freed twice.\n", errout.str()); - - check( - "void foo(DIR *p) {\n" - " closedir(p);\n" - " closedir(p);\n" - "}"); - ASSERT_EQUALS("[test.cpp:3]: (error) Directory handle 'p' closed twice.\n", errout.str()); - - check( - "void foo(DIR *p, DIR *r) {\n" - " closedir(p);\n" - " closedir(r);\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - check( - "void foo(DIR *p) {\n" - " if (x < 3) closedir(p);\n" - " else { if (x > 9) closedir(p); }\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - check( - "void foo(DIR *p) {\n" - " closedir(p);\n" - " gethandle(&p);\n" - " closedir(p);\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - check( - "void foo(DIR *p) {\n" - " closedir(p);\n" - " gethandle();\n" - " closedir(p);\n" - "}"); - ASSERT_EQUALS("[test.cpp:4]: (error) Directory handle 'p' closed twice.\n", errout.str()); - - check( - "void foo(Data* p) {\n" - " free(p->a);\n" - " free(p->b);\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - check( - "void f() {\n" - " char *p; p = malloc(100);\n" - " if (x) {\n" - " free(p);\n" - " exit();\n" - " }\n" - " free(p);\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - check( - "void f() {\n" - " char *p; p = malloc(100);\n" - " if (x) {\n" - " free(p);\n" - " x = 0;\n" - " }\n" - " free(p);\n" - "}"); - ASSERT_EQUALS("[test.cpp:7]: (error) Memory pointed to by 'p' is freed twice.\n", errout.str()); - - check( - "void f() {\n" - " char *p; p = do_something();\n" - " free(p);\n" - " p = do_something();\n" - " free(p);\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - check( - "void foo(char *p) {\n" - " g_free(p);\n" - " g_free(p);\n" - "}"); - ASSERT_EQUALS("[test.cpp:3]: (error) Memory pointed to by 'p' is freed twice.\n", errout.str()); - - check( - "void foo(char *p, char *r) {\n" - " g_free(p);\n" - " g_free(r);\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - check( - "void foo(char *p) {\n" - " g_free(p);\n" - " getNext(&p);\n" - " g_free(p);\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - check( - "void foo(char *p) {\n" - " g_free(p);\n" - " bar();\n" - " g_free(p);\n" - "}"); - ASSERT_EQUALS("[test.cpp:4]: (error) Memory pointed to by 'p' is freed twice.\n", errout.str()); - - check( - "void foo(char *p) {\n" - " delete p;\n" - " delete p;\n" - "}"); - ASSERT_EQUALS("[test.cpp:3]: (error) Memory pointed to by 'p' is freed twice.\n", errout.str()); - - check( - "void foo(char *p, char *r) {\n" - " delete p;\n" - " delete r;\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - check( - "void foo(char *p) {\n" - " delete p;\n" - " getNext(&p);\n" - " delete p;\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - check( - "void foo(char *p) {\n" - " delete p;\n" - " bar();\n" - " delete p;\n" - "}"); - ASSERT_EQUALS("[test.cpp:4]: (error) Memory pointed to by 'p' is freed twice.\n", errout.str()); - - check( - "void foo(char *p) {\n" - " delete[] p;\n" - " delete[] p;\n" - "}"); - ASSERT_EQUALS("[test.cpp:3]: (error) Memory pointed to by 'p' is freed twice.\n", errout.str()); - - check( - "void foo(char *p, char *r) {\n" - " delete[] p;\n" - " delete[] r;\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - check( - "void foo(char *p) {\n" - " delete[] p;\n" - " getNext(&p);\n" - " delete[] p;\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - check( - "void foo(char *p) {\n" - " delete[] p;\n" - " bar();\n" - " delete[] p;\n" - "}"); - ASSERT_EQUALS("[test.cpp:4]: (error) Memory pointed to by 'p' is freed twice.\n", errout.str()); - - check( - "~LineMarker() {\n" - " delete pxpm;\n" - "}\n" - "LineMarker &operator=(const LineMarker &) {\n" - " delete pxpm;\n" - " pxpm = NULL;\n" - " return *this;\n" - "}" - ); - ASSERT_EQUALS("", errout.str()); - - check( - "void foo()\n" - "{\n" - " int* ptr; ptr = NULL;\n" - " try\n" - " {\n" - " ptr = new int(4);\n" - " }\n" - " catch(...)\n" - " {\n" - " delete ptr;\n" - " throw;\n" - " }\n" - " delete ptr;\n" - "}" - ); - ASSERT_EQUALS("", errout.str()); - - check( - "int foo()\n" - "{\n" - " int* a; a = new int;\n" - " bool doDelete; doDelete = true;\n" - " if (a != 0)\n" - " {\n" - " doDelete = false;\n" - " delete a;\n" - " }\n" - " if(doDelete)\n" - " delete a;\n" - " return 0;\n" - "}" - ); - ASSERT_EQUALS("", errout.str()); - - check( - "void foo(int y)\n" - "{\n" - " char * x; x = NULL;\n" - " while(true) {\n" - " x = new char[100];\n" - " if (y++ > 100)\n" - " break;\n" - " delete[] x;\n" - " }\n" - " delete[] x;\n" - "}" - ); - ASSERT_EQUALS("", errout.str()); - - check( - "void foo(int y)\n" - "{\n" - " char * x; x = NULL;\n" - " for (int i = 0; i < 10000; i++) {\n" - " x = new char[100];\n" - " delete[] x;\n" - " }\n" - " delete[] x;\n" - "}" - ); - ASSERT_EQUALS("[test.cpp:8]: (error) Memory pointed to by 'x' is freed twice.\n", errout.str()); - - check( - "void foo(int y)\n" - "{\n" - " char * x; x = NULL;\n" - " while (isRunning()) {\n" - " x = new char[100];\n" - " delete[] x;\n" - " }\n" - " delete[] x;\n" - "}" - ); - ASSERT_EQUALS("[test.cpp:8]: (error) Memory pointed to by 'x' is freed twice.\n", errout.str()); - - check( - "void foo(int y)\n" - "{\n" - " char * x; x = NULL;\n" - " while (isRunning()) {\n" - " x = malloc(100);\n" - " free(x);\n" - " }\n" - " free(x);\n" - "}" - ); - TODO_ASSERT_EQUALS("[test.cpp:8]: (error) Memory pointed to by 'x' is freed twice.\n", "", errout.str()); - - check( - "void foo(int y)\n" - "{\n" - " char * x; x = NULL;\n" - " for (;;) {\n" - " x = new char[100];\n" - " if (y++ > 100)\n" - " break;\n" - " delete[] x;\n" - " }\n" - " delete[] x;\n" - "}" - ); - ASSERT_EQUALS("", errout.str()); - - check( - "void foo(int y)\n" - "{\n" - " char * x; x = NULL;\n" - " do {\n" - " x = new char[100];\n" - " if (y++ > 100)\n" - " break;\n" - " delete[] x;\n" - " } while (true);\n" - " delete[] x;\n" - "}" - ); - ASSERT_EQUALS("", errout.str()); - - check( - "void f()\n" - "{\n" - " char *p; p = 0;\n" - " if (x < 100) {\n" - " p = malloc(10);\n" - " free(p);\n" - " }\n" - " free(p);\n" - "}" - ); - TODO_ASSERT_EQUALS("[test.cpp:8]: (error) Memory pointed to by 'p' is freed twice.\n", "", errout.str()); - - check( - "void MyFunction()\n" - "{\n" - " char* data; data = new char[100];\n" - " try\n" - " {\n" - " }\n" - " catch(err)\n" - " {\n" - " delete[] data;\n" - " MyThrow(err);\n" - " }\n" - " delete[] data;\n" - "}\n" - - "void MyThrow(err)\n" - "{\n" - " throw(err);\n" - "}\n" - ); - ASSERT_EQUALS("", errout.str()); - - check( - "void MyFunction()\n" - "{\n" - " char* data; data = new char[100];\n" - " try\n" - " {\n" - " }\n" - " catch(err)\n" - " {\n" - " delete[] data;\n" - " MyExit(err);\n" - " }\n" - " delete[] data;\n" - "}\n" - - "void MyExit(err)\n" - "{\n" - " exit(err);\n" - "}\n" - ); - ASSERT_EQUALS("", errout.str()); - - check( // #6252 - "struct Wrapper {\n" - " Thing* m_thing;\n" - " Wrapper() : m_thing(0) {\n" - " }\n" - " ~Wrapper() {\n" - " delete m_thing;\n" - " }\n" - " void changeThing() {\n" - " delete m_thing;\n" - " m_thing = new Thing;\n" - " }\n" - "};\n" - ); - ASSERT_EQUALS("", errout.str()); - } - void checkInvalidFree() { check(