[PING][PATCH 2/4 v3] gdb/progspace: Add reverse safe iterator

[Aaron Merey <amerey@redhat.com>]

Ping

Thanks,
Aaron

On Fri, Jan 19, 2024 at 12:09 AM Aaron Merey <amerey@redhat.com> wrote:
>
> Hi Andrew,
>
> Thanks for the review.
>
> On Thu, Jan 18, 2024 at 7:03 AM Andrew Burgess <aburgess@redhat.com> wrote:
> >
> > Aaron Merey <amerey@redhat.com> writes:
> > > +using qf_list = std::forward_list<quick_symbol_functions_up>;
> > > +using qf_range = iterator_range<qf_list::iterator>;
> > > +using qf_safe_range = basic_safe_range<qf_range>;
> > > +
> > >  /* Sections in an objfile.  The section offsets are stored in the
> > >     OBJFILE.  */
> > >
> > > @@ -764,6 +770,10 @@ struct objfile
> > >       reader.  */
> > >    std::forward_list<quick_symbol_functions_up> qf;
> >
> > I think this should be updated to use qf_list.
>
> Done.
>
> > > --- a/gdb/symfile-debug.c
> > > +++ b/gdb/symfile-debug.c
> > > @@ -85,7 +85,7 @@ objfile::has_partial_symbols ()
> > >       them, then that is an indication that they are in fact available.  Without
> > >       this function the symbols may have been already read in but they also may
> > >       not be present in this objfile.  */
> > > -  for (const auto &iter : qf)
> > > +  for (const auto &iter : qf_safe ())
> >
> > I read the commit message a couple of times, but I'm still not 100% sure
> > I understand what's happening here.  Is the change from qf -> qf_safe()
> > because calling (in this case has_symbols()) might download the full
> > debug info, and in doing do, might delete a previously downloaded
> > .debug_info section?
>
> quick_functions_up made from a separately downloaded .gdb_index is at
> first associated with the parent objfile (i.e. the objfile of the shared
> library containing a .gnu_debuglink that refers to the separate debug info
> which actually contains this .gdb_index section).
>
> When the separate debuginfo is downloaded (possibly during iteration
> over the parent's qf_list), we need to remove the gdb_index qf from
> the parent's qf_list.  This prevents gdb from later attempting to read
> the parent objfile for debug info that is actually in the corresponding
> separate debug objfile.
>
> > If this is the case then is it true that we will almost never want to
> > iterate over `qf` directly as most quick_symbol_functions methods might
> > result in this download/delete behaviour?
> >
> > If I'm right, then I think it might be useful to place a warning in the
> > comment before `qf` explaining this.
>
> Done.
>
> > > +/* A reverse basic_safe_iterator.  See basic_safe_iterator for intended use.  */
> > > +
> > > +template<typename Iterator>
> > > +class basic_safe_reverse_iterator
> > > +{
> > > +public:
> > > +  typedef basic_safe_reverse_iterator self_type;
> > > +  typedef typename Iterator::value_type value_type;
> > > +  typedef typename Iterator::reference reference;
> > > +  typedef typename Iterator::pointer pointer;
> > > +  typedef typename Iterator::iterator_category iterator_category;
> > > +  typedef typename Iterator::difference_type difference_type;
> > > +
> > > +  /* Construct the iterator using ARG, and construct the end iterator
> > > +     using ARG2.  */
> >
> > Maybe this is a standard thing, and I'm just showing my lack of
> > experience, but, given the existence of methods like rbegin() and
> > rend(), I think the comments here should be doing more to explain what's
> > expected of this input.
> >
> > Plus this comment seems confusing, though I can sort of see what it's
> > trying to say.
> >
> > We're creating a *_iterator object, which itself contains other
> > iterators.  We need to be super clear which particular "iterator" we're
> > talking about in the comments.
> >
> > Also, I think we need to be explicit that for this reverse iterator, the
> > first argument to the constructor is going to be the LAST element that
> > is iterated over.  One might be tempted to think that a safe reverse iterator
> > should be initialised with rbegin() and rend(), which isn't going to be
> > correct (unless I'm not understand, please correct me if so).
>
> That's right, the arguments to basic_safe_reverse_iterator should be
> forward iterators.  This will not work with reverse iterators.
>
> I added more information to this comment to hopefully clear up any
> confusion.
>
> > > +
> > > +/* A range adapter that wraps a forward range, and then returns
> > > +   safe reverse iterators wrapping the original range's iterators.  */
> >
> > I think the fact that this takes a FORWARD range is really important,
> > and we should do more to advertise this.  E.g. I think there should be a
> > comment on the constructor saying that RANGE is a forward range over
> > which we will iterate backwards.
> >
>
> Done.  The updated patch is included below.
>
> Aaron
>
> Commit message:
>
> This patch changes progspace objfile_list insertion so that separate
> debug objfiles are placed into the list after the parent objfile,
> instead of before.  Additionally qf_require_partial_symbols now returns
> a safe_range.
>
> These changes are intended to prepare gdb for on-demand debuginfo
> downloading and the downloading of .gdb_index sections.
>
> With on-demand downloading enabled, gdb might need to delete a
> .gdb_index quick_symbol_functions from a parent objfile while looping
> the objfile's list of quick_symbol_functions because the separate
> debug objfile has just been downloaded.  The use of a safe_range
> prevents this removal from causing iterator invalidation.
>
> gdb might also download a debuginfo file during symtab expansion.
> In this case an objfile will be added to the current progspace's
> objfiles_list during iteration over the list (for example, in
> iterate_over_symtabs).  We want these loops to also iterate over
> newly downloaded objfiles.  So objfiles need to be inserted into
> objfiles_list after their parent since it is during the search of
> the parent objfile for some symbol or filename that the separate
> debug objfile might be downloaded.
>
> To facilitate the safe deletion of objfiles, this patch also adds
> basic_safe_reverse_range and basic_safe_reverse_iterator.  This allows
> objfiles to be removed from the objfiles_list in a loop without iterator
> invalidation.
>
> If a forward safe iterator were to be used, the deletion of an
> objfile could invalidate the safe iterator's reference to the next
> objfile in the objfiles_list.  This can happen when the deletion
> of an objfile causes the deletion of a separate debug objfile that
> happens to the be next element in the objfiles_list.
>
> The standard reverse iterator is not suitable for safe objfile deletion.
> In order to safely delete the first objfile in the objfiles_list, the
> standard reverse iterator's underlying begin iterator would have to be
> decremented, resulting in undefined behavior.
>
> A small change was also made to a testcase in py-objfile.exp to
> account for the new placement of separate debug objfiles in
> objfiles_list.
> ---
>  gdb/jit.c                               |   7 +-
>  gdb/objfiles.c                          |  14 +--
>  gdb/objfiles.h                          |  17 +++-
>  gdb/progspace.c                         |  19 +++-
>  gdb/progspace.h                         |  31 ++++---
>  gdb/symfile-debug.c                     |  34 +++----
>  gdb/testsuite/gdb.python/py-objfile.exp |   2 +-
>  gdbsupport/safe-iterator.h              | 115 ++++++++++++++++++++++++
>  8 files changed, 195 insertions(+), 44 deletions(-)
>
> diff --git a/gdb/jit.c b/gdb/jit.c
> index 85a10be3055..10ee5f9b749 100644
> --- a/gdb/jit.c
> +++ b/gdb/jit.c
> @@ -1240,11 +1240,10 @@ jit_breakpoint_re_set (void)
>  static void
>  jit_inferior_exit_hook (struct inferior *inf)
>  {
> -  for (objfile *objf : current_program_space->objfiles_safe ())
> +  current_program_space->unlink_objfiles_if ([&] (const objfile *objf)
>      {
> -      if (objf->jited_data != nullptr && objf->jited_data->addr != 0)
> -       objf->unlink ();
> -    }
> +      return (objf->jited_data != nullptr) && (objf->jited_data->addr != 0);
> +    });
>  }
>
>  void
> diff --git a/gdb/objfiles.c b/gdb/objfiles.c
> index 8f085b1bb7c..d53dfaca90b 100644
> --- a/gdb/objfiles.c
> +++ b/gdb/objfiles.c
> @@ -470,6 +470,12 @@ objfile::unlink ()
>    current_program_space->remove_objfile (this);
>  }
>
> +qf_safe_range
> +objfile::qf_safe ()
> +{
> +  return qf_safe_range (qf_range (qf.begin (), qf.end ()));
> +}
> +
>  /* Free all separate debug objfile of OBJFILE, but don't free OBJFILE
>     itself.  */
>
> @@ -793,14 +799,12 @@ have_full_symbols (void)
>  void
>  objfile_purge_solibs (void)
>  {
> -  for (objfile *objf : current_program_space->objfiles_safe ())
> +  current_program_space->unlink_objfiles_if ([&] (const objfile *objf)
>      {
>        /* We assume that the solib package has been purged already, or will
>          be soon.  */
> -
> -      if (!(objf->flags & OBJF_USERLOADED) && (objf->flags & OBJF_SHARED))
> -       objf->unlink ();
> -    }
> +      return !(objf->flags & OBJF_USERLOADED) && (objf->flags & OBJF_SHARED);
> +    });
>  }
>
>
> diff --git a/gdb/objfiles.h b/gdb/objfiles.h
> index 621342d22b7..b917886988d 100644
> --- a/gdb/objfiles.h
> +++ b/gdb/objfiles.h
> @@ -370,6 +370,12 @@ class separate_debug_iterator
>
>  typedef iterator_range<separate_debug_iterator> separate_debug_range;
>
> +/* See objfile::qf_safe.  */
> +
> +using qf_list = std::forward_list<quick_symbol_functions_up>;
> +using qf_range = iterator_range<qf_list::iterator>;
> +using qf_safe_range = basic_safe_range<qf_range>;
> +
>  /* Sections in an objfile.  The section offsets are stored in the
>     OBJFILE.  */
>
> @@ -761,8 +767,15 @@ struct objfile
>    const struct sym_fns *sf = nullptr;
>
>    /* The "quick" (aka partial) symbol functions for this symbol
> -     reader.  */
> -  std::forward_list<quick_symbol_functions_up> qf;
> +     reader.  Many quick_symbol_functions methods may result
> +     in the deletion of a quick_symbol_functions from this
> +     qf_list.  It is recommended that qf_safe be used to iterate
> +     over the qf_list.  */
> +  qf_list qf;
> +
> +  /* Returns an iterable object that allows for safe deletion during
> +     iteration.  See gdbsupport/safe-iterator.h.  */
> +  qf_safe_range qf_safe ();
>
>    /* Per objfile data-pointers required by other GDB modules.  */
>
> diff --git a/gdb/progspace.c b/gdb/progspace.c
> index 0fc1fdd57ab..c4cc2e2e8af 100644
> --- a/gdb/progspace.c
> +++ b/gdb/progspace.c
> @@ -141,19 +141,19 @@ program_space::free_all_objfiles ()
>
>  void
>  program_space::add_objfile (std::unique_ptr<objfile> &&objfile,
> -                           struct objfile *before)
> +                           struct objfile *after)
>  {
> -  if (before == nullptr)
> +  if (after == nullptr)
>      objfiles_list.push_back (std::move (objfile));
>    else
>      {
>        auto iter = std::find_if (objfiles_list.begin (), objfiles_list.end (),
>                                 [=] (const std::unique_ptr<::objfile> &objf)
>                                 {
> -                                 return objf.get () == before;
> +                                 return objf.get () == after;
>                                 });
>        gdb_assert (iter != objfiles_list.end ());
> -      objfiles_list.insert (iter, std::move (objfile));
> +      objfiles_list.insert (++iter, std::move (objfile));
>      }
>  }
>
> @@ -182,6 +182,17 @@ program_space::remove_objfile (struct objfile *objfile)
>
>  /* See progspace.h.  */
>
> +void
> +program_space::unlink_objfiles_if
> +  (gdb::function_view<bool (const objfile *objfile)> predicate)
> +{
> +  for (auto &it : objfiles_safe ())
> +    if (predicate (it.get ()))
> +      it->unlink ();
> +}
> +
> +/* See progspace.h.  */
> +
>  struct objfile *
>  program_space::objfile_for_address (CORE_ADDR address)
>  {
> diff --git a/gdb/progspace.h b/gdb/progspace.h
> index 163cbf6a0f8..f23a57dc842 100644
> --- a/gdb/progspace.h
> +++ b/gdb/progspace.h
> @@ -250,28 +250,32 @@ struct program_space
>         unwrapping_objfile_iterator (objfiles_list.end ()));
>    }
>
> -  using objfiles_safe_range = basic_safe_range<objfiles_range>;
> +  using objfiles_it_range = iterator_range<objfile_list::iterator>;
> +  using objfiles_safe_reverse_range
> +    = basic_safe_reverse_range<objfiles_it_range>;
>
>    /* An iterable object that can be used to iterate over all objfiles.
>       The basic use is in a foreach, like:
>
>       for (objfile *objf : pspace->objfiles_safe ()) { ... }
>
> -     This variant uses a basic_safe_iterator so that objfiles can be
> -     deleted during iteration.  */
> -  objfiles_safe_range objfiles_safe ()
> +     This variant uses a basic_safe_reverse_iterator so that objfiles
> +     can be deleted during iteration.
> +
> +     The use of a reverse iterator helps ensure that separate debug
> +     objfiles are deleted before their parent objfile.  This prevents
> +     iterator invalidation due to the deletion of a parent objfile.  */
> + objfiles_safe_reverse_range objfiles_safe ()
>    {
> -    return objfiles_safe_range
> -      (objfiles_range
> -        (unwrapping_objfile_iterator (objfiles_list.begin ()),
> -         unwrapping_objfile_iterator (objfiles_list.end ())));
> +    return objfiles_safe_reverse_range
> +      (objfiles_it_range (objfiles_list.begin (), objfiles_list.end ()));
>    }
>
> -  /* Add OBJFILE to the list of objfiles, putting it just before
> -     BEFORE.  If BEFORE is nullptr, it will go at the end of the
> +  /* Add OBJFILE to the list of objfiles, putting it just after
> +     AFTER.  If AFTER is nullptr, it will go at the end of the
>       list.  */
>    void add_objfile (std::unique_ptr<objfile> &&objfile,
> -                   struct objfile *before);
> +                   struct objfile *after);
>
>    /* Remove OBJFILE from the list of objfiles.  */
>    void remove_objfile (struct objfile *objfile);
> @@ -286,6 +290,11 @@ struct program_space
>    /* Free all the objfiles associated with this program space.  */
>    void free_all_objfiles ();
>
> +  /* Unlink all objfiles associated with this program space for which
> +     PREDICATE evaluates to true.  */
> +  void unlink_objfiles_if
> +    (gdb::function_view<bool (const objfile *objfile)> predicate);
> +
>    /* Return the objfile containing ADDRESS, or nullptr if the address
>       is outside all objfiles in this progspace.  */
>    struct objfile *objfile_for_address (CORE_ADDR address);
> diff --git a/gdb/symfile-debug.c b/gdb/symfile-debug.c
> index 32c3ed9d349..c684f9cdd43 100644
> --- a/gdb/symfile-debug.c
> +++ b/gdb/symfile-debug.c
> @@ -85,7 +85,7 @@ objfile::has_partial_symbols ()
>       them, then that is an indication that they are in fact available.  Without
>       this function the symbols may have been already read in but they also may
>       not be present in this objfile.  */
> -  for (const auto &iter : qf)
> +  for (const auto &iter : qf_safe ())
>      {
>        retval = iter->has_symbols (this);
>        if (retval)
> @@ -108,7 +108,7 @@ objfile::has_unexpanded_symtabs ()
>                 objfile_debug_name (this));
>
>    bool result = false;
> -  for (const auto &iter : qf)
> +  for (const auto &iter : qf_safe ())
>      {
>        if (iter->has_unexpanded_symtabs (this))
>         {
> @@ -133,7 +133,7 @@ objfile::find_last_source_symtab ()
>      gdb_printf (gdb_stdlog, "qf->find_last_source_symtab (%s)\n",
>                 objfile_debug_name (this));
>
> -  for (const auto &iter : qf)
> +  for (const auto &iter : qf_safe ())
>      {
>        retval = iter->find_last_source_symtab (this);
>        if (retval != nullptr)
> @@ -166,7 +166,7 @@ objfile::forget_cached_source_info ()
>         }
>      }
>
> -  for (const auto &iter : qf)
> +  for (const auto &iter : qf_safe ())
>      iter->forget_cached_source_info (this);
>  }
>
> @@ -213,7 +213,7 @@ objfile::map_symtabs_matching_filename
>      return result;
>    };
>
> -  for (const auto &iter : qf)
> +  for (const auto &iter : qf_safe ())
>      {
>        if (!iter->expand_symtabs_matching (this,
>                                           match_one_filename,
> @@ -278,7 +278,7 @@ objfile::lookup_symbol (block_enum kind, const char *name, domain_enum domain)
>      return true;
>    };
>
> -  for (const auto &iter : qf)
> +  for (const auto &iter : qf_safe ())
>      {
>        if (!iter->expand_symtabs_matching (this,
>                                           nullptr,
> @@ -309,7 +309,7 @@ objfile::print_stats (bool print_bcache)
>      gdb_printf (gdb_stdlog, "qf->print_stats (%s, %d)\n",
>                 objfile_debug_name (this), print_bcache);
>
> -  for (const auto &iter : qf)
> +  for (const auto &iter : qf_safe ())
>      iter->print_stats (this, print_bcache);
>  }
>
> @@ -320,7 +320,7 @@ objfile::dump ()
>      gdb_printf (gdb_stdlog, "qf->dump (%s)\n",
>                 objfile_debug_name (this));
>
> -  for (const auto &iter : qf)
> +  for (const auto &iter : qf_safe ())
>      iter->dump (this);
>  }
>
> @@ -335,7 +335,7 @@ objfile::expand_symtabs_for_function (const char *func_name)
>    lookup_name_info base_lookup (func_name, symbol_name_match_type::FULL);
>    lookup_name_info lookup_name = base_lookup.make_ignore_params ();
>
> -  for (const auto &iter : qf)
> +  for (const auto &iter : qf_safe ())
>      iter->expand_symtabs_matching (this,
>                                    nullptr,
>                                    &lookup_name,
> @@ -354,7 +354,7 @@ objfile::expand_all_symtabs ()
>      gdb_printf (gdb_stdlog, "qf->expand_all_symtabs (%s)\n",
>                 objfile_debug_name (this));
>
> -  for (const auto &iter : qf)
> +  for (const auto &iter : qf_safe ())
>      iter->expand_all_symtabs (this);
>  }
>
> @@ -372,7 +372,7 @@ objfile::expand_symtabs_with_fullname (const char *fullname)
>      return filename_cmp (basenames ? basename : fullname, filename) == 0;
>    };
>
> -  for (const auto &iter : qf)
> +  for (const auto &iter : qf_safe ())
>      iter->expand_symtabs_matching (this,
>                                    file_matcher,
>                                    nullptr,
> @@ -406,7 +406,7 @@ objfile::expand_symtabs_matching
>                 host_address_to_string (&expansion_notify),
>                 search_domain_name (kind));
>
> -  for (const auto &iter : qf)
> +  for (const auto &iter : qf_safe ())
>      if (!iter->expand_symtabs_matching (this, file_matcher, lookup_name,
>                                         symbol_matcher, expansion_notify,
>                                         search_flags, domain, kind))
> @@ -431,7 +431,7 @@ objfile::find_pc_sect_compunit_symtab (struct bound_minimal_symbol msymbol,
>                 host_address_to_string (section),
>                 warn_if_readin);
>
> -  for (const auto &iter : qf)
> +  for (const auto &iter : qf_safe ())
>      {
>        retval = iter->find_pc_sect_compunit_symtab (this, msymbol, pc, section,
>                                                    warn_if_readin);
> @@ -459,7 +459,7 @@ objfile::map_symbol_filenames (gdb::function_view<symbol_filename_ftype> fun,
>                 objfile_debug_name (this),
>                 need_fullname);
>
> -  for (const auto &iter : qf)
> +  for (const auto &iter : qf_safe ())
>      iter->map_symbol_filenames (this, fun, need_fullname);
>  }
>
> @@ -471,7 +471,7 @@ objfile::compute_main_name ()
>                 "qf->compute_main_name (%s)\n",
>                 objfile_debug_name (this));
>
> -  for (const auto &iter : qf)
> +  for (const auto &iter : qf_safe ())
>      iter->compute_main_name (this);
>  }
>
> @@ -485,7 +485,7 @@ objfile::find_compunit_symtab_by_address (CORE_ADDR address)
>                 hex_string (address));
>
>    struct compunit_symtab *result = NULL;
> -  for (const auto &iter : qf)
> +  for (const auto &iter : qf_safe ())
>      {
>        result = iter->find_compunit_symtab_by_address (this, address);
>        if (result != nullptr)
> @@ -510,7 +510,7 @@ objfile::lookup_global_symbol_language (const char *name,
>    enum language result = language_unknown;
>    *symbol_found_p = false;
>
> -  for (const auto &iter : qf)
> +  for (const auto &iter : qf_safe ())
>      {
>        result = iter->lookup_global_symbol_language (this, name, domain,
>                                                     symbol_found_p);
> diff --git a/gdb/testsuite/gdb.python/py-objfile.exp b/gdb/testsuite/gdb.python/py-objfile.exp
> index 61b9942de79..0bf49976b73 100644
> --- a/gdb/testsuite/gdb.python/py-objfile.exp
> +++ b/gdb/testsuite/gdb.python/py-objfile.exp
> @@ -135,7 +135,7 @@ gdb_test "p main" "= {<text variable, no debug info>} $hex <main>" \
>  gdb_py_test_silent_cmd "python objfile.add_separate_debug_file(\"${binfile}\")" \
>      "Add separate debug file file" 1
>
> -gdb_py_test_silent_cmd "python sep_objfile = gdb.objfiles()\[0\]" \
> +gdb_py_test_silent_cmd "python sep_objfile = gdb.objfiles()\[1\]" \
>      "Get separate debug info objfile" 1
>
>  gdb_test "python print (sep_objfile.owner.filename)" "${testfile}2" \
> diff --git a/gdbsupport/safe-iterator.h b/gdbsupport/safe-iterator.h
> index b4891328f1a..d1b9ce21ffc 100644
> --- a/gdbsupport/safe-iterator.h
> +++ b/gdbsupport/safe-iterator.h
> @@ -136,4 +136,119 @@ class basic_safe_range
>    Range m_range;
>  };
>
> +/* A reverse basic_safe_iterator.  See basic_safe_iterator for intended use.  */
> +
> +template<typename Iterator>
> +class basic_safe_reverse_iterator
> +{
> +public:
> +  typedef basic_safe_reverse_iterator self_type;
> +  typedef typename Iterator::value_type value_type;
> +  typedef typename Iterator::reference reference;
> +  typedef typename Iterator::pointer pointer;
> +  typedef typename Iterator::iterator_category iterator_category;
> +  typedef typename Iterator::difference_type difference_type;
> +
> +  /* Construct the iterator using ARG, and construct the end iterator
> +     using ARG2.  ARG and ARG2 should be forward iterators, typically
> +     from begin and end methods, respectively.
> +
> +     For example if ARG1 is created with container.begin and ARG2 is
> +     is created with container.end, then the basic_safe_reverse_iterator
> +     will traverse from the last element in the container to the first
> +     element in the container.  */
> +  template<typename Arg>
> +  explicit basic_safe_reverse_iterator (Arg &&arg, Arg &&arg2)
> +    : m_begin (std::forward<Arg> (arg)),
> +      m_end (std::forward<Arg> (arg2)),
> +      m_it (m_end),
> +      m_next (m_end)
> +  {
> +    /* M_IT and M_NEXT are initialized as one-past-end.  Set M_IT to point
> +       to the last element and set M_NEXT to point to the second last element,
> +       if such elements exist.  */
> +    if (m_it != m_begin)
> +      {
> +       --m_it;
> +
> +       if (m_it != m_begin)
> +         {
> +           --m_next;
> +           --m_next;
> +         }
> +      }
> +  }
> +
> +  typename std::invoke_result<decltype(&Iterator::operator*), Iterator>::type
> +    operator* () const
> +  { return *m_it; }
> +
> +  self_type &operator++ ()
> +  {
> +    m_it = m_next;
> +
> +    if (m_it != m_end)
> +      {
> +       /* Use M_BEGIN only if we sure that it is valid.  */
> +       if (m_it == m_begin)
> +         m_next = m_end;
> +       else
> +         --m_next;
> +      }
> +
> +    return *this;
> +  }
> +
> +  bool operator== (const self_type &other) const
> +  { return m_it == other.m_it; }
> +
> +  bool operator!= (const self_type &other) const
> +  { return m_it != other.m_it; }
> +
> +private:
> +  /* The first element.  */
> +  Iterator m_begin {};
> +
> +  /* A one-past-end iterator.  */
> +  Iterator m_end {};
> +
> +  /* The current element.  */
> +  Iterator m_it {};
> +
> +  /* The next element.  Always one element ahead of M_IT.  */
> +  Iterator m_next {};
> +};
> +
> +/* A range adapter that wraps a forward range, and then returns
> +   safe reverse iterators wrapping the original range's iterators.  */
> +
> +template<typename Range>
> +class basic_safe_reverse_range
> +{
> +public:
> +
> +  typedef basic_safe_reverse_iterator<typename Range::iterator> iterator;
> +
> +  /* RANGE must be a forward range.  basic_safe_reverse_iterators
> +     will be used to traverse the forward range from the last element
> +     to the first.  */
> +  explicit basic_safe_reverse_range (Range range)
> +    : m_range (range)
> +  {
> +  }
> +
> +  iterator begin ()
> +  {
> +    return iterator (m_range.begin (), m_range.end ());
> +  }
> +
> +  iterator end ()
> +  {
> +    return iterator (m_range.end (), m_range.end ());
> +  }
> +
> +private:
> +
> +  Range m_range;
> +};
>  #endif /* COMMON_SAFE_ITERATOR_H */
> --
> 2.43.0
>