public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/4] Remove remaining ALL_ iterator macros
@ 2023-04-16 16:28 Tom Tromey
  2023-04-16 16:28 ` [PATCH 1/4] Rename objfile::sections Tom Tromey
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Tom Tromey @ 2023-04-16 16:28 UTC (permalink / raw)
  To: gdb-patches

This series finishes my long-term goal of removing all the ALL_*
iterator macros from gdb.  It replaces the remaining ones with
iterator ranges so that for-each can be used instead.

Regression tested on x86-64 Fedora 36.

---
Tom Tromey (4):
      Rename objfile::sections
      Remove ALL_OBJFILE_OSECTIONS
      Remove ALL_DICT_SYMBOLS
      Remove ALL_BREAKPOINTS_SAFE

 gdb/arm-tdep.c        |   4 +-
 gdb/block.h           |   4 ++
 gdb/breakpoint.c      | 134 ++++++++++++++++++++-----------------------
 gdb/buildsym.c        |  15 ++---
 gdb/dictionary.h      |  55 ++++++++++++++----
 gdb/exec.c            |   4 +-
 gdb/gcore.c           |   6 +-
 gdb/hppa-bsd-tdep.c   |  59 +++++++++----------
 gdb/hppa-linux-tdep.c |  64 ++++++++++-----------
 gdb/hppa-tdep.c       |  29 +++++-----
 gdb/ia64-tdep.c       |  99 ++++++++++++++++----------------
 gdb/machoread.c       |   7 +--
 gdb/maint.c           |  10 ++--
 gdb/minsyms.c         |   4 +-
 gdb/objfiles.c        |  38 +++++--------
 gdb/objfiles.h        | 155 +++++++++++++++++++++++++++++++++++---------------
 gdb/printcmd.c        |   3 +-
 gdb/solib-aix.c       |   8 +--
 gdb/solib-dsbt.c      |   5 +-
 gdb/solib-frv.c       |   5 +-
 gdb/symfile.c         |  38 +++++--------
 gdb/symmisc.c         |   6 +-
 gdb/symtab.c          |   8 +--
 gdb/xcoffread.c       |   3 +-
 gdb/xstormy16-tdep.c  |  53 ++++++++---------
 gdb/z80-tdep.c        |  16 +++---
 26 files changed, 435 insertions(+), 397 deletions(-)
---
base-commit: b01ee1bf90fa6c0d6c0a23367433e5c1f0de87c0
change-id: 20230416-remove-all-macros-8317d531f169

Best regards,
-- 
Tom Tromey <tom@tromey.com>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/4] Rename objfile::sections
  2023-04-16 16:28 [PATCH 0/4] Remove remaining ALL_ iterator macros Tom Tromey
@ 2023-04-16 16:28 ` Tom Tromey
  2023-04-17 17:57   ` Keith Seitz
  2023-04-16 16:28 ` [PATCH 2/4] Remove ALL_OBJFILE_OSECTIONS Tom Tromey
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Tom Tromey @ 2023-04-16 16:28 UTC (permalink / raw)
  To: gdb-patches

I think objfile::sections makes sense as the name of the method to
iterate over an objfile's sections, so this patch renames the existing
field to objfile::sections_start in preparation for that.
---
 gdb/machoread.c  |  2 +-
 gdb/maint.c      |  6 +++---
 gdb/objfiles.c   | 12 ++++++------
 gdb/objfiles.h   |  4 ++--
 gdb/solib-aix.c  |  4 ++--
 gdb/solib-dsbt.c |  2 +-
 gdb/solib-frv.c  |  2 +-
 gdb/symfile.c    |  2 +-
 gdb/symmisc.c    |  2 +-
 gdb/symtab.c     |  4 ++--
 gdb/xcoffread.c  |  3 ++-
 11 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/gdb/machoread.c b/gdb/machoread.c
