Skip to content

Fix 10604: FP mismatchingContainerIterator with container member #3575

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 3 commits into from
Nov 23, 2021
Merged

Fix 10604: FP mismatchingContainerIterator with container member #3575

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
eb62aea
Fix 10604: FP mismatchingContainerIterator with container member
pfultz2 Nov 23, 2021
f887432
Format
pfultz2 Nov 23, 2021
770856e
Remove whitespace
pfultz2 Nov 23, 2021
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
61 changes: 45 additions & 16 deletions lib/checkstl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -697,14 +697,47 @@ static const Token * getIteratorExpression(const Token * tok)
return nullptr;
}

static const Token* getAddressContainer(const Token* tok)
{
if (Token::simpleMatch(tok, "[") && tok->astOperand1())
return tok->astOperand1();
return tok;
}

static bool isSameIteratorContainerExpression(const Token* tok1,
const Token* tok2,
const Library& library,
ValueFlow::Value::LifetimeKind kind = ValueFlow::Value::LifetimeKind::Iterator)
{
if (isSameExpression(true, false, tok1, tok2, library, false, false))
return true;
if (kind == ValueFlow::Value::LifetimeKind::Address) {
return isSameExpression(true, false, getAddressContainer(tok1), getAddressContainer(tok2), library, false, false);
}
return false;
}

static ValueFlow::Value getLifetimeIteratorValue(const Token* tok)
{
std::vector<ValueFlow::Value> values = getLifetimeObjValues(tok);
auto it = std::find_if(values.begin(), values.end(), [](const ValueFlow::Value& v) {
return v.lifetimeKind == ValueFlow::Value::LifetimeKind::Iterator;
});
if (it != values.end())
return *it;
if (values.size() == 1)
return values.front();
return ValueFlow::Value{};
}

bool CheckStl::checkIteratorPair(const Token* tok1, const Token* tok2)
{
if (!tok1)
return false;
if (!tok2)
return false;
ValueFlow::Value val1 = getLifetimeObjValue(tok1);
ValueFlow::Value val2 = getLifetimeObjValue(tok2);
ValueFlow::Value val1 = getLifetimeIteratorValue(tok1);
ValueFlow::Value val2 = getLifetimeIteratorValue(tok2);
if (val1.tokvalue && val2.tokvalue && val1.lifetimeKind == val2.lifetimeKind) {
if (val1.lifetimeKind == ValueFlow::Value::LifetimeKind::Lambda)
return false;
Expand All @@ -715,7 +748,7 @@ bool CheckStl::checkIteratorPair(const Token* tok1, const Token* tok2)
(!astIsContainer(val1.tokvalue) || !astIsContainer(val2.tokvalue)))
return false;
}
if (isSameExpression(true, false, val1.tokvalue, val2.tokvalue, mSettings->library, false, false))
if (isSameIteratorContainerExpression(val1.tokvalue, val2.tokvalue, mSettings->library, val1.lifetimeKind))
return false;
if (val1.tokvalue->expressionString() == val2.tokvalue->expressionString())
iteratorsError(tok1, val1.tokvalue, val1.tokvalue->expressionString());
Expand All @@ -731,7 +764,7 @@ bool CheckStl::checkIteratorPair(const Token* tok1, const Token* tok2)
}
const Token* iter1 = getIteratorExpression(tok1);
const Token* iter2 = getIteratorExpression(tok2);
if (iter1 && iter2 && !isSameExpression(true, false, iter1, iter2, mSettings->library, false, false)) {
if (iter1 && iter2 && !isSameIteratorContainerExpression(iter1, iter2, mSettings->library)) {
mismatchingContainerExpressionError(iter1, iter2);
return true;
}
Expand Down Expand Up @@ -808,9 +841,11 @@ void CheckStl::mismatchingContainerIterator()
for (const Token* tok = scope->bodyStart->next(); tok != scope->bodyEnd; tok = tok->next()) {
if (!astIsContainer(tok))
continue;
if (!Token::Match(tok, "%var% . %name% ( !!)"))
if (!astIsLHS(tok))
continue;
if (!Token::Match(tok->astParent(), ". %name% ( !!)"))
continue;
const Token * const ftok = tok->tokAt(2);
const Token* const ftok = tok->astParent()->next();
const std::vector<const Token *> args = getArguments(ftok);

const Library::Container * c = tok->valueType()->container;
Expand All @@ -831,20 +866,14 @@ void CheckStl::mismatchingContainerIterator()
continue;
}

ValueFlow::Value val = getLifetimeObjValue(iterTok);
ValueFlow::Value val = getLifetimeIteratorValue(iterTok);
if (!val.tokvalue)
continue;
if (val.lifetimeKind != ValueFlow::Value::LifetimeKind::Iterator)
continue;
for (const LifetimeToken& lt:getLifetimeTokens(tok)) {
if (lt.inconclusive)
continue;
const Token* contTok = lt.token;
if (isSameExpression(true, false, contTok, val.tokvalue, mSettings->library, false, false))
continue;
mismatchingContainerIteratorError(tok, iterTok);
}

if (isSameIteratorContainerExpression(tok, val.tokvalue, mSettings->library))
continue;
mismatchingContainerIteratorError(tok, iterTok);
}
}
}
Expand Down
18 changes: 18 additions & 0 deletions test/teststl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1658,6 +1658,15 @@ class TestStl : public TestFixture {
" if(f().begin(1) == f().end()) {}\n"
"}");
ASSERT_EQUALS("", errout.str());

check("void foo(const uint8_t* data, const uint32_t dataLength) {\n"
" const uint32_t minimumLength = sizeof(uint16_t) + sizeof(uint16_t);\n"
" if (dataLength >= minimumLength) {\n"
" char* payload = new char[dataLength - minimumLength];\n"
" std::copy(&data[minimumLength], &data[dataLength], payload);\n"
" }\n"
"}\n");
ASSERT_EQUALS("", errout.str());
}

void iteratorSameExpression() {
Expand Down Expand Up @@ -1742,6 +1751,15 @@ class TestStl : public TestFixture {
" }\n"
"}\n");
ASSERT_EQUALS("", errout.str());

// #10604
check("struct S {\n"
" std::vector<int> v;\n"
"};\n"
"void f(S& s, int m) {\n"
" s.v.erase(s.v.begin() + m);\n"
"}\n");
ASSERT_EQUALS("", errout.str());
}

// Dereferencing invalid pointer
Expand Down