[PATCH 00/13] Use gdb_bfd_sections in more places

[PATCH 00/13] Use gdb_bfd_sections in more places

[Tom Tromey]

As promised, here are some patches to use gdb_bfd_sections in more
places.

The main motivation for this patch is that ordinary iteration is
easier to understand than callback functions.  Often, it is also
shorter -- some patches in this series remove helper functions or
helper data structures.  Finally, iteration is more type-safe (at
least without additional wrappers to pass function-views through
bfd_map_over_sections).

I did not convert every call to bfd_map_over_sections.  Some of the
remaining calls are not easy to test.  For the calls in compile/, I
have another series that makes many changes in that code, and so I
didn't want to introduce clashes at this time.  (I don't recall
offhand if that series fixes this, but I can revisit when it lands.)

As usual with these cleanups, I don't plan to commit this until the
release branch is made.

Let me know what you think.

Tom

[PATCH 01/13] Add a new overload of gdb_bfd_sections

[Tom Tromey]

This adds a new overload of gdb_bfd_sections, that accepts a
gdb_bfd_ref_ptr.  This also fixes the formatting of the existing
function, since I happened to notice it was mildly off.

gdb/ChangeLog
2020-08-28  Tom Tromey  <tom@tromey.com>

	* gdb_bfd.h (gdb_bfd_sections): New overload.  Fix formatting of
	existing function.
---
 gdb/ChangeLog |  5 +++++
 gdb/gdb_bfd.h | 10 ++++++++--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/gdb/gdb_bfd.h b/gdb/gdb_bfd.h
index 0f450567d50..12330970873 100644
--- a/gdb/gdb_bfd.h
+++ b/gdb/gdb_bfd.h
@@ -205,10 +205,16 @@ bool gdb_bfd_get_full_section_contents (bfd *abfd, asection *section,
 using gdb_bfd_section_iterator = next_iterator<asection>;
 using gdb_bfd_section_range = next_adapter<asection, gdb_bfd_section_iterator>;
 
-static inline
-gdb_bfd_section_range gdb_bfd_sections (bfd *abfd)
+static inline gdb_bfd_section_range
+gdb_bfd_sections (bfd *abfd)
 {
   return gdb_bfd_section_range (abfd->sections);
 }
 
+static inline gdb_bfd_section_range
+gdb_bfd_sections (const gdb_bfd_ref_ptr &abfd)
+{
+  return gdb_bfd_section_range (abfd->sections);
+};
+
 #endif /* GDB_BFD_H */
-- 
2.17.2

[PATCH 02/13] Use gdb_bfd_sections in core_target_open

[Tom Tromey]

This changes core_target_open to avoid bfd_map_over_sections, in favor
of iteration.

gdb/ChangeLog
2020-08-28  Tom Tromey  <tom@tromey.com>

	* corelow.c (add_to_thread_list): Change parameters.
	(core_target_open): Use foreach.
---
 gdb/ChangeLog |  5 +++++
 gdb/corelow.c | 10 ++++------
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/gdb/corelow.c b/gdb/corelow.c
index b6ee219f57c..1a07fcd0333 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -270,8 +270,6 @@ core_target::build_file_mappings ()
       });
 }
 
-static void add_to_thread_list (bfd *, asection *, void *);
-
 /* An arbitrary identifier for the core inferior.  */
 #define CORELOW_PID 1
 
@@ -302,11 +300,10 @@ core_target::close ()
    extract the list of threads in a core file.  */
 
 static void
