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

Fix issue 10208: FP: knownConditionTrueFalse in for loop with function that assigns by ref (#3198)

This commit is contained in:
Paul Fultz II 2021-04-18 14:42:27 -05:00 committed by GitHub
parent 56773b82c4
commit 563c9dd9cc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 99 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
gui/threadhandler.cpp
View File

@ -198,7 +198,7 @@ void ThreadHandler::initialize(ResultsView *view)
this, &ThreadHandler::bughuntingReportLine);
}
void ThreadHandler::loadSettings(QSettings &settings)
void ThreadHandler::loadSettings(const QSettings &settings)
{
setThreadCount(settings.value(SETTINGS_CHECK_THREADS, 1).toInt());
}

2
gui/threadhandler.h
View File

@ -64,7 +64,7 @@ public:
* @brief Load settings
* @param settings QSettings to load settings from
*/
void loadSettings(QSettings &settings);
void loadSettings(const QSettings &settings);
/**
* @brief Save settings

28
lib/astutils.cpp
View File

@ -1774,12 +1774,14 @@ bool isVariableChangedByFunctionCall(const Token *tok, int indirect, const Setti
if (!arg)
continue;
conclusive = true;
if (addressOf || (indirect > 0 && arg->isPointer())) {
if (!arg->isConst())
if (addressOf || indirect > 0) {
if (!arg->isConst() && arg->isPointer())
return true;
// If const is applied to the pointer, then the value can still be modified
if (Token::simpleMatch(arg->typeEndToken(), "* const"))
return true;
if (!arg->isPointer())
return true;
}
if (!arg->isConst() && arg->isReference())
return true;
@ -1790,6 +1792,13 @@ bool isVariableChangedByFunctionCall(const Token *tok, int indirect, const Setti
return false;
}
bool isVariableChangedByFunctionCall(const Token* tok, int indirect, const Settings* settings)
{
bool inconclusive = false;
bool r = isVariableChangedByFunctionCall(tok, indirect, settings, &inconclusive);
return r || inconclusive;
}
bool isVariableChanged(const Token *tok, int indirect, const Settings *settings, bool cpp, int depth)
{
if (!tok)
@ -1837,13 +1846,17 @@ bool isVariableChanged(const Token *tok, int indirect, const Settings *settings,
const ValueType * valueType = var->valueType();
isConst = (valueType && valueType->pointer == 1 && valueType->constness == 1);
}
if (isConst)
return false;
const Token *ftok = tok->tokAt(2);
if (settings)
return !settings->library.isFunctionConst(ftok);
const Function * fun = ftok->function();
if (!isConst && (!fun || !fun->isConst()))
if (!fun)
return true;
else
return false;
return !fun->isConst();
}
const Token *ftok = tok2;
@ -1949,8 +1962,9 @@ Token* findVariableChanged(Token *start, const Token *end, int indirect, const n
if (globalvar && Token::Match(tok, "%name% ("))
// TODO: Is global variable really changed by function call?
return tok;
// Is aliased function call
if (Token::Match(tok, "%var% (") && std::any_of(tok->values().begin(), tok->values().end(), std::mem_fn(&ValueFlow::Value::isLifetimeValue))) {
// Is aliased function call or alias passed to function
if ((Token::Match(tok, "%var% (") || isVariableChangedByFunctionCall(tok, 1, settings)) &&
std::any_of(tok->values().begin(), tok->values().end(), std::mem_fn(&ValueFlow::Value::isLifetimeValue))) {
bool aliased = false;
// If we can't find the expression then assume it was modified
if (!getExprTok())

11
lib/checkautovariables.cpp
View File

@ -585,7 +585,16 @@ void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token
}
if (!isLifetimeBorrowed(tok, mSettings))
continue;
if (var && !var->isLocal() && !var->isArgument() && !isVariableChanged(tok->next(), tok->scope()->bodyEnd, var->declarationId(), var->isGlobal(), mSettings, mTokenizer->isCPP())) {
const Token* nextTok = nextAfterAstRightmostLeaf(tok->astTop());
if (!nextTok)
nextTok = tok->next();
if (var && !var->isLocal() && !var->isArgument() &&
!isVariableChanged(nextTok,
tok->scope()->bodyEnd,
var->declarationId(),
var->isGlobal(),
mSettings,
mTokenizer->isCPP())) {
errorDanglngLifetime(tok2, &val);
break;
}

3
lib/checkcondition.cpp
View File

@ -713,7 +713,8 @@ void CheckCondition::multiCondition2()
});
}
}
if (Token::Match(tok, "%name% (") && isVariablesChanged(tok, tok->linkAt(1), true, varsInCond, mSettings, mTokenizer->isCPP())) {
if (Token::Match(tok, "%name% (") &&
isVariablesChanged(tok, tok->linkAt(1), 0, varsInCond, mSettings, mTokenizer->isCPP())) {
break;
}
if (Token::Match(tok, "%type% (") && nonlocal && isNonConstFunctionCall(tok, mSettings->library)) // non const function call -> bailout if there are nonlocal variables

2
lib/library.cpp
View File

@ -1400,7 +1400,7 @@ bool Library::isFunctionConst(const std::string& functionName, bool pure) const
}
bool Library::isFunctionConst(const Token *ftok) const
{
if (ftok->function() && ftok->function()->isAttributeConst())
if (ftok->function() && ftok->function()->isConst())
return true;
if (isNotLibraryFunction(ftok))
return false;

15
lib/valueflow.cpp
View File

@ -6215,7 +6215,9 @@ static void valueFlowIteratorInfer(TokenList *tokenlist, const Settings *setting
}
}
static std::vector<ValueFlow::Value> getInitListSize(const Token* tok, const Library::Container *container)
static std::vector<ValueFlow::Value> getInitListSize(const Token* tok,
const Library::Container* container,
bool known = true)
{
std::vector<const Token*> args = getArguments(tok);
if ((args.size() == 1 && astIsContainer(args[0]) && args[0]->valueType()->container == container) ||
@ -6229,7 +6231,8 @@ static std::vector<ValueFlow::Value> getInitListSize(const Token* tok, const Lib
}
ValueFlow::Value value(args.size());
value.valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE;
value.setKnown();
if (known)
value.setKnown();
return {value};
}
@ -6238,6 +6241,7 @@ static void valueFlowContainerSize(TokenList *tokenlist, SymbolDatabase* symbold
std::map<int, std::size_t> static_sizes;
// declaration
for (const Variable *var : symboldatabase->variableList()) {
bool known = true;
if (!var || !var->isLocal() || var->isPointer() || var->isReference() || var->isStatic())
continue;
if (!var->valueType() || !var->valueType()->container)
@ -6249,6 +6253,8 @@ static void valueFlowContainerSize(TokenList *tokenlist, SymbolDatabase* symbold
if (!Token::Match(var->nameToken(), "%name% ;") &&
!(Token::Match(var->nameToken(), "%name% {") && Token::simpleMatch(var->nameToken()->next()->link(), "} ;")))
continue;
if (var->nameToken()->astTop() && Token::Match(var->nameToken()->astTop()->previous(), "for|while"))
known = !isVariableChanged(var, settings, true);
if (var->valueType()->container->size_templateArgNo >= 0) {
if (var->dimensions().size() == 1 && var->dimensions().front().known)
static_sizes[var->declarationId()] = var->dimensions().front().num;
@ -6256,10 +6262,11 @@ static void valueFlowContainerSize(TokenList *tokenlist, SymbolDatabase* symbold
}
std::vector<ValueFlow::Value> values{ValueFlow::Value{0}};
values.back().valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE;
values.back().setKnown();
if (known)
values.back().setKnown();
if (Token::simpleMatch(var->nameToken()->next(), "{")) {
const Token* initList = var->nameToken()->next();
values = getInitListSize(initList, var->valueType()->container);
values = getInitListSize(initList, var->valueType()->container, known);
}
for (const ValueFlow::Value& value : values)
valueFlowContainerForward(var->nameToken()->next(), var, value, tokenlist);

4
test/cfg/kde.cpp
View File

@ -14,7 +14,7 @@
class k_global_static_testclass1 {};
K_GLOBAL_STATIC(k_global_static_testclass1, k_global_static_testinstance1);
void valid_code(KConfigGroup &cfgGroup)
void valid_code(const KConfigGroup& cfgGroup)
{
k_global_static_testclass1 * pk_global_static_testclass1 = k_global_static_testinstance1;
printf("%p", pk_global_static_testclass1);
@ -23,7 +23,7 @@ void valid_code(KConfigGroup &cfgGroup)
if (entryTest) {}
}
void ignoredReturnValue(KConfigGroup & cfgGroup)
void ignoredReturnValue(const KConfigGroup& cfgGroup)
{
// cppcheck-suppress ignoredReturnValue
cfgGroup.readEntry("test", "default");

2
test/cfg/wxwidgets.cpp
View File

@ -110,7 +110,7 @@ bool invalidFunctionArgBool_wxPGProperty_Hide(wxPGProperty *pg, bool hide, int f
return pg->Hide(hide, flags);
}
wxTextCtrlHitTestResult nullPointer_wxTextCtrl_HitTest(wxTextCtrl &txtCtrl, const wxPoint &pos)
wxTextCtrlHitTestResult nullPointer_wxTextCtrl_HitTest(const wxTextCtrl& txtCtrl, const wxPoint& pos)
{
// no nullPointer-warning is expected
return txtCtrl.HitTest(pos, NULL);

49
test/testcondition.cpp
View File

@ -3561,6 +3561,43 @@ private:
" return false;\n"
"}\n");
ASSERT_EQUALS("", errout.str());
// #10208
check("bool GetFirst(std::string &first);\n"
"bool GetNext(std::string &next);\n"
"void g(const std::string& name);\n"
"void f() {\n"
" for (std::string name; name.empty() ? GetFirst(name) : GetNext(name);)\n"
" g(name);\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("bool GetFirst(std::string &first);\n"
"bool GetNext(std::string &next);\n"
"void g(const std::string& name);\n"
"void f() {\n"
" for (std::string name{}; name.empty() ? GetFirst(name) : GetNext(name);)\n"
" g(name);\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("bool GetFirst(std::string &first);\n"
"bool GetNext(std::string &next);\n"
"void g(const std::string& name);\n"
"void f() {\n"
" for (std::string name{'a', 'b'}; name.empty() ? GetFirst(name) : GetNext(name);)\n"
" g(name);\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("bool GetFirst(const std::string &first);\n"
"bool GetNext(const std::string &next);\n"
"void g(const std::string& name);\n"
"void f() {\n"
" for (std::string name; name.empty() ? GetFirst(name) : GetNext(name);)\n"
" g(name);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:5]: (style) Condition 'name.empty()' is always true\n", errout.str());
}
void alwaysTrueInfer() {
@ -3947,6 +3984,18 @@ private:
"}\n");
ASSERT_EQUALS("", errout.str());
check("void g(std::function<void()>);\n"
"void f(std::vector<int> v) {\n"
" auto x = [&v] { v.push_back(1); };\n"
" if(v.empty()) {\n"
" g(x);\n"
" }\n"
" if(v.empty())\n"
" return;\n"
" return;\n"
"}\n");
ASSERT_EQUALS("", errout.str());
// do not crash
check("void assign(const MMA& other) {\n"
" if (mPA.cols != other.mPA.cols || mPA.rows != other.mPA.rows)\n"