Skip to content

Commit 0335671

Browse files
pfultz2pfultz2
authored
Fix 10450: regression, FP : Iterator 'iter' from different container 'l' are used together (#3436)
1 parent cd5fa01 commit 0335671

4 files changed

Lines changed: 87 additions & 21 deletions

File tree

‎lib/astutils.cpp‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -807,7 +807,11 @@ static void followVariableExpressionError(const Token *tok1, const Token *tok2,
807807
errors->push_back(item);
808808
}
809809

810-
std::vector<ReferenceToken> followAllReferences(const Token* tok, bool inconclusive, ErrorPath errors, int depth)
810+
std::vector<ReferenceToken> followAllReferences(const Token* tok,
811+
bool temporary,
812+
bool inconclusive,
813+
ErrorPath errors,
814+
int depth)
811815
{
812816
struct ReferenceTokenLess {
813817
bool operator()(const ReferenceToken& x, const ReferenceToken& y) const {
@@ -831,10 +835,11 @@ std::vector<ReferenceToken> followAllReferences(const Token* tok, bool inconclus
831835
} else if (Token::simpleMatch(var->declEndToken(), "=")) {
832836
errors.emplace_back(var->declEndToken(), "Assigned to reference.");
833837
const Token *vartok = var->declEndToken()->astOperand2();
834-
if (vartok == tok)
838+
if (vartok == tok || (!temporary && isTemporary(true, vartok, nullptr, true) &&
839+
(var->isConst() || var->isRValueReference())))
835840
return {{tok, std::move(errors)}};
836841
if (vartok)
837-
return followAllReferences(vartok, inconclusive, std::move(errors), depth - 1);
842+
return followAllReferences(vartok, temporary, inconclusive, std::move(errors), depth - 1);
838843
} else {
839844
return {{tok, std::move(errors)}};
840845
}
@@ -844,9 +849,9 @@ std::vector<ReferenceToken> followAllReferences(const Token* tok, bool inconclus
844849
const Token* tok2 = tok->astOperand2();
845850

846851
std::vector<ReferenceToken> refs;
847-
refs = followAllReferences(tok2->astOperand1(), inconclusive, errors, depth - 1);
852+
refs = followAllReferences(tok2->astOperand1(), temporary, inconclusive, errors, depth - 1);
848853
result.insert(refs.begin(), refs.end());
849-
refs = followAllReferences(tok2->astOperand2(), inconclusive, errors, depth - 1);
854+
refs = followAllReferences(tok2->astOperand2(), temporary, inconclusive, errors, depth - 1);
850855
result.insert(refs.begin(), refs.end());
851856

852857
if (!inconclusive && result.size() != 1)
@@ -865,7 +870,8 @@ std::vector<ReferenceToken> followAllReferences(const Token* tok, bool inconclus
865870
for (const Token* returnTok : returns) {
866871
if (returnTok == tok)
867872
continue;
868-
for (const ReferenceToken& rt:followAllReferences(returnTok, inconclusive, errors, depth - returns.size())) {
873+
for (const ReferenceToken& rt :
874+
followAllReferences(returnTok, temporary, inconclusive, errors, depth - returns.size())) {
869875
const Variable* argvar = rt.token->variable();
870876
if (!argvar)
871877
return {{tok, std::move(errors)}};
@@ -880,7 +886,8 @@ std::vector<ReferenceToken> followAllReferences(const Token* tok, bool inconclus
880886
ErrorPath er = errors;
881887
er.emplace_back(returnTok, "Return reference.");
882888
er.emplace_back(tok->previous(), "Called function passing '" + argTok->expressionString() + "'.");
883-
std::vector<ReferenceToken> refs = followAllReferences(argTok, inconclusive, std::move(er), depth - returns.size());
889+
std::vector<ReferenceToken> refs =
890+
followAllReferences(argTok, temporary, inconclusive, std::move(er), depth - returns.size());
884891
result.insert(refs.begin(), refs.end());
885892
if (!inconclusive && result.size() > 1)
886893
return {{tok, std::move(errors)}};
@@ -898,7 +905,7 @@ const Token* followReferences(const Token* tok, ErrorPath* errors)
898905
{
899906
if (!tok)
900907
return nullptr;
901-
std::vector<ReferenceToken> refs = followAllReferences(tok, false);
908+
std::vector<ReferenceToken> refs = followAllReferences(tok, true, false);
902909
if (refs.size() == 1) {
903910
if (errors)
904911
*errors = refs.front().errors;

‎lib/astutils.h‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,11 @@ struct ReferenceToken {
145145
ErrorPath errors;
146146
};
147147

148-
std::vector<ReferenceToken> followAllReferences(const Token* tok, bool inconclusive = true, ErrorPath errors = ErrorPath{}, int depth = 20);
148+
std::vector<ReferenceToken> followAllReferences(const Token* tok,
149+
bool temporary = true,
150+
bool inconclusive = true,
151+
ErrorPath errors = ErrorPath{},
152+
int depth = 20);
149153
const Token* followReferences(const Token* tok, ErrorPath* errors = nullptr);
150154

151155
bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2, const Library& library, bool pure, bool followVar, ErrorPath* errors=nullptr);

‎lib/valueflow.cpp‎

Lines changed: 44 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3917,21 +3917,53 @@ static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase*, ErrorLogger
39173917
isContainerOfPointers = vt.pointer > 0;
39183918
}
39193919

3920-
LifetimeStore ls;
3921-
3922-
if (astIsIterator(parent->tokAt(2)))
3923-
ls = LifetimeStore{tok, "Iterator to container is created here.", ValueFlow::Value::LifetimeKind::Iterator};
3924-
else if ((astIsPointer(parent->tokAt(2)) && !isContainerOfPointers) || Token::Match(parent->next(), "data|c_str"))
3925-
ls = LifetimeStore{tok, "Pointer to container is created here.", ValueFlow::Value::LifetimeKind::Object};
3926-
else
3920+
ValueFlow::Value master;
3921+
master.valueType = ValueFlow::Value::ValueType::LIFETIME;
3922+
master.lifetimeScope = ValueFlow::Value::LifetimeScope::Local;
3923+
3924+
if (astIsIterator(parent->tokAt(2))) {
3925+
master.errorPath.emplace_back(parent->tokAt(2), "Iterator to container is created here.");
3926+
master.lifetimeKind = ValueFlow::Value::LifetimeKind::Iterator;
3927+
} else if ((astIsPointer(parent->tokAt(2)) && !isContainerOfPointers) ||
3928+
Token::Match(parent->next(), "data|c_str")) {
3929+
master.errorPath.emplace_back(parent->tokAt(2), "Pointer to container is created here.");
3930+
master.lifetimeKind = ValueFlow::Value::LifetimeKind::Object;
3931+
} else {
39273932
continue;
3933+
}
39283934

3929-
// Dereferencing
3930-
if (tok->isUnaryOp("*") || parent->originalName() == "->")
3931-
ls.byDerefCopy(parent->tokAt(2), tokenlist, errorLogger, settings);
3932-
else
3933-
ls.byRef(parent->tokAt(2), tokenlist, errorLogger, settings);
3935+
std::vector<const Token*> toks = {};
3936+
if (tok->isUnaryOp("*") || parent->originalName() == "->") {
3937+
for (const ValueFlow::Value& v : tok->values()) {
3938+
if (!v.isSymbolicValue())
3939+
continue;
3940+
if (v.isKnown())
3941+
continue;
3942+
if (v.intvalue != 0)
3943+
continue;
3944+
if (!v.tokvalue)
3945+
continue;
3946+
toks.push_back(v.tokvalue);
3947+
}
3948+
} else {
3949+
toks = {tok};
3950+
}
39343951

3952+
for (const Token* tok2 : toks) {
3953+
for (const ReferenceToken& rt : followAllReferences(tok2, false)) {
3954+
ValueFlow::Value value = master;
3955+
value.tokvalue = rt.token;
3956+
value.errorPath.insert(value.errorPath.begin(), rt.errors.begin(), rt.errors.end());
3957+
setTokenValue(parent->tokAt(2), value, tokenlist->getSettings());
3958+
3959+
if (!rt.token->variable()) {
3960+
LifetimeStore ls = LifetimeStore{
3961+
rt.token, master.errorPath.back().second, ValueFlow::Value::LifetimeKind::Object};
3962+
ls.byRef(parent->tokAt(2), tokenlist, errorLogger, settings);
3963+
}
3964+
}
3965+
}
3966+
valueFlowForwardLifetime(parent->tokAt(2), tokenlist, errorLogger, settings);
39353967
}
39363968
// Check constructors
39373969
else if (Token::Match(tok, "=|return|%type%|%var% {")) {

‎test/teststl.cpp‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ class TestStl : public TestFixture {
7070
TEST_CASE(iterator25); // #9742
7171
TEST_CASE(iterator26); // #9176
7272
TEST_CASE(iterator27); // #10378
73+
TEST_CASE(iterator28); // #10450
7374
TEST_CASE(iteratorExpression);
7475
TEST_CASE(iteratorSameExpression);
7576
TEST_CASE(mismatchingContainerIterator);
@@ -1487,6 +1488,28 @@ class TestStl : public TestFixture {
14871488
ASSERT_EQUALS("", errout.str());
14881489
}
14891490

1491+
void iterator28()
1492+
{
1493+
// #10450
1494+
check("struct S {\n"
1495+
" struct Private {\n"
1496+
" std::list<int> l;\n"
1497+
" };\n"
1498+
" std::unique_ptr<Private> p;\n"
1499+
" int foo();\n"
1500+
"};\n"
1501+
"int S::foo() {\n"
1502+
" for(auto iter = p->l.begin(); iter != p->l.end(); ++iter) {\n"
1503+
" if(*iter == 1) {\n"
1504+
" p->l.erase(iter);\n"
1505+
" return 1;\n"
1506+
" }\n"
1507+
" }\n"
1508+
" return 0;\n"
1509+
"}\n");
1510+
ASSERT_EQUALS("", errout.str());
1511+
}
1512+
14901513
void iteratorExpression() {
14911514
check("std::vector<int>& f();\n"
14921515
"std::vector<int>& g();\n"

0 commit comments

Comments
 (0)