diff --git a/lib/checkautovariables.cpp b/lib/checkautovariables.cpp index 92be9ae02..3fe603a87 100644 --- a/lib/checkautovariables.cpp +++ b/lib/checkautovariables.cpp @@ -167,14 +167,14 @@ static bool checkRvalueExpression(const Token * const vartok) return ((next->str() != "." || (!var->isPointer() && (!var->isClass() || var->type()))) && next->strAt(2) != "."); } -static bool isAddressOfLocalVariableRecursive(const Token *expr) +static bool isAddressOfLocalVariable(const Token *expr) { if (!expr) return false; if (Token::Match(expr, "+|-")) - return isAddressOfLocalVariableRecursive(expr->astOperand1()) || isAddressOfLocalVariableRecursive(expr->astOperand2()); + return isAddressOfLocalVariable(expr->astOperand1()) || isAddressOfLocalVariable(expr->astOperand2()); if (expr->isCast()) - return isAddressOfLocalVariableRecursive(expr->astOperand2() ? expr->astOperand2() : expr->astOperand1()); + return isAddressOfLocalVariable(expr->astOperand2() ? expr->astOperand2() : expr->astOperand1()); if (expr->isUnaryOp("&")) { const Token *op = expr->astOperand1(); bool deref = false; @@ -190,13 +190,6 @@ static bool isAddressOfLocalVariableRecursive(const Token *expr) return false; } -static bool isAddressOfLocalVariable(const Token *expr) -{ - if (!expr || !expr->valueType() || expr->valueType()->pointer == 0) - return false; - return isAddressOfLocalVariableRecursive(expr); -} - static bool variableIsUsedInScope(const Token* start, unsigned int varId, const Scope *scope) { if (!start) // Ticket #5024 @@ -271,6 +264,8 @@ void CheckAutoVariables::autoVariables() errorAutoVariableAssignment(tok->next(), false); } else if (Token::Match(tok, "[;{}] * %var% =") && isPtrArg(tok->tokAt(2)) && isAddressOfLocalVariable(tok->tokAt(3)->astOperand2())) { errorAutoVariableAssignment(tok->next(), false); + } else if (Token::Match(tok, "[;{}] %var% . %var% =") && isPtrArg(tok->next()) && isAddressOfLocalVariable(tok->tokAt(4)->astOperand2())) { + errorAutoVariableAssignment(tok->next(), false); } else if (mSettings->isEnabled(Settings::WARNING) && Token::Match(tok, "[;{}] %var% = &| %var% ;") && isGlobalPtr(tok->next())) { const Token * const pointer = tok->next(); if (isAutoVarArray(tok->tokAt(3))) { @@ -282,14 +277,6 @@ void CheckAutoVariables::autoVariables() if (!reassignedGlobalPointer(variable, pointer->varId(), mSettings, mTokenizer->isCPP())) errorAssignAddressOfLocalVariableToGlobalPointer(pointer, variable); } - } else if (Token::Match(tok, "[;{}] %var% . %var% = & %var%")) { - // TODO: check if the parameter is only changed temporarily (#2969) - if (printInconclusive && isPtrArg(tok->next())) { - const Token * const var2tok = tok->tokAt(6); - if (isAutoVar(var2tok) && checkRvalueExpression(var2tok)) - errorAutoVariableAssignment(tok->next(), true); - } - tok = tok->tokAt(6); } else if (Token::Match(tok, "[;{}] %var% . %var% = %var% ;")) { // TODO: check if the parameter is only changed temporarily (#2969) if (printInconclusive && isPtrArg(tok->next())) { @@ -304,11 +291,9 @@ void CheckAutoVariables::autoVariables() errorAutoVariableAssignment(tok->next(), false); } tok = tok->tokAt(4); - } else if (Token::Match(tok, "[;{}] %var% [") && Token::Match(tok->linkAt(2), "] = & %var%") && - (isPtrArg(tok->next()) || isArrayArg(tok->next())) && isAutoVar(tok->linkAt(2)->tokAt(3))) { - const Token* const varTok = tok->linkAt(2)->tokAt(3); - if (checkRvalueExpression(varTok)) - errorAutoVariableAssignment(tok->next(), false); + } else if (Token::Match(tok, "[;{}] %var% [") && Token::simpleMatch(tok->linkAt(2), "] =") && + (isPtrArg(tok->next()) || isArrayArg(tok->next())) && isAddressOfLocalVariable(tok->linkAt(2)->next()->astOperand2())) { + errorAutoVariableAssignment(tok->next(), false); } // Invalid pointer deallocation else if ((Token::Match(tok, "%name% ( %var% ) ;") && mSettings->library.dealloc(tok)) || @@ -354,14 +339,14 @@ void CheckAutoVariables::errorReturnPointerToLocalArray(const Token *tok) void CheckAutoVariables::errorAutoVariableAssignment(const Token *tok, bool inconclusive) { if (!inconclusive) { - reportError(tok, Severity::error, "autoVariables", + reportError(tok, Severity::warning, "autoVariables", "Address of local auto-variable assigned to a function parameter.\n" "Dangerous assignment - the function parameter is assigned the address of a local " "auto-variable. Local auto-variables are reserved from the stack which " "is freed when the function ends. So the pointer to a local variable " "is invalid after the function ends.", CWE562, false); } else { - reportError(tok, Severity::error, "autoVariables", + reportError(tok, Severity::warning, "autoVariables", "Address of local auto-variable assigned to a function parameter.\n" "Function parameter is assigned the address of a local auto-variable. " "Local auto-variables are reserved from the stack which is freed when " diff --git a/samples/autoVariables/out.txt b/samples/autoVariables/out.txt index a5f001b2f..c9d00cecc 100644 --- a/samples/autoVariables/out.txt +++ b/samples/autoVariables/out.txt @@ -1 +1 @@ -[samples\autoVariables\bad.c:4]: (error) Address of local auto-variable assigned to a function parameter. +[samples\autoVariables\bad.c:4]: (warning) Address of local auto-variable assigned to a function parameter. diff --git a/test/testautovariables.cpp b/test/testautovariables.cpp index b4a8931b1..eb4d67fcb 100644 --- a/test/testautovariables.cpp +++ b/test/testautovariables.cpp @@ -31,7 +31,7 @@ public: private: Settings settings; - void check(const char code[], bool inconclusive = false, bool runSimpleChecks = true, const char* filename = "test.cpp") { + void check(const char code[], bool inconclusive = false, const char* filename = "test.cpp") { // Clear the error buffer.. errout.str(""); @@ -42,17 +42,8 @@ private: std::istringstream istr(code); tokenizer.tokenize(istr, filename); - CheckAutoVariables checkAutoVariables(&tokenizer, &settings, this); - checkAutoVariables.returnReference(); - checkAutoVariables.assignFunctionArg(); - checkAutoVariables.checkVarLifetime(); - - if (runSimpleChecks) { - tokenizer.simplifyTokenList2(); - - // Check auto variables - checkAutoVariables.autoVariables(); - } + CheckAutoVariables checkAutoVariables; + checkAutoVariables.runChecks(&tokenizer, &settings, this); } void run() OVERRIDE { @@ -78,6 +69,7 @@ private: TEST_CASE(testautovar16); // ticket #8114 TEST_CASE(testautovar_array1); TEST_CASE(testautovar_array2); + TEST_CASE(testautovar_normal); // "normal" token list that does not remove casts etc TEST_CASE(testautovar_ptrptr); // ticket #6956 TEST_CASE(testautovar_return1); TEST_CASE(testautovar_return2); @@ -141,7 +133,7 @@ private: " int num = 2;\n" " *res = #\n" "}"); - ASSERT_EQUALS("[test.cpp:4]: (error) Address of local auto-variable assigned to a function parameter.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str()); check("void func1(int **res)\n" "{\n" @@ -167,7 +159,7 @@ private: " int num = 2;\n" " *res = #\n" "}"); - ASSERT_EQUALS("[test.cpp:7]: (error) Address of local auto-variable assigned to a function parameter.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:7]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str()); check("class Fred {\n" " void func1(int **res);\n" @@ -196,7 +188,7 @@ private: " int x[100];\n" " *p = x;\n" "}"); - ASSERT_EQUALS("[test.cpp:4]: (error) Address of local auto-variable assigned to a function parameter.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str()); } void testautovar4() { // ticket #2928 @@ -213,15 +205,8 @@ private: "{\n" " char a;\n" " ab->a = &a;\n" - "}", false); - ASSERT_EQUALS("", errout.str()); - - check("void foo(struct AB *ab)\n" - "{\n" - " char a;\n" - " ab->a = &a;\n" - "}", true); - ASSERT_EQUALS("[test.cpp:4]: (error, inconclusive) Address of local auto-variable assigned to a function parameter.\n", errout.str()); + "}"); + ASSERT_EQUALS("[test.cpp:4]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str()); } void testautovar6() { // ticket #2931 @@ -237,7 +222,7 @@ private: " char a[10];\n" " x->str = a;\n" "}", true); - ASSERT_EQUALS("[test.cpp:4]: (error, inconclusive) Address of local auto-variable assigned to a function parameter.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4]: (warning, inconclusive) Address of local auto-variable assigned to a function parameter.\n", errout.str()); } void testautovar7() { // ticket #3066 @@ -255,7 +240,7 @@ private: " int i = 0;\n" " p = &i;\n" "}", false); - ASSERT_EQUALS("[test.cpp:3]: (error) Address of local auto-variable assigned to a function parameter.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str()); check("void foo(std::string& s) {\n" " s = foo;\n" @@ -273,7 +258,7 @@ private: " p = &p_fp->i;\n" " p = &fp.f->i;\n" "}", false); - ASSERT_EQUALS("[test.cpp:6]: (error) Address of local auto-variable assigned to a function parameter.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:6]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str()); } void testautovar10() { // #2930 - assignment of function parameter @@ -350,7 +335,7 @@ private: " int i = d;\n" " d = i;\n" " return d;" - "}",false,false); + "}",false); ASSERT_EQUALS("", errout.str()); check("void foo(int* ptr) {\n" // #4793 @@ -386,7 +371,7 @@ private: " struct A a = bar();\n" " *p = &a.data[0];\n" "}"); - ASSERT_EQUALS("[test.cpp:6]: (error) Address of local auto-variable assigned to a function parameter.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:6]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str()); check("void f(char **out) {\n" " struct S *p = glob;\n" @@ -405,7 +390,7 @@ private: " s8 p[10];\n" // <- p is array => error " *out = &p[1];\n" "}"); - ASSERT_EQUALS("[test.cpp:3]: (error) Address of local auto-variable assigned to a function parameter.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str()); } void testautovar12() { // Ticket #5024, #5050 - Crash on invalid input @@ -447,7 +432,7 @@ private: " if (lumdiff > 5.0f)\n" " return &darkOutline;\n" " return 0;\n" - "}", false, false); + "}", false); ASSERT_EQUALS("", errout.str()); } @@ -465,7 +450,7 @@ private: " int num=2;" " arr[0]=#\n" "}"); - ASSERT_EQUALS("[test.cpp:3]: (error) Address of local auto-variable assigned to a function parameter.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str()); } void testautovar_array2() { @@ -477,7 +462,16 @@ private: " int num=2;" " arr[0]=#\n" "}"); - ASSERT_EQUALS("[test.cpp:6]: (error) Address of local auto-variable assigned to a function parameter.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:6]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str()); + } + + void testautovar_normal() { + check("void f(XmDestinationCallbackStruct *ds)\n" + "{\n" + " XPoint DropPoint;\n" + " ds->location_data = (XtPointer *)&DropPoint;\n" + "}"); + ASSERT_EQUALS("[test.cpp:4]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str()); } void testautovar_ptrptr() { // #6596 @@ -485,7 +479,7 @@ private: " char dead_slot;\n" " matches[0] = (char *)&dead_slot;\n" "}"); - ASSERT_EQUALS("[test.cpp:3]: (error) Address of local auto-variable assigned to a function parameter.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str()); } void testautovar_return1() { @@ -677,7 +671,7 @@ private: check("void svn_repos_dir_delta2() {\n" " struct context c;\n" " SVN_ERR(delete(&c, root_baton, src_entry, pool));\n" - "}\n", false, true, "test.c"); + "}\n", false, "test.c"); ASSERT_EQUALS("", errout.str()); } @@ -1055,7 +1049,7 @@ private: " double ret = getValue();\n" " rd = ret;\n" " return rd;\n" - "}", false, false); + "}", false); ASSERT_EQUALS("", errout.str()); }