Skip to content

Commit 8324caf

Browse files
authored
Fix 11557: FP derefInvalidIteratorRedundantCheck in and/or condition (#4892)
1 parent a4d2178 commit 8324caf

2 files changed

Lines changed: 50 additions & 23 deletions

File tree

lib/valueflow.cpp

Lines changed: 40 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5970,6 +5970,39 @@ struct ConditionHandler {
59705970
return findPath(true_values) | findPath(false_values);
59715971
}
59725972

5973+
Token* getContextAndValues(Token* condTok,
5974+
std::list<ValueFlow::Value>& thenValues,
5975+
std::list<ValueFlow::Value>& elseValues,
5976+
bool known = false) const
5977+
{
5978+
const MathLib::bigint path = getPath();
5979+
const bool allowKnown = path == 0;
5980+
const bool allowImpossible = impossible && allowKnown;
5981+
5982+
bool inverted2 = inverted;
5983+
Token* ctx = skipNotAndCasts(condTok, &inverted2);
5984+
bool then = !inverted || !inverted2;
5985+
5986+
if (!Token::Match(condTok, "!=|=|(|.") && condTok != vartok) {
5987+
thenValues.insert(thenValues.end(), true_values.cbegin(), true_values.cend());
5988+
if (allowImpossible && (known || isConditionKnown(ctx, !then)))
5989+
insertImpossible(elseValues, false_values);
5990+
}
5991+
if (!Token::Match(condTok, "==|!")) {
5992+
elseValues.insert(elseValues.end(), false_values.cbegin(), false_values.cend());
5993+
if (allowImpossible && (known || isConditionKnown(ctx, then))) {
5994+
insertImpossible(thenValues, true_values);
5995+
if (isBool())
5996+
insertNegateKnown(thenValues, true_values);
5997+
}
5998+
}
5999+
6000+
if (inverted2)
6001+
std::swap(thenValues, elseValues);
6002+
6003+
return ctx;
6004+
}
6005+
59736006
Condition() : vartok(nullptr), true_values(), false_values(), inverted(false), impossible(true) {}
59746007
};
59756008

@@ -6198,31 +6231,11 @@ struct ConditionHandler {
61986231

61996232
const MathLib::bigint path = cond.getPath();
62006233
const bool allowKnown = path == 0;
6201-
const bool allowImpossible = cond.impossible && allowKnown;
62026234

62036235
std::list<ValueFlow::Value> thenValues;
62046236
std::list<ValueFlow::Value> elseValues;
62056237

6206-
bool inverted = cond.inverted;
6207-
Token* ctx = skipNotAndCasts(condTok, &inverted);
6208-
bool then = cond.inverted ? !inverted : true;
6209-
6210-
if (!Token::Match(condTok, "!=|=|(|.") && condTok != cond.vartok) {
6211-
thenValues.insert(thenValues.end(), cond.true_values.cbegin(), cond.true_values.cend());
6212-
if (allowImpossible && isConditionKnown(ctx, !then))
6213-
insertImpossible(elseValues, cond.false_values);
6214-
}
6215-
if (!Token::Match(condTok, "==|!")) {
6216-
elseValues.insert(elseValues.end(), cond.false_values.cbegin(), cond.false_values.cend());
6217-
if (allowImpossible && isConditionKnown(ctx, then)) {
6218-
insertImpossible(thenValues, cond.true_values);
6219-
if (cond.isBool())
6220-
insertNegateKnown(thenValues, cond.true_values);
6221-
}
6222-
}
6223-
6224-
if (inverted)
6225-
std::swap(thenValues, elseValues);
6238+
Token* ctx = cond.getContextAndValues(condTok, thenValues, elseValues);
62266239

62276240
if (Token::Match(ctx->astParent(), "%oror%|&&")) {
62286241
Token* parent = ctx->astParent();
@@ -6237,12 +6250,16 @@ struct ConditionHandler {
62376250
if (astIsLHS(parent) && parent->astParent() && parent->astParent()->str() == parent->str()) {
62386251
nextExprs.push_back(parent->astParent()->astOperand2());
62396252
}
6253+
std::list<ValueFlow::Value> andValues;
6254+
std::list<ValueFlow::Value> orValues;
6255+
cond.getContextAndValues(condTok, andValues, orValues, true);
6256+
62406257
const std::string& op(parent->str());
62416258
std::list<ValueFlow::Value> values;
62426259
if (op == "&&")
6243-
values = thenValues;
6260+
values = andValues;
62446261
else if (op == "||")
6245-
values = elseValues;
6262+
values = orValues;
62466263
if (allowKnown && (Token::Match(condTok, "==|!=") || cond.isBool()))
62476264
changePossibleToKnown(values);
62486265
if (astIsFloat(cond.vartok, false) ||

test/teststl.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4740,6 +4740,16 @@ class TestStl : public TestFixture {
47404740
" return debug_valueflow(it)->second;\n"
47414741
"}\n");
47424742
ASSERT_EQUALS("", errout.str());
4743+
4744+
// #11557
4745+
check("bool f(const std::vector<int*>& v, std::vector<int*>::iterator it, bool b) {\n"
4746+
" if (it == v.end())\n"
4747+
" return false;\n"
4748+
" if (b && ((it + 1) == v.end() || (*(it + 1)) != nullptr))\n"
4749+
" return false;\n"
4750+
" return true;\n"
4751+
"}\n");
4752+
ASSERT_EQUALS("", errout.str());
47434753
}
47444754

47454755
void dereferenceInvalidIterator2() {

0 commit comments

Comments
 (0)