Re: [PATCH v2] doc: Update elf_begin.3
Hi Aaron, On Thu, 2025-06-05 at 17:12 -0400, Aaron Merey wrote: > Signed-off-by: Aaron Merey > --- > > v2: Address Mark's suggestions > https://patchwork.sourceware.org/project/elfutils/patch/20250603012245.411580-1-ame...@redhat.com/ It does indeed. Thanks. Just one nitpick, feel free to ignore, or resolve and just push the result. > +.TP > +\fIref\fP > +A reference to an existing \fBElf\fP descriptor. If not NULL, this will be > +used to duplicate a previous ELF descriptor. The reference count associated > +with \fIref\fP will be incremented and \fBelf_end(3)\fP will need to be > called > +an additional time to deallocate \fIref\fP. \fIref\fP must have been opened > +with read/write permissions consistent with \fIcmd\fP. If \fIref\fP is an AR > +file, then the ELF descriptor returned will be the first available object > member > +of the archive (see \fBelf_next(3)\fP for more information). Could we somehow break this more clearly between a ref being a "regular" ELF file and an AR file? Because elf_begin acts totally different between the two. Maybe split the above in two paragraphs? Also you mention elf_next here, but not ... > +.SH SEE ALSO > +.BR mmap (2), > +.BR elf_clone (3), > +.BR elf_end (3), > +.BR elf_rand (3), > +.BR libelf (3), > +.BR elf (5) ... here. Cheers, Mark
[Bug tools/33062] New: speed up readelf default (not -N) symbol resolution
https://sourceware.org/bugzilla/show_bug.cgi?id=33062 Bug ID: 33062 Summary: speed up readelf default (not -N) symbol resolution Product: elfutils Version: unspecified Status: NEW Severity: normal Priority: P2 Component: tools Assignee: unassigned at sourceware dot org Reporter: fche at redhat dot com CC: elfutils-devel at sourceware dot org Target Milestone: --- elfutils readelf defaults to symbol resolution of addresses found in dwarf references; binutils readelf doesn't. The performance implication of this default is stark. Operating on a large c++ binary (stap): - binutils readelf -w|| 7.24s user 0.17s system 99% cpu 7.432 total - stock eu-readelf -w|| 582.05s user 0.09s system 99% cpu 9:43.47 total - stock eu-readelf -w -N || 3.88s user 0.04s system 99% cpu 3.930 total i.e. a factor of ~200 slowdown. Surely it can be made faster. -- You are receiving this mail because: You are on the CC list for the bug.
[Bug libdw/26930] tsearch/tfind tree caches need locking
https://sourceware.org/bugzilla/show_bug.cgi?id=26930 Aaron Merey changed: What|Removed |Added CC||amerey at redhat dot com Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #3 from Aaron Merey --- Addressed in the following commit: commit d6443d1a4df6057f9012d105037f52daaca911f1 Author: Heather McIntyre Date: Fri Jul 12 18:23:56 2024 -0400 lib: Add eu_tsearch, eu_tfind, eu_tdelete and eu_tdestroy Add struct search_tree to hold tree root and lock. Add new eu_t* functions for ensuring synchronized tree access. Replace tsearch, tfind, etc with eu_t* equivalents. lib: * Makefile.am (libeu_a_SOURCES): Add eu-search.c. (noinst_HEADERS): Add eu-search.h and locks.h. * eu-config.h: Move rwlock macros to locks.h. * eu-search.c: New file containing tree search functions with locking. * eu-search.h: New file. * locks.h: New file containing rwlock macros previously in eu-config.h. libdw: * cfi.h (struct Dwarf_CFI_s): Change type of search tree members from void * to search_tree. * cie.c: Replace tree search functions with eu-search equivalents. * dwarf_begin_elf.c (valid_p): Initialize search trees. * dwarf_end.c (cu_free): Replace tree search functions with eu-search equivalents. * dwarf_getcfi.c (dwarf_getcfi): Initialize search trees. * dwarf_getlocations.c: Replace search tree functions with eu-search equivalents. (__libdw_intern_expression): Change type of cache parameter to search_tree *. * dwarf_getmacros.c: Replace tree search functions with eu-search equivalents. * dwarf_getsrclines.c: Ditto. * fde.c: Ditto. * frame-cache.c (__libdw_destroy_frame_cache): Initialize search trees. * libdwP.h (struct Dwarf): Change type of search tree members from void * to search_tree. (struct Dwarf_CU): Ditto. (__libdw_intern_expression): Change type of cache parameter to search_tree *. * libdw_find_split_unit.c: Replace tree search functions with eu-search equivalents. * libdw_findcu.c: Ditto. libdwfl: * cu.c: Ditto. * libdwflP.h (struct Dwfl_Module): Replace void *lazy_cu_root with search_tree lazy_cu_tree. libelf: * elf_begin.c (file_read_elf): Initialize rawchunck_tree. * elf_end.c (elf_end): Replace tree search function with eu-search equivalent. * elf_getdata_rawchunck.c: Ditto. * libelfP.h (struct Elf): Replace void * rawchuncks member with search_tree rawchunk_tree. -- You are receiving this mail because: You are on the CC list for the bug.
[Bug debuginfod/30589] debuginfod can't find sources even though they exist on disk
https://sourceware.org/bugzilla/show_bug.cgi?id=30589 Frank Ch. Eigler changed: What|Removed |Added Status|WAITING |RESOLVED Resolution|--- |WORKSFORME --- Comment #5 from Frank Ch. Eigler --- Can reexamine if given a newer reproducer w/ dwarf binary. -- You are receiving this mail because: You are on the CC list for the bug.
[Bug tools/33062] speed up readelf default (not -N) symbol resolution
https://sourceware.org/bugzilla/show_bug.cgi?id=33062 Mark Wielaard changed: What|Removed |Added CC||mark at klomp dot org --- Comment #1 from Mark Wielaard --- -N, --numeric-addressesDo not find symbol names for addresses in DWARF data So eu-readelf will try to print all addresses found through print_dwarf_addr. Without -N print_dwarf_addr will call dwfl_module_addrinfo (dwflmod, address, &off, &sym, NULL, NULL, NULL). dwfl_module_addrinfo is a wrapper around __libdwfl_addrsym defined in libdwfl/dwfl_module_addrsym.c __libdwfl_addrsym does a search through the (virtual) symbol table associated with the given Dwfl_Module. This is basically a linear search through the table(s) (it will try the global symbols first, then the local symbols). Obviously it would be much faster if on first search it would sort the symbols by address/range, so it could do some kind of binary search. What makes this slightly tricky is that symbols/addresses should be sorted by address range (with zero sized symbols being preferred only if there is no symbols with an address range closer) and symbols also being sorted by binding_value and the symbol matching must be associated with the section associated with the address (I symbol can be defined in a section, but have a value outside that section). -- You are receiving this mail because: You are on the CC list for the bug.
Re: [PATCH v2] doc: Update elf_begin.3
On Fri, Jun 6, 2025 at 8:49 AM Mark Wielaard wrote: > > Hi Aaron, > > On Thu, 2025-06-05 at 17:12 -0400, Aaron Merey wrote: > > Signed-off-by: Aaron Merey > > --- > > > > v2: Address Mark's suggestions > > https://patchwork.sourceware.org/project/elfutils/patch/20250603012245.411580-1-ame...@redhat.com/ > > It does indeed. Thanks. > > Just one nitpick, feel free to ignore, or resolve and just push the > result. > > > +.TP > > +\fIref\fP > > +A reference to an existing \fBElf\fP descriptor. If not NULL, this will be > > +used to duplicate a previous ELF descriptor. The reference count > > associated > > +with \fIref\fP will be incremented and \fBelf_end(3)\fP will need to be > > called > > +an additional time to deallocate \fIref\fP. \fIref\fP must have been > > opened > > +with read/write permissions consistent with \fIcmd\fP. If \fIref\fP is an > > AR > > +file, then the ELF descriptor returned will be the first available object > > member > > +of the archive (see \fBelf_next(3)\fP for more information). > > Could we somehow break this more clearly between a ref being a > "regular" ELF file and an AR file? Because elf_begin acts totally > different between the two. Maybe split the above in two paragraphs? > Also you mention elf_next here, but not ... > > > +.SH SEE ALSO > > +.BR mmap (2), > > +.BR elf_clone (3), > > +.BR elf_end (3), > > +.BR elf_rand (3), > > +.BR libelf (3), > > +.BR elf (5) > > ... here. Thanks Mark, I added elf_next here and split the description of ref into a couple paragraphs to clarify AR vs non-AR behavior. Aaron
Re: [PATCH 3/3 v2] src/readelf.c: Add support for print_debug_* output buffering
Hi Aaron,
On Thu, 2025-05-22 at 18:28 -0400, Aaron Merey wrote:
> Safely handle stdout output during concurrent calls to print_debug_*
> functions.
>
> For any print_debug_* function and any function that could be called
> from print_debug_* which also prints to stdout: add a FILE * argument
> and replace all printf, puts, putchar with fprintf. All printing
> to stdout will now be written to this FILE instead.
>
> The FILE * is an interface to a per-thread dynamically-sized buffer.
> libthread.a manages the allocation, printing and deallocation of
> these buffers.
I would have expected this patch to come before the libthread
enhancements. Just passing stdout around. And then add the per-thread
FILE* object.
In general this looks OK. It is just adding and extra argument to lots
of functions and using it.
But I don't fully understand why NULL is passed around in some cases.
It seems to be for the non-debug-section handling. But in those cases
why not just pass stdout instead of having to special case NULL?
> Signed-off-by: Aaron Merey
> ---
> v2:
> Minor change to print_dwarf_addr declaration so patch applies cleanly
> to main branch.
>
> src/readelf.c | 2258 +
> 1 file changed, 1157 insertions(+), 1101 deletions(-)
>
> diff --git a/src/readelf.c b/src/readelf.c
> index 38381156..8f233a77 100644
> --- a/src/readelf.c
> +++ b/src/readelf.c
> @@ -361,7 +361,7 @@ static void dump_strings (Ebl *ebl);
> static void print_strings (Ebl *ebl);
> static void dump_archive_index (Elf *, const char *);
> static void print_dwarf_addr (Dwfl_Module *dwflmod, int address_size,
> - Dwarf_Addr address, Dwarf_Addr raw);
> + Dwarf_Addr address, Dwarf_Addr raw, FILE *out);
> static void print_flag_info(void);
>
> enum dyn_idx
> @@ -2609,7 +2609,7 @@ handle_relocs_relr (Ebl *ebl, Dwfl_Module *mod, Elf_Scn
> *scn, GElf_Shdr *shdr)
> if ((entry & 1) == 0)
> {
> printf (" ");
> - print_dwarf_addr (mod, 4, entry, entry);
> + print_dwarf_addr (mod, 4, entry, entry, NULL);
> printf (" *\n");
>
> base = entry + 4;
> @@ -2622,7 +2622,7 @@ handle_relocs_relr (Ebl *ebl, Dwfl_Module *mod, Elf_Scn
> *scn, GElf_Shdr *shdr)
> if ((entry & 1) != 0)
> {
> printf (" ");
> - print_dwarf_addr (mod, 4, addr, addr);
> + print_dwarf_addr (mod, 4, addr, addr, NULL);
> printf ("\n");
> }
> base += 4 * (4 * 8 - 1);
> @@ -2648,7 +2648,7 @@ handle_relocs_relr (Ebl *ebl, Dwfl_Module *mod, Elf_Scn
> *scn, GElf_Shdr *shdr)
> if ((entry & 1) == 0)
> {
> printf (" ");
> - print_dwarf_addr (mod, 8, entry, entry);
> + print_dwarf_addr (mod, 8, entry, entry, NULL);
> printf (" *\n");
>
> base = entry + 8;
> @@ -2661,7 +2661,7 @@ handle_relocs_relr (Ebl *ebl, Dwfl_Module *mod, Elf_Scn
> *scn, GElf_Shdr *shdr)
> if ((entry & 1) != 0)
> {
> printf (" ");
> - print_dwarf_addr (mod, 8, addr, addr);
> + print_dwarf_addr (mod, 8, addr, addr, NULL);
> printf ("\n");
> }
> base += 8 * (8 * 8 - 1);
Like in these cases, I would have expected to pass around stdout.
> @@ -4369,8 +4369,12 @@ get_debug_elf_data (Dwarf *dbg, Ebl *ebl, int idx,
> Elf_Scn *scn)
>
> static void
> print_dwarf_addr (Dwfl_Module *dwflmod,
> - int address_size, Dwarf_Addr address, Dwarf_Addr raw)
> + int address_size, Dwarf_Addr address, Dwarf_Addr raw,
> + FILE *out)
> {
> + if (out == NULL)
> +out = stdout;
> +
So you don't have to special case it here.
>
> @@ -4985,13 +4991,17 @@ get_indexed_addr (Dwarf_CU *cu, Dwarf_Word idx,
> Dwarf_Addr *addr)
> static void
> print_ops (Dwfl_Module *dwflmod, Dwarf *dbg, int indent, int indentrest,
> unsigned int vers, unsigned int addrsize, unsigned int offset_size,
> -struct Dwarf_CU *cu, Dwarf_Word len, const unsigned char *data)
> +struct Dwarf_CU *cu, Dwarf_Word len, const unsigned char *data,
> +FILE *out)
> {
> + if (out == NULL)
> +out = stdout;
> +
Or here. I cannot find any invocation of print_ops with out being NULL.
Everything else looks obviously correct.
Thanks,
Mark
