Re: [PATCH] libelf: Rewrite elf_scnshndx, extended index table handling
Hi Mark, On Sun, Feb 23, 2025 at 6:31 PM Mark Wielaard wrote: > > elf_scnshndx is a elfutils extension to libelf that given a SHT_SYMTAB > section returns the index to the corresponding SHT_SYMTAB_SHNDX > section, if it exists. This is needed when there are more than 64K > sections and there are symbols that have to refer to a section with an > index larger than 64K, because the Elf Sym st_shndx field is only > 16 bits. LGTM. The new tests do a nice job of proving that libelf will properly handle binaries with >64K sections. Aaron
Re: [PATCH 4/9 v3] Add configure option --enable-helgrind
Hi Aaron, On Wed, Feb 19, 2025 at 11:36:39PM -0500, Aaron Merey wrote: > Like --enable-valgrind but uses helgrind instead of memcheck. > > If both --enable-valgrind and --enable-helgrind are given then > helgrind takes priority. > > --enable-helgrind requires --enable-valgrind-annotations. > > * configure.ac: Add --enable-helgrind option. > * tests/Makefile.am: If USE_HELGRIND is true, then include > --tool=helgrind in the valgrind command that tests are run > under. > > Signed-off-by: Aaron Merey > > --- > v3 changes: Add --track-fds to valgrind_cmd when USE_HELGRIND is true. Looks OK. Once this is in we should add a buildbot with this enabled. Thanks, Mark
Re: [PATCH 2/9 v3] libdw: Add locking to dwarf_getsrcfiles, dwarf_getsrclines, dwarf_macro_getsrcfiles
Hi Frank, On Thu, 2025-02-27 at 18:47 -0500, Frank Ch. Eigler wrote: > > We should make dwarf_getsrcfiles and dwarf_getsrclines independent so > > callers that onle need the file table don't need to "pay" for parsing > > the whole line table. (I thought we had a bug for that, but cannot > > find it now. If you also cannot find it, please let me know and I'll > > file a new one.) > > I believe you're referring to PR27405, which amerey himself implemented > in commit d4b0848be. Thanks! Indeed. My memory is clearly failing. Good this is already fixed. But now we don't have an easy excuse to revisit the locking :) > > [...] So you are serializing the file/line table parsing. > > (On the other hand, at this stage of concurrency management in > elfutils, it may make perfect sense to err on the side of > *correctness* now, and only complicating locking later when > serious contention hits to performance are indicated.) Sure. But I think the current locking proposal, using an indirect lock from a "parent" object, feels like it is harder to proof correct than having the lock in the object itself. Cheers, Mark
