[1/5] readelf: Consolidate get_[32|64]bit_section_headers

  Consolidate get_32bit_section_headers and get_64bit_section_headers into
get_section_headers.  Use BYTE_GET_SIZE to retrieve external ELF section
header fields.

	PR binutils/34356
	* elfcomm.h (BYTE_GET_SIZE): New.
	* readelf.c (get_32bit_section_headers): Moved to ...
	(get_section_headers): This.  Use BYTE_GET_SIZE to retrieve
	external ELF section header fields.
	(get_64bit_section_headers): Removed.

Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
---
 binutils/elfcomm.h |   6 ++
 binutils/readelf.c | 146 ++++++++++++---------------------------------
 2 files changed, 43 insertions(+), 109 deletions(-)
  

On 09.07.2026 14:40, H.J. Lu wrote:
> Consolidate get_32bit_section_headers and get_64bit_section_headers into
> get_section_headers.  Use BYTE_GET_SIZE to retrieve external ELF section
> header fields.
> 
> 	PR binutils/34356
> 	* elfcomm.h (BYTE_GET_SIZE): New.
> 	* readelf.c (get_32bit_section_headers): Moved to ...
> 	(get_section_headers): This.  Use BYTE_GET_SIZE to retrieve
> 	external ELF section header fields.
> 	(get_64bit_section_headers): Removed.
> 
> Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
> ---
>  binutils/elfcomm.h |   6 ++
>  binutils/readelf.c | 146 ++++++++++++---------------------------------
>  2 files changed, 43 insertions(+), 109 deletions(-)

While this of course is a nice reduction in code size, ...

> --- a/binutils/elfcomm.h
> +++ b/binutils/elfcomm.h
> @@ -43,6 +43,12 @@ extern uint64_t byte_get_big_endian (const unsigned char *, unsigned int);
>  #define BYTE_GET(field)		byte_get (field, sizeof (field))
>  #define BYTE_GET_SIGNED(field)	byte_get_signed (field, sizeof (field))
>  
> +#define BYTE_GET_SIZE(var, ptr, size) \
> +  { \
> +    (var) = byte_get (ptr, (size)); \
> +    ptr += (size); \
> +  }

... I'm not quite convinced of both this and its uses further down. Here
I find it concerning that "ptr" is updated without that being visible at
all at use sites.

As a nit, I also consider parenthesization of macro arguments inconsistent
here. In the byte_get() invocation, neither argument should require extra
parentheses. All other uses of the macro parameters might better be
consistently parenthesized, even if the use of parentheses on lvalues is
somewhat debatable.

Finally for a macro put in a header please properly use do {} while (0),
such that semicolons put at the end of use sites don't end up stray, and
won't be at risk of breaking code like this:

    if ( x )
        BYTE_GET_SIZE (...);
    else
        ...;

