diff --git a/lib/checkautovariables.cpp b/lib/checkautovariables.cpp index 956a4b39d..939ca6f62 100644 --- a/lib/checkautovariables.cpp +++ b/lib/checkautovariables.cpp @@ -46,35 +46,37 @@ static const CWE CWE398(398U); // Indicator of Poor Code Quality static const CWE CWE562(562U); // Return of Stack Variable Address static const CWE CWE590(590U); // Free of Memory not on the Heap -bool CheckAutoVariables::isPtrArg(const Token *tok) +static bool isPtrArg(const Token *tok) { const Variable *var = tok->variable(); - return (var && var->isArgument() && var->isPointer()); } -bool CheckAutoVariables::isArrayArg(const Token *tok) +static bool isGlobalPtr(const Token *tok) { const Variable *var = tok->variable(); + return (var && var->isGlobal() && var->isPointer()); +} +static bool isArrayArg(const Token *tok) +{ + const Variable *var = tok->variable(); return (var && var->isArgument() && var->isArray()); } -bool CheckAutoVariables::isRefPtrArg(const Token *tok) +static bool isRefPtrArg(const Token *tok) { const Variable *var = tok->variable(); - return (var && var->isArgument() && var->isReference() && var->isPointer()); } -bool CheckAutoVariables::isNonReferenceArg(const Token *tok) +static bool isNonReferenceArg(const Token *tok) { const Variable *var = tok->variable(); - return (var && var->isArgument() && !var->isReference() && (var->isPointer() || var->typeStartToken()->isStandardType() || var->type())); } -bool CheckAutoVariables::isAutoVar(const Token *tok) +static bool isAutoVar(const Token *tok) { const Variable *var = tok->variable(); @@ -98,7 +100,7 @@ bool CheckAutoVariables::isAutoVar(const Token *tok) return true; } -bool CheckAutoVariables::isAutoVarArray(const Token *tok) +static bool isAutoVarArray(const Token *tok) { if (!tok) return false; @@ -221,6 +223,25 @@ void CheckAutoVariables::autoVariables() } else if (Token::Match(tok, "[;{}] * %var% = & %var%") && isPtrArg(tok->tokAt(2)) && isAutoVar(tok->tokAt(5))) { if (checkRvalueExpression(tok->tokAt(5))) errorAutoVariableAssignment(tok->next(), false); + } else if (_settings->isEnabled(Settings::WARNING) && Token::Match(tok, "[;{}] %var% = %var% ;") && isGlobalPtr(tok->next()) && isAutoVarArray(tok->tokAt(3))) { + const Token * const pointer = tok->next(); + const Token * const array = tok->tokAt(3); + const Token * const end = array->variable()->typeStartToken()->scope()->classEnd; + const unsigned int varid = pointer->varId(); + bool writtenAgain = false; + for (const Token *tok2 = array; tok2 != nullptr && tok2 != end; tok2 = tok2->next()) { + if (Token::Match(tok2, "%varid% =", varid)) { + writtenAgain = true; + break; + } else if (Token::Match(tok2, "%name% (") && !Token::simpleMatch(tok2->linkAt(1), ") {")) { + // Bailout: possibly written + // TODO: check if it is written + writtenAgain = true; + break; + } + } + if (!writtenAgain) + errorAssignAddressOfLocalArrayToGlobalPointer(pointer, array); } else if (Token::Match(tok, "[;{}] %var% . %var% = & %var%")) { // TODO: check if the parameter is only changed temporarily (#2969) if (printInconclusive && isPtrArg(tok->next())) { @@ -333,6 +354,14 @@ void CheckAutoVariables::errorAutoVariableAssignment(const Token *tok, bool inco } } +void CheckAutoVariables::errorAssignAddressOfLocalArrayToGlobalPointer(const Token *pointer, const Token *array) +{ + const std::string pointerName = pointer ? pointer->str() : std::string("pointer"); + const std::string arrayName = array ? array->str() : std::string("array"); + reportError(pointer, Severity::warning, "autoVariablesAssignGlobalPointer", + "Address of local array " + arrayName + " is assigned to global pointer " + pointerName +" and not reassigned before " + arrayName + " goes out of scope.", CWE562, false); +} + void CheckAutoVariables::errorReturnAddressOfFunctionParameter(const Token *tok, const std::string &varname) { reportError(tok, Severity::error, "returnAddressOfFunctionParameter", diff --git a/lib/checkautovariables.h b/lib/checkautovariables.h index b73c326d5..b1c103cb7 100644 --- a/lib/checkautovariables.h +++ b/lib/checkautovariables.h @@ -74,13 +74,6 @@ public: void returnReference(); private: - static bool isPtrArg(const Token *tok); - static bool isArrayArg(const Token *tok); - static bool isRefPtrArg(const Token *tok); - static bool isNonReferenceArg(const Token *tok); - static bool isAutoVar(const Token *tok); - static bool isAutoVarArray(const Token *tok); - /** * Returning a temporary object? * @param tok pointing at the "return" token @@ -89,6 +82,7 @@ private: static bool returnTemporary(const Token *tok); void errorReturnAddressToAutoVariable(const Token *tok); + void errorAssignAddressOfLocalArrayToGlobalPointer(const Token *pointer, const Token *array); void errorReturnPointerToLocalArray(const Token *tok); void errorAutoVariableAssignment(const Token *tok, bool inconclusive); void errorReturnReference(const Token *tok); @@ -102,6 +96,7 @@ private: CheckAutoVariables c(nullptr,settings,errorLogger); c.errorAutoVariableAssignment(nullptr, false); c.errorReturnAddressToAutoVariable(nullptr); + c.errorAssignAddressOfLocalArrayToGlobalPointer(nullptr, nullptr); c.errorReturnPointerToLocalArray(nullptr); c.errorReturnReference(nullptr); c.errorReturnTempReference(nullptr); diff --git a/test/testautovariables.cpp b/test/testautovariables.cpp index 0c69b86ab..f82fc2c1d 100644 --- a/test/testautovariables.cpp +++ b/test/testautovariables.cpp @@ -87,6 +87,8 @@ private: TEST_CASE(testassign1); // Ticket #1819 TEST_CASE(testassign2); // Ticket #2765 + TEST_CASE(assignAddressOfLocalArrayToGlobalPointer); + TEST_CASE(returnLocalVariable1); TEST_CASE(returnLocalVariable2); TEST_CASE(returnLocalVariable3); // &x[0] @@ -623,6 +625,23 @@ private: ASSERT_EQUALS("", errout.str()); } + void assignAddressOfLocalArrayToGlobalPointer() { + check("int *p;\n" + "void f() {\n" + " int x[10];\n" + " p = x;\n" + "}"); + ASSERT_EQUALS("[test.cpp:4]: (warning) Address of local array x is assigned to global pointer p and not reassigned before x goes out of scope.\n", errout.str()); + + check("int *p;\n" + "void f() {\n" + " int x[10];\n" + " p = x;\n" + " p = 0;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + } + void returnLocalVariable1() { check("char *foo()\n" "{\n"