Patchwork PR 57779 New debug check

login
register
mail settings
Submitter François Dumont
Date July 31, 2013, 7:58 p.m.
Message ID <51F96C4C.1080800@gmail.com>
Download mbox | patch
Permalink /patch/263799/
State New
Headers show

Comments

François Dumont - July 31, 2013, 7:58 p.m.
Here is another proposal using std::common_type to find out how to 
instantiate std::less and std::greater_equal. Much easier with C++11 
features indeed.

Tested under linux x86_64 debug mode.

Ok to commit ?

François




On 07/25/2013 12:10 AM, Paolo Carlini wrote:
> On 07/24/2013 10:41 PM, François Dumont wrote:
>> On 07/24/2013 10:49 AM, Paolo Carlini wrote:
>>> Hi,
>>>
>>> I didn't follow the finest details of this (Jonathan will be able to 
>>> provide better comments) but it's not immediately obvious to me what 
>>> happens of the -1 + 1 trick when begin() == end().
>>>
>>> In my opinion, we should not add to cpp_type_traits.h, which would 
>>> become bigger even in normal-mode (in fact we would like to see it 
>>> going away...)
>>>
>>> Paolo.
>>>
>> The -1 +1 trick only take place if begin() != end().
> Ah good.
>> Do you want me to make part of the check only active in C++11 mode so 
>> that I simply rely on C++11 features ?
> Well, yes, I didn't consider that, but if it makes things much easier, 
> it seems a good idea. Given the way the headers are already structured 
> I think it's possible in c++11 mode to have std::less & co without, 
> eg, the big std::function, then the idea makes a lot of sense.
>
> Paolo.
>
Paolo Carlini - July 31, 2013, 9:50 p.m.
On 07/31/2013 09:58 PM, François Dumont wrote:
> Here is another proposal using std::common_type to find out how to 
> instantiate std::less and std::greater_equal. Much easier with C++11 
> features indeed.
>
> Tested under linux x86_64 debug mode.
>
> Ok to commit ?
Ok, thanks!

Paolo.

Patch

