Skip to content

Commit 27ebff7

Browse files
pfultz2danmar
pfultz2
authored and
danmar
committed
Add deeper analysis of when a function changes a containers size (#2149)
* Add deeper analysis of when a function changes a containers size * Fix issues * Track addressOf
1 parent 4531b31 commit 27ebff7

4 files changed

Lines changed: 180 additions & 32 deletions

File tree

‎lib/astutils.cpp‎

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -955,16 +955,9 @@ static bool isScopeBracket(const Token *tok)
955955
return false;
956956
}
957957

958-
bool isVariableChangedByFunctionCall(const Token *tok, int indirect, const Settings *settings, bool *inconclusive)
958+
const Token * getTokenArgumentFunction(const Token * tok, int& argn)
959959
{
960-
if (!tok)
961-
return false;
962-
963-
const Token * const tok1 = tok;
964-
965-
// address of variable
966-
const bool addressOf = tok->astParent() && tok->astParent()->isUnaryOp("&");
967-
960+
argn = -1;
968961
{
969962
const Token *parent = tok->astParent();
970963
if (parent && parent->isUnaryOp("&"))
@@ -981,16 +974,16 @@ bool isVariableChangedByFunctionCall(const Token *tok, int indirect, const Setti
981974
while (Token::simpleMatch(parent, ","))
982975
parent = parent->astParent();
983976
if (!parent || parent->str() != "(")
984-
return false;
977+
return nullptr;
985978
} else
986-
return false;
979+
return nullptr;
987980
}
988981

989-
// goto start of function call and get argnr
990-
int argnr = 0;
982+
// goto start of function call and get argn
983+
argn = 0;
991984
while (tok && !Token::simpleMatch(tok, ";") && !isScopeBracket(tok)) {
992985
if (tok->str() == ",")
993-
++argnr;
986+
++argn;
994987
else if (Token::Match(tok, ")|}"))
995988
tok = tok->link();
996989
else if (Token::Match(tok->previous(), "%name% (|{"))
@@ -1000,13 +993,33 @@ bool isVariableChangedByFunctionCall(const Token *tok, int indirect, const Setti
1000993
tok = tok->previous();
1001994
}
1002995
if (!Token::Match(tok, "{|("))
1003-
return false;
1004-
const bool possiblyPassedByReference = (tok->next() == tok1 || Token::Match(tok1->previous(), ", %name% [,)}]"));
996+
return nullptr;
1005997
tok = tok->previous();
1006998
if (tok && tok->link() && tok->str() == ">")
1007999
tok = tok->link()->previous();
10081000
if (!Token::Match(tok, "%name% [({<]"))
1001+
return nullptr;
1002+
return tok;
1003+
}
1004+
1005+
bool isVariableChangedByFunctionCall(const Token *tok, int indirect, const Settings *settings, bool *inconclusive)
1006+
{
1007+
if (!tok)
1008+
return false;
1009+
1010+
const Token * const tok1 = tok;
1011+
1012+
// address of variable
1013+
const bool addressOf = tok->astParent() && tok->astParent()->isUnaryOp("&");
1014+
1015+
int argnr;
1016+
tok = getTokenArgumentFunction(tok, argnr);
1017+
if (!tok)
10091018
return false; // not a function => variable not changed
1019+
const Token * parenTok = tok->next();
1020+
if (Token::simpleMatch(parenTok, "<") && parenTok->link())
1021+
parenTok = parenTok->link()->next();
1022+
const bool possiblyPassedByReference = (parenTok->next() == tok1 || Token::Match(tok1->previous(), ", %name% [,)}]"));
10101023

10111024
// Constructor call
10121025
if (tok->variable() && tok->variable()->nameToken() == tok) {

‎lib/astutils.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,9 @@ bool isUniqueExpression(const Token* tok);
120120
/** Is scope a return scope (scope will unconditionally return) */
121121
bool isReturnScope(const Token *endToken, const Settings * settings = nullptr, bool functionScope=false);
122122

123+
/// Return the token to the function and the argument number
124+
const Token * getTokenArgumentFunction(const Token * tok, int& argn);
125+
123126
/** Is variable changed by function call?
124127
* In case the answer of the question is inconclusive, e.g. because the function declaration is not known
125128
* the return value is false and the output parameter inconclusive is set to true

‎lib/valueflow.cpp‎

Lines changed: 41 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5297,23 +5297,48 @@ static bool isContainerEmpty(const Token* tok)
52975297
return false;
52985298
}
52995299

5300-
static bool isContainerSizeChanged(nonneg int varId, const Token *start, const Token *end);
5300+
static bool isContainerSizeChanged(nonneg int varId, const Token *start, const Token *end, int depth = 20);
53015301

5302-
static bool isContainerSizeChangedByFunction(const Token *tok)
5302+
static bool isContainerSizeChangedByFunction(const Token *tok, int depth = 20)
53035303
{
5304-
const Token *parent = tok->astParent();
5305-
if (parent && parent->str() == "&")
5306-
parent = parent->astParent();
5307-
while (parent && parent->str() == ",")
5308-
parent = parent->astParent();
5309-
if (!parent)
5304+
if (!tok->valueType() || !tok->valueType()->container)
53105305
return false;
5311-
if (Token::Match(parent->previous(), "%name% ("))
5312-
return true;
5313-
// some unsimplified template function, assume it modifies the container.
5314-
if (Token::simpleMatch(parent->previous(), ">") && parent->linkAt(-1))
5315-
return true;
5316-
return false;
5306+
// If we are accessing an element then we are not changing the container size
5307+
if (Token::Match(tok, "%name% . %name% (")) {
5308+
Library::Container::Yield yield = tok->valueType()->container->getYield(tok->strAt(2));
5309+
if (yield != Library::Container::Yield::NO_YIELD)
5310+
return false;
5311+
}
5312+
if (Token::simpleMatch(tok->astParent(), "["))
5313+
return false;
5314+
5315+
// address of variable
5316+
const bool addressOf = tok->astParent() && tok->astParent()->isUnaryOp("&");
5317+
5318+
int narg;
5319+
const Token * ftok = getTokenArgumentFunction(tok, narg);
5320+
if (!ftok)
5321+
return false; // not a function => variable not changed
5322+
const Function * fun = ftok->function();
5323+
if (fun) {
5324+
const Variable *arg = fun->getArgumentVar(narg);
5325+
if (!arg->isReference() && !addressOf)
5326+
return false;
5327+
if (arg->isConst())
5328+
return false;
5329+
const Scope * scope = fun->functionScope;
5330+
if (scope) {
5331+
// Argument not used
5332+
if (!arg->nameToken())
5333+
return false;
5334+
if (depth > 0)
5335+
return isContainerSizeChanged(arg->declarationId(), scope->bodyStart, scope->bodyEnd, depth - 1);
5336+
}
5337+
}
5338+
5339+
bool inconclusive = false;
5340+
const bool isChanged = isVariableChangedByFunctionCall(tok, 0, nullptr, &inconclusive);
5341+
return (isChanged || inconclusive);
53175342
}
53185343

53195344
static void valueFlowContainerReverse(Token *tok, nonneg int containerId, const ValueFlow::Value &value, const Settings *settings)
@@ -5398,7 +5423,7 @@ static void valueFlowContainerForward(Token *tok, nonneg int containerId, ValueF
53985423
}
53995424
}
54005425

5401-
static bool isContainerSizeChanged(nonneg int varId, const Token *start, const Token *end)
5426+
static bool isContainerSizeChanged(nonneg int varId, const Token *start, const Token *end, int depth)
54025427
{
54035428
for (const Token *tok = start; tok != end; tok = tok->next()) {
54045429
if (tok->varId() != varId)
@@ -5426,7 +5451,7 @@ static bool isContainerSizeChanged(nonneg int varId, const Token *start, const T
54265451
break;
54275452
};
54285453
}
5429-
if (isContainerSizeChangedByFunction(tok))
5454+
if (isContainerSizeChangedByFunction(tok, depth))
54305455
return true;
54315456
}
54325457
return false;

‎test/testvalueflow.cpp‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3937,6 +3937,113 @@ class TestValueFlow : public TestFixture {
39373937
"}";
39383938
ASSERT(tokenValues(code, "x . front").empty());
39393939

3940+
code = "void g(std::list<int>&);\n"
3941+
"void f() {\n"
3942+
" std::list<int> x;\n"
3943+
" g(x);\n"
3944+
" x.front();\n"
3945+
"}";
3946+
ASSERT(tokenValues(code, "x . front").empty());
3947+
3948+
code = "void g(std::list<int>*);\n"
3949+
"void f() {\n"
3950+
" std::list<int> x;\n"
3951+
" g(&x);\n"
3952+
" x.front();\n"
3953+
"}";
3954+
ASSERT(tokenValues(code, "x . front").empty());
3955+
3956+
code = "void g(const std::list<int>&);\n"
3957+
"void f() {\n"
3958+
" std::list<int> x;\n"
3959+
" g(x);\n"
3960+
" x.front();\n"
3961+
"}";
3962+
ASSERT_EQUALS("", isKnownContainerSizeValue(tokenValues(code, "x . front"), 0));
3963+
3964+
code = "void g(std::list<int>);\n"
3965+
"void f() {\n"
3966+
" std::list<int> x;\n"
3967+
" g(x);\n"
3968+
" x.front();\n"
3969+
"}";
3970+
ASSERT_EQUALS("", isKnownContainerSizeValue(tokenValues(code, "x . front"), 0));
3971+
3972+
code = "void g(int&);\n"
3973+
"void f() {\n"
3974+
" std::list<int> x;\n"
3975+
" g(x[0]);\n"
3976+
" x.front();\n"
3977+
"}";
3978+
ASSERT_EQUALS("", isKnownContainerSizeValue(tokenValues(code, "x . front"), 0));
3979+
3980+
code = "void g(int&);\n"
3981+
"void f() {\n"
3982+
" std::list<int> x;\n"
3983+
" g(x.back());\n"
3984+
" x.front();\n"
3985+
"}";
3986+
ASSERT_EQUALS("", isKnownContainerSizeValue(tokenValues(code, "x . front"), 0));
3987+
3988+
code = "void g(std::list<int>&) {}\n"
3989+
"void f() {\n"
3990+
" std::list<int> x;\n"
3991+
" g(x);\n"
3992+
" x.front();\n"
3993+
"}";
3994+
ASSERT_EQUALS("", isKnownContainerSizeValue(tokenValues(code, "x . front"), 0));
3995+
3996+
code = "void g(std::list<int>& y) { y.push_back(1); }\n"
3997+
"void f() {\n"
3998+
" std::list<int> x;\n"
3999+
" g(x);\n"
4000+
" x.front();\n"
4001+
"}";
4002+
ASSERT(tokenValues(code, "x . front").empty());
4003+
4004+
code = "void g(std::list<int>*) {}\n"
4005+
"void f() {\n"
4006+
" std::list<int> x;\n"
4007+
" g(&x);\n"
4008+
" x.front();\n"
4009+
"}";
4010+
ASSERT_EQUALS("", isKnownContainerSizeValue(tokenValues(code, "x . front"), 0));
4011+
4012+
code = "void g(std::list<int>* y) { y->push_back(1); }\n"
4013+
"void f() {\n"
4014+
" std::list<int> x;\n"
4015+
" g(&x);\n"
4016+
" x.front();\n"
4017+
"}";
4018+
ASSERT(tokenValues(code, "x . front").empty());
4019+
4020+
code = "void h(std::list<int>&);\n"
4021+
"void g(std::list<int>& y) { h(y); }\n"
4022+
"void f() {\n"
4023+
" std::list<int> x;\n"
4024+
" g(x);\n"
4025+
" x.front();\n"
4026+
"}";
4027+
ASSERT(tokenValues(code, "x . front").empty());
4028+
4029+
code = "void h(const std::list<int>&);\n"
4030+
"void g(std::list<int>& y) { h(y); }\n"
4031+
"void f() {\n"
4032+
" std::list<int> x;\n"
4033+
" g(x);\n"
4034+
" x.front();\n"
4035+
"}";
4036+
ASSERT_EQUALS("", isKnownContainerSizeValue(tokenValues(code, "x . front"), 0));
4037+
4038+
code = "void h(const std::list<int>&);\n"
4039+
"void g(std::list<int>& y) { h(y); y.push_back(1); }\n"
4040+
"void f() {\n"
4041+
" std::list<int> x;\n"
4042+
" g(x);\n"
4043+
" x.front();\n"
4044+
"}";
4045+
ASSERT(tokenValues(code, "x . front").empty());
4046+
39404047
code = "void f(std::vector<int> ints) {\n" // #8697
39414048
" if (ints.empty())\n"
39424049
" abort() << 123;\n"

0 commit comments

Comments
 (0)