diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 0206e5635..6c1e40a94 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -25,6 +25,7 @@ #include "symboldatabase.h" #include "token.h" #include "utils.h" +#include "astutils.h" #include #include @@ -304,6 +305,15 @@ void CheckStl::mismatchingContainersError(const Token *tok) reportError(tok, Severity::error, "mismatchingContainers", "Iterators of different containers are used together.", CWE664, false); } +void CheckStl::mismatchingContainerExpressionError(const Token *tok1, const Token *tok2) +{ + const std::string expr1(tok1 ? tok1->expressionString() : std::string()); + const std::string expr2(tok2 ? tok2->expressionString() : std::string()); + reportError(tok1, Severity::warning, "mismatchingContainerExpression", + "Iterators to containers from different expressions '" + + expr1 + "' and '" + expr2 + "' are used together.", 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" @@ -324,6 +334,7 @@ static const std::set algorithm1x1 = { // func(begin1, x, end1 static const std::string iteratorBeginFuncPattern = "begin|cbegin|rbegin|crbegin"; static const std::string iteratorEndFuncPattern = "end|cend|rend|crend"; +static const std::string iteratorFuncPattern = "begin|cbegin|rbegin|crbegin|end|cend|rend|crend ("; static const std::string pattern1x1_1 = "%name% . " + iteratorBeginFuncPattern + " ( ) , "; static const std::string pattern1x1_2 = "%name% . " + iteratorEndFuncPattern + " ( ) ,|)"; @@ -341,6 +352,20 @@ static const Variable *getContainer(const Token *argtok) return nullptr; } +static const Token * getIteratorExpression(const Token * tok, const Token * end) +{ + for(;tok != end;tok = tok->next()) { + if(Token::Match(tok, iteratorFuncPattern.c_str())) { + if(Token::Match(tok->previous(), ". %name% ( )")) { + return tok->previous()->astOperand1(); + } else if(Token::Match(tok, "%name% ( !!)")) { + return tok->next()->astOperand2(); + } + } + } + return nullptr; +} + void CheckStl::mismatchingContainers() { // Check if different containers are used in various calls of standard functions @@ -351,6 +376,7 @@ void CheckStl::mismatchingContainers() continue; const Token * const ftok = tok; const Token * const arg1 = tok->tokAt(2); + const Token * firstArg = nullptr; int argnr = 1; std::map containerNr; @@ -359,19 +385,31 @@ void CheckStl::mismatchingContainers() if (!i) continue; const Variable *c = getContainer(argTok); - if (!c) - continue; - std::map::const_iterator it = containerNr.find(c); - if (it == containerNr.end()) { - for (it = containerNr.begin(); it != containerNr.end(); ++it) { - if (it->second == i->container) { - mismatchingContainersError(argTok); - break; + if(c) { + std::map::const_iterator it = containerNr.find(c); + if (it == containerNr.end()) { + for (it = containerNr.begin(); it != containerNr.end(); ++it) { + if (it->second == i->container) { + mismatchingContainersError(argTok); + break; + } + } + containerNr[c] = i->container; + } else if (it->second != i->container) { + mismatchingContainersError(argTok); + } + } else { + if(i->first) { + firstArg = argTok; + } else if(i->last && firstArg && argTok) { + const Token * firstArgNext = firstArg->nextArgument() ? firstArg->nextArgument() : tok->linkAt(1); + const Token * iter1 = getIteratorExpression(firstArg, firstArgNext); + const Token * argTokNext = argTok->nextArgument() ? argTok->nextArgument() : tok->linkAt(1); + const Token * iter2 = getIteratorExpression(argTok, argTokNext); + if(iter1 && iter2 && !isSameExpression(true, false, iter1, iter2, mSettings->library, false)) { + mismatchingContainerExpressionError(iter1, iter2); } } - containerNr[c] = i->container; - } else if (it->second != i->container) { - mismatchingContainersError(argTok); } } const int ret = mSettings->library.returnValueContainer(ftok); diff --git a/lib/checkstl.h b/lib/checkstl.h index 5edf56605..469045311 100644 --- a/lib/checkstl.h +++ b/lib/checkstl.h @@ -183,6 +183,7 @@ private: void invalidIteratorError(const Token* tok, const std::string& iteratorName); 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 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); @@ -210,6 +211,7 @@ private: c.invalidIteratorError(nullptr, "iterator"); c.iteratorsError(nullptr, "container1", "container2"); c.mismatchingContainersError(nullptr); + c.mismatchingContainerExpressionError(nullptr, nullptr); c.dereferenceErasedError(nullptr, nullptr, "iter", false); c.stlOutOfBoundsError(nullptr, "i", "foo", false); c.negativeIndexError(nullptr, ValueFlow::Value(-1)); diff --git a/lib/token.cpp b/lib/token.cpp index 1e4e39bc4..69636b155 100644 --- a/lib/token.cpp +++ b/lib/token.cpp @@ -1248,7 +1248,7 @@ std::string Token::expressionString() const const Token * const top = this; const Token *start = top; while (start->astOperand1() && - (start->astOperand2() || !start->isUnaryPreOp() || Token::simpleMatch(start, "( )"))) + (start->astOperand2() || !start->isUnaryPreOp() || Token::simpleMatch(start, "( )") || start->str() == "{")) start = start->astOperand1(); const Token *end = top; while (end->astOperand1() && (end->astOperand2() || end->isUnaryPreOp())) { diff --git a/test/teststl.cpp b/test/teststl.cpp index d9986d889..2fca86c1d 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -54,6 +54,7 @@ private: TEST_CASE(iterator12); TEST_CASE(iterator13); TEST_CASE(iterator14); // #8191 + TEST_CASE(iteratorExpression); TEST_CASE(dereference); TEST_CASE(dereference_break); // #3644 - handle "break" @@ -532,6 +533,68 @@ private: ASSERT_EQUALS("", errout.str()); } + void iteratorExpression() { + check("std::vector& f();\n" + "std::vector& g();\n" + "void foo() {\n" + " (void)std::find(f().begin(), g().end(), 0);\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (warning) Iterators to containers from different expressions 'f()' and 'g()' are used together.\n", errout.str()); + + check("struct A {\n" + " std::vector& f();\n" + " std::vector& g();\n" + "};\n" + "void foo() {\n" + " (void)std::find(A().f().begin(), A().g().end(), 0);\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:6]: (warning) Iterators to containers from different expressions 'A().f()' and 'A().g()' are used together.\n", errout.str()); + + check("struct A {\n" + " std::vector& f();\n" + " std::vector& g();\n" + "};\n" + "void foo() {\n" + " (void)std::find(A{}.f().begin(), A{}.g().end(), 0);\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:6]: (warning) Iterators to containers from different expressions 'A{}.f()' and 'A{}.g()' are used together.\n", errout.str()); + + check("std::vector& f();\n" + "std::vector& g();\n" + "void foo() {\n" + " (void)std::find(begin(f()), end(g()), 0);\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (warning) Iterators to containers from different expressions 'f()' and 'g()' are used together.\n", errout.str()); + + check("struct A {\n" + " std::vector& f();\n" + " std::vector& g();\n" + "};\n" + "void foo() {\n" + " (void)std::find(A().f().begin(), A().f().end(), 0);\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("std::vector& f();\n" + "std::vector& g();\n" + "void foo() {\n" + " auto it = f().end();" + " (void)std::find(f().begin(), it, 0);\n" + " (void)std::find(f().begin(), it + 1, 0);\n" + " (void)std::find(f().begin() + 1, it + 1, 0);\n" + " (void)std::find(f().begin() + 1, it, 0);\n" + " (void)std::find(f().begin(), f().end(), 0);\n" + " (void)std::find(f().begin() + 1, f().end(), 0);\n" + " (void)std::find(f().begin(), f().end() - 1, 0);\n" + " (void)std::find(f().begin() + 1, f().end() - 1, 0);\n" + " (void)std::find(begin(f()), end(f()));\n" + " (void)std::find(begin(f()) + 1, end(f()), 0);\n" + " (void)std::find(begin(f()), end(f()) - 1, 0);\n" + " (void)std::find(begin(f()) + 1, end(f()) - 1, 0);\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + // Dereferencing invalid pointer void dereference() { check("void f()\n"