Index: include/debug/macros.h
===================================================================
--- include/debug/macros.h	(revision 201251)
+++ include/debug/macros.h	(working copy)
@@ -72,11 +72,11 @@ 
 */
 #define __glibcxx_check_insert(_Position)				\
 _GLIBCXX_DEBUG_VERIFY(!_Position._M_singular(),				\
-		      _M_message(__gnu_debug::__msg_insert_singular) \
+		      _M_message(__gnu_debug::__msg_insert_singular)	\
 		      ._M_sequence(*this, "this")			\
 		      ._M_iterator(_Position, #_Position));		\
 _GLIBCXX_DEBUG_VERIFY(_Position._M_attached_to(this),			\
-		      _M_message(__gnu_debug::__msg_insert_different) \
+		      _M_message(__gnu_debug::__msg_insert_different)	\
 		      ._M_sequence(*this, "this")			\
 		      ._M_iterator(_Position, #_Position))
 
@@ -101,15 +101,16 @@ 
  *  that it reference the sequence we are inserting into, and that the
  *  iterator range [_First, Last) is a valid (possibly empty)
  *  range. Note that this macro is only valid when the container is a
- *  _Safe_sequence and the 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.
+ *  _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_check_insert(_Position);					\
+_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__foreign_iterator(_Position,_First),\
+		      _M_message(__gnu_debug::__msg_insert_range_from_self)\
+		      ._M_iterator(_First, #_First)			\
+		      ._M_iterator(_Last, #_Last)			\
+		      ._M_sequence(*this, "this"))
 
 /** Verify that we can insert the values in the iterator range
  *  [_First, _Last) into *this after the iterator _Position.  Insertion
@@ -332,7 +333,7 @@ 
 		      _M_message(__gnu_debug::__msg_valid_load_factor)	\
                       ._M_sequence(*this, "this"))
 
-#define __glibcxx_check_equal_allocs(_Other)			\
+#define __glibcxx_check_equal_allocs(_Other)				\
 _GLIBCXX_DEBUG_VERIFY(this->get_allocator() == _Other.get_allocator(),	\
 		      _M_message(__gnu_debug::__msg_equal_allocs)	\
 		      ._M_sequence(*this, "this"))
Index: include/debug/list
===================================================================
--- include/debug/list	(revision 201251)
+++ include/debug/list	(working copy)
@@ -791,4 +791,11 @@ 
 } // namespace __debug
 } // namespace std
 
+namespace __gnu_debug
+{
+  template<class _Tp, class _Alloc>
+    struct _Insert_range_from_self_is_safe<std::__debug::list<_Tp, _Alloc> >
+    { enum { __value = 1 }; };
+}
+
 #endif
Index: include/debug/formatter.h
===================================================================
--- include/debug/formatter.h	(revision 201251)
+++ include/debug/formatter.h	(working copy)
@@ -114,7 +114,9 @@ 
     // unordered container buckets
     __msg_bucket_index_oob,
     __msg_valid_load_factor,
-    __msg_equal_allocs
+    // others
+    __msg_equal_allocs,
+    __msg_insert_range_from_self
   };
 
   class _Error_formatter
Index: include/debug/functions.h
===================================================================
--- include/debug/functions.h	(revision 201251)
+++ include/debug/functions.h	(working copy)
@@ -32,7 +32,11 @@ 
 #include <bits/c++config.h>
 #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/cpp_type_traits.h>	  // for __is_integer
+#if __cplusplus >= 201103L
+# include <bits/stl_function.h>		  // for less and greater_equal
+# include <type_traits>			  // for common_type
+#endif
 #include <debug/formatter.h>
 
 namespace __gnu_debug
@@ -40,6 +44,10 @@ 
   template<typename _Iterator, typename _Sequence>
     class _Safe_iterator;
 
+  template<typename _Sequence>
+    struct _Insert_range_from_self_is_safe
+    { enum { __value = 0 }; };
+
   // An arbitrary iterator pointer is not singular.
   inline bool
   __check_singular_aux(const void*) { return false; }
@@ -162,6 +170,127 @@ 
       return __first;
     }
 
+#if __cplusplus >= 201103L
+  template<typename _Iterator, typename _Sequence,
+	   typename _InputIterator,
+	   typename _PointerType1,
+	   typename _PointerType2>
+    inline bool
+    __foreign_iterator_aux4(const _Safe_iterator<_Iterator, _Sequence>& __it,
+			    _InputIterator __other,
+			    _PointerType1, _PointerType2)
+    {
+      typedef typename std::common_type<_PointerType1,
+					_PointerType2>::type _PointerType;
+      std::less<_PointerType> __l;
+      std::greater_equal<_PointerType> __ge;
+
+      return
+	__l(std::addressof(*__other),
+	    std::addressof(*(__it._M_get_sequence()->_M_base().begin())))
+	|| __ge(std::addressof(*__other),
+		std::addressof(*(__it._M_get_sequence()->_M_base().end() - 1)) + 1);
+      
+    }
+			  
+  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, __other,
+			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
+			   
+  /** Checks that iterators do not belong to the same sequence. */
+  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)
+    {
+      return
+#ifndef _GLIBCXX_DEBUG_PEDANTIC
+	_Insert_range_from_self_is_safe<_Sequence>::__value ||
+#endif
+	__it._M_get_sequence() != __other._M_get_sequence();
+    }
+			   
+#if __cplusplus >= 201103L
+  /* This overload detects when passing pointers to the contained elements rather
+     than using iterators.
+   */
+  template<typename _Iterator, typename _Sequence, typename _InputIterator>
+    inline bool
+    __foreign_iterator_aux2(const _Safe_iterator<_Iterator, _Sequence>& __it,
+			    _InputIterator __other,
+			    std::random_access_iterator_tag)
+    {
+      typedef typename _Sequence::const_iterator _ItType;
+      typedef typename std::iterator_traits<_ItType>::reference _Ref;
+      return __foreign_iterator_aux3(__it, __other,
+				     std::is_lvalue_reference<_Ref>());
+    }
+#endif
+			   
+  /* Fallback overload for which we can't say, assume it is valid. */
+  template<typename _Iterator, typename _Sequence, typename _InputIterator>
+    inline bool
+    __foreign_iterator_aux2(const _Safe_iterator<_Iterator, _Sequence>&,
+			   _InputIterator,
+			   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; }
+
+  template<typename _Iterator, typename _Sequence,
+	   typename _InputIterator>
+    inline bool
+    __foreign_iterator_aux(const _Safe_iterator<_Iterator, _Sequence>& __it,
+			   _InputIterator __other,
+			   std::__false_type)
+    {
+      return __foreign_iterator_aux2(__it, __other,
+				     std::__iterator_category(__it));
+    }
+
+  template<typename _Iterator, typename _Sequence,
+	   typename _InputIterator>
+    inline bool
+    __foreign_iterator(const _Safe_iterator<_Iterator, _Sequence>& __it,
+		       _InputIterator __other)
+    {
+      typedef typename std::__is_integer<_InputIterator>::__type _Integral;
+      return __foreign_iterator_aux(__it, __other, _Integral());
+    }
+
   /** Checks that __s is non-NULL or __n == 0, and then returns __s. */
   template<typename _CharT, typename _Integer>
     inline const _CharT*
Index: include/debug/forward_list
===================================================================
--- include/debug/forward_list	(revision 201251)
+++ include/debug/forward_list	(working copy)
@@ -796,6 +796,11 @@ 
       _S_Is_Beginnest(_BaseIt __it, const _Sequence* __seq)
       { return _S_Is(__it, __seq); }
     };
+
+  template<class _Tp, class _Alloc>
+    struct _Insert_range_from_self_is_safe<
+      std::__debug::forward_list<_Tp, _Alloc> >
+      { enum { __value = 1 }; };
 }
 
 #endif
Index: testsuite/util/debug/checks.h
===================================================================
--- testsuite/util/debug/checks.h	(revision 201251)
+++ testsuite/util/debug/checks.h	(working copy)
@@ -129,7 +129,7 @@ 
       c2.assign(last, first); // Expected failure
     }
 
-  // Check that invalid range of debug !random debug iterators is detected
+  // Check that invalid range of debug not random iterators is detected
   template<typename _Tp>
     void
     check_assign3()
@@ -377,6 +377,34 @@ 
     }
 
   template<typename _Tp>