index 9735de80d57..dc841c30af2 100644
--- a/gdb/machoread.c
+++ b/gdb/machoread.c
@@ -926,7 +926,7 @@ macho_symfile_offsets (struct objfile *objfile,
   ALL_OBJFILE_OSECTIONS (objfile, osect)
     {
       const char *bfd_sect_name = osect->the_bfd_section->name;
-      int sect_index = osect - objfile->sections;;
+      int sect_index = osect - objfile->sections_start;
 
       if (startswith (bfd_sect_name, "LC_SEGMENT."))
 	bfd_sect_name += 11;
diff --git a/gdb/maint.c b/gdb/maint.c
index a8afef0e16b..3cd2c5e899a 100644
--- a/gdb/maint.c
+++ b/gdb/maint.c
@@ -331,11 +331,11 @@ maint_obj_section_from_bfd_section (bfd *abfd,
 				    asection *asection,
 				    objfile *ofile)
 {
-  if (ofile->sections == nullptr)
+  if (ofile->sections_start == nullptr)
     return nullptr;
 
   obj_section *osect
-    = &ofile->sections[gdb_bfd_section_index (abfd, asection)];
+    = &ofile->sections_start[gdb_bfd_section_index (abfd, asection)];
 
   if (osect >= ofile->sections_end)
     return nullptr;
@@ -375,7 +375,7 @@ maint_print_all_sections (const char *header, bfd *abfd, objfile *objfile,
 
       if (objfile != nullptr)
 	{
-	  gdb_assert (objfile->sections != nullptr);
+	  gdb_assert (objfile->sections_start != nullptr);
 	  osect
 	    = maint_obj_section_from_bfd_section (abfd, sect, objfile);
 	  if (osect->the_bfd_section == nullptr)
diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index 9caebfefd59..e3fa691dd53 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -264,7 +264,7 @@ add_to_objfile_sections (struct bfd *abfd, struct bfd_section *asect,
 	return;
     }
 
-  section = &objfile->sections[gdb_bfd_section_index (abfd, asect)];
+  section = &objfile->sections_start[gdb_bfd_section_index (abfd, asect)];
   section->objfile = objfile;
   section->the_bfd_section = asect;
   section->ovly_mapped = 0;
@@ -280,10 +280,10 @@ build_objfile_section_table (struct objfile *objfile)
 {
   int count = gdb_bfd_count_sections (objfile->obfd.get ());
 
-  objfile->sections = OBSTACK_CALLOC (&objfile->objfile_obstack,
-				      count,
-				      struct obj_section);
-  objfile->sections_end = (objfile->sections + count);
+  objfile->sections_start = OBSTACK_CALLOC (&objfile->objfile_obstack,
+					    count,
+					    struct obj_section);
+  objfile->sections_end = (objfile->sections_start + count);
   for (asection *sect : gdb_bfd_sections (objfile->obfd))
     add_to_objfile_sections (objfile->obfd.get (), sect, objfile, 0);
 
@@ -660,7 +660,7 @@ objfile_relocate1 (struct objfile *objfile,
   struct obj_section *s;
   ALL_OBJFILE_OSECTIONS (objfile, s)
     {
-      int idx = s - objfile->sections;
+      int idx = s - objfile->sections_start;
 
       exec_set_section_address (bfd_get_filename (objfile->obfd.get ()), idx,
 				s->addr ());
diff --git a/gdb/objfiles.h b/gdb/objfiles.h
index 342aa09ac6a..0e326287907 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -128,7 +128,7 @@ struct entry_info
 };
 
 #define ALL_OBJFILE_OSECTIONS(objfile, osect)	\
-  for (osect = objfile->sections; osect < objfile->sections_end; osect++) \
+  for (osect = objfile->sections_start; osect < objfile->sections_end; osect++) \
     if (osect->the_bfd_section == NULL)					\
       {									\
 	/* Nothing.  */							\
@@ -722,7 +722,7 @@ struct objfile
      structure data is only valid for certain sections
      (e.g. non-empty, SEC_ALLOC).  */
 
-  struct obj_section *sections = nullptr;
+  struct obj_section *sections_start = nullptr;
   struct obj_section *sections_end = nullptr;
 
   /* GDB allows to have debug symbols in separate object files.  This is
diff --git a/gdb/solib-aix.c b/gdb/solib-aix.c
index d7062b4ee17..d3119db25bb 100644
--- a/gdb/solib-aix.c
+++ b/gdb/solib-aix.c
@@ -400,7 +400,7 @@ solib_aix_get_section_offsets (struct objfile *objfile,
   if (objfile->sect_index_text != -1)
     {
       struct bfd_section *sect
-	= objfile->sections[objfile->sect_index_text].the_bfd_section;
+	= objfile->sections_start[objfile->sect_index_text].the_bfd_section;
 
       offsets[objfile->sect_index_text]
 	= info->text_addr + sect->filepos - bfd_section_vma (sect);
@@ -411,7 +411,7 @@ solib_aix_get_section_offsets (struct objfile *objfile,
   if (objfile->sect_index_data != -1)
     {
       struct bfd_section *sect
-	= objfile->sections[objfile->sect_index_data].the_bfd_section;
+	= objfile->sections_start[objfile->sect_index_data].the_bfd_section;
 
       offsets[objfile->sect_index_data]
 	= info->data_addr - bfd_section_vma (sect);
diff --git a/gdb/solib-dsbt.c b/gdb/solib-dsbt.c
index 866087ef3a5..8106c342b15 100644
--- a/gdb/solib-dsbt.c
+++ b/gdb/solib-dsbt.c
@@ -822,7 +822,7 @@ dsbt_relocate_main_executable (void)
       int osect_idx;
       int seg;
 
-      osect_idx = osect - objf->sections;
+      osect_idx = osect - objf->sections_start;
 
       /* Current address of section.  */
       addr = osect->addr ();
diff --git a/gdb/solib-frv.c b/gdb/solib-frv.c
index 240b4da5d9d..7cce11d52da 100644
--- a/gdb/solib-frv.c
+++ b/gdb/solib-frv.c
@@ -757,7 +757,7 @@ frv_relocate_main_executable (void)
       int osect_idx;
       int seg;
       
-      osect_idx = osect - objf->sections;
+      osect_idx = osect - objf->sections_start;
 
       /* Current address of section.  */
       addr = osect->addr ();
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 8ae2177b159..3e84cd0e058 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -2571,7 +2571,7 @@ reread_symbols (int from_tty)
 	  /* NB: after this call to obstack_free, objfiles_changed
 	     will need to be called (see discussion below).  */
 	  obstack_free (&objfile->objfile_obstack, 0);
-	  objfile->sections = NULL;
+	  objfile->sections_start = NULL;
 	  objfile->section_offsets.clear ();
 	  objfile->sect_index_bss = -1;
 	  objfile->sect_index_data = -1;
diff --git a/gdb/symmisc.c b/gdb/symmisc.c
index 3d7fd560956..fb8a3ebf602 100644
--- a/gdb/symmisc.c
+++ b/gdb/symmisc.c
@@ -212,7 +212,7 @@ dump_msymbols (struct objfile *objfile, struct ui_file *outfile)
 			bfd_section_name (section->the_bfd_section));
 	  else
 	    gdb_printf (outfile, " spurious section %ld",
-			(long) (section - objfile->sections));
+			(long) (section - objfile->sections_start));
 	}
       if (msymbol->demangled_name () != NULL)
 	{
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 9e9798676cb..6c70d6e5ef1 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -1089,7 +1089,7 @@ struct obj_section *
 general_symbol_info::obj_section (const struct objfile *objfile) const
 {
   if (section_index () >= 0)
-    return &objfile->sections[section_index ()];
+    return &objfile->sections_start[section_index ()];
   return nullptr;
 }
 
@@ -1767,7 +1767,7 @@ fixup_symbol_section (struct symbol *sym, struct objfile *objfile)
 	  if ((bfd_section_flags (s->the_bfd_section) & SEC_ALLOC) == 0)
 	    continue;
 
-	  int idx = s - objfile->sections;
+	  int idx = s - objfile->sections_start;
 	  CORE_ADDR offset = objfile->section_offsets[idx];
 
 	  if (fallback == -1)
diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
index d71127b40f6..d6c91cf7709 100644
--- a/gdb/xcoffread.c
+++ b/gdb/xcoffread.c
@@ -2888,7 +2888,8 @@ xcoff_symfile_offsets (struct objfile *objfile,
   if (objfile->section_offsets.empty ())
     return; /* Is that even possible?  Better safe than sorry.  */
 
-  first_section_name = bfd_section_name (objfile->sections[0].the_bfd_section);
+  first_section_name
+    = bfd_section_name (objfile->sections_start[0].the_bfd_section);
 
   if (objfile->sect_index_text == 0
       && strcmp (first_section_name, ".text") != 0)

-- 
2.39.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/4] Remove ALL_OBJFILE_OSECTIONS
  2023-04-16 16:28 [PATCH 0/4] Remove remaining ALL_ iterator macros Tom Tromey
  2023-04-16 16:28 ` [PATCH 1/4] Rename objfile::sections Tom Tromey
@ 2023-04-16 16:28 ` Tom Tromey
  2023-04-16 16:28 ` [PATCH 3/4] Remove ALL_DICT_SYMBOLS Tom Tromey
  2023-04-16 16:28 ` [PATCH 4/4] Remove ALL_BREAKPOINTS_SAFE Tom Tromey
  3 siblings, 0 replies; 9+ messages in thread
From: Tom Tromey @ 2023-04-16 16:28 UTC (permalink / raw)
  To: gdb-patches

This replaces ALL_OBJFILE_OSECTIONS with an iterator so that for-each
can be used.
---
 gdb/arm-tdep.c        |   4 +-
 gdb/exec.c            |   4 +-
 gdb/gcore.c           |   6 +-
 gdb/hppa-bsd-tdep.c   |  59 +++++++++----------
 gdb/hppa-linux-tdep.c |  64 ++++++++++-----------
 gdb/hppa-tdep.c       |  29 +++++-----
 gdb/ia64-tdep.c       |  99 ++++++++++++++++----------------
 gdb/machoread.c       |   5 +-
 gdb/maint.c           |   4 +-
 gdb/minsyms.c         |   4 +-
 gdb/objfiles.c        |  17 ++----
 gdb/objfiles.h        | 153 +++++++++++++++++++++++++++++++++++---------------
 gdb/printcmd.c        |   3 +-
 gdb/solib-aix.c       |   4 +-
 gdb/solib-dsbt.c      |   3 +-
 gdb/solib-frv.c       |   3 +-
 gdb/symfile.c         |  36 +++++-------
 gdb/symtab.c          |   4 +-
 gdb/xstormy16-tdep.c  |  53 +++++++++--------
 gdb/z80-tdep.c        |  16 +++---
 20 files changed, 294 insertions(+), 276 deletions(-)

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index bfe7d63ed8f..88377edd7e8 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -2491,9 +2491,7 @@ static const registry<bfd>::key<arm_exidx_data> arm_exidx_data_key;
 static struct obj_section *
 arm_obj_section_from_vma (struct objfile *objfile, bfd_vma vma)
 {
-  struct obj_section *osect;
-
-  ALL_OBJFILE_OSECTIONS (objfile, osect)
+  for (obj_section *osect : objfile->sections ())
     if (bfd_section_flags (osect->the_bfd_section) & SEC_ALLOC)
       {
 	bfd_vma start, size;
diff --git a/gdb/exec.c b/gdb/exec.c
index ad543c9fc9e..07759725711 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -625,12 +625,10 @@ program_space::add_target_sections (void *owner,
 void
 program_space::add_target_sections (struct objfile *objfile)
 {
-  struct obj_section *osect;
-
   gdb_assert (objfile != nullptr);
 
   /* Compute the number of sections to add.  */
-  ALL_OBJFILE_OSECTIONS (objfile, osect)
+  for (obj_section *osect : objfile->sections ())
     {
       if (bfd_section_size (osect->the_bfd_section) == 0)
 	continue;
diff --git a/gdb/gcore.c b/gdb/gcore.c
index 973abadb013..05cad94526e 100644
--- a/gdb/gcore.c
+++ b/gdb/gcore.c
@@ -406,10 +406,9 @@ gcore_create_callback (CORE_ADDR vaddr, unsigned long size, int read,
     {
       /* See if this region of memory lies inside a known file on disk.
 	 If so, we can avoid copying its contents by clearing SEC_LOAD.  */
-      struct obj_section *objsec;
 
       for (objfile *objfile : current_program_space->objfiles ())
-	ALL_OBJFILE_OSECTIONS (objfile, objsec)
+	for (obj_section *objsec : objfile->sections ())
 	  {
 	    bfd *abfd = objfile->obfd.get ();
 	    asection *asec = objsec->the_bfd_section;
@@ -513,12 +512,11 @@ objfile_find_memory_regions (struct target_ops *self,
 			     find_memory_region_ftype func, void *obfd)
 {
   /* Use objfile data to create memory sections.  */
-  struct obj_section *objsec;
   bfd_vma temp_bottom, temp_top;
 
   /* Call callback function for each objfile section.  */
   for (objfile *objfile : current_program_space->objfiles ())
-    ALL_OBJFILE_OSECTIONS (objfile, objsec)
+    for (obj_section *objsec : objfile->sections ())
       {
 	asection *isec = objsec->the_bfd_section;
 	flagword flags = bfd_section_flags (isec);
diff --git a/gdb/hppa-bsd-tdep.c b/gdb/hppa-bsd-tdep.c
index c8b044fdbfd..746956711b3 100644
--- a/gdb/hppa-bsd-tdep.c
+++ b/gdb/hppa-bsd-tdep.c
@@ -54,48 +54,45 @@ hppabsd_find_global_pointer (struct gdbarch *gdbarch, struct value *function)
   faddr_sec = find_pc_section (faddr);
   if (faddr_sec != NULL)
     {
-      struct obj_section *sec;
-
-      ALL_OBJFILE_OSECTIONS (faddr_sec->objfile, sec)
+      for (struct obj_section *sec : faddr_sec->objfile->sections ())
 	{
 	  if (strcmp (sec->the_bfd_section->name, ".dynamic") == 0)
-	    break;
-	}
-
-      if (sec < faddr_sec->objfile->sections_end)
-	{
-	  CORE_ADDR addr = sec->addr ();
-	  CORE_ADDR endaddr = sec->endaddr ();
-
-	  while (addr < endaddr)
 	    {
-	      gdb_byte buf[4];
-	      LONGEST tag;
-
-	      if (target_read_memory (addr, buf, sizeof buf) != 0)
-		break;
+	      CORE_ADDR addr = sec->addr ();
+	      CORE_ADDR endaddr = sec->endaddr ();
 
-	      tag = extract_signed_integer (buf, byte_order);
-	      if (tag == DT_PLTGOT)
+	      while (addr < endaddr)
 		{
-		  CORE_ADDR pltgot;
+		  gdb_byte buf[4];
+		  LONGEST tag;
 
-		  if (target_read_memory (addr + 4, buf, sizeof buf) != 0)
+		  if (target_read_memory (addr, buf, sizeof buf) != 0)
 		    break;
 
-		  /* The NetBSD/OpenBSD ld.so doesn't relocate DT_PLTGOT, so
-		     we have to do it ourselves.  */
-		  pltgot = extract_unsigned_integer (buf, sizeof buf,
-						     byte_order);
-		  pltgot += sec->objfile->text_section_offset ();
+		  tag = extract_signed_integer (buf, byte_order);
+		  if (tag == DT_PLTGOT)
+		    {
+		      CORE_ADDR pltgot;
 
-		  return pltgot;
-		}
+		      if (target_read_memory (addr + 4, buf, sizeof buf) != 0)
+			break;
+
+		      /* The NetBSD/OpenBSD ld.so doesn't relocate
+			 DT_PLTGOT, so we have to do it ourselves.  */
+		      pltgot = extract_unsigned_integer (buf, sizeof buf,
+							 byte_order);
+		      pltgot += sec->objfile->text_section_offset ();
+
+		      return pltgot;
+		    }
 
-	      if (tag == DT_NULL)
-		break;
+		  if (tag == DT_NULL)
+		    break;
+
+		  addr += 8;
+		}
 
-	      addr += 8;
+	      break;
 	    }
 	}
     }
diff --git a/gdb/hppa-linux-tdep.c b/gdb/hppa-linux-tdep.c
index 32b13ae0575..1f440d8cbdc 100644
--- a/gdb/hppa-linux-tdep.c
+++ b/gdb/hppa-linux-tdep.c
@@ -360,49 +360,47 @@ hppa_linux_find_global_pointer (struct gdbarch *gdbarch,
   faddr_sect = find_pc_section (faddr);
   if (faddr_sect != NULL)
     {
-      struct obj_section *osect;
-
-      ALL_OBJFILE_OSECTIONS (faddr_sect->objfile, osect)
+      for (obj_section *osect : faddr_sect->objfile->sections ())
 	{
 	  if (strcmp (osect->the_bfd_section->name, ".dynamic") == 0)
-	    break;
-	}
-
-      if (osect < faddr_sect->objfile->sections_end)
-	{
-	  CORE_ADDR addr, endaddr;
-
-	  addr = osect->addr ();
-	  endaddr = osect->endaddr ();
-
-	  while (addr < endaddr)
 	    {
-	      int status;
-	      LONGEST tag;
-	      gdb_byte buf[4];
+	      CORE_ADDR addr, endaddr;
 
-	      status = target_read_memory (addr, buf, sizeof (buf));
-	      if (status != 0)
-		break;
-	      tag = extract_signed_integer (buf, byte_order);
+	      addr = osect->addr ();
+	      endaddr = osect->endaddr ();
 
-	      if (tag == DT_PLTGOT)
+	      while (addr < endaddr)
 		{
-		  CORE_ADDR global_pointer;
+		  int status;
+		  LONGEST tag;
+		  gdb_byte buf[4];
 
-		  status = target_read_memory (addr + 4, buf, sizeof (buf));
+		  status = target_read_memory (addr, buf, sizeof (buf));
 		  if (status != 0)
 		    break;
-		  global_pointer = extract_unsigned_integer (buf, sizeof (buf),
-							     byte_order);
-		  /* The payoff...  */
-		  return global_pointer;
-		}
-
-	      if (tag == DT_NULL)
-		break;
+		  tag = extract_signed_integer (buf, byte_order);
+
+		  if (tag == DT_PLTGOT)
+		    {
+		      CORE_ADDR global_pointer;
+
+		      status = target_read_memory (addr + 4, buf,
+						   sizeof (buf));
+		      if (status != 0)
+			break;
+		      global_pointer
+			= extract_unsigned_integer (buf, sizeof (buf),
+						    byte_order);
+		      /* The payoff...  */
+		      return global_pointer;
+		    }
+
+		  if (tag == DT_NULL)
+		    break;
 
-	      addr += 8;
+		  addr += 8;
+		}
+	      break;
 	    }
 	}
     }
diff --git a/gdb/hppa-tdep.c b/gdb/hppa-tdep.c
index d054e8011e8..b7c96e8b050 100644
--- a/gdb/hppa-tdep.c
+++ b/gdb/hppa-tdep.c
@@ -910,7 +910,7 @@ static CORE_ADDR
 hppa64_convert_code_addr_to_fptr (struct gdbarch *gdbarch, CORE_ADDR code)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
-  struct obj_section *sec, *opd;
+  struct obj_section *sec;
 
   sec = find_pc_section (code);
 
@@ -921,25 +921,24 @@ hppa64_convert_code_addr_to_fptr (struct gdbarch *gdbarch, CORE_ADDR code)
   if (!(sec->the_bfd_section->flags & SEC_CODE))
     return code;
 
-  ALL_OBJFILE_OSECTIONS (sec->objfile, opd)
+  for (obj_section *opd : sec->objfile->sections ())
     {
       if (strcmp (opd->the_bfd_section->name, ".opd") == 0)
-	break;
-    }
-
-  if (opd < sec->objfile->sections_end)
-    {
-      for (CORE_ADDR addr = opd->addr (); addr < opd->endaddr (); addr += 2 * 8)
 	{
-	  ULONGEST opdaddr;
-	  gdb_byte tmp[8];
+	  for (CORE_ADDR addr = opd->addr ();
+	       addr < opd->endaddr ();
+	       addr += 2 * 8)
+	    {
+	      ULONGEST opdaddr;
+	      gdb_byte tmp[8];
 
-	  if (target_read_memory (addr, tmp, sizeof (tmp)))
-	      break;
-	  opdaddr = extract_unsigned_integer (tmp, sizeof (tmp), byte_order);
+	      if (target_read_memory (addr, tmp, sizeof (tmp)))
+		break;
+	      opdaddr = extract_unsigned_integer (tmp, sizeof (tmp), byte_order);
 
-	  if (opdaddr == code)
-	    return addr - 16;
+	      if (opdaddr == code)
+		return addr - 16;
+	    }
 	}
     }
 
diff --git a/gdb/ia64-tdep.c b/gdb/ia64-tdep.c
index 37e5ce95539..27da839e27d 100644
--- a/gdb/ia64-tdep.c
+++ b/gdb/ia64-tdep.c
@@ -3431,48 +3431,47 @@ ia64_find_global_pointer_from_dynamic_section (struct gdbarch *gdbarch,
   faddr_sect = find_pc_section (faddr);
   if (faddr_sect != NULL)
     {
-      struct obj_section *osect;
-
-      ALL_OBJFILE_OSECTIONS (faddr_sect->objfile, osect)
+      for (obj_section *osect : faddr_sect->objfile->sections ())
 	{
 	  if (strcmp (osect->the_bfd_section->name, ".dynamic") == 0)
-	    break;
-	}
-
-      if (osect < faddr_sect->objfile->sections_end)
-	{
-	  CORE_ADDR addr = osect->addr ();
-	  CORE_ADDR endaddr = osect->endaddr ();
-
-	  while (addr < endaddr)
 	    {
-	      int status;
-	      LONGEST tag;
-	      gdb_byte buf[8];
+	      CORE_ADDR addr = osect->addr ();
+	      CORE_ADDR endaddr = osect->endaddr ();
 
-	      status = target_read_memory (addr, buf, sizeof (buf));
-	      if (status != 0)
-		break;
-	      tag = extract_signed_integer (buf, byte_order);
-
-	      if (tag == DT_PLTGOT)
+	      while (addr < endaddr)
 		{
-		  CORE_ADDR global_pointer;
+		  int status;
+		  LONGEST tag;
+		  gdb_byte buf[8];
 
-		  status = target_read_memory (addr + 8, buf, sizeof (buf));
+		  status = target_read_memory (addr, buf, sizeof (buf));
 		  if (status != 0)
 		    break;
-		  global_pointer = extract_unsigned_integer (buf, sizeof (buf),
-							     byte_order);
+		  tag = extract_signed_integer (buf, byte_order);
 
-		  /* The payoff...  */
-		  return global_pointer;
-		}
+		  if (tag == DT_PLTGOT)
+		    {
+		      CORE_ADDR global_pointer;
+
+		      status = target_read_memory (addr + 8, buf,
+						   sizeof (buf));
+		      if (status != 0)
+			break;
+		      global_pointer
+			= extract_unsigned_integer (buf, sizeof (buf),
+						    byte_order);
+
+		      /* The payoff...  */
+		      return global_pointer;
+		    }
+
+		  if (tag == DT_NULL)
+		    break;
 
-	      if (tag == DT_NULL)
-		break;
+		  addr += 16;
+		}
 
-	      addr += 16;
+	      break;
 	    }
 	}
     }
@@ -3513,33 +3512,31 @@ find_extant_func_descr (struct gdbarch *gdbarch, CORE_ADDR faddr)
 
   if (faddr_sect != NULL)
     {
-      struct obj_section *osect;
-      ALL_OBJFILE_OSECTIONS (faddr_sect->objfile, osect)
+      for (obj_section *osect : faddr_sect->objfile->sections ())
 	{
 	  if (strcmp (osect->the_bfd_section->name, ".opd") == 0)
-	    break;
-	}
+	    {
+	      CORE_ADDR addr = osect->addr ();
+	      CORE_ADDR endaddr = osect->endaddr ();
 
-      if (osect < faddr_sect->objfile->sections_end)
-	{
-	  CORE_ADDR addr = osect->addr ();
-	  CORE_ADDR endaddr = osect->endaddr ();
+	      while (addr < endaddr)
+		{
+		  int status;
+		  LONGEST faddr2;
+		  gdb_byte buf[8];
 
-	  while (addr < endaddr)
-	    {
-	      int status;
-	      LONGEST faddr2;
-	      gdb_byte buf[8];
+		  status = target_read_memory (addr, buf, sizeof (buf));
+		  if (status != 0)
+		    break;
+		  faddr2 = extract_signed_integer (buf, byte_order);
 
-	      status = target_read_memory (addr, buf, sizeof (buf));
-	      if (status != 0)
-		break;
-	      faddr2 = extract_signed_integer (buf, byte_order);
+		  if (faddr == faddr2)
+		    return addr;
 
-	      if (faddr == faddr2)
-		return addr;
+		  addr += 16;
+		}
 
-	      addr += 16;
+	      break;
 	    }
 	}
     }
diff --git a/gdb/machoread.c b/gdb/machoread.c
index dc841c30af2..daf62563754 100644
--- a/gdb/machoread.c
+++ b/gdb/machoread.c
@@ -893,7 +893,6 @@ macho_symfile_offsets (struct objfile *objfile,
 		       const section_addr_info &addrs)
 {
   unsigned int i;
-  struct obj_section *osect;
 
   /* Allocate section_offsets.  */
   objfile->section_offsets.assign (gdb_bfd_count_sections (objfile->obfd.get ()), 0);
@@ -909,7 +908,7 @@ macho_symfile_offsets (struct objfile *objfile,
 
   for (i = 0; i < addrs.size (); i++)
     {
-      ALL_OBJFILE_OSECTIONS (objfile, osect)
+      for (obj_section *osect : objfile->sections ())
 	{
 	  const char *bfd_sect_name = osect->the_bfd_section->name;
 
@@ -923,7 +922,7 @@ macho_symfile_offsets (struct objfile *objfile,
 
   objfile->sect_index_text = 0;
 
-  ALL_OBJFILE_OSECTIONS (objfile, osect)
+  for (obj_section *osect : objfile->sections ())
     {
       const char *bfd_sect_name = osect->the_bfd_section->name;
       int sect_index = osect - objfile->sections_start;
diff --git a/gdb/maint.c b/gdb/maint.c
index 3cd2c5e899a..c5f2e5cdce0 100644
--- a/gdb/maint.c
+++ b/gdb/maint.c
@@ -566,9 +566,9 @@ maintenance_translate_address (const char *arg, int from_tty)
       p = skip_spaces (p + 1);
 
       for (objfile *objfile : current_program_space->objfiles ())
-	ALL_OBJFILE_OSECTIONS (objfile, sect)
+	for (obj_section *iter : objfile->sections ())
 	  {
-	    if (strncmp (sect->the_bfd_section->name, arg, arg_len) == 0)
+	    if (strncmp (iter->the_bfd_section->name, arg, arg_len) == 0)
 	      goto found;
 	  }
 
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index 3fa07f47b9f..eb4ae055baa 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -679,9 +679,7 @@ static int
 frob_address (struct objfile *objfile, CORE_ADDR pc,
 	      unrelocated_addr *unrel_addr)
 {
-  struct obj_section *iter;
-
-  ALL_OBJFILE_OSECTIONS (objfile, iter)
+  for (obj_section *iter : objfile->sections ())
     {
       if (pc >= iter->addr () && pc < iter->endaddr ())
 	{
diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index e3fa691dd53..3fefc4ad846 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -657,8 +657,7 @@ objfile_relocate1 (struct objfile *objfile,
   get_objfile_pspace_data (objfile->pspace)->section_map_dirty = 1;
 
   /* Update the table in exec_ops, used to read memory.  */
-  struct obj_section *s;
-  ALL_OBJFILE_OSECTIONS (objfile, s)
+  for (obj_section *s : objfile->sections ())
     {
       int idx = s - objfile->sections_start;
 
@@ -876,9 +875,7 @@ sort_cmp (const struct obj_section *sect1, const obj_section *sect2)
 	     second case shouldn't occur during normal use, but std::sort
 	     does check that '!(a < a)' when compiled in debug mode.  */
 
-	  const struct obj_section *osect;
-
-	  ALL_OBJFILE_OSECTIONS (objfile1, osect)
+	  for (const obj_section *osect : objfile1->sections ())
 	    if (osect == sect2)
 	      return false;
 	    else if (osect == sect1)
@@ -1071,7 +1068,7 @@ update_section_map (struct program_space *pspace,
 {
   struct objfile_pspace_info *pspace_info;
   int alloc_size, map_size, i;
-  struct obj_section *s, **map;
+  struct obj_section **map;
 
   pspace_info = get_objfile_pspace_data (pspace);
   gdb_assert (pspace_info->section_map_dirty != 0
@@ -1082,7 +1079,7 @@ update_section_map (struct program_space *pspace,
 
   alloc_size = 0;
   for (objfile *objfile : pspace->objfiles ())
-    ALL_OBJFILE_OSECTIONS (objfile, s)
+    for (obj_section *s : objfile->sections ())
       if (insert_section_p (objfile->obfd.get (), s->the_bfd_section))
 	alloc_size += 1;
 
@@ -1098,7 +1095,7 @@ update_section_map (struct program_space *pspace,
 
   i = 0;
   for (objfile *objfile : pspace->objfiles ())
-    ALL_OBJFILE_OSECTIONS (objfile, s)
+    for (obj_section *s : objfile->sections ())
       if (insert_section_p (objfile->obfd.get (), s->the_bfd_section))
 	map[i++] = s;
 
@@ -1214,12 +1211,10 @@ inhibit_section_map_updates (struct program_space *pspace)
 bool
 is_addr_in_objfile (CORE_ADDR addr, const struct objfile *objfile)
 {
-  struct obj_section *osect;
-
   if (objfile == NULL)
     return false;
 
-  ALL_OBJFILE_OSECTIONS (objfile, osect)
+  for (obj_section *osect : objfile->sections ())
     {
       if (section_is_overlay (osect) && !section_is_mapped (osect))
 	continue;
diff --git a/gdb/objfiles.h b/gdb/objfiles.h
index 0e326287907..b00b035ffd9 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -127,14 +127,6 @@ struct entry_info
   unsigned initialized : 1;
 };
 
-#define ALL_OBJFILE_OSECTIONS(objfile, osect)	\
-  for (osect = objfile->sections_start; osect < objfile->sections_end; osect++) \
-    if (osect->the_bfd_section == NULL)					\
-      {									\
-	/* Nothing.  */							\
-      }									\
-    else
-
 #define SECT_OFF_DATA(objfile) \
      ((objfile->sect_index_data == -1) \
       ? (internal_error (_("sect_index_data not initialized")), -1)	\
@@ -379,6 +371,40 @@ class separate_debug_iterator
 
 typedef iterator_range<separate_debug_iterator> separate_debug_range;
 
+/* Sections in an objfile.  The section offsets are stored in the
+   OBJFILE.  */
+
+struct obj_section
+{
+  /* Relocation offset applied to the section.  */
+  CORE_ADDR offset () const;
+
+  /* Set the relocation offset applied to the section.  */
+  void set_offset (CORE_ADDR offset);
+
+  /* The memory address of the section (vma + offset).  */
+  CORE_ADDR addr () const
+  {
+    return bfd_section_vma (this->the_bfd_section) + this->offset ();
+  }
+
+  /* The one-passed-the-end memory address of the section
+     (vma + size + offset).  */
+  CORE_ADDR endaddr () const
+  {
+    return this->addr () + bfd_section_size (this->the_bfd_section);
+  }
+
+  /* BFD section pointer */
+  struct bfd_section *the_bfd_section;
+
+  /* Objfile this section is part of.  */
+  struct objfile *objfile;
+
+  /* True if this "overlay section" is mapped into an "overlay region".  */
+  int ovly_mapped;
+};
+
 /* Master structure for keeping track of each file from which
    gdb reads symbols.  There are several ways these get allocated: 1.
    The main symbol file, symfile_objfile, set by the symbol-file command,
@@ -609,6 +635,68 @@ struct objfile
     this->section_offsets[idx] = offset;
   }
 
+  class section_iterator
+  {
+  public:
+    section_iterator (const section_iterator &) = default;
+    section_iterator (section_iterator &&) = default;
+    section_iterator &operator= (const section_iterator &) = default;
+    section_iterator &operator= (section_iterator &&) = default;
+
+    typedef section_iterator self_type;
+    typedef obj_section *value_type;
+
+    value_type operator* ()
+    { return m_iter; }
+
+    section_iterator &operator++ ()
+    {
+      ++m_iter;
+      skip_null ();
+      return *this;
+    }
+
+    bool operator== (const section_iterator &other) const
+    { return m_iter == other.m_iter && m_end == other.m_end; }
+
+    bool operator!= (const section_iterator &other) const
+    { return !(*this == other); }
+
+  private:
+
+    friend class objfile;
+
+    section_iterator (obj_section *iter, obj_section *end)
+      : m_iter (iter),
+	m_end (end)
+    {
+      skip_null ();
+    }
+
+    void skip_null ()
+    {
+      while (m_iter < m_end && m_iter->the_bfd_section == nullptr)
+	++m_iter;
+    }
+
+    value_type m_iter;
+    value_type m_end;
+  };
+
+  iterator_range<section_iterator> sections ()
+  {
+    return (iterator_range<section_iterator>
+	    (section_iterator (sections_start, sections_end),
+	     section_iterator (sections_end, sections_end)));
+  }
+
+  iterator_range<section_iterator> sections () const
+  {
+    return (iterator_range<section_iterator>
+	    (section_iterator (sections_start, sections_end),
+	     section_iterator (sections_end, sections_end)));
+  }
+
 private:
 
   /* Ensure that partial symbols have been read and return the "quick" (aka
@@ -800,46 +888,19 @@ struct objfile_deleter
 
 typedef std::unique_ptr<objfile, objfile_deleter> objfile_up;
 
-
-/* Sections in an objfile.  The section offsets are stored in the
-   OBJFILE.  */
-
-struct obj_section
+/* Relocation offset applied to the section.  */
+inline CORE_ADDR
+obj_section::offset () const
 {
-  /* Relocation offset applied to the section.  */
-  CORE_ADDR offset () const
-  {
-    return this->objfile->section_offset (this->the_bfd_section);
-  }
-
-  /* Set the relocation offset applied to the section.  */
-  void set_offset (CORE_ADDR offset)
-  {
-    this->objfile->set_section_offset (this->the_bfd_section, offset);
-  }
-
-  /* The memory address of the section (vma + offset).  */
-  CORE_ADDR addr () const
-  {
-    return bfd_section_vma (this->the_bfd_section) + this->offset ();
-  }
-
-  /* The one-passed-the-end memory address of the section
-     (vma + size + offset).  */
-  CORE_ADDR endaddr () const
-  {
-    return this->addr () + bfd_section_size (this->the_bfd_section);
-  }
-
-  /* BFD section pointer */
-  struct bfd_section *the_bfd_section;
-
-  /* Objfile this section is part of.  */
-  struct objfile *objfile;
+  return this->objfile->section_offset (this->the_bfd_section);
+}
 
-  /* True if this "overlay section" is mapped into an "overlay region".  */
-  int ovly_mapped;
-};
+/* Set the relocation offset applied to the section.  */
+inline void
+obj_section::set_offset (CORE_ADDR offset)
+{
+  this->objfile->set_section_offset (this->the_bfd_section, offset);
+}
 
 /* Declarations for functions defined in objfiles.c */
 
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index dd92e31d31b..51eb3f5a95e 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -1541,7 +1541,6 @@ static void
 info_symbol_command (const char *arg, int from_tty)
 {
   struct minimal_symbol *msymbol;
-  struct obj_section *osect;
   CORE_ADDR addr, sect_addr;
   int matches = 0;
   unsigned int offset;
@@ -1551,7 +1550,7 @@ info_symbol_command (const char *arg, int from_tty)
 
   addr = parse_and_eval_address (arg);
   for (objfile *objfile : current_program_space->objfiles ())
-    ALL_OBJFILE_OSECTIONS (objfile, osect)
+    for (obj_section *osect : objfile->sections ())
       {
 	/* Only process each object file once, even if there's a separate
 	   debug file.  */
diff --git a/gdb/solib-aix.c b/gdb/solib-aix.c
index d3119db25bb..93aa6c4e040 100644
--- a/gdb/solib-aix.c
+++ b/gdb/solib-aix.c
@@ -662,9 +662,7 @@ solib_aix_bfd_open (const char *pathname)
 static struct obj_section *
 data_obj_section_from_objfile (struct objfile *objfile)
 {
-  struct obj_section *osect;
-
-  ALL_OBJFILE_OSECTIONS (objfile, osect)
+  for (obj_section *osect : objfile->sections ())
     if (strcmp (bfd_section_name (osect->the_bfd_section), ".data") == 0)
       return osect;
 
diff --git a/gdb/solib-dsbt.c b/gdb/solib-dsbt.c
index 8106c342b15..6dcb8d22b56 100644
--- a/gdb/solib-dsbt.c
+++ b/gdb/solib-dsbt.c
@@ -802,7 +802,6 @@ dsbt_relocate_main_executable (void)
 {
   struct int_elf32_dsbt_loadmap *ldm;
   int changed;
-  struct obj_section *osect;
   struct dsbt_info *info = get_dsbt_info ();
 
   dsbt_get_initial_loadmaps ();
@@ -816,7 +815,7 @@ dsbt_relocate_main_executable (void)
   section_offsets new_offsets (objf->section_offsets.size ());
   changed = 0;
 
-  ALL_OBJFILE_OSECTIONS (objf, osect)
+  for (obj_section *osect : objf->sections ())
     {
       CORE_ADDR orig_addr, addr, offset;
       int osect_idx;
diff --git a/gdb/solib-frv.c b/gdb/solib-frv.c
index 7cce11d52da..8b0e3a6e0b0 100644
--- a/gdb/solib-frv.c
+++ b/gdb/solib-frv.c
@@ -727,7 +727,6 @@ frv_relocate_main_executable (void)
   CORE_ADDR exec_addr, interp_addr;
   struct int_elf32_fdpic_loadmap *ldm;
   int changed;
-  struct obj_section *osect;
 
   status = frv_fdpic_loadmap_addresses (target_gdbarch (),
 					&interp_addr, &exec_addr);
@@ -751,7 +750,7 @@ frv_relocate_main_executable (void)
   section_offsets new_offsets (objf->section_offsets.size ());
   changed = 0;
 
-  ALL_OBJFILE_OSECTIONS (objf, osect)
+  for (obj_section *osect : objf->sections ())
     {
       CORE_ADDR orig_addr, addr, offset;
       int osect_idx;
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 3e84cd0e058..30b21f54a44 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -832,7 +832,6 @@ init_entry_point_info (struct objfile *objfile)
 
   if (ei->entry_point_p)
     {
-      struct obj_section *osect;
       CORE_ADDR entry_point =  ei->entry_point;
       int found;
 
@@ -847,7 +846,7 @@ init_entry_point_info (struct objfile *objfile)
 	= gdbarch_addr_bits_remove (objfile->arch (), entry_point);
 
       found = 0;
-      ALL_OBJFILE_OSECTIONS (objfile, osect)
+      for (obj_section *osect : objfile->sections ())
 	{
 	  struct bfd_section *sect = osect->the_bfd_section;
 
@@ -2999,10 +2998,8 @@ section_is_overlay (struct obj_section *section)
 static void
 overlay_invalidate_all (void)
 {
-  struct obj_section *sect;
-
   for (objfile *objfile : current_program_space->objfiles ())
-    ALL_OBJFILE_OSECTIONS (objfile, sect)
+    for (obj_section *sect : objfile->sections ())
       if (section_is_overlay (sect))
 	sect->ovly_mapped = -1;
 }
@@ -3174,12 +3171,12 @@ symbol_overlayed_address (CORE_ADDR address, struct obj_section *section)
 struct obj_section *
 find_pc_overlay (CORE_ADDR pc)
 {
-  struct obj_section *osect, *best_match = NULL;
+  struct obj_section *best_match = NULL;
 
   if (overlay_debugging)
     {
       for (objfile *objfile : current_program_space->objfiles ())
-	ALL_OBJFILE_OSECTIONS (objfile, osect)
+	for (obj_section *osect : objfile->sections ())
 	  if (section_is_overlay (osect))
 	    {
 	      if (pc_in_mapped_range (pc, osect))
@@ -3203,12 +3200,10 @@ find_pc_overlay (CORE_ADDR pc)
 struct obj_section *
 find_pc_mapped_section (CORE_ADDR pc)
 {
-  struct obj_section *osect;
-
   if (overlay_debugging)
     {
       for (objfile *objfile : current_program_space->objfiles ())
-	ALL_OBJFILE_OSECTIONS (objfile, osect)
+	for (obj_section *osect : objfile->sections ())
 	  if (pc_in_mapped_range (pc, osect) && section_is_mapped (osect))
 	    return osect;
     }
@@ -3223,12 +3218,11 @@ static void
 list_overlays_command (const char *args, int from_tty)
 {
   int nmapped = 0;
-  struct obj_section *osect;
 
   if (overlay_debugging)
     {
       for (objfile *objfile : current_program_space->objfiles ())
-	ALL_OBJFILE_OSECTIONS (objfile, osect)
+	for (obj_section *osect : objfile->sections ())
 	  if (section_is_mapped (osect))
 	    {
 	      struct gdbarch *gdbarch = objfile->arch ();
@@ -3264,8 +3258,6 @@ list_overlays_command (const char *args, int from_tty)
 static void
 map_overlay_command (const char *args, int from_tty)
 {
-  struct obj_section *sec, *sec2;
-
   if (!overlay_debugging)
     error (_("Overlay debugging not enabled.  Use "
 	     "either the 'overlay auto' or\n"
@@ -3276,7 +3268,7 @@ map_overlay_command (const char *args, int from_tty)
 
   /* First, find a section matching the user supplied argument.  */
   for (objfile *obj_file : current_program_space->objfiles ())
-    ALL_OBJFILE_OSECTIONS (obj_file, sec)
+    for (obj_section *sec : obj_file->sections ())
       if (!strcmp (bfd_section_name (sec->the_bfd_section), args))
 	{
 	  /* Now, check to see if the section is an overlay.  */
@@ -3289,7 +3281,7 @@ map_overlay_command (const char *args, int from_tty)
 	  /* Next, make a pass and unmap any sections that are
 	     overlapped by this new section: */
 	  for (objfile *objfile2 : current_program_space->objfiles ())
-	    ALL_OBJFILE_OSECTIONS (objfile2, sec2)
+	    for (obj_section *sec2 : objfile2->sections ())
 	      if (sec2->ovly_mapped && sec != sec2 && sections_overlap (sec,
 									sec2))
 		{
@@ -3310,8 +3302,6 @@ map_overlay_command (const char *args, int from_tty)
 static void
 unmap_overlay_command (const char *args, int from_tty)
 {
-  struct obj_section *sec = NULL;
-
   if (!overlay_debugging)
     error (_("Overlay debugging not enabled.  "
 	     "Use either the 'overlay auto' or\n"
@@ -3322,7 +3312,7 @@ unmap_overlay_command (const char *args, int from_tty)
 
   /* First, find a section matching the user supplied argument.  */
   for (objfile *objfile : current_program_space->objfiles ())
-    ALL_OBJFILE_OSECTIONS (objfile, sec)
+    for (obj_section *sec : objfile->sections ())
       if (!strcmp (bfd_section_name (sec->the_bfd_section), args))
 	{
 	  if (!sec->ovly_mapped)
@@ -3581,17 +3571,17 @@ simple_overlay_update (struct obj_section *osect)
 
   /* Now may as well update all sections, even if only one was requested.  */
   for (objfile *objfile : current_program_space->objfiles ())
-    ALL_OBJFILE_OSECTIONS (objfile, osect)
-      if (section_is_overlay (osect))
+    for (obj_section *sect : objfile->sections ())
+      if (section_is_overlay (sect))
 	{
 	  int i;
-	  asection *bsect = osect->the_bfd_section;
+	  asection *bsect = sect->the_bfd_section;
 
 	  for (i = 0; i < cache_novlys; i++)
 	    if (cache_ovly_table[i][VMA] == bfd_section_vma (bsect)
 		&& cache_ovly_table[i][LMA] == bfd_section_lma (bsect))
 	      { /* obj_section matches i'th entry in ovly_table.  */
-		osect->ovly_mapped = cache_ovly_table[i][MAPPED];
+		sect->ovly_mapped = cache_ovly_table[i][MAPPED];
 		break;		/* finished with inner for loop: break out.  */
 	      }
 	}
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 6c70d6e5ef1..36d456a4139 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -1760,9 +1760,7 @@ fixup_symbol_section (struct symbol *sym, struct objfile *objfile)
 	 this reason, we still attempt a lookup by name prior to doing
 	 a search of the section table.  */
 
-      struct obj_section *s;
-
-      ALL_OBJFILE_OSECTIONS (objfile, s)
+      for (obj_section *s : objfile->sections ())
 	{
 	  if ((bfd_section_flags (s->the_bfd_section) & SEC_ALLOC) == 0)
 	    continue;
diff --git a/gdb/xstormy16-tdep.c b/gdb/xstormy16-tdep.c
index c505281622a..3f47c8d68a0 100644
--- a/gdb/xstormy16-tdep.c
+++ b/gdb/xstormy16-tdep.c
@@ -541,41 +541,38 @@ xstormy16_find_jmp_table_entry (struct gdbarch *gdbarch, CORE_ADDR faddr)
 
   if (faddr_sect)
     {
-      struct obj_section *osect;
-
       /* Return faddr if it's already a pointer to a jump table entry.  */
       if (!strcmp (faddr_sect->the_bfd_section->name, ".plt"))
 	return faddr;
 
-      ALL_OBJFILE_OSECTIONS (faddr_sect->objfile, osect)
-      {
-	if (!strcmp (osect->the_bfd_section->name, ".plt"))
-	  break;
-      }
-
-      if (osect < faddr_sect->objfile->sections_end)
+      for (obj_section *osect : faddr_sect->objfile->sections ())
 	{
-	  CORE_ADDR addr, endaddr;
-
-	  addr = osect->addr ();
-	  endaddr = osect->endaddr ();
-
-	  for (; addr < endaddr; addr += 2 * xstormy16_inst_size)
+	  if (!strcmp (osect->the_bfd_section->name, ".plt"))
 	    {
-	      LONGEST inst, inst2, faddr2;
-	      gdb_byte buf[2 * xstormy16_inst_size];
+	      CORE_ADDR addr, endaddr;
+
+	      addr = osect->addr ();
+	      endaddr = osect->endaddr ();
+
+	      for (; addr < endaddr; addr += 2 * xstormy16_inst_size)
+		{
+		  LONGEST inst, inst2, faddr2;
+		  gdb_byte buf[2 * xstormy16_inst_size];
+
+		  if (target_read_memory (addr, buf, sizeof buf))
+		    return 0;
+		  inst = extract_unsigned_integer (buf,
+						   xstormy16_inst_size,
+						   byte_order);
+		  inst2 = extract_unsigned_integer (buf + xstormy16_inst_size,
+						    xstormy16_inst_size,
+						    byte_order);
+		  faddr2 = inst2 << 8 | (inst & 0xff);
+		  if (faddr == faddr2)
+		    return addr;
+		}
 
-	      if (target_read_memory (addr, buf, sizeof buf))
-		return 0;
-	      inst = extract_unsigned_integer (buf,
-					       xstormy16_inst_size,
-					       byte_order);
-	      inst2 = extract_unsigned_integer (buf + xstormy16_inst_size,
-						xstormy16_inst_size,
-						byte_order);
-	      faddr2 = inst2 << 8 | (inst & 0xff);
-	      if (faddr == faddr2)
-		return addr;
+	      break;
 	    }
 	}
     }
diff --git a/gdb/z80-tdep.c b/gdb/z80-tdep.c
index 27cdca1c9c7..4f3ad549046 100644
--- a/gdb/z80-tdep.c
+++ b/gdb/z80-tdep.c
@@ -962,11 +962,11 @@ z80_overlay_update_1 (struct obj_section *osect)
 
   /* we have interest for sections with same VMA */
   for (objfile *objfile : current_program_space->objfiles ())
-    ALL_OBJFILE_OSECTIONS (objfile, osect)
-      if (section_is_overlay (osect))
+    for (obj_section *sect : objfile->sections ())
+      if (section_is_overlay (sect))
 	{
-	  osect->ovly_mapped = (lma == bfd_section_lma (osect->the_bfd_section));
-	  i |= osect->ovly_mapped; /* true, if at least one section is mapped */
+	  sect->ovly_mapped = (lma == bfd_section_lma (sect->the_bfd_section));
+	  i |= sect->ovly_mapped; /* true, if at least one section is mapped */
 	}
   return i;
 }
@@ -985,18 +985,18 @@ z80_overlay_update (struct obj_section *osect)
 
   /* Update all sections, even if only one was requested.  */
   for (objfile *objfile : current_program_space->objfiles ())
-    ALL_OBJFILE_OSECTIONS (objfile, osect)
+    for (obj_section *sect : objfile->sections ())
       {
-	if (!section_is_overlay (osect))
+	if (!section_is_overlay (sect))
 	  continue;
 
-	asection *bsect = osect->the_bfd_section;
+	asection *bsect = sect->the_bfd_section;
 	bfd_vma lma = bfd_section_lma (bsect);
 	bfd_vma vma = bfd_section_vma (bsect);
 
 	for (int i = 0; i < cache_novly_regions; ++i)
 	  if (cache_ovly_region_table[i][Z80_VMA] == vma)
-	    osect->ovly_mapped =
+	    sect->ovly_mapped =
 	      (cache_ovly_region_table[i][Z80_MAPPED_TO_LMA] == lma);
       }
 }

-- 
2.39.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 3/4] Remove ALL_DICT_SYMBOLS
  2023-04-16 16:28 [PATCH 0/4] Remove remaining ALL_ iterator macros Tom Tromey
  2023-04-16 16:28 ` [PATCH 1/4] Rename objfile::sections Tom Tromey
  2023-04-16 16:28 ` [PATCH 2/4] Remove ALL_OBJFILE_OSECTIONS Tom Tromey
@ 2023-04-16 16:28 ` Tom Tromey
  2023-04-16 16:28 ` [PATCH 4/4] Remove ALL_BREAKPOINTS_SAFE Tom Tromey
  3 siblings, 0 replies; 9+ messages in thread
From: Tom Tromey @ 2023-04-16 16:28 UTC (permalink / raw)
  To: gdb-patches

This replaces ALL_DICT_SYMBOLS with an iterator so that for-each can
be used.
---
 gdb/block.h      |  4 ++++
 gdb/buildsym.c   | 15 ++++++---------
 gdb/dictionary.h | 55 ++++++++++++++++++++++++++++++++++++++++++++-----------
 gdb/objfiles.c   |  9 ++-------
 gdb/symmisc.c    |  4 +---
 5 files changed, 57 insertions(+), 30 deletions(-)

diff --git a/gdb/block.h b/gdb/block.h
index cdcee0844ec..f132d351bb6 100644
--- a/gdb/block.h
+++ b/gdb/block.h
@@ -143,6 +143,10 @@ struct block : public allocate_on_obstack
   multidictionary *multidict () const
   { return m_multidict; }
 
+  /* Return an iterator range for this block's multidict.  */
+  iterator_range<mdict_iterator_wrapper> multidict_symbols () const
+  { return iterator_range<mdict_iterator_wrapper> (m_multidict); }
+
   /* Set this block's multidict.  */
   void set_multidict (multidictionary *multidict)
   { m_multidict = multidict; }
diff --git a/gdb/buildsym.c b/gdb/buildsym.c
index f000233dafa..d12ad2187ab 100644
--- a/gdb/buildsym.c
+++ b/gdb/buildsym.c
@@ -244,7 +244,6 @@ buildsym_compunit::finish_block_internal
   if (symbol)
     {
       struct type *ftype = symbol->type ();
-      struct mdict_iterator miter;
       symbol->set_value_block (block);
       symbol->set_section_index (SECT_OFF_TEXT (m_objfile));
       block->set_function (symbol);
@@ -255,11 +254,10 @@ buildsym_compunit::finish_block_internal
 	     function's type.  Set that from the type of the
 	     parameter symbols.  */
 	  int nparams = 0, iparams;
-	  struct symbol *sym;
 
 	  /* Here we want to directly access the dictionary, because
 	     we haven't fully initialized the block yet.  */
-	  ALL_DICT_SYMBOLS (block->multidict (), miter, sym)
+	  for (struct symbol *sym : block->multidict_symbols ())
 	    {
 	      if (sym->is_argument ())
 		nparams++;
@@ -274,7 +272,7 @@ buildsym_compunit::finish_block_internal
 	      iparams = 0;
 	      /* Here we want to directly access the dictionary, because
 		 we haven't fully initialized the block yet.  */
-	      ALL_DICT_SYMBOLS (block->multidict (), miter, sym)
+	      for (struct symbol *sym : block->multidict_symbols ())
 		{
 		  if (iparams == nparams)
 		    break;
@@ -975,8 +973,6 @@ buildsym_compunit::end_compunit_symtab_with_blockvector
     for (block_i = 0; block_i < blockvector->num_blocks (); block_i++)
       {
 	struct block *block = blockvector->block (block_i);
-	struct symbol *sym;
-	struct mdict_iterator miter;
 
 	/* Inlined functions may have symbols not in the global or
 	   static symbol lists.  */
@@ -985,9 +981,10 @@ buildsym_compunit::end_compunit_symtab_with_blockvector
 	    block->function ()->set_symtab (symtab);
 
 	/* Note that we only want to fix up symbols from the local
-	   blocks, not blocks coming from included symtabs.  That is why
-	   we use ALL_DICT_SYMBOLS here and not a block iterator.  */
-	ALL_DICT_SYMBOLS (block->multidict (), miter, sym)
+	   blocks, not blocks coming from included symtabs.  That is
+	   why we use an mdict iterator here and not a block
+	   iterator.  */
+	for (struct symbol *sym : block->multidict_symbols ())
 	  if (sym->symtab () == NULL)
 	    sym->set_symtab (symtab);
       }
diff --git a/gdb/dictionary.h b/gdb/dictionary.h
index 9dc02c91e04..d982396cb31 100644
--- a/gdb/dictionary.h
+++ b/gdb/dictionary.h
@@ -164,16 +164,49 @@ extern struct symbol *mdict_iter_match_next (const lookup_name_info &name,
 
 extern int mdict_size (const struct multidictionary *mdict);
 
-/* Macro to loop through all symbols in a dictionary DICT, in no
-   particular order.  ITER is a struct dict_iterator (NOTE: __not__ a
-   struct dict_iterator *), and SYM points to the current symbol.
-
-   It's implemented as a single loop, so you can terminate the loop
-   early by a break if you desire.  */
-
-#define ALL_DICT_SYMBOLS(dict, iter, sym)			\
-	for ((sym) = mdict_iterator_first ((dict), &(iter));	\
-	     (sym);						\
-	     (sym) = mdict_iterator_next (&(iter)))
+/* An iterator that wraps an mdict_iterator.  The naming here is
+   unfortunate, but mdict_iterator was named before gdb switched to
+   C++.  */
+struct mdict_iterator_wrapper
+{
+  typedef mdict_iterator_wrapper self_type;
+  typedef struct symbol *value_type;
+
+  explicit mdict_iterator_wrapper (const struct multidictionary *mdict)
+    : m_sym (mdict_iterator_first (mdict, &m_iter))
+  {
+  }
+
+  mdict_iterator_wrapper ()
+    : m_sym (nullptr)
+  {
+  }
+
+  value_type operator* () const
+  {
+    return m_sym;
+  }
+
+  bool operator== (const self_type &other) const
+  {
+    return m_sym == other.m_sym;
+  }
+
+  bool operator!= (const self_type &other) const
+  {
+    return m_sym != other.m_sym;
+  }
+
+  self_type &operator++ ()
+  {
+    m_sym = mdict_iterator_next (&m_iter);
+    return *this;
+  }
+
+private:
+
+  struct symbol *m_sym;
+  struct mdict_iterator m_iter;
+};
 
 #endif /* DICTIONARY_H */
diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index 3fefc4ad846..5ba5f0a616d 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -625,9 +625,6 @@ objfile_relocate1 (struct objfile *objfile,
 
       for (block *b : bv->blocks ())
 	{
-	  struct symbol *sym;
-	  struct mdict_iterator miter;
-
 	  b->set_start (b->start () + delta[block_line_section]);
 	  b->set_end (b->end () + delta[block_line_section]);
 
@@ -639,10 +636,8 @@ objfile_relocate1 (struct objfile *objfile,
 
 	  /* We only want to iterate over the local symbols, not any
 	     symbols in included symtabs.  */
-	  ALL_DICT_SYMBOLS (b->multidict (), miter, sym)
-	    {
-	      relocate_one_symbol (sym, objfile, delta);
-	    }
+	  for (struct symbol *sym : b->multidict_symbols ())
+	    relocate_one_symbol (sym, objfile, delta);
 	}
     }
 
diff --git a/gdb/symmisc.c b/gdb/symmisc.c
index fb8a3ebf602..1d838710a66 100644
--- a/gdb/symmisc.c
+++ b/gdb/symmisc.c
@@ -236,9 +236,7 @@ dump_symtab_1 (struct symtab *symtab, struct ui_file *outfile)
 {
   struct objfile *objfile = symtab->compunit ()->objfile ();
   struct gdbarch *gdbarch = objfile->arch ();
-  struct mdict_iterator miter;
   const struct linetable *l;
-  struct symbol *sym;
   int depth;
 
   gdb_printf (outfile, "\nSymtab for file %s at %s\n",
@@ -307,7 +305,7 @@ dump_symtab_1 (struct symtab *symtab, struct ui_file *outfile)
 	  /* Now print each symbol in this block (in no particular order, if
 	     we're using a hashtable).  Note that we only want this
 	     block, not any blocks from included symtabs.  */
-	  ALL_DICT_SYMBOLS (b->multidict (), miter, sym)
+	  for (struct symbol *sym : b->multidict_symbols ())
 	    {
 	      try
 		{

-- 
2.39.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 4/4] Remove ALL_BREAKPOINTS_SAFE
  2023-04-16 16:28 [PATCH 0/4] Remove remaining ALL_ iterator macros Tom Tromey
                   ` (2 preceding siblings ...)
  2023-04-16 16:28 ` [PATCH 3/4] Remove ALL_DICT_SYMBOLS Tom Tromey
@ 2023-04-16 16:28 ` Tom Tromey
  2023-05-08  2:24   ` Simon Marchi
  3 siblings, 1 reply; 9+ messages in thread
From: Tom Tromey @ 2023-04-16 16:28 UTC (permalink / raw)
  To: gdb-patches

There's just a single remaining use of the ALL_BREAKPOINTS_SAFE macro;
this patch replaces it with a for-each and an explicit temporary
variable.
---
 gdb/breakpoint.c | 134 ++++++++++++++++++++++++++-----------------------------
 1 file changed, 63 insertions(+), 71 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 46287da5f87..b1460b51b0c 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -610,15 +610,6 @@ static int overlay_events_enabled;
 /* See description in breakpoint.h. */
 bool target_exact_watchpoints = false;
 
-/* Walk the following statement or block through all breakpoints.
-   ALL_BREAKPOINTS_SAFE does so even if the statement deletes the
-   current breakpoint.  */
-
-#define ALL_BREAKPOINTS_SAFE(B,TMP)	\
-	for (B = breakpoint_chain;	\
-	     B ? (TMP=B->next, 1): 0;	\
-	     B = TMP)
-
 /* Chains of all breakpoints defined.  */
 
 static struct breakpoint *breakpoint_chain;
@@ -7616,72 +7607,73 @@ set_longjmp_breakpoint_for_call_dummy (void)
 void
 check_longjmp_breakpoint_for_call_dummy (struct thread_info *tp)
 {
-  struct breakpoint *b, *b_tmp;
+  for (struct breakpoint *b : all_breakpoints_safe ())
+    {
+      struct breakpoint *b_tmp = b->next;
+      if (b->type == bp_longjmp_call_dummy && b->thread == tp->global_num)
+	{
+	  struct breakpoint *dummy_b = b->related_breakpoint;
 
-  ALL_BREAKPOINTS_SAFE (b, b_tmp)
-    if (b->type == bp_longjmp_call_dummy && b->thread == tp->global_num)
-      {
-	struct breakpoint *dummy_b = b->related_breakpoint;
-
-	/* Find the bp_call_dummy breakpoint in the list of breakpoints
-	   chained off b->related_breakpoint.  */
-	while (dummy_b != b && dummy_b->type != bp_call_dummy)
-	  dummy_b = dummy_b->related_breakpoint;
-
-	/* If there was no bp_call_dummy breakpoint then there's nothing
-	   more to do.  Or, if the dummy frame associated with the
-	   bp_call_dummy is still on the stack then we need to leave this
-	   bp_call_dummy in place.  */
-	if (dummy_b->type != bp_call_dummy
-	    || frame_find_by_id (dummy_b->frame_id) != NULL)
-	  continue;
+	  /* Find the bp_call_dummy breakpoint in the list of breakpoints
+	     chained off b->related_breakpoint.  */
+	  while (dummy_b != b && dummy_b->type != bp_call_dummy)
+	    dummy_b = dummy_b->related_breakpoint;
 
-	/* We didn't find the dummy frame on the stack, this could be
-	   because we have longjmp'd to a stack frame that is previous to
-	   the dummy frame, or it could be because the stack unwind is
-	   broken at some point between the longjmp frame and the dummy
-	   frame.
-
-	   Next we figure out why the stack unwind stopped.  If it looks
-	   like the unwind is complete then we assume the dummy frame has
-	   been jumped over, however, if the unwind stopped for an
-	   unexpected reason then we assume the stack unwind is currently
-	   broken, and that we will (eventually) return to the dummy
-	   frame.
-
-	   It might be tempting to consider using frame_id_inner here, but
-	   that is not safe.   There is no guarantee that the stack frames
-	   we are looking at here are even on the same stack as the
-	   original dummy frame, hence frame_id_inner can't be used.  See
-	   the comments on frame_id_inner for more details.  */
-	bool unwind_finished_unexpectedly = false;
-	for (frame_info_ptr fi = get_current_frame (); fi != nullptr; )
-	  {
-	    frame_info_ptr prev = get_prev_frame (fi);
-	    if (prev == nullptr)
-	      {
-		/* FI is the last stack frame.  Why did this frame not
-		   unwind further?  */
-		auto stop_reason = get_frame_unwind_stop_reason (fi);
-		if (stop_reason != UNWIND_NO_REASON
-		    && stop_reason != UNWIND_OUTERMOST)
-		  unwind_finished_unexpectedly = true;
-	      }
-	    fi = prev;
-	  }
-	if (unwind_finished_unexpectedly)
-	  continue;
+	  /* If there was no bp_call_dummy breakpoint then there's nothing
+	     more to do.  Or, if the dummy frame associated with the
+	     bp_call_dummy is still on the stack then we need to leave this
+	     bp_call_dummy in place.  */
+	  if (dummy_b->type != bp_call_dummy
+	      || frame_find_by_id (dummy_b->frame_id) != NULL)
+	    continue;
 
-	dummy_frame_discard (dummy_b->frame_id, tp);
+	  /* We didn't find the dummy frame on the stack, this could be
+	     because we have longjmp'd to a stack frame that is previous to
+	     the dummy frame, or it could be because the stack unwind is
+	     broken at some point between the longjmp frame and the dummy
+	     frame.
+
+	     Next we figure out why the stack unwind stopped.  If it looks
+	     like the unwind is complete then we assume the dummy frame has
+	     been jumped over, however, if the unwind stopped for an
+	     unexpected reason then we assume the stack unwind is currently
+	     broken, and that we will (eventually) return to the dummy
+	     frame.
+
+	     It might be tempting to consider using frame_id_inner here, but
+	     that is not safe.   There is no guarantee that the stack frames
+	     we are looking at here are even on the same stack as the
+	     original dummy frame, hence frame_id_inner can't be used.  See
+	     the comments on frame_id_inner for more details.  */
+	  bool unwind_finished_unexpectedly = false;
+	  for (frame_info_ptr fi = get_current_frame (); fi != nullptr; )
+	    {
+	      frame_info_ptr prev = get_prev_frame (fi);
+	      if (prev == nullptr)
+		{
+		  /* FI is the last stack frame.  Why did this frame not
+		     unwind further?  */
+		  auto stop_reason = get_frame_unwind_stop_reason (fi);
+		  if (stop_reason != UNWIND_NO_REASON
+		      && stop_reason != UNWIND_OUTERMOST)
+		    unwind_finished_unexpectedly = true;
+		}
+	      fi = prev;
+	    }
+	  if (unwind_finished_unexpectedly)
+	    continue;
 
-	while (b->related_breakpoint != b)
-	  {
-	    if (b_tmp == b->related_breakpoint)
-	      b_tmp = b->related_breakpoint->next;
-	    delete_breakpoint (b->related_breakpoint);
-	  }
-	delete_breakpoint (b);
-      }
+	  dummy_frame_discard (dummy_b->frame_id, tp);
+
+	  while (b->related_breakpoint != b)
+	    {
+	      if (b_tmp == b->related_breakpoint)
+		b_tmp = b->related_breakpoint->next;
+	      delete_breakpoint (b->related_breakpoint);
+	    }
+	  delete_breakpoint (b);
+	}
+    }
 }
 
 void

-- 
2.39.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/4] Rename objfile::sections
  2023-04-16 16:28 ` [PATCH 1/4] Rename objfile::sections Tom Tromey
@ 2023-04-17 17:57   ` Keith Seitz
  2023-05-07 18:43     ` Tom Tromey
  0 siblings, 1 reply; 9+ messages in thread
From: Keith Seitz @ 2023-04-17 17:57 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

On 4/16/23 09:28, Tom Tromey wrote:
> I think objfile::sections makes sense as the name of the method to
> iterate over an objfile's sections, so this patch renames the existing
> field to objfile::sections_start in preparation for that.

I rather like this rename, tbh, because the related member is named
"sections_end". I like symmetry. :-)

Just one little comment...

> diff --git a/gdb/objfiles.h b/gdb/objfiles.h
> index 342aa09ac6a..0e326287907 100644
> --- a/gdb/objfiles.h
> +++ b/gdb/objfiles.h
> @@ -128,7 +128,7 @@ struct entry_info
>   };
>   
>   #define ALL_OBJFILE_OSECTIONS(objfile, osect)	\
> -  for (osect = objfile->sections; osect < objfile->sections_end; osect++) \
> +  for (osect = objfile->sections_start; osect < objfile->sections_end; osect++) \
>       if (osect->the_bfd_section == NULL)					\
>         {									\
>   	/* Nothing.  */							\
> @@ -722,7 +722,7 @@ struct objfile
>        structure data is only valid for certain sections
>        (e.g. non-empty, SEC_ALLOC).  */
> 

... about a comment!

The paragraph/comment here needs updating, I think:

    "SECTIONS points to the first entry in the table, [...]"

Keith

> -  struct obj_section *sections = nullptr;
> +  struct obj_section *sections_start = nullptr;
>     struct obj_section *sections_end = nullptr;
>   
>     /* GDB allows to have debug symbols in separate object files.  This is


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/4] Rename objfile::sections
  2023-04-17 17:57   ` Keith Seitz
@ 2023-05-07 18:43     ` Tom Tromey
  0 siblings, 0 replies; 9+ messages in thread
From: Tom Tromey @ 2023-05-07 18:43 UTC (permalink / raw)
  To: Keith Seitz; +Cc: Tom Tromey, gdb-patches

>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:

...
Keith> The paragraph/comment here needs updating, I think:

Keith>    "SECTIONS points to the first entry in the table, [...]"

I made this change.
I'm going to check this in shortly.

Tom

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 4/4] Remove ALL_BREAKPOINTS_SAFE
  2023-04-16 16:28 ` [PATCH 4/4] Remove ALL_BREAKPOINTS_SAFE Tom Tromey
@ 2023-05-08  2:24   ` Simon Marchi
  2023-05-08 14:59     ` Simon Marchi
  0 siblings, 1 reply; 9+ messages in thread
From: Simon Marchi @ 2023-05-08  2:24 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

On 4/16/23 12:28, Tom Tromey wrote:
> There's just a single remaining use of the ALL_BREAKPOINTS_SAFE macro;
> this patch replaces it with a for-each and an explicit temporary
> variable.

Starting with this patch, I see:

(gdb) PASS: gdb.base/stale-infcall.exp: continue to breakpoint: break-run1
print infcall ()
=================================================================
==47472==ERROR: AddressSanitizer: heap-use-after-free on address 0x611000034980 at pc 0x563f7012c7bc bp 0x7ffdf3804d70 sp 0x7ffdf3804d60
READ of size 8 at 0x611000034980 thread T0
    #0 0x563f7012c7bb in next_iterator<breakpoint>::operator++() /home/smarchi/src/binutils-gdb/gdb/../gdbsupport/next-iterator.h:66
    #1 0x563f702ce8c0 in basic_safe_iterator<next_iterator<breakpoint> >::operator++() /home/smarchi/src/binutils-gdb/gdb/../gdbsupport/safe-iterator.h:84
    #2 0x563f7021522a in check_longjmp_breakpoint_for_call_dummy(thread_info*) /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:7611
    #3 0x563f714567b1 in process_event_stop_test /home/smarchi/src/binutils-gdb/gdb/infrun.c:6881
    #4 0x563f71454e07 in handle_signal_stop /home/smarchi/src/binutils-gdb/gdb/infrun.c:6769
    #5 0x563f7144b680 in handle_inferior_event /home/smarchi/src/binutils-gdb/gdb/infrun.c:6023
    #6 0x563f71436165 in fetch_inferior_event() /home/smarchi/src/binutils-gdb/gdb/infrun.c:4387
    #7 0x563f7136ff51 in inferior_event_handler(inferior_event_type) /home/smarchi/src/binutils-gdb/gdb/inf-loop.c:42
    #8 0x563f7168038d in handle_target_event /home/smarchi/src/binutils-gdb/gdb/linux-nat.c:4219
    #9 0x563f72fccb6d in handle_file_event /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:573
    #10 0x563f72fcd503 in gdb_wait_for_event /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:694
    #11 0x563f72fcaf2b in gdb_do_one_event(int) /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:217
    #12 0x563f7262b9bb in wait_sync_command_done() /home/smarchi/src/binutils-gdb/gdb/top.c:426
    #13 0x563f7137a7c3 in run_inferior_call /home/smarchi/src/binutils-gdb/gdb/infcall.c:650
    #14 0x563f71381295 in call_function_by_hand_dummy(value*, type*, gdb::array_view<value*>, void (*)(void*, int), void*) /home/smarchi/src/binutils-gdb/gdb/infcall.c:1332
    #15 0x563f7137c0e2 in call_function_by_hand(value*, type*, gdb::array_view<value*>) /home/smarchi/src/binutils-gdb/gdb/infcall.c:780
    #16 0x563f70fe5960 in evaluate_subexp_do_call(expression*, noside, value*, gdb::array_view<value*>, char const*, type*) /home/smarchi/src/binutils-gdb/gdb/eval.c:649
    #17 0x563f70fe6617 in expr::operation::evaluate_funcall(type*, expression*, noside, char const*, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/eval.c:677
    #18 0x563f6fd19668 in expr::operation::evaluate_funcall(type*, expression*, noside, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/expression.h:136
    #19 0x563f70fe6bba in expr::var_value_operation::evaluate_funcall(type*, expression*, noside, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/eval.c:689
    #20 0x563f704b71dc in expr::funcall_operation::evaluate(type*, expression*, noside) /home/smarchi/src/binutils-gdb/gdb/expop.h:2219
    #21 0x563f70fe0f02 in expression::evaluate(type*, noside) /home/smarchi/src/binutils-gdb/gdb/eval.c:110
    #22 0x563f71b1373e in process_print_command_args /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1319
    #23 0x563f71b1391b in print_command_1 /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1332
    #24 0x563f71b147ec in print_command /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1465
    #25 0x563f706029b8 in do_simple_func /home/smarchi/src/binutils-gdb/gdb/cli/cli-decode.c:95
    #26 0x563f7061972a in cmd_func(cmd_list_element*, char const*, int) /home/smarchi/src/binutils-gdb/gdb/cli/cli-decode.c:2735
    #27 0x563f7262d0ef in execute_command(char const*, int) /home/smarchi/src/binutils-gdb/gdb/top.c:572
    #28 0x563f7100ed9c in command_handler(char const*) /home/smarchi/src/binutils-gdb/gdb/event-top.c:543
    #29 0x563f7101014b in command_line_handler(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) /home/smarchi/src/binutils-gdb/gdb/event-top.c:779
    #30 0x563f72777942 in tui_command_line_handler /home/smarchi/src/binutils-gdb/gdb/tui/tui-interp.c:104
    #31 0x563f7100d059 in gdb_rl_callback_handler /home/smarchi/src/binutils-gdb/gdb/event-top.c:250
    #32 0x7f5a80418246 in rl_callback_read_char (/usr/lib/libreadline.so.8+0x3b246) (BuildId: 092e91fc4361b0ef94561e3ae03a75f69398acbb)
    #33 0x563f7100ca06 in gdb_rl_callback_read_char_wrapper_noexcept /home/smarchi/src/binutils-gdb/gdb/event-top.c:192
    #34 0x563f7100cc5e in gdb_rl_callback_read_char_wrapper /home/smarchi/src/binutils-gdb/gdb/event-top.c:225
    #35 0x563f728c70db in stdin_event_handler /home/smarchi/src/binutils-gdb/gdb/ui.c:155
    #36 0x563f72fccb6d in handle_file_event /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:573
    #37 0x563f72fcd503 in gdb_wait_for_event /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:694
    #38 0x563f72fcb15c in gdb_do_one_event(int) /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:264
    #39 0x563f7177ec1c in start_event_loop /home/smarchi/src/binutils-gdb/gdb/main.c:412
    #40 0x563f7177f12e in captured_command_loop /home/smarchi/src/binutils-gdb/gdb/main.c:476
    #41 0x563f717846e4 in captured_main /home/smarchi/src/binutils-gdb/gdb/main.c:1320
    #42 0x563f71784821 in gdb_main(captured_main_args*) /home/smarchi/src/binutils-gdb/gdb/main.c:1339
    #43 0x563f6fcedfbd in main /home/smarchi/src/binutils-gdb/gdb/gdb.c:32
    #44 0x7f5a7e43984f  (/usr/lib/libc.so.6+0x2384f) (BuildId: 2f005a79cd1a8e385972f5a102f16adba414d75e)
    #45 0x7f5a7e439909 in __libc_start_main (/usr/lib/libc.so.6+0x23909) (BuildId: 2f005a79cd1a8e385972f5a102f16adba414d75e)
    #46 0x563f6fcedd84 in _start (/home/smarchi/build/binutils-gdb/gdb/gdb+0xafb0d84) (BuildId: 50bd32e6e9d5e84543e9897b8faca34858ca3995)

0x611000034980 is located 0 bytes inside of 208-byte region [0x611000034980,0x611000034a50)
freed by thread T0 here:
    #0 0x7f5a7fce312a in operator delete(void*, unsigned long) /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_new_delete.cpp:164
    #1 0x563f702bd1fa in momentary_breakpoint::~momentary_breakpoint() /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:304
    #2 0x563f702771c5 in delete_breakpoint(breakpoint*) /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:12404
    #3 0x563f702150a7 in check_longjmp_breakpoint_for_call_dummy(thread_info*) /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:7673
    #4 0x563f714567b1 in process_event_stop_test /home/smarchi/src/binutils-gdb/gdb/infrun.c:6881
    #5 0x563f71454e07 in handle_signal_stop /home/smarchi/src/binutils-gdb/gdb/infrun.c:6769
    #6 0x563f7144b680 in handle_inferior_event /home/smarchi/src/binutils-gdb/gdb/infrun.c:6023
    #7 0x563f71436165 in fetch_inferior_event() /home/smarchi/src/binutils-gdb/gdb/infrun.c:4387
    #8 0x563f7136ff51 in inferior_event_handler(inferior_event_type) /home/smarchi/src/binutils-gdb/gdb/inf-loop.c:42
    #9 0x563f7168038d in handle_target_event /home/smarchi/src/binutils-gdb/gdb/linux-nat.c:4219
    #10 0x563f72fccb6d in handle_file_event /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:573
    #11 0x563f72fcd503 in gdb_wait_for_event /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:694
    #12 0x563f72fcaf2b in gdb_do_one_event(int) /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:217
    #13 0x563f7262b9bb in wait_sync_command_done() /home/smarchi/src/binutils-gdb/gdb/top.c:426
    #14 0x563f7137a7c3 in run_inferior_call /home/smarchi/src/binutils-gdb/gdb/infcall.c:650
    #15 0x563f71381295 in call_function_by_hand_dummy(value*, type*, gdb::array_view<value*>, void (*)(void*, int), void*) /home/smarchi/src/binutils-gdb/gdb/infcall.c:1332
    #16 0x563f7137c0e2 in call_function_by_hand(value*, type*, gdb::array_view<value*>) /home/smarchi/src/binutils-gdb/gdb/infcall.c:780
    #17 0x563f70fe5960 in evaluate_subexp_do_call(expression*, noside, value*, gdb::array_view<value*>, char const*, type*) /home/smarchi/src/binutils-gdb/gdb/eval.c:649
    #18 0x563f70fe6617 in expr::operation::evaluate_funcall(type*, expression*, noside, char const*, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/eval.c:677
    #19 0x563f6fd19668 in expr::operation::evaluate_funcall(type*, expression*, noside, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/expression.h:136
    #20 0x563f70fe6bba in expr::var_value_operation::evaluate_funcall(type*, expression*, noside, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/eval.c:689
    #21 0x563f704b71dc in expr::funcall_operation::evaluate(type*, expression*, noside) /home/smarchi/src/binutils-gdb/gdb/expop.h:2219
    #22 0x563f70fe0f02 in expression::evaluate(type*, noside) /home/smarchi/src/binutils-gdb/gdb/eval.c:110
    #23 0x563f71b1373e in process_print_command_args /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1319
    #24 0x563f71b1391b in print_command_1 /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1332
    #25 0x563f71b147ec in print_command /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1465
    #26 0x563f706029b8 in do_simple_func /home/smarchi/src/binutils-gdb/gdb/cli/cli-decode.c:95
    #27 0x563f7061972a in cmd_func(cmd_list_element*, char const*, int) /home/smarchi/src/binutils-gdb/gdb/cli/cli-decode.c:2735
    #28 0x563f7262d0ef in execute_command(char const*, int) /home/smarchi/src/binutils-gdb/gdb/top.c:572
    #29 0x563f7100ed9c in command_handler(char const*) /home/smarchi/src/binutils-gdb/gdb/event-top.c:543

previously allocated by thread T0 here:
    #0 0x7f5a7fce2012 in operator new(unsigned long) /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_new_delete.cpp:95
    #1 0x563f7029a9a3 in new_momentary_breakpoint<program_space*&, frame_id&, int&> /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:8129
    #2 0x563f702212f6 in momentary_breakpoint_from_master /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:8169
    #3 0x563f70212db1 in set_longjmp_breakpoint_for_call_dummy() /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:7582
    #4 0x563f713804db in call_function_by_hand_dummy(value*, type*, gdb::array_view<value*>, void (*)(void*, int), void*) /home/smarchi/src/binutils-gdb/gdb/infcall.c:1260
    #5 0x563f7137c0e2 in call_function_by_hand(value*, type*, gdb::array_view<value*>) /home/smarchi/src/binutils-gdb/gdb/infcall.c:780
    #6 0x563f70fe5960 in evaluate_subexp_do_call(expression*, noside, value*, gdb::array_view<value*>, char const*, type*) /home/smarchi/src/binutils-gdb/gdb/eval.c:649
    #7 0x563f70fe6617 in expr::operation::evaluate_funcall(type*, expression*, noside, char const*, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/eval.c:677
    #8 0x563f6fd19668 in expr::operation::evaluate_funcall(type*, expression*, noside, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/expression.h:136
    #9 0x563f70fe6bba in expr::var_value_operation::evaluate_funcall(type*, expression*, noside, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/eval.c:689
    #10 0x563f704b71dc in expr::funcall_operation::evaluate(type*, expression*, noside) /home/smarchi/src/binutils-gdb/gdb/expop.h:2219
    #11 0x563f70fe0f02 in expression::evaluate(type*, noside) /home/smarchi/src/binutils-gdb/gdb/eval.c:110
    #12 0x563f71b1373e in process_print_command_args /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1319
    #13 0x563f71b1391b in print_command_1 /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1332
    #14 0x563f71b147ec in print_command /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1465
    #15 0x563f706029b8 in do_simple_func /home/smarchi/src/binutils-gdb/gdb/cli/cli-decode.c:95
    #16 0x563f7061972a in cmd_func(cmd_list_element*, char const*, int) /home/smarchi/src/binutils-gdb/gdb/cli/cli-decode.c:2735
    #17 0x563f7262d0ef in execute_command(char const*, int) /home/smarchi/src/binutils-gdb/gdb/top.c:572
    #18 0x563f7100ed9c in command_handler(char const*) /home/smarchi/src/binutils-gdb/gdb/event-top.c:543
    #19 0x563f7101014b in command_line_handler(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) /home/smarchi/src/binutils-gdb/gdb/event-top.c:779
    #20 0x563f72777942 in tui_command_line_handler /home/smarchi/src/binutils-gdb/gdb/tui/tui-interp.c:104
    #21 0x563f7100d059 in gdb_rl_callback_handler /home/smarchi/src/binutils-gdb/gdb/event-top.c:250
    #22 0x7f5a80418246 in rl_callback_read_char (/usr/lib/libreadline.so.8+0x3b246) (BuildId: 092e91fc4361b0ef94561e3ae03a75f69398acbb)

Simon

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 4/4] Remove ALL_BREAKPOINTS_SAFE
  2023-05-08  2:24   ` Simon Marchi
@ 2023-05-08 14:59     ` Simon Marchi
  0 siblings, 0 replies; 9+ messages in thread
From: Simon Marchi @ 2023-05-08 14:59 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

On 5/7/23 22:24, Simon Marchi via Gdb-patches wrote:
> On 4/16/23 12:28, Tom Tromey wrote:
>> There's just a single remaining use of the ALL_BREAKPOINTS_SAFE macro;
>> this patch replaces it with a for-each and an explicit temporary
>> variable.
> 
> Starting with this patch, I see:
> 
> (gdb) PASS: gdb.base/stale-infcall.exp: continue to breakpoint: break-run1
> print infcall ()
> =================================================================
> ==47472==ERROR: AddressSanitizer: heap-use-after-free on address 0x611000034980 at pc 0x563f7012c7bc bp 0x7ffdf3804d70 sp 0x7ffdf3804d60
> READ of size 8 at 0x611000034980 thread T0

I just sent a patch for this:

https://inbox.sourceware.org/gdb-patches/20230508145913.29359-1-simon.marchi@efficios.com/T/#u

Simon

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-05-08 14:59 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-16 16:28 [PATCH 0/4] Remove remaining ALL_ iterator macros Tom Tromey
2023-04-16 16:28 ` [PATCH 1/4] Rename objfile::sections Tom Tromey
2023-04-17 17:57   ` Keith Seitz
2023-05-07 18:43     ` Tom Tromey
2023-04-16 16:28 ` [PATCH 2/4] Remove ALL_OBJFILE_OSECTIONS Tom Tromey
2023-04-16 16:28 ` [PATCH 3/4] Remove ALL_DICT_SYMBOLS Tom Tromey
2023-04-16 16:28 ` [PATCH 4/4] Remove ALL_BREAKPOINTS_SAFE Tom Tromey
2023-05-08  2:24   ` Simon Marchi
2023-05-08 14:59     ` Simon Marchi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).