diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 4c1e17e00..77ba95fae 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -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 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; @@ -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()); @@ -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; } @@ -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; - const Token * const ftok = tok->tokAt(2); + if (!Token::Match(tok->astParent(), ". %name% ( !!)")) + continue; + const Token* const ftok = tok->astParent()->next(); const std::vector args = getArguments(ftok); const Library::Container * c = tok->valueType()->container; @@ -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); } } } diff --git a/test/teststl.cpp b/test/teststl.cpp index 878e6fbb4..e76e532ec 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -1658,6 +1658,15 @@ private: " 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() { @@ -1742,6 +1751,15 @@ private: " }\n" "}\n"); ASSERT_EQUALS("", errout.str()); + + // #10604 + check("struct S {\n" + " std::vector 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