Fixed #9918 (False positive: autoVariable pointer is NULLed later)

This commit is contained in:
Daniel Marjamäki 2020-09-28 22:48:57 +02:00
parent 8395522390
commit bf236e91d7
3 changed files with 92 additions and 7 deletions

View File

@ -243,23 +243,23 @@ void CheckAutoVariables::autoVariables()
// Critical assignment // Critical assignment
if (Token::Match(tok, "[;{}] %var% = & %var%") && isRefPtrArg(tok->next()) && isAutoVar(tok->tokAt(4))) { if (Token::Match(tok, "[;{}] %var% = & %var%") && isRefPtrArg(tok->next()) && isAutoVar(tok->tokAt(4))) {
if (checkRvalueExpression(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())) { } 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())) { } 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% ;")) { } else if (Token::Match(tok, "[;{}] %var% . %var% = %var% ;")) {
// TODO: check if the parameter is only changed temporarily (#2969) // TODO: check if the parameter is only changed temporarily (#2969)
if (printInconclusive && isPtrArg(tok->next())) { if (printInconclusive && isPtrArg(tok->next())) {
if (isAutoVarArray(tok->tokAt(5))) if (isAutoVarArray(tok->tokAt(5)))
errorAutoVariableAssignment(tok->next(), true); checkAutoVariableAssignment(tok->next(), true);
} }
tok = tok->tokAt(5); tok = tok->tokAt(5);
} else if (Token::Match(tok, "[;{}] * %var% = %var% ;")) { } else if (Token::Match(tok, "[;{}] * %var% = %var% ;")) {
const Variable * var1 = tok->tokAt(2)->variable(); const Variable * var1 = tok->tokAt(2)->variable();
if (var1 && var1->isArgument() && Token::Match(var1->nameToken()->tokAt(-3), "%type% * *")) { if (var1 && var1->isArgument() && Token::Match(var1->nameToken()->tokAt(-3), "%type% * *")) {
if (isAutoVarArray(tok->tokAt(4))) if (isAutoVarArray(tok->tokAt(4)))
errorAutoVariableAssignment(tok->next(), false); checkAutoVariableAssignment(tok->next(), false);
} }
tok = tok->tokAt(4); tok = tok->tokAt(4);
} else if (Token::Match(tok, "[;{}] %var% [") && Token::simpleMatch(tok->linkAt(2), "] =") && } 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) void CheckAutoVariables::errorReturnAddressToAutoVariable(const Token *tok)

View File

@ -64,6 +64,11 @@ public:
/** Check auto variables */ /** Check auto variables */
void autoVariables(); 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 checkVarLifetime();
void checkVarLifetimeScope(const Token * start, const Token * end); void checkVarLifetimeScope(const Token * start, const Token * end);

View File

@ -77,6 +77,7 @@ private:
TEST_CASE(testautovar_return3); TEST_CASE(testautovar_return3);
TEST_CASE(testautovar_return4); TEST_CASE(testautovar_return4);
TEST_CASE(testautovar_extern); TEST_CASE(testautovar_extern);
TEST_CASE(testautovar_reassigned);
TEST_CASE(testinvaliddealloc); TEST_CASE(testinvaliddealloc);
TEST_CASE(testinvaliddealloc_C); TEST_CASE(testinvaliddealloc_C);
TEST_CASE(testassign1); // Ticket #1819 TEST_CASE(testassign1); // Ticket #1819
@ -278,10 +279,26 @@ private:
" FN fn;\n" " FN fn;\n"
" FP fp;\n" " FP fp;\n"
" p = &fn.i;\n" " p = &fn.i;\n"
" p = &p_fp->i;\n"
" p = &fp.f->i;\n"
"}", false); "}", 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]: (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 void testautovar10() { // #2930 - assignment of function parameter
@ -554,6 +571,34 @@ private:
ASSERT_EQUALS("", errout.str()); 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() { void testinvaliddealloc() {
check("void func1() {\n" check("void func1() {\n"
" char tmp1[256];\n" " char tmp1[256];\n"