[PATCH 1/5] readelf: Consolidate get_[32|64]bit_section_headers
Jan Beulich
jbeulich@suse.com
Fri Jul 17 09:16:15 GMT 2026
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
-------------- next part --------------
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
-------------- next part --------------
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 *
More information about the Binutils
mailing list