-add_to_thread_list (bfd *abfd, asection *asect, void *reg_sect_arg)
+add_to_thread_list (asection *asect, asection *reg_sect)
 {
   int core_tid;
   int pid, lwpid;
-  asection *reg_sect = (asection *) reg_sect_arg;
   bool fake_pid_p = false;
   struct inferior *inf;
 
@@ -473,8 +470,9 @@ core_target_open (const char *arg, int from_tty)
   /* Build up thread list from BFD sections, and possibly set the
      current thread to the .reg/NN section matching the .reg
      section.  */
-  bfd_map_over_sections (core_bfd, add_to_thread_list,
-			 bfd_get_section_by_name (core_bfd, ".reg"));
+  asection *reg_sect = bfd_get_section_by_name (core_bfd, ".reg");
+  for (asection *sect : gdb_bfd_sections (core_bfd))
+    add_to_thread_list (sect, reg_sect);
 
   if (inferior_ptid == null_ptid)
     {
-- 
2.17.2

[PATCH 03/13] Use gdb_bfd_sections in gdb_bfd_close_or_warn

[Tom Tromey]

This changes gdb_bfd_close_or_warn to avoid bfd_map_over_sections, in
favor of iteration.

gdb/ChangeLog
2020-08-28  Tom Tromey  <tom@tromey.com>

	* gdb_bfd.c (free_one_bfd_section): Remove 'abfd' and 'ignore'
	parameters.
	(gdb_bfd_close_or_warn): Use foreach.
---
 gdb/ChangeLog | 6 ++++++
 gdb/gdb_bfd.c | 5 +++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index 25e0178a8b8..43b576f12a0 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -484,7 +484,7 @@ gdb_bfd_open (const char *name, const char *target, int fd,
    BFD.  */
 
 static void
-free_one_bfd_section (bfd *abfd, asection *sectp, void *ignore)
+free_one_bfd_section (asection *sectp)
 {
   struct gdb_bfd_section_data *sect
     = (struct gdb_bfd_section_data *) bfd_section_userdata (sectp);
@@ -513,7 +513,8 @@ gdb_bfd_close_or_warn (struct bfd *abfd)
   int ret;
   const char *name = bfd_get_filename (abfd);
 
-  bfd_map_over_sections (abfd, free_one_bfd_section, NULL);
+  for (asection *sect : gdb_bfd_sections (abfd))
+    free_one_bfd_section (sect);
 
   ret = bfd_close (abfd);
 
-- 
2.17.2

[PATCH 04/13] Use gdb_bfd_sections in get_stap_base_address

[Tom Tromey]

This changes get_stap_base_address to avoid bfd_map_over_sections, in
favor of iteration.

gdb/ChangeLog
2020-08-28  Tom Tromey  <tom@tromey.com>

	* stap-probe.c (get_stap_base_address_1): Remove.
	(get_stap_base_address): Use foreach.
---
 gdb/ChangeLog    |  5 +++++
 gdb/stap-probe.c | 20 ++++++--------------
 2 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c
index 73596446cce..fb6010d14ff 100644
--- a/gdb/stap-probe.c
+++ b/gdb/stap-probe.c
@@ -1578,19 +1578,6 @@ handle_stap_probe (struct objfile *objfile, struct sdt_note *el,
   probesp->emplace_back (ret);
 }
 
-/* Helper function which tries to find the base address of the SystemTap
-   base section named STAP_BASE_SECTION_NAME.  */
-
-static void
-get_stap_base_address_1 (bfd *abfd, asection *sect, void *obj)
-{
-  asection **ret = (asection **) obj;
-
-  if ((sect->flags & (SEC_DATA | SEC_ALLOC | SEC_HAS_CONTENTS))
-      && sect->name && !strcmp (sect->name, STAP_BASE_SECTION_NAME))
-    *ret = sect;
-}
-
 /* Helper function which iterates over every section in the BFD file,
    trying to find the base address of the SystemTap base section.
    Returns 1 if found (setting BASE to the proper value), zero otherwise.  */
@@ -1600,7 +1587,12 @@ get_stap_base_address (bfd *obfd, bfd_vma *base)
 {
   asection *ret = NULL;
 
-  bfd_map_over_sections (obfd, get_stap_base_address_1, (void *) &ret);
+  for (asection *sect : gdb_bfd_sections (obfd))
+    {
+      if ((sect->flags & (SEC_DATA | SEC_ALLOC | SEC_HAS_CONTENTS))
+	  && sect->name && !strcmp (sect->name, STAP_BASE_SECTION_NAME))
+	ret = sect;
+    }
 
   if (ret == NULL)
     {
-- 
2.17.2

[PATCH 05/13] Use gdb_bfd_sections in build_objfile_section_table

[Tom Tromey]

This changes build_objfile_section_table to avoid
bfd_map_over_sections, in favor of iteration.

gdb/ChangeLog
2020-08-28  Tom Tromey  <tom@tromey.com>

	* objfiles.c (add_to_objfile_sections): Rename from
	add_to_objfile_sections_full.
	(add_to_objfile_sections): Remove.
	(build_objfile_section_table): Use foreach.
---
 gdb/ChangeLog  |  7 +++++++
 gdb/objfiles.c | 29 +++++++++++------------------
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index 3aa7973e0d5..471a5c4c393 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -257,13 +257,13 @@ objfile_lookup_static_link (struct objfile *objfile,
 
 \f
 
-/* Called via bfd_map_over_sections to build up the section table that
-   the objfile references.  The objfile contains pointers to the start
-   of the table (objfile->sections) and to the first location after
-   the end of the table (objfile->sections_end).  */
+/* Build up the section table that the objfile references.  The
+   objfile contains pointers to the start of the table
+   (objfile->sections) and to the first location after the end of the
+   table (objfile->sections_end).  */
 
 static void
-add_to_objfile_sections_full (struct bfd *abfd, struct bfd_section *asect,
+add_to_objfile_sections (struct bfd *abfd, struct bfd_section *asect,
 			      struct objfile *objfile, int force)
 {
   struct obj_section *section;
@@ -283,13 +283,6 @@ add_to_objfile_sections_full (struct bfd *abfd, struct bfd_section *asect,
   section->ovly_mapped = 0;
 }
 
-static void
-add_to_objfile_sections (struct bfd *abfd, struct bfd_section *asect,
-			 void *objfilep)
-{
-  add_to_objfile_sections_full (abfd, asect, (struct objfile *) objfilep, 0);
-}
-
 /* Builds a section table for OBJFILE.
 
    Note that the OFFSET and OVLY_MAPPED in each table entry are
@@ -304,14 +297,14 @@ build_objfile_section_table (struct objfile *objfile)
 				      count,
 				      struct obj_section);
   objfile->sections_end = (objfile->sections + count);
-  bfd_map_over_sections (objfile->obfd,
-			 add_to_objfile_sections, (void *) objfile);
+  for (asection *sect : gdb_bfd_sections (objfile->obfd))
+    add_to_objfile_sections (objfile->obfd, sect, objfile, 0);
 
   /* See gdb_bfd_section_index.  */
-  add_to_objfile_sections_full (objfile->obfd, bfd_com_section_ptr, objfile, 1);
-  add_to_objfile_sections_full (objfile->obfd, bfd_und_section_ptr, objfile, 1);
-  add_to_objfile_sections_full (objfile->obfd, bfd_abs_section_ptr, objfile, 1);
-  add_to_objfile_sections_full (objfile->obfd, bfd_ind_section_ptr, objfile, 1);
+  add_to_objfile_sections (objfile->obfd, bfd_com_section_ptr, objfile, 1);
+  add_to_objfile_sections (objfile->obfd, bfd_und_section_ptr, objfile, 1);
+  add_to_objfile_sections (objfile->obfd, bfd_abs_section_ptr, objfile, 1);
+  add_to_objfile_sections (objfile->obfd, bfd_ind_section_ptr, objfile, 1);
 }
 
 /* Given a pointer to an initialized bfd (ABFD) and some flag bits,
-- 
2.17.2

[PATCH 06/13] Use gdb_bfd_sections in symfile.c

[Tom Tromey]

This changes some functions in symfile.c to avoid
bfd_map_over_sections, in favor of iteration.  Some helper types can
also be removed due to this change.

gdb/ChangeLog
2020-08-28  Tom Tromey  <tom@tromey.com>

	* symfile.h: (find_lowest_section): Don't declare.
	* symfile.c (find_lowest_section): Now static.  Change
	parameters.
	(struct place_section_arg): Remove.
	(place_section): Change parameters.
	(addr_info_make_relative): Use foreach.
	(symfile_dummy_outputs): Remove.
	(default_symfile_relocate): Use foreach.
---
 gdb/ChangeLog | 11 +++++++++++
 gdb/symfile.c | 53 +++++++++++++++++----------------------------------
 gdb/symfile.h |  2 --
 3 files changed, 28 insertions(+), 38 deletions(-)

diff --git a/gdb/symfile.c b/gdb/symfile.c
index 2c38ce4431a..96d8b3f0b52 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -187,7 +187,6 @@ increment_reading_symtab (void)
 }
 
 /* Remember the lowest-addressed loadable section we've seen.
-   This function is called via bfd_map_over_sections.
 
    In case of equal vmas, the section with the largest size becomes the
    lowest-addressed loadable section.
@@ -195,11 +194,9 @@ increment_reading_symtab (void)
    If the vmas and sizes are equal, the last section is considered the
    lowest-addressed loadable section.  */
 
-void
-find_lowest_section (bfd *abfd, asection *sect, void *obj)
+static void
+find_lowest_section (asection *sect, asection **lowest)
 {
-  asection **lowest = (asection **) obj;
-
   if (0 == (bfd_section_flags (sect) & (SEC_ALLOC | SEC_LOAD)))
     return;
   if (!*lowest)
@@ -335,22 +332,13 @@ init_objfile_sect_indices (struct objfile *objfile)
     }
 }
 
-/* The arguments to place_section.  */
-
-struct place_section_arg
-{
-  section_offsets *offsets;
-  CORE_ADDR lowest;
-};
-
 /* Find a unique offset to use for loadable section SECT if
    the user did not provide an offset.  */
 
 static void
-place_section (bfd *abfd, asection *sect, void *obj)
+place_section (bfd *abfd, asection *sect, section_offsets &offsets,
+	       CORE_ADDR &lowest)
 {
-  struct place_section_arg *arg = (struct place_section_arg *) obj;
-  section_offsets &offsets = *arg->offsets;
   CORE_ADDR start_addr;
   int done;
   ULONGEST align = ((ULONGEST) 1) << bfd_section_alignment (sect);
@@ -364,7 +352,7 @@ place_section (bfd *abfd, asection *sect, void *obj)
     return;
 
   /* Otherwise, let's try to find a place for the section.  */
-  start_addr = (arg->lowest + align - 1) & -align;
+  start_addr = (lowest + align - 1) & -align;
 
   do {
     asection *cur_sec;
@@ -405,7 +393,7 @@ place_section (bfd *abfd, asection *sect, void *obj)
   while (!done);
 
   offsets[gdb_bfd_section_index (abfd, sect)] = start_addr;
-  arg->lowest = start_addr + bfd_section_size (sect);
+  lowest = start_addr + bfd_section_size (sect);
 }
 
 /* Store section_addr_info as prepared (made relative and with SECTINDEX
@@ -500,7 +488,8 @@ addr_info_make_relative (section_addr_info *addrs, bfd *abfd)
   /* Find lowest loadable section to be used as starting point for
      contiguous sections.  */
   lower_sect = NULL;
-  bfd_map_over_sections (abfd, find_lowest_section, &lower_sect);
+  for (asection *iter : gdb_bfd_sections (abfd))
+    find_lowest_section (iter, &lower_sect);
   if (lower_sect == NULL)
     {
       warning (_("no loadable sections found in added symbol-file %s"),
@@ -645,7 +634,6 @@ default_symfile_offsets (struct objfile *objfile,
      small.  */
   if ((bfd_get_file_flags (objfile->obfd) & (EXEC_P | DYNAMIC)) == 0)
     {
-      struct place_section_arg arg;
       bfd *abfd = objfile->obfd;
       asection *cur_sec;
 
@@ -661,9 +649,10 @@ default_symfile_offsets (struct objfile *objfile,
 
 	  /* Pick non-overlapping offsets for sections the user did not
 	     place explicitly.  */
-	  arg.offsets = &objfile->section_offsets;
-	  arg.lowest = 0;
-	  bfd_map_over_sections (objfile->obfd, place_section, &arg);
+	  CORE_ADDR lowest = 0;
+	  for (asection *sect : gdb_bfd_sections (objfile->obfd))
+	    place_section (objfile->obfd, sect, objfile->section_offsets,
+			   lowest);
 
 	  /* Correctly filling in the section offsets is not quite
 	     enough.  Relocatable files have two properties that
@@ -3570,18 +3559,6 @@ simple_overlay_update (struct obj_section *osect)
 	}
 }
 
-/* Set the output sections and output offsets for section SECTP in
-   ABFD.  The relocation code in BFD will read these offsets, so we
-   need to be sure they're initialized.  We map each section to itself,
-   with no offset; this means that SECTP->vma will be honored.  */
-
-static void
-symfile_dummy_outputs (bfd *abfd, asection *sectp, void *dummy)
-{
-  sectp->output_section = sectp;
-  sectp->output_offset = 0;
-}
-
 /* Default implementation for sym_relocate.  */
 
 bfd_byte *
@@ -3599,7 +3576,11 @@ default_symfile_relocate (struct objfile *objfile, asection *sectp,
 
   /* We will handle section offsets properly elsewhere, so relocate as if
      all sections begin at 0.  */
-  bfd_map_over_sections (abfd, symfile_dummy_outputs, NULL);
+  for (asection *sect : gdb_bfd_sections (abfd))
+    {
+      sect->output_section = sect;
+      sect->output_offset = 0;
+    }
 
   return bfd_simple_get_relocated_section_contents (abfd, sectp, buf, NULL);
 }
diff --git a/gdb/symfile.h b/gdb/symfile.h
index fe79f79a048..203620bc284 100644
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -474,8 +474,6 @@ extern bool auto_solib_add;
 
 extern void set_initial_language (void);
 
-extern void find_lowest_section (bfd *, asection *, void *);
-
 extern gdb_bfd_ref_ptr symfile_bfd_open (const char *);
 
 extern int get_section_index (struct objfile *, const char *);
-- 
2.17.2

[PATCH 07/13] Use gdb_bfd_sections in dwarf2/read.c

[Tom Tromey]

This changes some functions in dwarf2/read.c to avoid
bfd_map_over_sections, in favor of iteration.

gdb/ChangeLog
2020-08-28  Tom Tromey  <tom@tromey.com>

	* dwarf2/read.c (locate_dwz_sections): Change parameters.
	(dwarf2_get_dwz_file): Use foreach.
	(dwarf2_locate_dwo_sections): Change parameters.
	(open_and_init_dwo_file): Use foreach.
	(dwarf2_locate_common_dwp_sections): Change parameters.
	(open_and_init_dwp_file): Use foreach.
---
 gdb/ChangeLog     |  9 +++++++++
 gdb/dwarf2/read.c | 50 +++++++++++++++++++++++------------------------
 2 files changed, 33 insertions(+), 26 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 0ac8533263a..1da71c48084 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -2120,10 +2120,8 @@ dwarf2_get_section_info (struct objfile *objfile,
 /* A helper function to find the sections for a .dwz file.  */
 
 static void
-locate_dwz_sections (bfd *abfd, asection *sectp, void *arg)
+locate_dwz_sections (bfd *abfd, asection *sectp, dwz_file *dwz_file)
 {
-  struct dwz_file *dwz_file = (struct dwz_file *) arg;
-
   /* Note that we only support the standard ELF names, because .dwz
      is ELF-only (at the time of writing).  */
   if (section_is_p (sectp->name, &dwarf2_elf_names.abbrev))
@@ -2246,8 +2244,8 @@ dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd)
   std::unique_ptr<struct dwz_file> result
     (new struct dwz_file (std::move (dwz_bfd)));
 
-  bfd_map_over_sections (result->dwz_bfd.get (), locate_dwz_sections,
-			 result.get ());
+  for (asection *sec : gdb_bfd_sections (result->dwz_bfd))
+    locate_dwz_sections (result->dwz_bfd.get (), sec, result.get ());
 
   gdb_bfd_record_inclusion (per_bfd->obfd, result->dwz_bfd.get ());
   per_bfd->dwz_file = std::move (result);
@@ -11923,9 +11921,9 @@ create_dwp_hash_table (dwarf2_per_objfile *per_objfile,
 
 /* Update SECTIONS with the data from SECTP.
 
-   This function is like the other "locate" section routines that are
-   passed to bfd_map_over_sections, but in this context the sections to
-   read comes from the DWP V1 hash table, not the full ELF section table.
+   This function is like the other "locate" section routines, but in
+   this context the sections to read comes from the DWP V1 hash table,
+   not the full ELF section table.
 
    The result is non-zero for success, or zero if an error was found.  */
 
@@ -12738,9 +12736,9 @@ open_dwo_file (dwarf2_per_objfile *per_objfile,
    size of each of the DWO debugging sections we are interested in.  */
 
 static void
-dwarf2_locate_dwo_sections (bfd *abfd, asection *sectp, void *dwo_sections_ptr)
+dwarf2_locate_dwo_sections (bfd *abfd, asection *sectp,
+			    dwo_sections *dwo_sections)
 {
-  struct dwo_sections *dwo_sections = (struct dwo_sections *) dwo_sections_ptr;
   const struct dwop_section_names *names = &dwop_section_names;
 
   if (section_is_p (sectp->name, &names->abbrev_dwo))
@@ -12827,8 +12825,9 @@ open_and_init_dwo_file (dwarf2_cu *cu, const char *dwo_name,
   dwo_file->comp_dir = comp_dir;
   dwo_file->dbfd = std::move (dbfd);
 
-  bfd_map_over_sections (dwo_file->dbfd.get (), dwarf2_locate_dwo_sections,
-			 &dwo_file->sections);
+  for (asection *sec : gdb_bfd_sections (dwo_file->dbfd))
+    dwarf2_locate_dwo_sections (dwo_file->dbfd.get (), sec,
+				&dwo_file->sections);
 
   create_cus_hash_table (per_objfile, cu, *dwo_file, dwo_file->sections.info,
 			 dwo_file->cus);
@@ -12857,9 +12856,8 @@ open_and_init_dwo_file (dwarf2_cu *cu, const char *dwo_name,
 
 static void
 dwarf2_locate_common_dwp_sections (bfd *abfd, asection *sectp,
-				   void *dwp_file_ptr)
+				   dwp_file *dwp_file)
 {
-  struct dwp_file *dwp_file = (struct dwp_file *) dwp_file_ptr;
   const struct dwop_section_names *names = &dwop_section_names;
   unsigned int elf_section_nr = elf_section_data (sectp)->this_idx;
 
@@ -13123,9 +13121,9 @@ open_and_init_dwp_file (dwarf2_per_objfile *per_objfile)
     OBSTACK_CALLOC (&per_objfile->per_bfd->obstack,
 		    dwp_file->num_sections, asection *);
 
-  bfd_map_over_sections (dwp_file->dbfd.get (),
-			 dwarf2_locate_common_dwp_sections,
-			 dwp_file.get ());
+  for (asection *sec : gdb_bfd_sections (dwp_file->dbfd))
+    dwarf2_locate_common_dwp_sections (dwp_file->dbfd.get (), sec,
+				       dwp_file.get ());
 
   dwp_file->cus = create_dwp_hash_table (per_objfile, dwp_file.get (), 0);
 
@@ -13151,15 +13149,15 @@ open_and_init_dwp_file (dwarf2_per_objfile *per_objfile)
   else
     dwp_file->version = 2;
 
-  if (dwp_file->version == 2)
-    bfd_map_over_sections (dwp_file->dbfd.get (),
-			   dwarf2_locate_v2_dwp_sections,
-			   dwp_file.get ());
-  else if (dwp_file->version == 5)
-    bfd_map_over_sections (dwp_file->dbfd.get (),
-			   dwarf2_locate_v5_dwp_sections,
-			   dwp_file.get ());
-
+  for (asection *sec : gdb_bfd_sections (dwp_file->dbfd))
+    {
+      if (dwp_file->version == 2)
+	dwarf2_locate_v2_dwp_sections (dwp_file->dbfd.get (), sec,
+				       dwp_file.get ());
+      else
+	dwarf2_locate_v5_dwp_sections (dwp_file->dbfd.get (), sec,
+				       dwp_file.get ());
+    }
 
   dwp_file->loaded_cus = allocate_dwp_loaded_cutus_table ();
   dwp_file->loaded_tus = allocate_dwp_loaded_cutus_table ();
-- 
2.17.2

[PATCH 08/13] Use gdb_bfd_sections in ELF osabi tag sniffing

[Tom Tromey]

This changes some ELF osabi tag-sniffing functions to avoid
bfd_map_over_sections, in favor of iteration.  I could only readily
test the generic one.

gdb/ChangeLog
2020-08-28  Tom Tromey  <tom@tromey.com>

	* osabi.h (generic_elf_osabi_sniff_abi_tag_sections): Update.
	* osabi.c (generic_elf_osabi_sniff_abi_tag_sections): Change
	parameters.
	(generic_elf_osabi_sniffer): Use foreach.
	* mips-sde-tdep.c (mips_sde_elf_osabi_sniffer): Use foreach.
	* arm-tdep.c (arm_elf_osabi_sniffer): Use foreach.
---
 gdb/ChangeLog       |  9 +++++++++
 gdb/arm-tdep.c      |  7 ++++---
 gdb/mips-sde-tdep.c |  5 ++---
 gdb/osabi.c         | 10 +++++-----
 gdb/osabi.h         |  6 +++---
 5 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 074eedb4800..962ad1a40bc 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -8754,9 +8754,10 @@ arm_elf_osabi_sniffer (bfd *abfd)
   if (elfosabi == ELFOSABI_ARM)
     /* GNU tools use this value.  Check note sections in this case,
        as well.  */
-    bfd_map_over_sections (abfd,
-			   generic_elf_osabi_sniff_abi_tag_sections, 
-			   &osabi);
+    {
+      for (asection *sect : gdb_bfd_sections (abfd))
+	generic_elf_osabi_sniff_abi_tag_sections (abfd, sect, &osabi);
+    }
 
   /* Anything else will be handled by the generic ELF sniffer.  */
   return osabi;
diff --git a/gdb/mips-sde-tdep.c b/gdb/mips-sde-tdep.c
index 685c3c7263c..1d282c219e5 100644
--- a/gdb/mips-sde-tdep.c
+++ b/gdb/mips-sde-tdep.c
@@ -226,9 +226,8 @@ mips_sde_elf_osabi_sniffer (bfd *abfd)
 
   /* If the generic sniffer gets a hit, return and let other sniffers
      get a crack at it.  */
-  bfd_map_over_sections (abfd,
-			 generic_elf_osabi_sniff_abi_tag_sections,
-			 &osabi);
+  for (asection *sect : gdb_bfd_sections (abfd))
+    generic_elf_osabi_sniff_abi_tag_sections (abfd, sect, &osabi);
   if (osabi != GDB_OSABI_UNKNOWN)
     return GDB_OSABI_UNKNOWN;
 
diff --git a/gdb/osabi.c b/gdb/osabi.c
index 627b9d98151..e8a813b62f2 100644
--- a/gdb/osabi.c
+++ b/gdb/osabi.c
@@ -23,6 +23,7 @@
 #include "arch-utils.h"
 #include "gdbcmd.h"
 #include "command.h"
+#include "gdb_bfd.h"
 
 #include "elf-bfd.h"
 
@@ -439,9 +440,9 @@ check_note (bfd *abfd, asection *sect, char *note, unsigned int *sectsize,
 /* Generic sniffer for ELF flavoured files.  */
 
 void
-generic_elf_osabi_sniff_abi_tag_sections (bfd *abfd, asection *sect, void *obj)
+generic_elf_osabi_sniff_abi_tag_sections (bfd *abfd, asection *sect,
+					  enum gdb_osabi *osabi)
 {
-  enum gdb_osabi *osabi = (enum gdb_osabi *) obj;
   const char *name;
   unsigned int sectsize;
   char *note;
@@ -561,9 +562,8 @@ generic_elf_osabi_sniffer (bfd *abfd)
       /* And likewise ELFOSABI_HPUX.  For some reason the default
 	 value for the EI_OSABI field is ELFOSABI_HPUX for all PA-RISC
 	 targets (with the exception of GNU/Linux).  */
-      bfd_map_over_sections (abfd,
-			     generic_elf_osabi_sniff_abi_tag_sections,
-			     &osabi);
+      for (asection *sect : gdb_bfd_sections (abfd))
+	generic_elf_osabi_sniff_abi_tag_sections (abfd, sect, &osabi);
       break;
 
     case ELFOSABI_FREEBSD:
diff --git a/gdb/osabi.h b/gdb/osabi.h
index a7e6a10d019..9b7ab1818c0 100644
--- a/gdb/osabi.h
+++ b/gdb/osabi.h
@@ -86,8 +86,8 @@ const char *gdbarch_osabi_name (enum gdb_osabi);
 const char *osabi_triplet_regexp (enum gdb_osabi osabi);
 
 /* Helper routine for ELF file sniffers.  This looks at ABI tag note
-   sections to determine the OS ABI from the note.  It should be called
-   via bfd_map_over_sections.  */
-void generic_elf_osabi_sniff_abi_tag_sections (bfd *, asection *, void *);
+   sections to determine the OS ABI from the note.  */
+void generic_elf_osabi_sniff_abi_tag_sections (bfd *, asection *,
+					       enum gdb_osabi *);
 
 #endif /* OSABI_H */
-- 
2.17.2

[PATCH 09/13] Use gdb_bfd_sections in gcore_memory_sections

[Tom Tromey]

This changes gcore_memory_sections to avoid bfd_map_over_sections, in
favor of iteration.

gdb/ChangeLog
2020-08-28  Tom Tromey  <tom@tromey.com>

	* gcore.c (make_output_phdrs): Remove 'ignored' parameter.
	(gcore_copy_callback): Likewise.
	(gcore_memory_sections): Use foreach.
---
 gdb/ChangeLog |  6 ++++++
 gdb/gcore.c   | 10 ++++++----
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/gdb/gcore.c b/gdb/gcore.c
index d0e36b1a708..46ca2da5003 100644
--- a/gdb/gcore.c
+++ b/gdb/gcore.c
@@ -364,7 +364,7 @@ derive_heap_segment (bfd *abfd, bfd_vma *bottom, bfd_vma *top)
 }
 
 static void
-make_output_phdrs (bfd *obfd, asection *osec, void *ignored)
+make_output_phdrs (bfd *obfd, asection *osec)
 {
   int p_flags = 0;
   int p_type = 0;
@@ -531,7 +531,7 @@ objfile_find_memory_regions (struct target_ops *self,
 }
 
 static void
-gcore_copy_callback (bfd *obfd, asection *osec, void *ignored)
+gcore_copy_callback (bfd *obfd, asection *osec)
 {
   bfd_size_type size, total_size = bfd_section_size (osec);
   file_ptr offset = 0;
@@ -587,10 +587,12 @@ gcore_memory_sections (bfd *obfd)
     }
 
   /* Record phdrs for section-to-segment mapping.  */
-  bfd_map_over_sections (obfd, make_output_phdrs, NULL);
+  for (asection *sect : gdb_bfd_sections (obfd))
+    make_output_phdrs (obfd, sect);
 
   /* Copy memory region contents.  */
-  bfd_map_over_sections (obfd, gcore_copy_callback, NULL);
+  for (asection *sect : gdb_bfd_sections (obfd))
+    gcore_copy_callback (obfd, sect);
 
   return 1;
 }
-- 
2.17.2

[PATCH 10/13] Use gdb_bfd_sections in restore_command

[Tom Tromey]

This changes restore_command to avoid bfd_map_over_sections, in favor
of iteration.  A helper data structure can also be removed by this
patch.

gdb/ChangeLog
2020-08-28  Tom Tromey  <tom@tromey.com>

	* cli/cli-dump.c (struct callback_data): Remove.
	(restore_one_section): Rename from restore_section_callback.
	Change parameters.
	(restore_binary_file): Change parameters.
	(restore_command): Use foreach.
---
 gdb/ChangeLog      |  8 ++++
 gdb/cli/cli-dump.c | 94 ++++++++++++++++++++++------------------------
 2 files changed, 53 insertions(+), 49 deletions(-)

diff --git a/gdb/cli/cli-dump.c b/gdb/cli/cli-dump.c
index 567ef2eeded..849964ece6b 100644
--- a/gdb/cli/cli-dump.c
+++ b/gdb/cli/cli-dump.c
@@ -371,22 +371,14 @@ add_dump_command (const char *name,
     c->doc = concat ("Append ", c->doc + 6, (char *)NULL);
 }
 
-/* Opaque data for restore_section_callback.  */
-struct callback_data {
-  CORE_ADDR load_offset;
-  CORE_ADDR load_start;
-  CORE_ADDR load_end;
-};
-
-/* Function: restore_section_callback.
-
-   Callback function for bfd_map_over_sections.
-   Selectively loads the sections into memory.  */
+/* Selectively loads the sections into memory.  */
 
 static void
-restore_section_callback (bfd *ibfd, asection *isec, void *args)
+restore_one_section (bfd *ibfd, asection *isec,
+		     CORE_ADDR load_offset,
+		     CORE_ADDR load_start,
+		     CORE_ADDR load_end)
 {
-  struct callback_data *data = (struct callback_data *) args;
   bfd_vma sec_start  = bfd_section_vma (isec);
   bfd_size_type size = bfd_section_size (isec);
   bfd_vma sec_end    = sec_start + size;
@@ -399,8 +391,8 @@ restore_section_callback (bfd *ibfd, asection *isec, void *args)
     return;
 
   /* Does the section overlap with the desired restore range? */
-  if (sec_end <= data->load_start 
-      || (data->load_end > 0 && sec_start >= data->load_end))
+  if (sec_end <= load_start 
+      || (load_end > 0 && sec_start >= load_end))
     {
       /* No, no useable data in this section.  */
       printf_filtered (_("skipping section %s...\n"), 
@@ -411,12 +403,12 @@ restore_section_callback (bfd *ibfd, asection *isec, void *args)
   /* Compare section address range with user-requested
      address range (if any).  Compute where the actual
      transfer should start and end.  */
-  if (sec_start < data->load_start)
-    sec_offset = data->load_start - sec_start;
+  if (sec_start < load_start)
+    sec_offset = load_start - sec_start;
   /* Size of a partial transfer.  */
   sec_load_count -= sec_offset;
-  if (data->load_end > 0 && sec_end > data->load_end)
-    sec_load_count -= sec_end - data->load_end;
+  if (load_end > 0 && sec_end > load_end)
+    sec_load_count -= sec_end - load_end;
 
   /* Get the data.  */
   gdb::byte_vector buf (size);
@@ -429,26 +421,28 @@ restore_section_callback (bfd *ibfd, asection *isec, void *args)
 		   (unsigned long) sec_start, 
 		   (unsigned long) sec_end);
 
-  if (data->load_offset != 0 || data->load_start != 0 || data->load_end != 0)
+  if (load_offset != 0 || load_start != 0 || load_end != 0)
     printf_filtered (" into memory (%s to %s)\n",
 		     paddress (target_gdbarch (),
 			       (unsigned long) sec_start
-			       + sec_offset + data->load_offset), 
+			       + sec_offset + load_offset), 
 		     paddress (target_gdbarch (),
 			       (unsigned long) sec_start + sec_offset
-				+ data->load_offset + sec_load_count));
+				+ load_offset + sec_load_count));
   else
     puts_filtered ("\n");
 
   /* Write the data.  */
-  ret = target_write_memory (sec_start + sec_offset + data->load_offset, 
+  ret = target_write_memory (sec_start + sec_offset + load_offset, 
 			     &buf[sec_offset], sec_load_count);
   if (ret != 0)
     warning (_("restore: memory write failed (%s)."), safe_strerror (ret));
 }
 
 static void
-restore_binary_file (const char *filename, struct callback_data *data)
+restore_binary_file (const char *filename, CORE_ADDR load_offset,
+		     CORE_ADDR load_start, CORE_ADDR load_end)
+
 {
   gdb_file_up file = gdb_fopen_cloexec (filename, FOPEN_RB);
   long len;
@@ -466,25 +460,25 @@ restore_binary_file (const char *filename, struct callback_data *data)
   else
     perror_with_name (filename);
 
-  if (len <= data->load_start)
+  if (len <= load_start)
     error (_("Start address is greater than length of binary file %s."), 
 	   filename);
 
   /* Chop off "len" if it exceeds the requested load_end addr.  */
-  if (data->load_end != 0 && data->load_end < len)
-    len = data->load_end;
+  if (load_end != 0 && load_end < len)
+    len = load_end;
   /* Chop off "len" if the requested load_start addr skips some bytes.  */
-  if (data->load_start > 0)
-    len -= data->load_start;
+  if (load_start > 0)
+    len -= load_start;
 
   printf_filtered 
     ("Restoring binary file %s into memory (0x%lx to 0x%lx)\n", 
      filename, 
-     (unsigned long) (data->load_start + data->load_offset),
-     (unsigned long) (data->load_start + data->load_offset + len));
+     (unsigned long) (load_start + load_offset),
+     (unsigned long) (load_start + load_offset + len));
 
   /* Now set the file pos to the requested load start pos.  */
-  if (fseek (file.get (), data->load_start, SEEK_SET) != 0)
+  if (fseek (file.get (), load_start, SEEK_SET) != 0)
     perror_with_name (filename);
 
   /* Now allocate a buffer and read the file contents.  */
@@ -493,8 +487,7 @@ restore_binary_file (const char *filename, struct callback_data *data)
     perror_with_name (filename);
 
   /* Now write the buffer into target memory.  */
-  len = target_write_memory (data->load_start + data->load_offset,
-			     buf.data (), len);
+  len = target_write_memory (load_start + load_offset, buf.data (), len);
   if (len != 0)
     warning (_("restore: memory write failed (%s)."), safe_strerror (len));
 }
@@ -502,15 +495,14 @@ restore_binary_file (const char *filename, struct callback_data *data)
 static void
 restore_command (const char *args, int from_tty)
 {
-  struct callback_data data;
   int binary_flag = 0;
 
   if (!target_has_execution)
     noprocess ();
 
-  data.load_offset = 0;
-  data.load_start  = 0;
-  data.load_end    = 0;
+  CORE_ADDR load_offset = 0;
+  CORE_ADDR load_start  = 0;
+  CORE_ADDR load_end    = 0;
 
   /* Parse the input arguments.  First is filename (required).  */
   gdb::unique_xmalloc_ptr<char> filename = scan_filename (&args, NULL);
@@ -527,19 +519,20 @@ restore_command (const char *args, int from_tty)
 	}
       /* Parse offset (optional).  */
       if (args != NULL && *args != '\0')
-	data.load_offset = binary_flag ?
-	  parse_and_eval_address (scan_expression (&args, NULL).get ()) :
-	  parse_and_eval_long (scan_expression (&args, NULL).get ());
+	load_offset
+	  = (binary_flag
+	     ? parse_and_eval_address (scan_expression (&args, NULL).get ())
+	     : parse_and_eval_long (scan_expression (&args, NULL).get ()));
       if (args != NULL && *args != '\0')
 	{
 	  /* Parse start address (optional).  */
-	  data.load_start = 
+	  load_start = 
 	    parse_and_eval_long (scan_expression (&args, NULL).get ());
 	  if (args != NULL && *args != '\0')
 	    {
 	      /* Parse end address (optional).  */
-	      data.load_end = parse_and_eval_long (args);
-	      if (data.load_end <= data.load_start)
+	      load_end = parse_and_eval_long (args);
+	      if (load_end <= load_start)
 		error (_("Start must be less than end."));
 	    }
 	}
@@ -547,13 +540,14 @@ restore_command (const char *args, int from_tty)
 
   if (info_verbose)
     printf_filtered ("Restore file %s offset 0x%lx start 0x%lx end 0x%lx\n",
-		     filename.get (), (unsigned long) data.load_offset,
-		     (unsigned long) data.load_start, 
-		     (unsigned long) data.load_end);
+		     filename.get (), (unsigned long) load_offset,
+		     (unsigned long) load_start, 
+		     (unsigned long) load_end);
 
   if (binary_flag)
     {
-      restore_binary_file (filename.get (), &data);
+      restore_binary_file (filename.get (), load_offset, load_start,
+			   load_end);
     }
   else
     {
@@ -561,7 +555,9 @@ restore_command (const char *args, int from_tty)
       gdb_bfd_ref_ptr ibfd (bfd_openr_or_error (filename.get (), NULL));
 
       /* Process the sections.  */
-      bfd_map_over_sections (ibfd.get (), restore_section_callback, &data);
+      for (asection *sect : gdb_bfd_sections (ibfd))
+	restore_one_section (ibfd.get (), sect, load_offset, load_start,
+			     load_end);
     }
 }
 
-- 
2.17.2

[PATCH 11/13] Use gdb_bfd_sections in elf_symfile_read

[Tom Tromey]

This changes elf_symfile_read to avoid bfd_map_over_sections, in favor
of iteration.

gdb/ChangeLog
2020-08-28  Tom Tromey  <tom@tromey.com>

	* elfread.c (elf_locate_sections): Change parameters.
	(elf_symfile_read): Use foreach.
---
 gdb/ChangeLog |  5 +++++
 gdb/elfread.c | 10 +++++-----
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/gdb/elfread.c b/gdb/elfread.c
index 75bdd75250b..5dad3ddd940 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -176,11 +176,8 @@ elf_symfile_segments (bfd *abfd)
    -kingdon).  */
 
 static void
-elf_locate_sections (bfd *ignore_abfd, asection *sectp, void *eip)
+elf_locate_sections (asection *sectp, struct elfinfo *ei)
 {
-  struct elfinfo *ei;
-
-  ei = (struct elfinfo *) eip;
   if (strcmp (sectp->name, ".stab") == 0)
     {
       ei->stabsect = sectp;
@@ -1211,7 +1208,10 @@ elf_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
 
   memset ((char *) &ei, 0, sizeof (ei));
   if (!(objfile->flags & OBJF_READNEVER))
-    bfd_map_over_sections (abfd, elf_locate_sections, (void *) & ei);
+    {
+      for (asection *sect : gdb_bfd_sections (abfd))
+	elf_locate_sections (sect, &ei);
+    }
 
   elf_read_minimal_symbols (objfile, symfile_flags, &ei);
 
-- 
2.17.2

[PATCH 12/13] Use gdb_bfd_sections in build_section_table

[Tom Tromey]

This changes build_section_table to avoid bfd_map_over_sections, in
favor of iteration.  In this situation it seemed simple to just remove
the helper function entirely.

gdb/ChangeLog
2020-08-28  Tom Tromey  <tom@tromey.com>

	* exec.c (add_to_section_table): Remove.
	(build_section_table): Use foreach.
---
 gdb/ChangeLog |  5 +++++
 gdb/exec.c    | 50 ++++++++++++++++++++------------------------------
 2 files changed, 25 insertions(+), 30 deletions(-)

diff --git a/gdb/exec.c b/gdb/exec.c
index e50f38899d6..14cc6af7de0 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -587,35 +587,6 @@ file_command (const char *arg, int from_tty)
 }
 \f
 
-/* Locate all mappable sections of a BFD file.
-   table_pp_char is a char * to get it through bfd_map_over_sections;
-   we cast it back to its proper type.  */
-
-static void
-add_to_section_table (bfd *abfd, struct bfd_section *asect,
-		      void *table_pp_char)
-{
-  struct target_section **table_pp = (struct target_section **) table_pp_char;
-  flagword aflag;
-
-  gdb_assert (abfd == asect->owner);
-
-  /* Check the section flags, but do not discard zero-length sections, since
-     some symbols may still be attached to this section.  For instance, we
-     encountered on sparc-solaris 2.10 a shared library with an empty .bss
-     section to which a symbol named "_end" was attached.  The address
-     of this symbol still needs to be relocated.  */
-  aflag = bfd_section_flags (asect);
-  if (!(aflag & SEC_ALLOC))
-    return;
-
-  (*table_pp)->owner = NULL;
-  (*table_pp)->the_bfd_section = asect;
-  (*table_pp)->addr = bfd_section_vma (asect);
-  (*table_pp)->endaddr = (*table_pp)->addr + bfd_section_size (asect);
-  (*table_pp)++;
-}
-
 /* See exec.h.  */
 
 void
@@ -665,7 +636,26 @@ build_section_table (struct bfd *some_bfd, struct target_section **start,
   xfree (*start);
   *start = XNEWVEC (struct target_section, count);
   *end = *start;
-  bfd_map_over_sections (some_bfd, add_to_section_table, (char *) end);
+  for (asection *asect : gdb_bfd_sections (some_bfd))
+    {
+      flagword aflag;
+
+      /* Check the section flags, but do not discard zero-length
+	 sections, since some symbols may still be attached to this
+	 section.  For instance, we encountered on sparc-solaris 2.10
+	 a shared library with an empty .bss section to which a symbol
+	 named "_end" was attached.  The address of this symbol still
+	 needs to be relocated.  */
+      aflag = bfd_section_flags (asect);
+      if (!(aflag & SEC_ALLOC))
+	continue;
+
+      (*end)->owner = NULL;
+      (*end)->the_bfd_section = asect;
+      (*end)->addr = bfd_section_vma (asect);
+      (*end)->endaddr = (*end)->addr + bfd_section_size (asect);
+      (*end)++;
+    }
 
   gdb_assert (*end <= *start + count);
 
-- 
2.17.2

[PATCH 13/13] Use gdb_bfd_sections in generic_load

[Tom Tromey]

This changes generic_load to avoid bfd_map_over_sections, in favor of
iteration.

gdb/ChangeLog
2020-08-28  Tom Tromey  <tom@tromey.com>

	* symfile.c (add_section_size_callback): Remove.
	(load_one_section): Rename from load_section_callback.  Change
	parameters.
	(generic_load): Use foreach.
---
 gdb/ChangeLog |  7 +++++++
 gdb/symfile.c | 23 +++++++----------------
 2 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/gdb/symfile.c b/gdb/symfile.c
index 96d8b3f0b52..1c22d8582c0 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1842,16 +1842,6 @@ load_command (const char *arg, int from_tty)
 
 static int validate_download = 0;
 
-/* Callback service function for generic_load (bfd_map_over_sections).  */
-
-static void
-add_section_size_callback (bfd *abfd, asection *asec, void *data)
-{
-  bfd_size_type *sum = (bfd_size_type *) data;
-
-  *sum += bfd_section_size (asec);
-}
-
 /* Opaque data for load_progress.  */
 struct load_progress_data
 {
@@ -1966,12 +1956,12 @@ load_progress (ULONGEST bytes, void *untyped_arg)
 				   totals->total_size);
 }
 
-/* Callback service function for generic_load (bfd_map_over_sections).  */
+/* Service function for generic_load.  */
 
 static void
-load_section_callback (bfd *abfd, asection *asec, void *data)
+load_one_section (bfd *abfd, asection *asec,
+		  struct load_section_data *args)
 {
-  struct load_section_data *args = (struct load_section_data *) data;
   bfd_size_type size = bfd_section_size (asec);
   const char *sect_name = bfd_section_name (asec);
 
@@ -2040,10 +2030,11 @@ generic_load (const char *args, int from_tty)
 	     bfd_errmsg (bfd_get_error ()));
     }
 
-  bfd_map_over_sections (loadfile_bfd.get (), add_section_size_callback,
-			 (void *) &total_progress.total_size);
+  for (asection *asec : gdb_bfd_sections (loadfile_bfd))
+    total_progress.total_size += bfd_section_size (asec);
 
-  bfd_map_over_sections (loadfile_bfd.get (), load_section_callback, &cbdata);
+  for (asection *asec : gdb_bfd_sections (loadfile_bfd))
+    load_one_section (loadfile_bfd.get (), asec, &cbdata);
 
   using namespace std::chrono;
 
-- 
2.17.2

Re: [PATCH 01/13] Add a new overload of gdb_bfd_sections

[Simon Marchi]

On 2020-08-28 12:23 p.m., Tom Tromey wrote:
> This adds a new overload of gdb_bfd_sections, that accepts a
> gdb_bfd_ref_ptr.  This also fixes the formatting of the existing
> function, since I happened to notice it was mildly off.

Ah thanks for fixing the formatting.  I'm confusing different styles for
different code bases I'm working with at the moment :).

> 
> gdb/ChangeLog
> 2020-08-28  Tom Tromey  <tom@tromey.com>
> 
> 	* gdb_bfd.h (gdb_bfd_sections): New overload.  Fix formatting of
> 	existing function.
> ---
>  gdb/ChangeLog |  5 +++++
>  gdb/gdb_bfd.h | 10 ++++++++--
>  2 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/gdb_bfd.h b/gdb/gdb_bfd.h
> index 0f450567d50..12330970873 100644
> --- a/gdb/gdb_bfd.h
> +++ b/gdb/gdb_bfd.h
> @@ -205,10 +205,16 @@ bool gdb_bfd_get_full_section_contents (bfd *abfd, asection *section,
>  using gdb_bfd_section_iterator = next_iterator<asection>;
>  using gdb_bfd_section_range = next_adapter<asection, gdb_bfd_section_iterator>;
>  
> -static inline
> -gdb_bfd_section_range gdb_bfd_sections (bfd *abfd)
> +static inline gdb_bfd_section_range
> +gdb_bfd_sections (bfd *abfd)
>  {
>    return gdb_bfd_section_range (abfd->sections);
>  }
>  
> +static inline gdb_bfd_section_range
> +gdb_bfd_sections (const gdb_bfd_ref_ptr &abfd)
> +{
> +  return gdb_bfd_section_range (abfd->sections);
> +};
> +

Just to confirm, the point of this is to avoid having to do `.get ()` everywhere?

(it LGTM)

Simon

Re: [PATCH 06/13] Use gdb_bfd_sections in symfile.c

[Simon Marchi]

On 2020-08-28 12:23 p.m., Tom Tromey wrote:
> @@ -500,7 +488,8 @@ addr_info_make_relative (section_addr_info *addrs, bfd *abfd)
>    /* Find lowest loadable section to be used as starting point for
>       contiguous sections.  */
>    lower_sect = NULL;
> -  bfd_map_over_sections (abfd, find_lowest_section, &lower_sect);
> +  for (asection *iter : gdb_bfd_sections (abfd))
> +    find_lowest_section (iter, &lower_sect);
>    if (lower_sect == NULL)
>      {
>        warning (_("no loadable sections found in added symbol-file %s"),

FYI: I've been taught by Pedro [1] that we don't actually need curly braces in this case,
and I've been misleading people about this for a while :(.  For future code I write like
this, I won't use the curly braces.

[1] https://sourceware.org/pipermail/gdb-patches/2020-August/171532.html

Simon

Re: [PATCH 06/13] Use gdb_bfd_sections in symfile.c

[Simon Marchi]

On 2020-08-28 1:00 p.m., Simon Marchi wrote:
> On 2020-08-28 12:23 p.m., Tom Tromey wrote:
>> @@ -500,7 +488,8 @@ addr_info_make_relative (section_addr_info *addrs, bfd *abfd)
>>    /* Find lowest loadable section to be used as starting point for
>>       contiguous sections.  */
>>    lower_sect = NULL;
>> -  bfd_map_over_sections (abfd, find_lowest_section, &lower_sect);
>> +  for (asection *iter : gdb_bfd_sections (abfd))
>> +    find_lowest_section (iter, &lower_sect);
>>    if (lower_sect == NULL)
>>      {
>>        warning (_("no loadable sections found in added symbol-file %s"),
> 
> FYI: I've been taught by Pedro [1] that we don't actually need curly braces in this case,
> and I've been misleading people about this for a while :(.  For future code I write like
> this, I won't use the curly braces.
> 
> [1] https://sourceware.org/pipermail/gdb-patches/2020-August/171532.html
> 
> Simon
> 

Sorry, I replied this at the totally wrong place, in the wrong patch.

Simon

Re: [PATCH 04/13] Use gdb_bfd_sections in get_stap_base_address

[Simon Marchi]

On 2020-08-28 12:23 p.m., Tom Tromey wrote:
> This changes get_stap_base_address to avoid bfd_map_over_sections, in
> favor of iteration.
> 
> gdb/ChangeLog
> 2020-08-28  Tom Tromey  <tom@tromey.com>
> 
> 	* stap-probe.c (get_stap_base_address_1): Remove.
> 	(get_stap_base_address): Use foreach.
> ---
>  gdb/ChangeLog    |  5 +++++
>  gdb/stap-probe.c | 20 ++++++--------------
>  2 files changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c
> index 73596446cce..fb6010d14ff 100644
> --- a/gdb/stap-probe.c
> +++ b/gdb/stap-probe.c
> @@ -1578,19 +1578,6 @@ handle_stap_probe (struct objfile *objfile, struct sdt_note *el,
>    probesp->emplace_back (ret);
>  }
>  
> -/* Helper function which tries to find the base address of the SystemTap
> -   base section named STAP_BASE_SECTION_NAME.  */
> -
> -static void
> -get_stap_base_address_1 (bfd *abfd, asection *sect, void *obj)
> -{
> -  asection **ret = (asection **) obj;
> -
> -  if ((sect->flags & (SEC_DATA | SEC_ALLOC | SEC_HAS_CONTENTS))
> -      && sect->name && !strcmp (sect->name, STAP_BASE_SECTION_NAME))
> -    *ret = sect;
> -}
> -
>  /* Helper function which iterates over every section in the BFD file,
>     trying to find the base address of the SystemTap base section.
>     Returns 1 if found (setting BASE to the proper value), zero otherwise.  */
> @@ -1600,7 +1587,12 @@ get_stap_base_address (bfd *obfd, bfd_vma *base)
>  {
>    asection *ret = NULL;
>  
> -  bfd_map_over_sections (obfd, get_stap_base_address_1, (void *) &ret);
> +  for (asection *sect : gdb_bfd_sections (obfd))
> +    {
> +      if ((sect->flags & (SEC_DATA | SEC_ALLOC | SEC_HAS_CONTENTS))
> +	  && sect->name && !strcmp (sect->name, STAP_BASE_SECTION_NAME))
> +	ret = sect;
> +    }

This is where I meant to say:

FYI: I've been taught by Pedro [1] that we don't actually need curly braces in this case,
and I've been misleading people about this for a while .  For future code I write like
this, I won't use the curly braces.

[1] https://sourceware.org/pipermail/gdb-patches/2020-August/171532.html

Simon

Re: [PATCH 10/13] Use gdb_bfd_sections in restore_command

[Simon Marchi]

On 2020-08-28 12:23 p.m., Tom Tromey wrote:
> This changes restore_command to avoid bfd_map_over_sections, in favor
> of iteration.  A helper data structure can also be removed by this
> patch.
> 
> gdb/ChangeLog
> 2020-08-28  Tom Tromey  <tom@tromey.com>
> 
> 	* cli/cli-dump.c (struct callback_data): Remove.
> 	(restore_one_section): Rename from restore_section_callback.
> 	Change parameters.
> 	(restore_binary_file): Change parameters.
> 	(restore_command): Use foreach.

Warnings I got while applying:

Applying: Use gdb_bfd_sections in restore_command
.git/rebase-apply/patch:44: trailing whitespace.
  if (sec_end <= load_start
.git/rebase-apply/patch:76: trailing whitespace.
			       + sec_offset + load_offset),
.git/rebase-apply/patch:86: trailing whitespace.
  ret = target_write_memory (sec_start + sec_offset + load_offset,
.git/rebase-apply/patch:178: trailing whitespace.
	  load_start =
.git/rebase-apply/patch:198: trailing whitespace.
		     (unsigned long) load_start,
warning: 5 lines add whitespace errors.

Simon

Re: [PATCH 00/13] Use gdb_bfd_sections in more places

[Simon Marchi]

On 2020-08-28 12:23 p.m., Tom Tromey wrote:
> As promised, here are some patches to use gdb_bfd_sections in more
> places.
> 
> The main motivation for this patch is that ordinary iteration is
> easier to understand than callback functions.  Often, it is also
> shorter -- some patches in this series remove helper functions or
> helper data structures.  Finally, iteration is more type-safe (at
> least without additional wrappers to pass function-views through
> bfd_map_over_sections).
> 
> I did not convert every call to bfd_map_over_sections.  Some of the
> remaining calls are not easy to test.  For the calls in compile/, I
> have another series that makes many changes in that code, and so I
> didn't want to introduce clashes at this time.  (I don't recall
> offhand if that series fixes this, but I can revisit when it lands.)
> 
> As usual with these cleanups, I don't plan to commit this until the
> release branch is made.
> 
> Let me know what you think.
> 
> Tom
> 
> 

Woohoo, thanks!

Apart from the small nits I sent, this all LGTM.

Simon

Re: [PATCH 01/13] Add a new overload of gdb_bfd_sections

[Tom Tromey]

Simon> Ah thanks for fixing the formatting.  I'm confusing different styles for
Simon> different code bases I'm working with at the moment :).

It's definitely a pain when you work on more than one code base.
That happened to me when I was at Moz.  Actually the Moz tree itself had
two different styles, which as you can imagine is impossible to get right.

>> +static inline gdb_bfd_section_range
>> +gdb_bfd_sections (const gdb_bfd_ref_ptr &abfd)
>> +{
>> +  return gdb_bfd_section_range (abfd->sections);
>> +};
>> +

Simon> Just to confirm, the point of this is to avoid having to do `.get
Simon> ()` everywhere?

Yeah, it is just a convenience for exactly that reason.

Tom

Re: [PATCH 10/13] Use gdb_bfd_sections in restore_command

[Tom Tromey]

Simon> Warnings I got while applying:

These all seem to be pre-existing, but I since I was already touching
these lines, I went ahead and fixed them.

Tom

Re: [PATCH 10/13] Use gdb_bfd_sections in restore_command

[Simon Marchi]

On 2020-08-29 11:47 a.m., Tom Tromey wrote:
> Simon> Warnings I got while applying:
> 
> These all seem to be pre-existing, but I since I was already touching
> these lines, I went ahead and fixed them.

Yes that was the idea: since you touch the lines, might as well fix them.  Sorry for not being clear.

Simon

Re: [PATCH 00/13] Use gdb_bfd_sections in more places

[John Baldwin]

On 8/28/20 9:23 AM, Tom Tromey wrote:
> As promised, here are some patches to use gdb_bfd_sections in more
> places.
> 
> The main motivation for this patch is that ordinary iteration is
> easier to understand than callback functions.  Often, it is also
> shorter -- some patches in this series remove helper functions or
> helper data structures.  Finally, iteration is more type-safe (at
> least without additional wrappers to pass function-views through
> bfd_map_over_sections).
> 
> I did not convert every call to bfd_map_over_sections.  Some of the
> remaining calls are not easy to test.  For the calls in compile/, I
> have another series that makes many changes in that code, and so I
> didn't want to introduce clashes at this time.  (I don't recall
> offhand if that series fixes this, but I can revisit when it lands.)
> 
> As usual with these cleanups, I don't plan to commit this until the
> release branch is made.
> 
> Let me know what you think.

I didn't look at all the patches in detail, but in general I agree that
this is more readable and really like this change.  Ranged-for also
makes this very readable relative to what you would have to do in
C++03.

I see in some cases you also inlined the callback entirely when it was
short which I also think is a good idea.

-- 
John Baldwin

Re: [PATCH 00/13] Use gdb_bfd_sections in more places

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Tom> As promised, here are some patches to use gdb_bfd_sections in more
Tom> places.

I've fixed the review comments, which were very minor, and now I am
checking this in.

Tom