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

  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.
  

On Mon, Jul 27, 2026 at 5:20 PM Jan Beulich <jbeulich@suse.com> 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
>
  

On 27.07.2026 13:20, H.J. Lu wrote:
> On Mon, Jul 27, 2026 at 5:20 PM Jan Beulich <jbeulich@suse.com> 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?

I'm sure we (technically) could, but I'm unconvinced we should. Personally
I don't consider such a valid use of "header files". I did consider adding
a new .c file, but I couldn't think of a name which I would be at least
reasonably okay with.

Jan
  

On Jul 27 2026, Jan Beulich wrote:

> I did consider adding
> a new .c file, but I couldn't think of a name which I would be at least
> reasonably okay with.

Perhaps following the pattern in bfd: readelf-nn.c
  

On 27.07.2026 13:56, Andreas Schwab wrote:
> On Jul 27 2026, Jan Beulich wrote:
> 
>> I did consider adding
>> a new .c file, but I couldn't think of a name which I would be at least
>> reasonably okay with.
> 
> Perhaps following the pattern in bfd: readelf-nn.c

And then also generate readelf-32.c and readelf-64.c from it? I'd like to
avoid introducing new generated files, whenever possible.

Jan
  

On Mon, Jul 27, 2026 at 8:08 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 27.07.2026 13:56, Andreas Schwab wrote:
> > On Jul 27 2026, Jan Beulich wrote:
> >
> >> I did consider adding
> >> a new .c file, but I couldn't think of a name which I would be at least
> >> reasonably okay with.
> >
> > Perhaps following the pattern in bfd: readelf-nn.c
>
> And then also generate readelf-32.c and readelf-64.c from it? I'd like to
> avoid introducing new generated files, whenever possible.
>

You don't need to generate readelf-32.c and readelf-64.c.
You can just include readelf-nn.c twice.
  

On 27.07.2026 14:12, H.J. Lu wrote:
> On Mon, Jul 27, 2026 at 8:08 PM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 27.07.2026 13:56, Andreas Schwab wrote:
>>> On Jul 27 2026, Jan Beulich wrote:
>>>
>>>> I did consider adding
>>>> a new .c file, but I couldn't think of a name which I would be at least
>>>> reasonably okay with.
>>>
>>> Perhaps following the pattern in bfd: readelf-nn.c
>>
>> And then also generate readelf-32.c and readelf-64.c from it? I'd like to
>> avoid introducing new generated files, whenever possible.
> 
> You don't need to generate readelf-32.c and readelf-64.c.
> You can just include readelf-nn.c twice.

Of course I can; the question was solely towards Andreas making an analogy
with bfd/. Similar naming decisions may better (but don't need to) be used
together with similar coding approaches.

Jan