re PR libstdc++/60587 (debug-mode -std=c++11 vector::insert(pos, begin, end) dereferences begin too eagerly)

PR libstdc++/60587
	* include/debug/functions.h (_Is_contiguous_sequence): Define.
	(__foreign_iterator): Accept additional iterator. Do not dispatch on
	iterator category.
	(__foreign_iterator_aux2): Likewise. Add overload for iterators
	from different types of debug container. Use _Is_contiguous_sequence
	instead of is_lvalue_reference.
	(__foreign_iterator_aux3): Accept additional iterator. Avoid
	dereferencing past-the-end iterator.
	(__foreign_iterator_aux4): Use const value_type* instead of
	potentially user-defined const_pointer type.
	* include/debug/macros.h (__glibcxx_check_insert_range): Fix comment
	and pass end iterator to __gnu_debug::__foreign_iterator.
	(__glibcxx_check_insert_range_after): Likewise.
	(__glibcxx_check_max_load_factor): Fix comment.
	* include/debug/vector (_Is_contiguous_sequence): Define partial
	specializations.
	* testsuite/23_containers/vector/debug/57779_neg.cc: Remove
	-std=gnu++11 option and unused header.
	* testsuite/23_containers/vector/debug/60587.cc: New.
	* testsuite/23_containers/vector/debug/60587_neg.cc: New.

From-SVN: r208755

libstdc++-v3/ChangeLog

@@ -1,3 +1,27 @@
+2014-03-21  Jonathan Wakely  <jwakely@redhat.com>
+
+	PR libstdc++/60587
+	* include/debug/functions.h (_Is_contiguous_sequence): Define.
+	(__foreign_iterator): Accept additional iterator. Do not dispatch on
+	iterator category.
+	(__foreign_iterator_aux2): Likewise. Add overload for iterators
+	from different types of debug container. Use _Is_contiguous_sequence
+	instead of is_lvalue_reference.
+	(__foreign_iterator_aux3): Accept additional iterator. Avoid
+	dereferencing past-the-end iterator.
+	(__foreign_iterator_aux4): Use const value_type* instead of
+	potentially user-defined const_pointer type.
+	* include/debug/macros.h (__glibcxx_check_insert_range): Fix comment
+	and pass end iterator to __gnu_debug::__foreign_iterator.
+	(__glibcxx_check_insert_range_after): Likewise.
+	(__glibcxx_check_max_load_factor): Fix comment.
+	* include/debug/vector (_Is_contiguous_sequence): Define partial
+	specializations.
+	* testsuite/23_containers/vector/debug/57779_neg.cc: Remove
+	-std=gnu++11 option and unused header.
+	* testsuite/23_containers/vector/debug/60587.cc: New.
+	* testsuite/23_containers/vector/debug/60587_neg.cc: New.
+
 2014-03-20  Ulrich Weigand  <Ulrich.Weigand@de.ibm.com>
 
 	* crossconfig.m4: Support spu-*-elf* targets.

libstdc++-v3/include/debug/functions.h

@@ -34,8 +34,8 @@
 					  // _Iter_base
 #include <bits/cpp_type_traits.h>	  // for __is_integer
 #include <bits/move.h>                    // for __addressof and addressof
+# include <bits/stl_function.h>		  // for less
 #if __cplusplus >= 201103L
-# include <bits/stl_function.h>		  // for less and greater_equal
 # include <type_traits>			  // for is_lvalue_reference and __and_
 #endif
 #include <debug/formatter.h>
@@ -52,6 +52,9 @@ namespace __gnu_debug
     struct _Insert_range_from_self_is_safe
     { enum { __value = 0 }; };
 
+  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; }
@@ -175,123 +178,112 @@ namespace __gnu_debug
       return __first;
     }
 
