From eb62aea40804f68acc9dc7aeb055ef86c5d0a39a Mon Sep 17 00:00:00 2001 From: Paul Date: Mon, 22 Nov 2021 19:29:14 -0600 Subject: [PATCH 1/3] Fix 10604: FP mismatchingContainerIterator with container member --- lib/checkstl.cpp | 58 +++++++++++++++++++++++++++++++++++------------- test/teststl.cpp | 18 +++++++++++++++ 2 files changed, 60 insertions(+), 16 deletions(-) diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 4c1e17e0052..76b4d1d5ed7 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -697,14 +697,44 @@ 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 +745,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 +761,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 +838,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 args = getArguments(ftok); const Library::Container * c = tok->valueType()->container; @@ -831,20 +863,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 878e6fbb425..1f72f439924 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -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() { @@ -1742,6 +1751,15 @@ class TestStl : public TestFixture { " }\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 From f88743289094e5e9ae5aa32dfb78702d3a8262fb Mon Sep 17 00:00:00 2001 From: Paul Date: Mon, 22 Nov 2021 19:31:34 -0600 Subject: [PATCH 2/3] Format --- lib/checkstl.cpp | 7 +++++-- test/teststl.cpp | 22 +++++++++++----------- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 76b4d1d5ed7..29c637ef5ba 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -704,7 +704,10 @@ static const Token* getAddressContainer(const Token* tok) return tok; } -static bool isSameIteratorContainerExpression(const Token* tok1, const Token * tok2, const Library& library, ValueFlow::Value::LifetimeKind kind = ValueFlow::Value::LifetimeKind::Iterator) +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; @@ -842,7 +845,7 @@ void CheckStl::mismatchingContainerIterator() continue; if (!Token::Match(tok->astParent(), " . %name% ( !!)")) continue; - const Token * const ftok = tok->astParent()->next(); + const Token* const ftok = tok->astParent()->next(); const std::vector args = getArguments(ftok); const Library::Container * c = tok->valueType()->container; diff --git a/test/teststl.cpp b/test/teststl.cpp index 1f72f439924..e76e532eced 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -1660,12 +1660,12 @@ class TestStl : public TestFixture { 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"); + " 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()); } @@ -1754,11 +1754,11 @@ class TestStl : public TestFixture { // #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"); + " 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()); } From 770856e56f8c87bf93cdf332bfd6c087614cf5a5 Mon Sep 17 00:00:00 2001 From: Paul Date: Tue, 23 Nov 2021 10:51:01 -0600 Subject: [PATCH 3/3] Remove whitespace --- lib/checkstl.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 29c637ef5ba..77ba95fae18 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -843,7 +843,7 @@ void CheckStl::mismatchingContainerIterator() continue; if (!astIsLHS(tok)) continue; - if (!Token::Match(tok->astParent(), " . %name% ( !!)")) + if (!Token::Match(tok->astParent(), ". %name% ( !!)")) continue; const Token* const ftok = tok->astParent()->next(); const std::vector args = getArguments(ftok);