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

Fix issue 9756: false negative: invalid iterator from std::find_if (#2760)

This commit is contained in:
Paul Fultz II 2020-08-28 12:29:09 -05:00 committed by GitHub
parent 82bdbcd73b
commit 6ab3c93fb1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 121 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
lib/checkstl.h
View File

@ -84,7 +84,7 @@ public:
checkStl.knownEmptyContainerLoop();
checkStl.stlBoundaries();
checkStl.checkDereferenceInvalidIterator();
checkStl.checkDereferenceInvalidIterator();
checkStl.checkDereferenceInvalidIterator2();
checkStl.checkMutexes();

107
lib/valueflow.cpp
View File

@ -1703,6 +1703,14 @@ static void valueFlowGlobalStaticVar(TokenList *tokenList, const Settings *setti
}
}
static void valueFlowForwardVariable(Token* const startToken,
const Token* const endToken,
const Variable* const var,
std::list<ValueFlow::Value> values,
TokenList* const tokenlist,
const Settings* const settings);
// Old deprecated version
static void valueFlowForward(Token* startToken,
const Token* endToken,
const Token* exprTok,
@ -2696,6 +2704,17 @@ static void valueFlowForwardVariable(Token* const startToken,
}
}
static void valueFlowForwardVariable(Token* const startToken,
const Token* const endToken,
const Variable* const var,
std::list<ValueFlow::Value> values,
TokenList* const tokenlist,
const Settings* const settings)
{
valueFlowForwardVariable(startToken, endToken, var, std::move(values), getAliasesFromValues(values), tokenlist, settings);
}
// Old deprecated version
static bool valueFlowForwardVariable(Token* const startToken,
const Token* const endToken,
const Variable* const var,
@ -2707,7 +2726,7 @@ static bool valueFlowForwardVariable(Token* const startToken,
ErrorLogger* const,
const Settings* const settings)
{
valueFlowForwardVariable(startToken, endToken, var, std::move(values), getAliasesFromValues(values), tokenlist, settings);
valueFlowForwardVariable(startToken, endToken, var, std::move(values), tokenlist, settings);
return true;
}
@ -2857,10 +2876,7 @@ static void valueFlowForward(Token* startToken,
const Token* endToken,
const Token* exprTok,
std::list<ValueFlow::Value> values,
const bool constValue,
const bool subFunction,
TokenList* const tokenlist,
ErrorLogger* const errorLogger,
const Settings* settings)
{
const Token* expr = solveExprValues(exprTok, values);
@ -2868,18 +2884,28 @@ static void valueFlowForward(Token* startToken,
valueFlowForwardVariable(startToken,
endToken,
expr->variable(),
expr->varId(),
values,
constValue,
subFunction,
tokenlist,
errorLogger,
settings);
} else {
valueFlowForwardExpression(startToken, endToken, expr, values, tokenlist, settings);
}
}
// Old deprecated version
static void valueFlowForward(Token* startToken,
const Token* endToken,
const Token* exprTok,
std::list<ValueFlow::Value> values,
const bool,
const bool,
TokenList* const tokenlist,
ErrorLogger* const,
const Settings* settings)
{
valueFlowForward(startToken, endToken, exprTok, std::move(values), tokenlist, settings);
}
static int getArgumentPos(const Variable *var, const Function *f)
{
auto arg_it = std::find_if(f->argumentList.begin(), f->argumentList.end(), [&](const Variable &v) {
@ -4441,8 +4467,8 @@ static void valueFlowAfterCondition(TokenList *tokenlist,
const Token* stop,
const Token* vartok,
const std::list<ValueFlow::Value>& values,
bool constValue) {
valueFlowForward(start->next(), stop, vartok, values, constValue, false, tokenlist, errorLogger, settings);
bool) {
valueFlowForward(start->next(), stop, vartok, values, tokenlist, settings);
std::vector<const Variable*> vars = getExprVariables(vartok, tokenlist, symboldatabase, settings);
return isVariablesChanged(start, stop, 0, vars, settings, tokenlist->isCPP());
};
@ -5913,6 +5939,60 @@ static void valueFlowIterators(TokenList *tokenlist, const Settings *settings)
}
}
static std::list<ValueFlow::Value> getIteratorValues(std::list<ValueFlow::Value> values)
{
values.remove_if([&](const ValueFlow::Value& v) {
return !v.isIteratorEndValue() && !v.isIteratorStartValue();
});
return values;
}
static void valueFlowIteratorAfterCondition(TokenList *tokenlist,
SymbolDatabase *symboldatabase,
ErrorLogger *errorLogger,
const Settings *settings)
{
ValueFlowConditionHandler handler;
handler.forward = [&](Token* start,
const Token* stop,
const Token* vartok,
const std::list<ValueFlow::Value>& values,
bool) {
valueFlowForward(start->next(), stop, vartok, values, tokenlist, settings);
std::vector<const Variable*> vars = getExprVariables(vartok, tokenlist, symboldatabase, settings);
return isVariablesChanged(start, stop, 0, vars, settings, tokenlist->isCPP());
};
handler.parse = [&](const Token *tok) {
ValueFlowConditionHandler::Condition cond;
ValueFlow::Value true_value;
ValueFlow::Value false_value;
if (Token::Match(tok, "==|!=")) {
if (!tok->astOperand1() || !tok->astOperand2())
return cond;
std::list<ValueFlow::Value> values = getIteratorValues(tok->astOperand1()->values());
if (!values.empty()) {
cond.vartok = tok->astOperand2();
} else {
values = getIteratorValues(tok->astOperand2()->values());
if (!values.empty())
cond.vartok = tok->astOperand1();
}
for(ValueFlow::Value& v:values) {
v.setPossible();
v.assumeCondition(tok);
}
cond.true_values = values;
cond.false_values = values;
}
return cond;
};
handler.afterCondition(tokenlist, symboldatabase, errorLogger, settings);
}
static void valueFlowContainerSize(TokenList *tokenlist, SymbolDatabase* symboldatabase, ErrorLogger * /*errorLogger*/, const Settings *settings)
{
// declaration
@ -6411,6 +6491,12 @@ ValueFlow::Value::Value(const Token* c, long long val)
errorPath.emplace_back(c, "Assuming that condition '" + c->expressionString() + "' is not redundant");
}
void ValueFlow::Value::assumeCondition(const Token* tok)
{
condition = tok;
errorPath.emplace_back(tok, "Assuming that condition '" + tok->expressionString() + "' is not redundant");
}
std::string ValueFlow::Value::infoString() const
{
switch (valueType) {
@ -6509,6 +6595,7 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase,
if (tokenlist->isCPP()) {
valueFlowSmartPointer(tokenlist, errorLogger, settings);
valueFlowIterators(tokenlist, settings);
valueFlowIteratorAfterCondition(tokenlist, symboldatabase, errorLogger, settings);
valueFlowContainerSize(tokenlist, symboldatabase, errorLogger, settings);
valueFlowContainerAfterCondition(tokenlist, symboldatabase, errorLogger, settings);
}

2
lib/valueflow.h
View File

@ -170,6 +170,8 @@ namespace ValueFlow {
decreaseRange();
}
void assumeCondition(const Token* tok);
std::string infoString() const;
enum ValueType { INT, TOK, FLOAT, MOVED, UNINIT, CONTAINER_SIZE, LIFETIME, BUFFER_SIZE, ITERATOR_START, ITERATOR_END } valueType;

21
test/teststl.cpp
View File

@ -3619,6 +3619,27 @@ private:
" return *(v.begin() + pos);\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("int f(std::vector<int> v, int i) {\n"
" auto it = std::find(v.begin(), v.end(), i);\n"
" if (it != v.end()) {}\n"
" return *it;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (warning) Either the condition 'it!=v.end()' is redundant or there is possible dereference of an invalid iterator: it.\n", errout.str());
check("void f(std::vector<int> & v) {\n"
" std::vector<int>::iterator i= v.begin();\n"
" if(i == v.end() && *(i+1) == *i) {}\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:3]: (warning) Either the condition 'i==v.end()' is redundant or there is possible dereference of an invalid iterator: i+1.\n"
"[test.cpp:3] -> [test.cpp:3]: (warning) Either the condition 'i==v.end()' is redundant or there is possible dereference of an invalid iterator: i.\n", errout.str());
check("void f(std::vector<int> & v) {\n"
" std::vector<int>::iterator i= v.begin();\n"
" if(i == v.end() && *i == *(i+1)) {}\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:3]: (warning) Either the condition 'i==v.end()' is redundant or there is possible dereference of an invalid iterator: i.\n"
"[test.cpp:3] -> [test.cpp:3]: (warning) Either the condition 'i==v.end()' is redundant or there is possible dereference of an invalid iterator: i+1.\n", errout.str());
}
void dereferenceInvalidIterator2() {