Skip to content

Commit

Permalink
Fix 11898: FP knownEmptyContainer (#5394)
Browse files Browse the repository at this point in the history
  • Loading branch information
@pfultz2
pfultz2 committed Sep 2, 2023
1 parent ffbcfae commit fd12baa
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 25 deletions.
55 changes: 35 additions & 20 deletions lib/astutils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1001,29 +1001,43 @@ bool isAliasOf(const Token *tok, nonneg int varid, bool* inconclusive)
return false;
}

bool isAliasOf(const Token* tok, const Token* expr, bool* inconclusive)
bool isAliasOf(const Token* tok, const Token* expr, int* indirect, bool* inconclusive)
{
const bool pointer = astIsPointer(tok);
const ValueFlow::Value* value = nullptr;
const Token* r = findAstNode(expr, [&](const Token* childTok) {
for (const ValueFlow::Value& val : tok->values()) {
if (val.isImpossible())
continue;
if (val.isLocalLifetimeValue() || (pointer && val.isSymbolicValue() && val.intvalue == 0)) {
if (findAstNode(val.tokvalue,
[&](const Token* aliasTok) {
return aliasTok->exprId() == childTok->exprId();
})) {
if (val.isInconclusive() && inconclusive != nullptr) {
value = &val;
} else {
return true;
const Token* r = nullptr;
if (indirect)
*indirect = 1;
for (const ReferenceToken& ref : followAllReferences(tok)) {
const bool pointer = astIsPointer(ref.token);
r = findAstNode(expr, [&](const Token* childTok) {
if (childTok->exprId() == 0)
return false;
if (ref.token != tok && expr->exprId() == childTok->exprId()) {
if (indirect)
*indirect = 0;
return true;
}
for (const ValueFlow::Value& val : ref.token->values()) {
if (val.isImpossible())
continue;
if (val.isLocalLifetimeValue() || (pointer && val.isSymbolicValue() && val.intvalue == 0)) {
if (findAstNode(val.tokvalue,
[&](const Token* aliasTok) {
return aliasTok->exprId() == childTok->exprId();
})) {
if (val.isInconclusive() && inconclusive != nullptr) {
value = &val;
} else {
return true;
}
}
}
}
}
return false;
});
return false;
});
if (r)
break;
}
if (!r && value && inconclusive)
*inconclusive = true;
return r || value;
Expand Down Expand Up @@ -2728,16 +2742,17 @@ static bool isExpressionChangedAt(const F& getExprTok,
if (globalvar && !tok->isKeyword() && Token::Match(tok, "%name% (") && !(tok->function() && tok->function()->isAttributePure()))
// TODO: Is global variable really changed by function call?
return true;
int i = 1;
bool aliased = false;
// If we can't find the expression then assume it is an alias
auto expr = getExprTok();
if (!expr)
aliased = true;
if (!aliased)
aliased = isAliasOf(tok, expr);
aliased = isAliasOf(tok, expr, &i);
if (!aliased)
return false;
if (isVariableChanged(tok, indirect + 1, settings, cpp, depth))
if (isVariableChanged(tok, indirect + i, settings, cpp, depth))
return true;
// TODO: Try to traverse the lambda function
if (Token::Match(tok, "%var% ("))
Expand Down
2 changes: 1 addition & 1 deletion lib/astutils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ bool isExpressionChangedAt(const Token* expr,
/// If token is an alias if another variable
bool isAliasOf(const Token *tok, nonneg int varid, bool* inconclusive = nullptr);

bool isAliasOf(const Token* tok, const Token* expr, bool* inconclusive = nullptr);
bool isAliasOf(const Token* tok, const Token* expr, int* indirect = nullptr, bool* inconclusive = nullptr);

bool isAliased(const Variable *var);

Expand Down
8 changes: 4 additions & 4 deletions lib/valueflow.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8039,7 +8039,7 @@ static void valueFlowUninit(TokenList& tokenlist, const Settings* settings)
}
}

static bool isContainerSizeChanged(nonneg int varId,
static bool isContainerSizeChanged(const Token* expr,
const Token* start,
const Token* end,
int indirect,
Expand Down Expand Up @@ -8088,7 +8088,7 @@ static bool isContainerSizeChangedByFunction(const Token* tok,
if (!arg->nameToken())
return false;
if (depth > 0)
return isContainerSizeChanged(arg->declarationId(),
return isContainerSizeChanged(arg->nameToken(),
scope->bodyStart,
scope->bodyEnd,
addressOf ? indirect + 1 : indirect,
Expand Down Expand Up @@ -8342,15 +8342,15 @@ bool ValueFlow::isContainerSizeChanged(const Token* tok, int indirect, const Set
return isContainerSizeChangedByFunction(tok, indirect, settings, depth);
}

static bool isContainerSizeChanged(nonneg int varId,
static bool isContainerSizeChanged(const Token* expr,
const Token* start,
const Token* end,
int indirect,
const Settings* settings,
int depth)
{
for (const Token *tok = start; tok != end; tok = tok->next()) {
if (tok->varId() != varId)
if (tok->exprId() != expr->exprId() && !isAliasOf(tok, expr))
continue;
if (ValueFlow::isContainerSizeChanged(tok, indirect, settings, depth))
return true;
Expand Down
12 changes: 12 additions & 0 deletions test/testvalueflow.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6643,6 +6643,18 @@ class TestValueFlow : public TestFixture {

code = "int f() { auto a = std::array<int, 2>{}; return a[1]; }";
ASSERT_EQUALS("values.size():0", isKnownContainerSizeValue(tokenValues(code, "a ["), 0));

code = "void g(std::vector<int>* w) {\n"
" std::vector<int> &r = *w;\n"
" r.push_back(0);\n"
"}\n"
"int f() {\n"
" std::vector<int> v;\n"
" g(&v);\n"
" return v[0];\n"
"}\n";
ASSERT(!isKnownContainerSizeValue(tokenValues(code, "v ["), 0).empty());
ASSERT(!isPossibleContainerSizeValue(tokenValues(code, "v ["), 0).empty());
}

void valueFlowContainerElement()
Expand Down

1 comment on commit fd12baa

@firewave
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pfultz2 This caused a performance regression: https://trac.cppcheck.net/ticket/11921.

Please sign in to comment.