[PATCH 1/2] gdb: convert obj_section macros to methods

[PATCH 1/2] gdb: convert obj_section macros to methods

[Simon Marchi]

Convert these three macros to methods of obj_section.  The problem fixed
by the following patch is caused by an out of bound access of the
objfile::section_offsets vector.  Since this is deep in macros, we don't
get a clear backtrace and it's difficult to debug.  Changing that to
methods means we can step in them and break on them.

Because their implementation requires knowing about struct objfile, move
struct obj_section below struct objfile in objfiles.h.

The obj_section_offset was used in one place as an lvalue to set
offsets, in machoread.c.  Replace that with a set_offset method.

Add the objfile::section_offset and objfile::set_section_offset methods
to improve encapsulation (reduce other objects poking into struct
objfile's internals).

gdb/ChangeLog:

	* objfiles.h (struct obj_section): Move down.
	<offset, set_offset, addr, endaddr>: New.
	(obj_section_offset, obj_section_addr, obj_section_endaddr),
	replace all users to use obj_section methods.
	(struct objfile) <section_offset, set_section_offset>: New.

Change-Id: I97e8fcae93ab2353fbdadcb4a5ec10d7949a7334
---
 gdb/arm-tdep.c        | 15 ++++---
 gdb/exec.c            |  3 +-
 gdb/gcore.c           |  6 +--
 gdb/hppa-bsd-tdep.c   |  4 +-
 gdb/hppa-linux-tdep.c |  4 +-
 gdb/hppa-tdep.c       |  6 +--
 gdb/ia64-tdep.c       | 14 +++----
 gdb/machoread.c       |  2 +-
 gdb/maint.c           |  3 +-
 gdb/minsyms.c         |  8 ++--
 gdb/objfiles.c        | 27 ++++++-------
 gdb/objfiles.h        | 93 ++++++++++++++++++++++++++++---------------
 gdb/printcmd.c        |  3 +-
 gdb/solib-aix.c       |  3 +-
 gdb/solib-dsbt.c      |  2 +-
 gdb/solib-frv.c       |  2 +-
 gdb/symfile.c         | 14 +++----
 gdb/symtab.c          |  3 +-
 gdb/tilegx-tdep.c     |  2 +-
 gdb/tui/tui-disasm.c  |  2 +-
 gdb/xstormy16-tdep.c  |  4 +-
 21 files changed, 118 insertions(+), 102 deletions(-)

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 857a52a9a513..2f57c8bebf81 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -395,8 +395,7 @@ arm_find_mapping_symbol (CORE_ADDR memaddr, CORE_ADDR *start)
 	      data->section_maps_sorted[section_idx] = true;
 	    }
 
-	  struct arm_mapping_symbol map_key
-	    = { memaddr - obj_section_addr (sec), 0 };
+	  arm_mapping_symbol map_key = { memaddr - sec->addr (), 0 };
 	  arm_mapping_symbol_vec::const_iterator it
 	    = std::lower_bound (map.begin (), map.end (), map_key);
 
@@ -409,7 +408,7 @@ arm_find_mapping_symbol (CORE_ADDR memaddr, CORE_ADDR *start)
 	      if (it->value == map_key.value)
 		{
 		  if (start)
-		    *start = it->value + obj_section_addr (sec);
+		    *start = it->value + sec->addr ();
 		  return it->type;
 		}
 	    }
@@ -420,7 +419,7 @@ arm_find_mapping_symbol (CORE_ADDR memaddr, CORE_ADDR *start)
 		= it - 1;
 
 	      if (start)
-		*start = prev_it->value + obj_section_addr (sec);
+		*start = prev_it->value + sec->addr ();
 	      return prev_it->type;
 	    }
 	}
@@ -2218,7 +2217,7 @@ arm_exidx_new_objfile (struct objfile *objfile)
 			  NULL
 			};
 
-		      CORE_ADDR pc = pers + obj_section_offset (pers_sec);
+		      CORE_ADDR pc = pers + pers_sec->offset ();
 		      int k;
 
 		      for (k = 0; personality[k]; k++)
@@ -2303,7 +2302,7 @@ arm_find_exidx_entry (CORE_ADDR memaddr, CORE_ADDR *start)
   if (sec != NULL)
     {
       struct arm_exidx_data *data;
-      struct arm_exidx_entry map_key = { memaddr - obj_section_addr (sec), 0 };
+      struct arm_exidx_entry map_key = { memaddr - sec->addr (), 0 };
 
       data = arm_exidx_data_key.get (sec->objfile->obfd);
       if (data != NULL)
@@ -2323,7 +2322,7 @@ arm_find_exidx_entry (CORE_ADDR memaddr, CORE_ADDR *start)
 		  if (idx->addr == map_key.addr)
 		    {
 		      if (start)
-			*start = idx->addr + obj_section_addr (sec);
+			*start = idx->addr + sec->addr ();
 		      return idx->entry;
 		    }
 		}
@@ -2332,7 +2331,7 @@ arm_find_exidx_entry (CORE_ADDR memaddr, CORE_ADDR *start)
 		{
 		  idx = idx - 1;
 		  if (start)
-		    *start = idx->addr + obj_section_addr (sec);
+		    *start = idx->addr + sec->addr ();
 		  return idx->entry;
 		}
 	    }
