Re: [PATCH] Improvements to valid range checks in debug mode

[Jonathan Wakely <jwakely@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1420 bytes --]

On Wed, 18 Aug 2021 at 15:40, Jonathan Wakely wrote:
>
> On Mon, 16 Aug 2021 at 20:26, Jonathan Wakely via Libstdc++
> <libstdc++@gcc.gnu.org> wrote:
> >
> > On Fri, 13 Aug 2021 at 18:10, François Dumont via Libstdc++
> > <libstdc++@gcc.gnu.org> wrote:
> > >
> > > I just come back to this email and it sounds like the right moment to
> > > commit it.
> >
> > Ah yes, thanks for the reminder!
> >
> > I'll push it tomorrow.
> >
> > >
> > > On 27/02/20 2:06 pm, Jonathan Wakely wrote:
> > > > These should wait for stage 1 but I'm posting them now for comment.
> > > >
> > > > With the change to __gnu_debug::__valid_range we now get a debug
> > > > assertion for:
> > > >
> > > >   std::string s;
> > > >   std::min_element(std::string::iterator{}, s.end());
> > > >
> > > > where previously it would just crash with undefined behaviour.
>
> Actually, that change doesn't work. Some of our container iterators
> use a value-initialized iterator as the past-the-end value, so the
> check in the new __valid_range_aux function incorrectly rejects some
> valid ranges. Maybe I can make it work for bidirectional iterators,
> which must be attached to a container to be valid.

The attached patch is good enough for the original motivation, as shown above.

This makes the debug checks work with basic_string iterators, and for
all other container types we already have safe iterators.

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 1698 bytes --]

commit fb061814aa724727c5761b53e5a60007355da210
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Feb 27 11:20:54 2020

    libstdc++: Detect null __normal_iterator in valid range checks
    
    Enahnce debug mode checks for valid ranges to detect value-initialized
    __normal_iterator objects, so that basic_string iterators can be
    checked.
    
    Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
    
    libstdc++-v3/ChangeLog:
    
            * include/debug/helper_functions.h (__check_singular): Add overload
            for __normal_iterator that checks for value-initialized iterators.

diff --git a/libstdc++-v3/include/debug/helper_functions.h b/libstdc++-v3/include/debug/helper_functions.h
index c0144ced979..e0c69151e3a 100644
--- a/libstdc++-v3/include/debug/helper_functions.h
+++ b/libstdc++-v3/include/debug/helper_functions.h
@@ -36,6 +36,13 @@
 
 #include <bits/stl_pair.h>			// for pair
 
+namespace __gnu_cxx
+{
+_GLIBCXX_BEGIN_NAMESPACE_VERSION
+  template<typename _Iterator, typename _Container> class __normal_iterator;
+_GLIBCXX_END_NAMESPACE_VERSION
+}
+
 namespace __gnu_debug
 {
   template<typename _Iterator, typename _Sequence, typename _Category>
@@ -144,7 +151,13 @@ namespace __gnu_debug
 	__ptr == 0;
     }
 
-  /** We say that integral types for a valid range, and defer to other
+  template<typename _Tp, typename _Cont>
+    _GLIBCXX_CONSTEXPR
+    inline bool
+    __check_singular(__gnu_cxx::__normal_iterator<_Tp*, _Cont> const& __x)
+    { return __x.base() == 0; }
+
+  /** We say that integral types form a valid range, and defer to other
    *  routines to realize what to do with integral types instead of
    *  iterators.
   */