* 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
* 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).