[PATCH 0/8] More xcoffread cleanups

[PATCH 0/8] More xcoffread cleanups

[Simon Marchi]

Here is another batch of cleanups for xcoffread.c, removing things that
are no longer used.  I didn't manage to build GDB on one of the the AIX
machines on the compile farm.  I did, however, copy an AIX executable to
my Linux workstation and checked that GDB found what seemed to me like a
sensible TOC offset value.

Simon Marchi (8):
  gdb/xcoffread: remove name computation
  gdb/xcoffread: remove read of .debug section
  gdb/xcoffread: remove XCOFF_DATA macro
  gdb/xcoffread: remove xcoff_symfile_info::{symtbl,symtbl_num_syms}
  gdb/xcoffread: allocate symbol table using vector in scan_xcoff_symtab
  gdb/xcoffread: simplify xcoff_secnum_to_sections
  gdb/xcoffread: replace 2 switches with if
  gdb/xcoffread: stylistic cleanup

 gdb/xcoffread.c | 317 +++++++++++++-----------------------------------
 1 file changed, 81 insertions(+), 236 deletions(-)


base-commit: 3e5d318177d372278796cfbbb13a4d75966cffd1
-- 
2.52.0

[PATCH 1/8] gdb/xcoffread: remove name computation

[Simon Marchi]

It is no longer necessary to get names of symbols.  The only thing we
are interested in is the symbol to get the TOC offset, for which the
name doesn't matter.

Change-Id: I47e6009ed7600ea9c412fb25ed21a295cd0fec49
---
 gdb/xcoffread.c | 40 +++++-----------------------------------
 1 file changed, 5 insertions(+), 35 deletions(-)

diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
index ef78a4c9e594..d46b4eb23f3d 100644
--- a/gdb/xcoffread.c
+++ b/gdb/xcoffread.c
@@ -38,9 +38,6 @@
 \f
 struct xcoff_symfile_info
   {
-    /* Pointer to the string table.  */
-    char *strtbl = nullptr;
-
     /* Pointer to debug section.  */
     char *debugsec = nullptr;
 
@@ -141,40 +138,15 @@ xcoff_symfile_init (struct objfile *objfile)
   xcoff_objfile_data_key.emplace (objfile);
 }
 