> @@ -8031,96 +8039,28 @@ get_32bit_section_headers (Filedata * filedata, bool probe)
>    filedata->orig_section_headers = (Elf_Internal_Shdr **)
>      xcalloc2 (num, sizeof (Elf_Internal_Shdr *));
>  
> -  orig_internal = filedata->orig_section_headers;
> -  for (i = 0, internal = filedata->section_headers;
> -       i < num;
> -       i++, internal++, orig_internal++)
> -    {
> -      internal->sh_name      = BYTE_GET (shdrs[i].sh_name);
> -      internal->sh_type      = BYTE_GET (shdrs[i].sh_type);
> -      internal->sh_flags     = BYTE_GET (shdrs[i].sh_flags);
> -      internal->sh_addr      = BYTE_GET (shdrs[i].sh_addr);
> -      internal->sh_offset    = BYTE_GET (shdrs[i].sh_offset);
> -      internal->sh_size      = BYTE_GET (shdrs[i].sh_size);
> -      internal->sh_link      = BYTE_GET (shdrs[i].sh_link);
> -      internal->sh_info      = BYTE_GET (shdrs[i].sh_info);
> -      internal->sh_addralign = BYTE_GET (shdrs[i].sh_addralign);
> -      internal->sh_entsize   = BYTE_GET (shdrs[i].sh_entsize);
> -      validate_section_info (internal, orig_internal, i, filedata,
> -			     false, probe);
> -    }
> -
> -  free (shdrs);
> -  return true;
> -}
> -
> -/* Like get_32bit_section_headers, except that it fetches 64-bit headers.  */
> -
> -static bool
> -get_64bit_section_headers (Filedata * filedata, bool probe)
> -{
> -  Elf64_External_Shdr *  shdrs;
> -  Elf_Internal_Shdr *    internal;
> -  Elf_Internal_Shdr **   orig_internal;
> -  unsigned int           i;
> -  unsigned int           size = filedata->file_header.e_shentsize;
> -  unsigned int           num = probe ? 1 : filedata->file_header.e_shnum;
> -
> -  /* PR binutils/17531: Cope with unexpected section header sizes.  */
> -  if (size == 0 || num == 0)
> -    return false;
> -
> -  /* The section header cannot be at the start of the file - that is
> -     where the ELF file header is located.  A file with absolutely no
> -     sections in it will use a shoff of 0.  */
> -  if (filedata->file_header.e_shoff == 0)
> -    return false;
> -
> -  if (size < sizeof * shdrs)
> -    {
> -      if (! probe)
> -	error (_("The e_shentsize field in the ELF header is less than the size of an ELF section header\n"));
> -      return false;
> -    }
> -
> -  if (! probe && size > sizeof * shdrs)
> -    warn (_("The e_shentsize field in the ELF header is larger than the size of an ELF section header\n"));
> +  void *ptr = shdrs;
> +  uint32_t elf_class_size = is_32bit_elf ? 4 : 8;
>  
> -  shdrs = (Elf64_External_Shdr *) get_data (NULL, filedata,
> -					    filedata->file_header.e_shoff,
> -                                            size, num,
> -					    probe ? NULL : _("section headers"));
> -  if (shdrs == NULL)
> -    return false;
> -
> -  filedata->section_headers = (Elf_Internal_Shdr *)
> -    cmalloc (num, sizeof (Elf_Internal_Shdr));
> -  if (filedata->section_headers == NULL)
> -    {
> -      if (! probe)
> -	error (_("Out of memory reading %u section headers\n"), num);
> -      free (shdrs);
> -      return false;
> -    }
> -
> -  filedata->orig_section_headers = (Elf_Internal_Shdr **)
> -    xcalloc2 (num, sizeof (Elf_Internal_Shdr *));
> +  Elf_Internal_Shdr *internal;
> +  Elf_Internal_Shdr **orig_internal;
> +  unsigned int i;
>  
>    orig_internal = filedata->orig_section_headers;
>    for (i = 0, internal = filedata->section_headers;
>         i < num;
>         i++, internal++, orig_internal++)
>      {
> -      internal->sh_name      = BYTE_GET (shdrs[i].sh_name);
> -      internal->sh_type      = BYTE_GET (shdrs[i].sh_type);
> -      internal->sh_flags     = BYTE_GET (shdrs[i].sh_flags);
> -      internal->sh_addr      = BYTE_GET (shdrs[i].sh_addr);
> -      internal->sh_size      = BYTE_GET (shdrs[i].sh_size);
> -      internal->sh_entsize   = BYTE_GET (shdrs[i].sh_entsize);
> -      internal->sh_link      = BYTE_GET (shdrs[i].sh_link);
> -      internal->sh_info      = BYTE_GET (shdrs[i].sh_info);
> -      internal->sh_offset    = BYTE_GET (shdrs[i].sh_offset);
> -      internal->sh_addralign = BYTE_GET (shdrs[i].sh_addralign);
> +      BYTE_GET_SIZE (internal->sh_name, ptr, 4);
> +      BYTE_GET_SIZE (internal->sh_type, ptr, 4);
> +      BYTE_GET_SIZE (internal->sh_flags, ptr, elf_class_size);
> +      BYTE_GET_SIZE (internal->sh_addr, ptr, elf_class_size);
> +      BYTE_GET_SIZE (internal->sh_offset, ptr, elf_class_size);
> +      BYTE_GET_SIZE (internal->sh_size, ptr, elf_class_size);
> +      BYTE_GET_SIZE (internal->sh_link, ptr, 4);
> +      BYTE_GET_SIZE (internal->sh_info, ptr, 4);
> +      BYTE_GET_SIZE (internal->sh_addralign, ptr, elf_class_size);
> +      BYTE_GET_SIZE (internal->sh_entsize, ptr, elf_class_size);

Here is my main concern with this approach: This way you're open-coding
the structure layout and field types of Elf32_Shdr / Elf64_Shdr (and at
the same time the distinction between Xword, Addr, and Off is lost).
While those clearly can't change, that's still at least very close to a
no-go imo.

I'm curious what others think.

Jan
  

On Fri, Jul 10, 2026 at 9:41 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 09.07.2026 14:40, H.J. Lu wrote:
> > Consolidate get_32bit_section_headers and get_64bit_section_headers into
> > get_section_headers.  Use BYTE_GET_SIZE to retrieve external ELF section
> > header fields.
> >
> >       PR binutils/34356
> >       * elfcomm.h (BYTE_GET_SIZE): New.
> >       * readelf.c (get_32bit_section_headers): Moved to ...
> >       (get_section_headers): This.  Use BYTE_GET_SIZE to retrieve
> >       external ELF section header fields.
> >       (get_64bit_section_headers): Removed.
> >
> > Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
> > ---
> >  binutils/elfcomm.h |   6 ++
> >  binutils/readelf.c | 146 ++++++++++++---------------------------------
> >  2 files changed, 43 insertions(+), 109 deletions(-)
>
> While this of course is a nice reduction in code size, ...

Code size reduction isn't my main motivation.   I don't like
adding duplication codes to 2 different places.

> > --- a/binutils/elfcomm.h
> > +++ b/binutils/elfcomm.h
> > @@ -43,6 +43,12 @@ extern uint64_t byte_get_big_endian (const unsigned char *, unsigned int);
> >  #define BYTE_GET(field)              byte_get (field, sizeof (field))
> >  #define BYTE_GET_SIGNED(field)       byte_get_signed (field, sizeof (field))
> >
> > +#define BYTE_GET_SIZE(var, ptr, size) \
> > +  { \
> > +    (var) = byte_get (ptr, (size)); \
> > +    ptr += (size); \
> > +  }
>
> ... I'm not quite convinced of both this and its uses further down. Here
> I find it concerning that "ptr" is updated without that being visible at
> all at use sites.

I can change it to a real function with a pointer argument if it helps.

> As a nit, I also consider parenthesization of macro arguments inconsistent
> here. In the byte_get() invocation, neither argument should require extra
> parentheses. All other uses of the macro parameters might better be
> consistently parenthesized, even if the use of parentheses on lvalues is
> somewhat debatable.
>
> Finally for a macro put in a header please properly use do {} while (0),
> such that semicolons put at the end of use sites don't end up stray, and
> won't be at risk of breaking code like this:
>
>     if ( x )
>         BYTE_GET_SIZE (...);
>     else
>         ...;
>
> > @@ -8031,96 +8039,28 @@ get_32bit_section_headers (Filedata * filedata, bool probe)
> >    filedata->orig_section_headers = (Elf_Internal_Shdr **)
> >      xcalloc2 (num, sizeof (Elf_Internal_Shdr *));
> >
> > -  orig_internal = filedata->orig_section_headers;
> > -  for (i = 0, internal = filedata->section_headers;
> > -       i < num;
> > -       i++, internal++, orig_internal++)
> > -    {
> > -      internal->sh_name      = BYTE_GET (shdrs[i].sh_name);
> > -      internal->sh_type      = BYTE_GET (shdrs[i].sh_type);
> > -      internal->sh_flags     = BYTE_GET (shdrs[i].sh_flags);
> > -      internal->sh_addr      = BYTE_GET (shdrs[i].sh_addr);
> > -      internal->sh_offset    = BYTE_GET (shdrs[i].sh_offset);
> > -      internal->sh_size      = BYTE_GET (shdrs[i].sh_size);
> > -      internal->sh_link      = BYTE_GET (shdrs[i].sh_link);
> > -      internal->sh_info      = BYTE_GET (shdrs[i].sh_info);
> > -      internal->sh_addralign = BYTE_GET (shdrs[i].sh_addralign);
> > -      internal->sh_entsize   = BYTE_GET (shdrs[i].sh_entsize);
> > -      validate_section_info (internal, orig_internal, i, filedata,
> > -                          false, probe);
> > -    }
> > -
> > -  free (shdrs);
> > -  return true;
> > -}
> > -
> > -/* Like get_32bit_section_headers, except that it fetches 64-bit headers.  */
> > -
> > -static bool
> > -get_64bit_section_headers (Filedata * filedata, bool probe)
> > -{
> > -  Elf64_External_Shdr *  shdrs;
> > -  Elf_Internal_Shdr *    internal;
> > -  Elf_Internal_Shdr **   orig_internal;
> > -  unsigned int           i;
> > -  unsigned int           size = filedata->file_header.e_shentsize;
> > -  unsigned int           num = probe ? 1 : filedata->file_header.e_shnum;
> > -
> > -  /* PR binutils/17531: Cope with unexpected section header sizes.  */
> > -  if (size == 0 || num == 0)
> > -    return false;
> > -
> > -  /* The section header cannot be at the start of the file - that is
> > -     where the ELF file header is located.  A file with absolutely no
> > -     sections in it will use a shoff of 0.  */
> > -  if (filedata->file_header.e_shoff == 0)
> > -    return false;
> > -
> > -  if (size < sizeof * shdrs)
> > -    {
> > -      if (! probe)
> > -     error (_("The e_shentsize field in the ELF header is less than the size of an ELF section header\n"));
> > -      return false;
> > -    }
> > -
> > -  if (! probe && size > sizeof * shdrs)
> > -    warn (_("The e_shentsize field in the ELF header is larger than the size of an ELF section header\n"));
> > +  void *ptr = shdrs;
> > +  uint32_t elf_class_size = is_32bit_elf ? 4 : 8;
> >
> > -  shdrs = (Elf64_External_Shdr *) get_data (NULL, filedata,
> > -                                         filedata->file_header.e_shoff,
> > -                                            size, num,
> > -                                         probe ? NULL : _("section headers"));
> > -  if (shdrs == NULL)
> > -    return false;
> > -
> > -  filedata->section_headers = (Elf_Internal_Shdr *)
> > -    cmalloc (num, sizeof (Elf_Internal_Shdr));
> > -  if (filedata->section_headers == NULL)
> > -    {
> > -      if (! probe)
> > -     error (_("Out of memory reading %u section headers\n"), num);
> > -      free (shdrs);
> > -      return false;
> > -    }
> > -
> > -  filedata->orig_section_headers = (Elf_Internal_Shdr **)
> > -    xcalloc2 (num, sizeof (Elf_Internal_Shdr *));
> > +  Elf_Internal_Shdr *internal;
> > +  Elf_Internal_Shdr **orig_internal;
> > +  unsigned int i;
> >
> >    orig_internal = filedata->orig_section_headers;
> >    for (i = 0, internal = filedata->section_headers;
> >         i < num;
> >         i++, internal++, orig_internal++)
> >      {
> > -      internal->sh_name      = BYTE_GET (shdrs[i].sh_name);
> > -      internal->sh_type      = BYTE_GET (shdrs[i].sh_type);
> > -      internal->sh_flags     = BYTE_GET (shdrs[i].sh_flags);
> > -      internal->sh_addr      = BYTE_GET (shdrs[i].sh_addr);
> > -      internal->sh_size      = BYTE_GET (shdrs[i].sh_size);
> > -      internal->sh_entsize   = BYTE_GET (shdrs[i].sh_entsize);
> > -      internal->sh_link      = BYTE_GET (shdrs[i].sh_link);
> > -      internal->sh_info      = BYTE_GET (shdrs[i].sh_info);
> > -      internal->sh_offset    = BYTE_GET (shdrs[i].sh_offset);
> > -      internal->sh_addralign = BYTE_GET (shdrs[i].sh_addralign);
> > +      BYTE_GET_SIZE (internal->sh_name, ptr, 4);
> > +      BYTE_GET_SIZE (internal->sh_type, ptr, 4);
> > +      BYTE_GET_SIZE (internal->sh_flags, ptr, elf_class_size);
> > +      BYTE_GET_SIZE (internal->sh_addr, ptr, elf_class_size);
> > +      BYTE_GET_SIZE (internal->sh_offset, ptr, elf_class_size);
> > +      BYTE_GET_SIZE (internal->sh_size, ptr, elf_class_size);
> > +      BYTE_GET_SIZE (internal->sh_link, ptr, 4);
> > +      BYTE_GET_SIZE (internal->sh_info, ptr, 4);
> > +      BYTE_GET_SIZE (internal->sh_addralign, ptr, elf_class_size);
> > +      BYTE_GET_SIZE (internal->sh_entsize, ptr, elf_class_size);
>
> Here is my main concern with this approach: This way you're open-coding
> the structure layout and field types of Elf32_Shdr / Elf64_Shdr (and at
> the same time the distinction between Xword, Addr, and Off is lost).
> While those clearly can't change, that's still at least very close to a
> no-go imo.

Since it is close to no-go to you,  Alan and Nick have no opinion,
I am dropping this patch set.

> I'm curious what others think.
>
> Jan
  

On 17.07.2026 10:32, H.J. Lu wrote:
> On Fri, Jul 10, 2026 at 9:41 PM Jan Beulich <jbeulich@suse.com> wrote:
>> On 09.07.2026 14:40, H.J. Lu wrote:
>>> Consolidate get_32bit_section_headers and get_64bit_section_headers into
>>> get_section_headers.  Use BYTE_GET_SIZE to retrieve external ELF section
>>> header fields.
>>>
>>>       PR binutils/34356
>>>       * elfcomm.h (BYTE_GET_SIZE): New.
>>>       * readelf.c (get_32bit_section_headers): Moved to ...
>>>       (get_section_headers): This.  Use BYTE_GET_SIZE to retrieve
>>>       external ELF section header fields.
>>>       (get_64bit_section_headers): Removed.
>>>
>>> Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
>>> ---
>>>  binutils/elfcomm.h |   6 ++
>>>  binutils/readelf.c | 146 ++++++++++++---------------------------------
>>>  2 files changed, 43 insertions(+), 109 deletions(-)
>>
>> While this of course is a nice reduction in code size, ...
> 
> Code size reduction isn't my main motivation.   I don't like
> adding duplication codes to 2 different places.

Neither do I. How about the attached alternatives to two of your patches?

Jan
readelf: fold get_{32,64}bit_program_headers()

They're identical except for the types used and the order of fields
processed. The latter doesn't matter for correctness, and the former can
be addressed by compiling the same code twice.

--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -40,6 +40,8 @@
   ELF file than is provided by objdump.  In particular it can display DWARF
   debugging information which (at the moment) objdump cannot.  */
 
+#ifndef ElfXX
+
 #include "sysdep.h"
 #include <assert.h>
 #include <time.h>
@@ -7365,14 +7367,21 @@ process_file_header (Filedata * filedata
   return true;
 }
 
+#define ElfXX(n) Elf32 ## n
+#include "readelf.c"
+#define ElfXX(n) Elf64 ## n
+#include "readelf.c"
+
+#else /* ElfXX */
+
 /* Read in the program headers from FILEDATA and store them in PHEADERS.
-   Returns TRUE upon success, FALSE otherwise.  Loads 32-bit headers.  */
+   Returns TRUE upon success, FALSE otherwise.  */
 
 static bool
-get_32bit_program_headers (Filedata * filedata, Elf_Internal_Phdr * pheaders)
+ElfXX(_get_program_headers) (Filedata * filedata, Elf_Internal_Phdr * pheaders)
 {
-  Elf32_External_Phdr * phdrs;
-  Elf32_External_Phdr * external;
+  ElfXX(_External_Phdr) * phdrs;
+  const ElfXX(_External_Phdr) * external;
   Elf_Internal_Phdr *   internal;
   unsigned int i;
   unsigned int size = filedata->file_header.e_phentsize;
@@ -7389,8 +7398,8 @@ get_32bit_program_headers (Filedata * fi
   if (size > sizeof * phdrs)
     warn (_("The e_phentsize field in the ELF header is larger than the size of an ELF program header\n"));
 
-  phdrs = (Elf32_External_Phdr *) get_data (NULL, filedata, filedata->file_header.e_phoff,
-                                            size, num, _("program headers"));
+  phdrs = get_data (NULL, filedata, filedata->file_header.e_phoff, size, num,
+		    _("program headers"));
   if (phdrs == NULL)
     return false;
 
@@ -7412,52 +7421,9 @@ get_32bit_program_headers (Filedata * fi
   return true;
 }
 
-/* Read in the program headers from FILEDATA and store them in PHEADERS.
-   Returns TRUE upon success, FALSE otherwise.  Loads 64-bit headers.  */
-
-static bool
-get_64bit_program_headers (Filedata * filedata, Elf_Internal_Phdr * pheaders)
-{
-  Elf64_External_Phdr * phdrs;
-  Elf64_External_Phdr * external;
-  Elf_Internal_Phdr *   internal;
-  unsigned int i;
-  unsigned int size = filedata->file_header.e_phentsize;
-  unsigned int num  = filedata->file_header.e_phnum;
-
-  /* PR binutils/17531: Cope with unexpected section header sizes.  */
-  if (size == 0 || num == 0)
-    return false;
-  if (size < sizeof * phdrs)
-    {
-      error (_("The e_phentsize field in the ELF header is less than the size of an ELF program header\n"));
-      return false;
-    }
-  if (size > sizeof * phdrs)
-    warn (_("The e_phentsize field in the ELF header is larger than the size of an ELF program header\n"));
-
-  phdrs = (Elf64_External_Phdr *) get_data (NULL, filedata, filedata->file_header.e_phoff,
-                                            size, num, _("program headers"));
-  if (!phdrs)
-    return false;
-
-  for (i = 0, internal = pheaders, external = phdrs;
-       i < filedata->file_header.e_phnum;
-       i++, internal++, external++)
-    {
-      internal->p_type   = BYTE_GET (external->p_type);
-      internal->p_flags  = BYTE_GET (external->p_flags);
-      internal->p_offset = BYTE_GET (external->p_offset);
-      internal->p_vaddr  = BYTE_GET (external->p_vaddr);
-      internal->p_paddr  = BYTE_GET (external->p_paddr);
-      internal->p_filesz = BYTE_GET (external->p_filesz);
-      internal->p_memsz  = BYTE_GET (external->p_memsz);
-      internal->p_align  = BYTE_GET (external->p_align);
-    }
+#endif /* ElfXX */
 
-  free (phdrs);
-  return true;
-}
+#ifndef ElfXX
 
 /* Returns TRUE if the program headers were read into `program_headers'.  */
 
@@ -7491,8 +7457,8 @@ get_program_headers (Filedata * filedata
     }
 
   if (is_32bit_elf
-      ? get_32bit_program_headers (filedata, phdrs)
-      : get_64bit_program_headers (filedata, phdrs))
+      ? Elf32_get_program_headers (filedata, phdrs)
+      : Elf64_get_program_headers (filedata, phdrs))
     {
       filedata->program_headers = phdrs;
       return true;
@@ -25478,3 +25444,7 @@ main (int argc, char ** argv)
 
   return err ? EXIT_FAILURE : EXIT_SUCCESS;
 }
+
+#endif /* ElfXX */
+
+#undef ElfXX
readelf: fold get_{32,64}bit_section_headers()

They're identical except for the types used and the order of fields
processed. The latter doesn't matter for correctness, and the former can
be addressed by compiling the same code twice.

--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -7814,15 +7814,16 @@ offset_from_vma (Filedata * filedata, ui
   return vma;
 }
 
+#else /* ElfXX */
 
 /* Allocate memory and load the sections headers into FILEDATA->filedata->section_headers.
    If PROBE is true, this is just a probe and we do not generate any error
    messages if the load fails.  */
 
 static bool
-get_32bit_section_headers (Filedata * filedata, bool probe)
+ElfXX(_get_section_headers) (Filedata * filedata, bool probe)
 {
-  Elf32_External_Shdr * shdrs;
+  ElfXX(_External_Shdr) * shdrs;
   Elf_Internal_Shdr *   internal;
   unsigned int          i;
   unsigned int          size = filedata->file_header.e_shentsize;
@@ -7847,9 +7848,8 @@ get_32bit_section_headers (Filedata * fi
   if (!probe && size > sizeof * shdrs)
     warn (_("The e_shentsize field in the ELF header is larger than the size of an ELF section header\n"));
 
-  shdrs = (Elf32_External_Shdr *) get_data (NULL, filedata, filedata->file_header.e_shoff,
-                                            size, num,
-					    probe ? NULL : _("section headers"));
+  shdrs = get_data (NULL, filedata, filedata->file_header.e_shoff, size, num,
+				    probe ? NULL : _("section headers"));
   if (shdrs == NULL)
     return false;
 
@@ -7890,80 +7890,9 @@ get_32bit_section_headers (Filedata * fi
   return true;
 }
 
-/* Like get_32bit_section_headers, except that it fetches 64-bit headers.  */
+#endif /* ElfXX */
 
-static bool
-get_64bit_section_headers (Filedata * filedata, bool probe)
-{
-  Elf64_External_Shdr *  shdrs;
-  Elf_Internal_Shdr *    internal;
-  unsigned int           i;
-  unsigned int           size = filedata->file_header.e_shentsize;
-  unsigned int           num = probe ? 1 : filedata->file_header.e_shnum;
-
-  /* PR binutils/17531: Cope with unexpected section header sizes.  */
-  if (size == 0 || num == 0)
-    return false;
-
-  /* The section header cannot be at the start of the file - that is
-     where the ELF file header is located.  A file with absolutely no
-     sections in it will use a shoff of 0.  */
-  if (filedata->file_header.e_shoff == 0)
-    return false;
-
-  if (size < sizeof * shdrs)
-    {
-      if (! probe)
-	error (_("The e_shentsize field in the ELF header is less than the size of an ELF section header\n"));
-      return false;
-    }
-
-  if (! probe && size > sizeof * shdrs)
-    warn (_("The e_shentsize field in the ELF header is larger than the size of an ELF section header\n"));
-
-  shdrs = (Elf64_External_Shdr *) get_data (NULL, filedata,
-					    filedata->file_header.e_shoff,
-                                            size, num,
-					    probe ? NULL : _("section headers"));
-  if (shdrs == NULL)
-    return false;
-
-  filedata->section_headers = (Elf_Internal_Shdr *)
-    cmalloc (num, sizeof (Elf_Internal_Shdr));
-  if (filedata->section_headers == NULL)
-    {
-      if (! probe)
-	error (_("Out of memory reading %u section headers\n"), num);
-      free (shdrs);
-      return false;
-    }
-
-  for (i = 0, internal = filedata->section_headers;
-       i < num;
-       i++, internal++)
-    {
-      internal->sh_name      = BYTE_GET (shdrs[i].sh_name);
-      internal->sh_type      = BYTE_GET (shdrs[i].sh_type);
-      internal->sh_flags     = BYTE_GET (shdrs[i].sh_flags);
-      internal->sh_addr      = BYTE_GET (shdrs[i].sh_addr);
-      internal->sh_size      = BYTE_GET (shdrs[i].sh_size);
-      internal->sh_entsize   = BYTE_GET (shdrs[i].sh_entsize);
-      internal->sh_link      = BYTE_GET (shdrs[i].sh_link);
-      internal->sh_info      = BYTE_GET (shdrs[i].sh_info);
-      internal->sh_offset    = BYTE_GET (shdrs[i].sh_offset);
-      internal->sh_addralign = BYTE_GET (shdrs[i].sh_addralign);
-      if (!probe
-	  && internal->sh_link >= num
-	  && !special_defined_section_index (filedata,
-					     internal->sh_link))
-	warn (_("Section %u has an out of range sh_link value of %u\n"), i, internal->sh_link);
-      if (!probe && internal->sh_flags & SHF_INFO_LINK && internal->sh_info > num)
-	warn (_("Section %u has an out of range sh_info value of %u\n"), i, internal->sh_info);
-    }
-
-  free (shdrs);
-  return true;
-}
+#ifndef ElfXX
 
 static bool
 get_section_headers (Filedata *filedata, bool probe)
@@ -7972,9 +7901,9 @@ get_section_headers (Filedata *filedata,
     return true;
 
   if (is_32bit_elf)
-    return get_32bit_section_headers (filedata, probe);
+    return Elf32_get_section_headers (filedata, probe);
   else
-    return get_64bit_section_headers (filedata, probe);
+    return Elf64_get_section_headers (filedata, probe);
 }
 
 static Elf_Internal_Sym *
  

On Fri, Jul 17, 2026 at 5:16 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 17.07.2026 10:32, H.J. Lu wrote:
> > On Fri, Jul 10, 2026 at 9:41 PM Jan Beulich <jbeulich@suse.com> wrote:
> >> On 09.07.2026 14:40, H.J. Lu wrote:
> >>> Consolidate get_32bit_section_headers and get_64bit_section_headers into
> >>> get_section_headers.  Use BYTE_GET_SIZE to retrieve external ELF section
> >>> header fields.
> >>>
> >>>       PR binutils/34356
> >>>       * elfcomm.h (BYTE_GET_SIZE): New.
> >>>       * readelf.c (get_32bit_section_headers): Moved to ...
> >>>       (get_section_headers): This.  Use BYTE_GET_SIZE to retrieve
> >>>       external ELF section header fields.
> >>>       (get_64bit_section_headers): Removed.
> >>>
> >>> Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
> >>> ---
> >>>  binutils/elfcomm.h |   6 ++
> >>>  binutils/readelf.c | 146 ++++++++++++---------------------------------
> >>>  2 files changed, 43 insertions(+), 109 deletions(-)
> >>
> >> While this of course is a nice reduction in code size, ...
> >
> > Code size reduction isn't my main motivation.   I don't like
> > adding duplication codes to 2 different places.
>
> Neither do I. How about the attached alternatives to two of your patches?
>

These are similar to bfd/elfcode.h.  They should work.
  

On Fri, Jul 17, 2026 at 5:27 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Fri, Jul 17, 2026 at 5:16 PM Jan Beulich <jbeulich@suse.com> wrote:
> >
> > On 17.07.2026 10:32, H.J. Lu wrote:
> > > On Fri, Jul 10, 2026 at 9:41 PM Jan Beulich <jbeulich@suse.com> wrote:
> > >> On 09.07.2026 14:40, H.J. Lu wrote:
> > >>> Consolidate get_32bit_section_headers and get_64bit_section_headers into
> > >>> get_section_headers.  Use BYTE_GET_SIZE to retrieve external ELF section
> > >>> header fields.
> > >>>
> > >>>       PR binutils/34356
> > >>>       * elfcomm.h (BYTE_GET_SIZE): New.
> > >>>       * readelf.c (get_32bit_section_headers): Moved to ...
> > >>>       (get_section_headers): This.  Use BYTE_GET_SIZE to retrieve
> > >>>       external ELF section header fields.
> > >>>       (get_64bit_section_headers): Removed.
> > >>>
> > >>> Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
> > >>> ---
> > >>>  binutils/elfcomm.h |   6 ++
> > >>>  binutils/readelf.c | 146 ++++++++++++---------------------------------
> > >>>  2 files changed, 43 insertions(+), 109 deletions(-)
> > >>
> > >> While this of course is a nice reduction in code size, ...
> > >
> > > Code size reduction isn't my main motivation.   I don't like
> > > adding duplication codes to 2 different places.
> >
> > Neither do I. How about the attached alternatives to two of your patches?
> >
>
> These are similar to bfd/elfcode.h.  They should work.
>
>

elfcode.h kind of thing should cover all duplicated codes.
  

On 17.07.2026 11:28, H.J. Lu wrote:
> On Fri, Jul 17, 2026 at 5:27 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>>
>> On Fri, Jul 17, 2026 at 5:16 PM Jan Beulich <jbeulich@suse.com> wrote:
>>>
>>> On 17.07.2026 10:32, H.J. Lu wrote:
>>>> On Fri, Jul 10, 2026 at 9:41 PM Jan Beulich <jbeulich@suse.com> wrote:
>>>>> On 09.07.2026 14:40, H.J. Lu wrote:
>>>>>> Consolidate get_32bit_section_headers and get_64bit_section_headers into
>>>>>> get_section_headers.  Use BYTE_GET_SIZE to retrieve external ELF section
>>>>>> header fields.
>>>>>>
>>>>>>       PR binutils/34356
>>>>>>       * elfcomm.h (BYTE_GET_SIZE): New.
>>>>>>       * readelf.c (get_32bit_section_headers): Moved to ...
>>>>>>       (get_section_headers): This.  Use BYTE_GET_SIZE to retrieve
>>>>>>       external ELF section header fields.
>>>>>>       (get_64bit_section_headers): Removed.
>>>>>>
>>>>>> Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
>>>>>> ---
>>>>>>  binutils/elfcomm.h |   6 ++
>>>>>>  binutils/readelf.c | 146 ++++++++++++---------------------------------
>>>>>>  2 files changed, 43 insertions(+), 109 deletions(-)
>>>>>
>>>>> While this of course is a nice reduction in code size, ...
>>>>
>>>> Code size reduction isn't my main motivation.   I don't like
>>>> adding duplication codes to 2 different places.
>>>
>>> Neither do I. How about the attached alternatives to two of your patches?
>>>
>>
>> These are similar to bfd/elfcode.h.  They should work.
> 
> elfcode.h kind of thing should cover all duplicated codes.

Except that readelf carefully avoids to become dependent on libbfd.

Jan
  

On Fri, Jul 17, 2026 at 6:57 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 17.07.2026 11:28, H.J. Lu wrote:
> > On Fri, Jul 17, 2026 at 5:27 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >>
> >> On Fri, Jul 17, 2026 at 5:16 PM Jan Beulich <jbeulich@suse.com> wrote:
> >>>
> >>> On 17.07.2026 10:32, H.J. Lu wrote:
> >>>> On Fri, Jul 10, 2026 at 9:41 PM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>> On 09.07.2026 14:40, H.J. Lu wrote:
> >>>>>> Consolidate get_32bit_section_headers and get_64bit_section_headers into
> >>>>>> get_section_headers.  Use BYTE_GET_SIZE to retrieve external ELF section
> >>>>>> header fields.
> >>>>>>
> >>>>>>       PR binutils/34356
> >>>>>>       * elfcomm.h (BYTE_GET_SIZE): New.
> >>>>>>       * readelf.c (get_32bit_section_headers): Moved to ...
> >>>>>>       (get_section_headers): This.  Use BYTE_GET_SIZE to retrieve
> >>>>>>       external ELF section header fields.
> >>>>>>       (get_64bit_section_headers): Removed.
> >>>>>>
> >>>>>> Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
> >>>>>> ---
> >>>>>>  binutils/elfcomm.h |   6 ++
> >>>>>>  binutils/readelf.c | 146 ++++++++++++---------------------------------
> >>>>>>  2 files changed, 43 insertions(+), 109 deletions(-)
> >>>>>
> >>>>> While this of course is a nice reduction in code size, ...
> >>>>
> >>>> Code size reduction isn't my main motivation.   I don't like
> >>>> adding duplication codes to 2 different places.
> >>>
> >>> Neither do I. How about the attached alternatives to two of your patches?
> >>>
> >>
> >> These are similar to bfd/elfcode.h.  They should work.
> >
> > elfcode.h kind of thing should cover all duplicated codes.
>
> Except that readelf carefully avoids to become dependent on libbfd.
>

Similar approach, not use it directly.