From abea580b78cf428673b57aa9b5da79f0692fe2fc Mon Sep 17 00:00:00 2001
From: Rikard Falkeborn <rikard.falkeborn@gmail.com>
Date: Thu, 18 Jul 2019 15:23:19 +0200
Subject: [PATCH] Fix FP memory leak with unknown function call in condition
 (#2012)

* Fix FP memory leak with unknown function call in condition

This was introduced in 8513fb81d261fe62a822755f22ae4c24e112ae25 when
fixing memory leaks for global variables allocated in condition. The
refactored code had an inconsistency where c and c++ code behaved
slightly differently when `var` is NULL. This seemed to not have an
impact as the code was written prior to 8513fb81d261fe62a822755f22ae4c24e112ae25,
but when the same code was used for conditions, FPs were introduced.

The introduced FPs were memleak warnings when there should have been an
information message about missing configurations for code like

	void f() {
		char *p = malloc(10);
		if (set_data(p)) {}
	}

Fix this by always returning true if varTok->Variable() is NULL for
both c and c++ code.

* Improve function name
---
 lib/checkleakautovar.cpp | 14 +++++++-------
 test/testleakautovar.cpp | 24 ++++++++++++++++--------
 2 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/lib/checkleakautovar.cpp b/lib/checkleakautovar.cpp
index 60ce8dc8a..0b5edd594 100644
--- a/lib/checkleakautovar.cpp
+++ b/lib/checkleakautovar.cpp
@@ -229,21 +229,21 @@ static bool isPointerReleased(const Token *startToken, const Token *endToken, no
     return false;
 }
 
-static bool checkVariable(const Token *varTok, const bool isCpp)
+static bool isLocalVarNoAutoDealloc(const Token *varTok, const bool isCpp)
 {
     // not a local variable nor argument?
     const Variable *var = varTok->variable();
-    if (var && !var->isArgument() && (!var->isLocal() || var->isStatic()))
+    if (!var)
+        return true;
+    if (!var->isArgument() && (!var->isLocal() || var->isStatic()))
         return false;
 
     // Don't check reference variables
-    if (var && var->isReference())
+    if (var->isReference())
         return false;
 
     // non-pod variable
     if (isCpp) {
-        if (!var)
-            return false;
         // Possibly automatically deallocated memory
         if (isAutoDealloc(var) && Token::Match(varTok, "%var% = new"))
             return false;
@@ -365,7 +365,7 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken,
                 leakIfAllocated(varTok, *varInfo);
             varInfo->erase(varTok->varId());
 
-            if (!checkVariable(varTok, mTokenizer->isCPP()))
+            if (!isLocalVarNoAutoDealloc(varTok, mTokenizer->isCPP()))
                 continue;
 
             // allocation?
@@ -405,7 +405,7 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken,
             for (const Token *innerTok = tok->tokAt(2); innerTok && innerTok != closingParenthesis; innerTok = innerTok->next()) {
                 // TODO: replace with checkTokenInsideExpression()
 
-                if (!checkVariable(innerTok, mTokenizer->isCPP()))
+                if (!isLocalVarNoAutoDealloc(innerTok, mTokenizer->isCPP()))
                     continue;
 
                 if (Token::Match(innerTok, "%var% =") && innerTok->astParent() == innerTok->next()) {
diff --git a/test/testleakautovar.cpp b/test/testleakautovar.cpp
index 610cd966d..dedde87e5 100644
--- a/test/testleakautovar.cpp
+++ b/test/testleakautovar.cpp
@@ -1767,19 +1767,27 @@ private:
     }
 
     void configuration3() {
-        check("void f() {\n"
-              "    char *p = malloc(10);\n"
-              "    if (set_data(p)) { }\n"
-              "}");
+        const char * code = "void f() {\n"
+                            "    char *p = malloc(10);\n"
+                            "    if (set_data(p)) { }\n"
+                            "}";
+        check(code);
         ASSERT_EQUALS("[test.c:4]: (information) --check-library: Function set_data() should have <use>/<leak-ignore> configuration\n", errout.str());
+        check(code, true);
+        ASSERT_EQUALS("[test.cpp:4]: (information) --check-library: Function set_data() should have <use>/<leak-ignore> configuration\n", errout.str());
 
-        check("void f() {\n"
-              "    char *p = malloc(10);\n"
-              "    if (set_data(p)) { return; }\n"
-              "}");
+        code = "void f() {\n"
+               "    char *p = malloc(10);\n"
+               "    if (set_data(p)) { return; }\n"
+               "}";
+        check(code);
         ASSERT_EQUALS("[test.c:3]: (information) --check-library: Function set_data() should have <use>/<leak-ignore> configuration\n"
                       "[test.c:4]: (information) --check-library: Function set_data() should have <use>/<leak-ignore> configuration\n"
                       , errout.str());
+        check(code, true);
+        ASSERT_EQUALS("[test.cpp:3]: (information) --check-library: Function set_data() should have <use>/<leak-ignore> configuration\n"
+                      "[test.cpp:4]: (information) --check-library: Function set_data() should have <use>/<leak-ignore> configuration\n"
+                      , errout.str());
     }
 
     void configuration4() {