diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index e0bb83a18..52b80966f 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -314,6 +314,11 @@ void CheckStl::mismatchingContainerExpressionError(const Token *tok1, const Toke expr1 + "' and '" + expr2 + "' are used together.", CWE664, false); } +void CheckStl::sameIteratorExpressionError(const Token *tok) +{ + reportError(tok, Severity::style, "sameIteratorExpression", "Same iterators expression are used for algorithm.", CWE664, false); +} + static const std::set algorithm2 = { // func(begin1, end1 "binary_search", "copy", "copy_if", "equal_range" , "generate", "is_heap", "is_heap_until", "is_partitioned" @@ -392,6 +397,12 @@ void CheckStl::mismatchingContainers() if (!i) continue; const Token * const argTok = args[argnr - 1]; + if (i->first) { + firstArg = argTok; + } + if(i->last && firstArg && argTok && isSameExpression(true, false, firstArg, argTok, mSettings->library, false)) { + sameIteratorExpressionError(firstArg); + } const Variable *c = getContainer(argTok); if (c) { std::map::const_iterator it = containerNr.find(c); @@ -407,9 +418,7 @@ void CheckStl::mismatchingContainers() mismatchingContainersError(argTok); } } else { - if (i->first) { - firstArg = argTok; - } else if (i->last && firstArg && argTok) { + if (i->last && firstArg && argTok) { const Token * iter1 = getIteratorExpression(firstArg); const Token * iter2 = getIteratorExpression(argTok); if (iter1 && iter2 && !isSameExpression(true, false, iter1, iter2, mSettings->library, false)) { diff --git a/lib/checkstl.h b/lib/checkstl.h index 469045311..b867a767e 100644 --- a/lib/checkstl.h +++ b/lib/checkstl.h @@ -184,6 +184,7 @@ private: void iteratorsError(const Token* tok, const std::string& container1, const std::string& container2); void mismatchingContainersError(const Token* tok); void mismatchingContainerExpressionError(const Token *tok1, const Token *tok2); + void sameIteratorExpressionError(const Token *tok); void invalidIteratorError(const Token* tok, const std::string& func, const std::string& iterator_name); void invalidPointerError(const Token* tok, const std::string& func, const std::string& pointer_name); void stlBoundariesError(const Token* tok); @@ -212,6 +213,7 @@ private: c.iteratorsError(nullptr, "container1", "container2"); c.mismatchingContainersError(nullptr); c.mismatchingContainerExpressionError(nullptr, nullptr); + c.sameIteratorExpressionError(nullptr); c.dereferenceErasedError(nullptr, nullptr, "iter", false); c.stlOutOfBoundsError(nullptr, "i", "foo", false); c.negativeIndexError(nullptr, ValueFlow::Value(-1)); @@ -249,6 +251,7 @@ private: "- out of bounds errors\n" "- misuse of iterators when iterating through a container\n" "- mismatching containers in calls\n" + "- same iterators in calls\n" "- dereferencing an erased iterator\n" "- for vectors: using iterator/pointer after push_back has been used\n" "- optimisation: use empty() instead of size() to guarantee fast code\n" diff --git a/test/teststl.cpp b/test/teststl.cpp index 2fca86c1d..71d180ac6 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -55,6 +55,7 @@ private: TEST_CASE(iterator13); TEST_CASE(iterator14); // #8191 TEST_CASE(iteratorExpression); + TEST_CASE(iteratorSameExpression); TEST_CASE(dereference); TEST_CASE(dereference_break); // #3644 - handle "break" @@ -595,6 +596,41 @@ private: ASSERT_EQUALS("", errout.str()); } + void iteratorSameExpression() { + check("void f(std::vector v) {\n" + " std::for_each(v.begin(), v.begin(), [](int){});\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2]: (style) Same iterators expression are used for algorithm.\n", errout.str()); + + check("std::vector& g();\n" + "void f() {\n" + " std::for_each(g().begin(), g().begin(), [](int){});\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (style) Same iterators expression are used for algorithm.\n", errout.str()); + + check("void f(std::vector v) {\n" + " std::for_each(v.end(), v.end(), [](int){});\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2]: (style) Same iterators expression are used for algorithm.\n", errout.str()); + + check("std::vector& g();\n" + "void f() {\n" + " std::for_each(g().end(), g().end(), [](int){});\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (style) Same iterators expression are used for algorithm.\n", errout.str()); + + check("std::vector::iterator g();\n" + "void f(std::vector v) {\n" + " std::for_each(g(), g(), [](int){});\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (style) Same iterators expression are used for algorithm.\n", errout.str()); + + check("void f(std::vector::iterator it) {\n" + " std::for_each(it, it, [](int){});\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2]: (style) Same iterators expression are used for algorithm.\n", errout.str()); + } + // Dereferencing invalid pointer void dereference() { check("void f()\n"