[PATCH] Add assertion to _Rb_tree::erase to check for end iterators
Jonathan Wakely
jwakely@redhat.com
Fri Dec 16 18:23:00 GMT 2016
This adds assertions to catch assoc.erase(assoc.end()) when
_GLIBCXX_ASSERTIONS is defined. Without the assertion it usually leads
to a double free, which leads to termination for some mallocs anyway.
Because the erase(first, last) form called erase(first++) in a loop
I've changed that to bypass it and go straight to _M_erase_aux(first++),
so it doesn't do the assertion on every iteration of the loop. That
means the assertion won't fail in a case like this:
std::set<int> s1, s2;
s1.erase(s1.begin(), s2.end());
If we checked the assertion then when we reached the end of s1 we'd
abort, rather than try to keep incrementing the first iterator until
we reach the end of the other container (which will never happen). I
think this mistake is much less common, and checking the assertion on
every iteration isn't worth it. (The full-fat Debug Mode still catches
that mistake).
The erase(const Key&) form also called erase(iter) in a loop, and in
that case we know that iter != end() for all calls, so the assertion
is definitely not valuable.
* include/bits/stl_tree.h (_Rb_tree::_M_erase_aux(const_iterator)):
Add assertion for undefined argument.
(_Rb_tree::_M_erase_aux(const_iterator, const_iterator)): Call
_M_erase_aux directly instead of through erase.
(_Rb_tree::_M_erase_aux(const Key&)): Likewise.
* testsuite/23_containers/map/modifiers/erase/end_neg.cc: New test.
There's also a second patch to re-use the Doxygen comments for
erase(const_iterator) for the erase(iterator) overloads.
Tested x86_64-linux, committed to trunk.
-------------- next part --------------
commit 0179abf0b86f5b833a653213941f07448be2d5ab
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Fri Dec 16 14:21:53 2016 +0000
Add assertion to _Rb_tree::erase to check for end iterators
* include/bits/stl_tree.h (_Rb_tree::_M_erase_aux(const_iterator)):
Add assertion for undefined argument.
(_Rb_tree::_M_erase_aux(const_iterator, const_iterator)): Call
_M_erase_aux directly instead of through erase.
(_Rb_tree::_M_erase_aux(const Key&)): Likewise.
* testsuite/23_containers/map/modifiers/erase/end_neg.cc: New test.
diff --git a/libstdc++-v3/include/bits/stl_tree.h b/libstdc++-v3/include/bits/stl_tree.h
index 925066c..735fada 100644
--- a/libstdc++-v3/include/bits/stl_tree.h
+++ b/libstdc++-v3/include/bits/stl_tree.h
@@ -1104,6 +1104,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
iterator
erase(const_iterator __position)
{
+ __glibcxx_assert(__position != end());
const_iterator __result = __position;
++__result;
_M_erase_aux(__position);
@@ -1115,6 +1116,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
iterator
erase(iterator __position)
{
+ __glibcxx_assert(__position != end());
iterator __result = __position;
++__result;
_M_erase_aux(__position);
@@ -1123,11 +1125,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
#else
void
erase(iterator __position)
- { _M_erase_aux(__position); }
+ {
+ __glibcxx_assert(__position != end());
+ _M_erase_aux(__position);
+ }
void
erase(const_iterator __position)
- { _M_erase_aux(__position); }
+ {
+ __glibcxx_assert(__position != end());
+ _M_erase_aux(__position);
+ }
#endif
size_type
erase(const key_type& __x);
@@ -2477,7 +2485,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
clear();
else
while (__first != __last)
- erase(__first++);
+ _M_erase_aux(__first++);
}
template<typename _Key, typename _Val, typename _KeyOfValue,
@@ -2488,7 +2496,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
{
pair<iterator, iterator> __p = equal_range(__x);
const size_type __old_size = size();
- erase(__p.first, __p.second);
+ _M_erase_aux(__p.first, __p.second);
return __old_size - size();
}
diff --git a/libstdc++-v3/testsuite/23_containers/map/modifiers/erase/end_neg.cc b/libstdc++-v3/testsuite/23_containers/map/modifiers/erase/end_neg.cc
new file mode 100644
index 0000000..f01a99b
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/map/modifiers/erase/end_neg.cc
@@ -0,0 +1,35 @@
+// Copyright (C) 2016 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-D_GLIBCXX_ASSERTIONS" }
+// { dg-do run { xfail *-*-* } }
+
+#include <map>
+
+void
+test01()
+{
+ std::map<int, int> m;
+ m[0];
+ m.erase(m.end());
+}
+
+int
+main()
+{
+ test01();
+}
-------------- next part --------------
commit 8d800cbbdd323bfe3dae11a6f166e1e9807bc461
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Fri Dec 16 14:25:54 2016 +0000
Reuse Doxygen comments for map::erase overloads
* include/bits/stl_map.h (map::erase(iterator)): Add Doxygen markup
to reuse documentation for erase(const_iterator) overload.
* include/bits/stl_multimap.h (multimap::erase(iterator)): Likewise.
diff --git a/libstdc++-v3/include/bits/stl_map.h b/libstdc++-v3/include/bits/stl_map.h
index bbd0a97..2d8b8ec 100644
--- a/libstdc++-v3/include/bits/stl_map.h
+++ b/libstdc++-v3/include/bits/stl_map.h
@@ -999,6 +999,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
* the element, and that if the element is itself a pointer,
* the pointed-to memory is not touched in any way. Managing
* the pointer is the user's responsibility.
+ *
+ * @{
*/
iterator
erase(const_iterator __position)
@@ -1009,6 +1011,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
iterator
erase(iterator __position)
{ return _M_t.erase(__position); }
+ // @}
#else
/**
* @brief Erases an element from a %map.
diff --git a/libstdc++-v3/include/bits/stl_multimap.h b/libstdc++-v3/include/bits/stl_multimap.h
index a5f775b..418b1f5 100644
--- a/libstdc++-v3/include/bits/stl_multimap.h
+++ b/libstdc++-v3/include/bits/stl_multimap.h
@@ -669,6 +669,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
* and that if the element is itself a pointer, the pointed-to memory is
* not touched in any way. Managing the pointer is the user's
* responsibility.
+ *
+ * @{
*/
iterator
erase(const_iterator __position)
@@ -679,6 +681,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
iterator
erase(iterator __position)
{ return _M_t.erase(__position); }
+ // @}
#else
/**
* @brief Erases an element from a %multimap.
More information about the Libstdc++
mailing list