[Bug libelf/31034] New: iproute2 fails to build with elfutils 0.190 with musl libc

2023-11-06 Thread ncopa at alpinelinux dot org
https://sourceware.org/bugzilla/show_bug.cgi?id=31034

Bug ID: 31034
   Summary: iproute2 fails to build with elfutils 0.190 with musl
libc
   Product: elfutils
   Version: unspecified
Status: UNCONFIRMED
  Severity: normal
  Priority: P2
 Component: libelf
  Assignee: unassigned at sourceware dot org
  Reporter: ncopa at alpinelinux dot org
CC: elfutils-devel at sourceware dot org
  Target Milestone: ---

After elfutils was updated to 0.190, iproute2 fails to build with this error:

In file included from bpf_legacy.c:24:
/usr/include/gelf.h:86:9: error: unknown type name 'Elf64_Relr'
   86 | typedef Elf64_Relr GElf_Relr;
  | ^~


This appears to introduced with
https://sourceware.org/git/?p=elfutils.git;a=commit;h=39f2c500542f69c2f1a13fd0ae4eaa5778d2ed8d
which assumes that Elf64_Relr is typedef'ed if SHT_RELR is defined. This is not
(yet) the case in musl libc.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libelf/31034] iproute2 fails to build with elfutils 0.190 with musl libc

2023-11-06 Thread ncopa at alpinelinux dot org
https://sourceware.org/bugzilla/show_bug.cgi?id=31034

--- Comment #1 from ncopa at alpinelinux dot org ---
Patch sent to musl: https://www.openwall.com/lists/musl/2023/11/06/3

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libdw/30948] src/stack doesn't show source inforamtion if the target is compiled with clang

2023-11-06 Thread yamato at redhat dot com
https://sourceware.org/bugzilla/show_bug.cgi?id=30948

--- Comment #2 from Masatake YAMATO  ---
Thank you. With -gdwarf-aranges, the stack command works expectedly.

$ clang -O0 -g -gdwarf-aranges target.c -o target-clang
$ ./target-clang & 
[1] 4104789
$ ./src/stack -s -p  4104789
PID 4104789 - process
TID 4104789:
#0  0x00401149 f1
/home/yamato/var/elfutils/target.c:4:3
#1  0x00401139 f0
/home/yamato/var/elfutils/target.c:11:10
#2  0x00401124 main
/home/yamato/var/elfutils/target.c:16:10
#3  0x7fb7e9213b8a __libc_start_call_main
../sysdeps/nptl/libc_start_call_main.h:58:16
#4  0x7fb7e9213c4b __libc_start_main@@GLIBC_2.34
../csu/libc-start.c:360:3
#5  0x00401045 _start

-- 
You are receiving this mail because:
You are on the CC list for the bug.

Re: [PATCH 08/14] libdw: Parse DWARF package file index sections

2023-11-06 Thread Omar Sandoval
On Thu, Nov 02, 2023 at 12:07:04AM +0100, Mark Wielaard wrote:
> Hi Omar,
> 
> On Wed, Sep 27, 2023 at 11:20:57AM -0700, Omar Sandoval wrote:
> > The .debug_cu_index and .debug_tu_index sections in DWARF package files
> > are basically hash tables mapping a unit's 8 byte signature to an offset
> > and size in each section used by that unit [1].  Add support for parsing
> > and doing lookups in the index sections.
> > 
> > We look up a unit in the index when we intern it and cache its hash
> > table row in Dwarf_CU.
> 
> This looks good. Thanks for the various testcases.
> 
> Do I understand correctly that binutils dwp only does the DWARF4 GNU
> extension and llvm-dwp only does the DWARF5 standardized format? Maybe
> we should create a eu-dwp that does both.

llvm-dwp can do both the DWARF4 GNU format and the DWARF5 format.
binutils dwp can only do DWARF4.

> It looks like you are very careful checking boundaries, but it would
> be bad to do some fuzzing on this.
> 
> > Then, a new function, dwarf_cu_dwp_section_info,
> > can be used to look up the section offsets and sizes for a unit.  This
> > will mostly be used internally in libdw, but it will also be needed in
> > static inline functions shared with eu-readelf.  Additionally, making it
> > public it makes dwp support much easier for external tools that do their
> > own low-level parsing of DWARF information, like drgn [2].
> 
> Although I am not against this new interface, I am not super
> enthousiastic about it.
> 
> There is one odd part, DW_SECT_TYPES should be defined in dwarf.h with
> the other DW_SECT constants, otherwise it cannot be used (unless you
> define it yourself to 2).

