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

Fix #11599 false negative: constParameter (#4902) 

* Partial fix for #11599 false negative: constParameter

* Adjust test

* Update testother.cpp

* Update testother.cpp

* Fix #11599 false negative: constParameter

* Fix new warnings

* Format

* Add difference_type

* Remove isAliased()

* Undo

* Adjust test

* Add test

* Improve const check

* Tweak constness, add tests

* Add tests

* Use new helper function, fixtest

* Remove bailout, fix check for cast

* Prevent FP

* Fix constVariable check, add tests

* Format

* Format

* Add test for #11632
This commit is contained in:
chrchr-github 2023-04-02 20:36:23 +02:00 committed by GitHub
parent 634f5e254f
commit a336048d14
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 188 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
lib/astutils.cpp
View File

@ -2407,7 +2407,7 @@ bool isVariableChangedByFunctionCall(const Token *tok, int indirect, const Setti
continue;
conclusive = true;
if (indirect > 0) {
if (!arg->isConst() && arg->isPointer())
if (arg->isPointer() && !(arg->valueType() && arg->valueType()->isConst(indirect)))
return true;
// If const is applied to the pointer, then the value can still be modified
if (Token::simpleMatch(arg->typeEndToken(), "* const"))

51
lib/checkother.cpp
View File

@ -1391,6 +1391,14 @@ static bool isVariableMutableInInitializer(const Token* start, const Token * end
return false;
}
static const Token* isFuncArg(const Token* tok) {
while (Token::simpleMatch(tok, ","))
tok = tok->astParent();
if (Token::simpleMatch(tok, "(") && Token::Match(tok->astOperand1(), "%name% ("))
return tok->astOperand1();
return nullptr;
}
void CheckOther::checkConstVariable()
{
if (!mSettings->severity.isEnabled(Severity::style) || mTokenizer->isC())
@ -1433,8 +1441,6 @@ void CheckOther::checkConstVariable()
continue;
if (var->isVolatile())
continue;
if (isAliased(var))
continue;
if (isStructuredBindingVariable(var)) // TODO: check all bound variables
continue;
if (isVariableChanged(var, mSettings, mTokenizer->isCPP()))
@ -1446,7 +1452,7 @@ void CheckOther::checkConstVariable()
functionScope = functionScope->nestedIn;
} while (functionScope && !(function = functionScope->function));
}
if (function && Function::returnsReference(function) && !Function::returnsConst(function)) {
if (function && (Function::returnsReference(function) || Function::returnsPointer(function)) && !Function::returnsConst(function)) {
std::vector<const Token*> returns = Function::findReturns(function);
if (std::any_of(returns.cbegin(), returns.cend(), [&](const Token* retTok) {
if (retTok->varId() == var->declarationId())
@ -1455,13 +1461,12 @@ void CheckOther::checkConstVariable()
retTok = retTok->astOperand2();
while (Token::simpleMatch(retTok, "."))
retTok = retTok->astOperand2();
if (Token::simpleMatch(retTok, "&"))
retTok = retTok->astOperand1();
return ValueFlow::hasLifetimeToken(getParentLifetime(retTok), var->nameToken());
}))
continue;
}
// Skip if address is taken
if (Token::findmatch(var->nameToken(), "& %varid%", scope->bodyEnd, var->declarationId()))
continue;
// Skip if another non-const variable is initialized with this variable
{
//Is it the right side of an initialization of a non-const reference
@ -1474,6 +1479,25 @@ void CheckOther::checkConstVariable()
break;
}
}
if (Token::Match(tok, "& %varid%", var->declarationId())) {
const Token* opTok = tok->astParent();
if (opTok->isComparisonOp() || opTok->isAssignmentOp() || opTok->isCalculation()) {
if (opTok->isComparisonOp() || opTok->isCalculation()) {
if (opTok->astOperand1() != tok)
opTok = opTok->astOperand1();
else
opTok = opTok->astOperand2();
}
if (opTok->valueType() && var->valueType() && opTok->valueType()->isConst(var->valueType()->pointer))
continue;
} else if (const Token* ftok = isFuncArg(opTok)) {
bool inconclusive{};
if (var->valueType() && !isVariableChangedByFunctionCall(ftok, var->valueType()->pointer, var->declarationId(), mSettings, &inconclusive) && !inconclusive)
continue;
}
usedInAssignment = true;
break;
}
if (astIsRangeBasedForDecl(tok) && Token::Match(tok->astParent()->astOperand2(), "%varid%", var->declarationId())) {
const Variable* refvar = tok->astParent()->astOperand1()->variable();
if (refvar && refvar->isReference() && !refvar->isConst()) {
@ -1494,7 +1518,7 @@ void CheckOther::checkConstVariable()
castToNonConst = true; // safe guess
break;
}
const bool isConst = 0 != (tok->valueType()->constness & (1 << tok->valueType()->pointer));
const bool isConst = tok->valueType()->isConst(tok->valueType()->pointer);
if (!isConst) {
castToNonConst = true;
break;
@ -1565,15 +1589,10 @@ void CheckOther::checkConstPointer()
continue;
} else if (Token::simpleMatch(gparent, "[") && gparent->astOperand2() == parent)
continue;
else if (Token::Match(gparent, "(|,")) {
const Token* ftok = gparent;
while (Token::simpleMatch(ftok, ","))
ftok = ftok->astParent();
if (ftok && Token::Match(ftok->astOperand1(), "%name% (")) {
bool inconclusive{};
if (!isVariableChangedByFunctionCall(ftok->astOperand1(), vt->pointer, var->declarationId(), mSettings, &inconclusive) && !inconclusive)
continue;
}
else if (const Token* ftok = isFuncArg(gparent)) {
bool inconclusive{};
if (!isVariableChangedByFunctionCall(ftok, vt->pointer, var->declarationId(), mSettings, &inconclusive) && !inconclusive)
continue;
}
} else {
if (Token::Match(parent, "%oror%|%comp%|&&|?|!|-"))

7
lib/symboldatabase.cpp
View File

@ -3009,6 +3009,13 @@ bool Function::returnsReference(const Function* function, bool unknown)
});
}
bool Function::returnsPointer(const Function* function, bool unknown)
{
return checkReturns(function, unknown, false, [](UNUSED const Token* defStart, const Token* defEnd) {
return Token::simpleMatch(defEnd->previous(), "*");
});
}
bool Function::returnsStandardType(const Function* function, bool unknown)
{
return checkReturns(function, unknown, true, [](UNUSED const Token* defStart, const Token* defEnd) {

1
lib/symboldatabase.h
View File

@ -934,6 +934,7 @@ public:
static bool returnsConst(const Function* function, bool unknown = false);
static bool returnsPointer(const Function* function, bool unknown = false);
static bool returnsReference(const Function* function, bool unknown = false);
static bool returnsStandardType(const Function* function, bool unknown = false);

2
lib/tokenize.cpp
View File

@ -2182,8 +2182,6 @@ void Tokenizer::simplifyTypedefCpp()
syntaxError(tok2);
if (tok2->str() == "=") {
if (!tok2->next())
syntaxError(tok2);
if (tok2->next()->str() == "{")
tok2 = tok2->next()->link()->next();
else if (tok2->next()->str().at(0) == '\"')

111
test/testastutils.cpp
View File

@ -244,7 +244,9 @@ private:
std::istringstream istr(code);
ASSERT_LOC(tokenizer.tokenize(istr, "test.cpp"), file, line);
const Token * const argtok = Token::findmatch(tokenizer.tokens(), pattern);
return (isVariableChangedByFunctionCall)(argtok, 0, &settings, inconclusive);
ASSERT_LOC(argtok, file, line);
int indirect = (argtok->variable() && argtok->variable()->isArray());
return (isVariableChangedByFunctionCall)(argtok, indirect, &settings, inconclusive);
}
void isVariableChangedByFunctionCallTest() {
@ -262,8 +264,113 @@ private:
code = "int f(int x) {\n"
"return int(x);\n"
"}\n";
inconclusive = false;
ASSERT_EQUALS(false, isVariableChangedByFunctionCall(code, "x ) ;", &inconclusive));
TODO_ASSERT_EQUALS(false, true, inconclusive);
ASSERT_EQUALS(false, inconclusive);
code = "void g(int* p);\n"
"void f(int x) {\n"
" return g(&x);\n"
"}\n";
inconclusive = false;
ASSERT_EQUALS(true, isVariableChangedByFunctionCall(code, "x ) ;", &inconclusive));
ASSERT_EQUALS(false, inconclusive);
code = "void g(const int* p);\n"
"void f(int x) {\n"
" return g(&x);\n"
"}\n";
inconclusive = false;
ASSERT_EQUALS(false, isVariableChangedByFunctionCall(code, "x ) ;", &inconclusive));
ASSERT_EQUALS(false, inconclusive);
code = "void g(int** pp);\n"
"void f(int* p) {\n"
" return g(&p);\n"
"}\n";
inconclusive = false;
ASSERT_EQUALS(true, isVariableChangedByFunctionCall(code, "p ) ;", &inconclusive));
ASSERT_EQUALS(false, inconclusive);
code = "void g(int* const* pp);\n"
"void f(int* p) {\n"
" return g(&p);\n"
"}\n";
inconclusive = false;
ASSERT_EQUALS(false, isVariableChangedByFunctionCall(code, "p ) ;", &inconclusive));
ASSERT_EQUALS(false, inconclusive);
code = "void g(int a[2]);\n"
"void f() {\n"
" int b[2] = {};\n"
" return g(b);\n"
"}\n";
inconclusive = false;
ASSERT_EQUALS(true, isVariableChangedByFunctionCall(code, "b ) ;", &inconclusive));
ASSERT_EQUALS(false, inconclusive);
code = "void g(const int a[2]);\n"
"void f() {\n"
" int b[2] = {};\n"
" return g(b);\n"
"}\n";
inconclusive = false;
TODO_ASSERT_EQUALS(false, true, isVariableChangedByFunctionCall(code, "b ) ;", &inconclusive));
ASSERT_EQUALS(false, inconclusive);
code = "void g(std::array<int, 2> a);\n"
"void f() {\n"
" std::array<int, 2> b = {};\n"
" return g(b);\n"
"}\n";
inconclusive = false;
ASSERT_EQUALS(false, isVariableChangedByFunctionCall(code, "b ) ;", &inconclusive));
ASSERT_EQUALS(false, inconclusive);
code = "void g(std::array<int, 2>& a);\n"
"void f() {\n"
" std::array<int, 2> b = {};\n"
" return g(b);\n"
"}\n";
inconclusive = false;
ASSERT_EQUALS(true, isVariableChangedByFunctionCall(code, "b ) ;", &inconclusive));
ASSERT_EQUALS(false, inconclusive);
code = "void g(const std::array<int, 2>& a);\n"
"void f() {\n"
" std::array<int, 2> b = {};\n"
" return g(b);\n"
"}\n";
inconclusive = false;
ASSERT_EQUALS(false, isVariableChangedByFunctionCall(code, "b ) ;", &inconclusive));
ASSERT_EQUALS(false, inconclusive);
code = "void g(std::array<int, 2>* p);\n"
"void f() {\n"
" std::array<int, 2> b = {};\n"
" return g(&b);\n"
"}\n";
inconclusive = false;
ASSERT_EQUALS(true, isVariableChangedByFunctionCall(code, "b ) ;", &inconclusive));
ASSERT_EQUALS(false, inconclusive);
code = "struct S {};\n"
"void g(S);\n"
"void f(S* s) {\n"
" g(*s);\n"
"}\n";
inconclusive = false;
ASSERT_EQUALS(false, isVariableChangedByFunctionCall(code, "s ) ;", &inconclusive));
ASSERT_EQUALS(false, inconclusive);
code = "struct S {};\n"
"void g(const S&);\n"
"void f(S* s) {\n"
" g(*s);\n"
"}\n";
inconclusive = false;
ASSERT_EQUALS(false, isVariableChangedByFunctionCall(code, "s ) ;", &inconclusive));
ASSERT_EQUALS(false, inconclusive);
}
#define isExpressionChanged(code, var, startPattern, endPattern) \

37
test/testother.cpp
View File

@ -1419,6 +1419,19 @@ private:
" }\n"
" return *iter;\n"
"}");
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'vec' can be declared as reference to const\n", errout.str());
check("auto& foo(std::vector<int>& vec, bool flag) {\n"
" std::vector<int> dummy;\n"
" std::vector<int>::iterator iter;\n"
" if (flag)\n"
" iter = vec.begin();\n"
" else {\n"
" dummy.push_back(42);\n"
" iter = dummy.begin();\n"
" }\n"
" return *iter;\n"
"}");
ASSERT_EQUALS("", errout.str());
}
@ -3073,6 +3086,14 @@ private:
" return r;\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("template <typename T> void f(std::vector<T*>& d, const std::vector<T*>& s) {\n" // #11632
" for (const auto& e : s) {\n"
" T* newE = new T(*e);\n"
" d.push_back(newE);\n"
" }\n"
"}\n");
ASSERT_EQUALS("", errout.str());
}
void constParameterCallback() {
@ -3406,6 +3427,8 @@ private:
"void h(const S&);\n"
"void h(int, int, const S&);\n"
"void i(S&);\n"
"void j(const S*);\n"
"void j(int, int, const S*);\n"
"void f1(S* s) {\n"
" g(*s);\n"
"}\n"
@ -3417,10 +3440,18 @@ private:
"}\n"
"void f4(S* s) {\n"
" i(*s);\n"
"}\n"
"void f5(S& s) {\n"
" j(&s);\n"
"}\n"
"void f6(S& s) {\n"
" j(1, 2, &s);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:6]: (style) Parameter 's' can be declared as pointer to const\n"
"[test.cpp:9]: (style) Parameter 's' can be declared as pointer to const\n"
"[test.cpp:12]: (style) Parameter 's' can be declared as pointer to const\n",
ASSERT_EQUALS("[test.cpp:20]: (style) Parameter 's' can be declared as reference to const\n"
"[test.cpp:23]: (style) Parameter 's' can be declared as reference to const\n"
"[test.cpp:8]: (style) Parameter 's' can be declared as pointer to const\n"
"[test.cpp:11]: (style) Parameter 's' can be declared as pointer to const\n"
"[test.cpp:14]: (style) Parameter 's' can be declared as pointer to const\n",
errout.str());
}

6
test/testuninitvar.cpp
View File

@ -6551,11 +6551,7 @@ private:
" foo(123, &abc);\n"
" return abc.b;\n"
"}");
TODO_ASSERT_EQUALS("[test.cpp:5]: (error) Uninitialized variable: abc.a\n"
"[test.cpp:5]: (error) Uninitialized variable: abc.b\n"
"[test.cpp:5]: (error) Uninitialized variable: abc.c\n",
"",
errout.str());
ASSERT_EQUALS("[test.cpp:5]: (error) Uninitialized variable: &abc\n", errout.str());
valueFlowUninit("struct ABC { int a; int b; int c; };\n"
"void foo() {\n"