From 503c109a1c569b3f4da76582634d226432debda8 Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Fri, 9 Feb 2024 10:06:08 +0100 Subject: [PATCH] Fix #11093, #12377 FP mismatchingContainerIterator (#5911) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …r to get reference to container --- lib/checkstl.cpp | 30 ++++++++++++++++++++++++------ test/teststl.cpp | 45 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 6 deletions(-) diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index a9b12f61a0a..d1d10082821 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -686,11 +686,19 @@ void CheckStl::sameIteratorExpressionError(const Token *tok) reportError(tok, Severity::style, "sameIteratorExpression", "Same iterators expression are used for algorithm.", CWE664, Certainty::normal); } -static const Token* getAddressContainer(const Token* tok) +static std::vector getAddressContainer(const Token* tok) { if (Token::simpleMatch(tok, "[") && tok->astOperand1()) - return tok->astOperand1(); - return tok; + return { tok->astOperand1() }; + std::vector values = ValueFlow::getLifetimeObjValues(tok, /*inconclusive*/ false); + std::vector res; + for (const auto& v : values) { + if (v.tokvalue) + res.emplace_back(v.tokvalue); + } + if (res.empty()) + res.emplace_back(tok); + return res; } static bool isSameIteratorContainerExpression(const Token* tok1, @@ -701,10 +709,18 @@ static bool isSameIteratorContainerExpression(const Token* tok1, if (isSameExpression(true, false, tok1, tok2, library, false, false)) { return !astIsContainerOwned(tok1) || !isTemporary(true, tok1, &library); } - if (kind == ValueFlow::Value::LifetimeKind::Address) { - return isSameExpression(true, false, getAddressContainer(tok1), getAddressContainer(tok2), library, false, false); + if (astContainerYield(tok2) == Library::Container::Yield::ITEM) + return true; + if (kind == ValueFlow::Value::LifetimeKind::Address || kind == ValueFlow::Value::LifetimeKind::Iterator) { + const auto address1 = getAddressContainer(tok1); + const auto address2 = getAddressContainer(tok2); + return std::any_of(address1.begin(), address1.end(), [&](const Token* tok1) { + return std::any_of(address2.begin(), address2.end(), [&](const Token* tok2) { + return isSameExpression(true, false, tok1, tok2, library, false, false); + }); + }); } - return astContainerYield(tok2) == Library::Container::Yield::ITEM; + return false; } static ValueFlow::Value getLifetimeIteratorValue(const Token* tok, MathLib::bigint path = 0) @@ -866,6 +882,8 @@ void CheckStl::mismatchingContainerIterator() ValueFlow::Value val = getLifetimeIteratorValue(iterTok); if (!val.tokvalue) continue; + if (!val.isKnown() && Token::simpleMatch(val.tokvalue->astParent(), ":")) + continue; if (val.lifetimeKind != ValueFlow::Value::LifetimeKind::Iterator) continue; if (iterTok->str() == "*" && iterTok->astOperand1()->valueType() && iterTok->astOperand1()->valueType()->type == ValueType::ITERATOR) diff --git a/test/teststl.cpp b/test/teststl.cpp index 6768e78d81a..252239307a0 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -2134,6 +2134,51 @@ class TestStl : public TestFixture { "}\n"); ASSERT_EQUALS("", errout.str()); + // #11093 + check("struct S {\n" + " std::vector v1, v2;\n" + " void f(bool b) {\n" + " std::vector& v = b ? v1 : v2;\n" + " v.erase(v.begin());\n" + " }\n" + "};\n"); + ASSERT_EQUALS("", errout.str()); + + // #12377 + check("void f(bool b) {\n" + " std::vector *pv;\n" + " if (b) {\n" + " std::vector& r = get1();\n" + " pv = &r;\n" + " }\n" + " else {\n" + " std::vector& r = get2();\n" + " pv = &r;\n" + " }\n" + " std::vector::iterator it = pv->begin();\n" + " it = pv->erase(it, it + 2);\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("struct S {\n" + " std::vector v;\n" + " void f() {\n" + " std::vector* p = &v;\n" + " p->insert(std::find(p->begin(), p->end(), 0), 1);\n" + " }\n" + "};\n"); + ASSERT_EQUALS("", errout.str()); + + check("struct S {\n" + " std::vector v;\n" + " void f(int i) {\n" + " std::vector* p = &v;\n" + " if (p->size() > i)\n" + " p->erase(p->begin() + i, p->end());\n" + " }\n" + "};\n"); + ASSERT_EQUALS("", errout.str()); + // #11067 check("struct S {\n" " std::vector v;\n"