-#if __cplusplus >= 201103L
+  /* Handle the case where __other is a pointer to _Sequence::value_type. */
-  // Default implementation.
   template<typename _Iterator, typename _Sequence>
     inline bool
     __foreign_iterator_aux4(const _Safe_iterator<_Iterator, _Sequence>& __it,
-			    typename _Sequence::const_pointer __begin,
+			    const typename _Sequence::value_type* __other)
-			    typename _Sequence::const_pointer __other)
     {
-      typedef typename _Sequence::const_pointer _PointerType;
+      typedef const typename _Sequence::value_type* _PointerType;
-      constexpr std::less<_PointerType> __l{};
+      typedef std::less<_PointerType> _Less;
-
+#if __cplusplus >= 201103L
-      return (__l(__other, __begin)
+      constexpr _Less __l{};
-	      || __l(std::addressof(*(__it._M_get_sequence()->_M_base().end()
+#else
-				      - 1)), __other));
+      const _Less __l = _Less();
-    }
-
-  // Fallback when address type cannot be implicitely casted to sequence
-  // const_pointer.
-  template<typename _Iterator, typename _Sequence,
-	   typename _InputIterator>
-    inline bool
-    __foreign_iterator_aux4(const _Safe_iterator<_Iterator, _Sequence>&,
-			    _InputIterator, ...)
-    { return true; }
-
-  template<typename _Iterator, typename _Sequence, typename _InputIterator>
-    inline bool
-    __foreign_iterator_aux3(const _Safe_iterator<_Iterator, _Sequence>& __it,
-			    _InputIterator __other,
-			    std::true_type)
-    {
-      // Only containers with all elements in contiguous memory can have their
-      // elements passed through pointers.
-      // Arithmetics is here just to make sure we are not dereferencing
-      // past-the-end iterator.
-      if (__it._M_get_sequence()->_M_base().begin()
-	  != __it._M_get_sequence()->_M_base().end())
-	if (std::addressof(*(__it._M_get_sequence()->_M_base().end() - 1))
-	    - std::addressof(*(__it._M_get_sequence()->_M_base().begin()))
-	    == __it._M_get_sequence()->size() - 1)
-	  return (__foreign_iterator_aux4
-		  (__it,
-		   std::addressof(*(__it._M_get_sequence()->_M_base().begin())),
-		   std::addressof(*__other)));
-      return true;
-    }
-			   
-  /* Fallback overload for which we can't say, assume it is valid. */
-  template<typename _Iterator, typename _Sequence, typename _InputIterator>
-    inline bool
-    __foreign_iterator_aux3(const _Safe_iterator<_Iterator, _Sequence>& __it,
-			    _InputIterator __other,
-			    std::false_type)
-    { return true; }
 #endif
+      const _Sequence* __seq = __it._M_get_sequence();
+      const _PointerType __begin = std::__addressof(*__seq->_M_base().begin());
+      const _PointerType __end = std::__addressof(*(__seq->_M_base().end()-1));
 
-  /** Checks that iterators do not belong to the same sequence. */
+      // Check whether __other points within the contiguous storage.
+      return __l(__other, __begin) || __l(__end, __other);
+    }
+
+  /* Fallback overload for when we can't tell, assume it is valid. */
+  template<typename _Iterator, typename _Sequence>
+    inline bool
+    __foreign_iterator_aux4(const _Safe_iterator<_Iterator, _Sequence>&, ...)
+    { return true; }
+
+  /* Handle sequences with contiguous storage */
+  template<typename _Iterator, typename _Sequence, typename _InputIterator>
+    inline bool
+    __foreign_iterator_aux3(const _Safe_iterator<_Iterator, _Sequence>& __it,
+			    const _InputIterator& __other,
+			    const _InputIterator& __other_end,
+			    std::__true_type)
+    {
+      if (__other == __other_end)
+	return true;  // inserting nothing is safe even if not foreign iters
+      if (__it._M_get_sequence()->begin() == __it._M_get_sequence()->end())
+	return true;  // can't be self-inserting if self is empty
+      return __foreign_iterator_aux4(__it, std::__addressof(*__other));
+    }
+
+  /* Handle non-contiguous containers, assume it is valid. */
+  template<typename _Iterator, typename _Sequence, typename _InputIterator>
+    inline bool
+    __foreign_iterator_aux3(const _Safe_iterator<_Iterator, _Sequence>&,
+			    const _InputIterator&, const _InputIterator&,
+			    std::__false_type)
+    { return true; }
+
+  /** Handle debug iterators from the same type of container. */
   template<typename _Iterator, typename _Sequence, typename _OtherIterator>
     inline bool
     __foreign_iterator_aux2(const _Safe_iterator<_Iterator, _Sequence>& __it,
 		const _Safe_iterator<_OtherIterator, _Sequence>& __other,
-		std::input_iterator_tag)
+		const _Safe_iterator<_OtherIterator, _Sequence>&)
     { return __it._M_get_sequence() != __other._M_get_sequence(); }
 
