From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gnu.wildebeest.org (gnu.wildebeest.org [45.83.234.184]) by sourceware.org (Postfix) with ESMTPS id 9389A3858D35 for ; Thu, 2 Nov 2023 17:04:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9389A3858D35 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=klomp.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=klomp.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 9389A3858D35 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=45.83.234.184 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1698944684; cv=none; b=FVXF0qEzrSVn7mPkNvT/D8V7CMrVELbNkTvR1j0nfxWfQ7L7OLtnkLxeF1Qq10Rv3+SI+PyDFakU/ZWdF1tSUyTGXrGB6uPBxM0OyTPPSHneLqvO6pB5Kt5XkcxUyQd9yYYDajJlwhclLyYWRNCbZIwN6eCpbY1EWe0rEqdCLD0= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1698944684; c=relaxed/simple; bh=nDftP+UzqmLm6xYqvjaiTT//d4mKFPditJObR/xv7+o=; h=Message-ID:Subject:From:To:Date:MIME-Version; b=cNcpzc7xBKwzDSRDj9b4lezeefvHRgQBGyjSOttUssfAdK20FQrVaEpZf5MzpVTdYkJ6SHPNXcOxDemOEoDzQqkFuxBhG5jg+BEmp9DOuu9Q9+yXpKbzurFxNWK9QBHmHvJP3ug4fktRyOl928itB4LjXK+paDdD96tUJndwKIQ= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from r6.localdomain (82-217-174-174.cable.dynamic.v4.ziggo.nl [82.217.174.174]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by gnu.wildebeest.org (Postfix) with ESMTPSA id D5E9E302FDC3; Thu, 2 Nov 2023 18:04:27 +0100 (CET) Received: by r6.localdomain (Postfix, from userid 1000) id 796A5340422; Thu, 2 Nov 2023 18:04:27 +0100 (CET) Message-ID: <19d76b726f9b80c9320c5ac7c3af43ba60d34053.camel@klomp.org> Subject: Re: [PATCH 09/14] libdw, libdwfl: Save original path of ELF file From: Mark Wielaard To: Omar Sandoval , elfutils-devel@sourceware.org Date: Thu, 02 Nov 2023 18:04:27 +0100 In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.48.4 (3.48.4-1.fc38) MIME-Version: 1.0 X-Spam-Status: No, score=-3033.3 required=5.0 tests=BAYES_00,GIT_PATCH_0,JMQ_SPF_NEUTRAL,KAM_DMARC_STATUS,RCVD_IN_BARRACUDACENTRAL,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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. >=20 > 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(-) >=20 > 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 > diff --git a/libdw/dwarf_begin_elf.c b/libdw/dwarf_begin_elf.c > index 7936d343..323a91d0 100644 > --- a/libdw/dwarf_begin_elf.c > +++ b/libdw/dwarf_begin_elf.c > @@ -272,24 +272,27 @@ check_section (Dwarf *result, size_t shstrndx, Elf_= Scn *scn, bool inscngrp) > return result; > } > =20 > - > -/* Helper function to set debugdir field. We want to cache the dir > - where we found this Dwarf ELF file to locate alt and dwo files. */ > char * > -__libdw_debugdir (int fd) > +__libdw_elfpath (int fd) > { > /* strlen ("/proc/self/fd/") =3D 14 + strlen () =3D 10 + 1 =3D= 25. */ > char devfdpath[25]; > sprintf (devfdpath, "/proc/self/fd/%u", fd); > - char *fdpath =3D realpath (devfdpath, NULL); > - char *fddir; > - if (fdpath !=3D NULL && fdpath[0] =3D=3D '/' > - && (fddir =3D strrchr (fdpath, '/')) !=3D NULL) > - { > - *++fddir =3D '\0'; > - return fdpath; > - } > - return NULL; > + return realpath (devfdpath, NULL); > +} > + > + > +void > +__libdw_set_debugdir (Dwarf *dbg) > +{ > + if (dbg->elfpath =3D=3D NULL || dbg->elfpath[0] !=3D '/') > + return; > + size_t dirlen =3D strrchr (dbg->elfpath, '/') - dbg->elfpath + 1; > + dbg->debugdir =3D malloc (dirlen + 1); > + if (dbg->debugdir =3D=3D NULL) > + return; > + memcpy (dbg->debugdir, dbg->elfpath, dirlen); > + dbg->debugdir[dirlen] =3D '\0'; > } >=20 This looks slightly nicer than the original code it replaces. > =20 > @@ -421,7 +424,10 @@ valid_p (Dwarf *result) > } > =20 > if (result !=3D NULL) > - result->debugdir =3D __libdw_debugdir (result->elf->fildes); > + { > + result->elfpath =3D __libdw_elfpath (result->elf->fildes); > + __libdw_set_debugdir(result); > + } > =20 > return result; > } OK. > diff --git a/libdw/dwarf_end.c b/libdw/dwarf_end.c > index 18a419ce..b7f817d9 100644 > --- a/libdw/dwarf_end.c > +++ b/libdw/dwarf_end.c > @@ -147,7 +147,8 @@ dwarf_end (Dwarf *dwarf) > close (dwarf->alt_fd); > } > =20 > - /* The cached dir we found the Dwarf ELF file in. */ > + /* The cached path and dir we found the Dwarf ELF file in. */ > + free (dwarf->elfpath); > free (dwarf->debugdir); > =20 > /* Free the context descriptor. */ > diff --git a/libdw/libdwP.h b/libdw/libdwP.h > index 13ca58ce..214d1711 100644 > --- a/libdw/libdwP.h > +++ b/libdw/libdwP.h > @@ -169,6 +169,10 @@ struct Dwarf > /* The underlying ELF file. */ > Elf *elf; > =20 > + /* The (absolute) path to the ELF file, if known. To help locating > + dwp files. */ > + char *elfpath; > + > /* The (absolute) path to the ELF dir, if known. To help locating > alt and dwo files. */ > char *debugdir; > @@ -1385,9 +1389,13 @@ __libdw_link_skel_split (Dwarf_CU *skel, Dwarf_CU = *split) > int __libdw_addrx (Dwarf_CU *cu, Dwarf_Word idx, Dwarf_Addr *addr); > =20 > =20 > -/* Helper function to set debugdir field in Dwarf, used from dwarf_begin= _elf > +/* Helper function to set elfpath field in Dwarf, used from dwarf_begin_= elf > and libdwfl process_file. */ > -char * __libdw_debugdir (int fd); > +char * __libdw_elfpath (int fd); > + > +/* Helper function to set debugdir field in Dwarf after elfpath field ha= s been > + set. */ > +void __libdw_set_debugdir (Dwarf *dbg); > =20 > =20 > /* Given the directory of a debug file, an absolute or relative dir OK > diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog > index 54d85921..abbf0dfe 100644 > --- a/libdwfl/ChangeLog > +++ b/libdwfl/ChangeLog > @@ -1,3 +1,12 @@ > +2023-09-27 Omar Sandoval > + * dwfl_module.c (__libdwfl_module_free): Free mod->elfpath instead of > + mod->elfdir. > + * dwfl_module_getdwarf.c (load_dw): Set mod->dw->elfpath and call > + __libdw_set_debugdir instead of setting mod->dw->debugdir. > + * libdwflP.h (Dwfl_Module): Replace elfdir with elfpath. > + * offline.c (process_elf): Call __libdw_elfpath and set mod->elfpath > + instead of mod->elfdir. > + > 2023-04-24 John Gallagher > =20 > * gzip.c: Fix memory leak in unzip() > diff --git a/libdwfl/dwfl_module.c b/libdwfl/dwfl_module.c > index 221d726d..c4d872d4 100644 > --- a/libdwfl/dwfl_module.c > +++ b/libdwfl/dwfl_module.c > @@ -119,7 +119,7 @@ __libdwfl_module_free (Dwfl_Module *mod) > free (mod->reloc_info); > =20 > free (mod->name); > - free (mod->elfdir); > + free (mod->elfpath); > free (mod); > } OK > diff --git a/libdwfl/dwfl_module_getdwarf.c b/libdwfl/dwfl_module_getdwar= f.c > index 9ba499bb..6f98c02b 100644 > --- a/libdwfl/dwfl_module_getdwarf.c > +++ b/libdwfl/dwfl_module_getdwarf.c > @@ -1362,11 +1362,14 @@ load_dw (Dwfl_Module *mod, struct dwfl_file *debu= gfile) > } > =20 > /* We might have already closed the fd when we asked dwarf_begin_elf t= o > - create an Dwarf. Help out a little in case we need to find an alt = or > - dwo file later. */ > - if (mod->dw->debugdir =3D=3D NULL && mod->elfdir !=3D NULL > + create an Dwarf. Help out a little in case we need to find an alt, > + dwo, or dwp file later. */ > + if (mod->dw->elfpath =3D=3D NULL && mod->elfpath !=3D NULL > && debugfile =3D=3D &mod->main) > - mod->dw->debugdir =3D strdup (mod->elfdir); > + { > + mod->dw->elfpath =3D strdup (mod->elfpath); > + __libdw_set_debugdir (mod->dw); > + } This is the same pattern as above. When adding the thread-safety locks we might want to wrap this in a properly locked helper function. > /* Until we have iterated through all CU's, we might do lazy lookups. = */ > mod->lazycu =3D 1; > diff --git a/libdwfl/libdwflP.h b/libdwfl/libdwflP.h > index cdc528d0..b3dfea1d 100644 > --- a/libdwfl/libdwflP.h > +++ b/libdwfl/libdwflP.h > @@ -186,7 +186,7 @@ struct Dwfl_Module > Elf_Data *symxndxdata; /* Data in the extended section index table. */ > Elf_Data *aux_symxndxdata; /* Data in the extended auxiliary table. */ > =20 > - char *elfdir; /* The dir where we found the main Elf. */ > + char *elfpath; /* The path where we found the main Elf. */ > =20 > Dwarf *dw; /* libdw handle for its debugging info. */ > Dwarf *alt; /* Dwarf used for dwarf_setalt, or NULL. */ > diff --git a/libdwfl/offline.c b/libdwfl/offline.c > index e090b42b..50abe8c3 100644 > --- a/libdwfl/offline.c > +++ b/libdwfl/offline.c > @@ -151,9 +151,9 @@ process_elf (Dwfl *dwfl, const char *name, const char= *file_name, int fd, > /* Don't keep the file descriptor around. */ > if (mod->main.fd !=3D -1 && elf_cntl (mod->main.elf, ELF_C_FDREAD)= =3D=3D 0) > { > - /* Grab the dir path in case we want to report this file as > + /* Grab the path in case we want to report this file as > Dwarf later. */ > - mod->elfdir =3D __libdw_debugdir (mod->main.fd); > + mod->elfpath =3D __libdw_elfpath (mod->main.fd); > close (mod->main.fd); > mod->main.fd =3D -1; > } OK. Pushed, Mark