+    void
+    check_insert4()
+    {
+      bool test __attribute__((unused)) = true;
+
+      typedef _Tp cont_type;
+      typedef typename cont_type::value_type cont_val_type;
+      typedef typename CopyableValueType<cont_val_type>::value_type val_type;
+      typedef std::list<val_type> list_type;
+
+      generate_unique<val_type> gu;
+
+      list_type l;
+      for (int i = 0; i != 5; ++i)
+        l.push_back(gu.build());
+      VERIFY(l.size() == 5);
+
+      typename list_type::iterator first = l.begin(); ++first;
+      typename list_type::iterator last = first; ++last; ++last;
+
+      cont_type c1;
+      InsertRangeHelper<cont_type>::Insert(c1, l.begin(), l.end());
+      VERIFY(c1.size() == 5);
+
+      c1.insert(c1.begin(), c1.begin(), c1.end()); // Expected failure.
+    }
+
+  template<typename _Tp>
     void use_invalid_iterator()
     {
       bool test __attribute__((unused)) = true;
Index: testsuite/23_containers/vector/debug/insert5_neg.cc
===================================================================
--- testsuite/23_containers/vector/debug/insert5_neg.cc	(revision 0)
+++ testsuite/23_containers/vector/debug/insert5_neg.cc	(revision 0)
@@ -0,0 +1,33 @@ 
+// Copyright (C) 2013 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 *-*-* } }
+
+#include <vector>
+#include <debug/checks.h>
+
+void test01()
+{
+  __gnu_test::check_insert4<std::vector<int> >();
+}
+
+int main()
+{
+  test01();
+  return 0;
+}
Index: testsuite/23_containers/vector/debug/insert6_neg.cc
===================================================================
--- testsuite/23_containers/vector/debug/insert6_neg.cc	(revision 0)
+++ testsuite/23_containers/vector/debug/insert6_neg.cc	(revision 0)
@@ -0,0 +1,48 @@ 
+// Copyright (C) 2013 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-do run { xfail *-*-* } }
+
+#include <vector>
+#include <debug/vector>
+#include <debug/checks.h>
+
+void test01()
+{
+  std::vector<bool> v;
+  __gnu_debug::vector<bool> dv;
+  for (int i = 0; i != 10; ++i)
+    {
+      v.push_back((i % 2) != 0);
+      dv.push_back((i % 2) == 0);
+    }
+
+  dv.insert(dv.begin(), v.begin(), v.begin() + 5);
+  VERIFY( dv.size() == 15 );
+}
+
+void test02()
+{
+  __gnu_test::check_insert4<__gnu_debug::vector<bool> >();
+}
+
+int main()
+{
+  test01();
+  test02();
+  return 0;
+}
Index: testsuite/23_containers/vector/debug/57779_neg.cc
===================================================================
--- testsuite/23_containers/vector/debug/57779_neg.cc	(revision 0)
+++ testsuite/23_containers/vector/debug/57779_neg.cc	(revision 0)
@@ -0,0 +1,38 @@ 
+// Copyright (C) 2013 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 "-std=gnu++11" }
+// { dg-require-debug-mode "" }
+// { dg-do run { xfail *-*-* } }
+
+#include <vector>
+#include <debug/checks.h>
+
+void test01()
+{
+  std::vector<int> v;
+  for (int i = 0; i != 10; ++i)
+    v.push_back(i);
+
+  v.insert(v.begin(), v.data() + 1, v.data() + 5); // Expected failure
+}
+
+int main()
+{
+  test01();
+  return 0;
+}
Index: testsuite/23_containers/deque/debug/insert5_neg.cc
===================================================================
--- testsuite/23_containers/deque/debug/insert5_neg.cc	(revision 0)
+++ testsuite/23_containers/deque/debug/insert5_neg.cc	(revision 0)
@@ -0,0 +1,33 @@ 
+// Copyright (C) 2013 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 *-*-* } }
+
+#include <deque>
+#include <debug/checks.h>
+
+void test01()
+{
+  __gnu_test::check_insert4<std::deque<int> >();
+}
+
+int main()
+{
+  test01();
+  return 0;
+}
Index: testsuite/23_containers/list/debug/insert5_neg.cc
===================================================================
--- testsuite/23_containers/list/debug/insert5_neg.cc	(revision 0)
+++ testsuite/23_containers/list/debug/insert5_neg.cc	(revision 0)
@@ -0,0 +1,34 @@ 
+// Copyright (C) 2013 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-options "-D_GLIBCXX_DEBUG_PEDANTIC" }
+// { dg-do run { xfail *-*-* } }
+
+#include <list>
+#include <debug/checks.h>
+
+void test01()
+{
+  __gnu_test::check_insert4<std::list<int> >();
+}
+
+int main()
+{
+  test01();
+  return 0;
+}
Index: src/c++11/debug.cc
===================================================================
--- src/c++11/debug.cc	(revision 201251)
+++ src/c++11/debug.cc	(working copy)
@@ -181,7 +181,8 @@ 
     "attempt to access container with out-of-bounds bucket index %2;,"
     " container only holds %3; buckets",
     "load factor shall be positive",
-    "allocators must be equal"
+    "allocators must be equal",
+    "attempt to insert with an iterator range [%1.name;, %2.name;) from this container"
   };
 
   void
