Skip to content

Commit 828a5e2

Browse files
pfultz2danmar
pfultz2
authored and
danmar
committed
Fix issue 9930: valueFlowLifetime hang
1 parent c373be0 commit 828a5e2

2 files changed

Lines changed: 159 additions & 61 deletions

File tree

‎lib/valueflow.cpp‎

Lines changed: 129 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -3387,18 +3387,54 @@ struct LifetimeStore {
33873387
ValueFlow::Value::LifetimeKind type;
33883388
ErrorPath errorPath;
33893389
bool inconclusive;
3390+
bool forward;
3391+
3392+
struct Context {
3393+
Token* tok;
3394+
TokenList* tokenlist;
3395+
ErrorLogger* errorLogger;
3396+
const Settings* settings;
3397+
};
33903398

33913399
LifetimeStore()
3392-
: argtok(nullptr), message(), type(), errorPath(), inconclusive(false)
3400+
: argtok(nullptr), message(), type(), errorPath(), inconclusive(false), forward(true), mContext(nullptr)
33933401
{}
33943402

3395-
LifetimeStore(const Token *argtok,
3396-
const std::string &message,
3403+
LifetimeStore(const Token* argtok,
3404+
const std::string& message,
33973405
ValueFlow::Value::LifetimeKind type = ValueFlow::Value::LifetimeKind::Object,
33983406
bool inconclusive = false)
3399-
: argtok(argtok), message(message), type(type), errorPath(), inconclusive(inconclusive)
3407+
: argtok(argtok),
3408+
message(message),
3409+
type(type),
3410+
errorPath(),
3411+
inconclusive(inconclusive),
3412+
forward(true),
3413+
mContext(nullptr)
34003414
{}
34013415

3416+
template <class F>
3417+
static void forEach(const std::vector<const Token*>& argtoks,
3418+
const std::string& message,
3419+
ValueFlow::Value::LifetimeKind type,
3420+
F f)
3421+
{
3422+
std::map<const Token*, Context> forwardToks;
3423+
for (const Token* arg : argtoks) {
3424+
LifetimeStore ls{arg, message, type};
3425+
Context c{};
3426+
ls.mContext = &c;
3427+
ls.forward = false;
3428+
f(ls);
3429+
if (c.tok)
3430+
forwardToks[c.tok] = c;
3431+
}
3432+
for (const auto& p : forwardToks) {
3433+
const Context& c = p.second;
3434+
valueFlowForwardLifetime(c.tok, c.tokenlist, c.errorLogger, c.settings);
3435+
}
3436+
}
3437+
34023438
static LifetimeStore fromFunctionArg(const Function * f, Token *tok, const Variable *var, TokenList *tokenlist, ErrorLogger *errorLogger) {
34033439
if (!var)
34043440
return LifetimeStore{};
@@ -3423,18 +3459,20 @@ struct LifetimeStore {
34233459
}
34243460

34253461
template <class Predicate>
3426-
void byRef(Token *tok, TokenList *tokenlist, ErrorLogger *errorLogger, const Settings *settings, Predicate pred) const {
3462+
bool byRef(Token* tok, TokenList* tokenlist, ErrorLogger* errorLogger, const Settings* settings, Predicate pred) const
3463+
{
34273464
if (!argtok)
3428-
return;
3465+
return false;
3466+
bool update = false;
34293467
for (const LifetimeToken& lt : getLifetimeTokens(argtok)) {
34303468
if (!settings->inconclusive && lt.inconclusive)
34313469
continue;
34323470
ErrorPath er = errorPath;
34333471
er.insert(er.end(), lt.errorPath.begin(), lt.errorPath.end());
34343472
if (!lt.token)
3435-
return;
3473+
return false;
34363474
if (!pred(lt.token))
3437-
return;
3475+
return false;
34383476
er.emplace_back(argtok, message);
34393477

34403478
ValueFlow::Value value;
@@ -3446,22 +3484,26 @@ struct LifetimeStore {
34463484
value.setInconclusive(lt.inconclusive || inconclusive);
34473485
// Don't add the value a second time
34483486
if (std::find(tok->values().begin(), tok->values().end(), value) != tok->values().end())
3449-
return;
3487+
return false;
34503488
setTokenValue(tok, value, tokenlist->getSettings());
3451-
valueFlowForwardLifetime(tok, tokenlist, errorLogger, settings);
3489+
update = true;
34523490
}
3491+
if (update && forward)
3492+
forwardLifetime(tok, tokenlist, errorLogger, settings);
3493+
return update;
34533494
}
34543495

3455-
void byRef(Token *tok, TokenList *tokenlist, ErrorLogger *errorLogger, const Settings *settings) const {
3456-
byRef(tok, tokenlist, errorLogger, settings, [](const Token *) {
3457-
return true;
3458-
});
3496+
bool byRef(Token* tok, TokenList* tokenlist, ErrorLogger* errorLogger, const Settings* settings) const
3497+
{
3498+
return byRef(tok, tokenlist, errorLogger, settings, [](const Token*) { return true; });
34593499
}
34603500

34613501
template <class Predicate>
3462-
void byVal(Token *tok, TokenList *tokenlist, ErrorLogger *errorLogger, const Settings *settings, Predicate pred) const {
3502+
bool byVal(Token* tok, TokenList* tokenlist, ErrorLogger* errorLogger, const Settings* settings, Predicate pred) const
3503+
{
34633504
if (!argtok)
3464-
return;
3505+
return false;
3506+
bool update = false;
34653507
if (argtok->values().empty()) {
34663508
ErrorPath er;
34673509
er.emplace_back(argtok, message);
@@ -3476,9 +3518,9 @@ struct LifetimeStore {
34763518
value.setInconclusive(inconclusive);
34773519
// Don't add the value a second time
34783520
if (std::find(tok->values().begin(), tok->values().end(), value) != tok->values().end())
3479-
return;
3521+
return false;
34803522
setTokenValue(tok, value, tokenlist->getSettings());
3481-
valueFlowForwardLifetime(tok, tokenlist, errorLogger, settings);
3523+
update = true;
34823524
}
34833525
}
34843526
for (const ValueFlow::Value &v : argtok->values()) {
@@ -3491,9 +3533,9 @@ struct LifetimeStore {
34913533
ErrorPath er = v.errorPath;
34923534
er.insert(er.end(), lt.errorPath.begin(), lt.errorPath.end());
34933535
if (!lt.token)
3494-
return;
3536+
return false;
34953537
if (!pred(lt.token))
3496-
return;
3538+
return false;
34973539
er.emplace_back(argtok, message);
34983540
er.insert(er.end(), errorPath.begin(), errorPath.end());
34993541

@@ -3509,15 +3551,17 @@ struct LifetimeStore {
35093551
if (std::find(tok->values().begin(), tok->values().end(), value) != tok->values().end())
35103552
continue;
35113553
setTokenValue(tok, value, tokenlist->getSettings());
3512-
valueFlowForwardLifetime(tok, tokenlist, errorLogger, settings);
3554+
update = true;
35133555
}
35143556
}
3557+
if (update && forward)
3558+
forwardLifetime(tok, tokenlist, errorLogger, settings);
3559+
return update;
35153560
}
35163561

3517-
void byVal(Token *tok, TokenList *tokenlist, ErrorLogger *errorLogger, const Settings *settings) const {
3518-
byVal(tok, tokenlist, errorLogger, settings, [](const Token *) {
3519-
return true;
3520-
});
3562+
bool byVal(Token* tok, TokenList* tokenlist, ErrorLogger* errorLogger, const Settings* settings) const
3563+
{
3564+
return byVal(tok, tokenlist, errorLogger, settings, [](const Token*) { return true; });
35213565
}
35223566

35233567
template <class Predicate>
@@ -3549,6 +3593,19 @@ struct LifetimeStore {
35493593
return true;
35503594
});
35513595
}
3596+
3597+
private:
3598+
Context* mContext;
3599+
void forwardLifetime(Token* tok, TokenList* tokenlist, ErrorLogger* errorLogger, const Settings* settings) const
3600+
{
3601+
if (mContext) {
3602+
mContext->tok = tok;
3603+
mContext->tokenlist = tokenlist;
3604+
mContext->errorLogger = errorLogger;
3605+
mContext->settings = settings;
3606+
}
3607+
valueFlowForwardLifetime(tok, tokenlist, errorLogger, settings);
3608+
}
35523609
};
35533610

35543611
static void valueFlowLifetimeFunction(Token *tok, TokenList *tokenlist, ErrorLogger *errorLogger, const Settings *settings)
@@ -3603,14 +3660,16 @@ static void valueFlowLifetimeFunction(Token *tok, TokenList *tokenlist, ErrorLog
36033660
return;
36043661
std::vector<const Token*> returns = Function::findReturns(f);
36053662
const bool inconclusive = returns.size() > 1;
3663+
bool update = false;
36063664
for (const Token* returnTok : returns) {
36073665
if (returnTok == tok)
36083666
continue;
36093667
const Variable *returnVar = getLifetimeVariable(returnTok);
36103668
if (returnVar && returnVar->isArgument() && (returnVar->isConst() || !isVariableChanged(returnVar, settings, tokenlist->isCPP()))) {
36113669
LifetimeStore ls = LifetimeStore::fromFunctionArg(f, tok, returnVar, tokenlist, errorLogger);
36123670
ls.inconclusive = inconclusive;
3613-
ls.byVal(tok->next(), tokenlist, errorLogger, settings);
3671+
ls.forward = false;
3672+
update |= ls.byVal(tok->next(), tokenlist, errorLogger, settings);
36143673
}
36153674
for (const ValueFlow::Value &v : returnTok->values()) {
36163675
if (!v.isLifetimeValue())
@@ -3621,16 +3680,19 @@ static void valueFlowLifetimeFunction(Token *tok, TokenList *tokenlist, ErrorLog
36213680
LifetimeStore ls = LifetimeStore::fromFunctionArg(f, tok, var, tokenlist, errorLogger);
36223681
if (!ls.argtok)
36233682
continue;
3683+
ls.forward = false;
36243684
ls.inconclusive = inconclusive;
36253685
ls.errorPath = v.errorPath;
36263686
ls.errorPath.emplace_front(returnTok, "Return " + lifetimeType(returnTok, &v) + ".");
36273687
if (!v.isArgumentLifetimeValue() && (var->isReference() || var->isRValueReference())) {
3628-
ls.byRef(tok->next(), tokenlist, errorLogger, settings);
3688+
update |= ls.byRef(tok->next(), tokenlist, errorLogger, settings);
36293689
} else if (v.isArgumentLifetimeValue()) {
3630-
ls.byVal(tok->next(), tokenlist, errorLogger, settings);
3690+
update |= ls.byVal(tok->next(), tokenlist, errorLogger, settings);
36313691
}
36323692
}
36333693
}
3694+
if (update)
3695+
valueFlowForwardLifetime(tok->next(), tokenlist, errorLogger, settings);
36343696
}
36353697
}
36363698

@@ -3648,11 +3710,11 @@ static void valueFlowLifetimeConstructor(Token* tok,
36483710
// If the type is unknown then assume it captures by value in the
36493711
// constructor, but make each lifetime inconclusive
36503712
std::vector<const Token*> args = getArguments(tok);
3651-
for (const Token *argtok : args) {
3652-
LifetimeStore ls{argtok, "Passed to initializer list.", ValueFlow::Value::LifetimeKind::Object};
3653-
ls.inconclusive = true;
3654-
ls.byVal(tok, tokenlist, errorLogger, settings);
3655-
}
3713+
LifetimeStore::forEach(
3714+
args, "Passed to initializer list.", ValueFlow::Value::LifetimeKind::Object, [&](LifetimeStore& ls) {
3715+
ls.inconclusive = true;
3716+
ls.byVal(tok, tokenlist, errorLogger, settings);
3717+
});
36563718
return;
36573719
}
36583720
const Scope* scope = t->classScope;
@@ -3661,20 +3723,21 @@ static void valueFlowLifetimeConstructor(Token* tok,
36613723
// Only support aggregate constructors for now
36623724
if (scope->numConstructors == 0 && t->derivedFrom.empty() && (t->isClassType() || t->isStructType())) {
36633725
std::vector<const Token*> args = getArguments(tok);
3664-
std::size_t i = 0;
3665-
for (const Variable& var : scope->varlist) {
3666-
if (i >= args.size())
3667-
break;
3668-
const Token* argtok = args[i];
3669-
LifetimeStore ls{
3670-
argtok, "Passed to constructor of '" + t->name() + "'.", ValueFlow::Value::LifetimeKind::Object};
3671-
if (var.isReference() || var.isRValueReference()) {
3672-
ls.byRef(tok, tokenlist, errorLogger, settings);
3673-
} else {
3674-
ls.byVal(tok, tokenlist, errorLogger, settings);
3675-
}
3676-
i++;
3677-
}
3726+
auto it = scope->varlist.begin();
3727+
LifetimeStore::forEach(args,
3728+
"Passed to constructor of '" + t->name() + "'.",
3729+
ValueFlow::Value::LifetimeKind::Object,
3730+
[&](const LifetimeStore& ls) {
3731+
if (it == scope->varlist.end())
3732+
return;
3733+
const Variable& var = *it;
3734+
if (var.isReference() || var.isRValueReference()) {
3735+
ls.byRef(tok, tokenlist, errorLogger, settings);
3736+
} else {
3737+
ls.byVal(tok, tokenlist, errorLogger, settings);
3738+
}
3739+
it++;
3740+
});
36783741
}
36793742
}
36803743

@@ -3704,15 +3767,15 @@ static void valueFlowLifetimeConstructor(Token* tok, TokenList* tokenlist, Error
37043767
std::vector<const Token *> args = getArguments(tok);
37053768
// Assume range constructor if passed a pair of iterators
37063769
if (astIsContainer(parent) && args.size() == 2 && astIsIterator(args[0]) && astIsIterator(args[1])) {
3707-
for (const Token *argtok : args) {
3708-
LifetimeStore ls{argtok, "Passed to initializer list.", ValueFlow::Value::LifetimeKind::Object};
3709-
ls.byDerefCopy(tok, tokenlist, errorLogger, settings);
3710-
}
3770+
LifetimeStore::forEach(
3771+
args, "Passed to initializer list.", ValueFlow::Value::LifetimeKind::Object, [&](const LifetimeStore& ls) {
3772+
ls.byDerefCopy(tok, tokenlist, errorLogger, settings);
3773+
});
37113774
} else {
3712-
for (const Token *argtok : args) {
3713-
LifetimeStore ls{argtok, "Passed to initializer list.", ValueFlow::Value::LifetimeKind::Object};
3714-
ls.byVal(tok, tokenlist, errorLogger, settings);
3715-
}
3775+
LifetimeStore::forEach(args,
3776+
"Passed to initializer list.",
3777+
ValueFlow::Value::LifetimeKind::Object,
3778+
[&](const LifetimeStore& ls) { ls.byVal(tok, tokenlist, errorLogger, settings); });
37163779
}
37173780
} else {
37183781
valueFlowLifetimeConstructor(tok, Token::typeOf(tok->previous()), tokenlist, errorLogger, settings);
@@ -3822,16 +3885,21 @@ static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase*, ErrorLogger
38223885
return true;
38233886
};
38243887

3888+
bool update = false;
38253889
auto captureVariable = [&](const Token* tok2, Lambda::Capture c, std::function<bool(const Token*)> pred) {
38263890
if (varids.count(tok->varId()) > 0)
38273891
return;
38283892
ErrorPath errorPath;
38293893
if (c == Lambda::Capture::ByReference) {
3830-
LifetimeStore{tok2, "Lambda captures variable by reference here.", ValueFlow::Value::LifetimeKind::Lambda} .byRef(
3831-
tok, tokenlist, errorLogger, settings, pred);
3894+
LifetimeStore ls{
3895+
tok2, "Lambda captures variable by reference here.", ValueFlow::Value::LifetimeKind::Lambda};
3896+
ls.forward = false;
3897+
update |= ls.byRef(tok, tokenlist, errorLogger, settings, pred);
38323898
} else if (c == Lambda::Capture::ByValue) {
3833-
LifetimeStore{tok2, "Lambda captures variable by value here.", ValueFlow::Value::LifetimeKind::Lambda} .byVal(
3834-
tok, tokenlist, errorLogger, settings, pred);
3899+
LifetimeStore ls{
3900+
tok2, "Lambda captures variable by value here.", ValueFlow::Value::LifetimeKind::Lambda};
3901+
ls.forward = false;
3902+
update |= ls.byVal(tok, tokenlist, errorLogger, settings, pred);
38353903
}
38363904
};
38373905

@@ -3853,8 +3921,8 @@ static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase*, ErrorLogger
38533921
continue;
38543922
captureVariable(tok2, lam.implicitCapture, isImplicitCapturingVariable);
38553923
}
3856-
3857-
valueFlowForwardLifetime(tok, tokenlist, errorLogger, settings);
3924+
if (update)
3925+
valueFlowForwardLifetime(tok, tokenlist, errorLogger, settings);
38583926
}
38593927
// address of
38603928
else if (tok->isUnaryOp("&")) {

‎test/testvalueflow.cpp‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4897,6 +4897,36 @@ class TestValueFlow : public TestFixture {
48974897
" ) {}\n"
48984898
"}\n";
48994899
valueOfTok(code, "x");
4900+
4901+
code = "namespace {\n"
4902+
"struct a {\n"
4903+
" a(...) {}\n"
4904+
" a(std::initializer_list<std::pair<int, std::vector<std::vector<a>>>>) {}\n"
4905+
"} b{{0, {{&b, &b, &b, &b}}},\n"
4906+
" {0,\n"
4907+
" {{&b, &b, &b, &b, &b, &b, &b, &b, &b, &b},\n"
4908+
" {{&b, &b, &b, &b, &b, &b, &b}}}},\n"
4909+
" {0,\n"
4910+
" {{&b, &b, &b, &b, &b, &b, &b, &b, &b, &b, &b, &b, &b, &b},\n"
4911+
" {&b, &b, &b, &b, &b, &b, &b, &b, &b, &b, &b}}}};\n"
4912+
"}\n";
4913+
valueOfTok(code, "x");
4914+
4915+
code = "namespace {\n"
4916+
"struct a {\n"
4917+
" a(...) {}\n"
4918+
" a(std::initializer_list<std::pair<int, std::vector<std::vector<a>>>>) {}\n"
4919+
"} b{{0, {{&b}}},\n"
4920+
" {0, {{&b}}},\n"
4921+
" {0, {{&b}}},\n"
4922+
" {0, {{&b}}},\n"
4923+
" {0, {{&b}, {&b, &b, &b, &b, &b, &b, &b, &b, &b, &b, {&b}}}},\n"
4924+
" {0,\n"
4925+
" {{&b},\n"
4926+
" {&b, &b, &b, &b, &b, &b, &b, &b, &b, &b, &b, &b, &b, &b, &b, &b, &b, &b,\n"
4927+
" &b}}}};\n"
4928+
"}\n";
4929+
valueOfTok(code, "x");
49004930
}
49014931

49024932
void valueFlowCrashConstructorInitialization() { // #9577

0 commit comments

Comments
 (0)