diff mbox

PR 58191 patch

Message ID 5220FF97.9060309@gmail.com
State New
Headers show

Commit Message

François Dumont Aug. 30, 2013, 8:24 p.m. UTC 
Hi

     I finally generalized  this method to other debug functions, it is 
more consistent and clean the implementation of the debug checks. For 
4.8 branch I will limit it to just what need to be really fixed.

2013-08-30  François Dumont  <fdumont@gcc.gnu.org>

     PR libstdc++/58191
     * include/debug/macros.h (__glibcxx_check_partitioned_lower): Add
     __gnu_debug::__base calls on iterators passed to internal debug
     check.
     (__glibcxx_check_partitioned_lower_pred): Likewise.
     (__glibcxx_check_partitioned_upper): Likewise.
     (__glibcxx_check_partitioned_upper_pred): Likewise.
     (__glibcxx_check_sorted): Likewise.
     (__glibcxx_check_sorted_pred): Likewise.
     (__glibcxx_check_sorted_set): Likewise.
     (__glibcxx_check_sorted_set_pred): Likewise.
     * include/debug/functions.h (__check_partitioned_lower):
     Remove code to detect safe iterators.
     (__check_partitioned_upper): Likewise.
     (__check_sorted): Likewise.

François


On 08/27/2013 11:08 PM, Paolo Carlini wrote:
> On 08/27/2013 10:57 PM, François Dumont wrote:
>> Hi
>>
>>     Here is a patch to fix the small PR 58191 regression. I don't 
>> remember why I hadn't used the __gnu_debug::__base from the start 
>> rather than trying to reproduce its behavior within the 
>> __check_partitioned* methods. This way we only detect random access 
>> safe iterator to enhance performance but do not check the iterator 
>> category otherwise, concept checks are there for that reason.
>>
>>     The patch is generated from the 4.8 branch. I still need to reg 
>> test it but if it succeeded is it ok to commit ? Just in trunk or 
>> also in 4.8 branch ?
> Thanks. Let's play safe, let's apply it to mainline and let's ask 
> people on the audit trail of the bug too to test it. If everything 
> goes well let's backport it to the branch in a few weeks.
>
> Thanks again,
> Paolo.
>
diff mbox

Patch

Index: include/debug/functions.h
===================================================================
--- include/debug/functions.h	(revision 201966)
+++ include/debug/functions.h	(working copy)
@@ -336,15 +336,6 @@ 
       return true;
     }
 
-  // For performance reason, as the iterator range has been validated, check on
-  // random access safe iterators is done using the base iterator.
-  template<typename _Iterator, typename _Sequence>
-    inline bool
-    __check_sorted_aux(const _Safe_iterator<_Iterator, _Sequence>& __first,
-		       const _Safe_iterator<_Iterator, _Sequence>& __last,
-		       std::random_access_iterator_tag __tag)
-  { return __check_sorted_aux(__first.base(), __last.base(), __tag); }
-
   // Can't check if an input iterator sequence is sorted, because we can't step
   // through the sequence.
   template<typename _InputIterator, typename _Predicate>
@@ -371,17 +362,6 @@ 
       return true;
     }
 
-  // For performance reason, as the iterator range has been validated, check on
-  // random access safe iterators is done using the base iterator.
-  template<typename _Iterator, typename _Sequence,
-	   typename _Predicate>
-    inline bool
-    __check_sorted_aux(const _Safe_iterator<_Iterator, _Sequence>& __first,
-		       const _Safe_iterator<_Iterator, _Sequence>& __last,
-		       _Predicate __pred,
-		       std::random_access_iterator_tag __tag)
-  { return __check_sorted_aux(__first.base(), __last.base(), __pred, __tag); }
-
   // Determine if a sequence is sorted.
   template<typename _InputIterator>
     inline bool
@@ -470,11 +450,13 @@ 
       return __check_sorted_set_aux(__first, __last, __pred, _SameType());
    }
 