-#if __cplusplus >= 201103L
+  /** Handle debug iterators from different types of container. */
-  /* This overload detects when passing pointers to the contained elements
+  template<typename _Iterator, typename _Sequence, typename _OtherIterator,
-     rather than using iterators.
+	   typename _OtherSequence>
-   */
+    inline bool
+    __foreign_iterator_aux2(const _Safe_iterator<_Iterator, _Sequence>& __it,
+		const _Safe_iterator<_OtherIterator, _OtherSequence>&,
+		const _Safe_iterator<_OtherIterator, _OtherSequence>&)
+    { return true; }
+
+  /* Handle non-debug iterators. */
   template<typename _Iterator, typename _Sequence, typename _InputIterator>
     inline bool
     __foreign_iterator_aux2(const _Safe_iterator<_Iterator, _Sequence>& __it,
-			    _InputIterator __other,
+			    const _InputIterator& __other,
-			    std::random_access_iterator_tag)
+			    const _InputIterator& __other_end)
     {
-      typedef typename _Sequence::const_iterator _ItType;
+      return __foreign_iterator_aux3(__it, __other, __other_end,
-      typedef typename std::iterator_traits<_ItType>::reference _Ref;
+				     _Is_contiguous_sequence<_Sequence>());
-      return __foreign_iterator_aux3(__it, __other,
-				     std::is_lvalue_reference<_Ref>());
     }
-#endif
 
-  /* Fallback overload for which we can't say, assume it is valid. */
+  /* Handle the case where we aren't really inserting a range after all */
-  template<typename _Iterator, typename _Sequence, typename _InputIterator>
+  template<typename _Iterator, typename _Sequence, typename _Integral>
     inline bool
-    __foreign_iterator_aux2(const _Safe_iterator<_Iterator, _Sequence>&,
+    __foreign_iterator_aux(const _Safe_iterator<_Iterator, _Sequence>&,
-			   _InputIterator,
+			   _Integral, _Integral,
-			   std::input_iterator_tag)
-    { return true; }
-			   
-  template<typename _Iterator, typename _Sequence,
-	   typename _Integral>
-    inline bool
-    __foreign_iterator_aux(const _Safe_iterator<_Iterator, _Sequence>& __it,
-			   _Integral __other,
 			   std::__true_type)
     { return true; }
 
+  /* Handle all iterators. */
   template<typename _Iterator, typename _Sequence,
 	   typename _InputIterator>
     inline bool
     __foreign_iterator_aux(const _Safe_iterator<_Iterator, _Sequence>& __it,
-			   _InputIterator __other,
+			   _InputIterator __other, _InputIterator __other_end,
 			   std::__false_type)
     {
-      return (_Insert_range_from_self_is_safe<_Sequence>::__value
+      return _Insert_range_from_self_is_safe<_Sequence>::__value
-	      || __foreign_iterator_aux2(__it, __other,
+	     || __foreign_iterator_aux2(__it, __other, __other_end);
-					 std::__iterator_category(__it)));
     }
 
   template<typename _Iterator, typename _Sequence,
 	   typename _InputIterator>
     inline bool
     __foreign_iterator(const _Safe_iterator<_Iterator, _Sequence>& __it,
-		       _InputIterator __other)
+		       _InputIterator __other, _InputIterator __other_end)
     {
       typedef typename std::__is_integer<_InputIterator>::__type _Integral;
-      return __foreign_iterator_aux(__it, __other, _Integral());
+      return __foreign_iterator_aux(__it, __other, __other_end, _Integral());
     }
 
   /** Checks that __s is non-NULL or __n == 0, and then returns __s. */

libstdc++-v3/include/debug/macros.h

