Fix #10182 (FN memory leak with if-statement) (#3151)

Improve leak detections in if-statements. This is done by checking
for leaks every time a scope is left. This allows cppcheck to catch
more memory leaks, as well as improve some error messages which now
contain the line where the variable goes out of scope, instead of the
end of the function.

lib/checkleakautovar.cpp

@@ -195,22 +195,6 @@ void CheckLeakAutoVar::check()
         VarInfo varInfo;
 
         checkScope(scope->bodyStart, &varInfo, notzero, 0);
-
-        varInfo.conditionalAlloc.clear();
-
-        // Clear reference arguments from varInfo..
-        std::map<int, VarInfo::AllocInfo>::iterator it = varInfo.alloctype.begin();
-        while (it != varInfo.alloctype.end()) {
-            const Variable *var = symbolDatabase->getVariableFromVarId(it->first);
-            if (!var ||
-                (var->isArgument() && var->isReference()) ||
-                (!var->isArgument() && !var->isLocal()))
-                varInfo.alloctype.erase(it++);
-            else
-                ++it;
-        }
-
-        ret(scope->bodyEnd, varInfo);
     }
 }
 
@@ -741,6 +725,7 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken,
             changeAllocStatus(varInfo, allocation, vtok, vtok);
         }
     }
+    ret(endToken, *varInfo, true);
 }
 
 
@@ -973,15 +958,16 @@ void CheckLeakAutoVar::leakIfAllocated(const Token *vartok,
     }
 }
 
-void CheckLeakAutoVar::ret(const Token *tok, const VarInfo &varInfo)
+void CheckLeakAutoVar::ret(const Token *tok, VarInfo &varInfo, const bool isEndOfScope)
 {
     const std::map<int, VarInfo::AllocInfo> &alloctype = varInfo.alloctype;
     const std::map<int, std::string> &possibleUsage = varInfo.possibleUsage;
+    std::vector<int> toRemove;
 
     const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase();
     for (std::map<int, VarInfo::AllocInfo>::const_iterator it = alloctype.begin(); it != alloctype.end(); ++it) {
-        // don't warn if variable is conditionally allocated
-        if (!it->second.managed() && varInfo.conditionalAlloc.find(it->first) != varInfo.conditionalAlloc.end())
+        // don't warn if variable is conditionally allocated, unless it leaves the scope
+        if (!isEndOfScope && !it->second.managed() && varInfo.conditionalAlloc.find(it->first) != varInfo.conditionalAlloc.end())
             continue;
 
         // don't warn if there is a reference of the variable
@@ -991,6 +977,9 @@ void CheckLeakAutoVar::ret(const Token *tok, const VarInfo &varInfo)
         const int varid = it->first;
         const Variable *var = symbolDatabase->getVariableFromVarId(varid);
         if (var) {
+            // don't warn if we leave an inner scope
+            if (isEndOfScope && var->scope() && tok != var->scope()->bodyEnd)
+                continue;
             bool used = false;
             for (const Token *tok2 = tok; tok2; tok2 = tok2->next()) {
                 if (tok2->str() == ";")
@@ -1028,6 +1017,9 @@ void CheckLeakAutoVar::ret(const Token *tok, const VarInfo &varInfo)
                     configurationInfo(tok, use->second);
                 }
             }
+            toRemove.push_back(varid);
         }
     }
+    for (int varId : toRemove)
+        varInfo.erase(varId);
 }

lib/checkleakautovar.h

@@ -149,7 +149,7 @@ private:
     void changeAllocStatusIfRealloc(std::map<int, VarInfo::AllocInfo> &alloctype, const Token *fTok, const Token *retTok);
 
     /** return. either "return" or end of variable scope is seen */
-    void ret(const Token *tok, const VarInfo &varInfo);
+    void ret(const Token *tok, VarInfo &varInfo, const bool isEndOfScope = false);
 
     /** if variable is allocated then there is a leak */
     void leakIfAllocated(const Token *vartok, const VarInfo &varInfo);

test/testleakautovar.cpp

@@ -145,6 +145,8 @@ private:
         TEST_CASE(ifelse17); //  if (!!(!p))
         TEST_CASE(ifelse18);
         TEST_CASE(ifelse19);
+        TEST_CASE(ifelse20); // #10182
+        TEST_CASE(ifelse21);
 
         // switch
         TEST_CASE(switch1);
@@ -1611,6 +1613,39 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
+    void ifelse20() {
+        check("void f() {\n"
+              "    if (x > 0)\n"
+              "        void * p1 = malloc(5);\n"
+              "    else\n"
+              "        void * p2 = malloc(2);\n"
+              "    return;\n"
+              "}");
+        ASSERT_EQUALS("[test.c:3]: (error) Memory leak: p1\n"
+                      "[test.c:5]: (error) Memory leak: p2\n", errout.str());
+
+        check("void f() {\n"
+              "    if (x > 0)\n"
+              "        void * p1 = malloc(5);\n"
+              "    else\n"
+              "        void * p2 = malloc(2);\n"
+              "}");
+        ASSERT_EQUALS("[test.c:3]: (error) Memory leak: p1\n"
+                      "[test.c:5]: (error) Memory leak: p2\n", errout.str());
+    }
+
+    void ifelse21() {
+        check("void f() {\n"
+              "    if (y) {\n"
+              "        void * p;\n"
+              "        if (x > 0)\n"
+              "            p = malloc(5);\n"
+              "    }\n"
+              "    return;\n"
+              "}");
+        ASSERT_EQUALS("[test.c:6]: (error) Memory leak: p\n",  errout.str());
+    }
+
     void switch1() {
         check("void f() {\n"
               "    char *p = 0;\n"