walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Fix FP autoVariables (#5107) 

* Fix #11732 FP autoVariables when reassigning argv

* Fix #11732 FN autoVariables with array and std::string

* Add test

* Format

* Format

* Simplify check, fix inconclusive

* Fix merge, add test

* Fix FPs

* Format

* Format

* Undo move

* Format
This commit is contained in:
chrchr-github 2023-06-10 16:28:29 +02:00 committed by GitHub
parent 1c28457d2c
commit 24b0d08753
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 52 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

71
lib/checkautovariables.cpp
View File

@ -166,26 +166,6 @@ static bool isLocalContainerBuffer(const Token* tok)
return yield == Library::Container::Yield::BUFFER || yield == Library::Container::Yield::BUFFER_NT;
}
// Verification that we really take the address of a local variable
static bool checkRvalueExpression(const Token * const vartok)
{
const Variable * const var = vartok->variable();
if (var == nullptr)
return false;
if (Token::Match(vartok->previous(), "& %name% [") && var->isPointer())
return false;
const Token * const next = vartok->next();
// &a.b[0]
if (Token::Match(vartok, "%name% . %var% [") && !var->isPointer()) {
const Variable *var2 = next->next()->variable();
return var2 && !var2->isPointer();
}
return ((next->str() != "." || (!var->isPointer() && (!var->isClass() || var->type()))) && next->strAt(2) != ".");
}
static bool isAddressOfLocalVariable(const Token *expr)
{
if (!expr)
@ -259,6 +239,26 @@ static bool isAutoVariableRHS(const Token* tok) {
return isAddressOfLocalVariable(tok) || isAutoVarArray(tok) || isLocalContainerBuffer(tok);
}
static bool hasOverloadedAssignment(const Token* tok, bool c, bool& inconclusive)
{
inconclusive = false;
if (c)
return false;
if (const ValueType* vt = tok->valueType()) {
if (vt->pointer && !Token::simpleMatch(tok->astParent(), "*"))
return false;
if (vt->container && vt->container->stdStringLike)
return true;
if (vt->typeScope)
return std::any_of(vt->typeScope->functionList.begin(), vt->typeScope->functionList.end(), [](const Function& f) { // TODO: compare argument type
return f.name() == "operator=";
});
return false;
}
inconclusive = true;
return true;
}
void CheckAutoVariables::autoVariables()
{
const bool printInconclusive = mSettings->certainty.isEnabled(Certainty::inconclusive);
@ -271,27 +271,20 @@ void CheckAutoVariables::autoVariables()
continue;
}
// Critical assignment
if (Token::Match(tok, "[;{}] %var% = & %var%") && isRefPtrArg(tok->next()) && isAutoVar(tok->tokAt(4))) {
if (checkRvalueExpression(tok->tokAt(4)))
if (Token::Match(tok, "[;{}] %var% =") && isRefPtrArg(tok->next()) && isAutoVariableRHS(tok->tokAt(2)->astOperand2())) {
checkAutoVariableAssignment(tok->next(), false);
} else if (Token::Match(tok, "[;{}] * %var% =") && isPtrArg(tok->tokAt(2)) && isAddressOfLocalVariable(tok->tokAt(3)->astOperand2())) {
checkAutoVariableAssignment(tok->next(), false);
} else if (Token::Match(tok, "[;{}] %var% . %var% =") && isPtrArg(tok->next()) && isAddressOfLocalVariable(tok->tokAt(4)->astOperand2())) {
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)))
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)))
checkAutoVariableAssignment(tok->next(), false);
}
} else if (Token::Match(tok, "[;{}] * %var% =") && isPtrArg(tok->tokAt(2)) && isAutoVariableRHS(tok->tokAt(3)->astOperand2())) {
const Token* lhs = tok->tokAt(2);
bool inconclusive = false;
if (!hasOverloadedAssignment(lhs, mTokenizer->isC(), inconclusive) || (printInconclusive && inconclusive))
checkAutoVariableAssignment(tok->next(), inconclusive);
tok = tok->tokAt(4);
} else if (Token::Match(tok, "[;{}] %var% . %var% =") && isPtrArg(tok->next()) && isAutoVariableRHS(tok->tokAt(4)->astOperand2())) {
const Token* lhs = tok->tokAt(3);
bool inconclusive = false;
if (!hasOverloadedAssignment(lhs, mTokenizer->isC(), inconclusive) || (printInconclusive && inconclusive))
checkAutoVariableAssignment(tok->next(), inconclusive);
tok = tok->tokAt(5);
} else if (Token::Match(tok, "[;{}] %var% [") && Token::simpleMatch(tok->linkAt(2), "] =") &&
(isPtrArg(tok->next()) || isArrayArg(tok->next(), mSettings)) &&
isAutoVariableRHS(tok->linkAt(2)->next()->astOperand2())) {

25
test/testautovariables.cpp
View File

@ -243,7 +243,7 @@ private:
" char a;\n"
" ab->a = &a;\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]: (error, inconclusive) Address of local auto-variable assigned to a function parameter.\n", errout.str());
}
void testautovar6() { // ticket #2931
@ -536,6 +536,21 @@ private:
ASSERT_EQUALS("[test.cpp:3]: (error) Address of local auto-variable assigned to a function parameter.\n"
"[test.cpp:7]: (error) Address of local auto-variable assigned to a function parameter.\n",
errout.str());
check("struct String {\n"
" String& operator=(const char* c) { m = c; return *this; }\n"
" std::string m;\n"
"};\n"
"struct T { String s; };\n"
"void f(T* t) {\n"
" char a[] = \"abc\";\n"
" t->s = &a[0];\n"
"}\n"
"void g(std::string* s) {\n"
" char a[] = \"abc\";\n"
" *s = &a[0];\n"
"}\n");
ASSERT_EQUALS("", errout.str());
}
void testautovar_normal() {
@ -544,7 +559,7 @@ private:
" XPoint DropPoint;\n"
" ds->location_data = (XtPointer *)&DropPoint;\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]: (error, inconclusive) Address of local auto-variable assigned to a function parameter.\n", errout.str());
}
void testautovar_ptrptr() { // #6596
@ -628,7 +643,7 @@ private:
" 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());
ASSERT_EQUALS("[test.cpp:3]: (error, inconclusive) Address of local auto-variable assigned to a function parameter.\n", errout.str());
check("void foo(cb* pcb) {\n"
" int root0;\n"
@ -637,7 +652,7 @@ private:
" 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());
ASSERT_EQUALS("[test.cpp:3]: (error, inconclusive) Address of local auto-variable assigned to a function parameter.\n", errout.str());
check("struct S { int *p; };\n"
"void g(struct S* s) {\n"
@ -649,7 +664,7 @@ private:
" struct S s;\n"
" g(&s);\n"
"}");
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]: (error) Address of local auto-variable assigned to a function parameter.\n", errout.str());
}
void testinvaliddealloc() {