Re: [PATCH 1/5] readelf: fold get_{32,64}bit_program_headers()

On Mon, Jul 27, 2026 at 5:20 PM Jan Beulich <[email protected]> wrote:
>
> PR binutils/34356
>
> 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>
> @@ -7366,14 +7368,21 @@ process_file_header (Filedata * filedata
>    return true;
>  }
>
> +#define ElfXX(n) Elf32 ## n
> +#include "readelf.c"
> +#define ElfXX(n) Elf64 ## n
> +#include "readelf.c"

Can we extract this piece code into readelf.h and include
readelf.h instead?

> +#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;
> @@ -7390,8 +7399,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;
>
> @@ -7413,52 +7422,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'.  */
>
> @@ -7492,8 +7458,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;
> @@ -25618,3 +25584,7 @@ main (int argc, char ** argv)
>
>    return err ? EXIT_FAILURE : EXIT_SUCCESS;
>  }
> +
> +#endif /* ElfXX */
> +
> +#undef ElfXX
>


-- 
H.J.