@@ -99,14 +99,15 @@ _GLIBCXX_DEBUG_VERIFY(!_Position._M_is_end(),				\
  *  into a container at a specific position requires that the iterator
  *  be nonsingular (i.e., either dereferenceable or past-the-end),
  *  that it reference the sequence we are inserting into, and that the
- *  iterator range [_First, Last) is a valid (possibly empty)
+ *  iterator range [_First, _Last) is a valid (possibly empty)
- *  range. Note that this macro is only valid when the container is a
+ *  range which does not reference the sequence we are inserting into.
+ *  Note that this macro is only valid when the container is a
  *  _Safe_sequence and the _Position iterator is a _Safe_iterator.
 */
 #define __glibcxx_check_insert_range(_Position,_First,_Last)		\
 __glibcxx_check_valid_range(_First,_Last);				\
 __glibcxx_check_insert(_Position);					\
-_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__foreign_iterator(_Position,_First),\
+_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__foreign_iterator(_Position,_First,_Last),\
 		      _M_message(__gnu_debug::__msg_insert_range_from_self)\
 		      ._M_iterator(_First, #_First)			\
 		      ._M_iterator(_Last, #_Last)			\
@@ -117,18 +118,15 @@ _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__foreign_iterator(_Position,_First),\
  *  into a container after a specific position requires that the iterator
  *  be nonsingular (i.e., either dereferenceable or past-the-end),
  *  that it reference the sequence we are inserting into, and that the
- *  iterator range [_First, Last) is a valid (possibly empty)
+ *  iterator range [_First, _Last) is a valid (possibly empty)
- *  range. Note that this macro is only valid when the container is a
+ *  range which does not reference the sequence we are inserting into.
- *  _Safe_sequence and the iterator is a _Safe_iterator.
+ *  Note that this macro is only valid when the container is a
- *
+ *  _Safe_sequence and the _Position iterator is a _Safe_iterator.
- *  @todo We would like to be able to check for noninterference of
- *  _Position and the range [_First, _Last), but that can't (in
- *  general) be done.
 */
 #define __glibcxx_check_insert_range_after(_Position,_First,_Last)	\
 __glibcxx_check_valid_range(_First,_Last);				\
 __glibcxx_check_insert_after(_Position);				\
-_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__foreign_iterator(_Position,_First),\
+_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__foreign_iterator(_Position,_First,_Last),\
 		      _M_message(__gnu_debug::__msg_insert_range_from_self)\
 		      ._M_iterator(_First, #_First)			\
 		      ._M_iterator(_Last, #_Last)			\
@@ -343,7 +341,7 @@ _GLIBCXX_DEBUG_VERIFY(this != &_Other,					\
 		      _M_message(__gnu_debug::__msg_self_move_assign)	\
                       ._M_sequence(*this, "this"))
 
-// Verify that load factor is position
+// Verify that load factor is positive
 #define __glibcxx_check_max_load_factor(_F)				\
 _GLIBCXX_DEBUG_VERIFY(_F > 0.0f,					\
 		      _M_message(__gnu_debug::__msg_valid_load_factor)	\

libstdc++-v3/include/debug/vector

@@ -718,4 +718,17 @@ namespace __debug
 
 } // namespace std
 
+namespace __gnu_debug
+{
+  template<typename _Tp, typename _Alloc>
+    struct _Is_contiguous_sequence<std::__debug::vector<_Tp, _Alloc> >
+    : std::__true_type
+    { };
+
+  template<typename _Alloc>
+    struct _Is_contiguous_sequence<std::__debug::vector<bool, _Alloc> >
+    : std::__false_type
+    { };
+}
+
 #endif

libstdc++-v3/testsuite/23_containers/vector/debug/57779_neg.cc

@@ -15,12 +15,10 @@
 // with this library; see the file COPYING3.  If not see
 // <http://www.gnu.org/licenses/>.
 //
-// { dg-options "-std=gnu++11" }
 // { dg-require-debug-mode "" }
 // { dg-do run { xfail *-*-* } }
 
 #include <vector>
-#include <debug/checks.h>
 
 void test01()
 {

libstdc++-v3/testsuite/23_containers/vector/debug/60587.cc

@@ -0,0 +1,35 @@
+// Copyright (C) 2014 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-require-debug-mode "" }
+
+// PR libstdc++/60587
+
+#include <vector>
+
+int main() {
+    std::vector<int> a, b;
+    a.push_back(1);
+    a.insert(a.end(), b.begin(), b.end());
+    b.push_back(1L);
+    a.insert(a.end(), b.begin(), b.end());
+
+    std::vector<long> c;
+    a.insert(a.end(), c.begin(), c.end());
+    c.push_back(1L);
+    a.insert(a.end(), c.begin(), c.end());
+}

libstdc++-v3/testsuite/23_containers/vector/debug/60587_neg.cc

@@ -0,0 +1,29 @@
+// Copyright (C) 2014 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-require-debug-mode "" }
+// { dg-do run { xfail *-*-* } }
+
+// PR libstdc++/60587
+
+#include <vector>
+
+int main() {
+    std::vector<int> a;
+    a.push_back(1);
+    a.insert(a.end(), a.begin(), a.begin());  // Expected to abort here
+}