From 364a8c6349ddb75cd4ee18b1416b5f46a36ed713 Mon Sep 17 00:00:00 2001 From: chrchr Date: Tue, 23 Jan 2024 12:36:28 +0100 Subject: [PATCH 1/5] Fix #11093 FP: mismatchingContainerIterator caused by ternary operator to get reference to container --- lib/checkstl.cpp | 2 ++ test/teststl.cpp | 10 ++++++++++ 2 files changed, 12 insertions(+) diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index f1e7c01d23c..7c4cce79465 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -866,6 +866,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 (isSameIteratorContainerExpression(tok, val.tokvalue, mSettings->library)) diff --git a/test/teststl.cpp b/test/teststl.cpp index f02afead149..fb2d8834f35 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -2135,6 +2135,16 @@ class TestStl : public TestFixture { " s.v.erase(s.v.begin() + m);\n" "}\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()); } // Dereferencing invalid pointer From f97a8045d4bf83b4b834ff6c048b4e3aa5fca719 Mon Sep 17 00:00:00 2001 From: chrchr Date: Tue, 23 Jan 2024 15:39:41 +0100 Subject: [PATCH 2/5] Fix #12377 FP mismatchingContainerIterator with pointer to vector --- lib/checkstl.cpp | 23 ++++++++++++++++++----- test/teststl.cpp | 25 +++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 7c4cce79465..3077cfee81d 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,8 +709,13 @@ 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 (kind == ValueFlow::Value::LifetimeKind::Address || kind == ValueFlow::Value::LifetimeKind::Iterator) { + const auto address1 = getAddressContainer(tok1); + const auto address2 = getAddressContainer(tok2); + for (std::size_t i = 0; i < address1.size(); ++i) + for (std::size_t j = 0; j < address2.size(); ++j) + if (isSameExpression(true, false, address1[i], address2[j], library, false, false)) + return true; } return false; } diff --git a/test/teststl.cpp b/test/teststl.cpp index fb2d8834f35..44514c4f726 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -2145,6 +2145,31 @@ class TestStl : public TestFixture { " }\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()); } // Dereferencing invalid pointer From 3dd78b5ce330700ab737549410884f50017a5dd8 Mon Sep 17 00:00:00 2001 From: chrchr Date: Tue, 23 Jan 2024 18:09:39 +0100 Subject: [PATCH 3/5] Use stl algo --- lib/checkstl.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 3077cfee81d..2d700824109 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -712,10 +712,11 @@ static bool isSameIteratorContainerExpression(const Token* tok1, if (kind == ValueFlow::Value::LifetimeKind::Address || kind == ValueFlow::Value::LifetimeKind::Iterator) { const auto address1 = getAddressContainer(tok1); const auto address2 = getAddressContainer(tok2); - for (std::size_t i = 0; i < address1.size(); ++i) - for (std::size_t j = 0; j < address2.size(); ++j) - if (isSameExpression(true, false, address1[i], address2[j], library, false, false)) - return true; + 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 false; } From dcbd0dc32db50b89f192b3ab415e1b49a2a9761a Mon Sep 17 00:00:00 2001 From: chrchr Date: Thu, 25 Jan 2024 13:03:12 +0100 Subject: [PATCH 4/5] Add test --- test/teststl.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/teststl.cpp b/test/teststl.cpp index 44514c4f726..f82e8de2833 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -2170,6 +2170,16 @@ class TestStl : public TestFixture { " }\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()); } // Dereferencing invalid pointer From 0a2213532f0ca24c091af0bed6dbaee59948e164 Mon Sep 17 00:00:00 2001 From: chrchr Date: Mon, 29 Jan 2024 13:05:46 +0100 Subject: [PATCH 5/5] Fix --- lib/checkstl.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 5fa03316bf8..0eedb17563c 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -709,6 +709,8 @@ static bool isSameIteratorContainerExpression(const Token* tok1, if (isSameExpression(true, false, tok1, tok2, library, false, false)) { return !astIsContainerOwned(tok1) || !isTemporary(true, tok1, &library); } + 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); @@ -718,7 +720,7 @@ static bool isSameIteratorContainerExpression(const Token* tok1, }); }); } - return astContainerYield(tok2) == Library::Container::Yield::ITEM; + return false; } static ValueFlow::Value getLifetimeIteratorValue(const Token* tok, MathLib::bigint path = 0)