diff --git a/gdb/exec.c b/gdb/exec.c
index ff0db82a69b9..35bf7bd45066 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -636,8 +636,7 @@ program_space::add_target_sections (struct objfile *objfile)
       if (bfd_section_size (osect->the_bfd_section) == 0)
 	continue;
 
-      m_target_sections.emplace_back (obj_section_addr (osect),
-				      obj_section_endaddr (osect),
+      m_target_sections.emplace_back (osect->addr (), osect->endaddr (),
 				      osect->the_bfd_section, (void *) objfile);
     }
 }
diff --git a/gdb/gcore.c b/gdb/gcore.c
index 76e856d71a81..1ec0d1eb51b0 100644
--- a/gdb/gcore.c
+++ b/gdb/gcore.c
@@ -404,8 +404,8 @@ gcore_create_callback (CORE_ADDR vaddr, unsigned long size, int read,
 	    bfd *abfd = objfile->obfd;
 	    asection *asec = objsec->the_bfd_section;
 	    bfd_vma align = (bfd_vma) 1 << bfd_section_alignment (asec);
-	    bfd_vma start = obj_section_addr (objsec) & -align;
-	    bfd_vma end = (obj_section_endaddr (objsec) + align - 1) & -align;
+	    bfd_vma start = objsec->addr () & -align;
+	    bfd_vma end = (objsec->endaddr () + align - 1) & -align;
 
 	    /* Match if either the entire memory region lies inside the
 	       section (i.e. a mapping covering some pages of a large
@@ -479,7 +479,7 @@ objfile_find_memory_regions (struct target_ops *self,
 	    int size = bfd_section_size (isec);
 	    int ret;
 
-	    ret = (*func) (obj_section_addr (objsec), size, 
+	    ret = (*func) (objsec->addr (), size, 
 			   1, /* All sections will be readable.  */
 			   (flags & SEC_READONLY) == 0, /* Writable.  */
 			   (flags & SEC_CODE) != 0, /* Executable.  */
diff --git a/gdb/hppa-bsd-tdep.c b/gdb/hppa-bsd-tdep.c
index b5289d300430..c68aaf1f490f 100644
--- a/gdb/hppa-bsd-tdep.c
+++ b/gdb/hppa-bsd-tdep.c
@@ -64,8 +64,8 @@ hppabsd_find_global_pointer (struct gdbarch *gdbarch, struct value *function)
 
       if (sec < faddr_sec->objfile->sections_end)
 	{
-	  CORE_ADDR addr = obj_section_addr (sec);
-	  CORE_ADDR endaddr = obj_section_endaddr (sec);
+	  CORE_ADDR addr = sec->addr ();
+	  CORE_ADDR endaddr = sec->endaddr ();
 
 	  while (addr < endaddr)
 	    {
diff --git a/gdb/hppa-linux-tdep.c b/gdb/hppa-linux-tdep.c
index a4d1ceeeeb86..cea6768a1dd3 100644
--- a/gdb/hppa-linux-tdep.c
+++ b/gdb/hppa-linux-tdep.c
@@ -371,8 +371,8 @@ hppa_linux_find_global_pointer (struct gdbarch *gdbarch,
 	{
 	  CORE_ADDR addr, endaddr;
 
-	  addr = obj_section_addr (osect);
-	  endaddr = obj_section_endaddr (osect);
+	  addr = osect->addr ();
+	  endaddr = osect->endaddr ();
 
 	  while (addr < endaddr)
 	    {
diff --git a/gdb/hppa-tdep.c b/gdb/hppa-tdep.c
index 21e58f3c3926..ffb3394f71d4 100644
--- a/gdb/hppa-tdep.c
+++ b/gdb/hppa-tdep.c
@@ -943,11 +943,7 @@ hppa64_convert_code_addr_to_fptr (struct gdbarch *gdbarch, CORE_ADDR code)
 
   if (opd < sec->objfile->sections_end)
     {
-      CORE_ADDR addr;
-
-      for (addr = obj_section_addr (opd);
-	   addr < obj_section_endaddr (opd);
-	   addr += 2 * 8)
+      for (CORE_ADDR addr = opd->addr (); addr < opd->endaddr (); addr += 2 * 8)
 	{
 	  ULONGEST opdaddr;
 	  gdb_byte tmp[8];
diff --git a/gdb/ia64-tdep.c b/gdb/ia64-tdep.c
index 1c4fc4cca47a..895808ac08b9 100644
--- a/gdb/ia64-tdep.c
+++ b/gdb/ia64-tdep.c
@@ -2859,7 +2859,7 @@ ia64_get_dyn_info_list (unw_addr_space_t as,
       void *buf = NULL;
 
       text_sec = objfile->sections + SECT_OFF_TEXT (objfile);
-      ip = obj_section_addr (text_sec);
+      ip = text_sec->addr ();
       ret = ia64_find_unwind_table (objfile, ip, &di, &buf);
       if (ret >= 0)
 	{
@@ -3432,10 +3432,8 @@ ia64_find_global_pointer_from_dynamic_section (struct gdbarch *gdbarch,
 
       if (osect < faddr_sect->objfile->sections_end)
 	{
-	  CORE_ADDR addr, endaddr;
-
-	  addr = obj_section_addr (osect);
-	  endaddr = obj_section_endaddr (osect);
+	  CORE_ADDR addr = osect->addr ();
+	  CORE_ADDR endaddr = osect->endaddr ();
 
 	  while (addr < endaddr)
 	    {
@@ -3515,10 +3513,8 @@ find_extant_func_descr (struct gdbarch *gdbarch, CORE_ADDR faddr)
 
       if (osect < faddr_sect->objfile->sections_end)
 	{
-	  CORE_ADDR addr, endaddr;
-
-	  addr = obj_section_addr (osect);
-	  endaddr = obj_section_endaddr (osect);
+	  CORE_ADDR addr = osect->addr ();
+	  CORE_ADDR endaddr = osect->endaddr ();
 
 	  while (addr < endaddr)
 	    {
diff --git a/gdb/machoread.c b/gdb/machoread.c
index 17aa9ceee01c..a0f0307d5293 100644
--- a/gdb/machoread.c
+++ b/gdb/machoread.c
@@ -926,7 +926,7 @@ macho_symfile_offsets (struct objfile *objfile,
 
 	  if (bfd_sect_name == addrs[i].name)
 	    {
-	      obj_section_offset (osect) = addrs[i].addr;
+	      osect->set_offset (addrs[i].addr);
 	      break;
 	    }
 	}
diff --git a/gdb/maint.c b/gdb/maint.c
index 26bacbfeb768..4637fcbc86cd 100644
--- a/gdb/maint.c
+++ b/gdb/maint.c
@@ -319,8 +319,7 @@ print_objfile_section_info (bfd *abfd, struct obj_section *asect,
 
       print_section_index (abfd, asect->the_bfd_section, index_digits);
       maint_print_section_info (name, flags,
-				obj_section_addr (asect),
-				obj_section_endaddr (asect),
+				asect->addr (), asect->endaddr (),
 				asect->the_bfd_section->filepos,
 				addr_size);
     }
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index 80c3d431209e..33560e3c41c1 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -687,9 +687,9 @@ frob_address (struct objfile *objfile, CORE_ADDR *pc)
 
   ALL_OBJFILE_OSECTIONS (objfile, iter)
     {
-      if (*pc >= obj_section_addr (iter) && *pc < obj_section_endaddr (iter))
+      if (*pc >= iter->addr () && *pc < iter->endaddr ())
 	{
-	  *pc -= obj_section_offset (iter);
+	  *pc -= iter->offset ();
 	  return 1;
 	}
     }
@@ -1616,12 +1616,12 @@ minimal_symbol_upper_bound (struct bound_minimal_symbol minsym)
   obj_section = minsym.obj_section ();
   if (iter != past_the_end
       && (MSYMBOL_VALUE_ADDRESS (minsym.objfile, iter)
-	  < obj_section_endaddr (obj_section)))
+	  < obj_section->endaddr ()))
     result = MSYMBOL_VALUE_ADDRESS (minsym.objfile, iter);
   else
     /* We got the start address from the last msymbol in the objfile.
        So the end address is the end of the section.  */
-    result = obj_section_endaddr (obj_section);
+    result = obj_section->endaddr ();
 
   return result;
 }
diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index 16bbb91c0a64..b65fa8820cae 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -729,7 +729,7 @@ objfile_relocate1 (struct objfile *objfile,
       int idx = s - objfile->sections;
 
       exec_set_section_address (bfd_get_filename (objfile->obfd), idx,
-				obj_section_addr (s));
+				s->addr ());
     }
 
   /* Data changed.  */
@@ -899,8 +899,8 @@ have_minimal_symbols (void)
 static bool
 sort_cmp (const struct obj_section *sect1, const obj_section *sect2)
 {
-  const CORE_ADDR sect1_addr = obj_section_addr (sect1);
-  const CORE_ADDR sect2_addr = obj_section_addr (sect2);
+  const CORE_ADDR sect1_addr = sect1->addr ();
+  const CORE_ADDR sect2_addr = sect2->addr ();
 
   if (sect1_addr < sect2_addr)
     return true;
@@ -982,7 +982,7 @@ sort_cmp (const struct obj_section *sect1, const obj_section *sect2)
 static struct obj_section *
 preferred_obj_section (struct obj_section *a, struct obj_section *b)
 {
-  gdb_assert (obj_section_addr (a) == obj_section_addr (b));
+  gdb_assert (a->addr () == b->addr ());
   gdb_assert ((a->objfile->separate_debug_objfile == b->objfile)
 	      || (b->objfile->separate_debug_objfile == a->objfile));
   gdb_assert ((a->objfile->separate_debug_objfile_backlink == b->objfile)
@@ -1030,8 +1030,8 @@ filter_debuginfo_sections (struct obj_section **map, int map_size)
       struct obj_section *const sect2 = map[i + 1];
       const struct objfile *const objfile1 = sect1->objfile;
       const struct objfile *const objfile2 = sect2->objfile;
-      const CORE_ADDR sect1_addr = obj_section_addr (sect1);
-      const CORE_ADDR sect2_addr = obj_section_addr (sect2);
+      const CORE_ADDR sect1_addr = sect1->addr ();
+      const CORE_ADDR sect2_addr = sect2->addr ();
 
       if (sect1_addr == sect2_addr
 	  && (objfile1->separate_debug_objfile == objfile2
@@ -1075,9 +1075,9 @@ filter_overlapping_sections (struct obj_section **map, int map_size)
 	{
 	  struct obj_section *const sect1 = map[i];
 	  struct obj_section *const sect2 = map[k];
-	  const CORE_ADDR sect1_addr = obj_section_addr (sect1);
-	  const CORE_ADDR sect2_addr = obj_section_addr (sect2);
-	  const CORE_ADDR sect1_endaddr = obj_section_endaddr (sect1);
+	  const CORE_ADDR sect1_addr = sect1->addr ();
+	  const CORE_ADDR sect2_addr = sect2->addr ();
+	  const CORE_ADDR sect1_endaddr = sect1->endaddr ();
 
 	  gdb_assert (sect1_addr <= sect2_addr);
 
@@ -1093,7 +1093,7 @@ filter_overlapping_sections (struct obj_section **map, int map_size)
 	      const struct bfd_section *const bfds1 = sect1->the_bfd_section;
 	      const struct bfd_section *const bfds2 = sect2->the_bfd_section;
 
-	      const CORE_ADDR sect2_endaddr = obj_section_endaddr (sect2);
+	      const CORE_ADDR sect2_endaddr = sect2->endaddr ();
 
 	      struct gdbarch *const gdbarch = objf1->arch ();
 
@@ -1184,9 +1184,9 @@ bsearch_cmp (const void *key, const void *elt)
   const CORE_ADDR pc = *(CORE_ADDR *) key;
   const struct obj_section *section = *(const struct obj_section **) elt;
 
-  if (pc < obj_section_addr (section))
+  if (pc < section->addr ())
     return -1;
-  if (pc < obj_section_endaddr (section))
+  if (pc < section->endaddr ())
     return 0;
   return 1;
 }
@@ -1289,8 +1289,7 @@ is_addr_in_objfile (CORE_ADDR addr, const struct objfile *objfile)
       if (section_is_overlay (osect) && !section_is_mapped (osect))
 	continue;
 
-      if (obj_section_addr (osect) <= addr
-	  && addr < obj_section_endaddr (osect))
+      if (osect->addr () <= addr && addr < osect->endaddr ())
 	return true;
     }
   return false;
diff --git a/gdb/objfiles.h b/gdb/objfiles.h
index f947d699132c..766c4b409dfc 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -127,37 +127,6 @@ struct entry_info
   unsigned initialized : 1;
 };
 
-/* Sections in an objfile.  The section offsets are stored in the
-   OBJFILE.  */
-
-struct obj_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;
-};
-
-/* Relocation offset applied to S.  */
-#define obj_section_offset(s)						\
-  (((s)->objfile->section_offsets)[gdb_bfd_section_index ((s)->objfile->obfd, (s)->the_bfd_section)])
-
-/* The memory address of section S (vma + offset).  */
-#define obj_section_addr(s)				      		\
-  (bfd_section_vma (s->the_bfd_section)					\
-   + obj_section_offset (s))
-
-/* The one-passed-the-end memory address of section S
-   (vma + size + offset).  */
-#define obj_section_endaddr(s)						\
-  (bfd_section_vma (s->the_bfd_section)					\
-   + bfd_section_size ((s)->the_bfd_section)				\
-   + obj_section_offset (s))
-
 #define ALL_OBJFILE_OSECTIONS(objfile, osect)	\
   for (osect = objfile->sections; osect < objfile->sections_end; osect++) \
     if (osect->the_bfd_section == NULL)					\
@@ -664,6 +633,27 @@ struct objfile
   /* See quick_symbol_functions.  */
   void require_partial_symbols (bool verbose);
 
+  /* Return the relocation offset applied to SECTION.  */
+  CORE_ADDR section_offset (bfd_section *section) const
+  {
+    /* The section's owner can be nullptr if it is one of the _bfd_std_section
+       section.  */
+    gdb_assert (section->owner == nullptr || section->owner == this->obfd);
+
+    int idx = gdb_bfd_section_index (this->obfd, section);
+    return this->section_offsets[idx];
+  }
+
+  /* Set the relocation offset applied to SECTION.  */
+  void set_section_offset (bfd_section *section, CORE_ADDR offset)
+  {
+    /* The section's owner can be nullptr if it is one of the _bfd_std_section
+       section.  */
+    gdb_assert (section->owner == nullptr || section->owner == this->obfd);
+
+    int idx = gdb_bfd_section_index (this->obfd, section);
+    this->section_offsets[idx] = offset;
+  }
 
   /* The object file's original name as specified by the user,
      made absolute, and tilde-expanded.  However, it is not canonicalized
@@ -836,6 +826,47 @@ 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.  */
+  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;
+
+  /* True if this "overlay section" is mapped into an "overlay region".  */
+  int ovly_mapped;
+};
+
 /* Declarations for functions defined in objfiles.c */
 
 extern int entry_point_address_query (CORE_ADDR *entry_p);
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 22fa5c074d1c..3cd42f817f51 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -1525,8 +1525,7 @@ info_symbol_command (const char *arg, int from_tty)
 
 	sect_addr = overlay_mapped_address (addr, osect);
 
-	if (obj_section_addr (osect) <= sect_addr
-	    && sect_addr < obj_section_endaddr (osect)
+	if (osect->addr () <= sect_addr && sect_addr < osect->endaddr ()
 	    && (msymbol
 		= lookup_minimal_symbol_by_pc_section (sect_addr,
 						       osect).minsym))
diff --git a/gdb/solib-aix.c b/gdb/solib-aix.c
index 4b58058ffef8..772cf23d9a09 100644
--- a/gdb/solib-aix.c
+++ b/gdb/solib-aix.c
@@ -685,8 +685,7 @@ solib_aix_get_toc_value (CORE_ADDR pc)
 	     "(%s has no data section)"),
 	   core_addr_to_string (pc), objfile_name (pc_osect->objfile));
 
-  result = (obj_section_addr (data_osect)
-	    + xcoff_get_toc_offset (pc_osect->objfile));
+  result = data_osect->addr () + xcoff_get_toc_offset (pc_osect->objfile);
 
   solib_aix_debug_printf ("pc=%s -> %s", core_addr_to_string (pc),
 			  core_addr_to_string (result));
diff --git a/gdb/solib-dsbt.c b/gdb/solib-dsbt.c
index c279435c89a1..803467dd4896 100644
--- a/gdb/solib-dsbt.c
+++ b/gdb/solib-dsbt.c
@@ -925,7 +925,7 @@ dsbt_relocate_main_executable (void)
       osect_idx = osect - objf->sections;
 
       /* Current address of section.  */
-      addr = obj_section_addr (osect);
+      addr = osect->addr ();
       /* Offset from where this section started.  */
       offset = objf->section_offsets[osect_idx];
       /* Original address prior to any past relocations.  */
diff --git a/gdb/solib-frv.c b/gdb/solib-frv.c
index c78fd559387c..c0b546ba68f5 100644
--- a/gdb/solib-frv.c
+++ b/gdb/solib-frv.c
@@ -797,7 +797,7 @@ frv_relocate_main_executable (void)
       osect_idx = osect - objf->sections;
 
       /* Current address of section.  */
-      addr = obj_section_addr (osect);
+      addr = osect->addr ();
       /* Offset from where this section started.  */
       offset = objf->section_offsets[osect_idx];
       /* Original address prior to any past relocations.  */
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 5ae3093095cc..0eb48d04d5be 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -3012,7 +3012,7 @@ pc_in_unmapped_range (CORE_ADDR pc, struct obj_section *section)
 
       /* We assume the LMA is relocated by the same offset as the VMA.  */
       bfd_vma size = bfd_section_size (bfd_section);
-      CORE_ADDR offset = obj_section_offset (section);
+      CORE_ADDR offset = section->offset ();
 
       if (bfd_section_lma (bfd_section) + offset <= pc
 	  && pc < bfd_section_lma (bfd_section) + offset + size)
@@ -3030,8 +3030,8 @@ pc_in_mapped_range (CORE_ADDR pc, struct obj_section *section)
 {
   if (section_is_overlay (section))
     {
-      if (obj_section_addr (section) <= pc
-	  && pc < obj_section_endaddr (section))
+      if (section->addr () <= pc
+	  && pc < section->endaddr ())
 	return 1;
     }
 
@@ -3044,10 +3044,10 @@ pc_in_mapped_range (CORE_ADDR pc, struct obj_section *section)
 static int
 sections_overlap (struct obj_section *a, struct obj_section *b)
 {
-  CORE_ADDR a_start = obj_section_addr (a);
-  CORE_ADDR a_end = obj_section_endaddr (a);
-  CORE_ADDR b_start = obj_section_addr (b);
-  CORE_ADDR b_end = obj_section_endaddr (b);
+  CORE_ADDR a_start = a->addr ();
+  CORE_ADDR a_end = a->endaddr ();
+  CORE_ADDR b_start = b->addr ();
+  CORE_ADDR b_end = b->endaddr ();
 
   return (a_start < b_end && b_start < a_end);
 }
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 7fd037f97013..6a9c3dce6af0 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -1698,8 +1698,7 @@ fixup_section (struct general_symbol_info *ginfo,
 	  if (fallback == -1)
 	    fallback = idx;
 
-	  if (obj_section_addr (s) - offset <= addr
-	      && addr < obj_section_endaddr (s) - offset)
+	  if (s->addr () - offset <= addr && addr < s->endaddr () - offset)
 	    {
 	      ginfo->set_section_index (idx);
 	      return;
diff --git a/gdb/tilegx-tdep.c b/gdb/tilegx-tdep.c
index 9bd982f49f50..73ed1d8d8481 100644
--- a/gdb/tilegx-tdep.c
+++ b/gdb/tilegx-tdep.c
@@ -747,7 +747,7 @@ tilegx_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR start_pc)
   s = find_pc_section (start_pc);
   end_pc = start_pc + 8 * TILEGX_BUNDLE_SIZE_IN_BYTES;
   if (s != NULL)
-    end_pc = std::min (end_pc, obj_section_endaddr (s));
+    end_pc = std::min (end_pc, s->endaddr ());
 
   /* Otherwise, try to skip prologue the hard way.  */
   return tilegx_analyze_prologue (gdbarch,
diff --git a/gdb/tui/tui-disasm.c b/gdb/tui/tui-disasm.c
index 163552aede4a..ad4f4a3d7d0a 100644
--- a/gdb/tui/tui-disasm.c
+++ b/gdb/tui/tui-disasm.c
@@ -178,7 +178,7 @@ tui_find_backward_disassembly_start_address (CORE_ADDR addr)
      section.  */
   struct obj_section *section = find_pc_section (addr);
   if (section != NULL)
-    return obj_section_addr (section);
+    return section->addr ();
 
   return addr;
 }
diff --git a/gdb/xstormy16-tdep.c b/gdb/xstormy16-tdep.c
index 8a0199cd9eb1..834c8f0c8759 100644
--- a/gdb/xstormy16-tdep.c
+++ b/gdb/xstormy16-tdep.c
@@ -562,8 +562,8 @@ xstormy16_find_jmp_table_entry (struct gdbarch *gdbarch, CORE_ADDR faddr)
 	{
 	  CORE_ADDR addr, endaddr;
 
-	  addr = obj_section_addr (osect);
-	  endaddr = obj_section_endaddr (osect);
+	  addr = osect->addr ();
+	  endaddr = osect->endaddr ();
 
 	  for (; addr < endaddr; addr += 2 * xstormy16_inst_size)
 	    {
-- 
2.32.0

[PATCH 2/2] gdb: use gdb_bfd_count_sections in macho_symfile_offsets

[Simon Marchi]

When loading a mach-o (macOS) executable and trying to set a breakpoint,
a GDB built with ASan or -D_GLIBCXX_DEBUG will crash with an
out-of-bound vector access.  This can be reproduced on Linux using the
repro files in bug 28017 [1]:

    $ ./gdb -nx --data-directory=data-directory -q repro/test -ex "b main" -batch
    /usr/include/c++/11.1.0/debug/vector:445:
    In function:
        std::__debug::vector<_Tp, _Allocator>::const_reference
        std::__debug::vector<_Tp,
        _Allocator>::operator[](std::__debug::vector<_Tp,
        _Allocator>::size_type) const [with _Tp = long unsigned int; _Allocator
        = std::allocator<long unsigned int>; std::__debug::vector<_Tp,
        _Allocator>::const_reference = const long unsigned int&;
        std::__debug::vector<_Tp, _Allocator>::size_type = long unsigned int]

    Error: attempt to subscript container with out-of-bounds index 13, but
    container only holds 13 elements.

    Objects involved in the operation:
        sequence "this" @ 0x0x61300000a590 {
          type = std::__debug::vector<unsigned long, std::allocator<unsigned long> >;
        }

The out-of-bound access happens here:

    #0  0x00007ffff6405d22 in raise () from /usr/lib/libc.so.6
    #1  0x00007ffff63ef862 in abort () from /usr/lib/libc.so.6
    #2  0x00007ffff664e21e in __gnu_debug::_Error_formatter::_M_error() const [clone .cold] from /usr/lib/libstdc++.so.6
    #3  0x000055555699e5ff in std::__debug::vector<unsigned long, std::allocator<unsigned long> >::operator[] (this=0x61300000a590, __n=13) at /usr/include/c++/11.1.0/debug/vector:445
    #4  0x0000555556a58c17 in objfile::section_offset (this=0x61300000a4c0, section=0x55555bbe4ac0 <_bfd_std_section>) at /home/simark/src/binutils-gdb/gdb/objfiles.h:644
    #5  0x0000555556a58cac in obj_section::offset (this=0x62100016d2a8) at /home/simark/src/binutils-gdb/gdb/objfiles.h:838
    #6  0x0000555556a58cfa in obj_section::addr (this=0x62100016d2a8) at /home/simark/src/binutils-gdb/gdb/objfiles.h:850
    #7  0x000055555779f5f7 in sort_cmp (sect1=0x62100016d2a8, sect2=0x62100016d170) at /home/simark/src/binutils-gdb/gdb/objfiles.c:902
    #8  0x00005555577aae35 in __gnu_cxx::__ops::_Iter_comp_iter<bool (*)(obj_section const*, obj_section const*)>::operator()<obj_section**, obj_section**> (this=0x7fffffffa9e0, __it1=0x60c000015970, __it2=0x60c000015940) at /usr/include/c++/11.1.0/bits/predefined_ops.h:158
    #9  0x00005555577aa2b8 in std::__insertion_sort<obj_section**, __gnu_cxx::__ops::_Iter_comp_iter<bool (*)(obj_section const*, obj_section const*)> > (__first=0x60c000015940, __last=0x60c0000159c0, __comp=...) at /usr/include/c++/11.1.0/bits/stl_algo.h:1826
    #10 0x00005555577a8e26 in std::__final_insertion_sort<obj_section**, __gnu_cxx::__ops::_Iter_comp_iter<bool (*)(obj_section const*, obj_section const*)> > (__first=0x60c000015940, __last=0x60c0000159c0, __comp=...) at /usr/include/c++/11.1.0/bits/stl_algo.h:1871
    #11 0x00005555577a723c in std::__sort<obj_section**, __gnu_cxx::__ops::_Iter_comp_iter<bool (*)(obj_section const*, obj_section const*)> > (__first=0x60c000015940, __last=0x60c0000159c0, __comp=...) at /usr/include/c++/11.1.0/bits/stl_algo.h:1957
    #12 0x00005555577a50f4 in std::sort<obj_section**, bool (*)(obj_section const*, obj_section const*)> (__first=0x60c000015940, __last=0x60c0000159c0, __comp=0x55555779f4e7 <sort_cmp(obj_section const*, obj_section const*)>) at /usr/include/c++/11.1.0/bits/stl_algo.h:4875
    #13 0x00005555577a147e in update_section_map (pspace=0x61200001d2c0, pmap=0x6030000d40b0, pmap_size=0x6030000d40b8) at /home/simark/src/binutils-gdb/gdb/objfiles.c:1165
    #14 0x00005555577a19a0 in find_pc_section (pc=0x100003fa0) at /home/simark/src/binutils-gdb/gdb/objfiles.c:1212
    #15 0x00005555576dd39e in lookup_minimal_symbol_by_pc_section (pc_in=0x100003fa0, section=0x0, prefer=lookup_msym_prefer::TEXT, previous=0x0) at /home/simark/src/binutils-gdb/gdb/minsyms.c:750
    #16 0x00005555576de552 in lookup_minimal_symbol_by_pc (pc=0x100003fa0) at /home/simark/src/binutils-gdb/gdb/minsyms.c:986
    #17 0x0000555557d44b54 in find_pc_sect_line (pc=0x100003fa0, section=0x62100016d170, notcurrent=0) at /home/simark/src/binutils-gdb/gdb/symtab.c:3163
    #18 0x0000555557d489fa in find_function_start_sal_1 (func_addr=0x100003fa0, section=0x62100016d170, funfirstline=true) at /home/simark/src/binutils-gdb/gdb/symtab.c:3650
    #19 0x0000555557d49015 in find_function_start_sal (sym=0x621000191670, funfirstline=true) at /home/simark/src/binutils-gdb/gdb/symtab.c:3706
    #20 0x0000555557485283 in symbol_to_sal (result=0x7fffffffbb30, funfirstline=1, sym=0x621000191670) at /home/simark/src/binutils-gdb/gdb/linespec.c:4460
    #21 0x00005555574728c2 in convert_linespec_to_sals (state=0x7fffffffc390, ls=0x7fffffffc3e0) at /home/simark/src/binutils-gdb/gdb/linespec.c:2335
    #22 0x0000555557475a8e in parse_linespec (parser=0x7fffffffc360, arg=0x60200007a550 "main", match_type=symbol_name_match_type::WILD) at /home/simark/src/binutils-gdb/gdb/linespec.c:2716
    #23 0x0000555557479027 in event_location_to_sals (parser=0x7fffffffc360, location=0x606000097be0) at /home/simark/src/binutils-gdb/gdb/linespec.c:3173
    #24 0x00005555574798f7 in decode_line_full (location=0x606000097be0, flags=1, search_pspace=0x0, default_symtab=0x0, default_line=0, canonical=0x7fffffffcca0, select_mode=0x0, filter=0x0) at /home/simark/src/binutils-gdb/gdb/linespec.c:3253
    #25 0x0000555556b4949f in parse_breakpoint_sals (location=0x606000097be0, canonical=0x7fffffffcca0) at /home/simark/src/binutils-gdb/gdb/breakpoint.c:9134
    #26 0x0000555556b6ce95 in create_sals_from_location_default (location=0x606000097be0, canonical=0x7fffffffcca0, type_wanted=bp_breakpoint) at /home/simark/src/binutils-gdb/gdb/breakpoint.c:13819
    #27 0x0000555556b645a6 in bkpt_create_sals_from_location (location=0x606000097be0, canonical=0x7fffffffcca0, type_wanted=bp_breakpoint) at /home/simark/src/binutils-gdb/gdb/breakpoint.c:12631
    #28 0x0000555556b4badf in create_breakpoint (gdbarch=0x621000152d10, location=0x606000097be0, cond_string=0x0, thread=0, extra_string=0x0, force_condition=false, parse_extra=1, tempflag=0, type_wanted=bp_breakpoint, ignore_count=0, pending_break_support=AUTO_BOOLEAN_AUTO, ops=0x55555bd728a0 <bkpt_breakpoint_ops>, from_tty=0, enabled=1, internal=0, flags=0) at /home/simark/src/binutils-gdb/gdb/breakpoint.c:9410
    #29 0x0000555556b4d3b1 in break_command_1 (arg=0x7fffffffe291 "", flag=0, from_tty=0) at /home/simark/src/binutils-gdb/gdb/breakpoint.c:9590
    #30 0x0000555556b4dc1b in break_command (arg=0x7fffffffe28d "main", from_tty=0) at /home/simark/src/binutils-gdb/gdb/breakpoint.c:9660
    #31 0x0000555556d24ca9 in do_const_cfunc (c=0x61100003a240, args=0x7fffffffe28d "main", from_tty=0) at /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:102
    #32 0x0000555556d2fcd3 in cmd_func (cmd=0x61100003a240, args=0x7fffffffe28d "main", from_tty=0) at /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:2160
    #33 0x0000555557e84e93 in execute_command (p=0x7fffffffe290 "n", from_tty=0) at /home/simark/src/binutils-gdb/gdb/top.c:674
    #34 0x00005555575a9933 in catch_command_errors (command=0x555557e84043 <execute_command(char const*, int)>, arg=0x7fffffffe28b "b main", from_tty=0, do_bp_actions=true) at /home/simark/src/binutils-gdb/gdb/main.c:523
    #35 0x00005555575a9fdb in execute_cmdargs (cmdarg_vec=0x7fffffffd910, file_type=CMDARG_FILE, cmd_type=CMDARG_COMMAND, ret=0x7fffffffd5b0) at /home/simark/src/binutils-gdb/gdb/main.c:618
    #36 0x00005555575ad48a in captured_main_1 (context=0x7fffffffdd00) at /home/simark/src/binutils-gdb/gdb/main.c:1322
    #37 0x00005555575ada9c in captured_main (data=0x7fffffffdd00) at /home/simark/src/binutils-gdb/gdb/main.c:1343
    #38 0x00005555575adb31 in gdb_main (args=0x7fffffffdd00) at /home/simark/src/binutils-gdb/gdb/main.c:1368
    #39 0x000055555681e179 in main (argc=8, argv=0x7fffffffde78) at /home/simark/src/binutils-gdb/gdb/gdb.c:32

The section being dealt with at that moment is the special *COM*
section:

    (top-gdb) p section.name
    $1 = 0x55555a1bbe60 "*COM*"
    (top-gdb) p section
    $2 = (bfd_section *) 0x55555bbe4ac0 <_bfd_std_section>

I'm not too sure what this section is for, but this is one of four
special BFD sections that GDB puts after the regular sections in the
objfile::sections and objfile::section_offsets lists.  You can check
gdb_bfd_section_index to see how they are handled.
gdb_bfd_count_sections returns "+ 4" to account for those sections.

The problem is that macho_symfile_offsets uses bfd_count_sections
instead of gdb_bfd_count_sections when allocating the
objfile::section_offsets vector.  The vector will therefore contain,
say, 13 elements instead of 17.  When trying to access the section
offset of the *COM* section, the first after the regular sections, we
access section_offsets[13], which is out of bounds.

Fix that by using gdb_bfd_count_sections instead of bfd_count_sections.
I'm fairly confident that this is correct, as this is what
default_symfile_offsets does.

With this patch, the command shown above terminates normally:

    $ ./gdb -nx --data-directory=data-directory -q repro/test -ex "b main" -batch
    Breakpoint 1 at 0x100003fad: file test.c, line 2.

[1] https://sourceware.org/bugzilla/show_bug.cgi?id=28017

gdb/ChangeLog:

	PR gdb/28017
	* machoread.c (macho_symfile_offsets): Use
	gdb_bfd_count_sections to allocate objfile::section_offsets.

Change-Id: Ic3a56f46f7232e9f24581f8255fc1ab981935450
---
 gdb/machoread.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/machoread.c b/gdb/machoread.c
index a0f0307d5293..b3bd54940b40 100644
--- a/gdb/machoread.c
+++ b/gdb/machoread.c
@@ -907,7 +907,7 @@ macho_symfile_offsets (struct objfile *objfile,
   struct obj_section *osect;
 
   /* Allocate section_offsets.  */
-  objfile->section_offsets.assign (bfd_count_sections (objfile->obfd), 0);
+  objfile->section_offsets.assign (gdb_bfd_count_sections (objfile->obfd), 0);
 
   /* This code is run when we first add the objfile with
      symfile_add_with_addrs_or_offsets, when "addrs" not "offsets" are
-- 
2.32.0

Re: [PATCH 1/2] gdb: convert obj_section macros to methods

[Tom Tromey]

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> Convert these three macros to methods of obj_section.  The problem fixed
Simon> by the following patch is caused by an out of bound access of the
Simon> objfile::section_offsets vector.  Since this is deep in macros, we don't
Simon> get a clear backtrace and it's difficult to debug.  Changing that to
Simon> methods means we can step in them and break on them.

Thanks, this looks good.

Tom

Re: [PATCH 2/2] gdb: use gdb_bfd_count_sections in macho_symfile_offsets

[Tom Tromey]

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> gdb/ChangeLog:

Simon> 	PR gdb/28017
Simon> 	* machoread.c (macho_symfile_offsets): Use
Simon> 	gdb_bfd_count_sections to allocate objfile::section_offsets.

Looks good.  Thank you.

Tom

Re: [PATCH 2/2] gdb: use gdb_bfd_count_sections in macho_symfile_offsets

[Simon Marchi]

On 2021-06-28 2:46 p.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> gdb/ChangeLog:
> 
> Simon> 	PR gdb/28017
> Simon> 	* machoread.c (macho_symfile_offsets): Use
> Simon> 	gdb_bfd_count_sections to allocate objfile::section_offsets.
> 
> Looks good.  Thank you.
> 
> Tom
> 

Pushed both, thanks.

Simon