Skip to content

Commit 50c8a0d

Browse files
Fix #11553 pop_back on empty container is UB (#4789)
1 parent ce3ba5c commit 50c8a0d

3 files changed

Lines changed: 42 additions & 3 deletions

File tree

cfg/std.cfg

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8574,7 +8574,6 @@ initializer list (7) string& replace (const_iterator i1, const_iterator i2, init
85748574
<function name="pop_back" action="pop"/>
85758575
<function name="push_front" action="push"/>
85768576
<function name="emplace_front" action="push"/>
8577-
<function name="pop_front" action="pop"/>
85788577
</size>
85798578
<access indexOperator="array-like">
85808579
<function name="at" yields="at_index"/>
@@ -8594,6 +8593,9 @@ initializer list (7) string& replace (const_iterator i1, const_iterator i2, init
85948593
</container>
85958594
<container id="stdDeque" startPattern="std :: deque &lt;" inherits="stdVectorDeque">
85968595
<type unstable="erase insert"/>
8596+
<size>
8597+
<function name="pop_front" action="pop"/>
8598+
</size>
85978599
</container>
85988600
<container id="stdArray" startPattern="std :: array &lt;" inherits="stdContainer" opLessAllowed="true">
85998601
<size templateParameter="1">

lib/checkstl.cpp

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,16 @@ static bool containerYieldsElement(const Library::Container* container, const To
9797
return false;
9898
}
9999

100+
static bool containerPopsElement(const Library::Container* container, const Token* parent)
101+
{
102+
if (Token::Match(parent, ". %name% (")) {
103+
const Library::Container::Action action = container->getAction(parent->strAt(1));
104+
if (contains({ Library::Container::Action::POP }, action))
105+
return true;
106+
}
107+
return false;
108+
}
109+
100110
static const Token* getContainerIndex(const Library::Container* container, const Token* parent)
101111
{
102112
if (Token::Match(parent, ". %name% (")) {
@@ -148,8 +158,9 @@ void CheckStl::outOfBounds()
148158
continue;
149159
if (!value.errorSeverity() && !mSettings->severity.isEnabled(Severity::warning))
150160
continue;
151-
if (value.intvalue == 0 && (indexTok || (containerYieldsElement(container, parent) &&
152-
!containerAppendsElement(container, parent)))) {
161+
if (value.intvalue == 0 && (indexTok ||
162+
(containerYieldsElement(container, parent) && !containerAppendsElement(container, parent)) ||
163+
containerPopsElement(container, parent))) {
153164
std::string indexExpr;
154165
if (indexTok && !indexTok->hasKnownValue())
155166
indexExpr = indexTok->expressionString();

test/teststl.cpp

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ class TestStl : public TestFixture {
123123
TEST_CASE(pushback13);
124124
TEST_CASE(insert1);
125125
TEST_CASE(insert2);
126+
TEST_CASE(popback1);
126127

127128
TEST_CASE(stlBoundaries1);
128129
TEST_CASE(stlBoundaries2);
@@ -3061,6 +3062,31 @@ class TestStl : public TestFixture {
30613062
ASSERT_EQUALS("", errout.str());
30623063
}
30633064

3065+
void popback1() { // #11553
3066+
check("void f() {\n"
3067+
" std::vector<int> v;\n"
3068+
" v.pop_back();\n"
3069+
" std::list<int> l;\n"
3070+
" l.pop_front();\n"
3071+
"}\n");
3072+
ASSERT_EQUALS("[test.cpp:3]: (error) Out of bounds access in expression 'v.pop_back()' because 'v' is empty.\n"
3073+
"[test.cpp:5]: (error) Out of bounds access in expression 'l.pop_front()' because 'l' is empty.\n",
3074+
errout.str());
3075+
3076+
check("void f(std::vector<int>& v) {\n"
3077+
" if (v.empty()) {}\n"
3078+
" v.pop_back();\n"
3079+
"}\n");
3080+
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",
3081+
errout.str());
3082+
3083+
check("void f(std::vector<int>& v) {\n"
3084+
" v.pop_back();\n"
3085+
" if (v.empty()) {}\n"
3086+
"}\n");
3087+
ASSERT_EQUALS("", errout.str());
3088+
}
3089+
30643090
void stlBoundaries1() {
30653091
const std::string stlCont[] = {
30663092
"list", "set", "multiset", "map", "multimap"

0 commit comments

Comments
 (0)