Yeah, I wasn't sure about adding it or not since it's not technically
defined in the DWARF5 standard. But if we can count on future DWARF
standards not reusing the value 2 (which we probably can), I agree that
it seems better to add it to dwarf.h.

> For eu-readelf we don't really need it being public, it already cheats
> a little and uses some (non-public) libdwP.h functions. That is
> actually not great either. So if there is a public function that is
> available that is actually preferred. But if it is just for
> eu-readelf, I don't think it is really needed. And it seems you
> haven't actually added the support to eu-readelf to print these
> offsets.

Right, eu-readelf only uses it indirectly via the static inline
functions modified in patch 13. It looks like eu-readelf dynamically
links to libdw (on Fedora, at least), so either some part of the dwp
implementation needs to be exported, or a big chunk of it needs to be
inlined in libdwP.h, right?

Either way, I'd still love to have this interface for drgn.

> It is fine to expose these offsets and sizes, but how exactly are you
> using them in drgn? It seems we don't have any other interfaces in
> libdw that you can then use these with.

Here's the branch I linked to in my cover letter:
https://github.com/osandov/drgn/tree/dwp. I need this interface for the
couple of places where drgn parses DWARF itself.

One of those places is in the global name indexing step drgn does at
startup. We do this by enumerating all CUs using libdw, then using our
own purpose-built DWARF parser to do a fast, parallelized scan.