+  // _GLIBCXX_RESOLVE_LIB_DEFECTS
+  // 270. Binary search requirements overly strict
+  // Determine if a sequence is partitioned w.r.t. this element.
   template<typename _ForwardIterator, typename _Tp>
     inline bool
-  __check_partitioned_lower_aux(_ForwardIterator __first,
-				_ForwardIterator __last, const _Tp& __value,
-				std::forward_iterator_tag)
+    __check_partitioned_lower(_ForwardIterator __first,
+			      _ForwardIterator __last, const _Tp& __value)
     {
       while (__first != __last && *__first < __value)
 	++__first;
@@ -487,38 +469,11 @@ 
       return __first == __last;
     }
 
-  // For performance reason, as the iterator range has been validated, check on
-  // random access safe iterators is done using the base iterator.
-  template<typename _Iterator, typename _Sequence, typename _Tp>
-    inline bool
-    __check_partitioned_lower_aux(
-			const _Safe_iterator<_Iterator, _Sequence>& __first,
-			const _Safe_iterator<_Iterator, _Sequence>& __last,
-			const _Tp& __value,
-			std::random_access_iterator_tag __tag)
-    {
-      return __check_partitioned_lower_aux(__first.base(), __last.base(),
-					   __value, __tag);
-    }
-
-  // _GLIBCXX_RESOLVE_LIB_DEFECTS
-  // 270. Binary search requirements overly strict
-  // Determine if a sequence is partitioned w.r.t. this element.
   template<typename _ForwardIterator, typename _Tp>
     inline bool
