@@ -63,6 +63,8 @@
#include <initializer_list>
#endif
+#include <debug/debug.h>
+
namespace std _GLIBCXX_VISIBILITY(default)
{
_GLIBCXX_BEGIN_NAMESPACE_CONTAINER
@@ -778,7 +780,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
reference
operator[](size_type __n) _GLIBCXX_NOEXCEPT
- { return *(this->_M_impl._M_start + __n); }
+ {
+ __glibcxx_requires_subscript(__n);
+ return *(this->_M_impl._M_start + __n);
+ }
/**
* @brief Subscript access to the data contained in the %vector.
@@ -793,7 +798,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
const_reference
operator[](size_type __n) const _GLIBCXX_NOEXCEPT
- { return *(this->_M_impl._M_start + __n); }
+ {
+ __glibcxx_requires_subscript(__n);
+ return *(this->_M_impl._M_start + __n);
+ }
protected:
/// Safety check used only from at().
@@ -850,7 +858,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
reference
front() _GLIBCXX_NOEXCEPT
- { return *begin(); }
+ {
+ __glibcxx_requires_nonempty();
+ return *begin();
+ }
/**
* Returns a read-only (constant) reference to the data at the first
@@ -858,7 +869,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
const_reference
front() const _GLIBCXX_NOEXCEPT
- { return *begin(); }
+ {
+ __glibcxx_requires_nonempty();
+ return *begin();
+ }
/**
* Returns a read/write reference to the data at the last
@@ -866,7 +880,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
reference
back() _GLIBCXX_NOEXCEPT
- { return *(end() - 1); }
+ {
+ __glibcxx_requires_nonempty();
+ return *(end() - 1);
+ }
/**
* Returns a read-only (constant) reference to the data at the
@@ -874,7 +891,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
const_reference
back() const _GLIBCXX_NOEXCEPT
- { return *(end() - 1); }
+ {
+ __glibcxx_requires_nonempty();
+ return *(end() - 1);
+ }
// _GLIBCXX_RESOLVE_LIB_DEFECTS
// DR 464. Suggestion for new member functions in standard containers.
@@ -949,6 +969,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
void
pop_back() _GLIBCXX_NOEXCEPT
{
+ __glibcxx_requires_nonempty();
--this->_M_impl._M_finish;
_Alloc_traits::destroy(this->_M_impl, this->_M_impl._M_finish);
}
@@ -1051,6 +1072,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
iterator
insert(const_iterator __position, size_type __n, const value_type& __x)
{
+ __glibcxx_requires_valid_insert_position(__position);
difference_type __offset = __position - cbegin();
_M_fill_insert(begin() + __offset, __n, __x);
return begin() + __offset;
@@ -1071,7 +1093,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
void
insert(iterator __position, size_type __n, const value_type& __x)
- { _M_fill_insert(__position, __n, __x); }
+ {
+ __glibcxx_requires_valid_insert_position(__position);
+ _M_fill_insert(__position, __n, __x);
+ }
#endif
#if __cplusplus >= 201103L
@@ -1096,6 +1121,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
insert(const_iterator __position, _InputIterator __first,
_InputIterator __last)
{
+ __glibcxx_requires_valid_insert_position(__position);
difference_type __offset = __position - cbegin();
_M_insert_dispatch(begin() + __offset,
__first, __last, __false_type());
@@ -1121,6 +1147,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
insert(iterator __position, _InputIterator __first,
_InputIterator __last)
{
+ __glibcxx_requires_valid_insert_position(__position);
// Check whether it's an integral type. If so, it's not an iterator.
typedef typename std::__is_integer<_InputIterator>::__type _Integral;
_M_insert_dispatch(__position, __first, __last, _Integral());
@@ -1145,10 +1172,16 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
iterator
#if __cplusplus >= 201103L
erase(const_iterator __position)
- { return _M_erase(begin() + (__position - cbegin())); }
+ {
+ __glibcxx_requires_valid_erase_position(__position);
+ return _M_erase(begin() + (__position - cbegin()));
+ }
#else
erase(iterator __position)
- { return _M_erase(__position); }
+ {
+ __glibcxx_requires_valid_erase_position(__position);
+ return _M_erase(__position);
+ }
#endif
/**
@@ -1173,13 +1206,25 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
#if __cplusplus >= 201103L
erase(const_iterator __first, const_iterator __last)
{
+ __glibcxx_assert(__first < __last || !(__last < __first));
+ __glibcxx_assert((cbegin() < __first || !(__first < cbegin()))
+ && (__first < cend() || __first == __last));
+ __glibcxx_assert((cbegin() < __last || __first == __last)
+ && (__last < cend() || !(cend() < __last)));
const auto __beg = begin();
const auto __cbeg = cbegin();
return _M_erase(__beg + (__first - __cbeg), __beg + (__last - __cbeg));
}
#else
erase(iterator __first, iterator __last)
- { return _M_erase(__first, __last); }
+ {
+ __glibcxx_assert(__first < __last || !(__last < __first));
+ __glibcxx_assert((begin() < __first || !(__first < begin()))
+ && (__first < end() || __first == __last));
+ __glibcxx_assert((begin() < __last || __first == __last)
+ && (__last < end() || !(end() < __last)));
+ return _M_erase(__first, __last);
+ }
#endif
/**
@@ -1194,6 +1239,13 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
void
swap(vector& __x) _GLIBCXX_NOEXCEPT
{
+#if __cplusplus >= 201103L
+ __glibcxx_requires_cond(
+ _Alloc_traits::propagate_on_container_swap::value
+ || _M_get_Tp_allocator() == __x._M_get_Tp_allocator(),
+ _M_message(__gnu_debug::__msg_equal_allocs)
+ ._M_sequence(this, "this"));
+#endif
this->_M_impl._M_swap_data(__x._M_impl);
_Alloc_traits::_S_on_swap(_M_get_Tp_allocator(),
__x._M_get_Tp_allocator());
@@ -111,6 +111,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
insert(iterator __position, const value_type& __x)
#endif
{
+ __glibcxx_requires_valid_insert_position(__position);
const size_type __n = __position - begin();
if (this->_M_impl._M_finish != this->_M_impl._M_end_of_storage
&& __position == end())
@@ -301,6 +302,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
vector<_Tp, _Alloc>::
emplace(const_iterator __position, _Args&&... __args)
{
+ __glibcxx_requires_valid_insert_position(__position);
const size_type __n = __position - begin();
if (this->_M_impl._M_finish != this->_M_impl._M_end_of_storage
&& __position == end())
@@ -74,33 +74,26 @@ namespace __gnu_debug
# define __glibcxx_requires_heap_pred(_First,_Last,_Pred)
# define __glibcxx_requires_string(_String)
# define __glibcxx_requires_string_len(_String,_Len)
-# define __glibcxx_requires_subscript(_N)
# define __glibcxx_requires_irreflexive(_First,_Last)
# define __glibcxx_requires_irreflexive2(_First,_Last)
# define __glibcxx_requires_irreflexive_pred(_First,_Last,_Pred)
# define __glibcxx_requires_irreflexive_pred2(_First,_Last,_Pred)
-#ifdef _GLIBCXX_ASSERTIONS
-// Verify that [_First, _Last) forms a non-empty iterator range.
-# define __glibcxx_requires_non_empty_range(_First,_Last) \
- __glibcxx_assert(_First != _Last)
-// Verify that the container is nonempty
-# define __glibcxx_requires_nonempty() \
- __glibcxx_assert(! this->empty())
-#else
-# define __glibcxx_requires_non_empty_range(_First,_Last)
-# define __glibcxx_requires_nonempty()
#endif
+#ifndef _GLIBCXX_ASSERTIONS
+# define __glibcxx_requires_non_empty_range(_First,_Last)
+# define __glibcxx_requires_valid_insert_position(_Position)
+# define __glibcxx_requires_valid_erase_position(_Position)
+# define __glibcxx_requires_nonempty()
+# define __glibcxx_requires_subscript(_N)
#else
# include <debug/macros.h>
-# define __glibcxx_requires_cond(_Cond,_Msg) _GLIBCXX_DEBUG_VERIFY(_Cond,_Msg)
+# ifdef _GLIBCXX_DEBUG
# define __glibcxx_requires_valid_range(_First,_Last) \
__glibcxx_check_valid_range(_First,_Last)
-# define __glibcxx_requires_non_empty_range(_First,_Last) \
- __glibcxx_check_non_empty_range(_First,_Last)
# define __glibcxx_requires_sorted(_First,_Last) \
__glibcxx_check_sorted(_First,_Last)
# define __glibcxx_requires_sorted_pred(_First,_Last,_Pred) \
@@ -121,11 +114,9 @@ namespace __gnu_debug
__glibcxx_check_heap(_First,_Last)
# define __glibcxx_requires_heap_pred(_First,_Last,_Pred) \
__glibcxx_check_heap_pred(_First,_Last,_Pred)
-# define __glibcxx_requires_nonempty() __glibcxx_check_nonempty()
# define __glibcxx_requires_string(_String) __glibcxx_check_string(_String)
# define __glibcxx_requires_string_len(_String,_Len) \
__glibcxx_check_string_len(_String,_Len)
-# define __glibcxx_requires_subscript(_N) __glibcxx_check_subscript(_N)
# define __glibcxx_requires_irreflexive(_First,_Last) \
__glibcxx_check_irreflexive(_First,_Last)
# define __glibcxx_requires_irreflexive2(_First,_Last) \
@@ -136,6 +127,19 @@ namespace __gnu_debug
__glibcxx_check_irreflexive_pred2(_First,_Last,_Pred)
# include <debug/functions.h>
+#endif
+
+# define __glibcxx_requires_cond(_Cond,_Msg) _GLIBCXX_DEBUG_VERIFY(_Cond,_Msg)
+# define __glibcxx_requires_valid_insert_position(_Position) \
+ __glibcxx_check_insert2(_Position)
+# define __glibcxx_requires_valid_erase_position(_Position) \
+ __glibcxx_check_erase2(_Position)
+# define __glibcxx_requires_subscript(_N) __glibcxx_check_subscript(_N)
+# define __glibcxx_requires_non_empty_range(_First,_Last) \
+ __glibcxx_check_non_empty_range(_First,_Last)
+# define __glibcxx_requires_nonempty() __glibcxx_check_nonempty()
+
+# include <debug/formatter.h>
#endif
@@ -47,8 +47,22 @@ namespace __gnu_debug
{
using std::type_info;
+ // An arbitrary iterator pointer is not singular.
+ inline bool
+ __check_singular_aux(const void*) { return false; }
+
+ // We may have an iterator that derives from _Safe_iterator_base but isn't
+ // a _Safe_iterator.
template<typename _Iterator>
- bool __check_singular(const _Iterator&);
+ inline bool
+ __check_singular(const _Iterator& __x)
+ { return __check_singular_aux(std::__addressof(__x)); }
+
+ /** Non-NULL pointers are nonsingular. */
+ template<typename _Tp>
+ inline bool
+ __check_singular(const _Tp* __ptr)
+ { return __ptr == 0; }
class _Safe_sequence_base;
@@ -51,23 +51,6 @@ namespace __gnu_debug
template<typename _Sequence>
struct _Is_contiguous_sequence : std::__false_type { };
- // An arbitrary iterator pointer is not singular.
- inline bool
- __check_singular_aux(const void*) { return false; }
-
- // We may have an iterator that derives from _Safe_iterator_base but isn't
- // a _Safe_iterator.
- template<typename _Iterator>
- inline bool
- __check_singular(const _Iterator& __x)
- { return __check_singular_aux(std::__addressof(__x)); }
-
- /** Non-NULL pointers are nonsingular. */
- template<typename _Tp>
- inline bool
- __check_singular(const _Tp* __ptr)
- { return __ptr == 0; }
-
/** Assume that some arbitrary iterator is dereferenceable, because we
can't prove that it isn't. */
template<typename _Iterator>
@@ -86,6 +86,25 @@ _GLIBCXX_DEBUG_VERIFY(_Position._M_attached_to(this), \
._M_sequence(*this, "this") \
._M_iterator(_Position, #_Position))
+/** The same as previous macro but when _Position is not a debug iterator. */
+#if __cplusplus >= 201103L
+# define __glibcxx_check_insert2(_Position) \
+_GLIBCXX_DEBUG_VERIFY((cbegin() < _Position || !(_Position < cbegin())) \
+ && (_Position < cend() || !(cend() < _Position)), \
+ _M_message("invalid insert position outside container" \
+ " [begin, end) range") \
+ ._M_sequence(*this, "this") \
+ ._M_iterator(_Position, #_Position))
+#else
+# define __glibcxx_check_insert2(_Position) \
+_GLIBCXX_DEBUG_VERIFY((begin() < _Position || !(_Position < begin())) \
+ && (_Position < end() || !(end() < _Position)), \
+ _M_message("invalid insert position outside container" \
+ " [begin, end) range") \
+ ._M_sequence(*this, "this") \
+ ._M_iterator(_Position, #_Position))
+#endif
+
/** Verify that we can insert into *this after the iterator _Position.
* Insertion into a container after a specific position requires that
* the iterator be nonsingular, either dereferenceable or before-begin,
@@ -152,6 +171,24 @@ _GLIBCXX_DEBUG_VERIFY(_Position._M_attached_to(this), \
._M_sequence(*this, "this") \
._M_iterator(_Position, #_Position))
+/** Same as above but for normal (non-debug) containers. */
+#if __cplusplus >= 201103L
+# define __glibcxx_check_erase2(_Position) \
+_GLIBCXX_DEBUG_VERIFY((cbegin() < _Position || !(_Position < cbegin())) \
+ && _Position < cend(), \
+ _M_message(__gnu_debug::__msg_erase_bad) \
+ ._M_sequence(*this, "this") \
+ ._M_iterator(_Position, #_Position));
+#else
+# define __glibcxx_check_erase2(_Position) \
+_GLIBCXX_DEBUG_VERIFY((begin() < _Position || !(_Position < begin())) \
+ && _Position < end(), \
+ _M_message(__gnu_debug::__msg_erase_bad) \
+ ._M_sequence(*this, "this") \
+ ._M_iterator(_Position, #_Position));
+#endif
+
+
/** Verify that we can erase the element after the iterator
* _Position. We can erase the element if the _Position iterator is
* before a dereferenceable one and references this sequence.
@@ -406,49 +406,10 @@ namespace __debug
}
// element access:
- reference
- operator[](size_type __n) _GLIBCXX_NOEXCEPT
- {
- __glibcxx_check_subscript(__n);
- return _M_base()[__n];
- }
-
- const_reference
- operator[](size_type __n) const _GLIBCXX_NOEXCEPT
- {
- __glibcxx_check_subscript(__n);
- return _M_base()[__n];
- }
-
+ using _Base::operator[];
using _Base::at;
-
- reference
- front() _GLIBCXX_NOEXCEPT
- {
- __glibcxx_check_nonempty();
- return _Base::front();
- }
-
- const_reference
- front() const _GLIBCXX_NOEXCEPT
- {
- __glibcxx_check_nonempty();
- return _Base::front();
- }
-
- reference
- back() _GLIBCXX_NOEXCEPT
- {
- __glibcxx_check_nonempty();
- return _Base::back();
- }
-
- const_reference
- back() const _GLIBCXX_NOEXCEPT
- {
- __glibcxx_check_nonempty();
- return _Base::back();
- }
+ using _Base::front;
+ using _Base::back;
// _GLIBCXX_RESOLVE_LIB_DEFECTS
// DR 464. Suggestion for new member functions in standard containers.
new file mode 100644
@@ -0,0 +1,41 @@
+// -*- C++ -*-
+
+// Copyright (C) 2015 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 <iterator>
+
+#include <vector>
+
+void
+test01()
+{
+ std::vector<int> v1, v2;
+
+ v1.push_back(0);
+
+ v1.insert(v1.begin() + 2, v2.begin(), v2.end());
+}
+
+int
+main()
+{
+ test01();
+}
Hi I completed vector assertion mode. Here is the result of the new test you will find in the attached patch. With debug mode: /home/fdt/dev/gcc/build_git/x86_64-unknown-linux-gnu/libstdc++-v3/include/debug/safe_iterator.h:375: Error: attempt to advance a dereferenceable (start-of-sequence) iterator 2 steps, which falls outside its valid range. Objects involved in the operation: iterator @ 0x0x7fff1c346760 { type = __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*, std::__cxx1998::vector<int, std::allocator<int> > >, std::__debug::vector<int, std::allocator<int> > > (mutable iterator); state = dereferenceable (start-of-sequence); references sequence with type 'std::__debug::vector<int, std::allocator<int> >' @ 0x0x7fff1c3469a0 } XFAIL: 23_containers/vector/debug/insert8_neg.cc execution test With assertion mode: /home/fdt/dev/gcc/build_git/x86_64-unknown-linux-gnu/libstdc++-v3/include/bits/stl_vector.h:1124: Error: invalid insert position outside container [begin, end) range. Objects involved in the operation: sequence "this" @ 0x0x7fff60b1f870 { type = std::vector<int, std::allocator<int> >; } iterator "__position" @ 0x0x7fff60b1f860 { type = __gnu_cxx::__normal_iterator<int const*, std::vector<int, std::allocator<int> > >; } XFAIL: 23_containers/vector/debug/insert8_neg.cc execution test As expected assertion mode doesn't detect invalid operation on the iterator but only its usage later. There is still one debug assertion I would like to use in assertion mode: __valid_range. I could use it to check erase range and iterator range used on the insert method. But for that it means including <debug/helper_functions.h> and so: #include <bits/stl_iterator_base_types.h> // for iterator_traits, // categories and _Iter_base #include <bits/cpp_type_traits.h> // for __is_integer #include <bits/stl_pair.h> // for pair That looks fine to me, what do you think ? François On 19/09/2015 11:47, Jonathan Wakely wrote: > On 19 September 2015 at 10:12, Jonathan Wakely <jwakely.gcc@gmail.com> wrote: >> On 19 September 2015 at 08:31, François Dumont wrote: >>> On 16/09/2015 22:29, Jonathan Wakely wrote: >>>> No, because it is undefined to compare iterators that belong to >>>> different containers, or to compare pointers that point to different >>>> arrays. >>>> >>> (Written before Christopher reply:) >>> >>> At least program will compile only if iterator is coming from a vector >>> of the same type. So behavior is undefined only if user pass an invalid >>> iterator which is exactly what this check tries to detect, isn't it >>> paradoxical ? If this undefined behavior results in the program abortion >>> this is what should happen anyway. If it doesn't abort then the program >>> will definitely not behaves as expected so this check doesn't make >>> anything worst, no ? >> The problem is that undefined behaviour can "travel backwards in time". >> >> It's not as simple as saying that if the invalid check happens _then_ >> undefined behaviour happens afterwards. > Just to be clear, I agree that it can't hurt a correct program (except > for the small cost of doing the checks). > > My concern was that for an incorrect program (which is the entire > purpose of adding the checks) the results could be unpredictable. It > might abort, which is the desired behaviour, or it might do something > else and keep executing, and in that case it could be harmful for > debugging because users would look at the source and think "well my > iterators must be in range, otherwise that assertion would have > failed, so the bug must be elsewhere". > > However ... > >> However, Google seem to find these checks useful, and you and Chris >> are in favour, so let's keep them.