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.
commit 0179abf0b86f5b833a653213941f07448be2d5ab Author: Jonathan Wakely <jwak...@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(); +}
commit 8d800cbbdd323bfe3dae11a6f166e1e9807bc461 Author: Jonathan Wakely <jwak...@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.