Re: [PATCH r/7 v2.1] gdb/debuginfod: Support on-demand debuginfo downloading

[Andrew Burgess <aburgess@redhat.com>]

I'm replying here even though you posted a v2.2 -- but you didn't
include the commit message in that post :( which makes it harder to
reply.

Aaron Merey via Gdb-patches <gdb-patches@sourceware.org> writes:

> I've updated this patch with an improvement to objfiles_safe.
>
> Separate debug objfiles are normally placed in a progspace's objfiles_list
> immediately preceeding their parent objfile.  In the previous version of
> this patch [1], I changed this so that separate debug objfiles might be added
> to the end of progspace's objfiles_list.  This was done so that when
> debuginfo is downloaded during during, for instance, objfile::lookup_symbol,
> the newly added objfile wouldn't be skipped by the loop calling lookup_symbol
> on each objfile.
>
> This change to separate debug objfile placement in objfiles_list created
> the possibility for iterator invalidation when objfiles_safe is used to
> delete objfiles.
>
> objfiles_safe returns an iterator to objfiles_list that stores an iterator
> to the next objfile in the sequence.  This facilitates safe deletion of
> objfiles in a loop.  However deletion of the parent objfile causes deletion
> of the separate debug objfile.  If the separate debug objfiles happens to
> be placed in objfiles_list immediately after its parent, the iterator's
> reference to the next objfile in the list is invalidated, resulting in a
> use-after-free.
>
> To fix this, I changed add_objfile so that all separate debug objfiles
> are placed in objfiles_list after their parent, instead of before.  This
> ensures that any deferred debuginfo downloaded during iteration over
> objfiles_list aren't skipped by the loop.  I also modified objfiles_safe
> so that it returns a safe reverse iterator.  Reverse iteration prevents
> iterator invalidation by ensuring that separate debug objfiles are deleted
> before their parent objfile.

Reading this commit message I got the impression that this was a
refactoring commit -- changing the order in which items are stored in
objfiles_list, and how the iterators over that list worked.  Given that
I was reasonably comfortable that there are no tests.

But, looking through this patch I spotted the change to elfread.c that
seems to be adding new functionality related to separate debug file
downloading.  New functionality commits should really have tests.

I think this commit should be split in two.  First the refactor, change
the order in which objects are stored in objfiles_list, and how the
iterators work.

Then add the new functionality (along with tests) in a second commit.
This should mean that if problems are spotted later on it will be easier
to reason about whether it was the new functionality, or the refactor.

Thanks,
Andrew

>
> [1] https://sourceware.org/pipermail/gdb-patches/2023-April/198945.html
>
> ---
>  gdb/dwarf2/frame.c                      |  13 ++
>  gdb/dwarf2/frame.h                      |   4 +
>  gdb/dwarf2/index-cache.c                |  33 +++++
>  gdb/dwarf2/index-cache.h                |  13 ++
>  gdb/dwarf2/public.h                     |   7 ++
>  gdb/dwarf2/read-gdb-index.c             | 157 +++++++++++++++++++++---
>  gdb/dwarf2/read.c                       | 146 +++++++++++++++++++++-
>  gdb/dwarf2/read.h                       |  10 ++
>  gdb/dwarf2/section.c                    |   3 +-
>  gdb/elfread.c                           |   2 +-
>  gdb/objfile-flags.h                     |   4 +
>  gdb/objfiles.c                          |   1 +
>  gdb/objfiles.h                          |  22 +++-
>  gdb/progspace.c                         |   8 +-
>  gdb/progspace.h                         | 120 +++++++++++++-----
>  gdb/symfile-debug.c                     | 136 ++++++++++----------
>  gdb/symfile.c                           |   7 +-
>  gdb/testsuite/gdb.python/py-objfile.exp |   2 +-
>  gdbsupport/iterator-range.h             |  45 +++++++
>  19 files changed, 603 insertions(+), 130 deletions(-)
>
> diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
> index a561aaf3100..3613f8252a7 100644
> --- a/gdb/dwarf2/frame.c
> +++ b/gdb/dwarf2/frame.c
> @@ -1609,6 +1609,19 @@ set_comp_unit (struct objfile *objfile, struct comp_unit *unit)
>    return dwarf2_frame_bfd_data.set (abfd, unit);
>  }
>  
> +/* See frame.h.  */
> +
> +void
> +dwarf2_clear_frame_data (struct objfile *objfile)
> +{
> +  bfd *abfd = objfile->obfd.get ();
> +
> +  if (gdb_bfd_requires_relocations (abfd))
> +    dwarf2_frame_objfile_data.clear (objfile);
> +  else
> +    dwarf2_frame_bfd_data.clear (abfd);
> +}
> +
>  /* Find the FDE for *PC.  Return a pointer to the FDE, and store the
>     initial location associated with it into *PC.  */
>  
> diff --git a/gdb/dwarf2/frame.h b/gdb/dwarf2/frame.h
> index 5643e557513..2391e313e7c 100644
> --- a/gdb/dwarf2/frame.h
> +++ b/gdb/dwarf2/frame.h
> @@ -238,6 +238,10 @@ void dwarf2_append_unwinders (struct gdbarch *gdbarch);
>  extern const struct frame_base *
>    dwarf2_frame_base_sniffer (frame_info_ptr this_frame);
>  
> +/* Delete OBJFILEs comp_unit.  */
> +
> +extern void dwarf2_clear_frame_data (struct objfile * objfile);
> +
>  /* Compute the DWARF CFA for a frame.  */
>  
>  CORE_ADDR dwarf2_frame_cfa (frame_info_ptr this_frame);
> diff --git a/gdb/dwarf2/index-cache.c b/gdb/dwarf2/index-cache.c
> index 79ab706ee9d..bbafcd321b2 100644
> --- a/gdb/dwarf2/index-cache.c
> +++ b/gdb/dwarf2/index-cache.c
> @@ -216,6 +216,33 @@ index_cache::lookup_gdb_index (const bfd_build_id *build_id,
>    return {};
>  }
>  
> +/* See index-cache.h.  */
> +
> +gdb::array_view<const gdb_byte>
> +index_cache::lookup_gdb_index_debuginfod (const char *index_path,
> +					  std::unique_ptr<index_cache_resource> *resource)
> +{
> +  try
> +    {
> +      /* Try to map that file.  */
> +      index_cache_resource_mmap *mmap_resource
> +	= new index_cache_resource_mmap (index_path);
> +
> +      /* Hand the resource to the caller.  */
> +      resource->reset (mmap_resource);
> +
> +      return gdb::array_view<const gdb_byte>
> +	  ((const gdb_byte *) mmap_resource->mapping.get (),
> +	   mmap_resource->mapping.size ());
> +    }
> +  catch (const gdb_exception_error &except)
> +    {
> +      warning (_("Unable to read %s: %s"), index_path, except.what ());
> +    }
> +
> +  return {};
> +}
> +
>  #else /* !HAVE_SYS_MMAN_H */
>  
>  /* See dwarf-index-cache.h.  This is a no-op on unsupported systems.  */
> @@ -227,6 +254,12 @@ index_cache::lookup_gdb_index (const bfd_build_id *build_id,
>    return {};
>  }
>  
> +gdb::array_view<const gdb_byte>
> +index_cache::lookup_gdb_index_debuginfod (const char *index_path,
> +					  std::unique_ptr<index_cache_resource> *resource)
> +{
> +  return {};
> +}
>  #endif
>  
>  /* See dwarf-index-cache.h.  */
> diff --git a/gdb/dwarf2/index-cache.h b/gdb/dwarf2/index-cache.h
> index 1efff17049f..e400afd5123 100644
> --- a/gdb/dwarf2/index-cache.h
> +++ b/gdb/dwarf2/index-cache.h
> @@ -67,6 +67,19 @@ class index_cache
>    lookup_gdb_index (const bfd_build_id *build_id,
>  		    std::unique_ptr<index_cache_resource> *resource);
>  
> +  /* Look for an index file located at INDEX_PATH in the debuginfod cache.
> +     Unlike lookup_gdb_index, this function does not exit early if the
> +     index cache has not been enabled.
> +
> +     If found, return the contents as an array_view and store the underlying
> +     resources (allocated memory, mapped file, etc) in RESOURCE.  The returned
> +     array_view is valid as long as RESOURCE is not destroyed.
> +
> +     If no matching index file is found, return an empty array view.  */
> +  gdb::array_view<const gdb_byte>
> +  lookup_gdb_index_debuginfod (const char *index_path,
> +			       std::unique_ptr<index_cache_resource> *resource);
> +
>    /* Return the number of cache hits.  */
>    unsigned int n_hits () const
>    { return m_n_hits; }
> diff --git a/gdb/dwarf2/public.h b/gdb/dwarf2/public.h
> index 0e74857eb1a..4a44cdbc223 100644
> --- a/gdb/dwarf2/public.h
> +++ b/gdb/dwarf2/public.h
> @@ -40,4 +40,11 @@ extern void dwarf2_initialize_objfile (struct objfile *objfile);
>  
>  extern void dwarf2_build_frame_info (struct objfile *);
>  
> +/* Query debuginfod for the .gdb_index associated with OBJFILE.  If
> +   successful, create an objfile to hold the .gdb_index information
> +   and act as a placeholder until the full debuginfo needs to be
> +   downloaded.  */
> +
> +extern bool dwarf2_has_separate_index (struct objfile *);
> +
>  #endif /* DWARF2_PUBLIC_H */
> diff --git a/gdb/dwarf2/read-gdb-index.c b/gdb/dwarf2/read-gdb-index.c
> index 1006386cb2d..895fbede7c2 100644
> --- a/gdb/dwarf2/read-gdb-index.c
> +++ b/gdb/dwarf2/read-gdb-index.c
> @@ -136,6 +136,7 @@ struct dwarf2_gdb_index : public dwarf2_base_index_functions
>       gdb.dwarf2/gdb-index.exp testcase.  */
>    void dump (struct objfile *objfile) override;
>  
> +  /* Calls do_expand_matching_symbols and downloads debuginfo if necessary.  */
>    void expand_matching_symbols
>      (struct objfile *,
>       const lookup_name_info &lookup_name,
> @@ -143,6 +144,14 @@ struct dwarf2_gdb_index : public dwarf2_base_index_functions
>       int global,
>       symbol_compare_ftype *ordered_compare) override;
>  
> +  void do_expand_matching_symbols
> +    (struct objfile *,
> +     const lookup_name_info &lookup_name,
> +     domain_enum domain,
> +     int global,
> +     symbol_compare_ftype *ordered_compare);
> +
> +  /* Calls do_expand_symtabs_matching and downloads debuginfo if necessary.  */
>    bool expand_symtabs_matching
>      (struct objfile *objfile,
>       gdb::function_view<expand_symtabs_file_matcher_ftype> file_matcher,
> @@ -152,8 +161,59 @@ struct dwarf2_gdb_index : public dwarf2_base_index_functions
>       block_search_flags search_flags,
>       domain_enum domain,
>       enum search_domain kind) override;
> +
> +  bool do_expand_symtabs_matching
> +    (struct objfile *objfile,
> +     gdb::function_view<expand_symtabs_file_matcher_ftype> file_matcher,
> +     const lookup_name_info *lookup_name,
> +     gdb::function_view<expand_symtabs_symbol_matcher_ftype> symbol_matcher,
> +     gdb::function_view<expand_symtabs_exp_notify_ftype> expansion_notify,
> +     block_search_flags search_flags,
> +     domain_enum domain,
> +     enum search_domain kind);
> +
> +  /* Calls dwarf2_base_index_functions::expand_all_symtabs and downloads
> +     debuginfo if necessary.  */
> +  void expand_all_symtabs (struct objfile *objfile) override;
> +
> +  /* Calls dwarf2_base_index_functions::find_last_source_symtab and downloads
> +     debuginfo if necessary.  */
> +  struct symtab *find_last_source_symtab (struct objfile *objfile) override;
>  };
>  
> +void
> +dwarf2_gdb_index::expand_all_symtabs (struct objfile *objfile)
> +{
> +  try
> +    {
> +      dwarf2_base_index_functions::expand_all_symtabs (objfile);
> +    }
> +  catch (gdb_exception e)
> +    {
> +      if ((objfile->flags & OBJF_DOWNLOAD_DEFERRED) == 0)
> +	exception_print (gdb_stderr, e);
> +      else
> +	read_full_dwarf_from_debuginfod (objfile, this);
> +    }
> +}
> +
> +struct symtab *
> +dwarf2_gdb_index::find_last_source_symtab (struct objfile *objfile)
> +{
> +  try
> +    {
> +      return dwarf2_base_index_functions::find_last_source_symtab (objfile);
> +    }
> +  catch (gdb_exception e)
> +    {
> +      if ((objfile->flags & OBJF_DOWNLOAD_DEFERRED) == 0)
> +	exception_print (gdb_stderr, e);
> +      else
> +	read_full_dwarf_from_debuginfod (objfile, this);
> +      return nullptr;
> +    }
> +}
> +
>  /* This dumps minimal information about the index.
>     It is called via "mt print objfiles".
>     One use is to verify .gdb_index has been loaded by the
> @@ -315,7 +375,7 @@ dw2_symtab_iter_next (struct dw2_symtab_iterator *iter,
>  }
>  
>  void
> -dwarf2_gdb_index::expand_matching_symbols
> +dwarf2_gdb_index::do_expand_matching_symbols
>    (struct objfile *objfile,
>     const lookup_name_info &name, domain_enum domain,
>     int global,
> @@ -353,6 +413,30 @@ dwarf2_gdb_index::expand_matching_symbols
>      }, per_objfile);
>  }
>  
> +void
> +dwarf2_gdb_index::expand_matching_symbols
> +  (struct objfile *objfile,
> +   const lookup_name_info &lookup_name,
> +   domain_enum domain,
> +   int global,
> +   symbol_compare_ftype *ordered_compare)
> +{
> +  try
> +    {
> +      do_expand_matching_symbols (objfile, lookup_name, domain,
> +				  global, ordered_compare);
> +    }
> +  catch (gdb_exception e)
> +    {
> +      if ((objfile->flags & OBJF_DOWNLOAD_DEFERRED) == 0)
> +	exception_print (gdb_stderr, e);
> +      else
> +	read_full_dwarf_from_debuginfod (objfile, this);
> +      return;
> +    }
> +}
> +
> +
>  /* Helper for dw2_expand_matching symtabs.  Called on each symbol
>     matched, to expand corresponding CUs that were marked.  IDX is the
>     index of the symbol name that matched.  */
> @@ -455,7 +539,7 @@ dw2_expand_marked_cus
>  }
>  
>  bool
> -dwarf2_gdb_index::expand_symtabs_matching
> +dwarf2_gdb_index::do_expand_symtabs_matching
>      (struct objfile *objfile,
>       gdb::function_view<expand_symtabs_file_matcher_ftype> file_matcher,
>       const lookup_name_info *lookup_name,
> @@ -504,6 +588,39 @@ dwarf2_gdb_index::expand_symtabs_matching
>    return result;
>  }
>  
> +bool
> +dwarf2_gdb_index::expand_symtabs_matching
> +    (struct objfile *objfile,
> +     gdb::function_view<expand_symtabs_file_matcher_ftype> file_matcher,
> +     const lookup_name_info *lookup_name,
> +     gdb::function_view<expand_symtabs_symbol_matcher_ftype> symbol_matcher,
> +     gdb::function_view<expand_symtabs_exp_notify_ftype> expansion_notify,
> +     block_search_flags search_flags,
> +     domain_enum domain,
> +     enum search_domain kind)
> +{
> +  if (objfile->flags & OBJF_READNEVER)
> +    return false;
> +
> +  try
> +    {
> +      return do_expand_symtabs_matching (objfile, file_matcher, lookup_name,
> +					 symbol_matcher, expansion_notify,
> +					 search_flags, domain, kind);
> +    }
> +  catch (gdb_exception e)
> +    {
> +      if ((objfile->flags & OBJF_DOWNLOAD_DEFERRED) == 0)
> +	{
> +	  exception_print (gdb_stderr, e);
> +	  return false;
> +	}
> +
> +      read_full_dwarf_from_debuginfod (objfile, this);
> +      return true;
> +    }
> +}
> +
>  quick_symbol_functions_up
>  mapped_gdb_index::make_quick_functions () const
>  {
> @@ -797,28 +914,32 @@ dwarf2_read_gdb_index
>  
>    /* If there is a .dwz file, read it so we can get its CU list as
>       well.  */
> -  dwz = dwarf2_get_dwz_file (per_bfd);
> -  if (dwz != NULL)
> +  if (get_gdb_index_contents_dwz != nullptr)
>      {
>        mapped_gdb_index dwz_map;
>        const gdb_byte *dwz_types_ignore;
>        offset_type dwz_types_elements_ignore;
> +      dwz = dwarf2_get_dwz_file (per_bfd);
>  
> -      gdb::array_view<const gdb_byte> dwz_index_content
> -	= get_gdb_index_contents_dwz (objfile, dwz);
> -
> -      if (dwz_index_content.empty ())
> -	return 0;
> -
> -      if (!read_gdb_index_from_buffer (bfd_get_filename (dwz->dwz_bfd.get ()),
> -				       1, dwz_index_content, &dwz_map,
> -				       &dwz_list, &dwz_list_elements,
> -				       &dwz_types_ignore,
> -				       &dwz_types_elements_ignore))
> +      if (dwz != nullptr)
>  	{
> -	  warning (_("could not read '.gdb_index' section from %s; skipping"),
> -		   bfd_get_filename (dwz->dwz_bfd.get ()));
> -	  return 0;
> +	  gdb::array_view<const gdb_byte> dwz_index_content
> +	    = get_gdb_index_contents_dwz (objfile, dwz);
> +
> +	  if (dwz_index_content.empty ())
> +	    return 0;
> +
> +	  if (!read_gdb_index_from_buffer (bfd_get_filename
> +					     (dwz->dwz_bfd.get ()),
> +					   1, dwz_index_content, &dwz_map,
> +					   &dwz_list, &dwz_list_elements,
> +					   &dwz_types_ignore,
> +					   &dwz_types_elements_ignore))
> +	    {
> +	      warning (_("could not read '.gdb_index' section from %s; skipping"),
> +		       bfd_get_filename (dwz->dwz_bfd.get ()));
> +		return 0;
> +	    }
>  	}
>      }
>  
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index 29a95cb8b2f..cbf77e3af76 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -34,6 +34,7 @@
>  #include "dwarf2/attribute.h"
>  #include "dwarf2/comp-unit-head.h"
>  #include "dwarf2/cu.h"
> +#include "dwarf2/frame.h"
>  #include "dwarf2/index-cache.h"
>  #include "dwarf2/index-common.h"
>  #include "dwarf2/leb.h"
> @@ -95,6 +96,8 @@
>  #include "split-name.h"
>  #include "gdbsupport/parallel-for.h"
>  #include "gdbsupport/thread-pool.h"
> +#include "inferior.h"
> +#include "debuginfod-support.h"
>  
>  /* When == 1, print basic high level tracing messages.
>     When > 1, be more verbose.
> @@ -3163,7 +3166,7 @@ dwarf2_base_index_functions::find_per_cu (dwarf2_per_bfd *per_bfd,
>  }
>  
>  struct compunit_symtab *
> -dwarf2_base_index_functions::find_pc_sect_compunit_symtab
> +dwarf2_base_index_functions::do_find_pc_sect_compunit_symtab
>       (struct objfile *objfile,
>        struct bound_minimal_symbol msymbol,
>        CORE_ADDR pc,
> @@ -3194,6 +3197,32 @@ dwarf2_base_index_functions::find_pc_sect_compunit_symtab
>    return result;
>  }
>  
> +struct compunit_symtab *
> +dwarf2_base_index_functions::find_pc_sect_compunit_symtab
> +     (struct objfile *objfile,
> +      struct bound_minimal_symbol msymbol,
> +      CORE_ADDR pc,
> +      struct obj_section *section,
> +      int warn_if_readin)
> +{
> +  if (objfile->flags & OBJF_READNEVER)
> +    return nullptr;
> +
> +  try
> +    {
> +      return do_find_pc_sect_compunit_symtab (objfile, msymbol, pc,
> +					      section, warn_if_readin);
> +    }
> +  catch (gdb_exception e)
> +    {
> +      if ((objfile->flags & OBJF_DOWNLOAD_DEFERRED) == 0)
> +	exception_print (gdb_stderr, e);
> +      else
> +	read_full_dwarf_from_debuginfod (objfile, this);
> +      return nullptr;
> +    }
> +}
> +
>  void
>  dwarf2_base_index_functions::map_symbol_filenames
>       (struct objfile *objfile,
> @@ -3350,6 +3379,29 @@ get_gdb_index_contents_from_cache_dwz (objfile *obj, dwz_file *dwz)
>    return global_index_cache.lookup_gdb_index (build_id, &dwz->index_cache_res);
>  }
>  
> +/* Query debuginfod for the .gdb_index matching OBJFILE's build-id.  Return the
> +   contents if successful.  */
> +
> +static gdb::array_view<const gdb_byte>
> +get_gdb_index_contents_from_debuginfod (objfile *objfile, dwarf2_per_bfd *per_bfd)
> +{
> +  const bfd_build_id *build_id = build_id_bfd_get (objfile->obfd.get ());
> +  if (build_id == nullptr)
> +    return {};
> +
> +  gdb::unique_xmalloc_ptr<char> index_path;
> +  scoped_fd fd = debuginfod_section_query (build_id->data, build_id->size,
> +					   bfd_get_filename
> +					     (objfile->obfd.get ()),
> +					   ".gdb_index",
> +					   &index_path);
> +  if (fd.get () < 0)
> +    return {};
> +
> +  return global_index_cache.lookup_gdb_index_debuginfod
> +    (index_path.get (), &per_bfd->index_cache_res);
> +}
> +
>  static quick_symbol_functions_up make_cooked_index_funcs ();
>  
>  /* See dwarf2/public.h.  */
> @@ -3415,10 +3467,102 @@ dwarf2_initialize_objfile (struct objfile *objfile)
>        return;
>      }
>  
> +  if ((objfile->flags & OBJF_DOWNLOAD_DEFERRED)
> +      && dwarf2_read_gdb_index (per_objfile,
> +				get_gdb_index_contents_from_debuginfod,
> +				nullptr))
> +    {
> +      dwarf_read_debug_printf ("found .gdb_index from debuginfod");
> +      objfile->qf.push_front (per_bfd->index_table->make_quick_functions ());
> +      return;
> +    }
> +
>    global_index_cache.miss ();
>    objfile->qf.push_front (make_cooked_index_funcs ());
>  }
>  
> +/* See read.h.  */
> +
> +void
> +read_full_dwarf_from_debuginfod (struct objfile *objfile,
> +				 dwarf2_base_index_functions *fncs)
> +{
> +  gdb_assert (objfile->flags & OBJF_DOWNLOAD_DEFERRED);
> +
> +  const struct bfd_build_id *build_id = build_id_bfd_get (objfile->obfd.get ());
> +  const char *filename;
> +  gdb_bfd_ref_ptr debug_bfd;
> +  gdb::unique_xmalloc_ptr<char> symfile_path;
> +  scoped_fd fd;
> +
> +  if (build_id == nullptr)
> +    goto unset;
> +
> +  filename = bfd_get_filename (objfile->obfd.get ());
> +  fd = debuginfod_debuginfo_query (build_id->data, build_id->size,
> +				   filename, &symfile_path);
> +  if (fd.get () < 0)
> +    goto unset;
> +
> +  /* Separate debuginfo successfully retrieved from server.  */
> +  debug_bfd = symfile_bfd_open (symfile_path.get ());
> +  if (debug_bfd == nullptr
> +      || !build_id_verify (debug_bfd.get (), build_id->size, build_id->data))
> +    {
> +      warning (_("File \"%s\" from debuginfod cannot be opened as bfd"),
> +	       filename);
> +      goto unset;
> +    }
> +
> +  /* This may also trigger a dwz download.  */
> +  symbol_file_add_separate (debug_bfd, symfile_path.get (),
> +			    current_inferior ()->symfile_flags, objfile);
> +
> +  /* Clear frame data so it can be recalculated using DWARF.  */
> +  dwarf2_clear_frame_data (objfile);
> +
> +unset:
> +  objfile->flags &= ~OBJF_DOWNLOAD_DEFERRED;
> +
> +  /* Avoid reading this objfile's index from now on.  If available the
> +     separate debug objfile's index will be used instead, since it actually
> +     contains the symbols and CUs referenced in the index.  */
> +  objfile->remove_partial_symbol (fncs);
> +}
> +
> +/* See public.h.  */
> +
> +bool
> +dwarf2_has_separate_index (struct objfile *objfile)
> +{
> +  if (objfile->flags & OBJF_DOWNLOAD_DEFERRED)
> +    return true;
> +  if (objfile->flags & OBJF_MAINLINE)
> +    return false;
> +  if (!IS_DIR_SEPARATOR (*objfile_filename (objfile)))
> +    return false;
> +
> +  gdb::unique_xmalloc_ptr<char> index_path;
> +  const bfd_build_id *build_id = build_id_bfd_get (objfile->obfd.get ());
> +  scoped_fd fd = debuginfod_section_query (build_id->data,
> +					   build_id->size,
> +					   bfd_get_filename
> +					     (objfile->obfd.get ()),
> +					   ".gdb_index",
> +					   &index_path);
> +
> +  if (fd.get () < 0)
> +    return false;
> +
> +  /* We found a separate .gdb_index file so a separate debuginfo file
> +     should exist, but we don't want to download it until necessary.
> +     Attach the index to this objfile and defer the debuginfo download
> +     until gdb needs to expand symtabs referenced by the index.  */
> +  objfile->flags |= OBJF_DOWNLOAD_DEFERRED;
> +  dwarf2_initialize_objfile (objfile);
> +  return true;
> +}
> +
>  \f
>  
>  /* Build a partial symbol table.  */
> diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
> index 37023a20709..e3131693b81 100644
> --- a/gdb/dwarf2/read.h
> +++ b/gdb/dwarf2/read.h
> @@ -866,6 +866,10 @@ struct dwarf2_base_index_functions : public quick_symbol_functions
>       CORE_ADDR pc, struct obj_section *section, int warn_if_readin)
>         override final;
>  
> +  struct compunit_symtab *do_find_pc_sect_compunit_symtab
> +    (struct objfile *objfile, struct bound_minimal_symbol msymbol,
> +     CORE_ADDR pc, struct obj_section *section, int warn_if_readin);
> +
>    struct compunit_symtab *find_compunit_symtab_by_address
>      (struct objfile *objfile, CORE_ADDR address) override
>    {
> @@ -942,4 +946,10 @@ extern bool read_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
>  				       dwarf2_section_info *section,
>  				       addrmap *mutable_map);
>  
> +/* If OBJFILE contains information from a separately downloaded .gdb_index,
> +   attempt to download the full debuginfo.  */
> +
> +extern void read_full_dwarf_from_debuginfod (struct objfile *,
> +					     dwarf2_base_index_functions *);
> +
>  #endif /* DWARF2READ_H */
> diff --git a/gdb/dwarf2/section.c b/gdb/dwarf2/section.c
> index c9ef41893ee..8cb09e3381a 100644
> --- a/gdb/dwarf2/section.c
> +++ b/gdb/dwarf2/section.c
> @@ -54,7 +54,8 @@ dwarf2_section_info::get_bfd_owner () const
>        section = get_containing_section ();
>        gdb_assert (!section->is_virtual);
>      }
> -  gdb_assert (section->s.section != nullptr);
> +  if (section->s.section == nullptr)
> +    error (_("Can't find owner of DWARF section."));
>    return section->s.section->owner;
>  }
>  
> diff --git a/gdb/elfread.c b/gdb/elfread.c
> index 0305bf21894..01eccfaac36 100644
> --- a/gdb/elfread.c
> +++ b/gdb/elfread.c
> @@ -1239,7 +1239,7 @@ elf_symfile_read_dwarf2 (struct objfile *objfile,
>  	    symbol_file_add_separate (debug_bfd, debugfile.c_str (),
>  				      symfile_flags, objfile);
>  	}
> -      else
> +      else if (!dwarf2_has_separate_index (objfile))
>  	{
>  	  has_dwarf2 = false;
>  	  const struct bfd_build_id *build_id
> diff --git a/gdb/objfile-flags.h b/gdb/objfile-flags.h
> index 9dee2ee51a0..fb3f741c899 100644
> --- a/gdb/objfile-flags.h
> +++ b/gdb/objfile-flags.h
> @@ -60,6 +60,10 @@ enum objfile_flag : unsigned
>      /* User requested that we do not read this objfile's symbolic
>         information.  */
>      OBJF_READNEVER = 1 << 6,
> +
> +    /* A separate .gdb_index has been downloaded for this objfile.
> +       Debuginfo for this objfile can be downloaded when required.  */
> +    OBJF_DOWNLOAD_DEFERRED = 1 << 7,
>    };
>  
>  DEF_ENUM_FLAGS_TYPE (enum objfile_flag, objfile_flags);
> diff --git a/gdb/objfiles.c b/gdb/objfiles.c
> index 9caebfefd59..7f32037caa5 100644
> --- a/gdb/objfiles.c
> +++ b/gdb/objfiles.c
> @@ -52,6 +52,7 @@
>  #include "gdb_bfd.h"
>  #include "btrace.h"
>  #include "gdbsupport/pathstuff.h"
> +#include "symfile.h"
>  
>  #include <algorithm>
>  #include <vector>
> diff --git a/gdb/objfiles.h b/gdb/objfiles.h
> index 342aa09ac6a..d00c2b21933 100644
> --- a/gdb/objfiles.h
> +++ b/gdb/objfiles.h
> @@ -587,6 +587,17 @@ struct objfile
>    /* See quick_symbol_functions.  */
>    void require_partial_symbols (bool verbose);
>  
> +  /* Remove TARGET from this objfile's collection of quick_symbol_functions.  */
> +  void remove_partial_symbol (quick_symbol_functions *target)
> +  {
> +    for (quick_symbol_functions_up &qf_up : qf)
> +      if (qf_up.get () == target)
> +	{
> +	  qf.remove (qf_up);
> +	  return;
> +	}
> +  }
> +
>    /* Return the relocation offset applied to SECTION.  */
>    CORE_ADDR section_offset (bfd_section *section) const
>    {
> @@ -611,13 +622,20 @@ struct objfile
>  
>  private:
>  
> +  using qf_list = std::forward_list<quick_symbol_functions_up>;
> +  using unwrapping_qf_range = iterator_range<unwrapping_iterator<qf_list::iterator>>;
> +  using qf_safe_range = basic_safe_range<unwrapping_qf_range>;
> +
>    /* Ensure that partial symbols have been read and return the "quick" (aka
>       partial) symbol functions for this symbol reader.  */
> -  const std::forward_list<quick_symbol_functions_up> &
> +  qf_safe_range
>    qf_require_partial_symbols ()
>    {
>      this->require_partial_symbols (true);
> -    return qf;
> +    return qf_safe_range
> +      (unwrapping_qf_range
> +	(unwrapping_iterator<qf_list::iterator> (qf.begin ()),
> +	 unwrapping_iterator<qf_list::iterator> (qf.end ())));
>    }
>  
>  public:
> diff --git a/gdb/progspace.c b/gdb/progspace.c
> index 32bdfebcf7c..1ed75eef2f9 100644
> --- a/gdb/progspace.c
> +++ b/gdb/progspace.c
> @@ -139,19 +139,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));
>      }
>  }
>  
> diff --git a/gdb/progspace.h b/gdb/progspace.h
> index 85215f0e2f1..65f88e2d0a5 100644
> --- a/gdb/progspace.h
> +++ b/gdb/progspace.h
> @@ -40,42 +40,40 @@ struct address_space;
>  struct program_space;
>  struct so_list;
>  
> -typedef std::list<std::unique_ptr<objfile>> objfile_list;
> -
> -/* An iterator that wraps an iterator over std::unique_ptr<objfile>,
> -   and dereferences the returned object.  This is useful for iterating
> -   over a list of shared pointers and returning raw pointers -- which
> -   helped avoid touching a lot of code when changing how objfiles are
> -   managed.  */
> +/* An iterator that wraps an iterator over std::unique_ptr, and dereferences
> +   the returned object.  This is useful for iterating over a list of shared
> +   pointers and returning raw pointers -- which helped avoid touching a lot
> +   of code when changing how objfiles are managed.  */
>  
> -class unwrapping_objfile_iterator
> +template<typename UniquePtrIter>
> +class unwrapping_iterator
>  {
>  public:
>  
> -  typedef unwrapping_objfile_iterator self_type;
> -  typedef typename ::objfile *value_type;
> -  typedef typename ::objfile &reference;
> -  typedef typename ::objfile **pointer;
> -  typedef typename objfile_list::iterator::iterator_category iterator_category;
> -  typedef typename objfile_list::iterator::difference_type difference_type;
> +  typedef unwrapping_iterator self_type;
> +  typedef typename UniquePtrIter::value_type::pointer value_type;
> +  typedef typename UniquePtrIter::reference  reference;
> +  typedef typename UniquePtrIter::pointer pointer;
> +  typedef typename UniquePtrIter::iterator_category iterator_category;
> +  typedef typename UniquePtrIter::difference_type difference_type;
>  
> -  unwrapping_objfile_iterator (objfile_list::iterator iter)
> +  unwrapping_iterator (UniquePtrIter iter)
>      : m_iter (std::move (iter))
>    {
>    }
>  
> -  objfile *operator* () const
> +  value_type operator* () const
>    {
>      return m_iter->get ();
>    }
>  
> -  unwrapping_objfile_iterator operator++ ()
> +  unwrapping_iterator operator++ ()
>    {
>      ++m_iter;
>      return *this;
>    }
>  
> -  bool operator!= (const unwrapping_objfile_iterator &other) const
> +  bool operator!= (const unwrapping_iterator &other) const
>    {
>      return m_iter != other.m_iter;
>    }
> @@ -83,13 +81,59 @@ class unwrapping_objfile_iterator
>  private:
>  
>    /* The underlying iterator.  */
> -  objfile_list::iterator m_iter;
> +  UniquePtrIter m_iter;
>  };
>  
> +/* An reverse iterator that wraps an iterator over std::unique_ptr, and
> +   dereferences the returned object.  This is useful for iterating over
> +   a list of shared pointers and returning raw pointers -- which helped
> +   avoid touching a lot of code when changing how objfiles are managed.  */
> +
> +template<typename UniquePtrIter>
> +class unwrapping_reverse_iterator
> +{
> +public:
> +
> +  typedef unwrapping_reverse_iterator self_type;
> +  typedef typename UniquePtrIter::value_type::pointer value_type;
> +  typedef typename UniquePtrIter::reference  reference;
> +  typedef typename UniquePtrIter::pointer pointer;
> +  typedef typename UniquePtrIter::iterator_category iterator_category;
> +  typedef typename UniquePtrIter::difference_type difference_type;
> +
> +  unwrapping_reverse_iterator (UniquePtrIter iter)
> +    : m_iter (std::move (iter))
> +  {
> +  }
>  
> -/* A range that returns unwrapping_objfile_iterators.  */
> +  value_type operator* () const
> +  {
> +    return m_iter->get ();
> +  }
>  
> -using unwrapping_objfile_range = iterator_range<unwrapping_objfile_iterator>;
> +  unwrapping_reverse_iterator operator++ ()
> +  {
> +    --m_iter;
> +    return *this;
> +  }
> +
> +  bool operator!= (const unwrapping_reverse_iterator &other) const
> +  {
> +    return m_iter != other.m_iter;
> +  }
> +
> +private:
> +
> +  /* The underlying iterator.  */
> +  UniquePtrIter m_iter;
> +};
> +
> +typedef std::list<std::unique_ptr<objfile>> objfile_list;
> +
> +/* A range that returns unwrapping_iterators.  */
> +
> +using unwrapping_objfile_range
> +  = iterator_range<unwrapping_iterator<objfile_list::iterator>>;
>  
>  /* A program space represents a symbolic view of an address space.
>     Roughly speaking, it holds all the data associated with a
> @@ -209,11 +253,14 @@ struct program_space
>    objfiles_range objfiles ()
>    {
>      return objfiles_range
> -      (unwrapping_objfile_iterator (objfiles_list.begin ()),
> -       unwrapping_objfile_iterator (objfiles_list.end ()));
> +      (unwrapping_iterator<objfile_list::iterator> (objfiles_list.begin ()),
> +       unwrapping_iterator<objfile_list::iterator> (objfiles_list.end ()));
>    }
>  
> -  using objfiles_safe_range = basic_safe_range<objfiles_range>;
> +  using objfile_reverse_range = reverse_iterator_range
> +    <unwrapping_reverse_iterator<objfile_list::iterator>>;
> +
> +  using objfiles_safe_reverse_range = basic_safe_range<objfile_reverse_range>;
>  
>    /* An iterable object that can be used to iterate over all objfiles.
>       The basic use is in a foreach, like:
> @@ -221,20 +268,27 @@ struct program_space
>       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 ()
> +     deleted during iteration.
> +
> +     The use of a reverse iterator helps ensure that separate debug
> +     objfiles are deleted before their parent objfile.  This prevents
> +     the invalidation of an iterator 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
> +      (objfile_reverse_range
> +	(unwrapping_reverse_iterator<objfile_list::iterator>
> +	  (objfiles_list.begin ()),
> +	 unwrapping_reverse_iterator<objfile_list::iterator>
> +	  (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);
> diff --git a/gdb/symfile-debug.c b/gdb/symfile-debug.c
> index 9db5c47a8ce..784b81b5ca6 100644
> --- a/gdb/symfile-debug.c
> +++ b/gdb/symfile-debug.c
> @@ -109,9 +109,9 @@ objfile::has_unexpanded_symtabs ()
>  		objfile_debug_name (this));
>  
>    bool result = false;
> -  for (const auto &iter : qf_require_partial_symbols ())
> +  for (quick_symbol_functions *qf : qf_require_partial_symbols ())
>      {
> -      if (iter->has_unexpanded_symtabs (this))
> +      if (qf->has_unexpanded_symtabs (this))
>  	{
>  	  result = true;
>  	  break;
> @@ -134,9 +134,9 @@ 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_require_partial_symbols ())
> +  for (quick_symbol_functions *qf : qf_require_partial_symbols ())
>      {
> -      retval = iter->find_last_source_symtab (this);
> +      retval = qf->find_last_source_symtab (this);
>        if (retval != nullptr)
>  	break;
>      }
> @@ -167,8 +167,8 @@ objfile::forget_cached_source_info ()
>  	}
>      }
>  
> -  for (const auto &iter : qf_require_partial_symbols ())
> -    iter->forget_cached_source_info (this);
> +  for (quick_symbol_functions *qf : qf_require_partial_symbols ())
> +    qf->forget_cached_source_info (this);
>  }
>  
>  bool
> @@ -214,17 +214,17 @@ objfile::map_symtabs_matching_filename
>      return result;
>    };
>  
> -  for (const auto &iter : qf_require_partial_symbols ())
> +  for (quick_symbol_functions *qf : qf_require_partial_symbols ())
>      {
> -      if (!iter->expand_symtabs_matching (this,
> -					  match_one_filename,
> -					  nullptr,
> -					  nullptr,
> -					  on_expansion,
> -					  (SEARCH_GLOBAL_BLOCK
> -					   | SEARCH_STATIC_BLOCK),
> -					  UNDEF_DOMAIN,
> -					  ALL_DOMAIN))
> +      if (!qf->expand_symtabs_matching (this,
> +					match_one_filename,
> +					nullptr,
> +					nullptr,
> +					on_expansion,
> +					(SEARCH_GLOBAL_BLOCK
> +					 | SEARCH_STATIC_BLOCK),
> +					UNDEF_DOMAIN,
> +					ALL_DOMAIN))
>  	{
>  	  retval = false;
>  	  break;
> @@ -283,18 +283,18 @@ objfile::lookup_symbol (block_enum kind, const char *name, domain_enum domain)
>      return true;
>    };
>  
> -  for (const auto &iter : qf_require_partial_symbols ())
> +  for (quick_symbol_functions *qf : qf_require_partial_symbols ())
>      {
> -      if (!iter->expand_symtabs_matching (this,
> -					  nullptr,
> -					  &lookup_name,
> -					  nullptr,
> -					  search_one_symtab,
> -					  kind == GLOBAL_BLOCK
> -					  ? SEARCH_GLOBAL_BLOCK
> -					  : SEARCH_STATIC_BLOCK,
> -					  domain,
> -					  ALL_DOMAIN))
> +      if (!qf->expand_symtabs_matching (this,
> +					nullptr,
> +					&lookup_name,
> +					nullptr,
> +					search_one_symtab,
> +					kind == GLOBAL_BLOCK
> +					? SEARCH_GLOBAL_BLOCK
> +					: SEARCH_STATIC_BLOCK,
> +					domain,
> +					ALL_DOMAIN))
>  	break;
>      }
>  
> @@ -314,8 +314,8 @@ 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_require_partial_symbols ())
> -    iter->print_stats (this, print_bcache);
> +  for (quick_symbol_functions *qf : qf_require_partial_symbols ())
> +    qf->print_stats (this, print_bcache);
>  }
>  
>  void
> @@ -340,16 +340,16 @@ 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_require_partial_symbols ())
> -    iter->expand_symtabs_matching (this,
> -				   nullptr,
> -				   &lookup_name,
> -				   nullptr,
> -				   nullptr,
> -				   (SEARCH_GLOBAL_BLOCK
> -				    | SEARCH_STATIC_BLOCK),
> -				   VAR_DOMAIN,
> -				   ALL_DOMAIN);
> +  for (quick_symbol_functions *qf : qf_require_partial_symbols ())
> +    qf->expand_symtabs_matching (this,
> +				 nullptr,
> +				 &lookup_name,
> +				 nullptr,
> +				 nullptr,
> +				 (SEARCH_GLOBAL_BLOCK
> +				  | SEARCH_STATIC_BLOCK),
> +				 VAR_DOMAIN,
> +				 ALL_DOMAIN);
>  }
>  
>  void
> @@ -359,8 +359,8 @@ objfile::expand_all_symtabs ()
>      gdb_printf (gdb_stdlog, "qf->expand_all_symtabs (%s)\n",
>  		objfile_debug_name (this));
>  
> -  for (const auto &iter : qf_require_partial_symbols ())
> -    iter->expand_all_symtabs (this);
> +  for (quick_symbol_functions *qf : qf_require_partial_symbols ())
> +    qf->expand_all_symtabs (this);
>  }
>  
>  void
> @@ -377,16 +377,16 @@ objfile::expand_symtabs_with_fullname (const char *fullname)
>      return filename_cmp (basenames ? basename : fullname, filename) == 0;
>    };
>  
> -  for (const auto &iter : qf_require_partial_symbols ())
> -    iter->expand_symtabs_matching (this,
> -				   file_matcher,
> -				   nullptr,
> -				   nullptr,
> -				   nullptr,
> -				   (SEARCH_GLOBAL_BLOCK
> -				    | SEARCH_STATIC_BLOCK),
> -				   UNDEF_DOMAIN,
> -				   ALL_DOMAIN);
> +  for (quick_symbol_functions *qf : qf_require_partial_symbols ())
> +    qf->expand_symtabs_matching (this,
> +				 file_matcher,
> +				 nullptr,
> +				 nullptr,
> +				 nullptr,
> +				 (SEARCH_GLOBAL_BLOCK
> +				  | SEARCH_STATIC_BLOCK),
> +				 UNDEF_DOMAIN,
> +				 ALL_DOMAIN);
>  }
>  
>  void
> @@ -402,9 +402,9 @@ objfile::expand_matching_symbols
>  		domain_name (domain), global,
>  		host_address_to_string (ordered_compare));
>  
> -  for (const auto &iter : qf_require_partial_symbols ())
> -    iter->expand_matching_symbols (this, name, domain, global,
> -				   ordered_compare);
> +  for (quick_symbol_functions *qf : qf_require_partial_symbols ())
> +    qf->expand_matching_symbols (this, name, domain, global,
> +				 ordered_compare);
>  }
>  
>  bool
> @@ -429,10 +429,10 @@ objfile::expand_symtabs_matching
>  		host_address_to_string (&expansion_notify),
>  		search_domain_name (kind));
>  
> -  for (const auto &iter : qf_require_partial_symbols ())
> -    if (!iter->expand_symtabs_matching (this, file_matcher, lookup_name,
> -					symbol_matcher, expansion_notify,
> -					search_flags, domain, kind))
> +  for (quick_symbol_functions *qf : qf_require_partial_symbols ())
> +    if (!qf->expand_symtabs_matching (this, file_matcher, lookup_name,
> +				      symbol_matcher, expansion_notify,
> +				      search_flags, domain, kind))
>        return false;
>    return true;
>  }
> @@ -454,10 +454,10 @@ objfile::find_pc_sect_compunit_symtab (struct bound_minimal_symbol msymbol,
>  		host_address_to_string (section),
>  		warn_if_readin);
>  
> -  for (const auto &iter : qf_require_partial_symbols ())
> +  for (quick_symbol_functions *qf : qf_require_partial_symbols ())
>      {
> -      retval = iter->find_pc_sect_compunit_symtab (this, msymbol, pc, section,
> -						   warn_if_readin);
> +      retval = qf->find_pc_sect_compunit_symtab (this, msymbol, pc, section,
> +						 warn_if_readin);
>        if (retval != nullptr)
>  	break;
>      }
> @@ -482,8 +482,8 @@ objfile::map_symbol_filenames (gdb::function_view<symbol_filename_ftype> fun,
>  		objfile_debug_name (this),
>  		need_fullname);
>  
> -  for (const auto &iter : qf_require_partial_symbols ())
> -    iter->map_symbol_filenames (this, fun, need_fullname);
> +  for (quick_symbol_functions *qf : qf_require_partial_symbols ())
> +    qf->map_symbol_filenames (this, fun, need_fullname);
>  }
>  
>  struct compunit_symtab *
> @@ -496,9 +496,9 @@ objfile::find_compunit_symtab_by_address (CORE_ADDR address)
>  		hex_string (address));
>  
>    struct compunit_symtab *result = NULL;
> -  for (const auto &iter : qf_require_partial_symbols ())
> +  for (quick_symbol_functions *qf : qf_require_partial_symbols ())
>      {
> -      result = iter->find_compunit_symtab_by_address (this, address);
> +      result = qf->find_compunit_symtab_by_address (this, address);
>        if (result != nullptr)
>  	break;
>      }
> @@ -521,10 +521,10 @@ objfile::lookup_global_symbol_language (const char *name,
>    enum language result = language_unknown;
>    *symbol_found_p = false;
>  
> -  for (const auto &iter : qf_require_partial_symbols ())
> +  for (quick_symbol_functions *qf : qf_require_partial_symbols ())
>      {
> -      result = iter->lookup_global_symbol_language (this, name, domain,
> -						    symbol_found_p);
> +      result = qf->lookup_global_symbol_language (this, name, domain,
> +						  symbol_found_p);
>        if (*symbol_found_p)
>  	break;
>      }
> diff --git a/gdb/symfile.c b/gdb/symfile.c
> index 8ae2177b159..9c943c4d5be 100644
> --- a/gdb/symfile.c
> +++ b/gdb/symfile.c
> @@ -992,6 +992,10 @@ syms_from_objfile (struct objfile *objfile,
>  static void
>  finish_new_objfile (struct objfile *objfile, symfile_add_flags add_flags)
>  {
> +  struct objfile *parent = objfile->separate_debug_objfile_backlink;
> +  bool was_deferred
> +    = (parent != nullptr) && (parent->flags & OBJF_DOWNLOAD_DEFERRED);
> +
>    /* If this is the main symbol file we have to clean up all users of the
>       old main symbol file.  Otherwise it is sufficient to fixup all the
>       breakpoints that may have been redefined by this symbol file.  */
> @@ -1002,7 +1006,8 @@ finish_new_objfile (struct objfile *objfile, symfile_add_flags add_flags)
>  
>        clear_symtab_users (add_flags);
>      }
> -  else if ((add_flags & SYMFILE_DEFER_BP_RESET) == 0)
> +  else if ((add_flags & SYMFILE_DEFER_BP_RESET) == 0
> +	   && !was_deferred)
>      {
>        breakpoint_re_set ();
>      }
> 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/iterator-range.h b/gdbsupport/iterator-range.h
> index e934f5ebf01..853ebb9074a 100644
> --- a/gdbsupport/iterator-range.h
> +++ b/gdbsupport/iterator-range.h
> @@ -61,4 +61,49 @@ struct iterator_range
>    IteratorType m_begin, m_end;
>  };
>  
> +/* A wrapper that allows using ranged for-loops on a range described
> +   by two reverse iterators.  */
> +
> +template <typename IteratorType>
> +struct reverse_iterator_range
> +{
> +  using iterator = IteratorType;
> +
> +  /* Create a reverse iterator range using explicit BEGIN and END iterators.  */
> +  template <typename... Args>
> +  reverse_iterator_range (IteratorType begin, IteratorType end)
> +    : m_begin (std::move (end)), m_end (std::move (begin))
> +  {
> +    if (m_begin != m_end)
> +      {
> +	/* M_BEGIN is the base iterator's one-past-the-end.  Increment it
> +	   so it points to the ranges's last element.  */
> +	++m_begin;
> +
> +	/* M_END points to the range's first element.  Increment it so it's
> +	   one-past-the-end.  */
> +	++m_end;
> +      }
> +  }
> +
> +  /* Need these as the variadic constructor would be a better match
> +     otherwise.  */
> +  reverse_iterator_range (reverse_iterator_range &) = default;
> +  reverse_iterator_range (const reverse_iterator_range &) = default;
> +  reverse_iterator_range (reverse_iterator_range &&) = default;
> +
> +  IteratorType begin () const
> +  { return m_begin; }
> +
> +  IteratorType end () const
> +  { return m_end; }
> +
> +  /* The number of items in this reverse_iterator_range.  */
> +  std::size_t size () const
> +  { return std::distance (m_end, m_begin); }
> +
> +private:
> +  IteratorType m_begin, m_end;
> +};
> +
>  #endif /* GDBSUPPORT_ITERATOR_RANGE_H */
> -- 
> 2.39.2