* [PATCH 1/2] elf-from-memory: Restructure code to get rid of nested handle_segment()
2021-01-08 8:09 Remove nested functions from elf-from-memory.c tbaeder
@ 2021-01-08 8:09 ` tbaeder
2021-01-28 21:08 ` Mark Wielaard
2021-01-08 8:09 ` [PATCH 2/2] elf-from-memory: Refactor to get rid of nested function tbaeder
1 sibling, 1 reply; 5+ messages in thread
From: tbaeder @ 2021-01-08 8:09 UTC (permalink / raw)
To: elfutils-devel
From: Timm Bäder <tbaeder@redhat.com>
Use one loop for both 32 and 64 bit case. This allows for only one call
site of the old handle_segment(), which we can then inline into the for
loop.
Signed-off-by: Timm Bäder <tbaeder@redhat.com>
---
libdwfl/elf-from-memory.c | 81 ++++++++++++++++-----------------------
1 file changed, 34 insertions(+), 47 deletions(-)
diff --git a/libdwfl/elf-from-memory.c b/libdwfl/elf-from-memory.c
index c54c1b99..933ab864 100644
--- a/libdwfl/elf-from-memory.c
+++ b/libdwfl/elf-from-memory.c
@@ -223,61 +223,48 @@ elf_from_remote_memory (GElf_Addr ehdr_vma,
bool found_base = false;
Elf32_Phdr (*p32)[phnum] = phdrsp;
Elf64_Phdr (*p64)[phnum] = phdrsp;
- switch (ehdr.e32.e_ident[EI_CLASS])
+
+ if (class32)
{
- /* Sanity checks segments and calculates segment_end,
- segments_end, segments_end_mem and loadbase (if not
- found_base yet). Returns true if sanity checking failed,
- false otherwise. */
- inline bool handle_segment (GElf_Addr vaddr, GElf_Off offset,
- GElf_Xword filesz, GElf_Xword memsz)
- {
- /* Sanity check the segment load aligns with the pagesize. */
- if (((vaddr - offset) & (pagesize - 1)) != 0)
- return true;
+ if (! elf32_xlatetom (&xlateto, &xlatefrom, ehdr.e32.e_ident[EI_DATA]))
+ goto libelf_error;
+ }
+ else
+ {
+ if (! elf64_xlatetom (&xlateto, &xlatefrom, ehdr.e64.e_ident[EI_DATA]))
+ goto libelf_error;
+ }
- GElf_Off segment_end = ((offset + filesz + pagesize - 1)
- & -pagesize);
+ for (uint_fast16_t i = 0; i < phnum; ++i)
+ {
+ GElf_Word type = class32 ? (*p32)[i].p_type : (*p64)[i].p_type;
- if (segment_end > (GElf_Off) contents_size)
- contents_size = segment_end;
+ if (type != PT_LOAD)
+ continue;
- if (!found_base && (offset & -pagesize) == 0)
- {
- loadbase = ehdr_vma - (vaddr & -pagesize);
- found_base = true;
- }
+ GElf_Addr vaddr = class32 ? (*p32)[i].p_vaddr : (*p64)[i].p_vaddr;
+ GElf_Xword memsz = class32 ? (*p32)[i].p_memsz : (*p64)[i].p_memsz;
+ GElf_Off offset = class32 ? (*p32)[i].p_offset : (*p64)[i].p_offset;
+ GElf_Xword filesz = class32 ? (*p32)[i].p_filesz : (*p64)[i].p_filesz;
- segments_end = offset + filesz;
- segments_end_mem = offset + memsz;
- return false;
- }
+ /* Sanity check the segment load aligns with the pagesize. */
+ if (((vaddr - offset) & (pagesize - 1)) != 0)
+ goto bad_elf;
- case ELFCLASS32:
- if (elf32_xlatetom (&xlateto, &xlatefrom,
- ehdr.e32.e_ident[EI_DATA]) == NULL)
- goto libelf_error;
- for (uint_fast16_t i = 0; i < phnum; ++i)
- if ((*p32)[i].p_type == PT_LOAD)
- if (handle_segment ((*p32)[i].p_vaddr, (*p32)[i].p_offset,
- (*p32)[i].p_filesz, (*p32)[i].p_memsz))
- goto bad_elf;
- break;
+ GElf_Off segment_end = ((offset + filesz + pagesize - 1)
+ & -pagesize);
- case ELFCLASS64:
- if (elf64_xlatetom (&xlateto, &xlatefrom,
- ehdr.e64.e_ident[EI_DATA]) == NULL)
- goto libelf_error;
- for (uint_fast16_t i = 0; i < phnum; ++i)
- if ((*p64)[i].p_type == PT_LOAD)
- if (handle_segment ((*p64)[i].p_vaddr, (*p64)[i].p_offset,
- (*p64)[i].p_filesz, (*p64)[i].p_memsz))
- goto bad_elf;
- break;
+ if (segment_end > (GElf_Off) contents_size)
+ contents_size = segment_end;
- default:
- abort ();
- break;
+ if (!found_base && (offset & -pagesize) == 0)
+ {
+ loadbase = ehdr_vma - (vaddr & -pagesize);
+ found_base = true;
+ }
+
+ segments_end = offset + filesz;
+ segments_end_mem = offset + memsz;
}
/* Trim the last segment so we don't bother with zeros in the last page
--
2.26.2
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 2/2] elf-from-memory: Refactor to get rid of nested function
2021-01-08 8:09 Remove nested functions from elf-from-memory.c tbaeder
2021-01-08 8:09 ` [PATCH 1/2] elf-from-memory: Restructure code to get rid of nested handle_segment() tbaeder
@ 2021-01-08 8:09 ` tbaeder
2021-01-28 22:58 ` Mark Wielaard
1 sibling, 1 reply; 5+ messages in thread
From: tbaeder @ 2021-01-08 8:09 UTC (permalink / raw)
To: elfutils-devel
From: Timm Bäder <tbaeder@redhat.com>
Try to unify the 32/64 bit code paths and get rid of the nested
handle_segment() this way.
Signed-off-by: Timm Bäder <tbaeder@redhat.com>
---
libdwfl/elf-from-memory.c | 115 +++++++++++++++++---------------------
1 file changed, 50 insertions(+), 65 deletions(-)
diff --git a/libdwfl/elf-from-memory.c b/libdwfl/elf-from-memory.c
index 933ab864..a0ef0014 100644
--- a/libdwfl/elf-from-memory.c
+++ b/libdwfl/elf-from-memory.c
@@ -292,80 +292,65 @@ elf_from_remote_memory (GElf_Addr ehdr_vma,
goto no_memory;
}
- switch (ehdr.e32.e_ident[EI_CLASS])
+ for (uint_fast16_t i = 0; i < phnum; ++i)
{
- /* Reads the given segment. Returns true if reading fails,
- false otherwise. */
- inline bool handle_segment (GElf_Addr vaddr, GElf_Off offset,
- GElf_Xword filesz)
- {
- GElf_Off start = offset & -pagesize;
- GElf_Off end = (offset + filesz + pagesize - 1) & -pagesize;
- if (end > (GElf_Off) contents_size)
- end = contents_size;
- nread = (*read_memory) (arg, buffer + start,
- (loadbase + vaddr) & -pagesize,
- end - start, end - start);
- return nread <= 0;
- }
+ GElf_Word type = class32 ? (*p32)[i].p_type : (*p64)[i].p_type;
- case ELFCLASS32:
- for (uint_fast16_t i = 0; i < phnum; ++i)
- if ((*p32)[i].p_type == PT_LOAD)
- if (handle_segment ((*p32)[i].p_vaddr, (*p32)[i].p_offset,
- (*p32)[i].p_filesz))
- goto read_error;
-
- /* If the segments visible in memory didn't include the section
- headers, then clear them from the file header. */
- if (contents_size < shdrs_end)
- {
- ehdr.e32.e_shoff = 0;
- ehdr.e32.e_shnum = 0;
- ehdr.e32.e_shstrndx = 0;
- }
+ if (type != PT_LOAD)
+ continue;
+
+ GElf_Addr vaddr = class32 ? (*p32)[i].p_vaddr : (*p64)[i].p_vaddr;
+ GElf_Off offset = class32 ? (*p32)[i].p_offset : (*p64)[i].p_offset;
+ GElf_Xword filesz = class32 ? (*p32)[i].p_filesz : (*p64)[i].p_filesz;
+
+ GElf_Off start = offset & -pagesize;
+ GElf_Off end = (offset + filesz + pagesize - 1) & -pagesize;
+ if (end > (GElf_Off) contents_size)
+ end = contents_size;
+ nread = (*read_memory) (arg, buffer + start,
+ (loadbase + vaddr) & -pagesize,
+ end - start, end - start);
+ if (nread <= 0)
+ goto read_error;
+ }
+
+ /* If the segments visible in memory didn't include the section
+ headers, then clear them from the file header. */
+ if (contents_size < shdrs_end)
+ {
+ if (class32)
+ {
+ ehdr.e32.e_shoff = 0;
+ ehdr.e32.e_shnum = 0;
+ ehdr.e32.e_shstrndx = 0;
+ }
+ else
+ {
+ ehdr.e64.e_shoff = 0;
+ ehdr.e64.e_shnum = 0;
+ ehdr.e64.e_shstrndx = 0;
+ }
+ }
- /* This will normally have been in the first PT_LOAD segment. But it
- conceivably could be missing, and we might have just changed it. */
- xlatefrom.d_type = xlateto.d_type = ELF_T_EHDR;
+ /* This will normally have been in the first PT_LOAD segment. But it
+ conceivably could be missing, and we might have just changed it. */
+ xlatefrom.d_type = xlateto.d_type = ELF_T_EHDR;
+ xlateto.d_buf = buffer;
+ if (class32)
+ {
xlatefrom.d_size = xlateto.d_size = sizeof ehdr.e32;
xlatefrom.d_buf = &ehdr.e32;
- xlateto.d_buf = buffer;
if (elf32_xlatetof (&xlateto, &xlatefrom,
- ehdr.e32.e_ident[EI_DATA]) == NULL)
- goto libelf_error;
- break;
-
- case ELFCLASS64:
- for (uint_fast16_t i = 0; i < phnum; ++i)
- if ((*p64)[i].p_type == PT_LOAD)
- if (handle_segment ((*p64)[i].p_vaddr, (*p64)[i].p_offset,
- (*p64)[i].p_filesz))
- goto read_error;
-
- /* If the segments visible in memory didn't include the section
- headers, then clear them from the file header. */
- if (contents_size < shdrs_end)
- {
- ehdr.e64.e_shoff = 0;
- ehdr.e64.e_shnum = 0;
- ehdr.e64.e_shstrndx = 0;
- }
-
- /* This will normally have been in the first PT_LOAD segment. But it
- conceivably could be missing, and we might have just changed it. */
- xlatefrom.d_type = xlateto.d_type = ELF_T_EHDR;
+ ehdr.e32.e_ident[EI_DATA]) == NULL)
+ goto libelf_error;
+ }
+ else
+ {
xlatefrom.d_size = xlateto.d_size = sizeof ehdr.e64;
xlatefrom.d_buf = &ehdr.e64;
- xlateto.d_buf = buffer;
if (elf64_xlatetof (&xlateto, &xlatefrom,
- ehdr.e64.e_ident[EI_DATA]) == NULL)
- goto libelf_error;
- break;
-
- default:
- abort ();
- break;
+ ehdr.e64.e_ident[EI_DATA]) == NULL)
+ goto libelf_error;
}
free (phdrsp);
--
2.26.2
^ permalink raw reply [flat|nested] 5+ messages in thread