-/* Swap raw symbol at *RAW and put the name in *NAME, the symbol in
-   *SYMBOL, the first auxent in *AUX.  Advance *RAW and *SYMNUMP over
-   the symbol and its auxents.  */
+/* Swap raw symbol at *RAW.  Put the symbol in *SYMBOL and the first auxent in
+   *AUX.  Advance *RAW and *SYMNUMP over the symbol and its auxents.  */
 
 static void
 swap_sym (struct internal_syment *symbol, union internal_auxent *aux,
-	  const char **name, char **raw, unsigned int *symnump,
-	  struct objfile *objfile)
+	  char **raw, unsigned int *symnump, struct objfile *objfile)
 {
   bfd_coff_swap_sym_in (objfile->obfd.get (), *raw, symbol);
-  if (symbol->n_zeroes)
-    {
-      /* If it's exactly E_SYMNMLEN characters long it isn't
-	 '\0'-terminated.  */
-      if (symbol->n_name[E_SYMNMLEN - 1] != '\0')
-	{
-	  /* FIXME: wastes memory for symbols which we don't end up putting
-	     into the minimal symbols.  */
-	  *name = obstack_strndup (&objfile->objfile_obstack,
-				    symbol->n_name, E_SYMNMLEN);
-	}
-      else
-	/* Point to the unswapped name as that persists as long as the
-	   objfile does.  */
-	*name = ((struct external_syment *) *raw)->e.e_name;
-    }
-  else if (symbol->n_sclass & 0x80)
-    {
-      *name = XCOFF_DATA (objfile)->debugsec + symbol->n_offset;
-    }
-  else
-    {
-      *name = XCOFF_DATA (objfile)->strtbl + symbol->n_offset;
-    }
+
   ++*symnump;
   *raw += coff_data (objfile->obfd)->local_symesz;
   if (symbol->n_numaux > 0)
@@ -192,7 +164,6 @@ scan_xcoff_symtab (struct objfile *objfile)
 {
   CORE_ADDR toc_offset = 0;	/* toc offset value in data section.  */
 
-  const char *namestring;
   bfd *abfd;
   asection *bfd_sect = nullptr;
   int ignored;
@@ -224,8 +195,7 @@ scan_xcoff_symtab (struct objfile *objfile)
 	    /* The CSECT auxent--always the last auxent.  */
 	    union internal_auxent csect_aux;
 
-	    swap_sym (&symbol, &main_aux[0], &namestring, &sraw_symbol,
-		      &ssymnum, objfile);
+	    swap_sym (&symbol, &main_aux[0], &sraw_symbol, &ssymnum, objfile);
 	    if (symbol.n_numaux > 1)
 	      {
 		bfd_coff_swap_aux_in
-- 
2.52.0

Re: [PATCH 1/8] gdb/xcoffread: remove name computation

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:

Simon> It is no longer necessary to get names of symbols.  The only thing we
Simon> are interested in is the symbol to get the TOC offset, for which the
Simon> name doesn't matter.

FWIW I also convinced myself this was ok and not some oversight.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

[PATCH 2/8] gdb/xcoffread: remove read of .debug section

[Simon Marchi]

This is not used by anything.

Change-Id: Icc5e72b2f30521b8856db46f4d6cabf26f615b0b
---
 gdb/xcoffread.c | 34 ----------------------------------
 1 file changed, 34 deletions(-)

diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
index d46b4eb23f3d..43d3cec2e49b 100644
--- a/gdb/xcoffread.c
+++ b/gdb/xcoffread.c
@@ -38,9 +38,6 @@
 \f
 struct xcoff_symfile_info
   {
-    /* Pointer to debug section.  */
-    char *debugsec = nullptr;
-
     /* Pointer to the a.out symbol table.  */
     char *symtbl = nullptr;
 
@@ -300,37 +297,6 @@ xcoff_initial_scan (struct objfile *objfile, symfile_add_flags symfile_flags)
   num_symbols = bfd_get_symcount (abfd);	/* # of symbols */
   symtab_offset = obj_sym_filepos (abfd);	/* symbol table file offset */
 
-  if (num_symbols > 0)
-    {
-      /* Read the .debug section, if present and if we're not ignoring
-	 it.  */
-      if (!(objfile->flags & OBJF_READNEVER))
-	{
-	  struct bfd_section *secp;
-	  bfd_size_type length;
-	  bfd_byte *debugsec = NULL;
-
-	  secp = bfd_get_section_by_name (abfd, ".debug");
-	  if (secp)
-	    {
-	      length = bfd_get_section_alloc_size (abfd, secp);
-	      if (length)
-		{
-		  debugsec
-		    = (bfd_byte *) obstack_alloc (&objfile->objfile_obstack,
-						  length);
-
-		  if (!bfd_get_full_section_contents (abfd, secp, &debugsec))
-		    {
-		      error (_("Error reading .debug section of `%s': %s"),
-			     name, bfd_errmsg (bfd_get_error ()));
-		    }
-		}
-	    }
-	  info->debugsec = (char *) debugsec;
-	}
-    }
-
   /* Read the symbols.  We keep them in core because we will want to
      access them randomly in read_symbol*.  */
   val = bfd_seek (abfd, symtab_offset, SEEK_SET);
-- 
2.52.0

Re: [PATCH 2/8] gdb/xcoffread: remove read of .debug section

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:

Simon> This is not used by anything.

Yep.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

[PATCH 3/8] gdb/xcoffread: remove XCOFF_DATA macro

[Simon Marchi]

Change-Id: I3e2c681689f8386805041c9f007d399df545f6cd
---
 gdb/xcoffread.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
index 43d3cec2e49b..052bd6b78b28 100644
--- a/gdb/xcoffread.c
+++ b/gdb/xcoffread.c
@@ -52,11 +52,6 @@ struct xcoff_symfile_info
 
 static const registry<objfile>::key<xcoff_symfile_info> xcoff_objfile_data_key;
 
-/* Convenience macro to access the per-objfile XCOFF data.  */
-
-#define XCOFF_DATA(objfile)						\
-  xcoff_objfile_data_key.get (objfile)
-
 /* XCOFF names for dwarf sections.  There is no compressed sections.  */
 
 static const struct dwarf2_debug_sections dwarf2_xcoff_names = {
@@ -173,8 +168,8 @@ scan_xcoff_symtab (struct objfile *objfile)
 
   abfd = objfile->obfd.get ();
 
-  sraw_symbol = XCOFF_DATA (objfile)->symtbl;
-  nsyms = XCOFF_DATA (objfile)->symtbl_num_syms;
+  sraw_symbol = xcoff_objfile_data_key.get (objfile)->symtbl;
+  nsyms = xcoff_objfile_data_key.get (objfile)->symtbl_num_syms;
   ssymnum = 0;
   while (ssymnum < nsyms)
     {
@@ -257,7 +252,7 @@ scan_xcoff_symtab (struct objfile *objfile)
      Another place to obtain this information would be file auxiliary
      header.  */
 
-  XCOFF_DATA (objfile)->toc_offset = toc_offset;
+  xcoff_objfile_data_key.get (objfile)->toc_offset = toc_offset;
 }
 
 /* Return the toc offset value for a given objfile.  */
@@ -266,7 +261,7 @@ CORE_ADDR
 xcoff_get_toc_offset (struct objfile *objfile)
 {
   if (objfile)
-    return XCOFF_DATA (objfile)->toc_offset;
+    return xcoff_objfile_data_key.get (objfile)->toc_offset;
   return 0;
 }
 
@@ -290,7 +285,7 @@ xcoff_initial_scan (struct objfile *objfile, symfile_add_flags symfile_flags)
   const char *name;
   unsigned int size;
 
-  info = XCOFF_DATA (objfile);
+  info = xcoff_objfile_data_key.get (objfile);
   abfd = objfile->obfd.get ();
   name = objfile_name (objfile);
 
-- 
2.52.0

Re: [PATCH 3/8] gdb/xcoffread: remove XCOFF_DATA macro

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:

Ok.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

[PATCH 4/8] gdb/xcoffread: remove xcoff_symfile_info::{symtbl, symtbl_num_syms}

[Simon Marchi]

It is not necessary to save the symbol table content and number of
symbols in xcoff_symfile_info, because they are only needed for the
duration of scan_xcoff_symtab.

Move the reading of the symbol table content to scan_xcoff_symtab, and
keep everything as local variables.

Change-Id: I21e2b95a0b8df2255ba46904083658a4e8cb89f0
---
 gdb/xcoffread.c | 62 ++++++++++++++-----------------------------------
 1 file changed, 18 insertions(+), 44 deletions(-)

diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
index 052bd6b78b28..0f3470cae45d 100644
--- a/gdb/xcoffread.c
+++ b/gdb/xcoffread.c
@@ -38,12 +38,6 @@
 \f
 struct xcoff_symfile_info
   {
-    /* Pointer to the a.out symbol table.  */
-    char *symtbl = nullptr;
-
-    /* Number of symbols in symtbl.  */
-    int symtbl_num_syms = 0;
-
     /* Offset in data section to TOC anchor.  */
     CORE_ADDR toc_offset = 0;
   };
@@ -156,22 +150,32 @@ scan_xcoff_symtab (struct objfile *objfile)
 {
   CORE_ADDR toc_offset = 0;	/* toc offset value in data section.  */
 
-  bfd *abfd;
   asection *bfd_sect = nullptr;
   int ignored;
-  unsigned int nsyms;
-
-  char *sraw_symbol;
+  bfd *abfd = objfile->obfd.get ();
+  file_ptr symtab_offset = obj_sym_filepos (abfd);
   struct internal_syment symbol;
   union internal_auxent main_aux[5];
   unsigned int ssymnum;
 
-  abfd = objfile->obfd.get ();
+  /* Seek to symbol table location.  */
+  if (int ret = bfd_seek (abfd, symtab_offset, SEEK_SET);
+      ret < 0)
+    error (_("Error reading symbols from %s: %s"),
+	   objfile_name (objfile), bfd_errmsg (bfd_get_error ()));
 
-  sraw_symbol = xcoff_objfile_data_key.get (objfile)->symtbl;
-  nsyms = xcoff_objfile_data_key.get (objfile)->symtbl_num_syms;
+  unsigned int num_symbols = bfd_get_symcount (abfd);
+  size_t size = coff_data (abfd)->local_symesz * num_symbols;
+  char *symtbl = (char *) obstack_alloc (&objfile->objfile_obstack, size);
+
+  /* Read in symbol table.  */
+  if (int ret = bfd_read (symtbl, size, abfd);
+      ret != size)
+    error (_("reading symbol table: %s"), bfd_errmsg (bfd_get_error ()));
+
+  char *sraw_symbol = symtbl;
   ssymnum = 0;
-  while (ssymnum < nsyms)
+  while (ssymnum < num_symbols)
     {
       int sclass;
 
@@ -277,41 +281,11 @@ xcoff_get_toc_offset (struct objfile *objfile)
 static void
 xcoff_initial_scan (struct objfile *objfile, symfile_add_flags symfile_flags)
 {
-  bfd *abfd;
-  int val;
-  int num_symbols;		/* # of symbols */
-  file_ptr symtab_offset;	/* symbol table and */
-  struct xcoff_symfile_info *info;
-  const char *name;
-  unsigned int size;
-
-  info = xcoff_objfile_data_key.get (objfile);
-  abfd = objfile->obfd.get ();
-  name = objfile_name (objfile);
-
-  num_symbols = bfd_get_symcount (abfd);	/* # of symbols */
-  symtab_offset = obj_sym_filepos (abfd);	/* symbol table file offset */
-
-  /* Read the symbols.  We keep them in core because we will want to
-     access them randomly in read_symbol*.  */
-  val = bfd_seek (abfd, symtab_offset, SEEK_SET);
-  if (val < 0)
-    error (_("Error reading symbols from %s: %s"),
-	   name, bfd_errmsg (bfd_get_error ()));
-  size = coff_data (abfd)->local_symesz * num_symbols;
-  info->symtbl = (char *) obstack_alloc (&objfile->objfile_obstack, size);
-  info->symtbl_num_syms = num_symbols;
-
-  val = bfd_read (info->symtbl, size, abfd);
-  if (val != size)
-    error (_("reading symbol table: %s"), bfd_errmsg (bfd_get_error ()));
-
   /* We need to do this to get the TOC information only.  STABS
      format is no longer supported.  */
   scan_xcoff_symtab (objfile);
 
   /* DWARF2 sections.  */
-
   dwarf2_initialize_objfile (objfile, &dwarf2_xcoff_names);
 }
 \f
-- 
2.52.0

Re: [PATCH 4/8] gdb/xcoffread: remove xcoff_symfile_info::{symtbl, symtbl_num_syms}

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:

Simon> It is not necessary to save the symbol table content and number of
Simon> symbols in xcoff_symfile_info, because they are only needed for the
Simon> duration of scan_xcoff_symtab.

Simon> Move the reading of the symbol table content to scan_xcoff_symtab, and
Simon> keep everything as local variables.

Simon> +  /* Seek to symbol table location.  */
Simon> +  if (int ret = bfd_seek (abfd, symtab_offset, SEEK_SET);
Simon> +      ret < 0)
Simon> +    error (_("Error reading symbols from %s: %s"),
Simon> +	   objfile_name (objfile), bfd_errmsg (bfd_get_error ()));

'ret' doesn't really seem to be needed here.

Otherwise looks good.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

Re: [PATCH 4/8] gdb/xcoffread: remove xcoff_symfile_info::{symtbl, symtbl_num_syms}

[Simon Marchi]

On 1/8/26 3:36 PM, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:
> 
> Simon> It is not necessary to save the symbol table content and number of
> Simon> symbols in xcoff_symfile_info, because they are only needed for the
> Simon> duration of scan_xcoff_symtab.
> 
> Simon> Move the reading of the symbol table content to scan_xcoff_symtab, and
> Simon> keep everything as local variables.
> 
> Simon> +  /* Seek to symbol table location.  */
> Simon> +  if (int ret = bfd_seek (abfd, symtab_offset, SEEK_SET);
> Simon> +      ret < 0)
> Simon> +    error (_("Error reading symbols from %s: %s"),
> Simon> +	   objfile_name (objfile), bfd_errmsg (bfd_get_error ()));
> 
> 'ret' doesn't really seem to be needed here.

What do you mean, not needed?  Don't we want to check the return value
of bfd_seek?

Simon

Re: [PATCH 4/8] gdb/xcoffread: remove xcoff_symfile_info::{symtbl, symtbl_num_syms}

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> +  /* Seek to symbol table location.  */
Simon> +  if (int ret = bfd_seek (abfd, symtab_offset, SEEK_SET);
Simon> +      ret < 0)

>> 'ret' doesn't really seem to be needed here.

Simon> What do you mean, not needed?  Don't we want to check the return value
Simon> of bfd_seek?

I just mean the variable, so:

    if (bfd_seek (...) < 0)

Tom

Re: [PATCH 4/8] gdb/xcoffread: remove xcoff_symfile_info::{symtbl, symtbl_num_syms}

[Simon Marchi]

On 2026-01-08 17:17, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
> 
> Simon> +  /* Seek to symbol table location.  */
> Simon> +  if (int ret = bfd_seek (abfd, symtab_offset, SEEK_SET);
> Simon> +      ret < 0)
> 
>>> 'ret' doesn't really seem to be needed here.
> 
> Simon> What do you mean, not needed?  Don't we want to check the return value
> Simon> of bfd_seek?
> 
> I just mean the variable, so:
> 
>     if (bfd_seek (...) < 0)
> 
> Tom

Oh, silly me, make sense.  I'll change that and push the series, thanks.

Simon

[PATCH 5/8] gdb/xcoffread: allocate symbol table using vector in scan_xcoff_symtab

[Simon Marchi]

The symbol table content is only needed for the duration of
scan_xcoff_symtab, so it's not necessary to allocate it on the objfile
obstack.  Switch to using a vector that is freed at the end of the
function.

Change-Id: Ib5acdea460969300a92816a3e71b7d82c80faebf
---
 gdb/xcoffread.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
index 0f3470cae45d..7dd0f29cb107 100644
--- a/gdb/xcoffread.c
+++ b/gdb/xcoffread.c
@@ -166,14 +166,14 @@ scan_xcoff_symtab (struct objfile *objfile)
 
   unsigned int num_symbols = bfd_get_symcount (abfd);
   size_t size = coff_data (abfd)->local_symesz * num_symbols;
-  char *symtbl = (char *) obstack_alloc (&objfile->objfile_obstack, size);
+  gdb::char_vector symtbl (size);
 
   /* Read in symbol table.  */
-  if (int ret = bfd_read (symtbl, size, abfd);
+  if (int ret = bfd_read (symtbl.data (), size, abfd);
       ret != size)
     error (_("reading symbol table: %s"), bfd_errmsg (bfd_get_error ()));
 
-  char *sraw_symbol = symtbl;
+  char *sraw_symbol = symtbl.data ();
   ssymnum = 0;
   while (ssymnum < num_symbols)
     {
-- 
2.52.0

Re: [PATCH 5/8] gdb/xcoffread: allocate symbol table using vector in scan_xcoff_symtab

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:

Simon> The symbol table content is only needed for the duration of
Simon> scan_xcoff_symtab, so it's not necessary to allocate it on the objfile
Simon> obstack.  Switch to using a vector that is freed at the end of the
Simon> function.

Thanks, looks good.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

[PATCH 6/8] gdb/xcoffread: simplify xcoff_secnum_to_sections

[Simon Marchi]

The only remaining caller of xcoff_secnum_to_sections doesn't care about
the section index in the objfile, so remove that part.

Return the BFD section as the return value of the function, instead of
an output parameter.  Remove the unnecessary default in the switch, to
avoid a "jump to label over variable initialization" warning.

Rename to xcoff_secnum_to_section (singular).

Change-Id: I464d8b0e1425ea2732b0d61355c5b0c66218122e
---
 gdb/xcoffread.c | 41 ++++++++++-------------------------------
 1 file changed, 10 insertions(+), 31 deletions(-)

diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
index 7dd0f29cb107..5494bb0debbb 100644
--- a/gdb/xcoffread.c
+++ b/gdb/xcoffread.c
@@ -81,34 +81,18 @@ static void scan_xcoff_symtab (struct objfile *);
 static void xcoff_symfile_init (struct objfile *);
 
 /* Search all BFD sections for the section whose target_index is
-   equal to N_SCNUM.  Set *BFD_SECT to that section.  The section's
-   associated index in the objfile's section_offset table is also
-   stored in *SECNUM.
+   equal to N_SCNUM.
 
-   If no match is found, *BFD_SECT is set to NULL, and *SECNUM
-   is set to the text section's number.  */
+   If no match is found, return nullptr.  */
 
-static void
-xcoff_secnum_to_sections (int n_scnum, struct objfile *objfile,
-			  asection **bfd_sect, int *secnum)
+static asection *
+xcoff_secnum_to_section (int n_scnum, objfile *objfile)
 {
-  *bfd_sect = NULL;
-  *secnum = SECT_OFF_TEXT (objfile);
-
   for (asection *sec : gdb_bfd_sections (objfile->obfd.get ()))
-    {
-      if (sec->target_index == n_scnum)
-	{
-	  /* This is the section.  Figure out what SECT_OFF_* code it is.  */
-	  if (bfd_section_flags (sec) & SEC_CODE)
-	    *secnum = SECT_OFF_TEXT (objfile);
-	  else if (bfd_section_flags (sec) & SEC_LOAD)
-	    *secnum = SECT_OFF_DATA (objfile);
-	  else
-	    *secnum = gdb_bfd_section_index (objfile->obfd.get (), sec);
-	  *bfd_sect = sec;
-	}
-    }
+    if (sec->target_index == n_scnum)
+      return sec;
+
+  return nullptr;
 }
 
 /* Do initialization in preparation for reading symbols from OBJFILE.
@@ -150,8 +134,6 @@ scan_xcoff_symtab (struct objfile *objfile)
 {
   CORE_ADDR toc_offset = 0;	/* toc offset value in data section.  */
 
-  asection *bfd_sect = nullptr;
-  int ignored;
   bfd *abfd = objfile->obfd.get ();
   file_ptr symtab_offset = obj_sym_filepos (abfd);
   struct internal_syment symbol;
@@ -218,14 +200,11 @@ scan_xcoff_symtab (struct objfile *objfile)
 
 		    /* Make TOC offset relative to start address of
 		       section.  */
-		    xcoff_secnum_to_sections (symbol.n_scnum, objfile,
-							 &bfd_sect, &ignored);
+		    asection *bfd_sect
+		      = xcoff_secnum_to_section (symbol.n_scnum, objfile);
 		    if (bfd_sect)
 		      toc_offset -= bfd_section_vma (bfd_sect);
 		    break;
-
-		  default:
-		    break;
 		  }
 		break;
 	      }
-- 
2.52.0

Re: [PATCH 6/8] gdb/xcoffread: simplify xcoff_secnum_to_sections

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:

Simon> The only remaining caller of xcoff_secnum_to_sections doesn't care about
Simon> the section index in the objfile, so remove that part.

Simon> Return the BFD section as the return value of the function, instead of
Simon> an output parameter.  Remove the unnecessary default in the switch, to
Simon> avoid a "jump to label over variable initialization" warning.

Simon> Rename to xcoff_secnum_to_section (singular).

Looks good.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

[PATCH 7/8] gdb/xcoffread: replace 2 switches with if

[Simon Marchi]

Replace these 2 switches that have only one case with a single if
statement.  It is highly unlikely that we'll ever need to check for
other values.

Change-Id: I6a48d4f27ba4d1d128aaa39d5f3d693a11e99fc9
---
 gdb/xcoffread.c | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
index 5494bb0debbb..5ec7281b683b 100644
--- a/gdb/xcoffread.c
+++ b/gdb/xcoffread.c
@@ -188,24 +188,18 @@ scan_xcoff_symtab (struct objfile *objfile)
 	    else
 	      csect_aux = main_aux[0];
 
-	    switch (csect_aux.x_csect.x_smtyp & 0x7)
+	    if ((csect_aux.x_csect.x_smtyp & 0x7) == XTY_SD
+		&& csect_aux.x_csect.x_smclas == XMC_TC0)
 	      {
-	      case XTY_SD:
-		switch (csect_aux.x_csect.x_smclas)
-		  {
-		  case XMC_TC0:
-		    if (toc_offset)
-		      warning (_("More than one XMC_TC0 symbol found."));
-		    toc_offset = symbol.n_value;
+		if (toc_offset)
+		  warning (_("More than one XMC_TC0 symbol found."));
+		toc_offset = symbol.n_value;
 
-		    /* Make TOC offset relative to start address of
-		       section.  */
-		    asection *bfd_sect
-		      = xcoff_secnum_to_section (symbol.n_scnum, objfile);
-		    if (bfd_sect)
-		      toc_offset -= bfd_section_vma (bfd_sect);
-		    break;
-		  }
+		/* Make TOC offset relative to start address of section.  */
+		asection *bfd_sect
+		  = xcoff_secnum_to_section (symbol.n_scnum, objfile);
+		if (bfd_sect)
+		  toc_offset -= bfd_section_vma (bfd_sect);
 		break;
 	      }
 	  }
-- 
2.52.0

Re: [PATCH 7/8] gdb/xcoffread: replace 2 switches with if

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:

Simon> Replace these 2 switches that have only one case with a single if
Simon> statement.  It is highly unlikely that we'll ever need to check for
Simon> other values.

Thanks.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

[PATCH 8/8] gdb/xcoffread: stylistic cleanup

[Simon Marchi]

Do a few cleanups:

 - declaring variables where they are used
 - rename functions to be more precise
 - add explicit comparison against nullptr or 0
 - remove struct/union keywords
 - remove or update outdated comments

Change-Id: I3f9e8604f79ee4d36cbe3d65079cb23051fcc703
---
 gdb/xcoffread.c | 123 ++++++++++++++++++------------------------------
 1 file changed, 45 insertions(+), 78 deletions(-)

diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
index 5ec7281b683b..004035d1f31f 100644
--- a/gdb/xcoffread.c
+++ b/gdb/xcoffread.c
@@ -35,12 +35,11 @@
 #include "dwarf2/sect-names.h"
 #include "dwarf2/public.h"
 
-\f
 struct xcoff_symfile_info
-  {
-    /* Offset in data section to TOC anchor.  */
-    CORE_ADDR toc_offset = 0;
-  };
+{
+  /* Offset in data section to TOC anchor.  */
+  CORE_ADDR toc_offset = 0;
+};
 
 /* Key for XCOFF-associated data.  */
 
@@ -48,7 +47,7 @@ static const registry<objfile>::key<xcoff_symfile_info> xcoff_objfile_data_key;
 
 /* XCOFF names for dwarf sections.  There is no compressed sections.  */
 
-static const struct dwarf2_debug_sections dwarf2_xcoff_names = {
+static const dwarf2_debug_sections dwarf2_xcoff_names = {
   { ".dwinfo", NULL },
   { ".dwabrev", NULL },
   { ".dwline", NULL },
@@ -74,12 +73,6 @@ static const struct dwarf2_debug_sections dwarf2_xcoff_names = {
   23
 };
 
-static void xcoff_initial_scan (struct objfile *, symfile_add_flags);
-
-static void scan_xcoff_symtab (struct objfile *);
-
-static void xcoff_symfile_init (struct objfile *);
-
 /* Search all BFD sections for the section whose target_index is
    equal to N_SCNUM.
 
@@ -102,7 +95,7 @@ xcoff_secnum_to_section (int n_scnum, objfile *objfile)
    uses BFD's determination to vector to us.  */
 
 static void
-xcoff_symfile_init (struct objfile *objfile)
+xcoff_symfile_init (objfile *objfile)
 {
   /* Allocate struct to keep track of the symfile.  */
   xcoff_objfile_data_key.emplace (objfile);
@@ -129,16 +122,14 @@ swap_sym (struct internal_syment *symbol, union internal_auxent *aux,
     }
 }
 
-static void
-scan_xcoff_symtab (struct objfile *objfile)
-{
-  CORE_ADDR toc_offset = 0;	/* toc offset value in data section.  */
+/* Locate the TOC offset in the XCOFF symbol table and assign
+   xcoff_symfile_info::toc_offset.  */
 
+static void
+xcoff_find_toc_offset (objfile *objfile)
+{
   bfd *abfd = objfile->obfd.get ();
   file_ptr symtab_offset = obj_sym_filepos (abfd);
-  struct internal_syment symbol;
-  union internal_auxent main_aux[5];
-  unsigned int ssymnum;
 
   /* Seek to symbol table location.  */
   if (int ret = bfd_seek (abfd, symtab_offset, SEEK_SET);
@@ -156,22 +147,21 @@ scan_xcoff_symtab (struct objfile *objfile)
     error (_("reading symbol table: %s"), bfd_errmsg (bfd_get_error ()));
 
   char *sraw_symbol = symtbl.data ();
-  ssymnum = 0;
-  while (ssymnum < num_symbols)
+  CORE_ADDR toc_offset = 0;
+  for (unsigned int ssymnum = 0; ssymnum < num_symbols; )
     {
-      int sclass;
-
       QUIT;
 
+      internal_syment symbol;
       bfd_coff_swap_sym_in (abfd, sraw_symbol, &symbol);
-      sclass = symbol.n_sclass;
 
-      switch (sclass)
+      switch (symbol.n_sclass)
 	{
 	case C_HIDEXT:
 	  {
 	    /* The CSECT auxent--always the last auxent.  */
-	    union internal_auxent csect_aux;
+	    internal_auxent csect_aux;
+	    internal_auxent main_aux[5];
 
 	    swap_sym (&symbol, &main_aux[0], &sraw_symbol, &ssymnum, objfile);
 	    if (symbol.n_numaux > 1)
@@ -191,24 +181,24 @@ scan_xcoff_symtab (struct objfile *objfile)
 	    if ((csect_aux.x_csect.x_smtyp & 0x7) == XTY_SD
 		&& csect_aux.x_csect.x_smclas == XMC_TC0)
 	      {
-		if (toc_offset)
+		if (toc_offset != 0)
 		  warning (_("More than one XMC_TC0 symbol found."));
+
 		toc_offset = symbol.n_value;
 
 		/* Make TOC offset relative to start address of section.  */
 		asection *bfd_sect
 		  = xcoff_secnum_to_section (symbol.n_scnum, objfile);
-		if (bfd_sect)
+		if (bfd_sect != nullptr)
 		  toc_offset -= bfd_section_vma (bfd_sect);
 		break;
 	      }
 	  }
 	  break;
+
 	default:
-	  {
-	    complaint (_("Storage class %d not recognized during scan"),
-		       sclass);
-	  }
+	  complaint (_("Storage class %d not recognized during scan"),
+		     symbol.n_sclass);
 	  [[fallthrough]];
 
 	case C_RSYM:
@@ -235,39 +225,29 @@ scan_xcoff_symtab (struct objfile *objfile)
 /* Return the toc offset value for a given objfile.  */
 
 CORE_ADDR
-xcoff_get_toc_offset (struct objfile *objfile)
+xcoff_get_toc_offset (objfile *objfile)
 {
-  if (objfile)
+  if (objfile != nullptr)
     return xcoff_objfile_data_key.get (objfile)->toc_offset;
+
   return 0;
 }
 
-/* Scan and build partial symbols for a symbol file.
-   We have been initialized by a call to dbx_symfile_init, which
-   put all the relevant info into a "struct dbx_symfile_info",
-   hung off the objfile structure.
-
-   SECTION_OFFSETS contains offsets relative to which the symbols in the
-   various sections are (depending where the sections were actually
-   loaded).  */
+/* Read the XCOFF symbol table.  The only thing we are interested in is the TOC
+   offset value.  */
 
 static void
-xcoff_initial_scan (struct objfile *objfile, symfile_add_flags symfile_flags)
+xcoff_symfile_read (objfile *objfile, symfile_add_flags symfile_flags)
 {
-  /* We need to do this to get the TOC information only.  STABS
-     format is no longer supported.  */
-  scan_xcoff_symtab (objfile);
+  xcoff_find_toc_offset (objfile);
 
   /* DWARF2 sections.  */
   dwarf2_initialize_objfile (objfile, &dwarf2_xcoff_names);
 }
-\f
-static void
-xcoff_symfile_offsets (struct objfile *objfile,
-		       const section_addr_info &addrs)
-{
-  const char *first_section_name;
 
+static void
+xcoff_symfile_offsets (objfile *objfile, const section_addr_info &addrs)
+{
   default_symfile_offsets (objfile, addrs);
 
   /* Oneof the weird side-effects of default_symfile_offsets is that
@@ -280,7 +260,7 @@ xcoff_symfile_offsets (struct objfile *objfile,
   if (objfile->section_offsets.empty ())
     return; /* Is that even possible?  Better safe than sorry.  */
 
-  first_section_name
+  const char *first_section_name
     = bfd_section_name (objfile->sections_start[0].the_bfd_section);
 
   if (objfile->sect_index_text == 0
@@ -302,20 +282,10 @@ xcoff_symfile_offsets (struct objfile *objfile,
 
 /* Register our ability to parse symbols for xcoff BFD files.  */
 
-static const struct sym_fns xcoff_sym_fns =
+static const sym_fns xcoff_sym_fns =
 {
-
-  /* It is possible that coff and xcoff should be merged as
-     they do have fundamental similarities (for example, the extra storage
-     classes used for stabs could presumably be recognized in any COFF file).
-     However, in addition to obvious things like all the csect hair, there are
-     some subtler differences between xcoffread.c and coffread.c, notably
-     the fact that coffread.c has no need to read in all the symbols, but
-     xcoffread.c reads all the symbols and does in fact randomly access them
-     (in C_BSTAT and line number processing).  */
-
   xcoff_symfile_init,		/* read initial info, setup for sym_read() */
-  xcoff_initial_scan,		/* read a symbol file into symtab */
+  xcoff_symfile_read,		/* read a symbol file into symtab */
   xcoff_symfile_offsets,	/* xlate offsets ext->int form */
   default_symfile_segments,	/* Get segment information from a file.  */
   default_symfile_relocate,	/* Relocate a debug section.  */
@@ -328,24 +298,22 @@ static int
 xcoff_get_core_n_import_files (bfd *abfd)
 {
   asection *sect = bfd_get_section_by_name (abfd, ".ldinfo");
-  gdb_byte buf[4];
-  file_ptr offset = 0;
-  int n_entries = 0;
-
   if (sect == NULL)
     return -1;  /* Not a core file.  */
 
-  for (offset = 0; offset < bfd_section_size (sect);)
+  int n_entries = 0;
+  for (file_ptr offset = 0; offset < bfd_section_size (sect);)
     {
-      int next;
-
       n_entries++;
 
+      gdb_byte buf[4];
       if (!bfd_get_section_contents (abfd, sect, buf, offset, 4))
 	return -1;
-      next = bfd_get_32 (abfd, buf);
+
+      int next = bfd_get_32 (abfd, buf);
       if (next == 0)
 	break;  /* This is the last entry.  */
+
       offset += next;
     }
 
@@ -361,8 +329,6 @@ int
 xcoff_get_n_import_files (bfd *abfd)
 {
   asection *sect = bfd_get_section_by_name (abfd, ".loader");
-  gdb_byte buf[4];
-  int l_nimpid;
 
   /* If the ".loader" section does not exist, the objfile is probably
      not an executable.  Might be a core file...  */
@@ -372,10 +338,11 @@ xcoff_get_n_import_files (bfd *abfd)
   /* The number of entries in the Import Files Table is stored in
      field l_nimpid.  This field is always at offset 16, and is
      always 4 bytes long.  Read those 4 bytes.  */
-
+  gdb_byte buf[4];
   if (!bfd_get_section_contents (abfd, sect, buf, 16, 4))
     return -1;
-  l_nimpid = bfd_get_32 (abfd, buf);
+
+  int l_nimpid = bfd_get_32 (abfd, buf);
 
   /* By convention, the first entry is the default LIBPATH value
      to be used by the system loader, so it does not count towards
-- 
2.52.0

Re: [PATCH 8/8] gdb/xcoffread: stylistic cleanup

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:

Simon> Do a few cleanups:
Simon>  - declaring variables where they are used
Simon>  - rename functions to be more precise
Simon>  - add explicit comparison against nullptr or 0
Simon>  - remove struct/union keywords
Simon>  - remove or update outdated comments

Looks good.  Thanks for all the cleanups.
Approved-By: Tom Tromey <tom@tromey.com>

Tom