@@ -695,7 +696,7 @@ 
 	      }
 	    
 	    __formatter->_M_format_word(__buf, __bufsize, "@ 0x%p\n", 
-					_M_variant._M_sequence._M_address);
+					_M_variant._M_iterator._M_sequence);
 	    __formatter->_M_print_word(__buf);
 	  }
 	__formatter->_M_print_word("}\n");
@@ -808,8 +809,11 @@ 
     if (__length == 0)
       return;
     
-    if ((_M_column + __length < _M_max_length)
-	|| (__length >= _M_max_length && _M_column == 1)) 
+    size_t __visual_length
+      = __word[__length - 1] == '\n' ? __length - 1 : __length;
+    if (__visual_length == 0
+	|| (_M_column + __visual_length < _M_max_length)
+	|| (__visual_length >= _M_max_length && _M_column == 1)) 
       {
 	// If this isn't the first line, indent
 	if (_M_column == 1 && !_M_first_line)
@@ -823,17 +827,17 @@ 
 	  }
 	
 	fprintf(stderr, "%s", __word);
-	_M_column += __length;
 	
 	if (__word[__length - 1] == '\n') 
 	  {
 	    _M_first_line = false;
 	    _M_column = 1;
 	  }
+	else
+	  _M_column += __length;
       }
     else
       {
-	_M_column = 1;
 	_M_print_word("\n");
 	_M_print_word(__word);
       }