diff --git a/lib/checkautovariables.cpp b/lib/checkautovariables.cpp index 78075b44e..3d8f90037 100644 --- a/lib/checkautovariables.cpp +++ b/lib/checkautovariables.cpp @@ -243,23 +243,23 @@ void CheckAutoVariables::autoVariables() // Critical assignment if (Token::Match(tok, "[;{}] %var% = & %var%") && isRefPtrArg(tok->next()) && isAutoVar(tok->tokAt(4))) { if (checkRvalueExpression(tok->tokAt(4))) - errorAutoVariableAssignment(tok->next(), false); + checkAutoVariableAssignment(tok->next(), false); } else if (Token::Match(tok, "[;{}] * %var% =") && isPtrArg(tok->tokAt(2)) && isAddressOfLocalVariable(tok->tokAt(3)->astOperand2())) { - errorAutoVariableAssignment(tok->next(), false); + checkAutoVariableAssignment(tok->next(), false); } else if (Token::Match(tok, "[;{}] %var% . %var% =") && isPtrArg(tok->next()) && isAddressOfLocalVariable(tok->tokAt(4)->astOperand2())) { - errorAutoVariableAssignment(tok->next(), false); + checkAutoVariableAssignment(tok->next(), false); } else if (Token::Match(tok, "[;{}] %var% . %var% = %var% ;")) { // TODO: check if the parameter is only changed temporarily (#2969) if (printInconclusive && isPtrArg(tok->next())) { if (isAutoVarArray(tok->tokAt(5))) - errorAutoVariableAssignment(tok->next(), true); + checkAutoVariableAssignment(tok->next(), true); } tok = tok->tokAt(5); } else if (Token::Match(tok, "[;{}] * %var% = %var% ;")) { const Variable * var1 = tok->tokAt(2)->variable(); if (var1 && var1->isArgument() && Token::Match(var1->nameToken()->tokAt(-3), "%type% * *")) { if (isAutoVarArray(tok->tokAt(4))) - errorAutoVariableAssignment(tok->next(), false); + checkAutoVariableAssignment(tok->next(), false); } tok = tok->tokAt(4); } else if (Token::Match(tok, "[;{}] %var% [") && Token::simpleMatch(tok->linkAt(2), "] =") && @@ -292,6 +292,41 @@ void CheckAutoVariables::autoVariables() } } +bool CheckAutoVariables::checkAutoVariableAssignment(const Token *expr, bool inconclusive, const Token *startToken) +{ + if (!startToken) + startToken = Token::findsimplematch(expr, "=")->next(); + for (const Token *tok = startToken; tok; tok = tok->next()) { + if (tok->str() == "}" && tok->scope()->type == Scope::ScopeType::eFunction) + errorAutoVariableAssignment(expr, inconclusive); + + if (Token::Match(tok, "return|throw|break|continue")) { + errorAutoVariableAssignment(expr, inconclusive); + return true; + } + if (Token::simpleMatch(tok, "=")) { + const Token *lhs = tok; + while (Token::Match(lhs->previous(), "%name%|.|*")) + lhs = lhs->previous(); + const Token *e = expr; + while (e->str() != "=" && lhs->str() == e->str()) { + e = e->next(); + lhs = lhs->next(); + } + if (lhs->str() == "=") + return false; + } + + if (Token::simpleMatch(tok, "if (")) { + const Token *ifStart = tok->linkAt(1)->next(); + return checkAutoVariableAssignment(expr, inconclusive, ifStart) || checkAutoVariableAssignment(expr, inconclusive, ifStart->link()->next()); + } + if (Token::simpleMatch(tok, "} else {")) + tok = tok->linkAt(2); + } + return false; +} + //--------------------------------------------------------------------------- void CheckAutoVariables::errorReturnAddressToAutoVariable(const Token *tok) diff --git a/lib/checkautovariables.h b/lib/checkautovariables.h index 99356ca56..171ee6020 100644 --- a/lib/checkautovariables.h +++ b/lib/checkautovariables.h @@ -64,6 +64,11 @@ public: /** Check auto variables */ void autoVariables(); + /** + * Check variable assignment.. value must be changed later or there will be a error reported + * @return true if error is reported */ + bool checkAutoVariableAssignment(const Token *expr, bool inconclusive, const Token *startToken = nullptr); + void checkVarLifetime(); void checkVarLifetimeScope(const Token * start, const Token * end); diff --git a/test/testautovariables.cpp b/test/testautovariables.cpp index 481d99adb..5dabd4be0 100644 --- a/test/testautovariables.cpp +++ b/test/testautovariables.cpp @@ -77,6 +77,7 @@ private: TEST_CASE(testautovar_return3); TEST_CASE(testautovar_return4); TEST_CASE(testautovar_extern); + TEST_CASE(testautovar_reassigned); TEST_CASE(testinvaliddealloc); TEST_CASE(testinvaliddealloc_C); TEST_CASE(testassign1); // Ticket #1819 @@ -278,10 +279,26 @@ private: " FN fn;\n" " FP fp;\n" " p = &fn.i;\n" - " 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()); + + check("struct FN {int i;};\n" + "struct FP {FN* f};\n" + "void foo(int*& p, FN* p_fp) {\n" + " FN fn;\n" + " FP fp;\n" + " p = &p_fp->i;\n" + "}", false); + ASSERT_EQUALS("", errout.str()); + + check("struct FN {int i;};\n" + "struct FP {FN* f};\n" + "void foo(int*& p, FN* p_fp) {\n" + " FN fn;\n" + " FP fp;\n" + " p = &fp.f->i;\n" + "}", false); + ASSERT_EQUALS("", errout.str()); } void testautovar10() { // #2930 - assignment of function parameter @@ -554,6 +571,34 @@ private: ASSERT_EQUALS("", errout.str()); } + void testautovar_reassigned() { + check("void foo(cb* pcb) {\n" + " int root0;\n" + " pcb->root0 = &root0;\n" + " dostuff(pcb);\n" + " pcb->root0 = 0;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void foo(cb* pcb) {\n" + " int root0;\n" + " pcb->root0 = &root0;\n" + " dostuff(pcb);\n" + " if (condition) return;\n" // <- not reassigned => error + " pcb->root0 = 0;\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (error) Address of local auto-variable assigned to a function parameter.\n", errout.str()); + + check("void foo(cb* pcb) {\n" + " int root0;\n" + " pcb->root0 = &root0;\n" + " dostuff(pcb);\n" + " if (condition)\n" + " pcb->root0 = 0;\n" // <- conditional reassign => error + "}"); + ASSERT_EQUALS("[test.cpp:3]: (error) Address of local auto-variable assigned to a function parameter.\n", errout.str()); + } + void testinvaliddealloc() { check("void func1() {\n" " char tmp1[256];\n"