From: "François Dumont" <frs.dumont@gmail.com>
To: "libstdc++@gcc.gnu.org" <libstdc++@gcc.gnu.org>,
gcc-patches <gcc-patches@gcc.gnu.org>
Subject: Re: [Bug libstdc++/99402] [10/11 Regression] std::copy creates _GLIBCXX_DEBUG false positive for attempt to subscript a dereferenceable (start-of-sequence) iterator
Date: Sun, 7 Mar 2021 22:30:20 +0100 [thread overview]
Message-ID: <bd839e95-e8fc-647c-bf17-7dee1cf3b480@gmail.com> (raw)
In-Reply-To: <bug-99402-19885-WRC4AU1Dpn@http.gcc.gnu.org/bugzilla/>
[-- 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)
next parent reply other threads:[~2021-03-07 21:30 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
[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 ` François Dumont [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=bd839e95-e8fc-647c-bf17-7dee1cf3b480@gmail.com \
--to=frs.dumont@gmail.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=libstdc++@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).