diff --git a/cfg/std.cfg b/cfg/std.cfg index 5c5de9995..dec4d8988 100644 --- a/cfg/std.cfg +++ b/cfg/std.cfg @@ -8574,7 +8574,6 @@ initializer list (7) string& replace (const_iterator i1, const_iterator i2, init - @@ -8594,6 +8593,9 @@ initializer list (7) string& replace (const_iterator i1, const_iterator i2, init + + + diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 7daa723c9..83ae5fb53 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -97,6 +97,16 @@ static bool containerYieldsElement(const Library::Container* container, const To return false; } +static bool containerPopsElement(const Library::Container* container, const Token* parent) +{ + if (Token::Match(parent, ". %name% (")) { + const Library::Container::Action action = container->getAction(parent->strAt(1)); + if (contains({ Library::Container::Action::POP }, action)) + return true; + } + return false; +} + static const Token* getContainerIndex(const Library::Container* container, const Token* parent) { if (Token::Match(parent, ". %name% (")) { @@ -148,8 +158,9 @@ void CheckStl::outOfBounds() continue; if (!value.errorSeverity() && !mSettings->severity.isEnabled(Severity::warning)) continue; - if (value.intvalue == 0 && (indexTok || (containerYieldsElement(container, parent) && - !containerAppendsElement(container, parent)))) { + if (value.intvalue == 0 && (indexTok || + (containerYieldsElement(container, parent) && !containerAppendsElement(container, parent)) || + containerPopsElement(container, parent))) { std::string indexExpr; if (indexTok && !indexTok->hasKnownValue()) indexExpr = indexTok->expressionString(); diff --git a/test/teststl.cpp b/test/teststl.cpp index 43590966d..0f2efccec 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -123,6 +123,7 @@ private: TEST_CASE(pushback13); TEST_CASE(insert1); TEST_CASE(insert2); + TEST_CASE(popback1); TEST_CASE(stlBoundaries1); TEST_CASE(stlBoundaries2); @@ -3061,6 +3062,31 @@ private: ASSERT_EQUALS("", errout.str()); } + void popback1() { // #11553 + check("void f() {\n" + " std::vector v;\n" + " v.pop_back();\n" + " std::list l;\n" + " l.pop_front();\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (error) Out of bounds access in expression 'v.pop_back()' because 'v' is empty.\n" + "[test.cpp:5]: (error) Out of bounds access in expression 'l.pop_front()' because 'l' is empty.\n", + errout.str()); + + check("void f(std::vector& v) {\n" + " if (v.empty()) {}\n" + " v.pop_back();\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (warning) Either the condition 'v.empty()' is redundant or expression 'v.pop_back()' cause access out of bounds.\n", + errout.str()); + + check("void f(std::vector& v) {\n" + " v.pop_back();\n" + " if (v.empty()) {}\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + void stlBoundaries1() { const std::string stlCont[] = { "list", "set", "multiset", "map", "multimap"