Fix for conditional memory allocation inside if-condition (danmar#986)

bjd · danmar · commit 94031ef11d18 · 2017-10-26T19:11:00.000+02:00
* Add test cases for allocation inside if-condition

* Fix missed memory leak and false positive double free for allocation inside if-condition
diff --git 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
@@ -55,6 +55,7 @@ class TestLeakAutoVar : public TestFixture {
         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 @@ class TestLeakAutoVar : public TestFixture {
         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 @@ class TestLeakAutoVar : public TestFixture {
         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 @@ class TestLeakAutoVar : public TestFixture {
         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"
