From 8dcea26c10b854a2ad150157d227d1765a14fd70 Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Sat, 4 Dec 2021 05:55:56 -0600 Subject: [PATCH] Find iterator mismatch when using temporary containers (#3579) --- lib/checkstl.cpp | 16 ++++++++++++---- test/teststl.cpp | 21 +++++++++++++++++---- 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 77ba95fae..7c4661aae 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -344,9 +344,14 @@ void CheckStl::iteratorsError(const Token* tok, const Token* containerTok, const void CheckStl::iteratorsError(const Token* tok, const Token* containerTok, const std::string& containerName) { std::list callstack = { tok, containerTok }; - reportError(callstack, Severity::error, "iterators3", - "$symbol:" + containerName + "\n" - "Same iterator is used with containers '" + containerName + "' that are defined in different scopes.", CWE664, Certainty::normal); + reportError(callstack, + Severity::error, + "iterators3", + "$symbol:" + containerName + + "\n" + "Same iterator is used with containers '$symbol' that are temporaries or defined in different scopes.", + CWE664, + Certainty::normal); } // Error message used when dereferencing an iterator that has been erased.. @@ -709,8 +714,11 @@ static bool isSameIteratorContainerExpression(const Token* tok1, const Library& library, ValueFlow::Value::LifetimeKind kind = ValueFlow::Value::LifetimeKind::Iterator) { - if (isSameExpression(true, false, tok1, tok2, library, false, false)) + if (isSameExpression(true, false, tok1, tok2, library, false, false)) { + if (astIsContainerOwned(tok1) && isTemporary(true, tok1, &library)) + return false; return true; + } if (kind == ValueFlow::Value::LifetimeKind::Address) { return isSameExpression(true, false, getAddressContainer(tok1), getAddressContainer(tok2), library, false, false); } diff --git a/test/teststl.cpp b/test/teststl.cpp index 65190f140..2609fd599 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -1305,7 +1305,9 @@ private: " }\n" " }\n" "}"); - ASSERT_EQUALS("[test.cpp:7] -> [test.cpp:4]: (error) Same iterator is used with containers 'l1' that are defined in different scopes.\n", errout.str()); + ASSERT_EQUALS( + "[test.cpp:7] -> [test.cpp:4]: (error) Same iterator is used with containers 'l1' that are temporaries or defined in different scopes.\n", + errout.str()); check("void foo()\n" "{\n" @@ -1320,7 +1322,7 @@ private: "}"); TODO_ASSERT_EQUALS( "[test.cpp:7] -> [test.cpp:4]: (error) Same iterator is used with containers 'l1' that are defined in different scopes.\n", - "[test.cpp:7] -> [test.cpp:7]: (error) Same iterator is used with containers 'l1' that are defined in different scopes.\n[test.cpp:7]: (error) Dangerous comparison using operator< on iterator.\n", + "[test.cpp:7] -> [test.cpp:7]: (error) Same iterator is used with containers 'l1' that are temporaries or defined in different scopes.\n[test.cpp:7]: (error) Dangerous comparison using operator< on iterator.\n", errout.str()); check("void foo()\n" @@ -1336,7 +1338,7 @@ private: " }\n" "}"); ASSERT_EQUALS( - "[test.cpp:8] -> [test.cpp:4]: (error) Same iterator is used with containers 'l1' that are defined in different scopes.\n", + "[test.cpp:8] -> [test.cpp:4]: (error) Same iterator is used with containers 'l1' that are temporaries or defined in different scopes.\n", errout.str()); check("void foo()\n" @@ -1352,7 +1354,18 @@ private: " }\n" "}"); ASSERT_EQUALS( - "[test.cpp:8] -> [test.cpp:7]: (error) Same iterator is used with containers 'l1' that are defined in different scopes.\n", + "[test.cpp:8] -> [test.cpp:7]: (error) Same iterator is used with containers 'l1' that are temporaries or defined in different scopes.\n", + errout.str()); + + check("std::set g() {\n" + " static const std::set s = {1};\n" + " return s;\n" + "}\n" + "void f() {\n" + " if (g().find(2) == g().end()) {}\n" + "}\n"); + ASSERT_EQUALS( + "[test.cpp:6] -> [test.cpp:6]: (error) Same iterator is used with containers 'g()' that are temporaries or defined in different scopes.\n", errout.str()); }