Skip to content

Fix reverse analysis when modifying variable with function #4800

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
firewave merged 4 commits into from
Feb 23, 2023
Merged

Fix reverse analysis when modifying variable with function #4800

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
c76bd6b
Fix reverse analysis when modifying variable with function
pfultz2 Feb 16, 2023
4a504fc
Format
pfultz2 Feb 16, 2023
f219c3c
Fix tidy issues
pfultz2 Feb 19, 2023
b292b29
Format
pfultz2 Feb 19, 2023
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
4 changes: 2 additions & 2 deletions lib/reverseanalyzer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,12 @@ struct ReverseTraversal {

bool update(Token* tok) {
Analyzer::Action action = analyzer->analyze(tok, Analyzer::Direction::Reverse);
if (!action.isNone())
analyzer->update(tok, action, Analyzer::Direction::Reverse);
if (action.isInconclusive() && !analyzer->lowerToInconclusive())
return false;
if (action.isInvalid())
return false;
if (!action.isNone())
analyzer->update(tok, action, Analyzer::Direction::Reverse);
return true;
}

Expand Down
28 changes: 15 additions & 13 deletions lib/valueflow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7741,11 +7741,10 @@ struct ContainerExpressionAnalyzer : ExpressionAnalyzer {
return tok->exprId() == expr->exprId() || (astIsIterator(tok) && isAliasOf(tok, expr->exprId()));
}

Action isWritable(const Token* tok, Direction d) const override {
Action isWritable(const Token* tok, Direction /*d*/) const override
{
if (astIsIterator(tok))
return Action::None;
if (d == Direction::Reverse)
return Action::None;
if (!getValue(tok))
return Action::None;
if (!tok->valueType())
Expand Down Expand Up @@ -7778,8 +7777,6 @@ struct ContainerExpressionAnalyzer : ExpressionAnalyzer {
}

void writeValue(ValueFlow::Value* val, const Token* tok, Direction d) const override {
if (d == Direction::Reverse)
return;
if (!val)
return;
if (!tok->astParent())
Expand All @@ -7790,26 +7787,31 @@ struct ContainerExpressionAnalyzer : ExpressionAnalyzer {
return;
const Token* parent = tok->astParent();
const Library::Container* container = getLibraryContainer(tok);
int n = 0;

if (container->stdStringLike && Token::simpleMatch(parent, "+=") && parent->astOperand2()) {
const Token* rhs = parent->astOperand2();
const Library::Container* rhsContainer = getLibraryContainer(rhs);
if (rhs->tokType() == Token::eString)
val->intvalue += Token::getStrLength(rhs);
n = Token::getStrLength(rhs);
else if (rhsContainer && rhsContainer->stdStringLike) {
for (const ValueFlow::Value &rhsval : rhs->values()) {
if (rhsval.isKnown() && rhsval.isContainerSizeValue()) {
val->intvalue += rhsval.intvalue;
}
}
auto it = std::find_if(rhs->values().begin(), rhs->values().end(), [&](const ValueFlow::Value& rhsval) {
return rhsval.isKnown() && rhsval.isContainerSizeValue();
});
if (it != rhs->values().end())
n = it->intvalue;
}
} else if (astIsLHS(tok) && Token::Match(tok->astParent(), ". %name% (")) {
const Library::Container::Action action = container->getAction(tok->astParent()->strAt(1));
if (action == Library::Container::Action::PUSH)
val->intvalue++;
n = 1;
if (action == Library::Container::Action::POP)
val->intvalue--;
n = -1;
}
if (d == Direction::Reverse)
val->intvalue -= n;
else
val->intvalue += n;
}

int getIndirect(const Token* tok) const override
Expand Down
13 changes: 12 additions & 1 deletion test/testvalueflow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1529,27 +1529,31 @@ class TestValueFlow : public TestFixture {
" if (x == 4);\n"
"}";
ASSERT_EQUALS(true, testValueOfX(code, 2U, 2));
ASSERT_EQUALS(true, testValueOfX(code, 3U, 2));

code = "void f(int x) {\n"
" a = x;\n"
" x -= 2;\n"
" if (x == 4);\n"
"}";
ASSERT_EQUALS(true, testValueOfX(code, 2U, 6));
ASSERT_EQUALS(true, testValueOfX(code, 3U, 6));

code = "void f(int x) {\n"
" a = x;\n"
" x *= 2;\n"
" if (x == 42);\n"
"}";
ASSERT_EQUALS(true, testValueOfX(code, 2U, 21));
ASSERT_EQUALS(true, testValueOfX(code, 3U, 21));

code = "void f(int x) {\n"
" a = x;\n"
" x /= 5;\n"
" if (x == 42);\n"
"}";
ASSERT_EQUALS(true, testValueOfX(code, 2U, 210));
ASSERT_EQUALS(true, testValueOfX(code, 3U, 210));

// bailout: assignment
bailout("void f(int x) {\n"
Expand Down Expand Up @@ -1612,6 +1616,13 @@ class TestValueFlow : public TestFixture {
" if (x) {}\n"
"}";
ASSERT_EQUALS(false, testValueOfX(code, 3U, 0));

code = "void g(int&);"
"void f(int x) {\n"
" g(x);\n"
" if (x == 5);\n"
"}";
ASSERT_EQUALS(false, testValueOfX(code, 2U, 5));
}

void valueFlowBeforeConditionLoop() { // while, for, do-while
Expand Down Expand Up @@ -5666,7 +5677,7 @@ class TestValueFlow : public TestFixture {
" ints.pop_back();\n"
" if (ints.empty()) {}\n"
"}";
ASSERT(tokenValues(code, "ints . front").empty());
ASSERT_EQUALS("", isPossibleContainerSizeValue(tokenValues(code, "ints . front"), 1));

code = "void f(std::vector<int> v) {\n"
" v[10] = 0;\n" // <- container size can be 10
Expand Down