Our bespoke parser needs to know the base of the abbreviation table and
the string offsets table, which is easy without dwp. This interface also
makes it easy with dwp. Without this interface, I'd need to reimplement
the CU index parser in drgn :(

> Can we split off this public interface from the rest of this patch?
> But then we also need to split off the tests. So maybe keep them together?

Yeah, I included the interface in this patch exactly so that I could
test the implementation in the same patch. I'm happy to split that up
however you'd prefer.

[snip]

> > +  Dwarf_Package_Index *index = malloc (sizeof (*index));
> > +  if (index == NULL)
> > +{
> > +  __libdw_seterrno (DWARF_E_NOMEM);
> > +  return NULL;
> > +}
> > +
> > +  index->dbg = dbg;
> > +  /* Set absent sections to UINT32_MAX.  */
> > +  memset (index->sections, 0xff, sizeof (index->sections));
> 
> OK, although I would have preferred a simple for loop and let the
> compiler optimize it.

Ok, I can fix that.

[snip]

> > +static int
> > +__libdw_dwp_section_info (Dwarf_Package_Index *index, uint32_t unit_row,
> > + unsigned int section, Dwarf_Off *offsetp,
> > + Dwarf_Off *sizep)
> > +{
> > +  if (index == NULL)
> > +return -1;
> > +  if (unit_row == 0)
> > +{
> > +  __libdw_seterrno (DWARF_E_INVALID_DWARF);
> > +  return -1;
> > +}
> 
> OK, but why isn't the caller checking this?

Partially becase it simplified callers like __libdw_dwp_findcu_id to not
require a separate check, and partially just to be defensive.

[snip]

Thanks for taking a look!


Re: [PATCH 09/14] libdw, libdwfl: Save original path of ELF file

2023-11-06 Thread Omar Sandoval
On Thu, Nov 02, 2023 at 06:04:27PM +0100, Mark Wielaard wrote:
> Hi Omar,
> 
> On Wed, 2023-09-27 at 11:20 -0700, Omar Sandoval wrote:
> > libdw and libdwfl currently save the path of the directory containing
> > the ELF file to use when searching for alt and dwo files.  To search for
> > dwp files, we need the file name too.  Add an elfpath field to Dwarf,
> > and set the debugdir field from it.  Also update libdwfl to set elfpath
> > and debugdir.
> 
> This looks good. We will need some locking around this code when we
> integrate the thread-safety work. But that should be pretty clear.
> 
> > 
> > Signed-off-by: Omar Sandoval 
> > ---
> >  libdw/ChangeLog| 11 ++-
> >  libdw/dwarf_begin_elf.c| 34 --
> >  libdw/dwarf_end.c  |  3 ++-
> >  libdw/libdwP.h | 12 ++--
> >  libdwfl/ChangeLog  |  9 +
> >  libdwfl/dwfl_module.c  |  2 +-
> >  libdwfl/dwfl_module_getdwarf.c | 11 +++
> >  libdwfl/libdwflP.h |  2 +-
> >  libdwfl/offline.c  |  4 ++--
> >  9 files changed, 62 insertions(+), 26 deletions(-)
> > 
> > diff --git a/libdw/ChangeLog b/libdw/ChangeLog
> > index 1d229094..f491587f 100644
> > --- a/libdw/ChangeLog
> > +++ b/libdw/ChangeLog
> > @@ -20,7 +20,7 @@
> > instead of dbg parameter, which is now unused.
> > * libdwP.h (Dwarf_Macro_Op_Table): Replace is_64bit with address_size
> > and offset_size.  Add dbg.
> > -   (Dwarf): Add cu_index and tu_index.
> > +   (Dwarf): Add cu_index and tu_index.  Add elfpath.
> > (Dwarf_CU): Add dwp_row.
> > (Dwarf_Package_Index): New type.
> > (DW_SECT_TYPES): New macro.
> > @@ -28,6 +28,9 @@
> > (dwarf_cu_dwp_section_info): New INTDECL.
> > Add IDX_debug_cu_index and IDX_debug_tu_index.  Add
> > DWARF_E_UNKNOWN_SECTION.
> > +   (__libdw_debugdir): Replace declaration with...
> > +   (__libdw_elfpath): New declaration.
> > +   (__libdw_set_debugdir): New declaration.
> > * dwarf_begin_elf.c (dwarf_scnnames): Add IDX_debug_cu_index and
> > IDX_debug_tu_index.
> > (scn_to_string_section_idx): Ditto.
> > @@ -35,8 +38,14 @@
> > .zdebug_cu_index, and .zdebug_tu_index.
> > (check_section): Change .dwo suffix matching to account for
> > .debug_cu_index and .debug_tu_index.
> > +   (__libdw_debugdir): Replace with..
> > +   (__libdw_elfpath): New function.
> > +   (__libdw_set_debugdir): New function.
> > +   (valid_p): Call __libdw_elfpath and __libdw_set_debugdir instead of
> > +   __libdw_debugdir.
> > * Makefile.am (libdw_a_SOURCES): Add dwarf_cu_dwp_section_info.c.
> > * dwarf_end.c (dwarf_end): Free dwarf->cu_index and dwarf->tu_index.
> > +   Free dwarf->elfpath.
> > * dwarf_error.c (errmsgs): Add DWARF_E_UNKNOWN_SECTION.
> > * libdw.h (dwarf_cu_dwp_section_info): New declaration.
> > * libdw.map (ELFUTILS_0.190): Add dwarf_cu_dwp_section_info.
> 
> I had to recreate the ChangeLog entry because we skipped your patch 08.
> In the future lets just move the ChangeLog Entry into the commit
> message (we just updated CONTRIBUTING to recommend this). That makes
> rebasing slightly

Thanks for updating that, that's so much easier to work with :)


Re: [PATCH 10/14] libdw: Try .dwp file in __libdw_find_split_unit()

2023-11-06 Thread Omar Sandoval
On Thu, Nov 02, 2023 at 08:56:14PM +0100, Mark Wielaard wrote:
> Hi Omar,
> 
> On Wed, Sep 27, 2023 at 11:20:59AM -0700, Omar Sandoval wrote:
> > Try opening the file in the location suggested by the standard (the
> > skeleton file name + ".dwp") and looking up the unit in the package
> > index.  The rest is similar to .dwo files, with slightly different
> > cleanup since a single Dwarf handle is shared.
> 
> This seems a good default given it is what the standard says.
> But do you know of any distro doing this?

I don't. As far as I know, Meta and Google are the only ones using dwp
widely, and we're putting it in the location recommended by the
standard. (I think some Rust tooling is starting to pick dwp up, too,
but I haven't looked into it much.)

> The case I am wondering about is for separate .debug files (which
> surprisingly isn't standardized). In that case this would be
> foobar.debug.dwz (and not foobar.dwz).
> 
> It might also be an idea to just create one file with both the
> skeletons and the .dwo and dwz index sections.
>  
> > Signed-off-by: Omar Sandoval 
> > ---
> >  libdw/ChangeLog   | 11 -
> >  libdw/dwarf_begin_elf.c   |  1 +
> >  libdw/dwarf_cu_dwp_section_info.c | 19 
> >  libdw/dwarf_end.c | 10 -
> >  libdw/libdwP.h| 16 ++-
> >  libdw/libdw_find_split_unit.c | 75 ---
> >  6 files changed, 122 insertions(+), 10 deletions(-)
> > 
> > diff --git a/libdw/ChangeLog b/libdw/ChangeLog
> > index f491587f..f37d9411 100644
> > --- a/libdw/ChangeLog
> > +++ b/libdw/ChangeLog
> > @@ -1,6 +1,8 @@
> >  2023-09-27  Omar Sandoval  
> >  
> > * libdw_find_split_unit.c (try_split_file): Make static.
> > +   (try_dwp_file): New function.
> > +   (__libdw_find_split_unit): Call try_dwp_file.
> > * dwarf_entrypc.c (dwarf_entrypc): Call dwarf_lowpc.
> > * dwarf_ranges.c (dwarf_ranges): Use skeleton ranges section for
> > skeleton units.
> > @@ -20,7 +22,8 @@
> > instead of dbg parameter, which is now unused.
> > * libdwP.h (Dwarf_Macro_Op_Table): Replace is_64bit with address_size
> > and offset_size.  Add dbg.
> > -   (Dwarf): Add cu_index and tu_index.  Add elfpath.
> > +   (Dwarf): Add cu_index and tu_index.  Add elfpath.  Add dwp_dwarf and
> > +   dwp_fd.
> > (Dwarf_CU): Add dwp_row.
> > (Dwarf_Package_Index): New type.
> > (DW_SECT_TYPES): New macro.
> > @@ -31,6 +34,8 @@
> > (__libdw_debugdir): Replace declaration with...
> > (__libdw_elfpath): New declaration.
> > (__libdw_set_debugdir): New declaration.
> > +   (__libdw_dwp_findcu_id): New declaration.
> > +   (__libdw_link_skel_split): Handle .debug_addr for dwp.
> > * dwarf_begin_elf.c (dwarf_scnnames): Add IDX_debug_cu_index and
> > IDX_debug_tu_index.
> > (scn_to_string_section_idx): Ditto.
> > @@ -43,9 +48,11 @@
> > (__libdw_set_debugdir): New function.
> > (valid_p): Call __libdw_elfpath and __libdw_set_debugdir instead of
> > __libdw_debugdir.
> > +   (dwarf_begin_elf): Initialize result->dwp_fd.
> > * Makefile.am (libdw_a_SOURCES): Add dwarf_cu_dwp_section_info.c.
> > * dwarf_end.c (dwarf_end): Free dwarf->cu_index and dwarf->tu_index.
> > -   Free dwarf->elfpath.
> > +   Free dwarf->elfpath.  Free dwarf->dwp_dwarf and close dwarf->dwp_fd.
> > +   (cu_free): Don't free split dbg if it is dwp_dwarf.
> > * dwarf_error.c (errmsgs): Add DWARF_E_UNKNOWN_SECTION.
> > * libdw.h (dwarf_cu_dwp_section_info): New declaration.
> > * libdw.map (ELFUTILS_0.190): Add dwarf_cu_dwp_section_info.
> > diff --git a/libdw/dwarf_begin_elf.c b/libdw/dwarf_begin_elf.c
> > index 323a91d0..ca2b7e2a 100644
> > --- a/libdw/dwarf_begin_elf.c
> > +++ b/libdw/dwarf_begin_elf.c
> > @@ -567,6 +567,7 @@ dwarf_begin_elf (Elf *elf, Dwarf_Cmd cmd, Elf_Scn 
> > *scngrp)
> >  
> >result->elf = elf;
> >result->alt_fd = -1;
> > +  result->dwp_fd = -1;
> >  
> >/* Initialize the memory handling.  Initial blocks are allocated on first
> >   actual allocation.  */
> > diff --git a/libdw/dwarf_cu_dwp_section_info.c 
> > b/libdw/dwarf_cu_dwp_section_info.c
> > index 6766fb9a..7bf08d9d 100644
> > --- a/libdw/dwarf_cu_dwp_section_info.c
> > +++ b/libdw/dwarf_cu_dwp_section_info.c
> > @@ -340,6 +340,25 @@ __libdw_dwp_find_unit (Dwarf *dbg, bool debug_types, 
> > Dwarf_Off off,
> >abbrev_offsetp, NULL);
> >  }
> >  
> > +Dwarf_CU *
> > +internal_function
> > +__libdw_dwp_findcu_id (Dwarf *dbg, uint64_t unit_id8)
> > +{
> > +  Dwarf_Package_Index *index = __libdw_package_index (dbg, false);
> > +  uint32_t unit_row;
> > +  Dwarf_Off offset;
> > +  Dwarf_CU *cu;
> > +  if (__libdw_dwp_unit_row (index, unit_id8, &unit_row) == 0
> > +  && __libdw_dwp_section_info (index, unit_row, DW_SECT_INFO, &offset,
> > +  NULL) == 0
> > +  && (cu = __libdw_findcu (dbg, offset, false)) != NULL
> > +  && cu->unit_