-    __check_partitioned_lower(_ForwardIterator __first,
+    __check_partitioned_upper(_ForwardIterator __first,
 			      _ForwardIterator __last, const _Tp& __value)
     {
-      return __check_partitioned_lower_aux(__first, __last, __value,
-					   std::__iterator_category(__first));
-    }
-
-  template<typename _ForwardIterator, typename _Tp>
-    inline bool
-    __check_partitioned_upper_aux(_ForwardIterator __first,
-				  _ForwardIterator __last, const _Tp& __value,
-				  std::forward_iterator_tag)
-    {
       while (__first != __last && !(__value < *__first))
 	++__first;
       if (__first != __last)
@@ -530,35 +485,12 @@ 
       return __first == __last;
     }
 
-  // For performance reason, as the iterator range has been validated, check on
-  // random access safe iterators is done using the base iterator.
-  template<typename _Iterator, typename _Sequence, typename _Tp>
-    inline bool
-    __check_partitioned_upper_aux(
-			const _Safe_iterator<_Iterator, _Sequence>& __first,
-			const _Safe_iterator<_Iterator, _Sequence>& __last,
-			const _Tp& __value,
-			std::random_access_iterator_tag __tag)
-    {
-      return __check_partitioned_upper_aux(__first.base(), __last.base(),
-					   __value, __tag);
-    }
-
-  template<typename _ForwardIterator, typename _Tp>
-    inline bool
-    __check_partitioned_upper(_ForwardIterator __first,
-			      _ForwardIterator __last, const _Tp& __value)
-    {
-      return __check_partitioned_upper_aux(__first, __last, __value,
-					   std::__iterator_category(__first));
-    }
-
+  // Determine if a sequence is partitioned w.r.t. this element.
   template<typename _ForwardIterator, typename _Tp, typename _Pred>
     inline bool
-    __check_partitioned_lower_aux(_ForwardIterator __first,
-				  _ForwardIterator __last, const _Tp& __value,
-				  _Pred __pred,
-				  std::forward_iterator_tag)
+    __check_partitioned_lower(_ForwardIterator __first,
+			      _ForwardIterator __last, const _Tp& __value,
+			      _Pred __pred)
     {
       while (__first != __last && bool(__pred(*__first, __value)))
 	++__first;
@@ -571,39 +503,12 @@ 
       return __first == __last;
     }
 
-  // For performance reason, as the iterator range has been validated, check on
-  // random access safe iterators is done using the base iterator.
-  template<typename _Iterator, typename _Sequence,
-	   typename _Tp, typename _Pred>
-    inline bool
-    __check_partitioned_lower_aux(
-			const _Safe_iterator<_Iterator, _Sequence>& __first,
-			const _Safe_iterator<_Iterator, _Sequence>& __last,
-			const _Tp& __value, _Pred __pred,
-			std::random_access_iterator_tag __tag)
-    {
-      return __check_partitioned_lower_aux(__first.base(), __last.base(),
-					   __value, __pred, __tag);
-    }
-
-  // Determine if a sequence is partitioned w.r.t. this element.
   template<typename _ForwardIterator, typename _Tp, typename _Pred>
     inline bool
-    __check_partitioned_lower(_ForwardIterator __first,
+    __check_partitioned_upper(_ForwardIterator __first,
 			      _ForwardIterator __last, const _Tp& __value,
 			      _Pred __pred)
     {
-      return __check_partitioned_lower_aux(__first, __last, __value, __pred,
-					   std::__iterator_category(__first));
-    }
-
-  template<typename _ForwardIterator, typename _Tp, typename _Pred>
-    inline bool
-    __check_partitioned_upper_aux(_ForwardIterator __first,
-				  _ForwardIterator __last, const _Tp& __value,
-				  _Pred __pred,
-				  std::forward_iterator_tag)
-    {
       while (__first != __last && !bool(__pred(__value, *__first)))
 	++__first;
       if (__first != __last)
@@ -615,31 +520,6 @@ 
       return __first == __last;
     }
 
-  // For performance reason, as the iterator range has been validated, check on
-  // random access safe iterators is done using the base iterator.
-  template<typename _Iterator, typename _Sequence,
-	   typename _Tp, typename _Pred>
-    inline bool
-    __check_partitioned_upper_aux(
-			const _Safe_iterator<_Iterator, _Sequence>& __first,
-			const _Safe_iterator<_Iterator, _Sequence>& __last,
-			const _Tp& __value, _Pred __pred,
-			std::random_access_iterator_tag __tag)
-    {
-      return __check_partitioned_upper_aux(__first.base(), __last.base(),
-					   __value, __pred, __tag);
-    }
-
-  template<typename _ForwardIterator, typename _Tp, typename _Pred>
-    inline bool
-    __check_partitioned_upper(_ForwardIterator __first,
-			      _ForwardIterator __last, const _Tp& __value,
-			      _Pred __pred)
-    {
-      return __check_partitioned_upper_aux(__first, __last, __value, __pred,
-					   std::__iterator_category(__first));
-    }
-
   // Helper struct to detect random access safe iterators.
   template<typename _Iterator>
     struct __is_safe_random_iterator
Index: include/debug/macros.h
===================================================================
--- include/debug/macros.h	(revision 201966)
+++ include/debug/macros.h	(working copy)
@@ -229,7 +229,9 @@ 
 // Verify that the iterator range [_First, _Last) is sorted
 #define __glibcxx_check_sorted(_First,_Last)				\
 __glibcxx_check_valid_range(_First,_Last);				\
-_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_sorted(_First, _Last),	\
+ _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_sorted(			\
+			__gnu_debug::__base(_First),			\
+			__gnu_debug::__base(_Last)),			\
 		      _M_message(__gnu_debug::__msg_unsorted)	        \
                       ._M_iterator(_First, #_First)			\
 		      ._M_iterator(_Last, #_Last))
@@ -238,7 +240,9 @@ 
     predicate _Pred. */
 #define __glibcxx_check_sorted_pred(_First,_Last,_Pred)			\
 __glibcxx_check_valid_range(_First,_Last);				\
-_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_sorted(_First, _Last, _Pred), \
+_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_sorted(			\
+			__gnu_debug::__base(_First),			\
+			__gnu_debug::__base(_Last), _Pred),		\
 		      _M_message(__gnu_debug::__msg_unsorted_pred)      \
                       ._M_iterator(_First, #_First)			\
 		      ._M_iterator(_Last, #_Last)			\
@@ -248,7 +252,8 @@ 
 #define __glibcxx_check_sorted_set(_First1,_Last1,_First2)		\
 __glibcxx_check_valid_range(_First1,_Last1);				\
 _GLIBCXX_DEBUG_VERIFY(                                                  \
-  __gnu_debug::__check_sorted_set(_First1, _Last1, _First2),		\
+  __gnu_debug::__check_sorted_set(__gnu_debug::__base(_First1),		\
+				  __gnu_debug::__base(_Last1), _First2),\
   _M_message(__gnu_debug::__msg_unsorted)				\
   ._M_iterator(_First1, #_First1)					\
   ._M_iterator(_Last1, #_Last1))
@@ -257,7 +262,9 @@ 
 #define __glibcxx_check_sorted_set_pred(_First1,_Last1,_First2,_Pred)	\
 __glibcxx_check_valid_range(_First1,_Last1);        			\
 _GLIBCXX_DEBUG_VERIFY(							\
-  __gnu_debug::__check_sorted_set(_First1, _Last1, _First2, _Pred),	\
+  __gnu_debug::__check_sorted_set(__gnu_debug::__base(_First1),		\
+				  __gnu_debug::__base(_Last1),		\
+				  _First2, _Pred),			\
   _M_message(__gnu_debug::__msg_unsorted_pred)				\
   ._M_iterator(_First1, #_First1)					\
   ._M_iterator(_Last1, #_Last1)						\
@@ -267,8 +274,9 @@ 
     w.r.t. the value _Value. */
 #define __glibcxx_check_partitioned_lower(_First,_Last,_Value)		\
 __glibcxx_check_valid_range(_First,_Last);				\
-_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_partitioned_lower(_First, _Last, \
-							    _Value),	\
+_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_partitioned_lower(		\
+			__gnu_debug::__base(_First),			\
+			__gnu_debug::__base(_Last), _Value),		\
 		      _M_message(__gnu_debug::__msg_unpartitioned)      \
 		      ._M_iterator(_First, #_First)			\
 		      ._M_iterator(_Last, #_Last)			\
@@ -276,8 +284,9 @@ 
 
 #define __glibcxx_check_partitioned_upper(_First,_Last,_Value)		\
 __glibcxx_check_valid_range(_First,_Last);				\
-_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_partitioned_upper(_First, _Last, \
-							    _Value),	\
+_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_partitioned_upper(		\
+			__gnu_debug::__base(_First),			\
+			__gnu_debug::__base(_Last), _Value),		\
 		      _M_message(__gnu_debug::__msg_unpartitioned)      \
 		      ._M_iterator(_First, #_First)			\
 		      ._M_iterator(_Last, #_Last)			\
@@ -287,8 +296,9 @@ 
     w.r.t. the value _Value and predicate _Pred. */
 #define __glibcxx_check_partitioned_lower_pred(_First,_Last,_Value,_Pred) \
 __glibcxx_check_valid_range(_First,_Last);				\
-_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_partitioned_lower(_First, _Last, \
-							 _Value, _Pred), \
+_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_partitioned_lower(		\
+			__gnu_debug::__base(_First),			\
+			__gnu_debug::__base(_Last), _Value, _Pred),	\
 		      _M_message(__gnu_debug::__msg_unpartitioned_pred) \
 		      ._M_iterator(_First, #_First)			\
 		      ._M_iterator(_Last, #_Last)			\
@@ -299,8 +309,9 @@ 
     w.r.t. the value _Value and predicate _Pred. */
 #define __glibcxx_check_partitioned_upper_pred(_First,_Last,_Value,_Pred) \
 __glibcxx_check_valid_range(_First,_Last);				\
-_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_partitioned_upper(_First, _Last, \
-							 _Value, _Pred), \
+_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_partitioned_upper(		\
+			__gnu_debug::__base(_First),			\
+			__gnu_debug::__base(_Last), _Value, _Pred),	\
 		      _M_message(__gnu_debug::__msg_unpartitioned_pred) \
 		      ._M_iterator(_First, #_First)			\
 		      ._M_iterator(_Last, #_Last)			\