Skip to content

Add deeper analysis of when a function changes a containers size #2149

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
danmar merged 4 commits into from
Sep 6, 2019
Merged

Add deeper analysis of when a function changes a containers size #2149

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4556f90
Add deeper analysis of when a function changes a containers size
pfultz2 Sep 5, 2019
af1cdca
Merge
pfultz2 Sep 5, 2019
1059196
Fix issues
pfultz2 Sep 6, 2019
eea771b
Track addressOf
pfultz2 Sep 6, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 29 additions & 16 deletions lib/astutils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -955,16 +955,9 @@ static bool isScopeBracket(const Token *tok)
return false;
}

bool isVariableChangedByFunctionCall(const Token *tok, int indirect, const Settings *settings, bool *inconclusive)
const Token * getTokenArgumentFunction(const Token * tok, int& argn)
{
if (!tok)
return false;

const Token * const tok1 = tok;

// address of variable
const bool addressOf = tok->astParent() && tok->astParent()->isUnaryOp("&");

argn = -1;
{
const Token *parent = tok->astParent();
if (parent && parent->isUnaryOp("&"))
Expand All @@ -981,16 +974,16 @@ bool isVariableChangedByFunctionCall(const Token *tok, int indirect, const Setti
while (Token::simpleMatch(parent, ","))
parent = parent->astParent();
if (!parent || parent->str() != "(")
return false;
return nullptr;
} else
return false;
return nullptr;
}

// goto start of function call and get argnr
int argnr = 0;
// goto start of function call and get argn
argn = 0;
while (tok && !Token::simpleMatch(tok, ";") && !isScopeBracket(tok)) {
if (tok->str() == ",")
++argnr;
++argn;
else if (Token::Match(tok, ")|}"))
tok = tok->link();
else if (Token::Match(tok->previous(), "%name% (|{"))
Expand All @@ -1000,13 +993,33 @@ bool isVariableChangedByFunctionCall(const Token *tok, int indirect, const Setti
tok = tok->previous();
}
if (!Token::Match(tok, "{|("))
return false;
const bool possiblyPassedByReference = (tok->next() == tok1 || Token::Match(tok1->previous(), ", %name% [,)}]"));
return nullptr;
tok = tok->previous();
if (tok && tok->link() && tok->str() == ">")
tok = tok->link()->previous();
if (!Token::Match(tok, "%name% [({<]"))
return nullptr;
return tok;
}

bool isVariableChangedByFunctionCall(const Token *tok, int indirect, const Settings *settings, bool *inconclusive)
{
if (!tok)
return false;

const Token * const tok1 = tok;

// address of variable
const bool addressOf = tok->astParent() && tok->astParent()->isUnaryOp("&");

int argnr;
tok = getTokenArgumentFunction(tok, argnr);
if (!tok)
return false; // not a function => variable not changed
const Token * parenTok = tok->next();
if (Token::simpleMatch(parenTok, "<") && parenTok->link())
parenTok = parenTok->link()->next();
const bool possiblyPassedByReference = (parenTok->next() == tok1 || Token::Match(tok1->previous(), ", %name% [,)}]"));

// Constructor call
if (tok->variable() && tok->variable()->nameToken() == tok) {
Expand Down
3 changes: 3 additions & 0 deletions lib/astutils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,9 @@ bool isUniqueExpression(const Token* tok);
/** Is scope a return scope (scope will unconditionally return) */
bool isReturnScope(const Token *endToken, const Settings * settings = nullptr, bool functionScope=false);

/// Return the token to the function and the argument number
const Token * getTokenArgumentFunction(const Token * tok, int& argn);

/** Is variable changed by function call?
* In case the answer of the question is inconclusive, e.g. because the function declaration is not known
* the return value is false and the output parameter inconclusive is set to true
Expand Down
57 changes: 41 additions & 16 deletions lib/valueflow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5297,23 +5297,48 @@ static bool isContainerEmpty(const Token* tok)
return false;
}

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

static bool isContainerSizeChangedByFunction(const Token *tok)
static bool isContainerSizeChangedByFunction(const Token *tok, int depth = 20)
{
const Token *parent = tok->astParent();
if (parent && parent->str() == "&")
parent = parent->astParent();
while (parent && parent->str() == ",")
parent = parent->astParent();
if (!parent)
if (!tok->valueType() || !tok->valueType()->container)
return false;
if (Token::Match(parent->previous(), "%name% ("))
return true;
// some unsimplified template function, assume it modifies the container.
if (Token::simpleMatch(parent->previous(), ">") && parent->linkAt(-1))
return true;
return false;
// If we are accessing an element then we are not changing the container size
if (Token::Match(tok, "%name% . %name% (")) {
Library::Container::Yield yield = tok->valueType()->container->getYield(tok->strAt(2));
if (yield != Library::Container::Yield::NO_YIELD)
return false;
}
if (Token::simpleMatch(tok->astParent(), "["))
return false;

// address of variable
const bool addressOf = tok->astParent() && tok->astParent()->isUnaryOp("&");

int narg;
const Token * ftok = getTokenArgumentFunction(tok, narg);
if (!ftok)
return false; // not a function => variable not changed
const Function * fun = ftok->function();
if (fun) {
const Variable *arg = fun->getArgumentVar(narg);
if (!arg->isReference() && !addressOf)
return false;
if (arg->isConst())
return false;
const Scope * scope = fun->functionScope;
if (scope) {
// Argument not used
if (!arg->nameToken())
return false;
if (depth > 0)
return isContainerSizeChanged(arg->declarationId(), scope->bodyStart, scope->bodyEnd, depth - 1);
}
}

bool inconclusive = false;
const bool isChanged = isVariableChangedByFunctionCall(tok, 0, nullptr, &inconclusive);
return (isChanged || inconclusive);
}

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

static bool isContainerSizeChanged(nonneg int varId, const Token *start, const Token *end)
static bool isContainerSizeChanged(nonneg int varId, const Token *start, const Token *end, int depth)
{
for (const Token *tok = start; tok != end; tok = tok->next()) {
if (tok->varId() != varId)
Expand Down Expand Up @@ -5426,7 +5451,7 @@ static bool isContainerSizeChanged(nonneg int varId, const Token *start, const T
break;
};
}
if (isContainerSizeChangedByFunction(tok))
if (isContainerSizeChangedByFunction(tok, depth))
return true;
}
return false;
Expand Down
107 changes: 107 additions & 0 deletions test/testvalueflow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3937,6 +3937,113 @@ class TestValueFlow : public TestFixture {
"}";
ASSERT(tokenValues(code, "x . front").empty());

code = "void g(std::list<int>&);\n"
"void f() {\n"
" std::list<int> x;\n"
" g(x);\n"
" x.front();\n"
"}";
ASSERT(tokenValues(code, "x . front").empty());

code = "void g(std::list<int>*);\n"
"void f() {\n"
" std::list<int> x;\n"
" g(&x);\n"
" x.front();\n"
"}";
ASSERT(tokenValues(code, "x . front").empty());

code = "void g(const std::list<int>&);\n"
"void f() {\n"
" std::list<int> x;\n"
" g(x);\n"
" x.front();\n"
"}";
ASSERT_EQUALS("", isKnownContainerSizeValue(tokenValues(code, "x . front"), 0));

code = "void g(std::list<int>);\n"
"void f() {\n"
" std::list<int> x;\n"
" g(x);\n"
" x.front();\n"
"}";
ASSERT_EQUALS("", isKnownContainerSizeValue(tokenValues(code, "x . front"), 0));

code = "void g(int&);\n"
"void f() {\n"
" std::list<int> x;\n"
" g(x[0]);\n"
" x.front();\n"
"}";
ASSERT_EQUALS("", isKnownContainerSizeValue(tokenValues(code, "x . front"), 0));

code = "void g(int&);\n"
"void f() {\n"
" std::list<int> x;\n"
" g(x.back());\n"
" x.front();\n"
"}";
ASSERT_EQUALS("", isKnownContainerSizeValue(tokenValues(code, "x . front"), 0));

code = "void g(std::list<int>&) {}\n"
"void f() {\n"
" std::list<int> x;\n"
" g(x);\n"
" x.front();\n"
"}";
ASSERT_EQUALS("", isKnownContainerSizeValue(tokenValues(code, "x . front"), 0));

code = "void g(std::list<int>& y) { y.push_back(1); }\n"
"void f() {\n"
" std::list<int> x;\n"
" g(x);\n"
" x.front();\n"
"}";
ASSERT(tokenValues(code, "x . front").empty());

code = "void g(std::list<int>*) {}\n"
"void f() {\n"
" std::list<int> x;\n"
" g(&x);\n"
" x.front();\n"
"}";
ASSERT_EQUALS("", isKnownContainerSizeValue(tokenValues(code, "x . front"), 0));

code = "void g(std::list<int>* y) { y->push_back(1); }\n"
"void f() {\n"
" std::list<int> x;\n"
" g(&x);\n"
" x.front();\n"
"}";
ASSERT(tokenValues(code, "x . front").empty());

code = "void h(std::list<int>&);\n"
"void g(std::list<int>& y) { h(y); }\n"
"void f() {\n"
" std::list<int> x;\n"
" g(x);\n"
" x.front();\n"
"}";
ASSERT(tokenValues(code, "x . front").empty());

code = "void h(const std::list<int>&);\n"
"void g(std::list<int>& y) { h(y); }\n"
"void f() {\n"
" std::list<int> x;\n"
" g(x);\n"
" x.front();\n"
"}";
ASSERT_EQUALS("", isKnownContainerSizeValue(tokenValues(code, "x . front"), 0));

code = "void h(const std::list<int>&);\n"
"void g(std::list<int>& y) { h(y); y.push_back(1); }\n"
"void f() {\n"
" std::list<int> x;\n"
" g(x);\n"
" x.front();\n"
"}";
ASSERT(tokenValues(code, "x . front").empty());

code = "void f(std::vector<int> ints) {\n" // #8697
" if (ints.empty())\n"
" abort() << 123;\n"
Expand Down