Fixed #8826 (false negative: Invalid memory address freed)

lib/checkautovariables.cpp

@@ -64,6 +64,12 @@ static bool isArrayArg(const Token *tok)
     return (var && var->isArgument() && var->isArray());
 }
 
+static bool isArrayVar(const Token *tok)
+{
+    const Variable *var = tok->variable();
+    return (var && var->isArray());
+}
+
 static bool isRefPtrArg(const Token *tok)
 {
     const Variable *var = tok->variable();
@@ -304,13 +310,21 @@ void CheckAutoVariables::autoVariables()
             else if ((Token::Match(tok, "%name% ( %var% ) ;") && mSettings->library.dealloc(tok)) ||
                      (mTokenizer->isCPP() && Token::Match(tok, "delete [| ]| (| %var% !!["))) {
                 tok = Token::findmatch(tok->next(), "%var%");
-                if (isAutoVarArray(tok))
+                if (isArrayVar(tok))
-                    errorInvalidDeallocation(tok);
+                    errorInvalidDeallocation(tok, nullptr);
+                else if (tok && tok->variable() && tok->variable()->isPointer()) {
+                    for (const ValueFlow::Value &v : tok->values()) {
+                        if (v.isTokValue() && isArrayVar(v.tokvalue)) {
+                            errorInvalidDeallocation(tok, &v);
+                            break;
+                        }
+                    }
+                }
             } else if ((Token::Match(tok, "%name% ( & %var% ) ;") && mSettings->library.dealloc(tok)) ||
                        (mTokenizer->isCPP() && Token::Match(tok, "delete [| ]| (| & %var% !!["))) {
                 tok = Token::findmatch(tok->next(), "%var%");
                 if (isAutoVar(tok))
-                    errorInvalidDeallocation(tok);
+                    errorInvalidDeallocation(tok, nullptr);
             }
         }
     }
@@ -583,12 +597,25 @@ void CheckAutoVariables::errorReturnTempReference(const Token *tok)
     reportError(tok, Severity::error, "returnTempReference", "Reference to temporary returned.", CWE562, false);
 }
 
-void CheckAutoVariables::errorInvalidDeallocation(const Token *tok)
+void CheckAutoVariables::errorInvalidDeallocation(const Token *tok, const ValueFlow::Value *val)
 {
-    reportError(tok,
+    const Variable *var = val ? val->tokvalue->variable() : tok->variable();
+
+    std::string type = "auto-variable";
+    if (var) {
+        if (var->isGlobal())
+            type = "global variable";
+        else if (var->isStatic())
+            type = "static variable";
+    }
+
+    if (val)
+        type += " (" + val->tokvalue->str() + ")";
+
+    reportError(getErrorPath(tok, val, "Deallocating memory that was not dynamically allocated"),
                 Severity::error,
                 "autovarInvalidDeallocation",
-                "Deallocation of an auto-variable results in undefined behaviour.\n"
+                "Deallocation of an " + type + " results in undefined behaviour.\n"
-                "The deallocation of an auto-variable results in undefined behaviour. You should only free memory "
+                "The deallocation of an " + type + " results in undefined behaviour. You should only free memory "
                 "that has been allocated dynamically.", CWE590, false);
 }

lib/checkautovariables.h

@@ -89,7 +89,7 @@ private:
     void errorAutoVariableAssignment(const Token *tok, bool inconclusive);
     void errorReturnReference(const Token *tok);
     void errorReturnTempReference(const Token *tok);
-    void errorInvalidDeallocation(const Token *tok);
+    void errorInvalidDeallocation(const Token *tok, const ValueFlow::Value *val);
     void errorReturnAddressOfFunctionParameter(const Token *tok, const std::string &varname);
     void errorUselessAssignmentArg(const Token *tok);
     void errorUselessAssignmentPtrArg(const Token *tok);
@@ -102,7 +102,7 @@ private:
         c.errorReturnPointerToLocalArray(nullptr);
         c.errorReturnReference(nullptr);
         c.errorReturnTempReference(nullptr);
-        c.errorInvalidDeallocation(nullptr);
+        c.errorInvalidDeallocation(nullptr, nullptr);
         c.errorReturnAddressOfFunctionParameter(nullptr, "parameter");
         c.errorUselessAssignmentArg(nullptr);
         c.errorUselessAssignmentPtrArg(nullptr);

test/testautovariables.cpp

@@ -527,6 +527,20 @@ private:
               "}");
         ASSERT_EQUALS("", errout.str());
 
+        check("void func1() {\n"
+              "    static char tmp1[256];\n"
+              "    char *p = tmp1;\n"
+              "    free(p);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:4]: (error) Deallocation of an static variable (tmp1) results in undefined behaviour.\n", errout.str());
+
+        check("char tmp1[256];\n"
+              "void func1() {\n"
+              "    char *p; if (x) p = tmp1;\n"
+              "    free(p);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:4]: (error) Deallocation of an global variable (tmp1) results in undefined behaviour.\n", errout.str());
+
         check("void f()\n"
               "{\n"
               "    char psz_title[10];\n"