From 94031ef11d1821c4974e859c8df7bf5f2a1727ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rge=20Dijkstra?= Date: Thu, 26 Oct 2017 19:11:00 +0200 Subject: [PATCH] Fix for conditional memory allocation inside if-condition (#986) * Add test cases for allocation inside if-condition * Fix missed memory leak and false positive double free for allocation inside if-condition --- lib/checkleakautovar.cpp | 18 ++++++++++++++++++ test/testleakautovar.cpp | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/lib/checkleakautovar.cpp b/lib/checkleakautovar.cpp index 51ccf7c04..82d6d9762 100644 --- a/lib/checkleakautovar.cpp +++ b/lib/checkleakautovar.cpp @@ -325,6 +325,24 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken, else if (Token::simpleMatch(tok, "if (")) { // Parse function calls inside the condition for (const Token *innerTok = tok->tokAt(2); innerTok; innerTok = innerTok->next()) { + if (Token::Match(innerTok, "%var% =")) { + // allocation? + if (Token::Match(innerTok->tokAt(2), "%type% (")) { + const Library::AllocFunc* f = _settings->library.alloc(innerTok->tokAt(2)); + if (f && f->arg == -1) { + VarInfo::AllocInfo& varAlloc = alloctype[innerTok->varId()]; + varAlloc.type = f->groupId; + varAlloc.status = VarInfo::ALLOC; + } + } else if (_tokenizer->isCPP() && Token::Match(innerTok->tokAt(2), "new !!(")) { + const Token* tok2 = innerTok->tokAt(2)->astOperand1(); + bool arrayNew = (tok2 && (tok2->str() == "[" || (tok2->str() == "(" && tok2->astOperand1() && tok2->astOperand1()->str() == "["))); + VarInfo::AllocInfo& varAlloc = alloctype[innerTok->varId()]; + varAlloc.type = arrayNew ? -2 : -1; + varAlloc.status = VarInfo::ALLOC; + } + } + if (innerTok->str() == ")") break; if (innerTok->str() == "(" && innerTok->previous()->isName()) { diff --git a/test/testleakautovar.cpp b/test/testleakautovar.cpp index 2ffa820ff..514c56c78 100644 --- a/test/testleakautovar.cpp +++ b/test/testleakautovar.cpp @@ -55,6 +55,7 @@ private: TEST_CASE(assign11); // #3942: x = a(b(p)); TEST_CASE(assign12); // #4236: FP. bar(&x); TEST_CASE(assign13); // #4237: FP. char*&ref=p; p=malloc(10); free(ref); + TEST_CASE(assign14); TEST_CASE(deallocuse1); TEST_CASE(deallocuse2); @@ -70,6 +71,7 @@ private: TEST_CASE(doublefree4); // #5451 - FP when exit is called TEST_CASE(doublefree5); // #5522 TEST_CASE(doublefree6); // #7685 + TEST_CASE(doublefree7); // exit TEST_CASE(exit1); @@ -269,6 +271,20 @@ private: ASSERT_EQUALS("", errout.str()); } + void assign14() { + check("void f(int x) {\n" + " char *p;\n" + " if (x && (p = malloc(10))) { }" + "}"); + ASSERT_EQUALS("[test.c:3]: (error) Memory leak: p\n", errout.str()); + + check("void f(int x) {\n" + " char *p;\n" + " if (x && (p = new char[10])) { }" + "}", true); + ASSERT_EQUALS("[test.cpp:3]: (error) Memory leak: p\n", errout.str()); + } + void deallocuse1() { check("void f(char *p) {\n" " free(p);\n" @@ -857,6 +873,22 @@ private: ASSERT_EQUALS("", errout.str()); } + void doublefree7() { + check("void f(char *p, int x) {\n" + " free(p);\n" + " if (x && (p = malloc(10)))\n" + " free(p);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void f(char *p, int x) {\n" + " delete[] p;\n" + " if (x && (p = new char[10]))\n" + " delete[] p;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + } + void exit1() { check("void f() {\n" " char *p = malloc(10);\n"