* Re: [Bug libstdc++/99402] [10/11 Regression] std::copy creates _GLIBCXX_DEBUG false positive for attempt to subscript a dereferenceable (start-of-sequence) iterator [not found] ` <bug-99402-19885-WRC4AU1Dpn@http.gcc.gnu.org/bugzilla/> @ 2021-03-07 21:30 ` François Dumont 2021-03-11 17:51 ` François Dumont 0 siblings, 1 reply; 6+ messages in thread From: François Dumont @ 2021-03-07 21:30 UTC (permalink / raw) To: libstdc++, gcc-patches [-- Attachment #1: Type: text/plain, Size: 2440 bytes --] Here is the patch to correctly deal with the new __dp_sign_max_size. I prefer to introduce new __can_advance overloads for this to correctly deal with the _Distance_precision in it. _M_valid_range was also ignoring __dp_sign_max_size. libstdc++: [_GLIBCXX_DEBUG] Fix management of __dp_sign_max_size [PR 99402] libstdc++-v3/ChangeLog: PR libstdc++/99402 * include/debug/helper_functions.h (__can_advance(_InputIterator, const std::pair<_Diff, _Distance_precision>&, int)): New. (__can_advance(const _Safe_iterator<>&, const std::pair<_Diff, _Distance_precision>&, int)): New. * include/debug/macros.h (__glibcxx_check_can_increment_dist): New, use latter. (__glibcxx_check_can_increment_range): Adapt to use latter. (__glibcxx_check_can_decrement_range): Likewise. * include/debug/safe_iterator.h (_Safe_iterator<>::_M_can_advance(const std::pair<_Diff, _Distance_precision>&, int)): New. (__can_advance(const _Safe_iterator<>&, const std::pair<_Diff, _Distance_precision>&, int)): New. * include/debug/safe_iterator.tcc (_Safe_iterator<>::_M_can_advance(const std::pair<_Diff, _Distance_precision>&, int)): New. (_Safe_iterator<>::_M_valid_range(const _Safe_iterator<>&, std::pair<difference_type, _Distance_precision>&, bool)): Adapt for __dp_sign_max_size. (__copy_move_a): Adapt to use __glibcxx_check_can_increment_dist. (__copy_move_backward_a): Likewise. (__equal_aux): Likewise. * include/debug/stl_iterator.h (__can_advance(const std::reverse_iterator<>&, const std::pair<_Diff, _Distance_precision>&, int)): New. (__can_advance(const std::move_iterator<>&, const std::pair<_Diff, _Distance_precision>&, int)): New. * testsuite/25_algorithms/copy/debug/99402.cc: New test. Tested under Linux x86_64. Ok to commit ? François [-- Attachment #2: pr99402.patch --] [-- Type: text/x-patch, Size: 11919 bytes --] diff --git a/libstdc++-v3/include/debug/helper_functions.h b/libstdc++-v3/include/debug/helper_functions.h index 513e5460eba..c0144ced979 100644 --- a/libstdc++-v3/include/debug/helper_functions.h +++ b/libstdc++-v3/include/debug/helper_functions.h @@ -291,6 +291,18 @@ namespace __gnu_debug __can_advance(const _Safe_iterator<_Iterator, _Sequence, _Category>&, _Size); + template<typename _InputIterator, typename _Diff> + _GLIBCXX_CONSTEXPR + inline bool + __can_advance(_InputIterator, const std::pair<_Diff, _Distance_precision>&, int) + { return true; } + + template<typename _Iterator, typename _Sequence, typename _Category, + typename _Diff> + bool + __can_advance(const _Safe_iterator<_Iterator, _Sequence, _Category>&, + const std::pair<_Diff, _Distance_precision>&, int); + /** Helper function to extract base iterator of random access safe iterator * in order to reduce performance impact of debug mode. Limited to random * access iterator because it is the only category for which it is possible diff --git a/libstdc++-v3/include/debug/macros.h b/libstdc++-v3/include/debug/macros.h index 0988437046f..9ac52ebd09d 100644 --- a/libstdc++-v3/include/debug/macros.h +++ b/libstdc++-v3/include/debug/macros.h @@ -94,6 +94,12 @@ _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__can_advance(_First, _Size), \ ._M_iterator(_First, #_First) \ ._M_integer(_Size, #_Size)) +#define __glibcxx_check_can_increment_dist(_First,_Dist,_Way) \ + _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__can_advance(_First, _Dist, _Way), \ + _M_message(__gnu_debug::__msg_iter_subscript_oob) \ + ._M_iterator(_First, #_First) \ + ._M_integer(_Way * _Dist.first, #_Dist)) + #define __glibcxx_check_can_increment_range(_First1,_Last1,_First2) \ do \ { \ @@ -105,7 +111,7 @@ _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__can_advance(_First, _Size), \ ._M_iterator(_Last1, #_Last1), \ __FILE__,__LINE__,__PRETTY_FUNCTION__); \ _GLIBCXX_DEBUG_VERIFY_AT_F( \ - __gnu_debug::__can_advance(_First2, __dist.first),\ + __gnu_debug::__can_advance(_First2, __dist, 1), \ _M_message(__gnu_debug::__msg_iter_subscript_oob)\ ._M_iterator(_First2, #_First2) \ ._M_integer(__dist.first), \ @@ -123,7 +129,7 @@ _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__can_advance(_First, _Size), \ ._M_iterator(_Last1, #_Last1), \ __FILE__,__LINE__,__PRETTY_FUNCTION__); \ _GLIBCXX_DEBUG_VERIFY_AT_F( \ - __gnu_debug::__can_advance(_First2, -__dist.first),\ + __gnu_debug::__can_advance(_First2, __dist, -1), \ _M_message(__gnu_debug::__msg_iter_subscript_oob)\ ._M_iterator(_First2, #_First2) \ ._M_integer(-__dist.first), \ diff --git a/libstdc++-v3/include/debug/safe_iterator.h b/libstdc++-v3/include/debug/safe_iterator.h index a10df190969..8e138fd32e5 100644 --- a/libstdc++-v3/include/debug/safe_iterator.h +++ b/libstdc++-v3/include/debug/safe_iterator.h @@ -407,6 +407,12 @@ namespace __gnu_debug bool _M_can_advance(difference_type __n, bool __strict = false) const; + // Can we advance the iterator using @p __dist in @p __way direction. + template<typename _Diff> + bool + _M_can_advance(const std::pair<_Diff, _Distance_precision>& __dist, + int __way) const; + // Is the iterator range [*this, __rhs) valid? bool _M_valid_range(const _Safe_iterator& __rhs, @@ -958,6 +964,14 @@ namespace __gnu_debug _Size __n) { return __it._M_can_advance(__n); } + template<typename _Iterator, typename _Sequence, typename _Category, + typename _Diff> + inline bool + __can_advance(const _Safe_iterator<_Iterator, _Sequence, _Category>& __it, + const std::pair<_Diff, _Distance_precision>& __dist, + int __way) + { return __it._M_can_advance(__dist, __way); } + template<typename _Iterator, typename _Sequence> _Iterator __base(const _Safe_iterator<_Iterator, _Sequence, diff --git a/libstdc++-v3/include/debug/safe_iterator.tcc b/libstdc++-v3/include/debug/safe_iterator.tcc index 81deb10125b..bc2ef2632d4 100644 --- a/libstdc++-v3/include/debug/safe_iterator.tcc +++ b/libstdc++-v3/include/debug/safe_iterator.tcc @@ -92,22 +92,39 @@ namespace __gnu_debug if (__n == 0) return true; + std::pair<difference_type, _Distance_precision> __dist = __n < 0 + ? _M_get_distance_from_begin() + : _M_get_distance_to_end(); + if (__n < 0) - { - std::pair<difference_type, _Distance_precision> __dist = - _M_get_distance_from_begin(); - return __dist.second == __dp_exact - ? __dist.first >= -__n - : !__strict && __dist.first > 0; - } - else - { - std::pair<difference_type, _Distance_precision> __dist = - _M_get_distance_to_end(); + __n = -__n; + return __dist.second == __dp_exact ? __dist.first >= __n : !__strict && __dist.first > 0; } + + template<typename _Iterator, typename _Sequence, typename _Category> + template<typename _Diff> + bool + _Safe_iterator<_Iterator, _Sequence, _Category>:: + _M_can_advance(const std::pair<_Diff, _Distance_precision>& __dist, + int __way) const + { + if (this->_M_singular()) + return false; + + if (__dist.first == 0) + return true; + + std::pair<difference_type, _Distance_precision> __this_dist = __way < 0 + ? _M_get_distance_from_begin() + : _M_get_distance_to_end(); + + if (__dist.second == __dp_exact && __this_dist.second > __dp_sign) + return __this_dist.first >= __dist.first; + + return __this_dist.first > 0; } template<typename _Iterator, typename _Sequence, typename _Category> @@ -194,20 +211,15 @@ namespace __gnu_debug switch (__dist.second) { case __dp_equality: - if (__dist.first == 0) + // Assume that this is a valid range; we can't check anything else. return true; - break; - case __dp_sign: - case __dp_exact: + default: // If range is not empty first iterator must be dereferenceable. - if (__dist.first > 0) - return !__check_dereferenceable || _M_dereferenceable(); - return __dist.first == 0; + return __dist.first == 0 + || (__dist.first > 0 + && (!__check_dereferenceable || _M_dereferenceable())); } - - // Assume that this is a valid range; we can't check anything else. - return true; } template<typename _Iterator, typename _Sequence> @@ -225,9 +237,8 @@ namespace __gnu_debug __dist = std::make_pair(__rhs.base() - this->base(), __dp_exact); // If range is not empty first iterator must be dereferenceable. - if (__dist.first > 0) - return this->_M_dereferenceable(); - return __dist.first == 0; + return __dist.first == 0 + || (__dist.first > 0 && this->_M_dereferenceable()); } } // namespace __gnu_debug @@ -251,7 +262,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { typename ::__gnu_debug::_Distance_traits<_Ite>::__type __dist; __glibcxx_check_valid_range2(__first, __last, __dist); - __glibcxx_check_can_increment(__result, __dist.first); + __glibcxx_check_can_increment_dist(__result, __dist, 1); if (__dist.second > ::__gnu_debug::__dp_equality) return std::__copy_move_a<_IsMove>(__first.base(), __last.base(), @@ -268,7 +279,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { typename ::__gnu_debug::_Distance_traits<_II>::__type __dist; __glibcxx_check_valid_range2(__first, __last, __dist); - __glibcxx_check_can_increment(__result, __dist.first); + __glibcxx_check_can_increment_dist(__result, __dist, 1); if (__dist.second > ::__gnu_debug::__dp_sign && __result._M_can_advance(__dist.first, true)) @@ -290,7 +301,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { typename ::__gnu_debug::_Distance_traits<_IIte>::__type __dist; __glibcxx_check_valid_range2(__first, __last, __dist); - __glibcxx_check_can_increment(__result, __dist.first); + __glibcxx_check_can_increment_dist(__result, __dist, 1); if (__dist.second > ::__gnu_debug::__dp_equality) { @@ -318,7 +329,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { typename ::__gnu_debug::_Distance_traits<_Ite>::__type __dist; __glibcxx_check_valid_range2(__first, __last, __dist); - __glibcxx_check_can_increment(__result, -__dist.first); + __glibcxx_check_can_increment_dist(__result, __dist, -1); if (__dist.second > ::__gnu_debug::__dp_equality) return std::__copy_move_backward_a<_IsMove>( @@ -335,7 +346,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { typename ::__gnu_debug::_Distance_traits<_II>::__type __dist; __glibcxx_check_valid_range2(__first, __last, __dist); - __glibcxx_check_can_increment(__result, -__dist.first); + __glibcxx_check_can_increment_dist(__result, __dist, -1); if (__dist.second > ::__gnu_debug::__dp_sign && __result._M_can_advance(-__dist.first, true)) @@ -358,7 +369,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { typename ::__gnu_debug::_Distance_traits<_IIte>::__type __dist; __glibcxx_check_valid_range2(__first, __last, __dist); - __glibcxx_check_can_increment(__result, -__dist.first); + __glibcxx_check_can_increment_dist(__result, __dist, -1); if (__dist.second > ::__gnu_debug::__dp_equality) { @@ -423,7 +434,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { typename ::__gnu_debug::_Distance_traits<_II1>::__type __dist; __glibcxx_check_valid_range2(__first1, __last1, __dist); - __glibcxx_check_can_increment(__first2, __dist.first); + __glibcxx_check_can_increment_dist(__first2, __dist, 1); if (__dist.second > ::__gnu_debug::__dp_equality) return std::__equal_aux(__first1.base(), __last1.base(), __first2); @@ -438,7 +449,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { typename ::__gnu_debug::_Distance_traits<_II1>::__type __dist; __glibcxx_check_valid_range2(__first1, __last1, __dist); - __glibcxx_check_can_increment(__first2, __dist.first); + __glibcxx_check_can_increment_dist(__first2, __dist, 1); if (__dist.second > ::__gnu_debug::__dp_sign && __first2._M_can_advance(__dist.first, true)) @@ -457,7 +468,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { typename ::__gnu_debug::_Distance_traits<_II1>::__type __dist; __glibcxx_check_valid_range2(__first1, __last1, __dist); - __glibcxx_check_can_increment(__first2, __dist.first); + __glibcxx_check_can_increment_dist(__first2, __dist, 1); if (__dist.second > ::__gnu_debug::__dp_equality) { diff --git a/libstdc++-v3/include/debug/stl_iterator.h b/libstdc++-v3/include/debug/stl_iterator.h index a9dd5b6c08d..edeb42ebe98 100644 --- a/libstdc++-v3/include/debug/stl_iterator.h +++ b/libstdc++-v3/include/debug/stl_iterator.h @@ -52,6 +52,13 @@ namespace __gnu_debug __can_advance(const std::reverse_iterator<_Iterator>& __it, _Size __n) { return __can_advance(__it.base(), -__n); } + template<typename _Iterator, typename _Diff> + inline bool + __can_advance(const std::reverse_iterator<_Iterator>& __it, + const std::pair<_Diff, _Distance_precision>& __dist, + int __way) + { return __can_advance(__it.base(), __dist, -__way); } + template<typename _Iterator, typename _Sequence> inline std::reverse_iterator<_Iterator> __base(const std::reverse_iterator<_Safe_iterator< @@ -101,6 +108,13 @@ namespace __gnu_debug __can_advance(const std::move_iterator<_Iterator>& __it, _Size __n) { return __can_advance(__it.base(), __n); } + template<typename _Iterator, typename _Diff> + inline bool + __can_advance(const std::move_iterator<_Iterator>& __it, + const std::pair<_Diff, _Distance_precision>& __dist, + int __way) + { return __can_advance(__it.base(), __dist, __way); } + template<typename _Iterator> inline auto __unsafe(const std::move_iterator<_Iterator>& __it) ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Bug libstdc++/99402] [10/11 Regression] std::copy creates _GLIBCXX_DEBUG false positive for attempt to subscript a dereferenceable (start-of-sequence) iterator 2021-03-07 21:30 ` [Bug libstdc++/99402] [10/11 Regression] std::copy creates _GLIBCXX_DEBUG false positive for attempt to subscript a dereferenceable (start-of-sequence) iterator François Dumont @ 2021-03-11 17:51 ` François Dumont 2021-04-08 13:07 ` Jonathan Wakely 0 siblings, 1 reply; 6+ messages in thread From: François Dumont @ 2021-03-11 17:51 UTC (permalink / raw) To: libstdc++, gcc-patches [-- Attachment #1: Type: text/plain, Size: 5605 bytes --] I eventually prefer to propose this version. Compared to the previous one I have the _M_can_advance calling the former one with correct number of elements to check for advance. And the former _M_can_advance is also properly making use of the __dp_sign_max_size precision. Here is the revisited git log: libstdc++: [_GLIBCXX_DEBUG] Fix management of __dp_sign_max_size [PR 99402] __dp_sign precision indicates that we found out what iterator comes first or last in the range. __dp_sign_max_size is the same plus it gives the information of the max size of the range that is to say the max_size value such that distance(lhs, rhs) < max_size. Thanks to this additional information we are able to tell when a copy of n elements to that range will fail even if we do not know exactly how large it is. This patch makes sure that we are properly using this information. libstdc++-v3/ChangeLog: PR libstdc++/99402 * include/debug/helper_functions.h (__can_advance(_InputIterator, const std::pair<_Diff, _Distance_precision>&, int)): New. (__can_advance(const _Safe_iterator<>&, const std::pair<_Diff, _Distance_precision>&, int)): New. * include/debug/macros.h (__glibcxx_check_can_increment_dist): New, use latter. (__glibcxx_check_can_increment_range): Adapt to use latter. (__glibcxx_check_can_decrement_range): Likewise. * include/debug/safe_iterator.h (_Safe_iterator<>::_M_can_advance(const std::pair<_Diff, _Distance_precision>&, int)): New. (__can_advance(const _Safe_iterator<>&, const std::pair<_Diff, _Distance_precision>&, int)): New. * include/debug/safe_iterator.tcc (_Safe_iterator<>::_M_can_advance(const std::pair<_Diff, _Distance_precision>&, int)): New. (_Safe_iterator<>::_M_valid_range(const _Safe_iterator<>&, std::pair<difference_type, _Distance_precision>&, bool)): Adapt for __dp_sign_max_size. (__copy_move_a): Adapt to use __glibcxx_check_can_increment_dist. (__copy_move_backward_a): Likewise. (__equal_aux): Likewise. * include/debug/stl_iterator.h (__can_advance(const std::reverse_iterator<>&, const std::pair<_Diff, _Distance_precision>&, int)): New. (__can_advance(const std::move_iterator<>&, const std::pair<_Diff, _Distance_precision>&, int)): New. * testsuite/25_algorithms/copy/debug/99402.cc: New test. Ok to commit if tests are all PASS ? François On 07/03/21 10:30 pm, François Dumont wrote: > Here is the patch to correctly deal with the new __dp_sign_max_size. > > I prefer to introduce new __can_advance overloads for this to > correctly deal with the _Distance_precision in it. _M_valid_range was > also ignoring __dp_sign_max_size. > > libstdc++: [_GLIBCXX_DEBUG] Fix management of __dp_sign_max_size > [PR 99402] > > libstdc++-v3/ChangeLog: > > PR libstdc++/99402 > * include/debug/helper_functions.h > (__can_advance(_InputIterator, > const std::pair<_Diff, _Distance_precision>&, int)): New. > (__can_advance(const _Safe_iterator<>&, > const std::pair<_Diff, _Distance_precision>&, int)): New. > * include/debug/macros.h > (__glibcxx_check_can_increment_dist): New, > use latter. > (__glibcxx_check_can_increment_range): Adapt to use latter. > (__glibcxx_check_can_decrement_range): Likewise. > * include/debug/safe_iterator.h > (_Safe_iterator<>::_M_can_advance(const std::pair<_Diff, > _Distance_precision>&, > int)): New. > (__can_advance(const _Safe_iterator<>&, > const std::pair<_Diff, _Distance_precision>&, int)): New. > * include/debug/safe_iterator.tcc > (_Safe_iterator<>::_M_can_advance(const std::pair<_Diff, > _Distance_precision>&, > int)): New. > (_Safe_iterator<>::_M_valid_range(const _Safe_iterator<>&, > std::pair<difference_type, _Distance_precision>&, bool)): > Adapt for > __dp_sign_max_size. > (__copy_move_a): Adapt to use > __glibcxx_check_can_increment_dist. > (__copy_move_backward_a): Likewise. > (__equal_aux): Likewise. > * include/debug/stl_iterator.h (__can_advance(const > std::reverse_iterator<>&, > const std::pair<_Diff, _Distance_precision>&, int)): New. > (__can_advance(const std::move_iterator<>&, > const std::pair<_Diff, _Distance_precision>&, int)): New. > * testsuite/25_algorithms/copy/debug/99402.cc: New test. > > Tested under Linux x86_64. > > Ok to commit ? > > François > [-- Attachment #2: pr99402.patch --] [-- Type: text/x-patch, Size: 13154 bytes --] diff --git a/libstdc++-v3/include/debug/helper_functions.h b/libstdc++-v3/include/debug/helper_functions.h index 513e5460eba..c0144ced979 100644 --- a/libstdc++-v3/include/debug/helper_functions.h +++ b/libstdc++-v3/include/debug/helper_functions.h @@ -291,6 +291,18 @@ namespace __gnu_debug __can_advance(const _Safe_iterator<_Iterator, _Sequence, _Category>&, _Size); + template<typename _InputIterator, typename _Diff> + _GLIBCXX_CONSTEXPR + inline bool + __can_advance(_InputIterator, const std::pair<_Diff, _Distance_precision>&, int) + { return true; } + + template<typename _Iterator, typename _Sequence, typename _Category, + typename _Diff> + bool + __can_advance(const _Safe_iterator<_Iterator, _Sequence, _Category>&, + const std::pair<_Diff, _Distance_precision>&, int); + /** Helper function to extract base iterator of random access safe iterator * in order to reduce performance impact of debug mode. Limited to random * access iterator because it is the only category for which it is possible diff --git a/libstdc++-v3/include/debug/macros.h b/libstdc++-v3/include/debug/macros.h index 0988437046f..9ac52ebd09d 100644 --- a/libstdc++-v3/include/debug/macros.h +++ b/libstdc++-v3/include/debug/macros.h @@ -94,6 +94,12 @@ _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__can_advance(_First, _Size), \ ._M_iterator(_First, #_First) \ ._M_integer(_Size, #_Size)) +#define __glibcxx_check_can_increment_dist(_First,_Dist,_Way) \ + _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__can_advance(_First, _Dist, _Way), \ + _M_message(__gnu_debug::__msg_iter_subscript_oob) \ + ._M_iterator(_First, #_First) \ + ._M_integer(_Way * _Dist.first, #_Dist)) + #define __glibcxx_check_can_increment_range(_First1,_Last1,_First2) \ do \ { \ @@ -105,7 +111,7 @@ _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__can_advance(_First, _Size), \ ._M_iterator(_Last1, #_Last1), \ __FILE__,__LINE__,__PRETTY_FUNCTION__); \ _GLIBCXX_DEBUG_VERIFY_AT_F( \ - __gnu_debug::__can_advance(_First2, __dist.first),\ + __gnu_debug::__can_advance(_First2, __dist, 1), \ _M_message(__gnu_debug::__msg_iter_subscript_oob)\ ._M_iterator(_First2, #_First2) \ ._M_integer(__dist.first), \ @@ -123,7 +129,7 @@ _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__can_advance(_First, _Size), \ ._M_iterator(_Last1, #_Last1), \ __FILE__,__LINE__,__PRETTY_FUNCTION__); \ _GLIBCXX_DEBUG_VERIFY_AT_F( \ - __gnu_debug::__can_advance(_First2, -__dist.first),\ + __gnu_debug::__can_advance(_First2, __dist, -1), \ _M_message(__gnu_debug::__msg_iter_subscript_oob)\ ._M_iterator(_First2, #_First2) \ ._M_integer(-__dist.first), \ diff --git a/libstdc++-v3/include/debug/safe_iterator.h b/libstdc++-v3/include/debug/safe_iterator.h index a10df190969..8e138fd32e5 100644 --- a/libstdc++-v3/include/debug/safe_iterator.h +++ b/libstdc++-v3/include/debug/safe_iterator.h @@ -407,6 +407,12 @@ namespace __gnu_debug bool _M_can_advance(difference_type __n, bool __strict = false) const; + // Can we advance the iterator using @p __dist in @p __way direction. + template<typename _Diff> + bool + _M_can_advance(const std::pair<_Diff, _Distance_precision>& __dist, + int __way) const; + // Is the iterator range [*this, __rhs) valid? bool _M_valid_range(const _Safe_iterator& __rhs, @@ -958,6 +964,14 @@ namespace __gnu_debug _Size __n) { return __it._M_can_advance(__n); } + template<typename _Iterator, typename _Sequence, typename _Category, + typename _Diff> + inline bool + __can_advance(const _Safe_iterator<_Iterator, _Sequence, _Category>& __it, + const std::pair<_Diff, _Distance_precision>& __dist, + int __way) + { return __it._M_can_advance(__dist, __way); } + template<typename _Iterator, typename _Sequence> _Iterator __base(const _Safe_iterator<_Iterator, _Sequence, diff --git a/libstdc++-v3/include/debug/safe_iterator.tcc b/libstdc++-v3/include/debug/safe_iterator.tcc index 81deb10125b..6f14c40bf41 100644 --- a/libstdc++-v3/include/debug/safe_iterator.tcc +++ b/libstdc++-v3/include/debug/safe_iterator.tcc @@ -92,22 +92,30 @@ namespace __gnu_debug if (__n == 0) return true; + std::pair<difference_type, _Distance_precision> __dist = __n < 0 + ? _M_get_distance_from_begin() + : _M_get_distance_to_end(); + if (__n < 0) - { - std::pair<difference_type, _Distance_precision> __dist = - _M_get_distance_from_begin(); - return __dist.second == __dp_exact - ? __dist.first >= -__n + __n = -__n; + + return __dist.second > __dp_sign + ? __dist.first >= __n : !__strict && __dist.first > 0; } - else + + template<typename _Iterator, typename _Sequence, typename _Category> + template<typename _Diff> + bool + _Safe_iterator<_Iterator, _Sequence, _Category>:: + _M_can_advance(const std::pair<_Diff, _Distance_precision>& __dist, + int __way) const { - std::pair<difference_type, _Distance_precision> __dist = - _M_get_distance_to_end(); return __dist.second == __dp_exact - ? __dist.first >= __n - : !__strict && __dist.first > 0; - } + ? _M_can_advance(__way * __dist.first) + : _M_can_advance(__way * (__dist.first == 0 + ? 0 + : __dist.first < 0 ? -1 : 1)); } template<typename _Iterator, typename _Sequence, typename _Category> @@ -194,20 +202,15 @@ namespace __gnu_debug switch (__dist.second) { case __dp_equality: - if (__dist.first == 0) + // Assume that this is a valid range; we can't check anything else. return true; - break; - case __dp_sign: - case __dp_exact: + default: // If range is not empty first iterator must be dereferenceable. - if (__dist.first > 0) - return !__check_dereferenceable || _M_dereferenceable(); - return __dist.first == 0; + return __dist.first == 0 + || (__dist.first > 0 + && (!__check_dereferenceable || _M_dereferenceable())); } - - // Assume that this is a valid range; we can't check anything else. - return true; } template<typename _Iterator, typename _Sequence> @@ -225,9 +228,8 @@ namespace __gnu_debug __dist = std::make_pair(__rhs.base() - this->base(), __dp_exact); // If range is not empty first iterator must be dereferenceable. - if (__dist.first > 0) - return this->_M_dereferenceable(); - return __dist.first == 0; + return __dist.first == 0 + || (__dist.first > 0 && this->_M_dereferenceable()); } } // namespace __gnu_debug @@ -251,7 +253,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { typename ::__gnu_debug::_Distance_traits<_Ite>::__type __dist; __glibcxx_check_valid_range2(__first, __last, __dist); - __glibcxx_check_can_increment(__result, __dist.first); + __glibcxx_check_can_increment_dist(__result, __dist, 1); if (__dist.second > ::__gnu_debug::__dp_equality) return std::__copy_move_a<_IsMove>(__first.base(), __last.base(), @@ -268,7 +270,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { typename ::__gnu_debug::_Distance_traits<_II>::__type __dist; __glibcxx_check_valid_range2(__first, __last, __dist); - __glibcxx_check_can_increment(__result, __dist.first); + __glibcxx_check_can_increment_dist(__result, __dist, 1); if (__dist.second > ::__gnu_debug::__dp_sign && __result._M_can_advance(__dist.first, true)) @@ -290,7 +292,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { typename ::__gnu_debug::_Distance_traits<_IIte>::__type __dist; __glibcxx_check_valid_range2(__first, __last, __dist); - __glibcxx_check_can_increment(__result, __dist.first); + __glibcxx_check_can_increment_dist(__result, __dist, 1); if (__dist.second > ::__gnu_debug::__dp_equality) { @@ -318,7 +320,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { typename ::__gnu_debug::_Distance_traits<_Ite>::__type __dist; __glibcxx_check_valid_range2(__first, __last, __dist); - __glibcxx_check_can_increment(__result, -__dist.first); + __glibcxx_check_can_increment_dist(__result, __dist, -1); if (__dist.second > ::__gnu_debug::__dp_equality) return std::__copy_move_backward_a<_IsMove>( @@ -335,7 +337,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { typename ::__gnu_debug::_Distance_traits<_II>::__type __dist; __glibcxx_check_valid_range2(__first, __last, __dist); - __glibcxx_check_can_increment(__result, -__dist.first); + __glibcxx_check_can_increment_dist(__result, __dist, -1); if (__dist.second > ::__gnu_debug::__dp_sign && __result._M_can_advance(-__dist.first, true)) @@ -358,7 +360,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { typename ::__gnu_debug::_Distance_traits<_IIte>::__type __dist; __glibcxx_check_valid_range2(__first, __last, __dist); - __glibcxx_check_can_increment(__result, -__dist.first); + __glibcxx_check_can_increment_dist(__result, __dist, -1); if (__dist.second > ::__gnu_debug::__dp_equality) { @@ -423,7 +425,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { typename ::__gnu_debug::_Distance_traits<_II1>::__type __dist; __glibcxx_check_valid_range2(__first1, __last1, __dist); - __glibcxx_check_can_increment(__first2, __dist.first); + __glibcxx_check_can_increment_dist(__first2, __dist, 1); if (__dist.second > ::__gnu_debug::__dp_equality) return std::__equal_aux(__first1.base(), __last1.base(), __first2); @@ -438,7 +440,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { typename ::__gnu_debug::_Distance_traits<_II1>::__type __dist; __glibcxx_check_valid_range2(__first1, __last1, __dist); - __glibcxx_check_can_increment(__first2, __dist.first); + __glibcxx_check_can_increment_dist(__first2, __dist, 1); if (__dist.second > ::__gnu_debug::__dp_sign && __first2._M_can_advance(__dist.first, true)) @@ -457,7 +459,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { typename ::__gnu_debug::_Distance_traits<_II1>::__type __dist; __glibcxx_check_valid_range2(__first1, __last1, __dist); - __glibcxx_check_can_increment(__first2, __dist.first); + __glibcxx_check_can_increment_dist(__first2, __dist, 1); if (__dist.second > ::__gnu_debug::__dp_equality) { diff --git a/libstdc++-v3/include/debug/stl_iterator.h b/libstdc++-v3/include/debug/stl_iterator.h index a9dd5b6c08d..edeb42ebe98 100644 --- a/libstdc++-v3/include/debug/stl_iterator.h +++ b/libstdc++-v3/include/debug/stl_iterator.h @@ -52,6 +52,13 @@ namespace __gnu_debug __can_advance(const std::reverse_iterator<_Iterator>& __it, _Size __n) { return __can_advance(__it.base(), -__n); } + template<typename _Iterator, typename _Diff> + inline bool + __can_advance(const std::reverse_iterator<_Iterator>& __it, + const std::pair<_Diff, _Distance_precision>& __dist, + int __way) + { return __can_advance(__it.base(), __dist, -__way); } + template<typename _Iterator, typename _Sequence> inline std::reverse_iterator<_Iterator> __base(const std::reverse_iterator<_Safe_iterator< @@ -101,6 +108,13 @@ namespace __gnu_debug __can_advance(const std::move_iterator<_Iterator>& __it, _Size __n) { return __can_advance(__it.base(), __n); } + template<typename _Iterator, typename _Diff> + inline bool + __can_advance(const std::move_iterator<_Iterator>& __it, + const std::pair<_Diff, _Distance_precision>& __dist, + int __way) + { return __can_advance(__it.base(), __dist, __way); } + template<typename _Iterator> inline auto __unsafe(const std::move_iterator<_Iterator>& __it) diff --git a/libstdc++-v3/testsuite/25_algorithms/copy/debug/99402.cc b/libstdc++-v3/testsuite/25_algorithms/copy/debug/99402.cc new file mode 100644 index 00000000000..041d222d079 --- /dev/null +++ b/libstdc++-v3/testsuite/25_algorithms/copy/debug/99402.cc @@ -0,0 +1,35 @@ +// Copyright (C) 2021 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_DEBUG" } +// { dg-do run } + +#include <algorithm> +#include <set> +#include <vector> + +// PR libstdc++/99402 + +using namespace std; + +int main() +{ + // any container with non-random access iterators: + const set<int> source = { 0, 1 }; + vector<int> dest(1); + copy(source.begin(), ++source.begin(), dest.begin()); +} ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Bug libstdc++/99402] [10/11 Regression] std::copy creates _GLIBCXX_DEBUG false positive for attempt to subscript a dereferenceable (start-of-sequence) iterator 2021-03-11 17:51 ` François Dumont @ 2021-04-08 13:07 ` Jonathan Wakely 2021-04-09 20:02 ` François Dumont 0 siblings, 1 reply; 6+ messages in thread From: Jonathan Wakely @ 2021-04-08 13:07 UTC (permalink / raw) To: François Dumont; +Cc: libstdc++, gcc-patches On 11/03/21 18:51 +0100, François Dumont via Libstdc++ wrote: >I eventually prefer to propose this version. > >Compared to the previous one I have the _M_can_advance calling the >former one with correct number of elements to check for advance. And >the former _M_can_advance is also properly making use of the >__dp_sign_max_size precision. > >Here is the revisited git log: > > libstdc++: [_GLIBCXX_DEBUG] Fix management of __dp_sign_max_size >[PR 99402] > > __dp_sign precision indicates that we found out what iterator >comes first or > last in the range. __dp_sign_max_size is the same plus it gives >the information > of the max size of the range that is to say the max_size value >such that > distance(lhs, rhs) < max_size. > Thanks to this additional information we are able to tell when a >copy of n elements > to that range will fail even if we do not know exactly how large it is. > > This patch makes sure that we are properly using this information. > > libstdc++-v3/ChangeLog: > > PR libstdc++/99402 > * include/debug/helper_functions.h >(__can_advance(_InputIterator, > const std::pair<_Diff, _Distance_precision>&, int)): New. > (__can_advance(const _Safe_iterator<>&, > const std::pair<_Diff, _Distance_precision>&, int)): New. > * include/debug/macros.h >(__glibcxx_check_can_increment_dist): New, > use latter. > (__glibcxx_check_can_increment_range): Adapt to use latter. > (__glibcxx_check_can_decrement_range): Likewise. > * include/debug/safe_iterator.h > (_Safe_iterator<>::_M_can_advance(const std::pair<_Diff, >_Distance_precision>&, > int)): New. > (__can_advance(const _Safe_iterator<>&, > const std::pair<_Diff, _Distance_precision>&, int)): New. > * include/debug/safe_iterator.tcc > (_Safe_iterator<>::_M_can_advance(const std::pair<_Diff, >_Distance_precision>&, > int)): New. > (_Safe_iterator<>::_M_valid_range(const _Safe_iterator<>&, > std::pair<difference_type, _Distance_precision>&, bool)): >Adapt for > __dp_sign_max_size. > (__copy_move_a): Adapt to use >__glibcxx_check_can_increment_dist. > (__copy_move_backward_a): Likewise. > (__equal_aux): Likewise. > * include/debug/stl_iterator.h (__can_advance(const >std::reverse_iterator<>&, > const std::pair<_Diff, _Distance_precision>&, int)): New. > (__can_advance(const std::move_iterator<>&, > const std::pair<_Diff, _Distance_precision>&, int)): New. > * testsuite/25_algorithms/copy/debug/99402.cc: New test. > >Ok to commit if tests are all PASS ? > >François > >On 07/03/21 10:30 pm, François Dumont wrote: >>Here is the patch to correctly deal with the new __dp_sign_max_size. >> >>I prefer to introduce new __can_advance overloads for this to >>correctly deal with the _Distance_precision in it. _M_valid_range >>was also ignoring __dp_sign_max_size. >> >> libstdc++: [_GLIBCXX_DEBUG] Fix management of __dp_sign_max_size >>[PR 99402] >> >> libstdc++-v3/ChangeLog: >> >> PR libstdc++/99402 >> * include/debug/helper_functions.h >>(__can_advance(_InputIterator, >> const std::pair<_Diff, _Distance_precision>&, int)): New. >> (__can_advance(const _Safe_iterator<>&, >> const std::pair<_Diff, _Distance_precision>&, int)): New. >> * include/debug/macros.h >>(__glibcxx_check_can_increment_dist): New, >> use latter. >> (__glibcxx_check_can_increment_range): Adapt to use latter. >> (__glibcxx_check_can_decrement_range): Likewise. >> * include/debug/safe_iterator.h >> (_Safe_iterator<>::_M_can_advance(const std::pair<_Diff, >>_Distance_precision>&, >> int)): New. >> (__can_advance(const _Safe_iterator<>&, >> const std::pair<_Diff, _Distance_precision>&, int)): New. >> * include/debug/safe_iterator.tcc >> (_Safe_iterator<>::_M_can_advance(const std::pair<_Diff, >>_Distance_precision>&, >> int)): New. >> (_Safe_iterator<>::_M_valid_range(const _Safe_iterator<>&, >> std::pair<difference_type, _Distance_precision>&, >>bool)): Adapt for >> __dp_sign_max_size. >> (__copy_move_a): Adapt to use >>__glibcxx_check_can_increment_dist. >> (__copy_move_backward_a): Likewise. >> (__equal_aux): Likewise. >> * include/debug/stl_iterator.h (__can_advance(const >>std::reverse_iterator<>&, >> const std::pair<_Diff, _Distance_precision>&, int)): New. >> (__can_advance(const std::move_iterator<>&, >> const std::pair<_Diff, _Distance_precision>&, int)): New. >> * testsuite/25_algorithms/copy/debug/99402.cc: New test. >> >>Tested under Linux x86_64. >> >>Ok to commit ? >> >>François >> > >diff --git a/libstdc++-v3/include/debug/helper_functions.h b/libstdc++-v3/include/debug/helper_functions.h >index 513e5460eba..c0144ced979 100644 >--- a/libstdc++-v3/include/debug/helper_functions.h >+++ b/libstdc++-v3/include/debug/helper_functions.h >@@ -291,6 +291,18 @@ namespace __gnu_debug > __can_advance(const _Safe_iterator<_Iterator, _Sequence, _Category>&, > _Size); > >+ template<typename _InputIterator, typename _Diff> >+ _GLIBCXX_CONSTEXPR >+ inline bool >+ __can_advance(_InputIterator, const std::pair<_Diff, _Distance_precision>&, int) >+ { return true; } >+ >+ template<typename _Iterator, typename _Sequence, typename _Category, >+ typename _Diff> >+ bool >+ __can_advance(const _Safe_iterator<_Iterator, _Sequence, _Category>&, >+ const std::pair<_Diff, _Distance_precision>&, int); >+ > /** Helper function to extract base iterator of random access safe iterator > * in order to reduce performance impact of debug mode. Limited to random > * access iterator because it is the only category for which it is possible >diff --git a/libstdc++-v3/include/debug/macros.h b/libstdc++-v3/include/debug/macros.h >index 0988437046f..9ac52ebd09d 100644 >--- a/libstdc++-v3/include/debug/macros.h >+++ b/libstdc++-v3/include/debug/macros.h >@@ -94,6 +94,12 @@ _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__can_advance(_First, _Size), \ > ._M_iterator(_First, #_First) \ > ._M_integer(_Size, #_Size)) > >+#define __glibcxx_check_can_increment_dist(_First,_Dist,_Way) \ >+ _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__can_advance(_First, _Dist, _Way), \ >+ _M_message(__gnu_debug::__msg_iter_subscript_oob) \ >+ ._M_iterator(_First, #_First) \ >+ ._M_integer(_Way * _Dist.first, #_Dist)) >+ > #define __glibcxx_check_can_increment_range(_First1,_Last1,_First2) \ > do \ > { \ >@@ -105,7 +111,7 @@ _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__can_advance(_First, _Size), \ > ._M_iterator(_Last1, #_Last1), \ > __FILE__,__LINE__,__PRETTY_FUNCTION__); \ > _GLIBCXX_DEBUG_VERIFY_AT_F( \ >- __gnu_debug::__can_advance(_First2, __dist.first),\ >+ __gnu_debug::__can_advance(_First2, __dist, 1), \ > _M_message(__gnu_debug::__msg_iter_subscript_oob)\ > ._M_iterator(_First2, #_First2) \ > ._M_integer(__dist.first), \ >@@ -123,7 +129,7 @@ _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__can_advance(_First, _Size), \ > ._M_iterator(_Last1, #_Last1), \ > __FILE__,__LINE__,__PRETTY_FUNCTION__); \ > _GLIBCXX_DEBUG_VERIFY_AT_F( \ >- __gnu_debug::__can_advance(_First2, -__dist.first),\ >+ __gnu_debug::__can_advance(_First2, __dist, -1), \ > _M_message(__gnu_debug::__msg_iter_subscript_oob)\ > ._M_iterator(_First2, #_First2) \ > ._M_integer(-__dist.first), \ >diff --git a/libstdc++-v3/include/debug/safe_iterator.h b/libstdc++-v3/include/debug/safe_iterator.h >index a10df190969..8e138fd32e5 100644 >--- a/libstdc++-v3/include/debug/safe_iterator.h >+++ b/libstdc++-v3/include/debug/safe_iterator.h >@@ -407,6 +407,12 @@ namespace __gnu_debug > bool > _M_can_advance(difference_type __n, bool __strict = false) const; > >+ // Can we advance the iterator using @p __dist in @p __way direction. >+ template<typename _Diff> >+ bool >+ _M_can_advance(const std::pair<_Diff, _Distance_precision>& __dist, >+ int __way) const; >+ > // Is the iterator range [*this, __rhs) valid? > bool > _M_valid_range(const _Safe_iterator& __rhs, >@@ -958,6 +964,14 @@ namespace __gnu_debug > _Size __n) > { return __it._M_can_advance(__n); } > >+ template<typename _Iterator, typename _Sequence, typename _Category, >+ typename _Diff> >+ inline bool >+ __can_advance(const _Safe_iterator<_Iterator, _Sequence, _Category>& __it, >+ const std::pair<_Diff, _Distance_precision>& __dist, >+ int __way) >+ { return __it._M_can_advance(__dist, __way); } >+ > template<typename _Iterator, typename _Sequence> > _Iterator > __base(const _Safe_iterator<_Iterator, _Sequence, >diff --git a/libstdc++-v3/include/debug/safe_iterator.tcc b/libstdc++-v3/include/debug/safe_iterator.tcc >index 81deb10125b..6f14c40bf41 100644 >--- a/libstdc++-v3/include/debug/safe_iterator.tcc >+++ b/libstdc++-v3/include/debug/safe_iterator.tcc >@@ -92,22 +92,30 @@ namespace __gnu_debug > if (__n == 0) > return true; > >+ std::pair<difference_type, _Distance_precision> __dist = __n < 0 >+ ? _M_get_distance_from_begin() >+ : _M_get_distance_to_end(); >+ > if (__n < 0) >- { >- std::pair<difference_type, _Distance_precision> __dist = >- _M_get_distance_from_begin(); >- return __dist.second == __dp_exact >- ? __dist.first >= -__n >+ __n = -__n; >+ >+ return __dist.second > __dp_sign >+ ? __dist.first >= __n > : !__strict && __dist.first > 0; > } >- else >+ >+ template<typename _Iterator, typename _Sequence, typename _Category> >+ template<typename _Diff> >+ bool >+ _Safe_iterator<_Iterator, _Sequence, _Category>:: >+ _M_can_advance(const std::pair<_Diff, _Distance_precision>& __dist, >+ int __way) const > { >- std::pair<difference_type, _Distance_precision> __dist = >- _M_get_distance_to_end(); > return __dist.second == __dp_exact >- ? __dist.first >= __n >- : !__strict && __dist.first > 0; >- } >+ ? _M_can_advance(__way * __dist.first) >+ : _M_can_advance(__way * (__dist.first == 0 >+ ? 0 >+ : __dist.first < 0 ? -1 : 1)); > } > > template<typename _Iterator, typename _Sequence, typename _Category> >@@ -194,20 +202,15 @@ namespace __gnu_debug > switch (__dist.second) > { > case __dp_equality: >- if (__dist.first == 0) >+ // Assume that this is a valid range; we can't check anything else. > return true; >- break; > >- case __dp_sign: >- case __dp_exact: >+ default: > // If range is not empty first iterator must be dereferenceable. >- if (__dist.first > 0) >- return !__check_dereferenceable || _M_dereferenceable(); >- return __dist.first == 0; >+ return __dist.first == 0 >+ || (__dist.first > 0 >+ && (!__check_dereferenceable || _M_dereferenceable())); > } This doesn't really make sense to use a switch now, it could be simply: if (__dist.second != __dp_equality) { // If range is not empty first iterator must be dereferenceable. return __dist.first == 0 || (__dist.first > 0 && (!__check_dereferenceable || _M_dereferenceable())); } // Assume that this is a valid range; we can't check anything else. return true; I don't understand any of this code, but it's currently broken so OK for trunk. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Bug libstdc++/99402] [10/11 Regression] std::copy creates _GLIBCXX_DEBUG false positive for attempt to subscript a dereferenceable (start-of-sequence) iterator 2021-04-08 13:07 ` Jonathan Wakely @ 2021-04-09 20:02 ` François Dumont 0 siblings, 0 replies; 6+ messages in thread From: François Dumont @ 2021-04-09 20:02 UTC (permalink / raw) To: Jonathan Wakely; +Cc: libstdc++, gcc-patches On 08/04/21 3:07 pm, Jonathan Wakely wrote: > On 11/03/21 18:51 +0100, François Dumont via Libstdc++ wrote: >> I eventually prefer to propose this version. >> >> Compared to the previous one I have the _M_can_advance calling the >> former one with correct number of elements to check for advance. And >> the former _M_can_advance is also properly making use of the >> __dp_sign_max_size precision. >> >> Here is the revisited git log: >> >> libstdc++: [_GLIBCXX_DEBUG] Fix management of __dp_sign_max_size >> [PR 99402] >> >> __dp_sign precision indicates that we found out what iterator >> comes first or >> last in the range. __dp_sign_max_size is the same plus it gives >> the information >> of the max size of the range that is to say the max_size value >> such that >> distance(lhs, rhs) < max_size. >> Thanks to this additional information we are able to tell when a >> copy of n elements >> to that range will fail even if we do not know exactly how large >> it is. >> >> This patch makes sure that we are properly using this information. >> >> libstdc++-v3/ChangeLog: >> >> PR libstdc++/99402 >> * include/debug/helper_functions.h >> (__can_advance(_InputIterator, >> const std::pair<_Diff, _Distance_precision>&, int)): New. >> (__can_advance(const _Safe_iterator<>&, >> const std::pair<_Diff, _Distance_precision>&, int)): New. >> * include/debug/macros.h >> (__glibcxx_check_can_increment_dist): New, >> use latter. >> (__glibcxx_check_can_increment_range): Adapt to use latter. >> (__glibcxx_check_can_decrement_range): Likewise. >> * include/debug/safe_iterator.h >> (_Safe_iterator<>::_M_can_advance(const std::pair<_Diff, >> _Distance_precision>&, >> int)): New. >> (__can_advance(const _Safe_iterator<>&, >> const std::pair<_Diff, _Distance_precision>&, int)): New. >> * include/debug/safe_iterator.tcc >> (_Safe_iterator<>::_M_can_advance(const std::pair<_Diff, >> _Distance_precision>&, >> int)): New. >> (_Safe_iterator<>::_M_valid_range(const _Safe_iterator<>&, >> std::pair<difference_type, _Distance_precision>&, bool)): >> Adapt for >> __dp_sign_max_size. >> (__copy_move_a): Adapt to use >> __glibcxx_check_can_increment_dist. >> (__copy_move_backward_a): Likewise. >> (__equal_aux): Likewise. >> * include/debug/stl_iterator.h (__can_advance(const >> std::reverse_iterator<>&, >> const std::pair<_Diff, _Distance_precision>&, int)): New. >> (__can_advance(const std::move_iterator<>&, >> const std::pair<_Diff, _Distance_precision>&, int)): New. >> * testsuite/25_algorithms/copy/debug/99402.cc: New test. >> >> Ok to commit if tests are all PASS ? >> >> François >> >> On 07/03/21 10:30 pm, François Dumont wrote: >>> Here is the patch to correctly deal with the new __dp_sign_max_size. >>> >>> I prefer to introduce new __can_advance overloads for this to >>> correctly deal with the _Distance_precision in it. _M_valid_range >>> was also ignoring __dp_sign_max_size. >>> >>> libstdc++: [_GLIBCXX_DEBUG] Fix management of __dp_sign_max_size >>> [PR 99402] >>> >>> libstdc++-v3/ChangeLog: >>> >>> PR libstdc++/99402 >>> * include/debug/helper_functions.h >>> (__can_advance(_InputIterator, >>> const std::pair<_Diff, _Distance_precision>&, int)): New. >>> (__can_advance(const _Safe_iterator<>&, >>> const std::pair<_Diff, _Distance_precision>&, int)): New. >>> * include/debug/macros.h >>> (__glibcxx_check_can_increment_dist): New, >>> use latter. >>> (__glibcxx_check_can_increment_range): Adapt to use latter. >>> (__glibcxx_check_can_decrement_range): Likewise. >>> * include/debug/safe_iterator.h >>> (_Safe_iterator<>::_M_can_advance(const std::pair<_Diff, >>> _Distance_precision>&, >>> int)): New. >>> (__can_advance(const _Safe_iterator<>&, >>> const std::pair<_Diff, _Distance_precision>&, int)): New. >>> * include/debug/safe_iterator.tcc >>> (_Safe_iterator<>::_M_can_advance(const std::pair<_Diff, >>> _Distance_precision>&, >>> int)): New. >>> (_Safe_iterator<>::_M_valid_range(const _Safe_iterator<>&, >>> std::pair<difference_type, _Distance_precision>&, >>> bool)): Adapt for >>> __dp_sign_max_size. >>> (__copy_move_a): Adapt to use >>> __glibcxx_check_can_increment_dist. >>> (__copy_move_backward_a): Likewise. >>> (__equal_aux): Likewise. >>> * include/debug/stl_iterator.h (__can_advance(const >>> std::reverse_iterator<>&, >>> const std::pair<_Diff, _Distance_precision>&, int)): New. >>> (__can_advance(const std::move_iterator<>&, >>> const std::pair<_Diff, _Distance_precision>&, int)): New. >>> * testsuite/25_algorithms/copy/debug/99402.cc: New test. >>> >>> Tested under Linux x86_64. >>> >>> Ok to commit ? >>> >>> François >>> >> > >> diff --git a/libstdc++-v3/include/debug/helper_functions.h >> b/libstdc++-v3/include/debug/helper_functions.h >> index 513e5460eba..c0144ced979 100644 >> --- a/libstdc++-v3/include/debug/helper_functions.h >> +++ b/libstdc++-v3/include/debug/helper_functions.h >> @@ -291,6 +291,18 @@ namespace __gnu_debug >> __can_advance(const _Safe_iterator<_Iterator, _Sequence, >> _Category>&, >> _Size); >> >> + template<typename _InputIterator, typename _Diff> >> + _GLIBCXX_CONSTEXPR >> + inline bool >> + __can_advance(_InputIterator, const std::pair<_Diff, >> _Distance_precision>&, int) >> + { return true; } >> + >> + template<typename _Iterator, typename _Sequence, typename _Category, >> + typename _Diff> >> + bool >> + __can_advance(const _Safe_iterator<_Iterator, _Sequence, >> _Category>&, >> + const std::pair<_Diff, _Distance_precision>&, int); >> + >> /** Helper function to extract base iterator of random access safe >> iterator >> * in order to reduce performance impact of debug mode. Limited to >> random >> * access iterator because it is the only category for which it is >> possible >> diff --git a/libstdc++-v3/include/debug/macros.h >> b/libstdc++-v3/include/debug/macros.h >> index 0988437046f..9ac52ebd09d 100644 >> --- a/libstdc++-v3/include/debug/macros.h >> +++ b/libstdc++-v3/include/debug/macros.h >> @@ -94,6 +94,12 @@ >> _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__can_advance(_First, _Size), \ >> ._M_iterator(_First, #_First) \ >> ._M_integer(_Size, #_Size)) >> >> +#define __glibcxx_check_can_increment_dist(_First,_Dist,_Way) \ >> + _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__can_advance(_First, _Dist, >> _Way), \ >> + _M_message(__gnu_debug::__msg_iter_subscript_oob) \ >> + ._M_iterator(_First, #_First) \ >> + ._M_integer(_Way * _Dist.first, #_Dist)) >> + >> #define __glibcxx_check_can_increment_range(_First1,_Last1,_First2) \ >> do \ >> { \ >> @@ -105,7 +111,7 @@ >> _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__can_advance(_First, _Size), \ >> ._M_iterator(_Last1, #_Last1), \ >> __FILE__,__LINE__,__PRETTY_FUNCTION__); \ >> _GLIBCXX_DEBUG_VERIFY_AT_F( \ >> - __gnu_debug::__can_advance(_First2, __dist.first),\ >> + __gnu_debug::__can_advance(_First2, __dist, 1), \ >> _M_message(__gnu_debug::__msg_iter_subscript_oob)\ >> ._M_iterator(_First2, #_First2) \ >> ._M_integer(__dist.first), \ >> @@ -123,7 +129,7 @@ >> _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__can_advance(_First, _Size), \ >> ._M_iterator(_Last1, #_Last1), \ >> __FILE__,__LINE__,__PRETTY_FUNCTION__); \ >> _GLIBCXX_DEBUG_VERIFY_AT_F( \ >> - __gnu_debug::__can_advance(_First2, -__dist.first),\ >> + __gnu_debug::__can_advance(_First2, __dist, -1), \ >> _M_message(__gnu_debug::__msg_iter_subscript_oob)\ >> ._M_iterator(_First2, #_First2) \ >> ._M_integer(-__dist.first), \ >> diff --git a/libstdc++-v3/include/debug/safe_iterator.h >> b/libstdc++-v3/include/debug/safe_iterator.h >> index a10df190969..8e138fd32e5 100644 >> --- a/libstdc++-v3/include/debug/safe_iterator.h >> +++ b/libstdc++-v3/include/debug/safe_iterator.h >> @@ -407,6 +407,12 @@ namespace __gnu_debug >> bool >> _M_can_advance(difference_type __n, bool __strict = false) const; >> >> + // Can we advance the iterator using @p __dist in @p __way >> direction. >> + template<typename _Diff> >> + bool >> + _M_can_advance(const std::pair<_Diff, _Distance_precision>& __dist, >> + int __way) const; >> + >> // Is the iterator range [*this, __rhs) valid? >> bool >> _M_valid_range(const _Safe_iterator& __rhs, >> @@ -958,6 +964,14 @@ namespace __gnu_debug >> _Size __n) >> { return __it._M_can_advance(__n); } >> >> + template<typename _Iterator, typename _Sequence, typename _Category, >> + typename _Diff> >> + inline bool >> + __can_advance(const _Safe_iterator<_Iterator, _Sequence, >> _Category>& __it, >> + const std::pair<_Diff, _Distance_precision>& __dist, >> + int __way) >> + { return __it._M_can_advance(__dist, __way); } >> + >> template<typename _Iterator, typename _Sequence> >> _Iterator >> __base(const _Safe_iterator<_Iterator, _Sequence, >> diff --git a/libstdc++-v3/include/debug/safe_iterator.tcc >> b/libstdc++-v3/include/debug/safe_iterator.tcc >> index 81deb10125b..6f14c40bf41 100644 >> --- a/libstdc++-v3/include/debug/safe_iterator.tcc >> +++ b/libstdc++-v3/include/debug/safe_iterator.tcc >> @@ -92,22 +92,30 @@ namespace __gnu_debug >> if (__n == 0) >> return true; >> >> + std::pair<difference_type, _Distance_precision> __dist = __n < 0 >> + ? _M_get_distance_from_begin() >> + : _M_get_distance_to_end(); >> + >> if (__n < 0) >> - { >> - std::pair<difference_type, _Distance_precision> __dist = >> - _M_get_distance_from_begin(); >> - return __dist.second == __dp_exact >> - ? __dist.first >= -__n >> + __n = -__n; >> + >> + return __dist.second > __dp_sign >> + ? __dist.first >= __n >> : !__strict && __dist.first > 0; >> } >> - else >> + >> + template<typename _Iterator, typename _Sequence, typename _Category> >> + template<typename _Diff> >> + bool >> + _Safe_iterator<_Iterator, _Sequence, _Category>:: >> + _M_can_advance(const std::pair<_Diff, _Distance_precision>& >> __dist, >> + int __way) const >> { >> - std::pair<difference_type, _Distance_precision> __dist = >> - _M_get_distance_to_end(); >> return __dist.second == __dp_exact >> - ? __dist.first >= __n >> - : !__strict && __dist.first > 0; >> - } >> + ? _M_can_advance(__way * __dist.first) >> + : _M_can_advance(__way * (__dist.first == 0 >> + ? 0 >> + : __dist.first < 0 ? -1 : 1)); >> } >> >> template<typename _Iterator, typename _Sequence, typename _Category> >> @@ -194,20 +202,15 @@ namespace __gnu_debug >> switch (__dist.second) >> { >> case __dp_equality: >> - if (__dist.first == 0) >> + // Assume that this is a valid range; we can't check anything >> else. >> return true; >> - break; >> >> - case __dp_sign: >> - case __dp_exact: >> + default: >> // If range is not empty first iterator must be dereferenceable. >> - if (__dist.first > 0) >> - return !__check_dereferenceable || _M_dereferenceable(); >> - return __dist.first == 0; >> + return __dist.first == 0 >> + || (__dist.first > 0 >> + && (!__check_dereferenceable || _M_dereferenceable())); >> } > > This doesn't really make sense to use a switch now, it could be > simply: > > if (__dist.second != __dp_equality) > { > // If range is not empty first iterator must be dereferenceable. > return __dist.first == 0 > || (__dist.first > 0 > && (!__check_dereferenceable || _M_dereferenceable())); > } > > // Assume that this is a valid range; we can't check anything else. > return true; > Yes, I missed that. I tested and committed with this simplification. > > I don't understand any of this code, but it's currently broken so OK > for trunk. > Maybe there will be something to do to clarify then. ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <bug-99402-19885-PxIZ3sL1KH@http.gcc.gnu.org/bugzilla/>]
* Re: [Bug libstdc++/99402] [10 Regression] std::copy creates _GLIBCXX_DEBUG false positive for attempt to subscript a dereferenceable (start-of-sequence) iterator [not found] ` <bug-99402-19885-PxIZ3sL1KH@http.gcc.gnu.org/bugzilla/> @ 2021-04-18 13:28 ` François Dumont 2021-04-18 14:04 ` Jonathan Wakely 0 siblings, 1 reply; 6+ messages in thread From: François Dumont @ 2021-04-18 13:28 UTC (permalink / raw) To: libstdc++, gcc-patches [-- Attachment #1: Type: text/plain, Size: 3288 bytes --] Hi Ok to backport this to gcc-10 branch ? Tested under Linux x86_64. François On 13/04/21 10:51 pm, redi at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99402 > > Jonathan Wakely <redi at gcc dot gnu.org> changed: > > What |Removed |Added > ---------------------------------------------------------------------------- > Summary|[10/11 Regression] |[10 Regression] std::copy > |std::copy creates |creates _GLIBCXX_DEBUG > |_GLIBCXX_DEBUG false |false positive for attempt > |positive for attempt to |to subscript a > |subscript a dereferenceable |dereferenceable > |(start-of-sequence) |(start-of-sequence) > |iterator |iterator > > --- Comment #10 from Jonathan Wakely <redi at gcc dot gnu.org> --- > This was fixed on trunk by r11-8100: > > libstdc++: [_GLIBCXX_DEBUG] Fix management of __dp_sign_max_size [PR 99402] > > __dp_sign precision indicates that we found out what iterator comes first or > last in the range. __dp_sign_max_size is the same plus it gives the information > of the max size of the range that is to say the max_size value such that > distance(lhs, rhs) < max_size. > Thanks to this additional information we are able to tell when a copy of n > elements > to that range will fail even if we do not know exactly how large it is. > > This patch makes sure that we are properly using this information. > > libstdc++-v3/ChangeLog: > > PR libstdc++/99402 > * include/debug/helper_functions.h (__can_advance(_InputIterator, > const std::pair<_Diff, _Distance_precision>&, int)): New. > (__can_advance(const _Safe_iterator<>&, > const std::pair<_Diff, _Distance_precision>&, int)): New. > * include/debug/macros.h (__glibcxx_check_can_increment_dist): New, > use latter. > (__glibcxx_check_can_increment_range): Adapt to use latter. > (__glibcxx_check_can_decrement_range): Likewise. > * include/debug/safe_iterator.h > (_Safe_iterator<>::_M_can_advance(const std::pair<_Diff, > _Distance_precision>&, > int)): New. > (__can_advance(const _Safe_iterator<>&, > const std::pair<_Diff, _Distance_precision>&, int)): New. > * include/debug/safe_iterator.tcc > (_Safe_iterator<>::_M_can_advance(const std::pair<_Diff, > _Distance_precision>&, > int)): New. > (_Safe_iterator<>::_M_valid_range(const _Safe_iterator<>&, > std::pair<difference_type, _Distance_precision>&, bool)): Adapt for > __dp_sign_max_size. > (__copy_move_a): Adapt to use __glibcxx_check_can_increment_dist. > (__copy_move_backward_a): Likewise. > (__equal_aux): Likewise. > * include/debug/stl_iterator.h (__can_advance(const > std::reverse_iterator<>&, > const std::pair<_Diff, _Distance_precision>&, int)): New. > (__can_advance(const std::move_iterator<>&, > const std::pair<_Diff, _Distance_precision>&, int)): New. > * testsuite/25_algorithms/copy/debug/99402.cc: New test. > [-- Attachment #2: pr99402.patch --] [-- Type: text/x-patch, Size: 13645 bytes --] diff --git a/libstdc++-v3/include/debug/helper_functions.h b/libstdc++-v3/include/debug/helper_functions.h index 62d5309257f..9e2d206f06c 100644 --- a/libstdc++-v3/include/debug/helper_functions.h +++ b/libstdc++-v3/include/debug/helper_functions.h @@ -64,7 +64,7 @@ namespace __gnu_debug { private: typedef - typename std::iterator_traits<_Iterator>::difference_type _ItDiffType; + typename std::iterator_traits<_Iterator>::difference_type _ItDiffType; template<typename _DiffType, typename = typename std::__is_void<_DiffType>::__type> @@ -287,6 +287,18 @@ namespace __gnu_debug __can_advance(const _Safe_iterator<_Iterator, _Sequence, _Category>&, _Size); + template<typename _InputIterator, typename _Diff> + _GLIBCXX_CONSTEXPR + inline bool + __can_advance(_InputIterator, const std::pair<_Diff, _Distance_precision>&, int) + { return true; } + + template<typename _Iterator, typename _Sequence, typename _Category, + typename _Diff> + bool + __can_advance(const _Safe_iterator<_Iterator, _Sequence, _Category>&, + const std::pair<_Diff, _Distance_precision>&, int); + /** Helper function to extract base iterator of random access safe iterator * in order to reduce performance impact of debug mode. Limited to random * access iterator because it is the only category for which it is possible diff --git a/libstdc++-v3/include/debug/macros.h b/libstdc++-v3/include/debug/macros.h index 73fb50d0cbd..1f45f8e0adc 100644 --- a/libstdc++-v3/include/debug/macros.h +++ b/libstdc++-v3/include/debug/macros.h @@ -104,6 +104,12 @@ _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__can_advance(_First, _Size), \ ._M_iterator(_First, #_First) \ ._M_integer(_Size, #_Size)) +#define __glibcxx_check_can_increment_dist(_First,_Dist,_Way) \ + _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__can_advance(_First, _Dist, _Way), \ + _M_message(__gnu_debug::__msg_iter_subscript_oob) \ + ._M_iterator(_First, #_First) \ + ._M_integer(_Way * _Dist.first, #_Dist)) + #define __glibcxx_check_can_increment_range(_First1,_Last1,_First2) \ do \ { \ @@ -115,7 +121,7 @@ _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__can_advance(_First, _Size), \ ._M_iterator(_Last1, #_Last1), \ __FILE__,__LINE__,__PRETTY_FUNCTION__); \ _GLIBCXX_DEBUG_VERIFY_COND_AT( \ - __gnu_debug::__can_advance(_First2, __dist.first),\ + __gnu_debug::__can_advance(_First2, __dist, 1), \ _M_message(__gnu_debug::__msg_iter_subscript_oob)\ ._M_iterator(_First2, #_First2) \ ._M_integer(__dist.first), \ @@ -133,7 +139,7 @@ _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__can_advance(_First, _Size), \ ._M_iterator(_Last1, #_Last1), \ __FILE__,__LINE__,__PRETTY_FUNCTION__); \ _GLIBCXX_DEBUG_VERIFY_COND_AT( \ - __gnu_debug::__can_advance(_First2, -__dist.first),\ + __gnu_debug::__can_advance(_First2, __dist, -1), \ _M_message(__gnu_debug::__msg_iter_subscript_oob)\ ._M_iterator(_First2, #_First2) \ ._M_integer(-__dist.first), \ diff --git a/libstdc++-v3/include/debug/safe_iterator.h b/libstdc++-v3/include/debug/safe_iterator.h index 687b844fd75..2f51618dd88 100644 --- a/libstdc++-v3/include/debug/safe_iterator.h +++ b/libstdc++-v3/include/debug/safe_iterator.h @@ -405,6 +405,12 @@ namespace __gnu_debug bool _M_can_advance(difference_type __n, bool __strict = false) const; + // Can we advance the iterator using @p __dist in @p __way direction. + template<typename _Diff> + bool + _M_can_advance(const std::pair<_Diff, _Distance_precision>& __dist, + int __way) const; + // Is the iterator range [*this, __rhs) valid? bool _M_valid_range(const _Safe_iterator& __rhs, @@ -956,6 +962,14 @@ namespace __gnu_debug _Size __n) { return __it._M_can_advance(__n); } + template<typename _Iterator, typename _Sequence, typename _Category, + typename _Diff> + inline bool + __can_advance(const _Safe_iterator<_Iterator, _Sequence, _Category>& __it, + const std::pair<_Diff, _Distance_precision>& __dist, + int __way) + { return __it._M_can_advance(__dist, __way); } + template<typename _Iterator, typename _Sequence> _Iterator __base(const _Safe_iterator<_Iterator, _Sequence, diff --git a/libstdc++-v3/include/debug/safe_iterator.tcc b/libstdc++-v3/include/debug/safe_iterator.tcc index 312a88911ee..79a8ee28d97 100644 --- a/libstdc++-v3/include/debug/safe_iterator.tcc +++ b/libstdc++-v3/include/debug/safe_iterator.tcc @@ -92,24 +92,32 @@ namespace __gnu_debug if (__n == 0) return true; + std::pair<difference_type, _Distance_precision> __dist = __n < 0 + ? _M_get_distance_from_begin() + : _M_get_distance_to_end(); + if (__n < 0) - { - std::pair<difference_type, _Distance_precision> __dist = - _M_get_distance_from_begin(); - return __dist.second == __dp_exact - ? __dist.first >= -__n - : !__strict && __dist.first > 0; - } - else - { - std::pair<difference_type, _Distance_precision> __dist = - _M_get_distance_to_end(); - return __dist.second == __dp_exact - ? __dist.first >= __n - : !__strict && __dist.first > 0; - } + __n = -__n; + + return __dist.second > __dp_sign + ? __dist.first >= __n + : !__strict && __dist.first > 0; } + template<typename _Iterator, typename _Sequence, typename _Category> + template<typename _Diff> + bool + _Safe_iterator<_Iterator, _Sequence, _Category>:: + _M_can_advance(const std::pair<_Diff, _Distance_precision>& __dist, + int __way) const + { + return __dist.second == __dp_exact + ? _M_can_advance(__way * __dist.first) + : _M_can_advance(__way * (__dist.first == 0 + ? 0 + : __dist.first < 0 ? -1 : 1)); + } + template<typename _Iterator, typename _Sequence, typename _Category> typename _Distance_traits<_Iterator>::__type _Safe_iterator<_Iterator, _Sequence, _Category>:: @@ -191,19 +199,12 @@ namespace __gnu_debug /* Determine iterators order */ __dist = _M_get_distance_to(__rhs); - switch (__dist.second) + if (__dist.second != __dp_equality) { - case __dp_equality: - if (__dist.first == 0) - return true; - break; - - case __dp_sign: - case __dp_exact: // If range is not empty first iterator must be dereferenceable. - if (__dist.first > 0) - return !__check_dereferenceable || _M_dereferenceable(); - return __dist.first == 0; + return __dist.first == 0 + || (__dist.first > 0 + && (!__check_dereferenceable || _M_dereferenceable())); } // Assume that this is a valid range; we can't check anything else. @@ -224,9 +225,8 @@ namespace __gnu_debug __dist = std::make_pair(__rhs.base() - this->base(), __dp_exact); // If range is not empty first iterator must be dereferenceable. - if (__dist.first > 0) - return this->_M_dereferenceable(); - return __dist.first == 0; + return __dist.first == 0 + || (__dist.first > 0 && this->_M_dereferenceable()); } } // namespace __gnu_debug @@ -244,7 +244,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { typename ::__gnu_debug::_Distance_traits<_Ite>::__type __dist; __glibcxx_check_valid_range2(__first, __last, __dist); - __glibcxx_check_can_increment(__result, __dist.first); + __glibcxx_check_can_increment_dist(__result, __dist, 1); if (__dist.second > ::__gnu_debug::__dp_equality) return std::__copy_move_a<_IsMove>(__first.base(), __last.base(), @@ -261,7 +261,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { typename ::__gnu_debug::_Distance_traits<_II>::__type __dist; __glibcxx_check_valid_range2(__first, __last, __dist); - __glibcxx_check_can_increment(__result, __dist.first); + __glibcxx_check_can_increment_dist(__result, __dist, 1); if (__dist.second > ::__gnu_debug::__dp_sign && __result._M_can_advance(__dist.first, true)) @@ -283,7 +283,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { typename ::__gnu_debug::_Distance_traits<_IIte>::__type __dist; __glibcxx_check_valid_range2(__first, __last, __dist); - __glibcxx_check_can_increment(__result, __dist.first); + __glibcxx_check_can_increment_dist(__result, __dist, 1); if (__dist.second > ::__gnu_debug::__dp_equality) { @@ -311,7 +311,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { typename ::__gnu_debug::_Distance_traits<_Ite>::__type __dist; __glibcxx_check_valid_range2(__first, __last, __dist); - __glibcxx_check_can_increment(__result, -__dist.first); + __glibcxx_check_can_increment_dist(__result, __dist, -1); if (__dist.second > ::__gnu_debug::__dp_equality) return std::__copy_move_backward_a<_IsMove>( @@ -328,7 +328,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { typename ::__gnu_debug::_Distance_traits<_II>::__type __dist; __glibcxx_check_valid_range2(__first, __last, __dist); - __glibcxx_check_can_increment(__result, -__dist.first); + __glibcxx_check_can_increment_dist(__result, __dist, -1); if (__dist.second > ::__gnu_debug::__dp_sign && __result._M_can_advance(-__dist.first, true)) @@ -351,7 +351,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { typename ::__gnu_debug::_Distance_traits<_IIte>::__type __dist; __glibcxx_check_valid_range2(__first, __last, __dist); - __glibcxx_check_can_increment(__result, -__dist.first); + __glibcxx_check_can_increment_dist(__result, __dist, -1); if (__dist.second > ::__gnu_debug::__dp_equality) { @@ -416,7 +416,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { typename ::__gnu_debug::_Distance_traits<_II1>::__type __dist; __glibcxx_check_valid_range2(__first1, __last1, __dist); - __glibcxx_check_can_increment(__first2, __dist.first); + __glibcxx_check_can_increment_dist(__first2, __dist, 1); if (__dist.second > ::__gnu_debug::__dp_equality) return std::__equal_aux(__first1.base(), __last1.base(), __first2); @@ -431,7 +431,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { typename ::__gnu_debug::_Distance_traits<_II1>::__type __dist; __glibcxx_check_valid_range2(__first1, __last1, __dist); - __glibcxx_check_can_increment(__first2, __dist.first); + __glibcxx_check_can_increment_dist(__first2, __dist, 1); if (__dist.second > ::__gnu_debug::__dp_sign && __first2._M_can_advance(__dist.first, true)) @@ -450,7 +450,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { typename ::__gnu_debug::_Distance_traits<_II1>::__type __dist; __glibcxx_check_valid_range2(__first1, __last1, __dist); - __glibcxx_check_can_increment(__first2, __dist.first); + __glibcxx_check_can_increment_dist(__first2, __dist, 1); if (__dist.second > ::__gnu_debug::__dp_equality) { diff --git a/libstdc++-v3/include/debug/stl_iterator.h b/libstdc++-v3/include/debug/stl_iterator.h index 6044f3588d9..02adb6e3c06 100644 --- a/libstdc++-v3/include/debug/stl_iterator.h +++ b/libstdc++-v3/include/debug/stl_iterator.h @@ -52,6 +52,13 @@ namespace __gnu_debug __can_advance(const std::reverse_iterator<_Iterator>& __it, _Size __n) { return __can_advance(__it.base(), -__n); } + template<typename _Iterator, typename _Diff> + inline bool + __can_advance(const std::reverse_iterator<_Iterator>& __it, + const std::pair<_Diff, _Distance_precision>& __dist, + int __way) + { return __can_advance(__it.base(), __dist, -__way); } + template<typename _Iterator, typename _Sequence> inline std::reverse_iterator<_Iterator> __base(const std::reverse_iterator<_Safe_iterator< @@ -101,6 +108,13 @@ namespace __gnu_debug __can_advance(const std::move_iterator<_Iterator>& __it, _Size __n) { return __can_advance(__it.base(), __n); } + template<typename _Iterator, typename _Diff> + inline bool + __can_advance(const std::move_iterator<_Iterator>& __it, + const std::pair<_Diff, _Distance_precision>& __dist, + int __way) + { return __can_advance(__it.base(), __dist, __way); } + template<typename _Iterator> inline auto __unsafe(const std::move_iterator<_Iterator>& __it) diff --git a/libstdc++-v3/testsuite/25_algorithms/copy/debug/99402.cc b/libstdc++-v3/testsuite/25_algorithms/copy/debug/99402.cc new file mode 100644 index 00000000000..041d222d079 --- /dev/null +++ b/libstdc++-v3/testsuite/25_algorithms/copy/debug/99402.cc @@ -0,0 +1,35 @@ +// Copyright (C) 2021 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_DEBUG" } +// { dg-do run } + +#include <algorithm> +#include <set> +#include <vector> + +// PR libstdc++/99402 + +using namespace std; + +int main() +{ + // any container with non-random access iterators: + const set<int> source = { 0, 1 }; + vector<int> dest(1); + copy(source.begin(), ++source.begin(), dest.begin()); +} ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Bug libstdc++/99402] [10 Regression] std::copy creates _GLIBCXX_DEBUG false positive for attempt to subscript a dereferenceable (start-of-sequence) iterator 2021-04-18 13:28 ` [Bug libstdc++/99402] [10 " François Dumont @ 2021-04-18 14:04 ` Jonathan Wakely 0 siblings, 0 replies; 6+ messages in thread From: Jonathan Wakely @ 2021-04-18 14:04 UTC (permalink / raw) To: François Dumont; +Cc: libstdc++, gcc-patches On Sun, 18 Apr 2021, 15:01 François Dumont via Libstdc++, < libstdc++@gcc.gnu.org> wrote: > Hi > > Ok to backport this to gcc-10 branch ? > Yes please, thanks. > Tested under Linux x86_64. > > François > > > On 13/04/21 10:51 pm, redi at gcc dot gnu.org wrote: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99402 > > > > Jonathan Wakely <redi at gcc dot gnu.org> changed: > > > > What |Removed |Added > > > ---------------------------------------------------------------------------- > > Summary|[10/11 Regression] |[10 Regression] > std::copy > > |std::copy creates |creates _GLIBCXX_DEBUG > > |_GLIBCXX_DEBUG false |false positive for > attempt > > |positive for attempt to |to subscript a > > |subscript a dereferenceable |dereferenceable > > |(start-of-sequence) |(start-of-sequence) > > |iterator |iterator > > > > --- Comment #10 from Jonathan Wakely <redi at gcc dot gnu.org> --- > > This was fixed on trunk by r11-8100: > > > > libstdc++: [_GLIBCXX_DEBUG] Fix management of __dp_sign_max_size [PR > 99402] > > > > __dp_sign precision indicates that we found out what iterator comes > first or > > last in the range. __dp_sign_max_size is the same plus it gives the > information > > of the max size of the range that is to say the max_size value such that > > distance(lhs, rhs) < max_size. > > Thanks to this additional information we are able to tell when a copy of > n > > elements > > to that range will fail even if we do not know exactly how large it is. > > > > This patch makes sure that we are properly using this information. > > > > libstdc++-v3/ChangeLog: > > > > PR libstdc++/99402 > > * include/debug/helper_functions.h > (__can_advance(_InputIterator, > > const std::pair<_Diff, _Distance_precision>&, int)): New. > > (__can_advance(const _Safe_iterator<>&, > > const std::pair<_Diff, _Distance_precision>&, int)): New. > > * include/debug/macros.h (__glibcxx_check_can_increment_dist): > New, > > use latter. > > (__glibcxx_check_can_increment_range): Adapt to use latter. > > (__glibcxx_check_can_decrement_range): Likewise. > > * include/debug/safe_iterator.h > > (_Safe_iterator<>::_M_can_advance(const std::pair<_Diff, > > _Distance_precision>&, > > int)): New. > > (__can_advance(const _Safe_iterator<>&, > > const std::pair<_Diff, _Distance_precision>&, int)): New. > > * include/debug/safe_iterator.tcc > > (_Safe_iterator<>::_M_can_advance(const std::pair<_Diff, > > _Distance_precision>&, > > int)): New. > > (_Safe_iterator<>::_M_valid_range(const _Safe_iterator<>&, > > std::pair<difference_type, _Distance_precision>&, bool)): Adapt > for > > __dp_sign_max_size. > > (__copy_move_a): Adapt to use > __glibcxx_check_can_increment_dist. > > (__copy_move_backward_a): Likewise. > > (__equal_aux): Likewise. > > * include/debug/stl_iterator.h (__can_advance(const > > std::reverse_iterator<>&, > > const std::pair<_Diff, _Distance_precision>&, int)): New. > > (__can_advance(const std::move_iterator<>&, > > const std::pair<_Diff, _Distance_precision>&, int)): New. > > * testsuite/25_algorithms/copy/debug/99402.cc: New test. > > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-04-18 14:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <bug-99402-19885@http.gcc.gnu.org/bugzilla/>
[not found] ` <bug-99402-19885-WRC4AU1Dpn@http.gcc.gnu.org/bugzilla/>
2021-03-07 21:30 ` [Bug libstdc++/99402] [10/11 Regression] std::copy creates _GLIBCXX_DEBUG false positive for attempt to subscript a dereferenceable (start-of-sequence) iterator François Dumont
2021-03-11 17:51 ` François Dumont
2021-04-08 13:07 ` Jonathan Wakely
2021-04-09 20:02 ` François Dumont
[not found] ` <bug-99402-19885-PxIZ3sL1KH@http.gcc.gnu.org/bugzilla/>
2021-04-18 13:28 ` [Bug libstdc++/99402] [10 " François Dumont
2021-04-18 14:04 ` Jonathan Wakely
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).