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
Will eu-addr2line support debuginfod?
Hi, Is it planned to have eu-addr2line supporting debuginfod? I try to pass '[build-id]@address' (in the same format that eu-stack outputting) without success: # eu-stack -b -p 1 PID 1 - process TID 1: #0 0x7ffa8d4c2886 epoll_wait [c017df57d6194b6479cef409cba575bbaa537c94]@0x7ffa8d3c5000+0xfd886 ... $ export DEBUGINFOD_URLS=... $ eu-addr2line '[c017df57d6194b6479cef409cba575bbaa537c94]@0x7ffa8d3c5000+0xfd886' eu-addr2line: a.out: No such file or directory $ Thanks, ps. Additionally, I think, it would be extra cool to have addr2line request functionality added to debuginfod protocol. For example, to request like: '/buildid/123..DEF/addr2line/@0x7ffa8d3c5000+0xfd886'. Our experiments show, that to generate KDE stack traces via debuginfod requires gigabytes of traffic (downloading uncompressed .debug binary), which is many times more than installing (lzma compressed) -debuginfod .rpm file. Where, several remote addr2line requests would do the job.
Re: Will eu-addr2line support debuginfod?
Hi, Vitaly - > Is it planned to have eu-addr2line supporting debuginfod? I try to > pass '[build-id]@address' (in the same format that eu-stack outputting) > without success [...] For a broader discussion of the same topic, see: https://sourceware.org/bugzilla/show_bug.cgi?id=25793 I remain of the opinion that all elfutils tools should automagically attempt debuginfod queries, if they encounter incomplete data. If we cannot overcome objections to that, we may end up having to invent a new elfutils tool that does general elf/dwarf dumping. > ps. Additionally, I think, it would be extra cool to have addr2line request > functionality added to debuginfod protocol. For example, to request like: > '/buildid/123..DEF/addr2line/@0x7ffa8d3c5000+0xfd886'. Yes, adding higher level query operations to the webapi was in mind right from the beginning of the project. It's a matter of tasteful API design, analysis of other candidate client tools. I'd search for inspiration from, maybe those very same dumping tools contemplated by PR25793 and maybe complex tools like systemtap/gdb, and general query tools like dwgrep. A lot of work to do. > Our experiments show, that to generate KDE stack traces via > debuginfod requires gigabytes of traffic (downloading uncompressed > .debug binary), which is many times more than installing (lzma > compressed) -debuginfod .rpm file. Where, several remote addr2line > requests would do the job. You're right, debuginfod itself does not compress on its webapi - it ignores the Accept-Encoding request header. On a LAN-nearby server, that doesn't make any difference really. On a remote server, one should probably set up a reverse-proxy front-end such as httpd or nginx that does support compressed encodings, such as all of the public servers listed at https://sourceware.org/elfutils/Debuginfod.html . The debuginfod client will happily receive compressed-encoding responses from such servers. We could also entertain teaching debuginfod itself to compress on the fly. It's just a more tricky use of the libmicrohttpd apis. - FChE
[Bug tools/23787] eu-size: Bad handling of ar files inside are files
https://sourceware.org/bugzilla/show_bug.cgi?id=23787 Jack Die changed: What|Removed |Added CC||jackdie3438 at gmail dot com --- Comment #40 from Jack Die --- Thanks for help about handling of files.Read more for info about more files.https://bit.ly/3q9W94B -- You are receiving this mail because: You are on the CC list for the bug.