diff --git a/lib/checkautovariables.cpp b/lib/checkautovariables.cpp index 0a1ba0610..0c17653b8 100644 --- a/lib/checkautovariables.cpp +++ b/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)) - errorInvalidDeallocation(tok); + if (isArrayVar(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" - "The deallocation of an auto-variable results in undefined behaviour. You should only free memory " + "Deallocation of an " + type + " results in undefined behaviour.\n" + "The deallocation of an " + type + " results in undefined behaviour. You should only free memory " "that has been allocated dynamically.", CWE590, false); } diff --git a/lib/checkautovariables.h b/lib/checkautovariables.h index dee63419f..c6b309661 100644 --- a/lib/checkautovariables.h +++ b/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); diff --git a/test/testautovariables.cpp b/test/testautovariables.cpp index 2c19ff1f0..205f3fed0 100644 --- a/test/testautovariables.cpp +++ b/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"