[PATCH 02/14] segment_report_module: Pull segment_read into file scope
In preparation of getting rid of nested functions
Signed-off-by: Timm Bäder
---
libdwfl/dwfl_segment_report_module.c | 30 +---
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/libdwfl/dwfl_segment_report_module.c
b/libdwfl/dwfl_segment_report_module.c
index dd3fdb9e..9f06672a 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -232,6 +232,16 @@ invalid_elf (Elf *elf, bool disk_file_has_build_id,
return false;
}
+static inline bool
+segment_read (Dwfl *dwfl,
+ Dwfl_Memory_Callback *memory_callback, void *memory_callback_arg,
+ int segndx, void **buffer, size_t *buffer_available,
+ GElf_Addr addr, size_t minread)
+{
+ return ! (*memory_callback) (dwfl, segndx, buffer, buffer_available,
+ addr, minread, memory_callback_arg);
+}
+
int
dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
Dwfl_Memory_Callback *memory_callback,
@@ -257,18 +267,11 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
GElf_Addr start = dwfl->lookup_addr[segment];
- inline bool segment_read (int segndx,
- void **buffer, size_t *buffer_available,
- GElf_Addr addr, size_t minread)
- {
-return ! (*memory_callback) (dwfl, segndx, buffer, buffer_available,
-addr, minread, memory_callback_arg);
- }
-
inline void release_buffer (void **buffer, size_t *buffer_available)
{
if (*buffer != NULL)
- (void) segment_read (-1, buffer, buffer_available, 0, 0);
+ (void) segment_read (dwfl, memory_callback, memory_callback_arg,
+ -1, buffer, buffer_available, 0, 0);
}
/* First read in the file header and check its sanity. */
@@ -293,7 +296,8 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
return ndx;
}
- if (segment_read (ndx, &buffer, &buffer_available,
+ if (segment_read (dwfl, memory_callback, memory_callback_arg,
+ndx, &buffer, &buffer_available,
start, sizeof (Elf64_Ehdr))
|| memcmp (buffer, ELFMAG, SELFMAG) != 0)
return finish ();
@@ -312,7 +316,8 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
{
*data = NULL;
*data_size = filesz;
- return segment_read (addr_segndx (dwfl, segment, vaddr, false),
+ return segment_read (dwfl, memory_callback, memory_callback_arg,
+ addr_segndx (dwfl, segment, vaddr, false),
data, data_size, vaddr, filesz);
}
@@ -919,7 +924,8 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
{
void *into = contents + offset;
size_t read_size = size;
- (void) segment_read (addr_segndx (dwfl, segment, vaddr, false),
+ (void) segment_read (dwfl, memory_callback, memory_callback_arg,
+ addr_segndx (dwfl, segment, vaddr, false),
&into, &read_size, vaddr, size);
}
--
2.26.2
[PATCH 06/14] segment_report_module: Pull read_portion() into file scope
Signed-off-by: Timm Bäder
---
libdwfl/dwfl_segment_report_module.c | 79 +---
1 file changed, 48 insertions(+), 31 deletions(-)
diff --git a/libdwfl/dwfl_segment_report_module.c
b/libdwfl/dwfl_segment_report_module.c
index 751a96f1..00455aa4 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -254,6 +254,40 @@ finish_portion (Dwfl *dwfl,
-1, data, data_size, 0, 0);
}
+
+static inline bool
+read_portion (Dwfl *dwfl,
+ Dwfl_Memory_Callback *memory_callback,
+ void *memory_callback_arg,
+ void **data, size_t *data_size,
+ GElf_Addr vaddr, size_t filesz,
+ void *buffer,
+ size_t buffer_available,
+ GElf_Addr start,
+ size_t segment)
+{
+ /* Check whether we will have to read the segment data, or if it
+ can be returned from the existing buffer. */
+ if (filesz > buffer_available
+ || vaddr - start > buffer_available - filesz
+ /* If we're in string mode, then don't consider the buffer we have
+ sufficient unless it contains the terminator of the string. */
+ || (filesz == 0 && memchr (vaddr - start + buffer, '\0',
+ buffer_available - (vaddr - start)) == NULL))
+{
+ *data = NULL;
+ *data_size = filesz;
+ return segment_read (dwfl, memory_callback, memory_callback_arg,
+ addr_segndx (dwfl, segment, vaddr, false),
+ data, data_size, vaddr, filesz);
+}
+
+ /* We already have this whole note segment from our initial read. */
+ *data = vaddr - start + buffer;
+ *data_size = 0;
+ return false;
+}
+
int
dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
Dwfl_Memory_Callback *memory_callback,
@@ -296,31 +330,6 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
|| memcmp (buffer, ELFMAG, SELFMAG) != 0)
goto out;
- inline bool read_portion (void **data, size_t *data_size,
- GElf_Addr vaddr, size_t filesz)
- {
-/* Check whether we will have to read the segment data, or if it
- can be returned from the existing buffer. */
-if (filesz > buffer_available
- || vaddr - start > buffer_available - filesz
- /* If we're in string mode, then don't consider the buffer we have
- sufficient unless it contains the terminator of the string. */
- || (filesz == 0 && memchr (vaddr - start + buffer, '\0',
- buffer_available - (vaddr - start)) == NULL))
- {
- *data = NULL;
- *data_size = filesz;
- return segment_read (dwfl, memory_callback, memory_callback_arg,
- addr_segndx (dwfl, segment, vaddr, false),
-data, data_size, vaddr, filesz);
- }
-
-/* We already have this whole note segment from our initial read. */
-*data = vaddr - start + buffer;
-*data_size = 0;
-return false;
- }
-
/* Extract the information we need from the file header. */
const unsigned char *e_ident;
unsigned char ei_class;
@@ -401,8 +410,10 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
void *ph_buffer = NULL;
size_t ph_buffer_size = 0;
- if (read_portion (&ph_buffer, &ph_buffer_size,
- start + phoff, xlatefrom.d_size))
+ if (read_portion (dwfl, memory_callback, memory_callback_arg,
+&ph_buffer, &ph_buffer_size,
+start + phoff, xlatefrom.d_size,
+buffer, buffer_available, start, segment))
goto out;
/* ph_buffer_size will be zero if we got everything from the initial
@@ -459,7 +470,9 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
void *data;
size_t data_size;
-if (read_portion (&data, &data_size, vaddr, filesz))
+if (read_portion (dwfl, memory_callback, memory_callback_arg,
+ &data, &data_size, vaddr, filesz,
+ buffer, buffer_available, start, segment))
return;
/* data_size will be zero if we got everything from the initial
@@ -780,7 +793,9 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
void *dyn_data = NULL;
size_t dyn_data_size = 0;
if (dyn_filesz != 0 && dyn_filesz % dyn_entsize == 0
- && ! read_portion (&dyn_data, &dyn_data_size, dyn_vaddr, dyn_filesz))
+ && ! read_portion (dwfl, memory_callback, memory_callback_arg,
+ &dyn_data, &dyn_data_size, dyn_vaddr, dyn_filesz,
+ buffer, buffer_available, start, segment))
{
/* dyn_data_size will be zero if we got everything from the initial
buffer, otherwise it will be the size of the new buffer that
@@ -848,8 +863,10 @@ dwfl_segment_report_module (Dwfl *dwfl, int n
[PATCH 07/14] segment_report_module: Use a struct for build id information
Keep the three build id fields in a struct. This will be an important
clean up later.
Signed-off-by: Timm Bäder
---
libdwfl/dwfl_segment_report_module.c | 54
1 file changed, 31 insertions(+), 23 deletions(-)
diff --git a/libdwfl/dwfl_segment_report_module.c
b/libdwfl/dwfl_segment_report_module.c
index 00455aa4..44743fab 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -54,6 +54,13 @@
# define MY_ELFDATAELFDATA2MSB
#endif
+struct elf_build_id
+{
+ void *memory;
+ size_t len;
+ GElf_Addr vaddr;
+};
+
/* Return user segment index closest to ADDR but not above it.
If NEXT, return the closest to ADDR but not below it. */
@@ -206,16 +213,16 @@ handle_file_note (GElf_Addr module_start, GElf_Addr
module_end,
static bool
invalid_elf (Elf *elf, bool disk_file_has_build_id,
-const void *build_id, size_t build_id_len)
+ struct elf_build_id *build_id)
{
- if (! disk_file_has_build_id && build_id_len > 0)
+ if (! disk_file_has_build_id && build_id->len > 0)
{
/* Module found in segments with build-id is more reliable
than a module found via DT_DEBUG on disk without any
build-id. */
return true;
}
- if (disk_file_has_build_id && build_id_len > 0)
+ if (disk_file_has_build_id && build_id->len > 0)
{
const void *elf_build_id;
ssize_t elf_build_id_len;
@@ -224,8 +231,8 @@ invalid_elf (Elf *elf, bool disk_file_has_build_id,
elf_build_id_len = INTUSE(dwelf_elf_gnu_build_id) (elf, &elf_build_id);
if (elf_build_id_len > 0)
{
- if (build_id_len != (size_t) elf_build_id_len
- || memcmp (build_id, elf_build_id, build_id_len) != 0)
+ if (build_id->len != (size_t) elf_build_id_len
+ || memcmp (build_id->memory, elf_build_id, build_id->len) != 0)
return true;
}
}
@@ -456,16 +463,17 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
GElf_Xword dyn_filesz = 0;
/* Collect the build ID bits here. */
- void *build_id = NULL;
- size_t build_id_len = 0;
- GElf_Addr build_id_vaddr = 0;
+ struct elf_build_id build_id;
+ build_id.memory = NULL;
+ build_id.len = 0;
+ build_id.vaddr =0;
/* Consider a PT_NOTE we've found in the image. */
inline void consider_notes (GElf_Addr vaddr, GElf_Xword filesz,
GElf_Xword align)
{
/* If we have already seen a build ID, we don't care any more. */
-if (build_id != NULL || filesz == 0)
+if (build_id.memory != NULL || filesz == 0)
return;
void *data;
@@ -524,11 +532,11 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
&& nh->n_namesz == sizeof "GNU"
&& !memcmp (note_name, "GNU", sizeof "GNU"))
{
- build_id_vaddr = note_desc - (const void *) notes + vaddr;
- build_id_len = nh->n_descsz;
- build_id = malloc (nh->n_descsz);
- if (likely (build_id != NULL))
- memcpy (build_id, note_desc, build_id_len);
+build_id.vaddr = note_desc - (const void *) notes + vaddr;
+build_id.len = nh->n_descsz;
+build_id.memory = malloc (build_id.len);
+if (likely (build_id.memory != NULL))
+ memcpy (build_id.memory, note_desc, build_id.len);
break;
}
@@ -643,7 +651,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
header we read at START was not produced by these program headers. */
if (unlikely (!found_bias))
{
- free (build_id);
+ free (build_id.memory);
goto out;
}
@@ -698,7 +706,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
{
if (module->elf != NULL
&& invalid_elf (module->elf, module->disk_file_has_build_id,
- build_id, build_id_len))
+&build_id))
{
elf_end (module->elf);
close (module->fd);
@@ -714,7 +722,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
}
if (skip_this_module)
{
- free (build_id);
+ free (build_id.memory);
goto out;
}
}
@@ -733,7 +741,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
Dwfl_Error error = __libdw_open_file (&fd, &elf, true, false);
if (error == DWFL_E_NOERROR)
invalid = invalid_elf (elf, true /* disk_file_has_build_id */,
- build_id, build_id_len);
+ &build_id);
}
if (invalid)
{
@@ -881,11 +889,11 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
if (mod != NULL && (execlike || ehdr.e32.e_type == ET_EXEC))
mod->is_executable = true;
- i
[PATCH 05/14] segment_report_module: Pull finish_portion() info file scope
Signed-off-by: Timm Bäder
---
libdwfl/dwfl_segment_report_module.c | 27 ---
1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/libdwfl/dwfl_segment_report_module.c
b/libdwfl/dwfl_segment_report_module.c
index ba11b60a..751a96f1 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -242,6 +242,18 @@ segment_read (Dwfl *dwfl,
addr, minread, memory_callback_arg);
}
+
+static inline void
+finish_portion (Dwfl *dwfl,
+Dwfl_Memory_Callback *memory_callback,
+void *memory_callback_arg,
+void **data, size_t *data_size)
+{
+ if (*data_size != 0 && *data != NULL)
+(void) segment_read (dwfl, memory_callback, memory_callback_arg,
+ -1, data, data_size, 0, 0);
+}
+
int
dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
Dwfl_Memory_Callback *memory_callback,
@@ -309,13 +321,6 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
return false;
}
- inline void finish_portion (void **data, size_t *data_size)
- {
-if (*data_size != 0 && *data != NULL)
- (void) segment_read (dwfl, memory_callback, memory_callback_arg,
- -1, data, data_size, 0, 0);
- }
-
/* Extract the information we need from the file header. */
const unsigned char *e_ident;
unsigned char ei_class;
@@ -522,7 +527,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
done:
if (notes != data)
free (notes);
-finish_portion (&data, &data_size);
+finish_portion (dwfl, memory_callback, memory_callback_arg, &data,
&data_size);
}
/* Consider each of the program headers we've read from the image. */
@@ -619,7 +624,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
p64[i].p_align);
}
- finish_portion (&ph_buffer, &ph_buffer_size);
+ finish_portion (dwfl, memory_callback, memory_callback_arg, &ph_buffer,
&ph_buffer_size);
/* We must have seen the segment covering offset 0, or else the ELF
header we read at START was not produced by these program headers. */
@@ -811,7 +816,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
}
free (dyns);
}
- finish_portion (&dyn_data, &dyn_data_size);
+ finish_portion (dwfl, memory_callback, memory_callback_arg, &dyn_data,
&dyn_data_size);
/* We'll use the name passed in or a stupid default if not DT_SONAME. */
if (name == NULL)
@@ -872,7 +877,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
/* At this point we do not need BUILD_ID or NAME any more.
They have been copied. */
free (build_id);
- finish_portion (&soname, &soname_size);
+ finish_portion (dwfl, memory_callback, memory_callback_arg, &soname,
&soname_size);
if (unlikely (mod == NULL))
{
--
2.26.2
[PATCH 14/14] segment_report_module: Inline consider_phdr() into only caller
Get rid of the nested function this way
Signed-off-by: Timm Bäder
---
libdwfl/dwfl_segment_report_module.c | 142 +--
1 file changed, 66 insertions(+), 76 deletions(-)
diff --git a/libdwfl/dwfl_segment_report_module.c
b/libdwfl/dwfl_segment_report_module.c
index 0679453e..f93c60e2 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -475,7 +475,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
/* NOTE if the number of sections is > 0xff00 then e_shnum
is zero and the actual number would come from the section
zero sh_size field. We ignore this here because getting shdrs
-is just a nice bonus (see below in consider_phdr PT_LOAD
+is just a nice bonus (see below in the type == PT_LOAD case
where we trim the last segment). */
shdrs_end = ehdr.e32.e_shoff + ehdr.e32.e_shnum * ehdr.e32.e_shentsize;
break;
@@ -561,80 +561,6 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
build_id.len = 0;
build_id.vaddr =0;
- /* Consider each of the program headers we've read from the image. */
- inline void consider_phdr (GElf_Word type,
-GElf_Addr vaddr, GElf_Xword memsz,
-GElf_Off offset, GElf_Xword filesz,
-GElf_Xword align)
- {
-switch (type)
- {
- case PT_DYNAMIC:
- dyn_vaddr = vaddr;
- dyn_filesz = filesz;
- break;
-
- case PT_NOTE:
- /* We calculate from the p_offset of the note segment,
- because we don't yet know the bias for its p_vaddr. */
- consider_notes (dwfl, memory_callback, memory_callback_arg,
- start + offset, filesz, align,
- buffer, buffer_available, start, segment,
- ei_data, &build_id,
- &xlatefrom, &xlateto,
- ehdr.e32.e_ident[EI_DATA]);
- break;
-
- case PT_LOAD:
- align = dwfl->segment_align > 1 ? dwfl->segment_align : align ?: 1;
-
- GElf_Addr vaddr_end = (vaddr + memsz + align - 1) & -align;
- GElf_Addr filesz_vaddr = filesz < memsz ? vaddr + filesz : vaddr_end;
- GElf_Off filesz_offset = filesz_vaddr - vaddr + offset;
-
- if (file_trimmed_end < offset + filesz)
- {
- file_trimmed_end = offset + filesz;
-
- /* Trim the last segment so we don't bother with zeros
- in the last page that are off the end of the file.
- However, if the extra bit in that page includes the
- section headers, keep them. */
- if (shdrs_end <= filesz_offset && shdrs_end > file_trimmed_end)
- {
- filesz += shdrs_end - file_trimmed_end;
- file_trimmed_end = shdrs_end;
- }
- }
-
- total_filesz += filesz;
-
- if (file_end < filesz_offset)
- {
- file_end = filesz_offset;
- if (filesz_vaddr - start == filesz_offset)
- contiguous = file_end;
- }
-
- if (!found_bias && (offset & -align) == 0
- && likely (filesz_offset >= phoff + phnum * phentsize))
- {
- bias = start - vaddr;
- found_bias = true;
- }
-
- if ((vaddr & -align) < module_start)
- {
- module_start = vaddr & -align;
- module_address_sync = vaddr + memsz;
- }
-
- if (module_end < vaddr_end)
- module_end = vaddr_end;
- break;
- }
- }
-
Elf32_Phdr *p32 = phdrsp;
Elf64_Phdr *p64 = phdrsp;
if ((ei_class == ELFCLASS32 &&
@@ -646,6 +572,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
}
else
{
+ /* Consider each of the program headers we've read from the image. */
for (uint_fast16_t i = 0; i < phnum; ++i)
{
bool is32 = (ei_class == ELFCLASS32);
@@ -656,7 +583,70 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
GElf_Xword filesz = is32 ? p32[i].p_filesz : p64[i].p_filesz;
GElf_Xword align = is32 ? p32[i].p_align : p64[i].p_align;
- consider_phdr (type, vaddr, memsz, offset, filesz, align);
+ if (type == PT_DYNAMIC)
+{
+ dyn_vaddr = vaddr;
+ dyn_filesz = filesz;
+}
+ else if (type == PT_NOTE)
+{
+ /* We calculate from the p_offset of the note segment,
+ because we don't yet know the bias for its p_vaddr. */
+ consider_notes (dwfl, memory_callback, memory_callback_arg,
+ start + offset, filesz, align,
+ buffer, buffer_available, start, segment,
+ ei_data, &build_id,
+ &xlatefrom, &xlateto,
+
[PATCH 13/14] segment_report_module: Inline consider_dyn() into only caller
Signed-off-by: Timm Bäder
---
libdwfl/dwfl_segment_report_module.c | 40 +---
1 file changed, 12 insertions(+), 28 deletions(-)
diff --git a/libdwfl/dwfl_segment_report_module.c
b/libdwfl/dwfl_segment_report_module.c
index 7c97784f..0679453e 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -784,33 +784,6 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
GElf_Addr dynstr_vaddr = 0;
GElf_Xword dynstrsz = 0;
bool execlike = false;
- inline bool consider_dyn (GElf_Sxword tag, GElf_Xword val)
- {
-switch (tag)
- {
- default:
- return false;
-
- case DT_DEBUG:
- execlike = true;
- break;
-
- case DT_SONAME:
- soname_stroff = val;
- break;
-
- case DT_STRTAB:
- dynstr_vaddr = val;
- break;
-
- case DT_STRSZ:
- dynstrsz = val;
- break;
- }
-
-return soname_stroff != 0 && dynstr_vaddr != 0 && dynstrsz != 0;
- }
-
const size_t dyn_entsize = (ei_class == ELFCLASS32
? sizeof (Elf32_Dyn) : sizeof (Elf64_Dyn));
void *dyn_data = NULL;
@@ -848,7 +821,18 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
GElf_Sxword tag = is32 ? d32[i].d_tag : d64[i].d_tag;
GElf_Xword val = is32 ? d32[i].d_un.d_val : d64[i].d_un.d_val;
- if (consider_dyn (tag, val))
+ if (tag == DT_DEBUG)
+execlike = true;
+ else if (tag == DT_SONAME)
+soname_stroff = val;
+ else if (tag == DT_STRTAB)
+dynstr_vaddr = val;
+ else if (tag == DT_STRSZ)
+dynstrsz = val;
+ else
+continue;
+
+ if (soname_stroff != 0 && dynstr_vaddr != 0 && dynstrsz != 0)
break;
}
}
--
2.26.2
[PATCH 04/14] segment_report_module: Remove nested release_buffer() function
Signed-off-by: Timm Bäder
---
libdwfl/dwfl_segment_report_module.c | 16 ++--
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/libdwfl/dwfl_segment_report_module.c
b/libdwfl/dwfl_segment_report_module.c
index d06d0ba0..ba11b60a 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -267,13 +267,6 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
GElf_Addr start = dwfl->lookup_addr[segment];
- inline void release_buffer (void **buffer, size_t *buffer_available)
- {
-if (*buffer != NULL)
- (void) segment_read (dwfl, memory_callback, memory_callback_arg,
- -1, buffer, buffer_available, 0, 0);
- }
-
/* First read in the file header and check its sanity. */
void *buffer = NULL;
@@ -318,8 +311,9 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
inline void finish_portion (void **data, size_t *data_size)
{
-if (*data_size != 0)
- release_buffer (data, data_size);
+if (*data_size != 0 && *data != NULL)
+ (void) segment_read (dwfl, memory_callback, memory_callback_arg,
+ -1, data, data_size, 0, 0);
}
/* Extract the information we need from the file header. */
@@ -972,7 +966,9 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
out:
free (phdrsp);
- release_buffer (&buffer, &buffer_available);
+ if (buffer != NULL)
+(void) segment_read (dwfl, memory_callback, memory_callback_arg,
+ -1, &buffer, &buffer_available, 0, 0);
if (elf != NULL)
elf_end (elf);
if (fd != -1)
--
2.26.2
[PATCH 08/14] segment_report_module: Pull consider_notes() into file scope
Signed-off-by: Timm Bäder
---
libdwfl/dwfl_segment_report_module.c | 187 +++
1 file changed, 101 insertions(+), 86 deletions(-)
diff --git a/libdwfl/dwfl_segment_report_module.c
b/libdwfl/dwfl_segment_report_module.c
index 44743fab..76686a72 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -295,6 +295,99 @@ read_portion (Dwfl *dwfl,
return false;
}
+
+/* Consider a PT_NOTE we've found in the image. */
+static inline void
+consider_notes (Dwfl *dwfl,
+Dwfl_Memory_Callback *memory_callback, void
*memory_callback_arg,
+GElf_Addr vaddr, GElf_Xword filesz,
+GElf_Xword align,
+void *buffer, size_t buffer_available,
+GElf_Addr start,
+size_t segment,
+unsigned char ei_data,
+struct elf_build_id *build_id,
+Elf_Data *xlatefrom,
+Elf_Data *xlateto,
+unsigned int xencoding)
+{
+ if (build_id->memory != NULL || filesz == 0)
+return;
+
+ void *data;
+ size_t data_size;
+ if (read_portion (dwfl, memory_callback, memory_callback_arg,
+&data, &data_size, vaddr, filesz,
+buffer, buffer_available, start, segment))
+return;
+
+ /* data_size will be zero if we got everything from the initial
+ buffer, otherwise it will be the size of the new buffer that
+ could be read. */
+ if (data_size != 0)
+filesz = data_size;
+
+ assert (sizeof (Elf32_Nhdr) == sizeof (Elf64_Nhdr));
+
+ void *notes;
+ if (ei_data == MY_ELFDATA)
+notes = data;
+ else
+{
+ notes = malloc (filesz);
+ if (unlikely (notes == NULL))
+return;
+ xlatefrom->d_type = xlateto->d_type = (align == 8
+ ? ELF_T_NHDR8 : ELF_T_NHDR);
+ xlatefrom->d_buf = (void *) data;
+ xlatefrom->d_size = filesz;
+ xlateto->d_buf = notes;
+ xlateto->d_size = filesz;
+ if (elf32_xlatetom (xlateto, xlatefrom, xencoding) == NULL)
+goto done;
+}
+
+ const GElf_Nhdr *nh = notes;
+ size_t len = 0;
+ while (filesz > len + sizeof (*nh))
+{
+ const void *note_name;
+ const void *note_desc;
+
+ len += sizeof (*nh);
+ note_name = notes + len;
+
+ len += nh->n_namesz;
+ len = align == 8 ? NOTE_ALIGN8 (len) : NOTE_ALIGN4 (len);
+ note_desc = notes + len;
+
+ if (unlikely (filesz < len + nh->n_descsz))
+break;
+
+ if (nh->n_type == NT_GNU_BUILD_ID
+ && nh->n_descsz > 0
+ && nh->n_namesz == sizeof "GNU"
+ && !memcmp (note_name, "GNU", sizeof "GNU"))
+{
+ build_id->vaddr = note_desc - (const void *) notes + vaddr;
+ build_id->len = nh->n_descsz;
+ build_id->memory = malloc (build_id->len);
+ if (likely (build_id->memory != NULL))
+memcpy (build_id->memory, note_desc, build_id->len);
+ break;
+}
+
+ len += nh->n_descsz;
+ len = align == 8 ? NOTE_ALIGN8 (len) : NOTE_ALIGN4 (len);
+ nh = (void *) notes + len;
+}
+
+done:
+ if (notes != data)
+free (notes);
+ finish_portion (dwfl, memory_callback, memory_callback_arg, &data,
&data_size);
+}
+
int
dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
Dwfl_Memory_Callback *memory_callback,
@@ -468,89 +561,6 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
build_id.len = 0;
build_id.vaddr =0;
- /* Consider a PT_NOTE we've found in the image. */
- inline void consider_notes (GElf_Addr vaddr, GElf_Xword filesz,
- GElf_Xword align)
- {
-/* If we have already seen a build ID, we don't care any more. */
-if (build_id.memory != NULL || filesz == 0)
- return;
-
-void *data;
-size_t data_size;
-if (read_portion (dwfl, memory_callback, memory_callback_arg,
- &data, &data_size, vaddr, filesz,
- buffer, buffer_available, start, segment))
- return;
-
-/* data_size will be zero if we got everything from the initial
- buffer, otherwise it will be the size of the new buffer that
- could be read. */
-if (data_size != 0)
- filesz = data_size;
-
-assert (sizeof (Elf32_Nhdr) == sizeof (Elf64_Nhdr));
-
-void *notes;
-if (ei_data == MY_ELFDATA)
- notes = data;
-else
- {
- notes = malloc (filesz);
- if (unlikely (notes == NULL))
- return;
- xlatefrom.d_type = xlateto.d_type = (align == 8
-? ELF_T_NHDR8 : ELF_T_NHDR);
- xlatefrom.d_buf = (void *) data;
- xlatefrom.d_size = filesz;
- xlateto.d_buf = notes;
- xlateto.d_size = filesz;
- if (elf32_xlatetom (&xlateto, &xlatefrom,
- ehdr.e32.e_ident[EI_DATA]) == NULL)
-
[PATCH 01/14] segment_report_module: Get rid of variable-length arrays
This prevents a jump which is needed in a later patch.
Signed-off-by: Timm Bäder
---
libdwfl/dwfl_segment_report_module.c | 36 ++--
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/libdwfl/dwfl_segment_report_module.c
b/libdwfl/dwfl_segment_report_module.c
index 430e13d5..dd3fdb9e 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -606,18 +606,18 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
}
}
- Elf32_Phdr (*p32)[phnum] = phdrsp;
- Elf64_Phdr (*p64)[phnum] = phdrsp;
+ Elf32_Phdr *p32 = phdrsp;
+ Elf64_Phdr *p64 = phdrsp;
if (ei_class == ELFCLASS32)
{
if (elf32_xlatetom (&xlateto, &xlatefrom, ei_data) == NULL)
found_bias = false; /* Trigger error check. */
else
for (uint_fast16_t i = 0; i < phnum; ++i)
- consider_phdr ((*p32)[i].p_type,
-(*p32)[i].p_vaddr, (*p32)[i].p_memsz,
-(*p32)[i].p_offset, (*p32)[i].p_filesz,
-(*p32)[i].p_align);
+ consider_phdr (p32[i].p_type,
+p32[i].p_vaddr, p32[i].p_memsz,
+p32[i].p_offset, p32[i].p_filesz,
+p32[i].p_align);
}
else
{
@@ -625,10 +625,10 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
found_bias = false; /* Trigger error check. */
else
for (uint_fast16_t i = 0; i < phnum; ++i)
- consider_phdr ((*p64)[i].p_type,
-(*p64)[i].p_vaddr, (*p64)[i].p_memsz,
-(*p64)[i].p_offset, (*p64)[i].p_filesz,
-(*p64)[i].p_align);
+ consider_phdr (p64[i].p_type,
+p64[i].p_vaddr, p64[i].p_memsz,
+p64[i].p_offset, p64[i].p_filesz,
+p64[i].p_align);
}
finish_portion (&ph_buffer, &ph_buffer_size);
@@ -796,8 +796,8 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
dyn_filesz = dyn_data_size;
void *dyns = malloc (dyn_filesz);
- Elf32_Dyn (*d32)[dyn_filesz / sizeof (Elf32_Dyn)] = dyns;
- Elf64_Dyn (*d64)[dyn_filesz / sizeof (Elf64_Dyn)] = dyns;
+ Elf32_Dyn *d32 = dyns;
+ Elf64_Dyn *d64 = dyns;
if (unlikely (dyns == NULL))
return finish ();
@@ -811,14 +811,14 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
{
if (elf32_xlatetom (&xlateto, &xlatefrom, ei_data) != NULL)
for (size_t i = 0; i < dyn_filesz / sizeof (Elf32_Dyn); ++i)
- if (consider_dyn ((*d32)[i].d_tag, (*d32)[i].d_un.d_val))
+ if (consider_dyn (d32[i].d_tag, d32[i].d_un.d_val))
break;
}
else
{
if (elf64_xlatetom (&xlateto, &xlatefrom, ei_data) != NULL)
for (size_t i = 0; i < dyn_filesz / sizeof (Elf64_Dyn); ++i)
- if (consider_dyn ((*d64)[i].d_tag, (*d64)[i].d_un.d_val))
+ if (consider_dyn (d64[i].d_tag, d64[i].d_un.d_val))
break;
}
free (dyns);
@@ -937,12 +937,12 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
if (ei_class == ELFCLASS32)
for (uint_fast16_t i = 0; i < phnum; ++i)
- read_phdr ((*p32)[i].p_type, (*p32)[i].p_vaddr,
-(*p32)[i].p_offset, (*p32)[i].p_filesz);
+ read_phdr (p32[i].p_type, p32[i].p_vaddr,
+p32[i].p_offset, p32[i].p_filesz);
else
for (uint_fast16_t i = 0; i < phnum; ++i)
- read_phdr ((*p64)[i].p_type, (*p64)[i].p_vaddr,
-(*p64)[i].p_offset, (*p64)[i].p_filesz);
+ read_phdr (p64[i].p_type, p64[i].p_vaddr,
+p64[i].p_offset, p64[i].p_filesz);
}
else
{
--
2.26.2
[PATCH 10/14] segment_report_module: Use one loop for p32/p64 arrays
Do one loop check for 32/64 bit inside the loop, instead of outside.
This way we have only one call site for the function called in the loop
body.
Signed-off-by: Timm Bäder
---
libdwfl/dwfl_segment_report_module.c | 52 +++-
1 file changed, 27 insertions(+), 25 deletions(-)
diff --git a/libdwfl/dwfl_segment_report_module.c
b/libdwfl/dwfl_segment_report_module.c
index 80f97cfd..2535f9e3 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -637,27 +637,27 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
Elf32_Phdr *p32 = phdrsp;
Elf64_Phdr *p64 = phdrsp;
- if (ei_class == ELFCLASS32)
+ if ((ei_class == ELFCLASS32 &&
+ elf32_xlatetom (&xlateto, &xlatefrom, ei_data) == NULL) ||
+ (ei_class == ELFCLASS64 &&
+ elf64_xlatetom (&xlateto, &xlatefrom, ei_data) == NULL))
{
- if (elf32_xlatetom (&xlateto, &xlatefrom, ei_data) == NULL)
- found_bias = false; /* Trigger error check. */
- else
- for (uint_fast16_t i = 0; i < phnum; ++i)
- consider_phdr (p32[i].p_type,
-p32[i].p_vaddr, p32[i].p_memsz,
-p32[i].p_offset, p32[i].p_filesz,
-p32[i].p_align);
+ found_bias = false; /* Trigger error check */
}
else
{
- if (elf64_xlatetom (&xlateto, &xlatefrom, ei_data) == NULL)
- found_bias = false; /* Trigger error check. */
- else
- for (uint_fast16_t i = 0; i < phnum; ++i)
- consider_phdr (p64[i].p_type,
-p64[i].p_vaddr, p64[i].p_memsz,
-p64[i].p_offset, p64[i].p_filesz,
-p64[i].p_align);
+ for (uint_fast16_t i = 0; i < phnum; ++i)
+{
+ bool is32 = (ei_class == ELFCLASS32);
+ GElf_Word type = is32 ? p32[i].p_type : p64[i].p_type;
+ GElf_Addr vaddr = is32 ? p32[i].p_vaddr : p64[i].p_vaddr;
+ GElf_Xword memsz = is32 ? p32[i].p_memsz : p64[i].p_memsz;
+ GElf_Off offset = is32 ? p32[i].p_offset : p64[i].p_offset;
+ GElf_Xword filesz = is32 ? p32[i].p_filesz : p64[i].p_filesz;
+ GElf_Xword align = is32 ? p32[i].p_align : p64[i].p_align;
+
+ consider_phdr (type, vaddr, memsz, offset, filesz, align);
+}
}
finish_portion (dwfl, memory_callback, memory_callback_arg, &ph_buffer,
&ph_buffer_size);
@@ -967,14 +967,16 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
}
}
- if (ei_class == ELFCLASS32)
- for (uint_fast16_t i = 0; i < phnum; ++i)
- read_phdr (p32[i].p_type, p32[i].p_vaddr,
-p32[i].p_offset, p32[i].p_filesz);
- else
- for (uint_fast16_t i = 0; i < phnum; ++i)
- read_phdr (p64[i].p_type, p64[i].p_vaddr,
-p64[i].p_offset, p64[i].p_filesz);
+ for (uint_fast16_t i = 0; i < phnum; ++i)
+{
+ bool is32 = (ei_class == ELFCLASS32);
+ GElf_Word type = is32 ? p32[i].p_type : p64[i].p_type;
+ GElf_Addr vaddr = is32 ? p32[i].p_vaddr : p64[i].p_vaddr;
+ GElf_Off offset = is32 ? p32[i].p_offset : p64[i].p_offset;
+ GElf_Xword filesz = is32 ? p32[i].p_filesz : p64[i].p_filesz;
+
+ read_phdr (type, vaddr, offset, filesz);
+}
}
else
{
--
2.26.2
Removing gnu99 constructs from elfutils
Hi, I'm looking into removing both the nested functions as well as variable-length arrays in the elfutils source code, so it can be built using clang. This is a continuation of https://sourceware.org/bugzilla/show_bug.cgi?id=24964 basically. I did try to incorporate some cleanup commits as well so the result does not get too ugly. Some of the nested functions have quite a few hidden dependencies on the surrounding code. I started with libdwfl/dwfl_segment_report_module.c but am planning on converting everything else as well of course. What do you think? Thanks, Timm
[PATCH 09/14] segment_report_module: Get rid of nested final_read() function
Signed-off-by: Timm Bäder
---
libdwfl/dwfl_segment_report_module.c | 26 +++---
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/libdwfl/dwfl_segment_report_module.c
b/libdwfl/dwfl_segment_report_module.c
index 76686a72..80f97cfd 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -948,15 +948,6 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
if (unlikely (contents == NULL))
goto out;
- inline void final_read (size_t offset, GElf_Addr vaddr, size_t size)
- {
- void *into = contents + offset;
- size_t read_size = size;
- (void) segment_read (dwfl, memory_callback, memory_callback_arg,
- addr_segndx (dwfl, segment, vaddr, false),
-&into, &read_size, vaddr, size);
- }
-
if (contiguous < file_trimmed_end)
{
/* We can't use the memory image verbatim as the file image.
@@ -966,7 +957,14 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
GElf_Off offset, GElf_Xword filesz)
{
if (type == PT_LOAD)
- final_read (offset, vaddr + bias, filesz);
+ {
+void *into = contents + offset;
+size_t read_size = filesz;
+(void)segment_read (dwfl, memory_callback, memory_callback_arg,
+addr_segndx (dwfl, segment, vaddr + bias,
false),
+&into, &read_size, vaddr + bias,
read_size);
+
+ }
}
if (ei_class == ELFCLASS32)
@@ -987,7 +985,13 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
memcpy (contents, buffer, have);
if (have < file_trimmed_end)
- final_read (have, start + have, file_trimmed_end - have);
+{
+ void *into = contents + have;
+ size_t read_size = file_trimmed_end - have;
+ (void)segment_read (dwfl, memory_callback, memory_callback_arg,
+ addr_segndx (dwfl, segment, start + have,
false),
+ &into, &read_size, start + have, read_size);
+}
}
elf = elf_memory (contents, file_trimmed_end);
--
2.26.2
[PATCH 03/14] segment_report_module: Remove nested finish() function
This works just as well with a goto-out style label.
Signed-off-by: Timm Bäder
---
libdwfl/dwfl_segment_report_module.c | 50 +---
1 file changed, 23 insertions(+), 27 deletions(-)
diff --git a/libdwfl/dwfl_segment_report_module.c
b/libdwfl/dwfl_segment_report_module.c
index 9f06672a..d06d0ba0 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -285,22 +285,11 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
here so we can always safely free it. */
void *phdrsp = NULL;
- inline int finish (void)
- {
-free (phdrsp);
-release_buffer (&buffer, &buffer_available);
-if (elf != NULL)
- elf_end (elf);
-if (fd != -1)
- close (fd);
-return ndx;
- }
-
if (segment_read (dwfl, memory_callback, memory_callback_arg,
ndx, &buffer, &buffer_available,
start, sizeof (Elf64_Ehdr))
|| memcmp (buffer, ELFMAG, SELFMAG) != 0)
-return finish ();
+goto out;
inline bool read_portion (void **data, size_t *data_size,
GElf_Addr vaddr, size_t filesz)
@@ -368,13 +357,13 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
case ELFCLASS32:
xlatefrom.d_size = sizeof (Elf32_Ehdr);
if (elf32_xlatetom (&xlateto, &xlatefrom, ei_data) == NULL)
- return finish ();
+ goto out;
e_type = ehdr.e32.e_type;
phoff = ehdr.e32.e_phoff;
phnum = ehdr.e32.e_phnum;
phentsize = ehdr.e32.e_phentsize;
if (phentsize != sizeof (Elf32_Phdr))
- return finish ();
+ goto out;
/* NOTE if the number of sections is > 0xff00 then e_shnum
is zero and the actual number would come from the section
zero sh_size field. We ignore this here because getting shdrs
@@ -386,19 +375,19 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
case ELFCLASS64:
xlatefrom.d_size = sizeof (Elf64_Ehdr);
if (elf64_xlatetom (&xlateto, &xlatefrom, ei_data) == NULL)
- return finish ();
+ goto out;
e_type = ehdr.e64.e_type;
phoff = ehdr.e64.e_phoff;
phnum = ehdr.e64.e_phnum;
phentsize = ehdr.e64.e_phentsize;
if (phentsize != sizeof (Elf64_Phdr))
- return finish ();
+ goto out;
/* See the NOTE above for shdrs_end and ehdr.e32.e_shnum. */
shdrs_end = ehdr.e64.e_shoff + ehdr.e64.e_shnum * ehdr.e64.e_shentsize;
break;
default:
- return finish ();
+ goto out;
}
/* The file header tells where to find the program headers.
@@ -406,7 +395,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
Without them, we don't have a module to report. */
if (phnum == 0)
-return finish ();
+goto out;
xlatefrom.d_type = xlateto.d_type = ELF_T_PHDR;
xlatefrom.d_size = phnum * phentsize;
@@ -415,7 +404,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
size_t ph_buffer_size = 0;
if (read_portion (&ph_buffer, &ph_buffer_size,
start + phoff, xlatefrom.d_size))
-return finish ();
+goto out;
/* ph_buffer_size will be zero if we got everything from the initial
buffer, otherwise it will be the size of the new buffer that
@@ -428,11 +417,11 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
bool class32 = ei_class == ELFCLASS32;
size_t phdr_size = class32 ? sizeof (Elf32_Phdr) : sizeof (Elf64_Phdr);
if (unlikely (phnum > SIZE_MAX / phdr_size))
-return finish ();
+goto out;
const size_t phdrsp_bytes = phnum * phdr_size;
phdrsp = malloc (phdrsp_bytes);
if (unlikely (phdrsp == NULL))
-return finish ();
+goto out;
xlateto.d_buf = phdrsp;
xlateto.d_size = phdrsp_bytes;
@@ -643,7 +632,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
if (unlikely (!found_bias))
{
free (build_id);
- return finish ();
+ goto out;
}
/* Now we know enough to report a module for sure: its bounds. */
@@ -714,7 +703,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
if (skip_this_module)
{
free (build_id);
- return finish ();
+ goto out;
}
}
@@ -804,7 +793,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
Elf32_Dyn *d32 = dyns;
Elf64_Dyn *d64 = dyns;
if (unlikely (dyns == NULL))
- return finish ();
+ goto out;
xlatefrom.d_type = xlateto.d_type = ELF_T_DYN;
xlatefrom.d_buf = (void *) dyn_data;
@@ -894,7 +883,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
if (unlikely (mod == NULL))
{
ndx = -1;
- return finish ();
+ goto out;
}
/* We have reported the module. Now let the caller decide whether we
@@ -918,7 +907,7 @@ dwfl_segment_repor
[PATCH 11/14] segment_report_module: Inline read_phdr() into only caller
There is now only one caller for this nested function, so get rid of it
by just inlining it there.
Signed-off-by: Timm Bäder
---
libdwfl/dwfl_segment_report_module.c | 25 +
1 file changed, 9 insertions(+), 16 deletions(-)
diff --git a/libdwfl/dwfl_segment_report_module.c
b/libdwfl/dwfl_segment_report_module.c
index 2535f9e3..bcf69fe7 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -952,30 +952,23 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
{
/* We can't use the memory image verbatim as the file image.
So we'll be reading into a local image of the virtual file. */
-
- inline void read_phdr (GElf_Word type, GElf_Addr vaddr,
-GElf_Off offset, GElf_Xword filesz)
- {
- if (type == PT_LOAD)
- {
-void *into = contents + offset;
-size_t read_size = filesz;
-(void)segment_read (dwfl, memory_callback, memory_callback_arg,
-addr_segndx (dwfl, segment, vaddr + bias,
false),
-&into, &read_size, vaddr + bias,
read_size);
-
- }
- }
-
for (uint_fast16_t i = 0; i < phnum; ++i)
{
bool is32 = (ei_class == ELFCLASS32);
GElf_Word type = is32 ? p32[i].p_type : p64[i].p_type;
+
+ if (type != PT_LOAD)
+continue;
+
GElf_Addr vaddr = is32 ? p32[i].p_vaddr : p64[i].p_vaddr;
GElf_Off offset = is32 ? p32[i].p_offset : p64[i].p_offset;
GElf_Xword filesz = is32 ? p32[i].p_filesz : p64[i].p_filesz;
- read_phdr (type, vaddr, offset, filesz);
+ void *into = contents + offset;
+ size_t read_size = filesz;
+ (void)segment_read (dwfl, memory_callback, memory_callback_arg,
+ addr_segndx (dwfl, segment, vaddr + bias,
false),
+ &into, &read_size, vaddr + bias, read_size);
}
}
else
--
2.26.2
[PATCH 12/14] segment_report_module: Unify d32/d64 loops
Just like we did before, use only one loop here and check for 32/64 bit
in the loop body. This way we only have one call site for consider_dyn
Signed-off-by: Timm Bäder
---
libdwfl/dwfl_segment_report_module.c | 28 ++--
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/libdwfl/dwfl_segment_report_module.c
b/libdwfl/dwfl_segment_report_module.c
index bcf69fe7..7c97784f 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -838,20 +838,20 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
xlateto.d_buf = dyns;
xlateto.d_size = dyn_filesz;
- if (ei_class == ELFCLASS32)
- {
- if (elf32_xlatetom (&xlateto, &xlatefrom, ei_data) != NULL)
- for (size_t i = 0; i < dyn_filesz / sizeof (Elf32_Dyn); ++i)
- if (consider_dyn (d32[i].d_tag, d32[i].d_un.d_val))
- break;
- }
- else
- {
- if (elf64_xlatetom (&xlateto, &xlatefrom, ei_data) != NULL)
- for (size_t i = 0; i < dyn_filesz / sizeof (Elf64_Dyn); ++i)
- if (consider_dyn (d64[i].d_tag, d64[i].d_un.d_val))
- break;
- }
+ bool is32 = (ei_class == ELFCLASS32);
+ if ((is32 && elf32_xlatetom (&xlateto, &xlatefrom, ei_data) != NULL)
+ || (!is32 && elf64_xlatetom (&xlateto, &xlatefrom, ei_data) != NULL))
+{
+ size_t n = is32 ? (dyn_filesz / sizeof (Elf32_Dyn)) : (dyn_filesz /
sizeof (Elf64_Dyn));
+ for (size_t i = 0; i < n; ++i)
+{
+ GElf_Sxword tag = is32 ? d32[i].d_tag : d64[i].d_tag;
+ GElf_Xword val = is32 ? d32[i].d_un.d_val : d64[i].d_un.d_val;
+
+ if (consider_dyn (tag, val))
+break;
+}
+}
free (dyns);
}
finish_portion (dwfl, memory_callback, memory_callback_arg, &dyn_data,
&dyn_data_size);
--
2.26.2
Re: [PATCH 14/14] segment_report_module: Inline consider_phdr() into only caller
On 12/11/2020 17:52, Navin P wrote: Hi, I already have a patch that makes elfutils compile with clang. Since you are working this will be of use to you. I've attached the patch since it is big. Here are some of the changes 1. All functions at file scope have static qualifier so that no collison with other files. 2. Non global variables declared in the outer function should be passed as pointer variable in the new outer file scope function whenever they are assigned in the nested function. The argument addition in new functions are at the end. 3. With the applied patch above , gcc passes all 220 tests where clang fails 3 tests which is due to llvm_addrsig (change in libelf/elf.h ) and other 2 tests are error related to .rela.eh_frame. Thanks, Navin. Has this been proposed for inclusion in elfutils? What's the status on that? Or are you just keeping this locally? Looking at the patch, I'm not really a fan of a few of those changes, from a code point of view. consider_phdr() takes 35 arguments now for example. Do you have more information on the test failures? Are they caused by LLVM/clang bugs? Thanks, Timm
Re: Removing gnu99 constructs from elfutils
On 13/11/2020 13:38, Mark Wielaard wrote: OK. The mail subject is a little misleading, this is far from turning the code base into ISO C99. gnu99 is much more than those two features: https://gcc.gnu.org/onlinedocs/gcc/C-Extensions.html Right, I wasn't trying to get rid of all the gnu99 features used in the code base. Personally I think it would be nicer if compilers that claim to support gnu99 implemented them all instead of just cherry-picking some. I agree, but even if clang stops claiming it supports gnu99, we still need to stop using those features if elfutils should be compile-able with clang. Thanks for splitting this up into easily reviewable patches. I'll go over them next week. I haven't immediately seen anything objectionable, so I think your approach is good. Maybe you can work together with Navin to untangle his large patch in a similar way. I have a local working build now and around 45 patches. I did run into the test suite failures Navin described however and am now looking at what exactly is wrong there. Next week sounds fine with me, I'll wait for feedback on the general approach before posting the rest of the patches. Thanks, Timm -- Red Hat GmbH, http://www.de.redhat.com/, Registered seat: Grasbrunn, Commercial register: Amtsgericht Muenchen, HRB 153243, Managing Directors: Charles Cachera, Michael O'Neill, Tom Savage, Eric Shander
Remove nested functions from libdwfl V2
Hi again, version 2 of this patch set. I removed segmend_read() entirely now, which meant modifying a bunch of later patches. Other than that, they are the same. Hope the --from to git send-email worked out, too. Thanks, Timm
[PATCH 01/12] segment_report_module: Get rid of segment_read()
From: Timm Bäder
Just inline the memory_callback call everywhere segmenty_read was used.
Signed-off-by: Timm Bäder
---
libdwfl/dwfl_segment_report_module.c | 24 +---
1 file changed, 9 insertions(+), 15 deletions(-)
diff --git a/libdwfl/dwfl_segment_report_module.c
b/libdwfl/dwfl_segment_report_module.c
index c7725002..c587efd7 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -257,18 +257,11 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
GElf_Addr start = dwfl->lookup_addr[segment];
- inline bool segment_read (int segndx,
- void **buffer, size_t *buffer_available,
- GElf_Addr addr, size_t minread)
- {
-return ! (*memory_callback) (dwfl, segndx, buffer, buffer_available,
-addr, minread, memory_callback_arg);
- }
-
inline void release_buffer (void **buffer, size_t *buffer_available)
{
if (*buffer != NULL)
- (void) segment_read (-1, buffer, buffer_available, 0, 0);
+ (*memory_callback) (dwfl, -1, buffer, buffer_available, 0, 0,
+ memory_callback_arg);
}
/* First read in the file header and check its sanity. */
@@ -282,8 +275,8 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
here so we can always safely free it. */
void *phdrsp = NULL;
- if (segment_read (ndx, &buffer, &buffer_available,
- start, sizeof (Elf64_Ehdr))
+ if (! (*memory_callback) (dwfl, ndx, &buffer, &buffer_available,
+start, sizeof (Elf64_Ehdr), memory_callback_arg)
|| memcmp (buffer, ELFMAG, SELFMAG) != 0)
goto out;
@@ -301,8 +294,8 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
{
*data = NULL;
*data_size = filesz;
- return segment_read (addr_segndx (dwfl, segment, vaddr, false),
-data, data_size, vaddr, filesz);
+return ! (*memory_callback) (dwfl, addr_segndx (dwfl, segment, vaddr,
false),
+ data, data_size, vaddr, filesz,
memory_callback_arg);
}
/* We already have this whole note segment from our initial read. */
@@ -908,8 +901,9 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
{
void *into = contents + offset;
size_t read_size = size;
- (void) segment_read (addr_segndx (dwfl, segment, vaddr, false),
-&into, &read_size, vaddr, size);
+(*memory_callback) (dwfl, addr_segndx (dwfl, segment, vaddr, false),
+&into, &read_size, vaddr, size,
+memory_callback_arg);
}
if (contiguous < file_trimmed_end)
--
2.26.2
[PATCH 02/12] segment_report_module: Remove nested release_buffer() function
From: Timm Bäder
Signed-off-by: Timm Bäder
---
libdwfl/dwfl_segment_report_module.c | 16 ++--
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/libdwfl/dwfl_segment_report_module.c
b/libdwfl/dwfl_segment_report_module.c
index c587efd7..848c3bec 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -257,13 +257,6 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
GElf_Addr start = dwfl->lookup_addr[segment];
- inline void release_buffer (void **buffer, size_t *buffer_available)
- {
-if (*buffer != NULL)
- (*memory_callback) (dwfl, -1, buffer, buffer_available, 0, 0,
- memory_callback_arg);
- }
-
/* First read in the file header and check its sanity. */
void *buffer = NULL;
@@ -306,8 +299,8 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
inline void finish_portion (void **data, size_t *data_size)
{
-if (*data_size != 0)
- release_buffer (data, data_size);
+if (*data_size != 0 && *data != NULL)
+ (*memory_callback) (dwfl, -1, data, data_size, 0, 0,
memory_callback_arg);
}
/* Extract the information we need from the file header. */
@@ -960,7 +953,10 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
out:
free (phdrsp);
- release_buffer (&buffer, &buffer_available);
+ if (buffer != NULL)
+(*memory_callback) (dwfl, -1, &buffer, &buffer_available, 0, 0,
+memory_callback_arg);
+
if (elf != NULL)
elf_end (elf);
if (fd != -1)
--
2.26.2
[PATCH 05/12] segment_report_module: Use a struct for build id information
From: Timm Bäder
Keep the three build id fields in a struct. This will be an important
clean up later.
Signed-off-by: Timm Bäder
---
libdwfl/dwfl_segment_report_module.c | 54
1 file changed, 31 insertions(+), 23 deletions(-)
diff --git a/libdwfl/dwfl_segment_report_module.c
b/libdwfl/dwfl_segment_report_module.c
index 01adfe9e..6abbf992 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -54,6 +54,13 @@
# define MY_ELFDATAELFDATA2MSB
#endif
+struct elf_build_id
+{
+ void *memory;
+ size_t len;
+ GElf_Addr vaddr;
+};
+
/* Return user segment index closest to ADDR but not above it.
If NEXT, return the closest to ADDR but not below it. */
@@ -206,16 +213,16 @@ handle_file_note (GElf_Addr module_start, GElf_Addr
module_end,
static bool
invalid_elf (Elf *elf, bool disk_file_has_build_id,
-const void *build_id, size_t build_id_len)
+ struct elf_build_id *build_id)
{
- if (! disk_file_has_build_id && build_id_len > 0)
+ if (! disk_file_has_build_id && build_id->len > 0)
{
/* Module found in segments with build-id is more reliable
than a module found via DT_DEBUG on disk without any
build-id. */
return true;
}
- if (disk_file_has_build_id && build_id_len > 0)
+ if (disk_file_has_build_id && build_id->len > 0)
{
const void *elf_build_id;
ssize_t elf_build_id_len;
@@ -224,8 +231,8 @@ invalid_elf (Elf *elf, bool disk_file_has_build_id,
elf_build_id_len = INTUSE(dwelf_elf_gnu_build_id) (elf, &elf_build_id);
if (elf_build_id_len > 0)
{
- if (build_id_len != (size_t) elf_build_id_len
- || memcmp (build_id, elf_build_id, build_id_len) != 0)
+ if (build_id->len != (size_t) elf_build_id_len
+ || memcmp (build_id->memory, elf_build_id, build_id->len) != 0)
return true;
}
}
@@ -442,16 +449,17 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
GElf_Xword dyn_filesz = 0;
/* Collect the build ID bits here. */
- void *build_id = NULL;
- size_t build_id_len = 0;
- GElf_Addr build_id_vaddr = 0;
+ struct elf_build_id build_id;
+ build_id.memory = NULL;
+ build_id.len = 0;
+ build_id.vaddr =0;
/* Consider a PT_NOTE we've found in the image. */
inline void consider_notes (GElf_Addr vaddr, GElf_Xword filesz,
GElf_Xword align)
{
/* If we have already seen a build ID, we don't care any more. */
-if (build_id != NULL || filesz == 0)
+if (build_id.memory != NULL || filesz == 0)
return;
void *data;
@@ -510,11 +518,11 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
&& nh->n_namesz == sizeof "GNU"
&& !memcmp (note_name, "GNU", sizeof "GNU"))
{
- build_id_vaddr = note_desc - (const void *) notes + vaddr;
- build_id_len = nh->n_descsz;
- build_id = malloc (nh->n_descsz);
- if (likely (build_id != NULL))
- memcpy (build_id, note_desc, build_id_len);
+build_id.vaddr = note_desc - (const void *) notes + vaddr;
+build_id.len = nh->n_descsz;
+build_id.memory = malloc (build_id.len);
+if (likely (build_id.memory != NULL))
+ memcpy (build_id.memory, note_desc, build_id.len);
break;
}
@@ -629,7 +637,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
header we read at START was not produced by these program headers. */
if (unlikely (!found_bias))
{
- free (build_id);
+ free (build_id.memory);
goto out;
}
@@ -684,7 +692,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
{
if (module->elf != NULL
&& invalid_elf (module->elf, module->disk_file_has_build_id,
- build_id, build_id_len))
+&build_id))
{
elf_end (module->elf);
close (module->fd);
@@ -700,7 +708,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
}
if (skip_this_module)
{
- free (build_id);
+ free (build_id.memory);
goto out;
}
}
@@ -719,7 +727,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
Dwfl_Error error = __libdw_open_file (&fd, &elf, true, false);
if (error == DWFL_E_NOERROR)
invalid = invalid_elf (elf, true /* disk_file_has_build_id */,
- build_id, build_id_len);
+ &build_id);
}
if (invalid)
{
@@ -867,11 +875,11 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
if (mod != NULL && (execlike || ehdr.e32.e_type == ET_EXEC))
mod->is_execut
[PATCH 06/12] segment_report_module: Pull consider_notes() into file scope
From: Timm Bäder
Signed-off-by: Timm Bäder
---
libdwfl/dwfl_segment_report_module.c | 187 +++
1 file changed, 101 insertions(+), 86 deletions(-)
diff --git a/libdwfl/dwfl_segment_report_module.c
b/libdwfl/dwfl_segment_report_module.c
index 6abbf992..6c6f9f37 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -282,6 +282,99 @@ read_portion (Dwfl *dwfl,
return false;
}
+
+/* Consider a PT_NOTE we've found in the image. */
+static inline void
+consider_notes (Dwfl *dwfl,
+Dwfl_Memory_Callback *memory_callback, void
*memory_callback_arg,
+GElf_Addr vaddr, GElf_Xword filesz,
+GElf_Xword align,
+void *buffer, size_t buffer_available,
+GElf_Addr start,
+size_t segment,
+unsigned char ei_data,
+struct elf_build_id *build_id,
+Elf_Data *xlatefrom,
+Elf_Data *xlateto,
+unsigned int xencoding)
+{
+ if (build_id->memory != NULL || filesz == 0)
+return;
+
+ void *data;
+ size_t data_size;
+ if (read_portion (dwfl, memory_callback, memory_callback_arg,
+&data, &data_size, vaddr, filesz,
+buffer, buffer_available, start, segment))
+return;
+
+ /* data_size will be zero if we got everything from the initial
+ buffer, otherwise it will be the size of the new buffer that
+ could be read. */
+ if (data_size != 0)
+filesz = data_size;
+
+ assert (sizeof (Elf32_Nhdr) == sizeof (Elf64_Nhdr));
+
+ void *notes;
+ if (ei_data == MY_ELFDATA)
+notes = data;
+ else
+{
+ notes = malloc (filesz);
+ if (unlikely (notes == NULL))
+return;
+ xlatefrom->d_type = xlateto->d_type = (align == 8
+ ? ELF_T_NHDR8 : ELF_T_NHDR);
+ xlatefrom->d_buf = (void *) data;
+ xlatefrom->d_size = filesz;
+ xlateto->d_buf = notes;
+ xlateto->d_size = filesz;
+ if (elf32_xlatetom (xlateto, xlatefrom, xencoding) == NULL)
+goto done;
+}
+
+ const GElf_Nhdr *nh = notes;
+ size_t len = 0;
+ while (filesz > len + sizeof (*nh))
+{
+ const void *note_name;
+ const void *note_desc;
+
+ len += sizeof (*nh);
+ note_name = notes + len;
+
+ len += nh->n_namesz;
+ len = align == 8 ? NOTE_ALIGN8 (len) : NOTE_ALIGN4 (len);
+ note_desc = notes + len;
+
+ if (unlikely (filesz < len + nh->n_descsz))
+break;
+
+ if (nh->n_type == NT_GNU_BUILD_ID
+ && nh->n_descsz > 0
+ && nh->n_namesz == sizeof "GNU"
+ && !memcmp (note_name, "GNU", sizeof "GNU"))
+{
+ build_id->vaddr = note_desc - (const void *) notes + vaddr;
+ build_id->len = nh->n_descsz;
+ build_id->memory = malloc (build_id->len);
+ if (likely (build_id->memory != NULL))
+memcpy (build_id->memory, note_desc, build_id->len);
+ break;
+}
+
+ len += nh->n_descsz;
+ len = align == 8 ? NOTE_ALIGN8 (len) : NOTE_ALIGN4 (len);
+ nh = (void *) notes + len;
+}
+
+done:
+ if (notes != data)
+free (notes);
+ finish_portion (dwfl, memory_callback, memory_callback_arg, &data,
&data_size);
+}
+
int
dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
Dwfl_Memory_Callback *memory_callback,
@@ -454,89 +547,6 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
build_id.len = 0;
build_id.vaddr =0;
- /* Consider a PT_NOTE we've found in the image. */
- inline void consider_notes (GElf_Addr vaddr, GElf_Xword filesz,
- GElf_Xword align)
- {
-/* If we have already seen a build ID, we don't care any more. */
-if (build_id.memory != NULL || filesz == 0)
- return;
-
-void *data;
-size_t data_size;
-if (read_portion (dwfl, memory_callback, memory_callback_arg,
- &data, &data_size, vaddr, filesz,
- buffer, buffer_available, start, segment))
- return;
-
-/* data_size will be zero if we got everything from the initial
- buffer, otherwise it will be the size of the new buffer that
- could be read. */
-if (data_size != 0)
- filesz = data_size;
-
-assert (sizeof (Elf32_Nhdr) == sizeof (Elf64_Nhdr));
-
-void *notes;
-if (ei_data == MY_ELFDATA)
- notes = data;
-else
- {
- notes = malloc (filesz);
- if (unlikely (notes == NULL))
- return;
- xlatefrom.d_type = xlateto.d_type = (align == 8
-? ELF_T_NHDR8 : ELF_T_NHDR);
- xlatefrom.d_buf = (void *) data;
- xlatefrom.d_size = filesz;
- xlateto.d_buf = notes;
- xlateto.d_size = filesz;
- if (elf32_xlatetom (&xlateto, &xlatefrom,
- ehdr.e32.e_ident[E
[PATCH 07/12] segment_report_module: Get rid of nested final_read() function
From: Timm Bäder
Signed-off-by: Timm Bäder
---
libdwfl/dwfl_segment_report_module.c | 25 ++---
1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/libdwfl/dwfl_segment_report_module.c
b/libdwfl/dwfl_segment_report_module.c
index 6c6f9f37..a69ead8f 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -934,15 +934,6 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
if (unlikely (contents == NULL))
goto out;
- inline void final_read (size_t offset, GElf_Addr vaddr, size_t size)
- {
- void *into = contents + offset;
- size_t read_size = size;
-(*memory_callback) (dwfl, addr_segndx (dwfl, segment, vaddr, false),
-&into, &read_size, vaddr, size,
-memory_callback_arg);
- }
-
if (contiguous < file_trimmed_end)
{
/* We can't use the memory image verbatim as the file image.
@@ -952,7 +943,13 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
GElf_Off offset, GElf_Xword filesz)
{
if (type == PT_LOAD)
- final_read (offset, vaddr + bias, filesz);
+ {
+void *into = contents + offset;
+size_t read_size = filesz;
+(*memory_callback) (dwfl, addr_segndx (dwfl, segment, vaddr +
bias, false),
+&into, &read_size, vaddr + bias, read_size,
+memory_callback_arg);
+ }
}
if (ei_class == ELFCLASS32)
@@ -973,7 +970,13 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
memcpy (contents, buffer, have);
if (have < file_trimmed_end)
- final_read (have, start + have, file_trimmed_end - have);
+{
+ void *into = contents + have;
+ size_t read_size = file_trimmed_end - have;
+(*memory_callback) (dwfl, addr_segndx (dwfl, segment, start +
have, false),
+&into, &read_size, start + have, read_size,
+memory_callback_arg);
+}
}
elf = elf_memory (contents, file_trimmed_end);
--
2.26.2
[PATCH 09/12] segment_report_module: Inline read_phdr() into only caller
From: Timm Bäder
There is now only one caller for this nested function, so get rid of it
by just inlining it there.
Signed-off-by: Timm Bäder
---
libdwfl/dwfl_segment_report_module.c | 24 +---
1 file changed, 9 insertions(+), 15 deletions(-)
diff --git a/libdwfl/dwfl_segment_report_module.c
b/libdwfl/dwfl_segment_report_module.c
index 276e9117..10212964 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -938,29 +938,23 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
{
/* We can't use the memory image verbatim as the file image.
So we'll be reading into a local image of the virtual file. */
-
- inline void read_phdr (GElf_Word type, GElf_Addr vaddr,
-GElf_Off offset, GElf_Xword filesz)
- {
- if (type == PT_LOAD)
- {
-void *into = contents + offset;
-size_t read_size = filesz;
-(*memory_callback) (dwfl, addr_segndx (dwfl, segment, vaddr +
bias, false),
-&into, &read_size, vaddr + bias, read_size,
-memory_callback_arg);
- }
- }
-
for (uint_fast16_t i = 0; i < phnum; ++i)
{
bool is32 = (ei_class == ELFCLASS32);
GElf_Word type = is32 ? p32[i].p_type : p64[i].p_type;
+
+ if (type != PT_LOAD)
+continue;
+
GElf_Addr vaddr = is32 ? p32[i].p_vaddr : p64[i].p_vaddr;
GElf_Off offset = is32 ? p32[i].p_offset : p64[i].p_offset;
GElf_Xword filesz = is32 ? p32[i].p_filesz : p64[i].p_filesz;
- read_phdr (type, vaddr, offset, filesz);
+ void *into = contents + offset;
+ size_t read_size = filesz;
+ (*memory_callback) (dwfl, addr_segndx (dwfl, segment, vaddr +
bias, false),
+ &into, &read_size, vaddr + bias, read_size,
+ memory_callback_arg);
}
}
else
--
2.26.2
[PATCH 12/12] segment_report_module: Inline consider_phdr() into only caller
From: Timm Bäder
Get rid of the nested function this way
Signed-off-by: Timm Bäder
---
libdwfl/dwfl_segment_report_module.c | 142 +--
1 file changed, 66 insertions(+), 76 deletions(-)
diff --git a/libdwfl/dwfl_segment_report_module.c
b/libdwfl/dwfl_segment_report_module.c
index 046d5381..26d4e80a 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -461,7 +461,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
/* NOTE if the number of sections is > 0xff00 then e_shnum
is zero and the actual number would come from the section
zero sh_size field. We ignore this here because getting shdrs
-is just a nice bonus (see below in consider_phdr PT_LOAD
+is just a nice bonus (see below in the type == PT_LOAD case
where we trim the last segment). */
shdrs_end = ehdr.e32.e_shoff + ehdr.e32.e_shnum * ehdr.e32.e_shentsize;
break;
@@ -547,80 +547,6 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
build_id.len = 0;
build_id.vaddr =0;
- /* Consider each of the program headers we've read from the image. */
- inline void consider_phdr (GElf_Word type,
-GElf_Addr vaddr, GElf_Xword memsz,
-GElf_Off offset, GElf_Xword filesz,
-GElf_Xword align)
- {
-switch (type)
- {
- case PT_DYNAMIC:
- dyn_vaddr = vaddr;
- dyn_filesz = filesz;
- break;
-
- case PT_NOTE:
- /* We calculate from the p_offset of the note segment,
- because we don't yet know the bias for its p_vaddr. */
- consider_notes (dwfl, memory_callback, memory_callback_arg,
- start + offset, filesz, align,
- buffer, buffer_available, start, segment,
- ei_data, &build_id,
- &xlatefrom, &xlateto,
- ehdr.e32.e_ident[EI_DATA]);
- break;
-
- case PT_LOAD:
- align = dwfl->segment_align > 1 ? dwfl->segment_align : align ?: 1;
-
- GElf_Addr vaddr_end = (vaddr + memsz + align - 1) & -align;
- GElf_Addr filesz_vaddr = filesz < memsz ? vaddr + filesz : vaddr_end;
- GElf_Off filesz_offset = filesz_vaddr - vaddr + offset;
-
- if (file_trimmed_end < offset + filesz)
- {
- file_trimmed_end = offset + filesz;
-
- /* Trim the last segment so we don't bother with zeros
- in the last page that are off the end of the file.
- However, if the extra bit in that page includes the
- section headers, keep them. */
- if (shdrs_end <= filesz_offset && shdrs_end > file_trimmed_end)
- {
- filesz += shdrs_end - file_trimmed_end;
- file_trimmed_end = shdrs_end;
- }
- }
-
- total_filesz += filesz;
-
- if (file_end < filesz_offset)
- {
- file_end = filesz_offset;
- if (filesz_vaddr - start == filesz_offset)
- contiguous = file_end;
- }
-
- if (!found_bias && (offset & -align) == 0
- && likely (filesz_offset >= phoff + phnum * phentsize))
- {
- bias = start - vaddr;
- found_bias = true;
- }
-
- if ((vaddr & -align) < module_start)
- {
- module_start = vaddr & -align;
- module_address_sync = vaddr + memsz;
- }
-
- if (module_end < vaddr_end)
- module_end = vaddr_end;
- break;
- }
- }
-
Elf32_Phdr *p32 = phdrsp;
Elf64_Phdr *p64 = phdrsp;
if ((ei_class == ELFCLASS32
@@ -632,6 +558,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
}
else
{
+ /* Consider each of the program headers we've read from the image. */
for (uint_fast16_t i = 0; i < phnum; ++i)
{
bool is32 = (ei_class == ELFCLASS32);
@@ -642,7 +569,70 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
GElf_Xword filesz = is32 ? p32[i].p_filesz : p64[i].p_filesz;
GElf_Xword align = is32 ? p32[i].p_align : p64[i].p_align;
- consider_phdr (type, vaddr, memsz, offset, filesz, align);
+ if (type == PT_DYNAMIC)
+{
+ dyn_vaddr = vaddr;
+ dyn_filesz = filesz;
+}
+ else if (type == PT_NOTE)
+{
+ /* We calculate from the p_offset of the note segment,
+ because we don't yet know the bias for its p_vaddr. */
+ consider_notes (dwfl, memory_callback, memory_callback_arg,
+ start + offset, filesz, align,
+ buffer, buffer_available, start, segment,
+ ei_data, &build_id,
+ &xlatefrom, &xlateto,
+
[PATCH 11/12] segment_report_module: Inline consider_dyn() into only caller
From: Timm Bäder
Signed-off-by: Timm Bäder
---
libdwfl/dwfl_segment_report_module.c | 40 +---
1 file changed, 12 insertions(+), 28 deletions(-)
diff --git a/libdwfl/dwfl_segment_report_module.c
b/libdwfl/dwfl_segment_report_module.c
index 8613ce21..046d5381 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -770,33 +770,6 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
GElf_Addr dynstr_vaddr = 0;
GElf_Xword dynstrsz = 0;
bool execlike = false;
- inline bool consider_dyn (GElf_Sxword tag, GElf_Xword val)
- {
-switch (tag)
- {
- default:
- return false;
-
- case DT_DEBUG:
- execlike = true;
- break;
-
- case DT_SONAME:
- soname_stroff = val;
- break;
-
- case DT_STRTAB:
- dynstr_vaddr = val;
- break;
-
- case DT_STRSZ:
- dynstrsz = val;
- break;
- }
-
-return soname_stroff != 0 && dynstr_vaddr != 0 && dynstrsz != 0;
- }
-
const size_t dyn_entsize = (ei_class == ELFCLASS32
? sizeof (Elf32_Dyn) : sizeof (Elf64_Dyn));
void *dyn_data = NULL;
@@ -834,7 +807,18 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
GElf_Sxword tag = is32 ? d32[i].d_tag : d64[i].d_tag;
GElf_Xword val = is32 ? d32[i].d_un.d_val : d64[i].d_un.d_val;
- if (consider_dyn (tag, val))
+ if (tag == DT_DEBUG)
+execlike = true;
+ else if (tag == DT_SONAME)
+soname_stroff = val;
+ else if (tag == DT_STRTAB)
+dynstr_vaddr = val;
+ else if (tag == DT_STRSZ)
+dynstrsz = val;
+ else
+continue;
+
+ if (soname_stroff != 0 && dynstr_vaddr != 0 && dynstrsz != 0)
break;
}
}
--
2.26.2
[PATCH 03/12] segment_report_module: Pull finish_portion() info file scope
From: Timm Bäder
Signed-off-by: Timm Bäder
---
libdwfl/dwfl_segment_report_module.c | 24 ++--
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/libdwfl/dwfl_segment_report_module.c
b/libdwfl/dwfl_segment_report_module.c
index 848c3bec..c55168ed 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -232,6 +232,16 @@ invalid_elf (Elf *elf, bool disk_file_has_build_id,
return false;
}
+static inline void
+finish_portion (Dwfl *dwfl,
+Dwfl_Memory_Callback *memory_callback,
+void *memory_callback_arg,
+void **data, size_t *data_size)
+{
+ if (*data_size != 0 && *data != NULL)
+(*memory_callback) (dwfl, -1, data, data_size, 0, 0, memory_callback_arg);
+}
+
int
dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
Dwfl_Memory_Callback *memory_callback,
@@ -297,12 +307,6 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
return false;
}
- inline void finish_portion (void **data, size_t *data_size)
- {
-if (*data_size != 0 && *data != NULL)
- (*memory_callback) (dwfl, -1, data, data_size, 0, 0,
memory_callback_arg);
- }
-
/* Extract the information we need from the file header. */
const unsigned char *e_ident;
unsigned char ei_class;
@@ -509,7 +513,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
done:
if (notes != data)
free (notes);
-finish_portion (&data, &data_size);
+finish_portion (dwfl, memory_callback, memory_callback_arg, &data,
&data_size);
}
/* Consider each of the program headers we've read from the image. */
@@ -606,7 +610,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
p64[i].p_align);
}
- finish_portion (&ph_buffer, &ph_buffer_size);
+ finish_portion (dwfl, memory_callback, memory_callback_arg, &ph_buffer,
&ph_buffer_size);
/* We must have seen the segment covering offset 0, or else the ELF
header we read at START was not produced by these program headers. */
@@ -798,7 +802,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
}
free (dyns);
}
- finish_portion (&dyn_data, &dyn_data_size);
+ finish_portion (dwfl, memory_callback, memory_callback_arg, &dyn_data,
&dyn_data_size);
/* We'll use the name passed in or a stupid default if not DT_SONAME. */
if (name == NULL)
@@ -859,7 +863,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
/* At this point we do not need BUILD_ID or NAME any more.
They have been copied. */
free (build_id);
- finish_portion (&soname, &soname_size);
+ finish_portion (dwfl, memory_callback, memory_callback_arg, &soname,
&soname_size);
if (unlikely (mod == NULL))
{
--
2.26.2
[PATCH 04/12] segment_report_module: Pull read_portion() into file scope
From: Timm Bäder
Signed-off-by: Timm Bäder
---
libdwfl/dwfl_segment_report_module.c | 77 +---
1 file changed, 47 insertions(+), 30 deletions(-)
diff --git a/libdwfl/dwfl_segment_report_module.c
b/libdwfl/dwfl_segment_report_module.c
index c55168ed..01adfe9e 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -242,6 +242,39 @@ finish_portion (Dwfl *dwfl,
(*memory_callback) (dwfl, -1, data, data_size, 0, 0, memory_callback_arg);
}
+
+static inline bool
+read_portion (Dwfl *dwfl,
+ Dwfl_Memory_Callback *memory_callback,
+ void *memory_callback_arg,
+ void **data, size_t *data_size,
+ GElf_Addr vaddr, size_t filesz,
+ void *buffer,
+ size_t buffer_available,
+ GElf_Addr start,
+ size_t segment)
+{
+ /* Check whether we will have to read the segment data, or if it
+ can be returned from the existing buffer. */
+ if (filesz > buffer_available
+ || vaddr - start > buffer_available - filesz
+ /* If we're in string mode, then don't consider the buffer we have
+ sufficient unless it contains the terminator of the string. */
+ || (filesz == 0 && memchr (vaddr - start + buffer, '\0',
+ buffer_available - (vaddr - start)) == NULL))
+{
+ *data = NULL;
+ *data_size = filesz;
+ return ! (*memory_callback) (dwfl, addr_segndx (dwfl, segment, vaddr,
false),
+ data, data_size, vaddr, filesz,
memory_callback_arg);
+}
+
+ /* We already have this whole note segment from our initial read. */
+ *data = vaddr - start + buffer;
+ *data_size = 0;
+ return false;
+}
+
int
dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
Dwfl_Memory_Callback *memory_callback,
@@ -283,30 +316,6 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
|| memcmp (buffer, ELFMAG, SELFMAG) != 0)
goto out;
- inline bool read_portion (void **data, size_t *data_size,
- GElf_Addr vaddr, size_t filesz)
- {
-/* Check whether we will have to read the segment data, or if it
- can be returned from the existing buffer. */
-if (filesz > buffer_available
- || vaddr - start > buffer_available - filesz
- /* If we're in string mode, then don't consider the buffer we have
- sufficient unless it contains the terminator of the string. */
- || (filesz == 0 && memchr (vaddr - start + buffer, '\0',
- buffer_available - (vaddr - start)) == NULL))
- {
- *data = NULL;
- *data_size = filesz;
-return ! (*memory_callback) (dwfl, addr_segndx (dwfl, segment, vaddr,
false),
- data, data_size, vaddr, filesz,
memory_callback_arg);
- }
-
-/* We already have this whole note segment from our initial read. */
-*data = vaddr - start + buffer;
-*data_size = 0;
-return false;
- }
-
/* Extract the information we need from the file header. */
const unsigned char *e_ident;
unsigned char ei_class;
@@ -387,8 +396,10 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
void *ph_buffer = NULL;
size_t ph_buffer_size = 0;
- if (read_portion (&ph_buffer, &ph_buffer_size,
- start + phoff, xlatefrom.d_size))
+ if (read_portion (dwfl, memory_callback, memory_callback_arg,
+&ph_buffer, &ph_buffer_size,
+start + phoff, xlatefrom.d_size,
+buffer, buffer_available, start, segment))
goto out;
/* ph_buffer_size will be zero if we got everything from the initial
@@ -445,7 +456,9 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
void *data;
size_t data_size;
-if (read_portion (&data, &data_size, vaddr, filesz))
+if (read_portion (dwfl, memory_callback, memory_callback_arg,
+ &data, &data_size, vaddr, filesz,
+ buffer, buffer_available, start, segment))
return;
/* data_size will be zero if we got everything from the initial
@@ -766,7 +779,9 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
void *dyn_data = NULL;
size_t dyn_data_size = 0;
if (dyn_filesz != 0 && dyn_filesz % dyn_entsize == 0
- && ! read_portion (&dyn_data, &dyn_data_size, dyn_vaddr, dyn_filesz))
+ && ! read_portion (dwfl, memory_callback, memory_callback_arg,
+ &dyn_data, &dyn_data_size, dyn_vaddr, dyn_filesz,
+ buffer, buffer_available, start, segment))
{
/* dyn_data_size will be zero if we got everything from the initial
buffer, otherwise it will be the size of the new buffer that
@@ -834,8 +849,10 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
[PATCH 08/12] segment_report_module: Use one loop for p32/p64 arrays
From: Timm Bäder
Do one loop check for 32/64 bit inside the loop, instead of outside.
This way we have only one call site for the function called in the loop
body.
Signed-off-by: Timm Bäder
---
libdwfl/dwfl_segment_report_module.c | 52 +++-
1 file changed, 27 insertions(+), 25 deletions(-)
diff --git a/libdwfl/dwfl_segment_report_module.c
b/libdwfl/dwfl_segment_report_module.c
index a69ead8f..276e9117 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -623,27 +623,27 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
Elf32_Phdr *p32 = phdrsp;
Elf64_Phdr *p64 = phdrsp;
- if (ei_class == ELFCLASS32)
+ if ((ei_class == ELFCLASS32
+ && elf32_xlatetom (&xlateto, &xlatefrom, ei_data) == NULL)
+ || (ei_class == ELFCLASS64
+ && elf64_xlatetom (&xlateto, &xlatefrom, ei_data) == NULL))
{
- if (elf32_xlatetom (&xlateto, &xlatefrom, ei_data) == NULL)
- found_bias = false; /* Trigger error check. */
- else
- for (uint_fast16_t i = 0; i < phnum; ++i)
- consider_phdr (p32[i].p_type,
-p32[i].p_vaddr, p32[i].p_memsz,
-p32[i].p_offset, p32[i].p_filesz,
-p32[i].p_align);
+ found_bias = false; /* Trigger error check */
}
else
{
- if (elf64_xlatetom (&xlateto, &xlatefrom, ei_data) == NULL)
- found_bias = false; /* Trigger error check. */
- else
- for (uint_fast16_t i = 0; i < phnum; ++i)
- consider_phdr (p64[i].p_type,
-p64[i].p_vaddr, p64[i].p_memsz,
-p64[i].p_offset, p64[i].p_filesz,
-p64[i].p_align);
+ for (uint_fast16_t i = 0; i < phnum; ++i)
+{
+ bool is32 = (ei_class == ELFCLASS32);
+ GElf_Word type = is32 ? p32[i].p_type : p64[i].p_type;
+ GElf_Addr vaddr = is32 ? p32[i].p_vaddr : p64[i].p_vaddr;
+ GElf_Xword memsz = is32 ? p32[i].p_memsz : p64[i].p_memsz;
+ GElf_Off offset = is32 ? p32[i].p_offset : p64[i].p_offset;
+ GElf_Xword filesz = is32 ? p32[i].p_filesz : p64[i].p_filesz;
+ GElf_Xword align = is32 ? p32[i].p_align : p64[i].p_align;
+
+ consider_phdr (type, vaddr, memsz, offset, filesz, align);
+}
}
finish_portion (dwfl, memory_callback, memory_callback_arg, &ph_buffer,
&ph_buffer_size);
@@ -952,14 +952,16 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
}
}
- if (ei_class == ELFCLASS32)
- for (uint_fast16_t i = 0; i < phnum; ++i)
- read_phdr (p32[i].p_type, p32[i].p_vaddr,
-p32[i].p_offset, p32[i].p_filesz);
- else
- for (uint_fast16_t i = 0; i < phnum; ++i)
- read_phdr (p64[i].p_type, p64[i].p_vaddr,
-p64[i].p_offset, p64[i].p_filesz);
+ for (uint_fast16_t i = 0; i < phnum; ++i)
+{
+ bool is32 = (ei_class == ELFCLASS32);
+ GElf_Word type = is32 ? p32[i].p_type : p64[i].p_type;
+ GElf_Addr vaddr = is32 ? p32[i].p_vaddr : p64[i].p_vaddr;
+ GElf_Off offset = is32 ? p32[i].p_offset : p64[i].p_offset;
+ GElf_Xword filesz = is32 ? p32[i].p_filesz : p64[i].p_filesz;
+
+ read_phdr (type, vaddr, offset, filesz);
+}
}
else
{
--
2.26.2
[PATCH 10/12] segment_report_module: Unify d32/d64 loops
From: Timm Bäder
Just like we did before, use only one loop here and check for 32/64 bit
in the loop body. This way we only have one call site for consider_dyn
Signed-off-by: Timm Bäder
---
libdwfl/dwfl_segment_report_module.c | 28 ++--
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/libdwfl/dwfl_segment_report_module.c
b/libdwfl/dwfl_segment_report_module.c
index 10212964..8613ce21 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -824,20 +824,20 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
xlateto.d_buf = dyns;
xlateto.d_size = dyn_filesz;
- if (ei_class == ELFCLASS32)
- {
- if (elf32_xlatetom (&xlateto, &xlatefrom, ei_data) != NULL)
- for (size_t i = 0; i < dyn_filesz / sizeof (Elf32_Dyn); ++i)
- if (consider_dyn (d32[i].d_tag, d32[i].d_un.d_val))
- break;
- }
- else
- {
- if (elf64_xlatetom (&xlateto, &xlatefrom, ei_data) != NULL)
- for (size_t i = 0; i < dyn_filesz / sizeof (Elf64_Dyn); ++i)
- if (consider_dyn (d64[i].d_tag, d64[i].d_un.d_val))
- break;
- }
+ bool is32 = (ei_class == ELFCLASS32);
+ if ((is32 && elf32_xlatetom (&xlateto, &xlatefrom, ei_data) != NULL)
+ || (!is32 && elf64_xlatetom (&xlateto, &xlatefrom, ei_data) != NULL))
+{
+ size_t n = is32 ? (dyn_filesz / sizeof (Elf32_Dyn)) : (dyn_filesz /
sizeof (Elf64_Dyn));
+ for (size_t i = 0; i < n; ++i)
+{
+ GElf_Sxword tag = is32 ? d32[i].d_tag : d64[i].d_tag;
+ GElf_Xword val = is32 ? d32[i].d_un.d_val : d64[i].d_un.d_val;
+
+ if (consider_dyn (tag, val))
+break;
+}
+}
free (dyns);
}
finish_portion (dwfl, memory_callback, memory_callback_arg, &dyn_data,
&dyn_data_size);
--
2.26.2
Re: Remove nested functions from libdwfl V2
On 25/11/2020 17:33, Mark Wielaard wrote:
Hi Timm,
On Mon, 2020-11-23 at 13:27 +0100, Timm Bäder via Elfutils-devel wrote:
version 2 of this patch set. I removed segmend_read() entirely now,
which meant modifying a bunch of later patches. Other than that, they
are the same.
Hope the --from to git send-email worked out, too.
It did, thanks.
I immediately picked up 9 of these patches that clearly looked like
they improved the code. I added ChangeLog entries and might have
slightly tweaked the large lines to be a little shorter (also fixed up
some tab vs space indents, but the file wasn't really consistent to
begin with).
The last three I skipped for now were:
- segment_report_module: Pull finish_portion() info file scope
- segment_report_module: Pull read_portion() into file scope
- segment_report_module: Pull consider_notes() into file scope
The first two aren't so bad, but maybe we can find a way to not pass so
many arguments around (have a state struct with dwfl,
memory_callback[_arg], data and size maybe?)
I also initially did the state struct, i.e.
struct read_state
{
Dwfl *dwfl;
Dwfl_Memory_Callback *memory_callback;
void *memory_callback_arg;
void **buffer;
size_t *buffer_available;
};
but we usually pass a data+data_size pair around separately. I now have
a patch with this struct anyway.
consider_notes might be better just inlined because it is used only
once.
Saw that now too and inlined it.
I'll send the patches soon.
Thanks,
Timm
--
Red Hat GmbH, http://www.de.redhat.com/, Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Michael O'Neill, Tom Savage, Eric
Shander
Remove remaining nested functions from libdwfl
Here are the three patches to remove the three remaining nested functions from libdwfl/dwfl_segment_report_module.c.
[PATCH 1/3] segment_report_module: Pull finish_portion() into file scope
From: Timm Bäder
Use a read_state struct here to minimize the amount of parameters we
pass.
---
libdwfl/dwfl_segment_report_module.c | 38
1 file changed, 28 insertions(+), 10 deletions(-)
diff --git a/libdwfl/dwfl_segment_report_module.c
b/libdwfl/dwfl_segment_report_module.c
index 7e747184..391fd761 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -61,6 +61,14 @@ struct elf_build_id
GElf_Addr vaddr;
};
+struct read_state
+{
+ Dwfl *dwfl;
+ Dwfl_Memory_Callback *memory_callback;
+ void *memory_callback_arg;
+ void **buffer;
+ size_t *buffer_available;
+};
/* Return user segment index closest to ADDR but not above it.
If NEXT, return the closest to ADDR but not below it. */
@@ -239,6 +247,15 @@ invalid_elf (Elf *elf, bool disk_file_has_build_id,
return false;
}
+static void
+finish_portion (struct read_state *read_state,
+ void **data, size_t *data_size)
+{
+ if (*data_size != 0 && *data != NULL)
+(*read_state->memory_callback) (read_state->dwfl, -1, data, data_size,
+ 0, 0, read_state->memory_callback_arg);
+}
+
int
dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
Dwfl_Memory_Callback *memory_callback,
@@ -249,6 +266,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
const struct r_debug_info *r_debug_info)
{
size_t segment = ndx;
+ struct read_state read_state;
if (segment >= dwfl->lookup_elts)
segment = dwfl->lookup_elts - 1;
@@ -271,6 +289,12 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
Elf *elf = NULL;
int fd = -1;
+ read_state.dwfl = dwfl;
+ read_state.memory_callback = memory_callback;
+ read_state.memory_callback_arg = memory_callback_arg;
+ read_state.buffer = &buffer;
+ read_state.buffer_available = &buffer_available;
+
/* We might have to reserve some memory for the phdrs. Set to NULL
here so we can always safely free it. */
void *phdrsp = NULL;
@@ -306,12 +330,6 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
return false;
}
- inline void finish_portion (void **data, size_t *data_size)
- {
-if (*data_size != 0 && *data != NULL)
- (*memory_callback) (dwfl, -1, data, data_size, 0, 0,
memory_callback_arg);
- }
-
/* Extract the information we need from the file header. */
const unsigned char *e_ident;
unsigned char ei_class;
@@ -519,7 +537,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
done:
if (notes != data)
free (notes);
-finish_portion (&data, &data_size);
+finish_portion (&read_state, &data, &data_size);
}
Elf32_Phdr *p32 = phdrsp;
@@ -609,7 +627,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
}
}
- finish_portion (&ph_buffer, &ph_buffer_size);
+ finish_portion (&read_state, &ph_buffer, &ph_buffer_size);
/* We must have seen the segment covering offset 0, or else the ELF
header we read at START was not produced by these program headers. */
@@ -787,7 +805,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
}
free (dyns);
}
- finish_portion (&dyn_data, &dyn_data_size);
+ finish_portion (&read_state, &dyn_data, &dyn_data_size);
/* We'll use the name passed in or a stupid default if not DT_SONAME. */
if (name == NULL)
@@ -848,7 +866,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
/* At this point we do not need BUILD_ID or NAME any more.
They have been copied. */
free (build_id.memory);
- finish_portion (&soname, &soname_size);
+ finish_portion (&read_state, &soname, &soname_size);
if (unlikely (mod == NULL))
{
--
2.26.2
[PATCH 2/3] segment_report_module: Pull read_portion() into file scope
From: Timm Bäder
---
libdwfl/dwfl_segment_report_module.c | 65 +++-
1 file changed, 35 insertions(+), 30 deletions(-)
diff --git a/libdwfl/dwfl_segment_report_module.c
b/libdwfl/dwfl_segment_report_module.c
index 391fd761..a082886a 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -256,6 +256,35 @@ finish_portion (struct read_state *read_state,
0, 0, read_state->memory_callback_arg);
}
+static inline bool
+read_portion (struct read_state *read_state,
+ void **data, size_t *data_size,
+ GElf_Addr start, size_t segment,
+ GElf_Addr vaddr, size_t filesz)
+{
+ /* Check whether we will have to read the segment data, or if it
+ can be returned from the existing buffer. */
+ if (filesz > *read_state->buffer_available
+ || vaddr - start > *read_state->buffer_available - filesz
+ /* If we're in string mode, then don't consider the buffer we have
+sufficient unless it contains the terminator of the string. */
+ || (filesz == 0 && memchr (vaddr - start + *read_state->buffer, '\0',
+*read_state->buffer_available - (vaddr -
start)) == NULL))
+{
+ *data = NULL;
+ *data_size = filesz;
+ return ! (*read_state->memory_callback) (read_state->dwfl,
+ addr_segndx (read_state->dwfl,
segment, vaddr, false),
+ data, data_size, vaddr, filesz,
+ read_state->memory_callback_arg);
+}
+
+ /* We already have this whole note segment from our initial read. */
+ *data = vaddr - start + (*read_state->buffer);
+ *data_size = 0;
+ return false;
+}
+
int
dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
Dwfl_Memory_Callback *memory_callback,
@@ -304,32 +333,6 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
|| memcmp (buffer, ELFMAG, SELFMAG) != 0)
goto out;
- inline bool read_portion (void **data, size_t *data_size,
- GElf_Addr vaddr, size_t filesz)
- {
-/* Check whether we will have to read the segment data, or if it
- can be returned from the existing buffer. */
-if (filesz > buffer_available
- || vaddr - start > buffer_available - filesz
- /* If we're in string mode, then don't consider the buffer we have
- sufficient unless it contains the terminator of the string. */
- || (filesz == 0 && memchr (vaddr - start + buffer, '\0',
- buffer_available - (vaddr - start)) == NULL))
- {
- *data = NULL;
- *data_size = filesz;
- return ! (*memory_callback) (dwfl, addr_segndx (dwfl, segment,
- vaddr, false),
-data, data_size, vaddr, filesz,
-memory_callback_arg);
- }
-
-/* We already have this whole note segment from our initial read. */
-*data = vaddr - start + buffer;
-*data_size = 0;
-return false;
- }
-
/* Extract the information we need from the file header. */
const unsigned char *e_ident;
unsigned char ei_class;
@@ -410,7 +413,8 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
void *ph_buffer = NULL;
size_t ph_buffer_size = 0;
- if (read_portion (&ph_buffer, &ph_buffer_size,
+ if (read_portion (&read_state, &ph_buffer, &ph_buffer_size,
+ start, segment,
start + phoff, xlatefrom.d_size))
goto out;
@@ -469,7 +473,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
void *data;
size_t data_size;
-if (read_portion (&data, &data_size, vaddr, filesz))
+if (read_portion (&read_state, &data, &data_size, start, segment, vaddr,
filesz))
return;
/* data_size will be zero if we got everything from the initial
@@ -756,7 +760,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
void *dyn_data = NULL;
size_t dyn_data_size = 0;
if (dyn_filesz != 0 && dyn_filesz % dyn_entsize == 0
- && ! read_portion (&dyn_data, &dyn_data_size, dyn_vaddr, dyn_filesz))
+ && ! read_portion (&read_state, &dyn_data, &dyn_data_size, start,
segment, dyn_vaddr, dyn_filesz))
{
/* dyn_data_size will be zero if we got everything from the initial
buffer, otherwise it will be the size of the new buffer that
@@ -837,7 +841,8 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
/* Try to get the DT_SONAME string. */
if (soname_stroff != 0 && soname_stroff + 1 < dynstrsz
- && ! read_portion (&soname, &soname_size,
+ && ! read_portion (&read_state, &soname, &soname_size,
+start, segment,
[PATCH 3/3] segment_report_module: Inline consider_notes() into only caller
From: Timm Bäder
Get rid of a nested function this way.
---
libdwfl/dwfl_segment_report_module.c | 162 +--
1 file changed, 80 insertions(+), 82 deletions(-)
diff --git a/libdwfl/dwfl_segment_report_module.c
b/libdwfl/dwfl_segment_report_module.c
index a082886a..c48d9ab7 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -463,87 +463,6 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
build_id.len = 0;
build_id.vaddr =0;
- /* Consider a PT_NOTE we've found in the image. */
- inline void consider_notes (GElf_Addr vaddr, GElf_Xword filesz,
- GElf_Xword align)
- {
-/* If we have already seen a build ID, we don't care any more. */
-if (build_id.memory != NULL || filesz == 0)
- return;
-
-void *data;
-size_t data_size;
-if (read_portion (&read_state, &data, &data_size, start, segment, vaddr,
filesz))
- return;
-
-/* data_size will be zero if we got everything from the initial
- buffer, otherwise it will be the size of the new buffer that
- could be read. */
-if (data_size != 0)
- filesz = data_size;
-
-assert (sizeof (Elf32_Nhdr) == sizeof (Elf64_Nhdr));
-
-void *notes;
-if (ei_data == MY_ELFDATA)
- notes = data;
-else
- {
- notes = malloc (filesz);
- if (unlikely (notes == NULL))
- return;
- xlatefrom.d_type = xlateto.d_type = (align == 8
-? ELF_T_NHDR8 : ELF_T_NHDR);
- xlatefrom.d_buf = (void *) data;
- xlatefrom.d_size = filesz;
- xlateto.d_buf = notes;
- xlateto.d_size = filesz;
- if (elf32_xlatetom (&xlateto, &xlatefrom,
- ehdr.e32.e_ident[EI_DATA]) == NULL)
- goto done;
- }
-
-const GElf_Nhdr *nh = notes;
-size_t len = 0;
-while (filesz > len + sizeof (*nh))
- {
- const void *note_name;
- const void *note_desc;
-
- len += sizeof (*nh);
- note_name = notes + len;
-
- len += nh->n_namesz;
- len = align == 8 ? NOTE_ALIGN8 (len) : NOTE_ALIGN4 (len);
- note_desc = notes + len;
-
- if (unlikely (filesz < len + nh->n_descsz))
- break;
-
-if (nh->n_type == NT_GNU_BUILD_ID
- && nh->n_descsz > 0
- && nh->n_namesz == sizeof "GNU"
- && !memcmp (note_name, "GNU", sizeof "GNU"))
- {
- build_id.vaddr = note_desc - (const void *) notes + vaddr;
- build_id.len = nh->n_descsz;
- build_id.memory = malloc (build_id.len);
- if (likely (build_id.memory != NULL))
- memcpy (build_id.memory, note_desc, build_id.len);
- break;
- }
-
- len += nh->n_descsz;
- len = align == 8 ? NOTE_ALIGN8 (len) : NOTE_ALIGN4 (len);
- nh = (void *) notes + len;
- }
-
- done:
-if (notes != data)
- free (notes);
-finish_portion (&read_state, &data, &data_size);
- }
-
Elf32_Phdr *p32 = phdrsp;
Elf64_Phdr *p64 = phdrsp;
if ((ei_class == ELFCLASS32
@@ -573,9 +492,88 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
}
else if (type == PT_NOTE)
{
+ /* If we have already seen a build ID, we don't care any more.
*/
+ if (build_id.memory != NULL || filesz == 0)
+continue; /* Next header */
+
/* We calculate from the p_offset of the note segment,
because we don't yet know the bias for its p_vaddr. */
- consider_notes ( start + offset, filesz, align);
+ const size_t note_vaddr = start + offset;
+ void *data;
+ size_t data_size;
+ if (read_portion (&read_state, &data, &data_size, start,
segment, note_vaddr, filesz))
+continue; /* Next header */
+
+ /* data_size will be zero if we got everything from the initial
+ buffer, otherwise it will be the size of the new buffer that
+ could be read. */
+ if (data_size != 0)
+filesz = data_size;
+
+ assert (sizeof (Elf32_Nhdr) == sizeof (Elf64_Nhdr));
+
+ void *notes;
+ if (ei_data == MY_ELFDATA)
+notes = data;
+ else
+{
+ const unsigned int xencoding = ehdr.e32.e_ident[EI_DATA];
+
+ notes = malloc (filesz);
+ if (unlikely (notes == NULL))
+continue; /* Next header */
+ xlatefrom.d_type = xlateto.d_type = (align == 8
+ ? ELF_T_NHDR8 :
ELF_T_NHDR);
+ xlatefrom.d_buf = (void *) data;
+ xlatefrom.d_size = filesz;
+ xlateto.d_buf = notes;
+ xlateto.d_size = filesz;
+
link_map: Remove nested functions
Hi, the attached patches get rid of nested functions in libdwfl/link_map.c. I wrote these a while back and just looked at them again and we could use the same read_state struct here as well. I just quickly checked, but it seems a bit more involved due to the integrated_memory_callback handling. I can look into that anyway if required. Thanks, Timm
[PATCH 1/3] link_map: Pull release_buffer() into file scope
From: Timm Bäder
Get rid of another nested function
---
libdwfl/link_map.c | 47 +++---
1 file changed, 32 insertions(+), 15 deletions(-)
diff --git a/libdwfl/link_map.c b/libdwfl/link_map.c
index 29307c74..5c39c631 100644
--- a/libdwfl/link_map.c
+++ b/libdwfl/link_map.c
@@ -225,6 +225,18 @@ addrsize (uint_fast8_t elfclass)
return elfclass * 4;
}
+static inline int
+release_buffer (Dwfl *dwfl,
+Dwfl_Memory_Callback *memory_callback, void
*memory_callback_arg,
+void **buffer, size_t *buffer_available,
+int result)
+{
+ if (buffer != NULL)
+(void) (*memory_callback) (dwfl, -1, buffer, buffer_available, 0, 0,
+ memory_callback_arg);
+ return result;
+}
+
/* Report a module for each struct link_map in the linked list at r_map
in the struct r_debug at R_DEBUG_VADDR. For r_debug_info description
see dwfl_link_map_report in libdwflP.h. If R_DEBUG_INFO is not NULL then no
@@ -249,13 +261,6 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t
elfdata,
void *buffer = NULL;
size_t buffer_available = 0;
- inline int release_buffer (int result)
- {
-if (buffer != NULL)
- (void) (*memory_callback) (dwfl, -1, &buffer, &buffer_available, 0, 0,
-memory_callback_arg);
-return result;
- }
GElf_Addr addrs[4];
inline bool read_addrs (GElf_Addr vaddr, size_t n)
@@ -267,7 +272,8 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t elfdata,
|| vaddr < read_vaddr
|| vaddr - read_vaddr + nb > buffer_available)
{
- release_buffer (0);
+ release_buffer (dwfl, memory_callback, memory_callback_arg,
+&buffer, &buffer_available, 0);
read_vaddr = vaddr;
int segndx = INTUSE(dwfl_addrsegment) (dwfl, vaddr, NULL);
@@ -304,7 +310,9 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t elfdata,
}
if (unlikely (read_addrs (read_vaddr, 1)))
-return release_buffer (-1);
+release_buffer (dwfl, memory_callback, memory_callback_arg,
+&buffer, &buffer_available, -1);
+
GElf_Addr next = addrs[0];
@@ -319,7 +327,8 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t elfdata,
while (next != 0 && ++iterations < dwfl->lookup_elts)
{
if (read_addrs (next, 4))
- return release_buffer (-1);
+return release_buffer (dwfl, memory_callback, memory_callback_arg,
+ &buffer, &buffer_available, -1);
/* Unused: l_addr is the difference between the address in memory
and the ELF file when the core was created. We need to
@@ -345,7 +354,9 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t elfdata,
name = l_name - read_vaddr + buffer;
else
{
- release_buffer (0);
+ release_buffer (dwfl, memory_callback, memory_callback_arg,
+ &buffer, &buffer_available, 0);
+
read_vaddr = l_name;
int segndx = INTUSE(dwfl_addrsegment) (dwfl, l_name, NULL);
if (likely (segndx >= 0)
@@ -372,7 +383,9 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t elfdata,
r_debug_info_module = malloc (sizeof (*r_debug_info_module)
+ strlen (name1) + 1);
if (unlikely (r_debug_info_module == NULL))
- return release_buffer (result);
+return release_buffer (dwfl, memory_callback, memory_callback_arg,
+ &buffer, &buffer_available, result);
+
r_debug_info_module->fd = -1;
r_debug_info_module->elf = NULL;
r_debug_info_module->l_ld = l_ld;
@@ -413,7 +426,9 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t elfdata,
GElf_Addr build_id_vaddr = (build_id_elfaddr
- elf_dynamic_vaddr + l_ld);
- release_buffer (0);
+ release_buffer (dwfl, memory_callback,
memory_callback_arg,
+ &buffer, &buffer_available, 0);
+
int segndx = INTUSE(dwfl_addrsegment) (dwfl,
build_id_vaddr,
NULL);
@@ -432,7 +447,8 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t elfdata,
/* File has valid build-id which does not match
the one in memory. */
valid = false;
- release_buffer (0);
+ release_buffer (dwfl, memory_callback,
memory_callback_arg,
+ &buffer, &buffer_available, 0);
}
}
@@ -498,7 +514,8 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t elfdata,
}
[PATCH 2/3] link_map: Pull read_addrs() in file scope
From: Timm Bäder
Get rid of another nested function this way
---
libdwfl/link_map.c | 114 ++---
1 file changed, 66 insertions(+), 48 deletions(-)
diff --git a/libdwfl/link_map.c b/libdwfl/link_map.c
index 5c39c631..64baaec4 100644
--- a/libdwfl/link_map.c
+++ b/libdwfl/link_map.c
@@ -237,6 +237,64 @@ release_buffer (Dwfl *dwfl,
return result;
}
+static inline bool
+read_addrs (Dwfl *dwfl,
+Dwfl_Memory_Callback *memory_callback,
+void *memory_callback_arg,
+void **buffer,
+size_t *buffer_available,
+GElf_Addr vaddr, GElf_Addr *read_vaddr,
+size_t n,
+uint_fast8_t elfclass,
+uint_fast8_t elfdata,
+GElf_Addr *addrs /* [4] */)
+{
+ size_t nb = n * addrsize (elfclass); /* Address words -> bytes to read. */
+
+ /* Read a new buffer if the old one doesn't cover these words. */
+ if (buffer == NULL
+ || vaddr < *read_vaddr
+ || vaddr - (*read_vaddr) + nb > *buffer_available)
+{
+ release_buffer (dwfl, memory_callback, memory_callback_arg,
+ buffer, buffer_available, 0);
+
+ *read_vaddr = vaddr;
+ int segndx = INTUSE(dwfl_addrsegment) (dwfl, vaddr, NULL);
+ if (unlikely (segndx < 0)
+ || unlikely (! (*memory_callback) (dwfl, segndx,
+ buffer, buffer_available,
+ vaddr, nb, memory_callback_arg)))
+return true;
+}
+
+ Elf32_Addr (*a32)[n] = vaddr - (*read_vaddr) + *buffer;
+ Elf64_Addr (*a64)[n] = (void *) a32;
+
+ if (elfclass == ELFCLASS32)
+{
+ if (elfdata == ELFDATA2MSB)
+for (size_t i = 0; i < n; ++i)
+ addrs[i] = BE32 (read_4ubyte_unaligned_noncvt (&(*a32)[i]));
+ else
+for (size_t i = 0; i < n; ++i)
+ addrs[i] = LE32 (read_4ubyte_unaligned_noncvt (&(*a32)[i]));
+}
+ else
+{
+ if (elfdata == ELFDATA2MSB)
+for (size_t i = 0; i < n; ++i)
+ addrs[i] = BE64 (read_8ubyte_unaligned_noncvt (&(*a64)[i]));
+ else
+for (size_t i = 0; i < n; ++i)
+ addrs[i] = LE64 (read_8ubyte_unaligned_noncvt (&(*a64)[i]));
+}
+
+ return false;
+}
+
+
+
/* Report a module for each struct link_map in the linked list at r_map
in the struct r_debug at R_DEBUG_VADDR. For r_debug_info description
see dwfl_link_map_report in libdwflP.h. If R_DEBUG_INFO is not NULL then no
@@ -262,54 +320,12 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t
elfdata,
void *buffer = NULL;
size_t buffer_available = 0;
- GElf_Addr addrs[4];
- inline bool read_addrs (GElf_Addr vaddr, size_t n)
- {
-size_t nb = n * addrsize (elfclass); /* Address words -> bytes to read. */
-
-/* Read a new buffer if the old one doesn't cover these words. */
-if (buffer == NULL
- || vaddr < read_vaddr
- || vaddr - read_vaddr + nb > buffer_available)
- {
- release_buffer (dwfl, memory_callback, memory_callback_arg,
-&buffer, &buffer_available, 0);
-
- read_vaddr = vaddr;
- int segndx = INTUSE(dwfl_addrsegment) (dwfl, vaddr, NULL);
- if (unlikely (segndx < 0)
- || unlikely (! (*memory_callback) (dwfl, segndx,
- &buffer, &buffer_available,
- vaddr, nb, memory_callback_arg)))
- return true;
- }
-
-Elf32_Addr (*a32)[n] = vaddr - read_vaddr + buffer;
-Elf64_Addr (*a64)[n] = (void *) a32;
-
-if (elfclass == ELFCLASS32)
- {
- if (elfdata == ELFDATA2MSB)
- for (size_t i = 0; i < n; ++i)
- addrs[i] = BE32 (read_4ubyte_unaligned_noncvt (&(*a32)[i]));
- else
- for (size_t i = 0; i < n; ++i)
- addrs[i] = LE32 (read_4ubyte_unaligned_noncvt (&(*a32)[i]));
- }
-else
- {
- if (elfdata == ELFDATA2MSB)
- for (size_t i = 0; i < n; ++i)
- addrs[i] = BE64 (read_8ubyte_unaligned_noncvt (&(*a64)[i]));
- else
- for (size_t i = 0; i < n; ++i)
- addrs[i] = LE64 (read_8ubyte_unaligned_noncvt (&(*a64)[i]));
- }
-
-return false;
- }
+ GElf_Addr addrs[4] = { 0, 0, 0, 0 };
- if (unlikely (read_addrs (read_vaddr, 1)))
+ if (unlikely (read_addrs (dwfl, memory_callback, memory_callback_arg,
+&buffer, &buffer_available,
+read_vaddr, &read_vaddr, 1,
+elfclass, elfdata, addrs)))
release_buffer (dwfl, memory_callback, memory_callback_arg,
&buffer, &buffer_available, -1);
@@ -326,7 +342,9 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t elfdata,
size_t iterations = 0;
while (next != 0 && ++iterations < dwfl->lookup_elts)
{
- if (read_addrs (next, 4))
+ if (read_addrs (dwfl, memory_callback, memory_callba
[PATCH 3/3] link_map: Inline consider_phdr() into only caller
From: Timm Bäder
This gets rid of the tested function and is shorter.
---
libdwfl/link_map.c | 66 ++
1 file changed, 26 insertions(+), 40 deletions(-)
diff --git a/libdwfl/link_map.c b/libdwfl/link_map.c
index 64baaec4..8a19f358 100644
--- a/libdwfl/link_map.c
+++ b/libdwfl/link_map.c
@@ -793,31 +793,6 @@ dwfl_link_map_report (Dwfl *dwfl, const void *auxv, size_t
auxv_size,
GElf_Xword dyn_filesz = 0;
GElf_Addr dyn_bias = (GElf_Addr) -1;
- inline bool consider_phdr (GElf_Word type,
-GElf_Addr vaddr, GElf_Xword filesz)
- {
- switch (type)
- {
- case PT_PHDR:
- if (dyn_bias == (GElf_Addr) -1
- /* Do a sanity check on the putative address. */
- && ((vaddr & (dwfl->segment_align - 1))
- == (phdr & (dwfl->segment_align - 1
- {
- dyn_bias = phdr - vaddr;
- return dyn_vaddr != 0;
- }
- break;
-
- case PT_DYNAMIC:
- dyn_vaddr = vaddr;
- dyn_filesz = filesz;
- return dyn_bias != (GElf_Addr) -1;
- }
-
- return false;
- }
-
if (phdr != 0 && phnum != 0)
{
Dwfl_Module *phdr_mod;
@@ -930,22 +905,33 @@ dwfl_link_map_report (Dwfl *dwfl, const void *auxv,
size_t auxv_size,
? elf32_xlatetom : elf64_xlatetom)
(&out, &in, elfdata) != NULL))
{
- /* We are looking for PT_DYNAMIC. */
- if (elfclass == ELFCLASS32)
+ bool is32 = (elfclass == ELFCLASS32);
+ for (size_t i = 0; i < phnum; ++i)
{
- for (size_t i = 0; i < phnum; ++i)
- if (consider_phdr ((*p32)[i].p_type,
- (*p32)[i].p_vaddr,
- (*p32)[i].p_filesz))
- break;
- }
- else
- {
- for (size_t i = 0; i < phnum; ++i)
- if (consider_phdr ((*p64)[i].p_type,
- (*p64)[i].p_vaddr,
- (*p64)[i].p_filesz))
- break;
+ GElf_Word type = is32 ? (*p32)[i].p_type :
(*p64)[i].p_type;
+ GElf_Addr vaddr = is32 ? (*p32)[i].p_vaddr :
(*p64)[i].p_vaddr;
+ GElf_Xword filesz = is32 ? (*p32)[i].p_filesz :
(*p64)[i].p_filesz;
+
+ if (type == PT_PHDR)
+ {
+ if (dyn_bias == (GElf_Addr) -1
+ /* Do a sanity check on the putative address. */
+ && ((vaddr & (dwfl->segment_align - 1))
+ == (phdr & (dwfl->segment_align - 1
+ {
+ dyn_bias = phdr - vaddr;
+ if (dyn_vaddr != 0)
+ break;
+ }
+
+ }
+ else if (type == PT_DYNAMIC)
+ {
+ dyn_vaddr = vaddr;
+ dyn_filesz = filesz;
+ if (dyn_bias != (GElf_Addr) -1)
+ break;
+ }
}
}
--
2.26.2
Remove nested functions from link_map.c V2
Same as the old thread. I used a memory_closure struct this time. I quickly looked over the patches and I think we could even save buffer and buffer_available in there to reduce the parameter count further but then we should rename the struct (but read_data isn't the best name either). Hope the changelog entries are fine, I'll try to add them from now on. Thanks, Timm
[PATCH 2/2] link_map: Pull read_addrs() into file scope
From: Timm Bäder
Get rid of a nested function this way.
Signed-off-by: Timm Bäder
---
libdwfl/ChangeLog | 5 +++
libdwfl/link_map.c | 104 +
2 files changed, 62 insertions(+), 47 deletions(-)
diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index a5e6a4a7..d4e77f55 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,8 @@
+2020-12-07 Timm Bäder
+
+ * link_map.c (report_r_debug): Pull read_addrs() function into
+ file scope.
+
2020-12-07 Timm Bäder
* link_map.c (report_r_debug): Pull release_buffer() function into
diff --git a/libdwfl/link_map.c b/libdwfl/link_map.c
index f0119a97..5a6a4dd9 100644
--- a/libdwfl/link_map.c
+++ b/libdwfl/link_map.c
@@ -243,6 +243,57 @@ release_buffer (struct memory_closure *closure,
return result;
}
+static inline bool
+read_addrs (struct memory_closure *closure,
+ uint_fast8_t elfclass, uint_fast8_t elfdata,
+ void **buffer, size_t *buffer_available,
+ GElf_Addr vaddr, GElf_Addr *read_vaddr,
+ size_t n, GElf_Addr *addrs /* [4] */)
+{
+ size_t nb = n * addrsize (elfclass); /* Address words -> bytes to read. */
+ Dwfl *dwfl = closure->dwfl;
+
+ /* Read a new buffer if the old one doesn't cover these words. */
+ if (buffer == NULL
+ || vaddr < *read_vaddr
+ || vaddr - (*read_vaddr) + nb > *buffer_available)
+{
+ release_buffer (closure, buffer, buffer_available, 0);
+
+ *read_vaddr = vaddr;
+ int segndx = INTUSE(dwfl_addrsegment) (dwfl, vaddr, NULL);
+ if (unlikely (segndx < 0)
+ || unlikely (! (*closure->callback) (dwfl, segndx,
+ buffer, buffer_available,
+ vaddr, nb, closure->arg)))
+ return true;
+}
+
+ Elf32_Addr (*a32)[n] = vaddr - (*read_vaddr) + (*buffer);
+ Elf64_Addr (*a64)[n] = (void *) a32;
+
+ if (elfclass == ELFCLASS32)
+{
+ if (elfdata == ELFDATA2MSB)
+ for (size_t i = 0; i < n; ++i)
+ addrs[i] = BE32 (read_4ubyte_unaligned_noncvt (&(*a32)[i]));
+ else
+ for (size_t i = 0; i < n; ++i)
+ addrs[i] = LE32 (read_4ubyte_unaligned_noncvt (&(*a32)[i]));
+}
+ else
+{
+ if (elfdata == ELFDATA2MSB)
+ for (size_t i = 0; i < n; ++i)
+ addrs[i] = BE64 (read_8ubyte_unaligned_noncvt (&(*a64)[i]));
+ else
+ for (size_t i = 0; i < n; ++i)
+ addrs[i] = LE64 (read_8ubyte_unaligned_noncvt (&(*a64)[i]));
+}
+
+ return false;
+}
+
/* Report a module for each struct link_map in the linked list at r_map
in the struct r_debug at R_DEBUG_VADDR. For r_debug_info description
see dwfl_link_map_report in libdwflP.h. If R_DEBUG_INFO is not NULL then no
@@ -270,52 +321,9 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t
elfdata,
GElf_Addr addrs[4];
struct memory_closure memory_closure = { dwfl, memory_callback,
memory_callback_arg };
- inline bool read_addrs (GElf_Addr vaddr, size_t n)
- {
-size_t nb = n * addrsize (elfclass); /* Address words -> bytes to read. */
-
-/* Read a new buffer if the old one doesn't cover these words. */
-if (buffer == NULL
- || vaddr < read_vaddr
- || vaddr - read_vaddr + nb > buffer_available)
- {
- release_buffer (&memory_closure, &buffer, &buffer_available, 0);
-
- read_vaddr = vaddr;
- int segndx = INTUSE(dwfl_addrsegment) (dwfl, vaddr, NULL);
- if (unlikely (segndx < 0)
- || unlikely (! (*memory_callback) (dwfl, segndx,
- &buffer, &buffer_available,
- vaddr, nb, memory_callback_arg)))
- return true;
- }
-
-Elf32_Addr (*a32)[n] = vaddr - read_vaddr + buffer;
-Elf64_Addr (*a64)[n] = (void *) a32;
-
-if (elfclass == ELFCLASS32)
- {
- if (elfdata == ELFDATA2MSB)
- for (size_t i = 0; i < n; ++i)
- addrs[i] = BE32 (read_4ubyte_unaligned_noncvt (&(*a32)[i]));
- else
- for (size_t i = 0; i < n; ++i)
- addrs[i] = LE32 (read_4ubyte_unaligned_noncvt (&(*a32)[i]));
- }
-else
- {
- if (elfdata == ELFDATA2MSB)
- for (size_t i = 0; i < n; ++i)
- addrs[i] = BE64 (read_8ubyte_unaligned_noncvt (&(*a64)[i]));
- else
- for (size_t i = 0; i < n; ++i)
- addrs[i] = LE64 (read_8ubyte_unaligned_noncvt (&(*a64)[i]));
- }
-
-return false;
- }
-
- if (unlikely (read_addrs (read_vaddr, 1)))
+ if (unlikely (read_addrs (&memory_closure, elfclass, elfdata,
+ &buffer, &buffer_available, read_vaddr, &read_vaddr,
+ 1, addrs)))
return release_buffer (&memory_closure, &buffer, &buffer_available, -1);
GElf_Addr next = addrs[0];
@@ -330,7 +338,9 @@ report_r_debug (uint_fast8_t elfclass, ui
[PATCH 1/2] link_map: Pull release_buffer() into file scope
From: Timm Bäder
Get rid of a nested function this way. Add a memory_closure struct to
keep the functions clean.
Signed-off-by: Timm Bäder
---
libdwfl/ChangeLog | 5 +
libdwfl/link_map.c | 48 ++
2 files changed, 37 insertions(+), 16 deletions(-)
diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index f11abb80..a5e6a4a7 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,8 @@
+2020-12-07 Timm Bäder
+
+ * link_map.c (report_r_debug): Pull release_buffer() function into
+ file scope. Add memory_closure struct.
+
2020-12-01 Timm Bäder
* link_map.c (dwfl_link_map_report): Removed consider_phdr function
diff --git a/libdwfl/link_map.c b/libdwfl/link_map.c
index bcff8db5..f0119a97 100644
--- a/libdwfl/link_map.c
+++ b/libdwfl/link_map.c
@@ -225,6 +225,24 @@ addrsize (uint_fast8_t elfclass)
return elfclass * 4;
}
+struct memory_closure
+{
+ Dwfl *dwfl;
+ Dwfl_Memory_Callback *callback;
+ void *arg;
+};
+
+static inline int
+release_buffer (struct memory_closure *closure,
+void **buffer, size_t *buffer_available, int result)
+{
+ if (*buffer != NULL)
+(*closure->callback) (closure->dwfl, -1, buffer, buffer_available, 0, 0,
+ closure->arg);
+
+ return result;
+}
+
/* Report a module for each struct link_map in the linked list at r_map
in the struct r_debug at R_DEBUG_VADDR. For r_debug_info description
see dwfl_link_map_report in libdwflP.h. If R_DEBUG_INFO is not NULL then no
@@ -249,15 +267,9 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t
elfdata,
void *buffer = NULL;
size_t buffer_available = 0;
- inline int release_buffer (int result)
- {
-if (buffer != NULL)
- (void) (*memory_callback) (dwfl, -1, &buffer, &buffer_available, 0, 0,
-memory_callback_arg);
-return result;
- }
-
GElf_Addr addrs[4];
+ struct memory_closure memory_closure = { dwfl, memory_callback,
+ memory_callback_arg };
inline bool read_addrs (GElf_Addr vaddr, size_t n)
{
size_t nb = n * addrsize (elfclass); /* Address words -> bytes to read. */
@@ -267,7 +279,7 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t elfdata,
|| vaddr < read_vaddr
|| vaddr - read_vaddr + nb > buffer_available)
{
- release_buffer (0);
+ release_buffer (&memory_closure, &buffer, &buffer_available, 0);
read_vaddr = vaddr;
int segndx = INTUSE(dwfl_addrsegment) (dwfl, vaddr, NULL);
@@ -304,7 +316,7 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t elfdata,
}
if (unlikely (read_addrs (read_vaddr, 1)))
-return release_buffer (-1);
+return release_buffer (&memory_closure, &buffer, &buffer_available, -1);
GElf_Addr next = addrs[0];
@@ -319,7 +331,7 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t elfdata,
while (next != 0 && ++iterations < dwfl->lookup_elts)
{
if (read_addrs (next, 4))
- return release_buffer (-1);
+ return release_buffer (&memory_closure, &buffer, &buffer_available, 0);
/* Unused: l_addr is the difference between the address in memory
and the ELF file when the core was created. We need to
@@ -345,7 +357,7 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t elfdata,
name = l_name - read_vaddr + buffer;
else
{
- release_buffer (0);
+ release_buffer (&memory_closure, &buffer, &buffer_available, 0);
read_vaddr = l_name;
int segndx = INTUSE(dwfl_addrsegment) (dwfl, l_name, NULL);
if (likely (segndx >= 0)
@@ -372,7 +384,8 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t elfdata,
r_debug_info_module = malloc (sizeof (*r_debug_info_module)
+ strlen (name1) + 1);
if (unlikely (r_debug_info_module == NULL))
- return release_buffer (result);
+ release_buffer (&memory_closure, &buffer,
+&buffer_available, result);
r_debug_info_module->fd = -1;
r_debug_info_module->elf = NULL;
r_debug_info_module->l_ld = l_ld;
@@ -413,7 +426,8 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t elfdata,
GElf_Addr build_id_vaddr = (build_id_elfaddr
- elf_dynamic_vaddr + l_ld);
- release_buffer (0);
+ release_buffer (&memory_closure, &buffer,
+ &buffer_available, 0);
int segndx = INTUSE(dwfl_addrsegment) (dwfl,
build_id_vaddr,
NULL);
@@ -432,7 +446,9 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t elfdata,
/* File has valid build-id wh
Remove nested functions from src/strip.c
Hi, here a couple of patches to remove the nested functions from src/strip.c. I tried to keep the resulting code clean but had to do some refactorings to get that done. I hope the result is worth considering. Otherwise, I'm open for suggestions. - Timm
[PATCH 1/4] strip: Replace nested check_preserved function with loop
From: Timm Bäder
Signed-off-by: Timm Bäder
---
src/strip.c | 37 +
1 file changed, 21 insertions(+), 16 deletions(-)
diff --git a/src/strip.c b/src/strip.c
index 7ce14ab8..c971b6c2 100644
--- a/src/strip.c
+++ b/src/strip.c
@@ -1535,25 +1535,30 @@ handle_elf (int fd, Elf *elf, const char *prefix, const
char *fname,
files by setting the .debug_data pointer to the original
file's .data pointer. Below, we'll copy the section
contents. */
+ size_t shdr_indices[2] = { shdr_info[cnt].shdr.sh_link, 0 };
+ int n = 1;
- inline void check_preserved (size_t i)
- {
- if (i != 0 && i < shnum + 2 && shdr_info[i].idx != 0
- && shdr_info[i].debug_data == NULL)
- {
- if (shdr_info[i].data == NULL)
- shdr_info[i].data = elf_getdata (shdr_info[i].scn, NULL);
- if (shdr_info[i].data == NULL)
- INTERNAL_ERROR (fname);
+ if (SH_INFO_LINK_P (&shdr_info[cnt].shdr))
+ {
+ shdr_indices[1] = shdr_info[cnt].shdr.sh_info;
+ n++;
+ }
- shdr_info[i].debug_data = shdr_info[i].data;
- changes |= i < cnt;
- }
- }
+ for (int j = 0; j < n; j++)
+ {
+ size_t i = shdr_indices[j];
+ if (i != 0 && i < shnum + 2 && shdr_info[i].idx != 0
+ && shdr_info[i].debug_data == NULL)
+ {
+ if (shdr_info[i].data == NULL)
+ shdr_info[i].data = elf_getdata (shdr_info[i].scn,
NULL);
+ if (shdr_info[i].data == NULL)
+ INTERNAL_ERROR (fname);
- check_preserved (shdr_info[cnt].shdr.sh_link);
- if (SH_INFO_LINK_P (&shdr_info[cnt].shdr))
- check_preserved (shdr_info[cnt].shdr.sh_info);
+ shdr_info[i].debug_data = shdr_info[i].data;
+ changes |= i < cnt;
+ }
+ }
}
}
}
--
2.26.2
[PATCH 2/4] strip: Pull relocate() info file scope
From: Timm Bäder
Pull relocate() info file scope and get rid of a nested function this
way. Refactor remove_debug_relocations() to minimize the parameters we
need to pass to relocate().
Signed-off-by: Timm Bäder
---
src/strip.c | 347
1 file changed, 184 insertions(+), 163 deletions(-)
diff --git a/src/strip.c b/src/strip.c
index c971b6c2..71913fac 100644
--- a/src/strip.c
+++ b/src/strip.c
@@ -442,6 +442,127 @@ update_shdrstrndx (Elf *elf, size_t shdrstrndx)
return 0;
}
+
+/* Apply one relocation. Returns true when trivial
+ relocation actually done. */
+static bool
+relocate (Elf *elf, GElf_Addr offset, const GElf_Sxword addend,
+ Elf_Data *tdata, unsigned int ei_data, const char *fname,
+ bool is_rela, GElf_Sym *sym, int addsub, Elf_Type type)
+{
+ /* These are the types we can relocate. */
+#define TYPES DO_TYPE (BYTE, Byte); DO_TYPE (HALF, Half);\
+ DO_TYPE (WORD, Word); DO_TYPE (SWORD, Sword);\
+ DO_TYPE (XWORD, Xword); DO_TYPE (SXWORD, Sxword)
+
+ size_t size;
+
+#define DO_TYPE(NAME, Name) GElf_##Name Name;
+ union { TYPES; } tmpbuf;
+#undef DO_TYPE
+
+ switch (type)
+{
+#define DO_TYPE(NAME, Name)\
+ case ELF_T_##NAME: \
+ size = sizeof (GElf_##Name);\
+ tmpbuf.Name = 0;\
+ break;
+ TYPES;
+#undef DO_TYPE
+default:
+ return false;
+}
+
+ if (offset > tdata->d_size
+ || tdata->d_size - offset < size)
+{
+ cleanup_debug ();
+ error (EXIT_FAILURE, 0, _("bad relocation"));
+}
+
+ /* When the symbol value is zero then for SHT_REL
+ sections this is all that needs to be checked.
+ The addend is contained in the original data at
+ the offset already. So if the (section) symbol
+ address is zero and the given addend is zero
+ just remove the relocation, it isn't needed
+ anymore. */
+ if (addend == 0 && sym->st_value == 0)
+return true;
+
+ Elf_Data tmpdata =
+{
+ .d_type = type,
+ .d_buf = &tmpbuf,
+ .d_size = size,
+ .d_version = EV_CURRENT,
+};
+ Elf_Data rdata =
+{
+ .d_type = type,
+ .d_buf = tdata->d_buf + offset,
+ .d_size = size,
+ .d_version = EV_CURRENT,
+};
+
+ GElf_Addr value = sym->st_value;
+ if (is_rela)
+{
+ /* For SHT_RELA sections we just take the
+given addend and add it to the value. */
+ value += addend;
+ /* For ADD/SUB relocations we need to fetch the
+current section contents. */
+ if (addsub != 0)
+ {
+ Elf_Data *d = gelf_xlatetom (elf, &tmpdata,
+ &rdata,
+ ei_data);
+ if (d == NULL)
+ INTERNAL_ERROR (fname);
+ assert (d == &tmpdata);
+ }
+}
+ else
+{
+ /* For SHT_REL sections we have to peek at
+what is already in the section at the given
+offset to get the addend. */
+ Elf_Data *d = gelf_xlatetom (elf, &tmpdata,
+ &rdata,
+ ei_data);
+ if (d == NULL)
+ INTERNAL_ERROR (fname);
+ assert (d == &tmpdata);
+}
+
+ switch (type)
+{
+#define DO_TYPE(NAME, Name) \
+ case ELF_T_##NAME:\
+ if (addsub < 0) \
+ tmpbuf.Name -= (GElf_##Name) value;\
+ else \
+ tmpbuf.Name += (GElf_##Name) value;\
+ break;
+ TYPES;
+#undef DO_TYPE
+default:
+ abort ();
+}
+
+ /* Now finally put in the new value. */
+ Elf_Data *s = gelf_xlatetof (elf, &rdata,
+ &tmpdata,
+ ei_data);
+ if (s == NULL)
+INTERNAL_ERROR (fname);
+ assert (s == &rdata);
+
+ return true;
+}
+
/* Remove any relocations between debug sections in ET_REL
for the debug file when requested. These relocations are always
zero based between the unallocated sections. */
@@ -517,180 +638,80 @@ remove_debug_relocations (Ebl *ebl, Elf *elf, GElf_Ehdr
*ehdr,
if (symdata == NULL)
INTERNAL_ERROR (fname);
- /* Apply one relocation. Returns true when trivial
-relocation actually done. */
- bool relocate (GElf_Addr offset, const GElf_Sxword addend,
-bool is_rela, int rtype, int symndx)
- {
- /* R_*_NONE relocs can always just be removed. */
- if (rtype == 0)
- return true;
+ if (shdr->sh_entsize == 0)
+ INTERNAL_ERROR (fname);
- /* We only do simple absolute relocations. */
- int addsub = 0;
- Elf_Type type = ebl_reloc_simple_type (ebl, rtype, &addsub);
- if (type ==
[PATCH 3/4] strip: Pull update_section_size() into file scope
From: Timm Bäder
Get rid of a nested function this way.
Signed-off-by: Timm Bäder
---
src/strip.c | 51 ---
1 file changed, 28 insertions(+), 23 deletions(-)
diff --git a/src/strip.c b/src/strip.c
index 71913fac..e608dc5e 100644
--- a/src/strip.c
+++ b/src/strip.c
@@ -939,6 +939,31 @@ handle_debug_relocs (Elf *elf, Ebl *ebl, Elf *new_elf,
return 0;
}
+/* Update section headers when the data size has changed.
+ We also update the SHT_NOBITS section in the debug
+ file so that the section headers match in sh_size. */
+static inline void
+update_section_size (Elf_Scn *scn,
+const Elf_Data *newdata,
+Elf *debugelf,
+size_t cnt,
+const char *fname)
+{
+ GElf_Shdr shdr_mem;
+ GElf_Shdr *shdr = gelf_getshdr (scn, &shdr_mem);
+ shdr->sh_size = newdata->d_size;
+ (void) gelf_update_shdr (scn, shdr);
+ if (debugelf != NULL)
+{
+ /* libelf will use d_size to set sh_size. */
+ Elf_Data *debugdata = elf_getdata (elf_getscn (debugelf,
+cnt), NULL);
+ if (debugdata == NULL)
+ INTERNAL_ERROR (fname);
+ debugdata->d_size = newdata->d_size;
+}
+}
+
/* Maximum size of array allocated on stack. */
#define MAX_STACK_ALLOC(400 * 1024)
@@ -2150,26 +2175,6 @@ handle_elf (int fd, Elf *elf, const char *prefix, const
char *fname,
/* Find all relocation sections which use this symbol table. */
for (cnt = 1; cnt <= shdridx; ++cnt)
{
- /* Update section headers when the data size has changed.
- We also update the SHT_NOBITS section in the debug
- file so that the section headers match in sh_size. */
- inline void update_section_size (const Elf_Data *newdata)
- {
- GElf_Shdr shdr_mem;
- GElf_Shdr *shdr = gelf_getshdr (scn, &shdr_mem);
- shdr->sh_size = newdata->d_size;
- (void) gelf_update_shdr (scn, shdr);
- if (debugelf != NULL)
- {
- /* libelf will use d_size to set sh_size. */
- Elf_Data *debugdata = elf_getdata (elf_getscn (debugelf,
-cnt), NULL);
- if (debugdata == NULL)
- INTERNAL_ERROR (fname);
- debugdata->d_size = newdata->d_size;
- }
- }
-
if (shdr_info[cnt].idx == 0 && debug_fname == NULL)
/* Ignore sections which are discarded. When we are saving a
relocation section in a separate debug file, we must fix up
@@ -2299,7 +2304,7 @@ handle_elf (int fd, Elf *elf, const char *prefix, const
char *fname,
* sizeof (Elf32_Word));
elf_assert (n_size <= hashd->d_size);
hashd->d_size = n_size;
- update_section_size (hashd);
+ update_section_size (scn, hashd, debugelf, cnt, fname);
/* Clear the arrays. */
memset (bucket, '\0',
@@ -2361,7 +2366,7 @@ handle_elf (int fd, Elf *elf, const char *prefix, const
char *fname,
* sizeof (Elf64_Xword));
elf_assert (n_size <= hashd->d_size);
hashd->d_size = n_size;
- update_section_size (hashd);
+ update_section_size (scn, hashd, debugelf, cnt, fname);
/* Clear the arrays. */
memset (bucket, '\0',
@@ -2435,7 +2440,7 @@ handle_elf (int fd, Elf *elf, const char *prefix, const
char *fname,
/ gelf_fsize (elf, symd->d_type, 1,
EV_CURRENT),
EV_CURRENT);
- update_section_size (verd);
+ update_section_size (scn, verd, debugelf, cnt, fname);
break;
case SHT_GROUP:
--
2.26.2
[PATCH 4/4] strip: Remove no_symtab_updates() function
From: Timm Bäder
The no_symtab_updates() function was being called at the beginning of
all case labels in this switch, so we can just call it once before the
switch. Then it only has one call-site, so inline this short function
there.
Signed-off-by: Timm Bäder
---
src/strip.c | 152
1 file changed, 69 insertions(+), 83 deletions(-)
diff --git a/src/strip.c b/src/strip.c
index e608dc5e..dd1e27ac 100644
--- a/src/strip.c
+++ b/src/strip.c
@@ -2175,98 +2175,91 @@ handle_elf (int fd, Elf *elf, const char *prefix, const
char *fname,
/* Find all relocation sections which use this symbol table. */
for (cnt = 1; cnt <= shdridx; ++cnt)
{
- if (shdr_info[cnt].idx == 0 && debug_fname == NULL)
+ struct shdr_info *info = &shdr_info[cnt];
+ if (info->idx == 0 && debug_fname == NULL)
/* Ignore sections which are discarded. When we are saving a
relocation section in a separate debug file, we must fix up
the symbol table references. */
continue;
- const Elf32_Word symtabidx = shdr_info[cnt].old_sh_link;
+ const Elf32_Word symtabidx = info->old_sh_link;
elf_assert (symtabidx < shnum + 2);
const Elf32_Word *const newsymidx = shdr_info[symtabidx].newsymidx;
- switch (shdr_info[cnt].shdr.sh_type)
- {
- inline bool no_symtab_updates (void)
- {
- /* If the symbol table hasn't changed, do not do anything. */
- if (shdr_info[symtabidx].newsymidx == NULL)
- return true;
-
- /* If the symbol table is not discarded, but additionally
-duplicated in the separate debug file and this section
-is discarded, don't adjust anything. */
- return (shdr_info[cnt].idx == 0
- && shdr_info[symtabidx].debug_data != NULL);
- }
+ /* If the symbol table hasn't changed, do not do anything. */
+ if (newsymidx == NULL)
+ continue;
+
+ /* If the symbol table is not discarded, but additionally
+ duplicated in the separate debug file and this section
+ is discarded, don't adjust anything. */
+ if (info->idx == 0 && shdr_info[symtabidx].debug_data != NULL)
+ continue;
+
+ switch (info->shdr.sh_type)
+ {
case SHT_REL:
case SHT_RELA:
- if (no_symtab_updates ())
- break;
-
- Elf_Data *d = elf_getdata (shdr_info[cnt].idx == 0
- ? elf_getscn (debugelf, cnt)
- : elf_getscn (newelf,
-shdr_info[cnt].idx),
- NULL);
- elf_assert (d != NULL && d->d_buf != NULL
- && shdr_info[cnt].shdr.sh_entsize != 0);
- size_t nrels = (shdr_info[cnt].shdr.sh_size
- / shdr_info[cnt].shdr.sh_entsize);
-
- size_t symsize = gelf_fsize (elf, ELF_T_SYM, 1, EV_CURRENT);
- const Elf32_Word symidxn = (shdr_info[symtabidx].data->d_size
- / symsize);
- if (shdr_info[cnt].shdr.sh_type == SHT_REL)
- for (size_t relidx = 0; relidx < nrels; ++relidx)
- {
- GElf_Rel rel_mem;
- if (gelf_getrel (d, relidx, &rel_mem) == NULL)
- INTERNAL_ERROR (fname);
+ {
+ Elf_Data *d = elf_getdata (info->idx == 0
+? elf_getscn (debugelf, cnt)
+: elf_getscn (newelf, info->idx),
+NULL);
+ elf_assert (d != NULL && d->d_buf != NULL
+ && info->shdr.sh_entsize != 0);
+ size_t nrels = (info->shdr.sh_size / info->shdr.sh_entsize);
+
+ size_t symsize = gelf_fsize (elf, ELF_T_SYM, 1, EV_CURRENT);
+ const Elf32_Word symidxn = (shdr_info[symtabidx].data->d_size
+ / symsize);
+ if (info->shdr.sh_type == SHT_REL)
+ for (size_t relidx = 0; relidx < nrels; ++relidx)
+ {
+ GElf_Rel rel_mem;
+ if (gelf_getrel (d, relidx, &rel_mem) == NULL)
+ INTERNAL_ERROR (fname);
- size_t symidx = GELF_R_SYM (rel_mem.r_info);
- elf_assert (symidx < symidxn);
- if (newsymidx[symidx] != symidx)
- {
- rel_mem.r_info
- = GELF_R_INFO (newsymidx[symidx],
- GELF_R_TYPE (rel_mem.r_info));
+ size_t symidx = GELF_R_SYM (rel_mem.r_info);
+ elf_assert (symidx < symidxn);
+ if (newsymidx[symidx] != s
Remove nested functions from elf-from-memory.c
Hey, these are relatively straight-forward I think. Let me know if that's not the case. :) - Timm
[PATCH 1/2] elf-from-memory: Restructure code to get rid of nested handle_segment()
From: Timm Bäder
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
---
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
[PATCH 2/2] elf-from-memory: Refactor to get rid of nested function
From: Timm Bäder
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
---
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;
+
Misc single nested function removals
Hi, here a few patches to remove single nested functions from - addr2line.c - tests/zstrptr.c - ar.c - arlib-argp.c I think they are pretty straight-forward again. - Timm
[PATCH 1/4] addr2line: Pull show_note() and show_int() in file scope
From: Timm Bäder
Get rid of the nested functions
Signed-off-by: Timm Bäder
---
src/addr2line.c | 47 ++-
1 file changed, 26 insertions(+), 21 deletions(-)
diff --git a/src/addr2line.c b/src/addr2line.c
index ea01c1be..34945046 100644
--- a/src/addr2line.c
+++ b/src/addr2line.c
@@ -598,6 +598,26 @@ get_addr_width (Dwfl_Module *mod)
return width;
}
+static inline void
+show_note (int (*get) (Dwarf_Line *, bool *),
+ Dwarf_Line *info,
+ const char *note)
+{
+ bool flag;
+ if ((*get) (info, &flag) == 0 && flag)
+fputs (note, stdout);
+}
+
+static inline void
+show_int (int (*get) (Dwarf_Line *, unsigned int *),
+ Dwarf_Line *info,
+ const char *name)
+{
+ unsigned int val;
+ if ((*get) (info, &val) == 0 && val != 0)
+printf (" (%s %u)", name, val);
+}
+
static int
handle_address (const char *string, Dwfl *dwfl)
{
@@ -692,27 +712,12 @@ handle_address (const char *string, Dwfl *dwfl)
Dwarf_Line *info = dwfl_dwarf_line (line, &bias);
assert (info != NULL);
- inline void show (int (*get) (Dwarf_Line *, bool *),
- const char *note)
- {
- bool flag;
- if ((*get) (info, &flag) == 0 && flag)
- fputs (note, stdout);
- }
- inline void show_int (int (*get) (Dwarf_Line *, unsigned int *),
- const char *name)
- {
- unsigned int val;
- if ((*get) (info, &val) == 0 && val != 0)
- printf (" (%s %u)", name, val);
- }
-
- show (&dwarf_linebeginstatement, " (is_stmt)");
- show (&dwarf_lineblock, " (basic_block)");
- show (&dwarf_lineprologueend, " (prologue_end)");
- show (&dwarf_lineepiloguebegin, " (epilogue_begin)");
- show_int (&dwarf_lineisa, "isa");
- show_int (&dwarf_linediscriminator, "discriminator");
+ show_note (&dwarf_linebeginstatement, info, " (is_stmt)");
+ show_note (&dwarf_lineblock, info, " (basic_block)");
+ show_note (&dwarf_lineprologueend, info, " (prologue_end)");
+ show_note (&dwarf_lineepiloguebegin, info, " (epilogue_begin)");
+ show_int (&dwarf_lineisa, info, "isa");
+ show_int (&dwarf_linediscriminator, info, "discriminator");
}
putchar ('\n');
}
--
2.26.2
[PATCH 3/4] ar: Pull should_truncate_fname() into file scope
From: Timm Bäder
Get rid of a nested function this way.
Signed-off-by: Timm Bäder
---
src/ar.c | 34 +-
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/src/ar.c b/src/ar.c
index 2a17d0d3..7d0298dd 100644
--- a/src/ar.c
+++ b/src/ar.c
@@ -436,6 +436,21 @@ copy_content (Elf *elf, int newfd, off_t off, size_t n)
return write_retry (newfd, rawfile + off, n) != (ssize_t) n;
}
+static inline bool
+should_truncate_fname (size_t name_max)
+{
+ if (errno == ENAMETOOLONG && allow_truncate_fname)
+{
+ if (name_max == 0)
+ {
+ long int len = pathconf (".", _PC_NAME_MAX);
+ if (len > 0)
+ name_max = len;
+ }
+ return name_max != 0;
+}
+ return false;
+}
static int
do_oper_extract (int oper, const char *arfname, char **argv, int argc,
@@ -445,21 +460,6 @@ do_oper_extract (int oper, const char *arfname, char
**argv, int argc,
memset (found, '\0', sizeof (found));
size_t name_max = 0;
- inline bool should_truncate_fname (void)
- {
-if (errno == ENAMETOOLONG && allow_truncate_fname)
- {
- if (name_max == 0)
- {
- long int len = pathconf (".", _PC_NAME_MAX);
- if (len > 0)
- name_max = len;
- }
- return name_max != 0;
- }
-return false;
- }
-
off_t index_off = -1;
size_t index_size = 0;
off_t cur_off = SARMAG;
@@ -615,7 +615,7 @@ do_oper_extract (int oper, const char *arfname, char
**argv, int argc,
{
int printlen = INT_MAX;
- if (should_truncate_fname ())
+ if (should_truncate_fname (name_max))
{
/* Try to truncate the name. First find out by how
much. */
@@ -704,7 +704,7 @@ do_oper_extract (int oper, const char *arfname, char
**argv, int argc,
{
int printlen = INT_MAX;
- if (should_truncate_fname ())
+ if (should_truncate_fname (name_max))
{
/* Try to truncate the name. First find out by how
much. */
--
2.26.2
[PATCH 2/4] zstrptr: Pull print_string() into file scope
From: Timm Bäder
Get rid of a nested function this way.
Signed-off-by: Timm Bäder
---
tests/zstrptr.c | 43 ++-
1 file changed, 22 insertions(+), 21 deletions(-)
diff --git a/tests/zstrptr.c b/tests/zstrptr.c
index 6d8e19f7..9fb42e28 100644
--- a/tests/zstrptr.c
+++ b/tests/zstrptr.c
@@ -30,6 +30,26 @@
#include ELFUTILS_HEADER(elf)
#include
+static void
+print_strings (Elf_Scn *scn, Elf *elf, size_t ndx)
+{
+ GElf_Shdr shdr_mem;
+ GElf_Shdr *shdr = gelf_getshdr (scn, &shdr_mem);
+
+ printf ("Strings in section %zd (%s):\n", ndx,
+ ((shdr->sh_flags & SHF_COMPRESSED) != 0
+ ? "compressed" : "uncompressed"));
+
+ size_t off = 0;
+ const char *str = elf_strptr (elf, ndx, off);
+ while (str != NULL)
+{
+ printf ("[%zx] '%s'\n", off, str);
+ off += strlen (str) + 1;
+ str = elf_strptr (elf, ndx, off);
+}
+}
+
int
main (int argc, char *argv[])
{
@@ -79,38 +99,19 @@ main (int argc, char *argv[])
exit (1);
}
- void print_strings (void)
- {
-GElf_Shdr shdr_mem;
-GElf_Shdr *shdr = gelf_getshdr (scn, &shdr_mem);
-
-printf ("Strings in section %zd (%s):\n", ndx,
- ((shdr->sh_flags & SHF_COMPRESSED) != 0
-? "compressed" : "uncompressed"));
-
-size_t off = 0;
-const char *str = elf_strptr (elf, ndx, off);
-while (str != NULL)
- {
- printf ("[%zx] '%s'\n", off, str);
- off += strlen (str) + 1;
- str = elf_strptr (elf, ndx, off);
- }
- }
-
if (elf_compress (scn, ELFCOMPRESS_ZLIB, 0) < 0)
{
printf ("Couldn't compress section %zd: %s\n", ndx, elf_errmsg (-1));
exit (1);
}
- print_strings ();
+ print_strings (scn, elf, ndx);
if (elf_compress (scn, 0, 0) < 0)
{
printf ("Couldn't decompress section %zd: %s\n", ndx, elf_errmsg (-1));
exit (1);
}
- print_strings ();
+ print_strings (scn, elf, ndx);
if (elf_end (elf) != 0)
{
--
2.26.2
[PATCH 4/4] arlib-argp: Pull text_for_default() into file scope
From: Timm Bäder
Get rid of a nested function this way.
Signed-off-by: Timm Bäder
---
src/arlib-argp.c | 21 +++--
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/src/arlib-argp.c b/src/arlib-argp.c
index c07d9299..a3c12e4d 100644
--- a/src/arlib-argp.c
+++ b/src/arlib-argp.c
@@ -57,25 +57,26 @@ parse_opt (int key, char *arg __attribute__ ((unused)),
}
static char *
-help_filter (int key, const char *text, void *input __attribute__ ((unused)))
+text_for_default (const char *text)
{
- inline char *text_for_default (void)
- {
-char *new_text;
-if (unlikely (asprintf (&new_text, _("%s (default)"), text) < 0))
- return (char *) text;
-return new_text;
- }
+ char *new_text;
+ if (unlikely (asprintf (&new_text, _("%s (default)"), text) < 0))
+return (char *) text;
+ return new_text;
+}
+static char *
+help_filter (int key, const char *text, void *input __attribute__ ((unused)))
+{
switch (key)
{
case 'D':
if (DEFAULT_AR_DETERMINISTIC)
-return text_for_default ();
+return text_for_default (text);
break;
case 'U':
if (! DEFAULT_AR_DETERMINISTIC)
-return text_for_default ();
+return text_for_default (text);
break;
}
--
2.26.2
Remove nested functions from readelf.c
Hi, here another round for src/readelf.c. I think they are simple, but I'm not happy with the advance_pc() commit. I'm open for suggestions here but I can't come up with something better right now. I'll keep looking in to that in the meantime. - Timm
[PATCH 1/5] readelf: Pull add_dump_section() into file scope
From: Timm Bäder
Get rid of a nested function this way.
Signed-off-by: Timm Bäder
---
src/readelf.c | 34 ++
1 file changed, 18 insertions(+), 16 deletions(-)
diff --git a/src/readelf.c b/src/readelf.c
index 829a418d..a95fc0aa 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -381,24 +381,26 @@ main (int argc, char *argv[])
return error_message_count != 0;
}
+static void
+add_dump_section (const char *name,
+ int key,
+ bool implicit)
+{
+ struct section_argument *a = xmalloc (sizeof *a);
+ a->arg = name;
+ a->next = NULL;
+ a->implicit = implicit;
+ struct section_argument ***tailp
+= key == 'x' ? &dump_data_sections_tail : &string_sections_tail;
+ **tailp = a;
+ *tailp = &a->next;
+}
/* Handle program arguments. */
static error_t
parse_opt (int key, char *arg,
struct argp_state *state __attribute__ ((unused)))
{
- void add_dump_section (const char *name, bool implicit)
- {
-struct section_argument *a = xmalloc (sizeof *a);
-a->arg = name;
-a->next = NULL;
-a->implicit = implicit;
-struct section_argument ***tailp
- = key == 'x' ? &dump_data_sections_tail : &string_sections_tail;
-**tailp = a;
-*tailp = &a->next;
- }
-
switch (key)
{
case 'a':
@@ -414,9 +416,9 @@ parse_opt (int key, char *arg,
print_arch = true;
print_notes = true;
implicit_debug_sections |= section_exception;
- add_dump_section (".strtab", true);
- add_dump_section (".dynstr", true);
- add_dump_section (".comment", true);
+ add_dump_section (".strtab", key, true);
+ add_dump_section (".dynstr", key, true);
+ add_dump_section (".comment", key, true);
any_control_option = true;
break;
case 'A':
@@ -562,7 +564,7 @@ parse_opt (int key, char *arg,
}
FALLTHROUGH;
case 'x':
- add_dump_section (arg, false);
+ add_dump_section (arg, key, false);
any_control_option = true;
break;
case 'N':
--
2.26.2
[PATCH 2/5] readelf: Pull same_set() info file scope
From: Timm Bäder
Get rid of a nested function this way
Signed-off-by: Timm Bäder
---
src/readelf.c | 24 ++--
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/src/readelf.c b/src/readelf.c
index a95fc0aa..0157f8a2 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -12072,6 +12072,18 @@ compare_register_sets (const void *a, const void *b)
return compare_sets_by_info (*p1, *p2);
}
+static inline bool
+same_set (const struct register_info *a,
+ const struct register_info *b,
+ const struct register_info *regs,
+ size_t maxnreg)
+{
+ return (a < ®s[maxnreg] && a->regloc != NULL
+ && b < ®s[maxnreg] && b->regloc != NULL
+ && a->bits == b->bits
+ && (a->set == b->set || !strcmp (a->set, b->set)));
+}
+
static unsigned int
handle_core_registers (Ebl *ebl, Elf *core, const void *desc,
const Ebl_Register_Location *reglocs, size_t nregloc)
@@ -12110,19 +12122,11 @@ handle_core_registers (Ebl *ebl, Elf *core, const
void *desc,
qsort (regs, maxreg + 1, sizeof regs[0], &compare_registers);
/* Collect the unique sets and sort them. */
- inline bool same_set (const struct register_info *a,
- const struct register_info *b)
- {
-return (a < ®s[maxnreg] && a->regloc != NULL
- && b < ®s[maxnreg] && b->regloc != NULL
- && a->bits == b->bits
- && (a->set == b->set || !strcmp (a->set, b->set)));
- }
struct register_info *sets[maxreg + 1];
sets[0] = ®s[0];
size_t nsets = 1;
for (int i = 1; i <= maxreg; ++i)
-if (regs[i].regloc != NULL && !same_set (®s[i], ®s[i - 1]))
+if (regs[i].regloc != NULL && !same_set (®s[i], ®s[i - 1], regs,
maxnreg))
sets[nsets++] = ®s[i];
qsort (sets, nsets, sizeof sets[0], &compare_register_sets);
@@ -12133,7 +12137,7 @@ handle_core_registers (Ebl *ebl, Elf *core, const void
*desc,
/* Find the longest name of a register in this set. */
size_t maxname = 0;
const struct register_info *end;
- for (end = sets[i]; same_set (sets[i], end); ++end)
+ for (end = sets[i]; same_set (sets[i], end, regs, maxnreg); ++end)
{
size_t len = strlen (end->name);
if (len > maxname)
--
2.26.2
[PATCH 3/5] readelf: Pull left() info file scope
From: Timm Bäder
Get rid of a nested function this way.
Signed-off-by: Timm Bäder
---
src/readelf.c | 16 +---
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/src/readelf.c b/src/readelf.c
index 0157f8a2..99e90c34 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -3571,6 +3571,13 @@ print_liblist (Ebl *ebl)
}
}
+static inline size_t
+left (Elf_Data *data,
+ const unsigned char *p)
+{
+ return (const unsigned char *) data->d_buf + data->d_size - p;
+}
+
static void
print_attributes (Ebl *ebl, const GElf_Ehdr *ehdr)
{
@@ -3615,13 +3622,8 @@ print_attributes (Ebl *ebl, const GElf_Ehdr *ehdr)
fputs_unlocked (_(" Owner Size\n"), stdout);
- inline size_t left (void)
- {
- return (const unsigned char *) data->d_buf + data->d_size - p;
- }
-
/* Loop over the sections. */
- while (left () >= 4)
+ while (left (data, p) >= 4)
{
/* Section length. */
uint32_t len;
@@ -3630,7 +3632,7 @@ print_attributes (Ebl *ebl, const GElf_Ehdr *ehdr)
if (MY_ELFDATA != ehdr->e_ident[EI_DATA])
CONVERT (len);
- if (unlikely (len > left ()))
+ if (unlikely (len > left (data, p)))
break;
/* Section vendor name. */
--
2.26.2
[PATCH 4/5] readelf: Pull regname() into file scope
From: Timm Bäder
Get rid of a nested function this way.
Signed-off-by: Timm Bäder
---
src/readelf.c | 42 --
1 file changed, 24 insertions(+), 18 deletions(-)
diff --git a/src/readelf.c b/src/readelf.c
index 99e90c34..e0163891 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -6206,6 +6206,13 @@ read_encoded (unsigned int encoding, const unsigned char
*readp,
return readp;
}
+static const char *
+regname (Ebl *ebl, unsigned int regno, char *regnamebuf)
+{
+ register_info (ebl, regno, NULL, regnamebuf, NULL, NULL);
+
+ return regnamebuf;
+}
static void
print_cfa_program (const unsigned char *readp, const unsigned char *const endp,
@@ -6216,11 +6223,6 @@ print_cfa_program (const unsigned char *readp, const
unsigned char *const endp,
Dwfl_Module *dwflmod, Ebl *ebl, GElf_Ehdr *ehdr, Dwarf *dbg)
{
char regnamebuf[REGNAMESZ];
- const char *regname (unsigned int regno)
- {
-register_info (ebl, regno, NULL, regnamebuf, NULL, NULL);
-return regnamebuf;
- }
puts ("\n Program:");
Dwarf_Word pc = vma_base;
@@ -6277,26 +6279,28 @@ print_cfa_program (const unsigned char *readp, const
unsigned char *const endp,
get_uleb128 (op2, readp, endp);
printf (" offset_extended r%" PRIu64 " (%s) at cfa%+" PRId64
"\n",
- op1, regname (op1), op2 * data_align);
+ op1, regname (ebl, op1, regnamebuf), op2 * data_align);
break;
case DW_CFA_restore_extended:
if ((uint64_t) (endp - readp) < 1)
goto invalid;
get_uleb128 (op1, readp, endp);
printf (" restore_extended r%" PRIu64 " (%s)\n",
- op1, regname (op1));
+ op1, regname (ebl, op1, regnamebuf));
break;
case DW_CFA_undefined:
if ((uint64_t) (endp - readp) < 1)
goto invalid;
get_uleb128 (op1, readp, endp);
- printf (" undefined r%" PRIu64 " (%s)\n", op1, regname (op1));
+ printf (" undefined r%" PRIu64 " (%s)\n", op1,
+ regname (ebl, op1, regnamebuf));
break;
case DW_CFA_same_value:
if ((uint64_t) (endp - readp) < 1)
goto invalid;
get_uleb128 (op1, readp, endp);
- printf (" same_value r%" PRIu64 " (%s)\n", op1, regname (op1));
+ printf (" same_value r%" PRIu64 " (%s)\n", op1,
+ regname (ebl, op1, regnamebuf));
break;
case DW_CFA_register:
if ((uint64_t) (endp - readp) < 1)
@@ -6306,7 +6310,8 @@ print_cfa_program (const unsigned char *readp, const
unsigned char *const endp,
goto invalid;
get_uleb128 (op2, readp, endp);
printf (" register r%" PRIu64 " (%s) in r%" PRIu64 " (%s)\n",
- op1, regname (op1), op2, regname (op2));
+ op1, regname (ebl, op1, regnamebuf), op2,
+ regname (ebl, op2, regnamebuf));
break;
case DW_CFA_remember_state:
puts (" remember_state");
@@ -6322,14 +6327,14 @@ print_cfa_program (const unsigned char *readp, const
unsigned char *const endp,
goto invalid;
get_uleb128 (op2, readp, endp);
printf (" def_cfa r%" PRIu64 " (%s) at offset %" PRIu64 "\n",
- op1, regname (op1), op2);
+ op1, regname (ebl, op1, regnamebuf), op2);
break;
case DW_CFA_def_cfa_register:
if ((uint64_t) (endp - readp) < 1)
goto invalid;
get_uleb128 (op1, readp, endp);
printf (" def_cfa_register r%" PRIu64 " (%s)\n",
- op1, regname (op1));
+ op1, regname (ebl, op1, regnamebuf));
break;
case DW_CFA_def_cfa_offset:
if ((uint64_t) (endp - readp) < 1)
@@ -6360,7 +6365,7 @@ print_cfa_program (const unsigned char *readp, const
unsigned char *const endp,
goto invalid;
get_uleb128 (op2, readp, endp); /* Length of DW_FORM_block. */
printf (" expression r%" PRIu64 " (%s) \n",
- op1, regname (op1));
+ op1, regname (ebl, op1, regnamebuf));
if ((uint64_t) (endp - readp) < op2)
goto invalid;
print_ops (dwflmod, dbg, 10, 10, version, ptr_size, 0, NULL,
@@ -6376,7 +6381,7 @@ print_cfa_program (const unsigned char *readp, const
unsigned char *const endp,
get_sleb128 (sop2, readp, endp);
printf (" offset_extended_sf r%" PRIu64 " (%s) at cfa%+"
PRId64 "\n",
- op1, regname (op1), sop2 * data_align);
+ op1, regname (ebl, op1, regnamebuf), sop2 * data_align);
break;
case DW_CFA_def_cfa_sf:
if ((uint
[PATCH 5/5] readelf: Pull advance_pc() into file scope
From: Timm Bäder
Get rid of a nested function this way
Signed-off-by: Timm Bäder
---
src/readelf.c | 39 +++
1 file changed, 27 insertions(+), 12 deletions(-)
diff --git a/src/readelf.c b/src/readelf.c
index e0163891..9758d338 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -8356,6 +8356,23 @@ print_form_data (Dwarf *dbg, int form, const unsigned
char *readp,
return readp;
}
+static inline void
+advance_pc (unsigned int op_advance,
+ uint_fast8_t max_ops_per_instr,
+ uint_fast8_t minimum_instr_len,
+ Dwarf_Word *address,
+ unsigned int *op_addr_advance,
+ unsigned int *op_index,
+ bool *show_op_index)
+{
+ *op_addr_advance = minimum_instr_len * ((*op_index + op_advance)
+/ max_ops_per_instr);
+ (*address) += *op_addr_advance;
+ *show_op_index = (*op_index > 0 ||
+ (*op_index + op_advance) % max_ops_per_instr > 0);
+ *op_index = (*op_index + op_advance) % max_ops_per_instr;
+}
+
static void
print_debug_line_section (Dwfl_Module *dwflmod, Ebl *ebl, GElf_Ehdr *ehdr,
Elf_Scn *scn, GElf_Shdr *shdr, Dwarf *dbg)
@@ -8747,15 +8764,6 @@ print_debug_line_section (Dwfl_Module *dwflmod, Ebl
*ebl, GElf_Ehdr *ehdr,
or DW_LNS_advance_pc (as per DWARF4 6.2.5.1). */
unsigned int op_addr_advance;
bool show_op_index;
- inline void advance_pc (unsigned int op_advance)
- {
- op_addr_advance = minimum_instr_len * ((op_index + op_advance)
- / max_ops_per_instr);
- address += op_addr_advance;
- show_op_index = (op_index > 0 ||
-(op_index + op_advance) % max_ops_per_instr > 0);
- op_index = (op_index + op_advance) % max_ops_per_instr;
- }
if (max_ops_per_instr == 0)
{
@@ -8792,7 +8800,9 @@ print_debug_line_section (Dwfl_Module *dwflmod, Ebl *ebl,
GElf_Ehdr *ehdr,
/* Perform the increments. */
line += line_increment;
- advance_pc ((opcode - opcode_base) / line_range);
+ advance_pc ((opcode - opcode_base) / line_range,
+ max_ops_per_instr, minimum_instr_len, &address,
+ &op_addr_advance, &op_index, &show_op_index);
printf (_(" special opcode %u: address+%u = "),
opcode, op_addr_advance);
@@ -8910,7 +8920,9 @@ print_debug_line_section (Dwfl_Module *dwflmod, Ebl *ebl,
GElf_Ehdr *ehdr,
if (lineendp - linep < 1)
goto invalid_unit;
get_uleb128 (u128, linep, lineendp);
- advance_pc (u128);
+ advance_pc (u128, max_ops_per_instr, minimum_instr_len,
+ &address, &op_addr_advance, &op_index,
+ &show_op_index);
{
printf (_(" advance address by %u to "),
op_addr_advance);
@@ -8971,7 +8983,10 @@ print_debug_line_section (Dwfl_Module *dwflmod, Ebl
*ebl, GElf_Ehdr *ehdr,
if (unlikely (line_range == 0))
goto invalid_unit;
- advance_pc ((255 - opcode_base) / line_range);
+ advance_pc ((255 - opcode_base) / line_range,
+ max_ops_per_instr, minimum_instr_len,
+ &address, &op_addr_advance, &op_index,
+ &show_op_index);
{
printf (_(" advance address by constant %u to "),
op_addr_advance);
--
2.26.2
Re: Remove nested functions from readelf.c
On 01/02/2021 15:21, Mark Wielaard wrote: On Fri, 2021-01-08 at 09:16 +0100, Timm Bäder via Elfutils-devel wrote: here another round for src/readelf.c. I think they are simple, but I'm not happy with the advance_pc() commit. I'm open for suggestions here but I can't come up with something better right now. I'll keep looking in to that in the meantime. Yeah, the advance_pc function is really why you want nested functions in the first place IMHO. Passing around the captured state as 6 extra arguments seems to not really make the code much clearer. Especially because on its own it isn't immediately clear what the code is about or that it is to be used as part of the special opcode or DW_LNS_advance_pc opcode (where DW_LNS_const_add_pc acts like special opcode 255) handling. It might be helpful to spell out in a comment to the function which part of the DWARF4 6.2.5.1 Special Opcodes handling the advance_pc function is responsible for. But honestly in this case I think just keeping the nested function is the cleanest way to handle this. Cheers, Mark That's what I was worried about as well of course. But I think I can work on this a bit and solve it in a cleaner way, with some additional refactorings. I'll try to come up with some patches. - Timm -- Red Hat GmbH, http://www.de.redhat.com/, Registered seat: Grasbrunn, Commercial register: Amtsgericht Muenchen, HRB 153243, Managing Directors: Charles Cachera, Michael O'Neill, Tom Savage, Eric Shander
[PATCH] readelf: Remove show_op_index variable
From: Timm Bäder
advance_pc() uses show_op_index to save whether the current op_index is
> 0 OR the new op_index is > 0.
The new op index is calculated via
new_op_index = (op_index + op_advance) % max_ops_per_instr;
since all of the variables involved are unsigned,
new_op_index >= op_index is always true.
So...
if op_index > 0, then new_op_index > 0
if op_index == 0, then new_op_index >= 0
and if the new_op_index is > 0, then the old one was as well.
In any case, we only need to check the new_op_index, since show_op_index
used to OR the two comparisons.
In other words:
op_index > 0 | new_op_index > 0 || show_op_index
true truetrue
false truetrue
true falsetrue xx
false falsefalse
... but since the third line (marked with xx) is not possible,
the table becomes:
op_index > 0 | new_op_index > 0 || show_op_index
true truetrue
false truetrue
false falsefalse
... and show_op_index is equal to (new_op_index > 0).
So, remove the show_op_index variable and simply replace it by comparing
the new op_index > 0.
---
src/ChangeLog | 5 +
src/readelf.c | 9 +++--
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/src/ChangeLog b/src/ChangeLog
index 1f53ca7b..9e30df31 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,8 @@
+2021-02-02 Timm Bäder
+
+ * readelf.c (print_debug_line_section): Remove unnecessary
+ show_op_index variable, replace with (op_index > 0).
+
2021-01-08 Timm Bäder
* readelf.c (print_cfa_program): Lift regname function to...
diff --git a/src/readelf.c b/src/readelf.c
index 9943c211..11692bb5 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -8752,14 +8752,11 @@ print_debug_line_section (Dwfl_Module *dwflmod, Ebl
*ebl, GElf_Ehdr *ehdr,
/* Apply the "operation advance" from a special opcode
or DW_LNS_advance_pc (as per DWARF4 6.2.5.1). */
unsigned int op_addr_advance;
- bool show_op_index;
inline void advance_pc (unsigned int op_advance)
{
op_addr_advance = minimum_instr_len * ((op_index + op_advance)
/ max_ops_per_instr);
address += op_addr_advance;
- show_op_index = (op_index > 0 ||
-(op_index + op_advance) % max_ops_per_instr > 0);
op_index = (op_index + op_advance) % max_ops_per_instr;
}
@@ -8803,7 +8800,7 @@ print_debug_line_section (Dwfl_Module *dwflmod, Ebl *ebl,
GElf_Ehdr *ehdr,
printf (_(" special opcode %u: address+%u = "),
opcode, op_addr_advance);
print_dwarf_addr (dwflmod, 0, address, address);
- if (show_op_index)
+ if (op_index > 0)
printf (_(", op_index = %u, line%+d = %zu\n"),
op_index, line_increment, line);
else
@@ -8921,7 +8918,7 @@ print_debug_line_section (Dwfl_Module *dwflmod, Ebl *ebl,
GElf_Ehdr *ehdr,
printf (_(" advance address by %u to "),
op_addr_advance);
print_dwarf_addr (dwflmod, 0, address, address);
- if (show_op_index)
+ if (op_index > 0)
printf (_(", op_index to %u"), op_index);
printf ("\n");
}
@@ -8982,7 +8979,7 @@ print_debug_line_section (Dwfl_Module *dwflmod, Ebl *ebl,
GElf_Ehdr *ehdr,
printf (_(" advance address by constant %u to "),
op_addr_advance);
print_dwarf_addr (dwflmod, 0, address, address);
- if (show_op_index)
+ if (op_index > 0)
printf (_(", op_index to %u"), op_index);
printf ("\n");
}
--
2.26.2
Re: [PATCH 3/4] ar: Pull should_truncate_fname() into file scope
On 29/01/2021 21:48, Mark Wielaard wrote: Hi Timm, On Fri, 2021-01-08 at 09:13 +0100, Timm Bäder via Elfutils-devel wrote: Get rid of a nested function this way. Skipping this one for now since I don't believe I understand this code. Could you explain what the code does what it does and why it works fine when moved this way? Right, I guess you mean because the old code used to assign a new value to name_max, but the new code doesn't do that? I fixed that locally now by taking a size_t* inout argument instead, so assigning to name_max should have the desired effect again. - Timm -- Red Hat GmbH, http://www.de.redhat.com/, Registered seat: Grasbrunn, Commercial register: Amtsgericht Muenchen, HRB 153243, Managing Directors: Charles Cachera, Michael O'Neill, Tom Savage, Eric Shander
[PATCH] ar: Pull should_truncate_fname() into file scope
From: Timm Bäder
Get rid of a nested function this way.
Signed-off-by: Timm Bäder
---
src/ar.c | 34 +-
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/src/ar.c b/src/ar.c
index 2a17d0d3..66b2c4fd 100644
--- a/src/ar.c
+++ b/src/ar.c
@@ -436,6 +436,21 @@ copy_content (Elf *elf, int newfd, off_t off, size_t n)
return write_retry (newfd, rawfile + off, n) != (ssize_t) n;
}
+static inline bool
+should_truncate_fname (size_t *name_max)
+{
+ if (errno == ENAMETOOLONG && allow_truncate_fname)
+{
+ if (*name_max == 0)
+ {
+ long int len = pathconf (".", _PC_NAME_MAX);
+ if (len > 0)
+ *name_max = len;
+ }
+ return *name_max != 0;
+}
+ return false;
+}
static int
do_oper_extract (int oper, const char *arfname, char **argv, int argc,
@@ -445,21 +460,6 @@ do_oper_extract (int oper, const char *arfname, char
**argv, int argc,
memset (found, '\0', sizeof (found));
size_t name_max = 0;
- inline bool should_truncate_fname (void)
- {
-if (errno == ENAMETOOLONG && allow_truncate_fname)
- {
- if (name_max == 0)
- {
- long int len = pathconf (".", _PC_NAME_MAX);
- if (len > 0)
- name_max = len;
- }
- return name_max != 0;
- }
-return false;
- }
-
off_t index_off = -1;
size_t index_size = 0;
off_t cur_off = SARMAG;
@@ -615,7 +615,7 @@ do_oper_extract (int oper, const char *arfname, char
**argv, int argc,
{
int printlen = INT_MAX;
- if (should_truncate_fname ())
+ if (should_truncate_fname (&name_max))
{
/* Try to truncate the name. First find out by how
much. */
@@ -704,7 +704,7 @@ do_oper_extract (int oper, const char *arfname, char
**argv, int argc,
{
int printlen = INT_MAX;
- if (should_truncate_fname ())
+ if (should_truncate_fname (&name_max))
{
/* Try to truncate the name. First find out by how
much. */
--
2.26.2
[PATCH] Pull header reading code into libdwP.h
From: Timm Bäder
src/readelf.c and libdw/dwarf_getsrclines.c contain the same code to
read header_length, unit_length, minimum_instr_len, etc.
Pull this code out into libdwP.h and into a header_state struct that
goes with the line_state struct in dwarf_getsrclines.c.
Signed-off-by: Timm Bäder
---
libdw/dwarf_getsrclines.c | 172 ---
libdw/libdwP.h| 149 ++
src/readelf.c | 184 +++---
3 files changed, 237 insertions(+), 268 deletions(-)
diff --git a/libdw/dwarf_getsrclines.c b/libdw/dwarf_getsrclines.c
index d6a581ad..c54b3bb7 100644
--- a/libdw/dwarf_getsrclines.c
+++ b/libdw/dwarf_getsrclines.c
@@ -96,12 +96,12 @@ struct line_state
};
static inline void
-run_advance_pc (struct line_state *state, unsigned int op_advance,
-uint_fast8_t minimum_instr_len, uint_fast8_t max_ops_per_instr)
+run_advance_pc (struct line_state *state, const struct header_state *header,
+ unsigned int op_advance)
{
- state->addr += minimum_instr_len * ((state->op_index + op_advance)
- / max_ops_per_instr);
- state->op_index = (state->op_index + op_advance) % max_ops_per_instr;
+ state->addr += header->minimum_instr_len * ((state->op_index + op_advance)
+ / header->max_ops_per_instr);
+ state->op_index = (state->op_index + op_advance) % header->max_ops_per_instr;
}
static inline bool
@@ -183,6 +183,8 @@ read_srclines (Dwarf *dbg,
.discriminator = 0
};
+ struct header_state header;
+
/* The dirs normally go on the stack, but if there are too many
we alloc them all. Set up stack storage early, so we can check on
error if we need to free them or not. */
@@ -194,103 +196,14 @@ read_srclines (Dwarf *dbg,
struct dirlist dirstack[MAX_STACK_DIRS];
struct dirlist *dirarray = dirstack;
- if (unlikely (linep + 4 > lineendp))
+ int header_parse_error = header_state_parse (dbg, &header, &linep, &lineendp,
+ address_size);
+ if (header_parse_error != DWARF_E_NOERROR)
{
invalid_data:
__libdw_seterrno (DWARF_E_INVALID_DEBUG_LINE);
goto out;
}
-
- Dwarf_Word unit_length = read_4ubyte_unaligned_inc (dbg, linep);
- unsigned int length = 4;
- if (unlikely (unit_length == DWARF3_LENGTH_64_BIT))
-{
- if (unlikely (linep + 8 > lineendp))
- goto invalid_data;
- unit_length = read_8ubyte_unaligned_inc (dbg, linep);
- length = 8;
-}
-
- /* Check whether we have enough room in the section. */
- if (unlikely (unit_length > (size_t) (lineendp - linep)))
-goto invalid_data;
- lineendp = linep + unit_length;
-
- /* The next element of the header is the version identifier. */
- if ((size_t) (lineendp - linep) < 2)
-goto invalid_data;
- uint_fast16_t version = read_2ubyte_unaligned_inc (dbg, linep);
- if (unlikely (version < 2) || unlikely (version > 5))
-{
- __libdw_seterrno (DWARF_E_VERSION);
- goto out;
-}
-
- /* DWARF5 explicitly lists address and segment_selector sizes. */
- if (version >= 5)
-{
- if ((size_t) (lineendp - linep) < 2)
- goto invalid_data;
- size_t line_address_size = *linep++;
- size_t segment_selector_size = *linep++;
- if (line_address_size != address_size || segment_selector_size != 0)
- goto invalid_data;
-}
-
- /* Next comes the header length. */
- Dwarf_Word header_length;
- if (length == 4)
-{
- if ((size_t) (lineendp - linep) < 4)
- goto invalid_data;
- header_length = read_4ubyte_unaligned_inc (dbg, linep);
-}
- else
-{
- if ((size_t) (lineendp - linep) < 8)
- goto invalid_data;
- header_length = read_8ubyte_unaligned_inc (dbg, linep);
-}
- const unsigned char *header_start = linep;
-
- /* Next the minimum instruction length. */
- uint_fast8_t minimum_instr_len = *linep++;
-
- /* Next the maximum operations per instruction, in version 4 format. */
- uint_fast8_t max_ops_per_instr = 1;
- if (version >= 4)
-{
- if (unlikely ((size_t) (lineendp - linep) < 1))
- goto invalid_data;
- max_ops_per_instr = *linep++;
- if (unlikely (max_ops_per_instr == 0))
- goto invalid_data;
-}
-
- /* 4 more bytes, is_stmt, line_base, line_range and opcode_base. */
- if ((size_t) (lineendp - linep) < 4)
-goto invalid_data;
-
- /* Then the flag determining the default value of the is_stmt
- register. */
- uint_fast8_t default_is_stmt = *linep++;
-
- /* Now the line base. */
- int_fast8_t line_base = (int8_t) *linep++;
-
- /* And the line range. */
- uint_fast8_t line_range = *linep++;
-
- /* The opcode base. */
- uint_fast8_t opcode_base = *linep++;
-
- /* Remember array with the standard opcode length (-1 to account for
- the opcode with value zero not being mentioned). */
- cons
Pull header reading code into libdwP.h
There is quite a bit of code duplication between src/readelf.c and libdw/dwarf_getsrclines.c when parsing header_length, unit_length, etc. Pull the code out into libdwP.h and use it in both of those files. It doesn't save a much code as I'd hoped and I'm not sure about the naming, but here's the patch. Thanks, Timm
[PATCH 1/2] elflint: Pull pos() info file scope
From: Timm Bäder
Rename it to buffer_pos() to be a bit more descriptive and get rid of a
nested function this way.
Signed-off-by: Timm Bäder
---
src/elflint.c | 39 ---
1 file changed, 20 insertions(+), 19 deletions(-)
diff --git a/src/elflint.c b/src/elflint.c
index 6a946838..4df6f6e5 100644
--- a/src/elflint.c
+++ b/src/elflint.c
@@ -3428,6 +3428,12 @@ section [%2d] '%s': unknown parent version '%s'\n"),
}
}
+static inline size_t
+buffer_pos (Elf_Data *data, const unsigned char *p)
+{
+ return p - (const unsigned char *) data->d_buf;
+}
+
static void
check_attributes (Ebl *ebl, GElf_Ehdr *ehdr, GElf_Shdr *shdr, int idx)
{
@@ -3446,11 +3452,6 @@ check_attributes (Ebl *ebl, GElf_Ehdr *ehdr, GElf_Shdr
*shdr, int idx)
return;
}
- inline size_t pos (const unsigned char *p)
- {
-return p - (const unsigned char *) data->d_buf;
- }
-
const unsigned char *p = data->d_buf;
if (*p++ != 'A')
{
@@ -3472,7 +3473,7 @@ check_attributes (Ebl *ebl, GElf_Ehdr *ehdr, GElf_Shdr
*shdr, int idx)
if (len == 0)
ERROR (_("\
section [%2d] '%s': offset %zu: zero length field in attribute section\n"),
- idx, section_name (ebl, idx), pos (p));
+ idx, section_name (ebl, idx), buffer_pos (data, p));
if (MY_ELFDATA != ehdr->e_ident[EI_DATA])
CONVERT (len);
@@ -3481,7 +3482,7 @@ section [%2d] '%s': offset %zu: zero length field in
attribute section\n"),
{
ERROR (_("\
section [%2d] '%s': offset %zu: invalid length in attribute section\n"),
-idx, section_name (ebl, idx), pos (p));
+idx, section_name (ebl, idx), buffer_pos (data, p));
break;
}
@@ -3493,7 +3494,7 @@ section [%2d] '%s': offset %zu: invalid length in
attribute section\n"),
{
ERROR (_("\
section [%2d] '%s': offset %zu: unterminated vendor name string\n"),
-idx, section_name (ebl, idx), pos (p));
+idx, section_name (ebl, idx), buffer_pos (data, p));
break;
}
++q;
@@ -3510,7 +3511,7 @@ section [%2d] '%s': offset %zu: unterminated vendor name
string\n"),
{
ERROR (_("\
section [%2d] '%s': offset %zu: endless ULEB128 in attribute subsection
tag\n"),
- idx, section_name (ebl, idx), pos (chunk));
+ idx, section_name (ebl, idx), buffer_pos (data, chunk));
break;
}
@@ -3519,7 +3520,7 @@ section [%2d] '%s': offset %zu: endless ULEB128 in
attribute subsection tag\n"),
{
ERROR (_("\
section [%2d] '%s': offset %zu: truncated attribute section\n"),
- idx, section_name (ebl, idx), pos (q));
+ idx, section_name (ebl, idx), buffer_pos (data, q));
break;
}
@@ -3528,7 +3529,7 @@ section [%2d] '%s': offset %zu: truncated attribute
section\n"),
{
ERROR (_("\
section [%2d] '%s': offset %zu: zero length field in attribute subsection\n"),
- idx, section_name (ebl, idx), pos (q));
+ idx, section_name (ebl, idx), buffer_pos (data, q));
q += sizeof subsection_len;
continue;
@@ -3543,7 +3544,7 @@ section [%2d] '%s': offset %zu: zero length field in
attribute subsection\n"),
{
ERROR (_("\
section [%2d] '%s': offset %zu: invalid length in attribute subsection\n"),
- idx, section_name (ebl, idx), pos (q));
+ idx, section_name (ebl, idx), buffer_pos (data, q));
break;
}
@@ -3554,7 +3555,7 @@ section [%2d] '%s': offset %zu: invalid length in
attribute subsection\n"),
if (subsection_tag != 1) /* Tag_File */
ERROR (_("\
section [%2d] '%s': offset %zu: attribute subsection has unexpected tag %u\n"),
-idx, section_name (ebl, idx), pos (chunk), subsection_tag);
+idx, section_name (ebl, idx), buffer_pos (data, chunk),
subsection_tag);
else
{
chunk += sizeof subsection_len;
@@ -3572,7 +3573,7 @@ section [%2d] '%s': offset %zu: attribute subsection has
unexpected tag %u\n"),
{
ERROR (_("\
section [%2d] '%s': offset %zu: endless ULEB128 in attribute tag\n"),
- idx, section_name (ebl, idx), pos (chunk));
+ idx, section_name (ebl, idx), buffer_pos
(data, chunk));
break;
}
}
@@ -3583,7 +3584,7 @@ section [%2d] '%s': offset %zu: endless ULEB128 in
attribute tag\n"),
{
ERROR (_("\
section [%2d] '%s': offset %zu: unterminated string in attribute\n"),
-
[PATCH 2/2] elflint: Pull left() in file scope
From: Timm Bäder
And rename it to buffer_left() to be a bit more descriptive
Signed-off-by: Timm Bäder
---
src/elflint.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/src/elflint.c b/src/elflint.c
index 4df6f6e5..85cc7833 100644
--- a/src/elflint.c
+++ b/src/elflint.c
@@ -3434,6 +3434,12 @@ buffer_pos (Elf_Data *data, const unsigned char *p)
return p - (const unsigned char *) data->d_buf;
}
+inline size_t
+buffer_left (Elf_Data *data, const unsigned char *p)
+{
+ return (const unsigned char *) data->d_buf + data->d_size - p;
+}
+
static void
check_attributes (Ebl *ebl, GElf_Ehdr *ehdr, GElf_Shdr *shdr, int idx)
{
@@ -3460,12 +3466,7 @@ check_attributes (Ebl *ebl, GElf_Ehdr *ehdr, GElf_Shdr
*shdr, int idx)
return;
}
- inline size_t left (void)
- {
-return (const unsigned char *) data->d_buf + data->d_size - p;
- }
-
- while (left () >= 4)
+ while (buffer_left (data, p) >= 4)
{
uint32_t len;
memcpy (&len, p, sizeof len);
@@ -3478,7 +3479,7 @@ section [%2d] '%s': offset %zu: zero length field in
attribute section\n"),
if (MY_ELFDATA != ehdr->e_ident[EI_DATA])
CONVERT (len);
- if (len > left ())
+ if (len > buffer_left (data, p))
{
ERROR (_("\
section [%2d] '%s': offset %zu: invalid length in attribute section\n"),
@@ -3614,7 +3615,7 @@ section [%2d] '%s': offset %zu: vendor '%s' unknown\n"),
idx, section_name (ebl, idx), buffer_pos (data, p), name);
}
- if (left () != 0)
+ if (buffer_left (data, p) != 0)
ERROR (_("\
section [%2d] '%s': offset %zu: extra bytes after last attribute section\n"),
idx, section_name (ebl, idx), buffer_pos (data, p));
--
2.26.2
[PATCH 1/5] unstrip: Pull adjust_reloc() into file scope
From: Timm Bäder
Get rid of a nested function this way.
Signed-off-by: Timm Bäder
---
src/unstrip.c | 28 +++-
1 file changed, 15 insertions(+), 13 deletions(-)
diff --git a/src/unstrip.c b/src/unstrip.c
index 85803295..6e874c3a 100644
--- a/src/unstrip.c
+++ b/src/unstrip.c
@@ -432,6 +432,19 @@ update_sh_size (Elf_Scn *outscn, const Elf_Data *data)
update_shdr (outscn, newshdr);
}
+static inline void
+adjust_reloc (GElf_Xword *info,
+ size_t map[], size_t map_size)
+{
+ size_t ndx = GELF_R_SYM (*info);
+ if (ndx != STN_UNDEF)
+{
+ if (ndx > map_size)
+ error (EXIT_FAILURE, 0, "bad symbol ndx section");
+ *info = GELF_R_INFO (map[ndx - 1], GELF_R_TYPE (*info));
+}
+}
+
/* Update relocation sections using the symbol table. */
static void
adjust_relocs (Elf_Scn *outscn, Elf_Scn *inscn, const GElf_Shdr *shdr,
@@ -439,17 +452,6 @@ adjust_relocs (Elf_Scn *outscn, Elf_Scn *inscn, const
GElf_Shdr *shdr,
{
Elf_Data *data = elf_getdata (outscn, NULL);
- inline void adjust_reloc (GElf_Xword *info)
-{
- size_t ndx = GELF_R_SYM (*info);
- if (ndx != STN_UNDEF)
- {
- if (ndx > map_size)
- error (EXIT_FAILURE, 0, "bad symbol ndx section");
- *info = GELF_R_INFO (map[ndx - 1], GELF_R_TYPE (*info));
- }
-}
-
switch (shdr->sh_type)
{
case SHT_REL:
@@ -460,7 +462,7 @@ adjust_relocs (Elf_Scn *outscn, Elf_Scn *inscn, const
GElf_Shdr *shdr,
{
GElf_Rel rel_mem;
GElf_Rel *rel = gelf_getrel (data, i, &rel_mem);
- adjust_reloc (&rel->r_info);
+ adjust_reloc (&rel->r_info, map, map_size);
ELF_CHECK (gelf_update_rel (data, i, rel),
_("cannot update relocation: %s"));
}
@@ -474,7 +476,7 @@ adjust_relocs (Elf_Scn *outscn, Elf_Scn *inscn, const
GElf_Shdr *shdr,
{
GElf_Rela rela_mem;
GElf_Rela *rela = gelf_getrela (data, i, &rela_mem);
- adjust_reloc (&rela->r_info);
+ adjust_reloc (&rela->r_info, map, map_size);
ELF_CHECK (gelf_update_rela (data, i, rela),
_("cannot update relocation: %s"));
}
--
2.26.2
[PATCH 2/5] unstrip: Pull check_match() into file scope
From: Timm Bäder
Get rid of a nested function this way.
Signed-off-by: Timm Bäder
---
src/unstrip.c | 28
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/src/unstrip.c b/src/unstrip.c
index 6e874c3a..72fabac8 100644
--- a/src/unstrip.c
+++ b/src/unstrip.c
@@ -1065,6 +1065,20 @@ get_group_sig (Elf *elf, GElf_Shdr *shdr)
return sig;
}
+static inline bool
+check_match (bool match, Elf_Scn *scn, const char *name)
+{
+ if (!match)
+{
+ error (0, 0, _("cannot find matching section for [%zu] '%s'"),
+elf_ndxscn (scn), name);
+ return true;
+}
+
+ return false;
+}
+
+
/* Fix things up when prelink has moved some allocated sections around
and the debuginfo file's section headers no longer match up.
This fills in SECTIONS[0..NALLOC-1].outscn or exits.
@@ -1200,16 +1214,6 @@ find_alloc_sections_prelink (Elf *debug, Elf_Data
*debug_shstrtab,
}
bool fail = false;
- inline void check_match (bool match, Elf_Scn *scn, const char *name)
-{
- if (!match)
- {
- fail = true;
- error (0, 0, _("cannot find matching section for [%zu] '%s'"),
-elf_ndxscn (scn), name);
- }
-}
-
Elf_Scn *scn = NULL;
while ((scn = elf_nextscn (debug, scn)) != NULL)
{
@@ -1240,7 +1244,7 @@ find_alloc_sections_prelink (Elf *debug, Elf_Data
*debug_shstrtab,
for (size_t i = 0; shdr != NULL && i < nalloc; ++i)
if (sections[i].outscn == scn)
shdr = NULL;
- check_match (shdr == NULL, scn, name);
+ fail |= check_match (shdr == NULL, scn, name);
}
if (fail)
@@ -1296,7 +1300,7 @@ find_alloc_sections_prelink (Elf *debug, Elf_Data
*debug_shstrtab,
}
}
- check_match (undo_sec == NULL, scn, name);
+ fail |= check_match (undo_sec == NULL, scn, name);
}
free (undo_sections);
--
2.26.2
[PATCH 3/5] unstrip: Inline find_unalloc_section() into only caller
From: Timm Bäder
Get rid of an unnecessary nested function this way.
Signed-off-by: Timm Bäder
---
src/unstrip.c | 47 +--
1 file changed, 21 insertions(+), 26 deletions(-)
diff --git a/src/unstrip.c b/src/unstrip.c
index 72fabac8..90e02831 100644
--- a/src/unstrip.c
+++ b/src/unstrip.c
@@ -1452,29 +1452,6 @@ more sections in stripped file than debug file --
arguments reversed?"));
stripped_symtab = §ions[nalloc];
}
- /* Locate a matching unallocated section in SECTIONS. */
- inline struct section *find_unalloc_section (const GElf_Shdr *shdr,
- const char *name,
- const char *sig)
-{
- size_t l = nalloc, u = stripped_shnum - 1;
- while (l < u)
- {
- size_t i = (l + u) / 2;
- struct section *sec = §ions[i];
- int cmp = compare_unalloc_sections (shdr, &sec->shdr,
- name, sec->name,
- sig, sec->sig);
- if (cmp < 0)
- u = i;
- else if (cmp > 0)
- l = i + 1;
- else
- return sec;
- }
- return NULL;
-}
-
Elf_Data *shstrtab = elf_getdata (elf_getscn (unstripped,
unstripped_shstrndx), NULL);
ELF_CHECK (shstrtab != NULL,
@@ -1536,9 +1513,27 @@ more sections in stripped file than debug file --
arguments reversed?"));
}
else
{
- /* Look for the section that matches. */
- sec = find_unalloc_section (shdr, name,
- get_group_sig (unstripped, shdr));
+ /* Locate a matching unallocated section in SECTIONS. */
+ const char *sig = get_group_sig (unstripped, shdr);
+ size_t l = nalloc, u = stripped_shnum - 1;
+ while (l < u)
+ {
+ size_t i = (l + u) / 2;
+ struct section *section = §ions[i];
+ int cmp = compare_unalloc_sections (shdr, §ion->shdr,
+ name, section->name,
+ sig, section->sig);
+ if (cmp < 0)
+ u = i;
+ else if (cmp > 0)
+ l = i + 1;
+ else
+ {
+ sec = section;
+ break;
+ }
+ }
+
if (sec == NULL)
{
/* An additional unallocated section is fine if not SHT_NOBITS.
--
2.26.2
[PATCH 4/5] unstrip: Pull warn() into file scope
From: Timm Bäder
Get rid of a nested function this way.
Signed-off-by: Timm Bäder
---
src/unstrip.c | 35 ---
1 file changed, 20 insertions(+), 15 deletions(-)
diff --git a/src/unstrip.c b/src/unstrip.c
index 90e02831..3822a19b 100644
--- a/src/unstrip.c
+++ b/src/unstrip.c
@@ -2225,22 +2225,23 @@ open_file (const char *file, bool writable)
return fd;
}
+/* Warn, and exit if not forced to continue, if some ELF header
+ sanity check for the stripped and unstripped files failed. */
+static void
+warn (const char *msg, bool force,
+ const char *stripped_file, const char *unstripped_file)
+{
+ error (force ? 0 : EXIT_FAILURE, 0, "%s'%s' and '%s' %s%s.",
+force ? _("WARNING: ") : "",
+stripped_file, unstripped_file, msg,
+force ? "" : _(", use --force"));
+}
+
/* Handle a pair of files we need to open by name. */
static void
handle_explicit_files (const char *output_file, bool create_dirs, bool force,
const char *stripped_file, const char *unstripped_file)
{
-
- /* Warn, and exit if not forced to continue, if some ELF header
- sanity check for the stripped and unstripped files failed. */
- void warn (const char *msg)
- {
-error (force ? 0 : EXIT_FAILURE, 0, "%s'%s' and '%s' %s%s.",
- force ? _("WARNING: ") : "",
- stripped_file, unstripped_file, msg,
- force ? "" : _(", use --force"));
- }
-
int stripped_fd = open_file (stripped_file, false);
Elf *stripped = elf_begin (stripped_fd, ELF_C_READ, NULL);
GElf_Ehdr stripped_ehdr;
@@ -2261,16 +2262,20 @@ handle_explicit_files (const char *output_file, bool
create_dirs, bool force,
if (memcmp (stripped_ehdr.e_ident,
unstripped_ehdr.e_ident, EI_NIDENT) != 0)
- warn (_("ELF header identification (e_ident) different"));
+ warn (_("ELF header identification (e_ident) different"), force,
+ stripped_file, unstripped_file);
if (stripped_ehdr.e_type != unstripped_ehdr.e_type)
- warn (_("ELF header type (e_type) different"));
+ warn (_("ELF header type (e_type) different"), force,
+ stripped_file, unstripped_file);
if (stripped_ehdr.e_machine != unstripped_ehdr.e_machine)
- warn (_("ELF header machine type (e_machine) different"));
+ warn (_("ELF header machine type (e_machine) different"), force,
+ stripped_file, unstripped_file);
if (stripped_ehdr.e_phnum < unstripped_ehdr.e_phnum)
- warn (_("stripped program header (e_phnum) smaller than unstripped"));
+ warn (_("stripped program header (e_phnum) smaller than unstripped"),
force,
+ stripped_file, unstripped_file);
}
handle_file (output_file, create_dirs, stripped, &stripped_ehdr, unstripped);
--
2.26.2
[PATCH 5/5] unstrip: Remove nested next() function
From: Timm Bäder
This is a simple one-liner, so inline this into the few callers.
Get rid of a nested function this way.
Signed-off-by: Timm Bäder
---
src/unstrip.c | 14 ++
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/src/unstrip.c b/src/unstrip.c
index 3822a19b..caa72a93 100644
--- a/src/unstrip.c
+++ b/src/unstrip.c
@@ -2498,21 +2498,18 @@ static void
handle_implicit_modules (const struct arg_info *info)
{
struct match_module_info mmi = { info->args, NULL, info->match_files };
- inline ptrdiff_t next (ptrdiff_t offset)
-{
- return dwfl_getmodules (info->dwfl, &match_module, &mmi, offset);
-}
- ptrdiff_t offset = next (0);
+ ptrdiff_t offset = dwfl_getmodules (info->dwfl, &match_module, &mmi, 0);
if (offset == 0)
error (EXIT_FAILURE, 0, _("no matching modules found"));
if (info->list)
do
list_module (mmi.found);
-while ((offset = next (offset)) > 0);
+while ((offset = dwfl_getmodules (info->dwfl, &match_module, &mmi,
+ offset)) > 0);
else if (info->output_dir == NULL)
{
- if (next (offset) != 0)
+ if (dwfl_getmodules (info->dwfl, &match_module, &mmi, offset) != 0)
error (EXIT_FAILURE, 0, _("matched more than one module"));
handle_dwfl_module (info->output_file, false, info->force, mmi.found,
info->all, info->ignore, info->relocate);
@@ -2522,7 +2519,8 @@ handle_implicit_modules (const struct arg_info *info)
handle_output_dir_module (info->output_dir, mmi.found, info->force,
info->all, info->ignore,
info->modnames, info->relocate);
-while ((offset = next (offset)) > 0);
+while ((offset = dwfl_getmodules (info->dwfl, &match_module, &mmi,
+ offset)) > 0);
}
int
--
2.26.2
[PATCH 1/4] elfcompress: Pull set_section() into file scope
From: Timm Bäder
Get rid of a nested function this way.
Signed-off-by: Timm Bäder
---
src/elfcompress.c | 16 +---
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/src/elfcompress.c b/src/elfcompress.c
index 1b5b1e36..65a922a7 100644
--- a/src/elfcompress.c
+++ b/src/elfcompress.c
@@ -52,6 +52,8 @@ static const char *foutput = NULL;
#define T_DECOMPRESS 1/* none */
#define T_COMPRESS_ZLIB 2 /* zlib */
#define T_COMPRESS_GNU 3 /* zlib-gnu */
+#define WORD_BITS (8U * sizeof (unsigned int))
+
static int type = T_UNSET;
struct section_pattern
@@ -242,6 +244,12 @@ compress_section (Elf_Scn *scn, size_t orig_size, const
char *name,
return res;
}
+static void
+set_section (unsigned int *sections, size_t ndx)
+{
+ sections[ndx / WORD_BITS] |= (1U << (ndx % WORD_BITS));
+}
+
static int
process_file (const char *fname)
{
@@ -275,12 +283,6 @@ process_file (const char *fname)
/* How many sections are we talking about? */
size_t shnum = 0;
-#define WORD_BITS (8U * sizeof (unsigned int))
- void set_section (size_t ndx)
- {
-sections[ndx / WORD_BITS] |= (1U << (ndx % WORD_BITS));
- }
-
bool get_section (size_t ndx)
{
return (sections[ndx / WORD_BITS] & (1U << (ndx % WORD_BITS))) != 0;
@@ -498,7 +500,7 @@ process_file (const char *fname)
else if (shdr->sh_type != SHT_NOBITS
&& (shdr->sh_flags & SHF_ALLOC) == 0)
{
- set_section (ndx);
+ set_section (sections, ndx);
/* Check if we might want to change this section name. */
if (! adjust_names
&& ((type != T_COMPRESS_GNU
--
2.26.2
[PATCH 2/4] elfcompress: Pull get_section() into file scope
From: Timm Bäder
Get rid of a nested function this way.
Signed-off-by: Timm Bäder
---
src/elfcompress.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/src/elfcompress.c b/src/elfcompress.c
index 65a922a7..2dc74a0c 100644
--- a/src/elfcompress.c
+++ b/src/elfcompress.c
@@ -250,6 +250,12 @@ set_section (unsigned int *sections, size_t ndx)
sections[ndx / WORD_BITS] |= (1U << (ndx % WORD_BITS));
}
+static bool
+get_section (unsigned int *sections, size_t ndx)
+{
+ return (sections[ndx / WORD_BITS] & (1U << (ndx % WORD_BITS))) != 0;
+}
+
static int
process_file (const char *fname)
{
@@ -283,11 +289,6 @@ process_file (const char *fname)
/* How many sections are we talking about? */
size_t shnum = 0;
- bool get_section (size_t ndx)
- {
-return (sections[ndx / WORD_BITS] & (1U << (ndx % WORD_BITS))) != 0;
- }
-
/* How many sections are we going to change? */
size_t get_sections (void)
{
@@ -689,7 +690,7 @@ process_file (const char *fname)
/* (de)compress if section matched. */
char *sname = NULL;
char *newname = NULL;
- if (get_section (ndx))
+ if (get_section (sections, ndx))
{
GElf_Shdr shdr_mem;
GElf_Shdr *shdr = gelf_getshdr (scn, &shdr_mem);
--
2.26.2
[PATCH 3/4] elfcompress: Pull get_sections() into file scope
From: Timm Bäder
Get rid of a nested function this way.
Signed-off-by: Timm Bäder
---
src/elfcompress.c | 21 +++--
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/src/elfcompress.c b/src/elfcompress.c
index 2dc74a0c..65e28f0e 100644
--- a/src/elfcompress.c
+++ b/src/elfcompress.c
@@ -256,6 +256,16 @@ get_section (unsigned int *sections, size_t ndx)
return (sections[ndx / WORD_BITS] & (1U << (ndx % WORD_BITS))) != 0;
}
+/* How many sections are we going to change? */
+static size_t
+get_sections (unsigned int *sections, size_t shnum)
+{
+ size_t s = 0;
+ for (size_t i = 0; i < shnum / WORD_BITS + 1; i++)
+s += __builtin_popcount (sections[i]);
+ return s;
+}
+
static int
process_file (const char *fname)
{
@@ -289,15 +299,6 @@ process_file (const char *fname)
/* How many sections are we talking about? */
size_t shnum = 0;
- /* How many sections are we going to change? */
- size_t get_sections (void)
- {
-size_t s = 0;
-for (size_t i = 0; i < shnum / WORD_BITS + 1; i++)
- s += __builtin_popcount (sections[i]);
-return s;
- }
-
int cleanup (int res)
{
elf_end (elf);
@@ -552,7 +553,7 @@ process_file (const char *fname)
}
}
- if (foutput == NULL && get_sections () == 0)
+ if (foutput == NULL && get_sections (sections, shnum) == 0)
{
if (verbose > 0)
printf ("Nothing to do.\n");
--
2.26.2
[PATCH 4/4] elfcompress: Replace cleanup() with label
From: Timm Bäder
Get rid of a nested function this way.
Signed-off-by: Timm Bäder
---
src/elfcompress.c | 218 +++---
1 file changed, 109 insertions(+), 109 deletions(-)
diff --git a/src/elfcompress.c b/src/elfcompress.c
index 65e28f0e..ba08e73f 100644
--- a/src/elfcompress.c
+++ b/src/elfcompress.c
@@ -298,47 +298,13 @@ process_file (const char *fname)
/* How many sections are we talking about? */
size_t shnum = 0;
-
- int cleanup (int res)
- {
-elf_end (elf);
-close (fd);
-
-elf_end (elfnew);
-close (fdnew);
-
-if (fnew != NULL)
- {
- unlink (fnew);
- free (fnew);
- fnew = NULL;
- }
-
-free (snamebuf);
-if (names != NULL)
- {
- dwelf_strtab_free (names);
- free (scnstrents);
- free (symstrents);
- free (namesbuf);
- if (scnnames != NULL)
- {
- for (size_t n = 0; n < shnum; n++)
- free (scnnames[n]);
- free (scnnames);
- }
- }
-
-free (sections);
-
-return res;
- }
+ int res = 0;
fd = open (fname, O_RDONLY);
if (fd < 0)
{
error (0, errno, "Couldn't open %s\n", fname);
- return cleanup (-1);
+ goto error;
}
elf = elf_begin (fd, ELF_C_READ, NULL);
@@ -346,7 +312,7 @@ process_file (const char *fname)
{
error (0, 0, "Couldn't open ELF file %s for reading: %s",
fname, elf_errmsg (-1));
- return cleanup (-1);
+ goto error;
}
/* We don't handle ar files (or anything else), we probably should. */
@@ -357,21 +323,21 @@ process_file (const char *fname)
error (0, 0, "Cannot handle ar files: %s", fname);
else
error (0, 0, "Unknown file type: %s", fname);
- return cleanup (-1);
+ goto error;
}
struct stat st;
if (fstat (fd, &st) != 0)
{
error (0, errno, "Couldn't fstat %s", fname);
- return cleanup (-1);
+ goto error;
}
GElf_Ehdr ehdr;
if (gelf_getehdr (elf, &ehdr) == NULL)
{
error (0, 0, "Couldn't get ehdr for %s: %s", fname, elf_errmsg (-1));
- return cleanup (-1);
+ goto error;
}
/* Get the section header string table. */
@@ -380,7 +346,7 @@ process_file (const char *fname)
{
error (0, 0, "Couldn't get section header string table index in %s: %s",
fname, elf_errmsg (-1));
- return cleanup (-1);
+ goto error;
}
/* How many sections are we talking about? */
@@ -388,13 +354,13 @@ process_file (const char *fname)
{
error (0, 0, "Couldn't get number of sections in %s: %s",
fname, elf_errmsg (1));
- return cleanup (-1);
+ goto error;
}
if (shnum == 0)
{
error (0, 0, "ELF file %s has no sections", fname);
- return cleanup (-1);
+ goto error;
}
sections = xcalloc (shnum / 8 + 1, sizeof (unsigned int));
@@ -403,7 +369,7 @@ process_file (const char *fname)
if (elf_getphdrnum (elf, &phnum) != 0)
{
error (0, 0, "Couldn't get phdrnum: %s", elf_errmsg (-1));
- return cleanup (-1);
+ goto error;
}
/* Whether we need to adjust any section names (going to/from GNU
@@ -460,7 +426,7 @@ process_file (const char *fname)
{
error (0, 0, "Unexpected section number %zd, expected only %zd",
ndx, shnum);
- cleanup (-1);
+ goto error;
}
GElf_Shdr shdr_mem;
@@ -468,14 +434,14 @@ process_file (const char *fname)
if (shdr == NULL)
{
error (0, 0, "Couldn't get shdr for section %zd", ndx);
- return cleanup (-1);
+ goto error;
}
const char *sname = elf_strptr (elf, shdrstrndx, shdr->sh_name);
if (sname == NULL)
{
error (0, 0, "Couldn't get name for section %zd", ndx);
- return cleanup (-1);
+ goto error;
}
if (section_name_matches (sname))
@@ -536,7 +502,7 @@ process_file (const char *fname)
{
error (0, 0,
"Multiple symbol tables (%zd, %zd) using the same
string table unsupported", symtabndx, ndx);
- return cleanup (-1);
+ goto error;
}
symtabndx = ndx;
}
@@ -558,7 +524,7 @@ process_file (const char *fname)
if (verbose > 0)
printf ("Nothing to do.\n");
fnew = NULL;
- return cleanup (0);
+ goto cleanup;
}
if (adjust_names)
@@ -567,7 +533,7 @@ process_file (const char *fname)
if (names == NULL)
{
error (0, 0, "Not enough memory for new strtab");
- return cleanup (-1);
+ goto error;
}
scnstrents = xmalloc (shnum
* sizeof (Dwelf_Strent *));
@@ -594,7 +560,7 @@ process_file (const char *fname)
/* Since we didn't create it we don't want to try to unlink
[PATCH 1/2] elfstrmerge: Pull newsecndx() info file scope
From: Timm Bäder
Get rid of a nested function this way.
Signed-off-by: Timm Bäder
---
tests/elfstrmerge.c | 76 +
1 file changed, 42 insertions(+), 34 deletions(-)
diff --git a/tests/elfstrmerge.c b/tests/elfstrmerge.c
index abbdf3fd..00f025ec 100644
--- a/tests/elfstrmerge.c
+++ b/tests/elfstrmerge.c
@@ -147,6 +147,33 @@ fail_elf_idx (const char *msg, const char *fname, size_t
idx)
abort();
}
+/* section index mapping and sanity checking. */
+static size_t
+newsecndx (size_t secndx, size_t shdrstrndx, size_t shdrnum,
+ const char *fname,
+ const char *what, size_t widx,
+ const char *member, size_t midx)
+{
+ if (unlikely (secndx == 0 || secndx == shdrstrndx || secndx >= shdrnum))
+{
+ /* Don't use fail... too specialized messages. Call release
+outselves and then error. Ignores midx if widx is
+zero. */
+ release ();
+ if (widx == 0)
+ error (1, 0, "%s: bad section index %zd in %s for %s",
+ fname, secndx, what, member);
+ else if (midx == 0)
+ error (1, 0, "%s: bad section index %zd in %s %zd for %s",
+ fname, secndx, what, widx, member);
+ else
+ error (1, 0, "%s: bad section index %zd in %s %zd for %s %zd",
+ fname, secndx, what, widx, member, midx);
+}
+
+ return secndx < shdrstrndx ? secndx : secndx - 1;
+}
+
int
main (int argc, char **argv)
{
@@ -325,30 +352,6 @@ main (int argc, char **argv)
if (newstrtabdata.d_size >= shdrstrshdr->sh_size + strtabshdr->sh_size)
fail ("Impossible, merged string table is larger", fname);
- /* section index mapping and sanity checking. */
- size_t newsecndx (size_t secndx, const char *what, size_t widx,
- const char *member, size_t midx)
- {
-if (unlikely (secndx == 0 || secndx == shdrstrndx || secndx >= shdrnum))
- {
- /* Don't use fail... too specialized messages. Call release
- ourselves and then error. Ignores midx if widx is
- zero. */
- release ();
- if (widx == 0)
- error (1, 0, "%s: bad section index %zd in %s for %s",
-fname, secndx, what, member);
- else if (midx == 0)
- error (1, 0, "%s: bad section index %zd in %s %zd for %s",
-fname, secndx, what, widx, member);
- else
- error (1, 0, "%s: bad section index %zd in %s %zd for %s %zd",
-fname, secndx, what, widx, member, midx);
- }
-
-return secndx < shdrstrndx ? secndx : secndx - 1;
- }
-
struct stat st;
if (fstat (fd, &st) != 0)
fail_errno("Couldn't fstat", fname);
@@ -392,7 +395,8 @@ main (int argc, char **argv)
newehdr.e_flags = ehdr.e_flags;
/* The new file uses the new strtab as shstrtab. */
- size_t newstrtabndx = newsecndx (strtabndx, "ehdr", 0, "e_shstrndx", 0);
+ size_t newstrtabndx = newsecndx (strtabndx, shdrstrndx, shdrnum,
+ fname, "ehdr", 0, "e_shstrndx", 0);
if (newstrtabndx < SHN_LORESERVE)
newehdr.e_shstrndx = newstrtabndx;
else
@@ -460,11 +464,14 @@ main (int argc, char **argv)
newshdr.sh_addr = shdr->sh_addr;
newshdr.sh_size = shdr->sh_size;
if (shdr->sh_link != 0)
- newshdr.sh_link = newsecndx (shdr->sh_link, "shdr", ndx, "sh_link", 0);
+ newshdr.sh_link = newsecndx (shdr->sh_link, shdrstrndx, shdrnum,
+fname, "shdr", ndx, "sh_link", 0);
else
newshdr.sh_link = 0;
if (SH_INFO_LINK_P (shdr) && shdr->sh_info != 0)
- newshdr.sh_info = newsecndx (shdr->sh_info, "shdr", ndx, "sh_info", 0);
+ newshdr.sh_info = newsecndx (shdr->sh_info, shdrstrndx, shdrnum,
+fname, "shdr", ndx, "sh_info", 0);
+
else
newshdr.sh_info = shdr->sh_info;
newshdr.sh_entsize = shdr->sh_entsize;
@@ -481,7 +488,8 @@ main (int argc, char **argv)
void *b = malloc (d->d_size);
if (b == NULL)
fail_idx ("Couldn't allocated buffer for section", NULL, ndx);
- newscnbufs[newsecndx (ndx, "section", ndx, "d_buf", 0)] = d->d_buf = b;
+ newscnbufs[newsecndx (ndx, shdrstrndx, shdrnum, fname,
+ "section", ndx, "d_buf", 0)] = d->d_buf = b;
}
Elf_Data *newdata = elf_newdata (newscn);
@@ -526,8 +534,8 @@ main (int argc, char **argv)
" for old shdrstrndx %zd\n", ndx, i, shdrstrndx);
else if (sym.st_shndx != SHN_UNDEF
&& sym.st_shndx < SHN_LORESERVE)
- sym.st_shndx = newsecndx (sym.st_shndx, "section", ndx,
- "symbol", i);
+ sym.st_shndx = newsecndx (sym.st_shndx, shdrstrndx,
shdrnum,
+fname, "section", ndx, "symbol", i);
if (update_name && sym.st_
[PATCH 2/2] elfstrmerge: Pull new_data_buf() into file scope
From: Timm Bäder
Get rid of a nested function this way.
Signed-off-by: Timm Bäder
---
tests/elfstrmerge.c | 32 +---
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/tests/elfstrmerge.c b/tests/elfstrmerge.c
index 00f025ec..197c6a5d 100644
--- a/tests/elfstrmerge.c
+++ b/tests/elfstrmerge.c
@@ -174,6 +174,20 @@ newsecndx (size_t secndx, size_t shdrstrndx, size_t
shdrnum,
return secndx < shdrstrndx ? secndx : secndx - 1;
}
+static void
+new_data_buf (Elf_Data *d, const char *fname,
+ size_t ndx, size_t shdrstrndx, size_t shdrnum)
+{
+ size_t s = d->d_size;
+ if (s == 0)
+fail_idx ("Expected data in section", fname, ndx);
+ void *b = malloc (d->d_size);
+ if (b == NULL)
+fail_idx ("Couldn't allocated buffer for section", NULL, ndx);
+ newscnbufs[newsecndx (ndx, shdrstrndx, shdrnum, fname,
+ "section", ndx, "d_buf", 0)] = d->d_buf = b;
+}
+
int
main (int argc, char **argv)
{
@@ -480,18 +494,6 @@ main (int argc, char **argv)
manipulate the original data. Allocate and check here, so we
have a list of all data buffers we might need to release when
done. */
- void new_data_buf (Elf_Data *d)
- {
- size_t s = d->d_size;
- if (s == 0)
- fail_idx ("Expected data in section", fname, ndx);
- void *b = malloc (d->d_size);
- if (b == NULL)
- fail_idx ("Couldn't allocated buffer for section", NULL, ndx);
- newscnbufs[newsecndx (ndx, shdrstrndx, shdrnum, fname,
- "section", ndx, "d_buf", 0)] = d->d_buf = b;
- }
-
Elf_Data *newdata = elf_newdata (newscn);
if (newdata == NULL)
fail_elf_idx ("Couldn't create new data for section", fnew, ndx);
@@ -518,7 +520,7 @@ main (int argc, char **argv)
const bool update_name = shdr->sh_link == strtabndx;
if (update_name && ndx != symtabndx)
fail ("Only one symbol table using strtab expected", fname);
- new_data_buf (newdata);
+ new_data_buf (newdata, fname, ndx, shdrstrndx, shdrnum);
size_t syms = (data->d_size
/ gelf_fsize (elf, ELF_T_SYM, 1, EV_CURRENT));
for (size_t i = 0; i < syms; i++)
@@ -549,7 +551,7 @@ main (int argc, char **argv)
case SHT_GROUP:
{
- new_data_buf (newdata);
+ new_data_buf (newdata, fname, ndx, shdrstrndx, shdrnum);
/* A section group contains Elf32_Words. The first
word is a flag value, the rest of the words are
indexes of the sections belonging to the group. */
@@ -567,7 +569,7 @@ main (int argc, char **argv)
case SHT_SYMTAB_SHNDX:
{
- new_data_buf (newdata);
+ new_data_buf (newdata, fname, ndx, shdrstrndx, shdrnum);
/* A SHNDX just contains an array of section indexes
for the corresponding symbol table. The entry is
SHN_UNDEF unless the corresponding symbol is
--
2.26.2
[PATCH 3/3] build: Check for -Wno-packed-not-aligned support
From: Timm Bäder Clang does not support this warning, so check for compiler support before using it. Signed-off-by: Timm Bäder --- config/eu.am | 10 -- configure.ac | 9 + 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/config/eu.am b/config/eu.am index 02512570..2c3e4571 100644 --- a/config/eu.am +++ b/config/eu.am @@ -79,6 +79,12 @@ else TRAMPOLINES_WARNING= endif +if HAVE_NO_PACKED_NOT_ALIGNED_WARNING +NO_PACKED_NOT_ALIGNED_WARNING=-Wno-packed-not-aligned +else +NO_PACKED_NOT_ALIGNED_WARNING= +endif + AM_CFLAGS = -std=gnu99 -Wall -Wshadow -Wformat=2 \ -Wold-style-definition -Wstrict-prototypes $(TRAMPOLINES_WARNING) \ $(LOGICAL_OP_WARNING) $(DUPLICATED_COND_WARNING) \ @@ -86,7 +92,7 @@ AM_CFLAGS = -std=gnu99 -Wall -Wshadow -Wformat=2 \ $(if $($(*F)_no_Werror),,-Werror) \ $(if $($(*F)_no_Wunused),,-Wunused -Wextra) \ $(if $($(*F)_no_Wstack_usage),,$(STACK_USAGE_WARNING)) \ - $(if $($(*F)_no_Wpacked_not_aligned),-Wno-packed-not-aligned,) \ + $(if $($(*F)_no_Wpacked_not_aligned),$(NO_PACKED_NOT_ALIGNED_WARNING),) \ $($(*F)_CFLAGS) AM_CXXFLAGS = -std=c++11 -Wall -Wshadow \ @@ -96,7 +102,7 @@ AM_CXXFLAGS = -std=c++11 -Wall -Wshadow \ $(if $($(*F)_no_Werror),,-Werror) \ $(if $($(*F)_no_Wunused),,-Wunused -Wextra) \ $(if $($(*F)_no_Wstack_usage),,$(STACK_USAGE_WARNING)) \ - $(if $($(*F)_no_Wpacked_not_aligned),-Wno-packed-not-aligned,) \ + $(if $($(*F)_no_Wpacked_not_aligned),$(NO_PACKED_NOT_ALIGNED_WARNING),) \ $($(*F)_CXXFLAGS) COMPILE.os = $(filter-out -fprofile-arcs -ftest-coverage, $(COMPILE)) diff --git a/configure.ac b/configure.ac index 5f3321aa..aa8439e8 100644 --- a/configure.ac +++ b/configure.ac @@ -546,6 +546,15 @@ CFLAGS="$old_CFLAGS"]) AM_CONDITIONAL(HAVE_TRAMPOLINES_WARNING, [test "x$ac_cv_trampolines" != "xno"]) +AC_CACHE_CHECK([whether the compiler accepts -Wno-packed-not-aligned], ac_cv_no_packed_not_aligned, [dnl +old_CFLAGS="$CFLAGS" +CFLAGS="$CFLAGS -Wno-packed-not-aligned -Werror" +AC_COMPILE_IFELSE([AC_LANG_SOURCE([])], + ac_cv_no_packed_not_aligned=yes, ac_cv_no_packed_not_aligned=no) +CFLAGS="$old_CFLAGS"]) +AM_CONDITIONAL(HAVE_NO_PACKED_NOT_ALIGNED_WARNING, + [test "x$ac_cv_no_packed_not_aligned" != "xno"]) + saved_LIBS="$LIBS" AC_SEARCH_LIBS([argp_parse], [argp]) LIBS="$saved_LIBS" -- 2.26.2
[PATCH 1/3] build: Check for -Wimplicit-fallthrough=5 separately
From: Timm Bäder GCC accepts the =5, which means it doesn't try to parse any comments and only accepts the fallthrough attribute in code. Clang does not ever parse any comments and always wants the fallthrough attribute anyway. Clang also doesn't accept the =n parameter for -Wimplicit-fallthrough. Test for =5 separately and use it if supported and fall back to just -Wimplicit-fallthrough otherwise. Signed-off-by: Timm Bäder --- config/eu.am | 4 configure.ac | 12 2 files changed, 16 insertions(+) diff --git a/config/eu.am b/config/eu.am index 6c3c444f..e109ffd3 100644 --- a/config/eu.am +++ b/config/eu.am @@ -64,8 +64,12 @@ endif if HAVE_IMPLICIT_FALLTHROUGH_WARNING # Use strict fallthrough. Only __attribute__((fallthrough)) will prevent the # warning +if HAVE_IMPLICIT_FALLTHROUGH_5_WARNING IMPLICIT_FALLTHROUGH_WARNING=-Wimplicit-fallthrough=5 else +IMPLICIT_FALLTHROUGH_WARNING=-Wimplicit-fallthrough +endif +else IMPLICIT_FALLTHROUGH_WARNING= endif diff --git a/configure.ac b/configure.ac index d345495d..e56aeb6a 100644 --- a/configure.ac +++ b/configure.ac @@ -519,6 +519,18 @@ CFLAGS="$old_CFLAGS"]) AM_CONDITIONAL(HAVE_IMPLICIT_FALLTHROUGH_WARNING, [test "x$ac_cv_implicit_fallthrough" != "xno"]) +# Check whether the compiler additionally accepts -Wimplicit-fallthrough=5 +# GCC accepts this and 5 means "don't parse any fallthrough comments and +# only accept the fallthrough attribute" +AC_CACHE_CHECK([whether the compiler accepts -Wimplicit-fallthrough=5], ac_cv_implicit_fallthrough_5, [dnl +old_CFLAGS="$CFLAGS" +CFLAGS="$CFLAGS -Wimplicit-fallthrough=5 -Werror" +AC_COMPILE_IFELSE([AC_LANG_SOURCE([])], + ac_cv_implicit_fallthrough_5=yes, ac_cv_implicit_fallthrough_5=no) +CFLAGS="$old_CFLAGS"]) +AM_CONDITIONAL(HAVE_IMPLICIT_FALLTHROUGH_5_WARNING, + [test "x$ac_cv_implicit_fallthrough_5" != "xno"]) + # Assume the fallthrough attribute is supported if -Wimplict-fallthrough is supported if test "$ac_cv_implicit_fallthrough" = "yes"; then AC_DEFINE([HAVE_FALLTHROUGH], [1], -- 2.26.2
[PATCH 2/3] build: Check for -Wtrampolines support
From: Timm Bäder Clang does not support -Wtrampolines, so check if the compiler supports it before using it. Signed-off-by: Timm Bäder --- config/eu.am | 10 -- configure.ac | 9 + 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/config/eu.am b/config/eu.am index e109ffd3..02512570 100644 --- a/config/eu.am +++ b/config/eu.am @@ -73,8 +73,14 @@ else IMPLICIT_FALLTHROUGH_WARNING= endif +if HAVE_TRAMPOLINES_WARNING +TRAMPOLINES_WARNING=-Wtrampolines +else +TRAMPOLINES_WARNING= +endif + AM_CFLAGS = -std=gnu99 -Wall -Wshadow -Wformat=2 \ - -Wold-style-definition -Wstrict-prototypes -Wtrampolines \ + -Wold-style-definition -Wstrict-prototypes $(TRAMPOLINES_WARNING) \ $(LOGICAL_OP_WARNING) $(DUPLICATED_COND_WARNING) \ $(NULL_DEREFERENCE_WARNING) $(IMPLICIT_FALLTHROUGH_WARNING) \ $(if $($(*F)_no_Werror),,-Werror) \ @@ -84,7 +90,7 @@ AM_CFLAGS = -std=gnu99 -Wall -Wshadow -Wformat=2 \ $($(*F)_CFLAGS) AM_CXXFLAGS = -std=c++11 -Wall -Wshadow \ - -Wtrampolines \ + $(TRAMPOLINES_WARNING) \ $(LOGICAL_OP_WARNING) $(DUPLICATED_COND_WARNING) \ $(NULL_DEREFERENCE_WARNING) $(IMPLICIT_FALLTHROUGH_WARNING) \ $(if $($(*F)_no_Werror),,-Werror) \ diff --git a/configure.ac b/configure.ac index e56aeb6a..5f3321aa 100644 --- a/configure.ac +++ b/configure.ac @@ -537,6 +537,15 @@ if test "$ac_cv_implicit_fallthrough" = "yes"; then [Defined if __attribute__((fallthrough)) is supported]) fi +AC_CACHE_CHECK([whether the compiler accepts -Wtrampolines], ac_cv_trampolines, [dnl +old_CFLAGS="$CFLAGS" +CFLAGS="$CFLAGS -Wtrampolines -Werror" +AC_COMPILE_IFELSE([AC_LANG_SOURCE([])], + ac_cv_trampolines=yes, ac_cv_trampolines=no) +CFLAGS="$old_CFLAGS"]) +AM_CONDITIONAL(HAVE_TRAMPOLINES_WARNING, + [test "x$ac_cv_trampolines" != "xno"]) + saved_LIBS="$LIBS" AC_SEARCH_LIBS([argp_parse], [argp]) LIBS="$saved_LIBS" -- 2.26.2
Remove misleading XOR
Hi, I'm mainly leaving this patch here to discuss what to do about it. Clang complains because of -Wxor-used-as-pow. So of course one possibility is to use -Wno-xor-used-as-pow. It also prints '0x2 ^ NO_RESIZING' as an alternative to silence the warning. Given the comment just above the changed line, I think the 0x2 version is better than what I have in the patch now, since it still uses NO_RESIZING. But I'm open for suggestions on how to keep the code as clear as possible. Thanks, Timm
[PATCH] Remove misleading XOR
From: Timm Bäder Clang complains about this because it has -Wxor-used-as-pow: ../../lib/dynamicsizehash_concurrent.c:288:61: error: result of 'CLEANING ^ NO_RESIZING' is 2; did you mean '1 << NO_RESIZING' (1)? [-Werror,-Wxor-used-as-pow] atomic_fetch_xor_explicit(&htab->resizing_state, CLEANING ^ NO_RESIZING, ~^ 1 ../../lib/dynamicsizehash_concurrent.c:288:61: note: replace expression with '0x2 ^ NO_RESIZING' to silence this warning The result of CLEANING ^ NO_RESIZING is CLEANING, since NO_RESIZING is 0. --- lib/dynamicsizehash_concurrent.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/dynamicsizehash_concurrent.c b/lib/dynamicsizehash_concurrent.c index 2d53bec6..7d314d32 100644 --- a/lib/dynamicsizehash_concurrent.c +++ b/lib/dynamicsizehash_concurrent.c @@ -285,7 +285,7 @@ resize_master(NAME *htab) free(htab->old_table); /* Change state to NO_RESIZING */ - atomic_fetch_xor_explicit(&htab->resizing_state, CLEANING ^ NO_RESIZING, + atomic_fetch_xor_explicit(&htab->resizing_state, CLEANING, memory_order_relaxed); } -- 2.26.2
Re: Remove misleading XOR
On 17/02/2021 17:49, Mark Wielaard wrote: I think both the comment and the warning message are a little misleading. The comment should really read "Change state from CLEANING to NO_RESIZING". You are right that NO_RESIZING is just zero, but still removing it seems to make the code less clear. All this is slightly confusing since it is doing "double" xor. First it does an OLD_STATE_BITS ^ NEW_STATE_BITS and then a atomic_fetch_xor of the result of that xor with the resizing_state variable, Which is expected to be OLD_STATE, so the bits that are left should be the NEW_STATE bits after the double xor. This is how I understood the code. Given that all of the atomic_fetch_xor_explicit() calls in that file preceded by a comment explaining what they do, it might be useful to have a function change_resizing_state(htab, from, to, memory_order) instead and remove the comments. That also works around the clang warning of course, but those are not the only places where resizing_state is modified, so it's weird in a different way. Could you see if something like this works around the warning? diff --git a/lib/dynamicsizehash_concurrent.c b/lib/dynamicsizehash_concurrent.c index 2d53bec6..0283eea1 100644 --- a/lib/dynamicsizehash_concurrent.c +++ b/lib/dynamicsizehash_concurrent.c @@ -160,10 +160,10 @@ insert_helper (NAME *htab, HASHTYPE hval, TYPE val) } } -#define NO_RESIZING 0u -#define ALLOCATING_MEMORY 1u -#define MOVING_DATA 3u -#define CLEANING 2u +#define NO_RESIZING 0x0 +#define ALLOCATING_MEMORY 0x1 +#define MOVING_DATA 0x3 +#define CLEANING 0x2 Nope, same result. :( -- Red Hat GmbH, http://www.de.redhat.com/, Registered seat: Grasbrunn, Commercial register: Amtsgericht Muenchen, HRB 153243, Managing Directors: Charles Cachera, Michael O'Neill, Tom Savage, Eric Shander
Re: Remove misleading XOR
On 24/02/2021 14:16, Mark Wielaard wrote:
Hmmm. I am not sure why that doesn't work. What if you make them
explicitly unsinged (adding u at the end). Or does it simply ignore
the values and just warn because it sees the macro name and not an
explicit number?
I think I don't really understand anymore what this warning is warning
about. Could you give an example of where this would flag something
that we would like to fix?
I found https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90885, which has a
few more use-cases (in the form of tweets but anyway). GCC might also
get this warning in the future.
For the clang implementation, see https://reviews.llvm.org/D63423
So it only seems to warn for 2 ^ x because that's a common error:
#define NO_RESIZING 0x0u
#define CLEANING0x2u
int main(void) {
unsigned p = CLEANING ^ NO_RESIZING;
unsigned j = NO_RESIZING ^ CLEANING;
unsigned k = 17 ^ 3;
}
This code only warns for the declaration of p.
Maybe it is easiest to just suppress this warning, when is it enabled?
That's fine with me, I just didn't want to add compiler-specific flags
but I guess we can just check for -Wno-xor-used-as-pow and add that when
the compiler supports it.
- Timm
--
Red Hat GmbH, http://www.de.redhat.com/, Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Michael O'Neill, Tom Savage, Eric
Shander
Re: Remove misleading XOR
On 24/02/2021 14:46, Timm Bäder wrote: On 24/02/2021 14:16, Mark Wielaard wrote: Hmmm. I am not sure why that doesn't work. What if you make them explicitly unsinged (adding u at the end). Or does it simply ignore the values and just warn because it sees the macro name and not an explicit number? I think I don't really understand anymore what this warning is warning about. Could you give an example of where this would flag something that we would like to fix? I found https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90885, which has a few more use-cases (in the form of tweets but anyway). GCC might also get this warning in the future. For the clang implementation, see https://reviews.llvm.org/D63423 Looking at the code, it has an explicit "don't diagnose binary, hexadecimal, octal literals" comment. Your suggestion of changing to hexadecimal literals would work, but clang doesn't resolve the macros first, see https://godbolt.org/z/3EMYY6. -- Red Hat GmbH, http://www.de.redhat.com/, Registered seat: Grasbrunn, Commercial register: Amtsgericht Muenchen, HRB 153243, Managing Directors: Charles Cachera, Michael O'Neill, Tom Savage, Eric Shander
Re: Remove misleading XOR
On 24/02/2021 14:16, Mark Wielaard wrote:> Hmmm. I am not sure why that doesn't work. What if you make them explicitly unsinged (adding u at the end). Or does it simply ignore the values and just warn because it sees the macro name and not an explicit number? Another viable workaround is to just use variables instead of macros: https://godbolt.org/z/9zEEve -- Red Hat GmbH, http://www.de.redhat.com/, Registered seat: Grasbrunn, Commercial register: Amtsgericht Muenchen, HRB 153243, Managing Directors: Charles Cachera, Michael O'Neill, Tom Savage, Eric Shander
Re: Remove misleading XOR
On 24/02/2021 14:16, Mark Wielaard wrote:>>> -#define NO_RESIZING 0u -#define ALLOCATING_MEMORY 1u -#define MOVING_DATA 3u -#define CLEANING 2u +#define NO_RESIZING 0x0 +#define ALLOCATING_MEMORY 0x1 +#define MOVING_DATA 0x3 +#define CLEANING 0x2 Nope, same result. :( Hmmm. I am not sure why that doesn't work. What if you make them explicitly unsinged (adding u at the end). Or does it simply ignore the values and just warn because it sees the macro name and not an explicit number? I opened https://reviews.llvm.org/D97445 upstream and it looks like the behavior with both LHS and RHS being a macro is just a bug. If that patch gets accepted, it will fix the problems in the elfutils code base. Would still be nice to have a work-around in the meantime I think. - Timm -- Red Hat GmbH, http://www.de.redhat.com/, Registered seat: Grasbrunn, Commercial register: Amtsgericht Muenchen, HRB 153243, Managing Directors: Charles Cachera, Michael O'Neill, Tom Savage, Eric Shander
[PATCH] elfcompress: Replace cleanup() with label
From: Timm Bäder
This unifies the error handling with the rest of the code base and gets
rid of a nested function.
Signed-off-by: Timm Bäder
---
src/ChangeLog | 6 ++
src/elfcompress.c | 215 +++---
2 files changed, 112 insertions(+), 109 deletions(-)
diff --git a/src/ChangeLog b/src/ChangeLog
index 29f04e71..791015bb 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,9 @@
+2021-03-02 Timm Bäder
+
+ * elfcompress.c (process_file): Remove cleanup() function and
+ replace it with a cleanup label at the end of the function.
+ Initialize res to -1.
+
2021-02-17 Timm Bäder
* elfcompress.c (process_file): Move get_sections function...
diff --git a/src/elfcompress.c b/src/elfcompress.c
index 65e28f0e..c5ba6c34 100644
--- a/src/elfcompress.c
+++ b/src/elfcompress.c
@@ -298,47 +298,13 @@ process_file (const char *fname)
/* How many sections are we talking about? */
size_t shnum = 0;
-
- int cleanup (int res)
- {
-elf_end (elf);
-close (fd);
-
-elf_end (elfnew);
-close (fdnew);
-
-if (fnew != NULL)
- {
- unlink (fnew);
- free (fnew);
- fnew = NULL;
- }
-
-free (snamebuf);
-if (names != NULL)
- {
- dwelf_strtab_free (names);
- free (scnstrents);
- free (symstrents);
- free (namesbuf);
- if (scnnames != NULL)
- {
- for (size_t n = 0; n < shnum; n++)
- free (scnnames[n]);
- free (scnnames);
- }
- }
-
-free (sections);
-
-return res;
- }
+ int res = -1;
fd = open (fname, O_RDONLY);
if (fd < 0)
{
error (0, errno, "Couldn't open %s\n", fname);
- return cleanup (-1);
+ goto cleanup;
}
elf = elf_begin (fd, ELF_C_READ, NULL);
@@ -346,7 +312,7 @@ process_file (const char *fname)
{
error (0, 0, "Couldn't open ELF file %s for reading: %s",
fname, elf_errmsg (-1));
- return cleanup (-1);
+ goto cleanup;
}
/* We don't handle ar files (or anything else), we probably should. */
@@ -357,21 +323,21 @@ process_file (const char *fname)
error (0, 0, "Cannot handle ar files: %s", fname);
else
error (0, 0, "Unknown file type: %s", fname);
- return cleanup (-1);
+ goto cleanup;
}
struct stat st;
if (fstat (fd, &st) != 0)
{
error (0, errno, "Couldn't fstat %s", fname);
- return cleanup (-1);
+ goto cleanup;
}
GElf_Ehdr ehdr;
if (gelf_getehdr (elf, &ehdr) == NULL)
{
error (0, 0, "Couldn't get ehdr for %s: %s", fname, elf_errmsg (-1));
- return cleanup (-1);
+ goto cleanup;
}
/* Get the section header string table. */
@@ -380,7 +346,7 @@ process_file (const char *fname)
{
error (0, 0, "Couldn't get section header string table index in %s: %s",
fname, elf_errmsg (-1));
- return cleanup (-1);
+ goto cleanup;
}
/* How many sections are we talking about? */
@@ -388,13 +354,13 @@ process_file (const char *fname)
{
error (0, 0, "Couldn't get number of sections in %s: %s",
fname, elf_errmsg (1));
- return cleanup (-1);
+ goto cleanup;
}
if (shnum == 0)
{
error (0, 0, "ELF file %s has no sections", fname);
- return cleanup (-1);
+ goto cleanup;
}
sections = xcalloc (shnum / 8 + 1, sizeof (unsigned int));
@@ -403,7 +369,7 @@ process_file (const char *fname)
if (elf_getphdrnum (elf, &phnum) != 0)
{
error (0, 0, "Couldn't get phdrnum: %s", elf_errmsg (-1));
- return cleanup (-1);
+ goto cleanup;
}
/* Whether we need to adjust any section names (going to/from GNU
@@ -460,7 +426,7 @@ process_file (const char *fname)
{
error (0, 0, "Unexpected section number %zd, expected only %zd",
ndx, shnum);
- cleanup (-1);
+ goto cleanup;
}
GElf_Shdr shdr_mem;
@@ -468,14 +434,14 @@ process_file (const char *fname)
if (shdr == NULL)
{
error (0, 0, "Couldn't get shdr for section %zd", ndx);
- return cleanup (-1);
+ goto cleanup;
}
const char *sname = elf_strptr (elf, shdrstrndx, shdr->sh_name);
if (sname == NULL)
{
error (0, 0, "Couldn't get name for section %zd", ndx);
- return cleanup (-1);
+ goto cleanup;
}
if (section_name_matches (sname))
@@ -536,7 +502,7 @@ process_file (const char *fname)
{
error (0, 0,
"Multiple symbol tables (%zd, %zd) using the same
string table unsupported", symtabndx, ndx);
- return cleanup (-1);
+ goto cleanup;
}
symtabndx = ndx;
}
@@ -558,7 +524,7 @@ process_file (const char *fname)
if (verbose > 0)
printf (
debuginfod: Remove always-false comparisons with LONG_MAX
If I understand the code correctly, these comparisons exist only for the curl_off_t cases, in which case dl and cl might be greater than LONG_MAX. However, in case they are declares as double values, this cannot be the case and the comparisons are unnecessary. - Timm
[PATCH] debuginfod-client: Remove always-false comparisons
From: Timm Bäder When comparing a long to a double, clang prints the following warning: ../../debuginfod/debuginfod-client.c:917:28: error: implicit conversion from 'long' to 'double' changes value from 9223372036854775807 to 9223372036854775808 [-Werror,-Wimplicit-int-float-conversion] pb = (cl > LONG_MAX ? LONG_MAX : (long)cl); ~ ^~~~ /usr/lib64/clang/10.0.1/include/limits.h:47:19: note: expanded from macro 'LONG_MAX' ^~~~ :38:22: note: expanded from here ^~~~ 1 error generated. However, cl and dl in are doubles and will never be greater than LONG_MAX, so always just assign them to pa or pb. Signed-off-by: Timm Bäder --- debuginfod/debuginfod-client.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c index de26af5b..c5f2ace5 100644 --- a/debuginfod/debuginfod-client.c +++ b/debuginfod/debuginfod-client.c @@ -896,7 +896,7 @@ debuginfod_query_server (debuginfod_client *c, CURLINFO_SIZE_DOWNLOAD, &dl); if (curl_res == 0) -pa = (dl > LONG_MAX ? LONG_MAX : (long)dl); +pa = (long)dl; #endif /* NB: If going through deflate-compressing proxies, this @@ -914,7 +914,7 @@ debuginfod_query_server (debuginfod_client *c, CURLINFO_CONTENT_LENGTH_DOWNLOAD, &cl); if (curl_res == 0) -pb = (cl > LONG_MAX ? LONG_MAX : (long)cl); +pb = (long)cl; #endif } -- 2.26.2
[PATCH] debuginfod-client: Don't compare a double to a long
From: Timm Bäder clang warns about this: ../../debuginfod/debuginfod-client.c:899:28: error: implicit conversion from 'long' to 'double' changes value from 9223372036854775807 to 9223372036854775808 [-Werror,-Wimplicit-int-float-conversion] pa = (dl > LONG_MAX ? LONG_MAX : (long)dl); ~ ^~~~ /usr/lib64/clang/10.0.1/include/limits.h:47:19: note: expanded from macro 'LONG_MAX' ^~~~ :38:22: note: expanded from here ^~~~ ../../debuginfod/debuginfod-client.c:917:28: error: implicit conversion from 'long' to 'double' changes value from 9223372036854775807 to 9223372036854775808 [-Werror,-Wimplicit-int-float-conversion] pb = (cl > LONG_MAX ? LONG_MAX : (long)cl); ~ ^~~~ /usr/lib64/clang/10.0.1/include/limits.h:47:19: note: expanded from macro 'LONG_MAX' ^~~~ :38:22: note: expanded from here ^~~~ --- debuginfod/debuginfod-client.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c index de26af5b..df238e07 100644 --- a/debuginfod/debuginfod-client.c +++ b/debuginfod/debuginfod-client.c @@ -896,7 +896,7 @@ debuginfod_query_server (debuginfod_client *c, CURLINFO_SIZE_DOWNLOAD, &dl); if (curl_res == 0) -pa = (dl > LONG_MAX ? LONG_MAX : (long)dl); +pa = (dl > (double)LONG_MAX ? LONG_MAX : (long)dl); #endif /* NB: If going through deflate-compressing proxies, this @@ -914,7 +914,7 @@ debuginfod_query_server (debuginfod_client *c, CURLINFO_CONTENT_LENGTH_DOWNLOAD, &cl); if (curl_res == 0) -pb = (cl > LONG_MAX ? LONG_MAX : (long)cl); +pb = (cl > (double)LONG_MAX ? LONG_MAX : (long)cl); #endif } -- 2.26.2
debuginfod-client: Fix typo in curl feature detection
I just looked at this for too long and became confused by the naming. It seems like there's one CURLINFO_ too much in there. Googling seems to confirm. - Timm
[PATCH] debginfod-client: Fix typo in curl feature detection
From: Timm Bäder CURLINFO_CURLINFO_CONTENT_LENGTH_DOWNLOAD_T is one CURLINFO_ too much. Signed-off-by: Timm Bäder --- debuginfod/debuginfod-client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c index df238e07..a1394737 100644 --- a/debuginfod/debuginfod-client.c +++ b/debuginfod/debuginfod-client.c @@ -901,7 +901,7 @@ debuginfod_query_server (debuginfod_client *c, /* NB: If going through deflate-compressing proxies, this number is likely to be unavailable, so -1 may show. */ -#ifdef CURLINFO_CURLINFO_CONTENT_LENGTH_DOWNLOAD_T +#ifdef CURLINFO_CONTENT_LENGTH_DOWNLOAD_T curl_off_t cl; curl_res = curl_easy_getinfo(target_handle, CURLINFO_CONTENT_LENGTH_DOWNLOAD_T, -- 2.26.2
Re: [PATCH] define SHT_LLVM_ADDRSIG section rather than error out
On 18/11/2020 06:34, Navin P via Elfutils-devel wrote: diff --git a/src/elflint.c b/src/elflint.c index ef3e3732..62663800 100644 --- a/src/elflint.c +++ b/src/elflint.c @@ -3905,6 +3905,7 @@ section [%2zu] '%s': size not multiple of entry size\n"), && shdr->sh_type != SHT_GNU_ATTRIBUTES && shdr->sh_type != SHT_GNU_LIBLIST && shdr->sh_type != SHT_CHECKSUM + && shdr->sh_type != SHT_LLVM_ADDRSIG && shdr->sh_type != SHT_GNU_verdef && shdr->sh_type != SHT_GNU_verneed && shdr->sh_type != SHT_GNU_versym Note that for various of these SHT_GNU extensions we actually do have some extra checks. Do we need to check anything for a section marked SHT_LLVM_ADDRSIG? We can do two things here a) Recognize the section exists but ignore its contents which is what i do. This needn't be the correct approach. You may need to check the contents to sht_llvm_addrsig but that is lot of work after the format has been frozen. https://www.mail-archive.com/netdev@vger.kernel.org/msg348254.html readelf output [22] .llvm_addrsig LOOS+0xfff4c03 ... b) If we don't want to recognize SHT_LLVM_ADDRSIG you can strip section from object file by objcopy -R .llvm_addrsig size.o conditionally based on clang compiler. Hey Navin and Mark, any update on this? I see that SHT_LLVM_ADDRSIG is still not in upstream glibc. Are you working on that, Navin? As for the checks, I'm not sure we can do anything here since elfutils can't know whether a symbol is rightfully marked as address-significant. Thanks, Timm -- Red Hat GmbH, http://www.de.redhat.com/, Registered seat: Grasbrunn, Commercial register: Amtsgericht Muenchen, HRB 153243, Managing Directors: Charles Cachera, Michael O'Neill, Tom Savage, Eric Shander
Re: [PATCH] define SHT_LLVM_ADDRSIG section rather than error out
On 04/03/2021 14:59, Mark Wielaard wrote:
Hi Timm,
On Thu, 2021-03-04 at 14:44 +0100, Timm Bäder wrote:
any update on this? I see that SHT_LLVM_ADDRSIG is still not in upstream
glibc. Are you working on that, Navin?
As for the checks, I'm not sure we can do anything here since elfutils
can't know whether a symbol is rightfully marked as address-significant.
I tried to lookup some more information about SHT_LLVM_ADDRSIG, but
couldn't really find much. There is this comment about it from the
binutils maintainers:
https://www.sourceware.org/bugzilla/show_bug.cgi?id=23817#c1
Hm, that is unfortunate of course. I can't find a bug report about this
in llvm's bugzilla. I could try to get a discussion going there.
Are you opposed to work around this in elflint in the meantime?
E.g. ignore all sections starting with 'llvm_'?
diff --git a/src/elflint.c b/src/elflint.c
index 85cc7833..9a40045c 100644
--- a/src/elflint.c
+++ b/src/elflint.c
@@ -3766,6 +3766,9 @@ cannot get section header for section [%2zu] '%s':
%s\n"),
const char *scnname = elf_strptr (ebl->elf, shstrndx,
shdr->sh_name);
+ if (strncmp (scnname, ".llvm_", 6) == 0)
+continue;
+
if (scnname == NULL)
ERROR (_("section [%2zu]: invalid name\n"), cnt);
else
(But I guess that belong after the NULL check)
Or does elflint use the any llvm_ section?
Asking because they only exist when compiling with clang of course and
elflint not handling (or ignoring) them causes the testsuite to fail.
- Timm
--
Red Hat GmbH, http://www.de.redhat.com/, Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Michael O'Neill, Tom Savage, Eric
Shander
[PATCH] readelf: Pull advance_pc() in file scope
From: Timm Bäder
Make advance_pc() a static function so we can get rid of another nested
function. Rename it to run_advance_pc() and use a local advance_pc()
macro to pass all the local variables. This is similar to what the
equivalent code in libdw/dwarf_getsrclines.c is doing.
Signed-off-by: Timm Bäder
---
src/readelf.c | 26 +++---
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/src/readelf.c b/src/readelf.c
index b9740455..c44d06d1 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -8373,6 +8373,23 @@ print_form_data (Dwarf *dbg, int form, const unsigned
char *readp,
return readp;
}
+/* Only used via run_advance_pc() macro */
+static inline void
+run_advance_pc (unsigned int op_advance,
+unsigned int minimum_instr_len,
+unsigned int max_ops_per_instr,
+unsigned int *op_addr_advance,
+Dwarf_Word *address,
+unsigned int *op_index)
+{
+ const unsigned int advanced_op_index = (*op_index) + op_advance;
+
+ *op_addr_advance = minimum_instr_len * (advanced_op_index
+ / max_ops_per_instr);
+ *address = *address + *op_addr_advance;
+ *op_index = advanced_op_index % max_ops_per_instr;
+}
+
static void
print_debug_line_section (Dwfl_Module *dwflmod, Ebl *ebl, GElf_Ehdr *ehdr,
Elf_Scn *scn, GElf_Shdr *shdr, Dwarf *dbg)
@@ -8763,13 +8780,8 @@ print_debug_line_section (Dwfl_Module *dwflmod, Ebl
*ebl, GElf_Ehdr *ehdr,
/* Apply the "operation advance" from a special opcode
or DW_LNS_advance_pc (as per DWARF4 6.2.5.1). */
unsigned int op_addr_advance;
- inline void advance_pc (unsigned int op_advance)
- {
- op_addr_advance = minimum_instr_len * ((op_index + op_advance)
- / max_ops_per_instr);
- address += op_addr_advance;
- op_index = (op_index + op_advance) % max_ops_per_instr;
- }
+#define advance_pc(op_advance) run_advance_pc(op_advance, minimum_instr_len, \
+ max_ops_per_instr, &op_addr_advance, &address, &op_index)
if (max_ops_per_instr == 0)
{
--
2.26.3
readelf: Pull advance_pc() in file scope
Giving this another shot since pulling all the parsing code into libdwP.h seems a bit overkill (and potentially buggy of course). The downside is that run_advance_pc() takes 6 arguments instead of the 4 that it takes in libdwarf_getsrclines.c
