Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH v3 0/3] Add debuginfod core file support
@ 2021-08-12  4:24 Aaron Merey via Gdb-patches
  2021-08-12  4:24 ` [PATCH 1/3] gdb/solib: Refactor scan_dyntag Aaron Merey via Gdb-patches
                   ` (2 more replies)
  0 siblings, 3 replies; 22+ messages in thread
From: Aaron Merey via Gdb-patches @ 2021-08-12  4:24 UTC (permalink / raw)
  To: gdb-patches, simon.marchi, tom

Hello,

This patch series has been updated based on feedback from
https://sourceware.org/pipermail/gdb-patches/2021-July/180956.html

I want to address some points from Simon's review.

> Instead of duplicating scan_dyntag, can we please extract a common
> base?  It is some non-trivial code, so it would be really good to
> avoid having two copies.
>
> Everything until setting the dynptr looks like it could be
> commonized.  The common function could work like scan_dyntag, where you
> pass a desired dyntag.  But since you are actually looking for two
> values, that would require two calls, so two scans.  So instead I would
> suggest making a "foreach_dyntag" function that takes a callback (a
> gdb::function_view), where you invoke the callback for each dyntag.
> The rest could then easily be implemented on top of that, and dyntag
> parsing code would be at a single place.

The first patch in this series moves the multiple implementations of
scan_dyntag to solib.c so that duplication is avoided. I decided not 
to add a foreach_dyntag function because the reason for having more
than one call to scan_dyntag was just to get the value of the DT_STRTAB
tag.  I don't think it's really necessary to get this value because,
as far as I know, the value of DT_STRTAB always refers to the .dynstr
section, which is easily found with bfd_get_section_by_name. Unless
there are cases where DT_STRTAB doesn't refer to .dynstr, a
foreach_dyntag shouldn't be needed.

> What's the point of calling bfd_check_format without checking the
> result?  It looks like a function without side-effects.

For some reason this function appears to update the flags and build-id
fields of the bfd with the correct values.

> In fact, that soname to buildid mapping doesn't seem related to
> debuginfod at all, it's just a generic soname to buildid mapping.  That
> could exist and be useful even if debuginfod didn't exist, right?  I'm
> not sure what is the best place.  But that information is produced by
> the core target and consumed by the solib subsystem... so it should
> probably be close to one of them.

The second patch in this series implements a per-program_space soname to
build-id mapping that is independent of debuginfod.

> Not related to your patch but... would it be a good idea to show the
> build-ids in "info proc mappings"?  It might sound useful to have that
> information in order to determine / find the right binaries that your
> process was using.

I think it would be useful in some circumstances. The changes to
linux_read_core_file_mappings in the second patch of this series should
make it easy to include build-ids in 'info proc mappings'.

> > @@ -536,6 +538,23 @@ solib_map_sections (struct so_list *so)
> >    gdb::unique_xmalloc_ptr<char> filename (tilde_expand (so->so_name));
> >    gdb_bfd_ref_ptr abfd (ops->bfd_open (filename.get ()));
> > 
> > +  if (abfd == NULL)
> > +    {
> > +      gdb::unique_xmalloc_ptr<char> build_id_hex;
> > +
> > +      /* If a build-id was previously associated with this soname
> > +      then use it to query debuginfod for the library.  */
> > +      if (debuginfod_get_soname_buildid (so->so_name, &build_id_hex))
> > +     {
> > +       scoped_fd fd = debuginfod_exec_query
> > +                      ((const unsigned char*) build_id_hex.get (),
> > +                       0, so->so_name, &filename);
> > +
> > +       if (fd.get () >= 0)
> > +         abfd = ops->bfd_open (filename.get ());
> > +     }
> > +    }
>
> I have a question about the order of the operations here.  Let's say I
> generate a core on my ARM machine and bring it back to debug it on my
> x86-64 machine.  And let's say I have a /usr/lib/foo.so on both
> machines.  Will the first ops->bfd_open manage to open the one of my
> development machine (the wrong one), and abfd will be != NULL?
>
> I am not sure what happens in that case, but if we could somehow
> determine that we didn't open the right library (for example, seeing
> that the lib's build-id doesn't match the expected build-id), and
> instead try downloading the right one using debuginfod... would that
> make sense?

If the solib installed at the time of debugging has the same soname
mentioned in the core file but a different build-id, then gdb will
silently use this installed solib even though it's the wrong build.
At least this is what happens when the arch stays the same. I have
not tested this senario when using multiple architectures. The third
patch in this series prints a warning message when a build-id mismatch
happens. If debuginfod is enabled then gdb will also query debuginfod
for the correct build of the solib.

One thing I should point out is that if we have the wrong build of the
solib installed locally and the debuginfod query fails because of some
error, the user will see something like the following:

warning: Build-id of /lib64/libc.so.6 does not match core file.
Download failed: No route to host. Continuing without executable for /lib64/libc.so.6.

However gdb is continuing with libc.so.6, although it is the wrong
build. We could modify solib_map_sections so that if there is
a build-id mismatch gdb refuses to use the solib. Or we could
add a parameter to debuginfod_exec_query that replaces "Continuing
without executable..." with some other string indicating that
gdb will use the local solib despite the mismatch.

Thanks,
Aaron

Aaron Merey (3):
  gdb/solib: Refactor scan_dyntag
  gdb: Add soname to build-id mapping for corefiles
  PR gdb/27570: missing support for debuginfod in
    core_target::build_file_mappings

 gdb/arch-utils.c                              |  21 ++-
 gdb/arch-utils.h                              |  21 ++-
 gdb/build-id.h                                |   2 +
 gdb/corelow.c                                 |  46 ++++-
 gdb/debuginfod-support.c                      |  46 +++++
 gdb/debuginfod-support.h                      |  16 ++
 gdb/gcore.in                                  |   3 +
 gdb/gdbarch.c                                 |   2 +-
 gdb/gdbarch.h                                 |   4 +-
 gdb/gdbarch.sh                                |   2 +-
 gdb/linux-tdep.c                              |  52 ++++--
 gdb/progspace.c                               |  36 ++++
 gdb/progspace.h                               |  17 ++
 gdb/solib-dsbt.c                              | 104 +----------
 gdb/solib-svr4.c                              | 118 +-----------
 gdb/solib.c                                   | 174 ++++++++++++++++++
 gdb/solib.h                                   |  11 ++
 .../gdb.debuginfod/fetch_src_and_symbols.exp  |  25 ++-
 18 files changed, 451 insertions(+), 249 deletions(-)

-- 
2.31.1


^ permalink raw reply	[flat|nested] 22+ messages in thread
* [PATCH v4 0/3] Add debuginfod core file support
@ 2021-11-10  1:47 Aaron Merey via Gdb-patches
  2021-11-10  1:47 ` [PATCH 3/3] PR gdb/27570: missing support for debuginfod in core_target::build_file_mappings Aaron Merey via Gdb-patches
  0 siblings, 1 reply; 22+ messages in thread
From: Aaron Merey via Gdb-patches @ 2021-11-10  1:47 UTC (permalink / raw)
  To: gdb-patches; +Cc: tom, lsix

v3 of this series can be found here:
https://sourceware.org/pipermail/gdb-patches/2021-August/181416.html

Aaron Merey (3):
  gdb: Add aliases for read_core_file_mappings callbacks
  gdb: Add soname to build-id mapping for corefiles
  PR gdb/27570: missing support for debuginfod in
    core_target::build_file_mappings

 gdb/arch-utils.c                              | 15 ++--
 gdb/arch-utils.h                              | 15 ++--
 gdb/corelow.c                                 | 46 ++++++++++++-
 gdb/debuginfod-support.c                      | 44 ++++++++++++
 gdb/debuginfod-support.h                      | 17 +++++
 gdb/gcore.in                                  |  3 +
 gdb/gdbarch.c                                 |  2 +-
 gdb/gdbarch.h                                 | 16 ++++-
 gdb/gdbarch.sh                                | 14 +++-
 gdb/linux-tdep.c                              | 52 ++++++++++----
 gdb/progspace.c                               | 32 +++++++++
 gdb/progspace.h                               | 18 +++++
 gdb/solib.c                                   | 69 +++++++++++++++++++
 gdb/solib.h                                   |  5 ++
 .../gdb.debuginfod/fetch_src_and_symbols.exp  | 26 ++++++-
 15 files changed, 335 insertions(+), 39 deletions(-)

-- 
2.31.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2022-02-09  2:37 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-12  4:24 [PATCH v3 0/3] Add debuginfod core file support Aaron Merey via Gdb-patches
2021-08-12  4:24 ` [PATCH 1/3] gdb/solib: Refactor scan_dyntag Aaron Merey via Gdb-patches
2021-08-17 13:28   ` Simon Marchi via Gdb-patches
2021-08-19  2:30     ` Aaron Merey via Gdb-patches
2021-08-12  4:24 ` [PATCH 2/3] gdb: Add soname to build-id mapping for corefiles Aaron Merey via Gdb-patches
2021-08-15 14:51   ` Lancelot SIX via Gdb-patches
2021-08-17 13:58     ` Simon Marchi via Gdb-patches
2021-08-19  2:22       ` Aaron Merey via Gdb-patches
2021-09-29  1:12         ` Aaron Merey via Gdb-patches
2021-10-18 23:06           ` [PING**2][PATCH " Aaron Merey via Gdb-patches
2021-11-03 18:12             ` [PING**3][PATCH " Aaron Merey via Gdb-patches
2021-11-04  1:32         ` [PATCH " Simon Marchi via Gdb-patches
2021-08-12  4:24 ` [PATCH 3/3] PR gdb/27570: missing support for debuginfod in core_target::build_file_mappings Aaron Merey via Gdb-patches
2021-09-29  1:13   ` Aaron Merey via Gdb-patches
2021-10-18 23:05     ` [PING**2][PATCH " Aaron Merey via Gdb-patches
2021-11-03 18:11       ` [PING**3][PATCH " Aaron Merey via Gdb-patches
2021-11-04  1:37   ` [PATCH " Simon Marchi via Gdb-patches
2021-11-10  1:47 [PATCH v4 0/3] Add debuginfod core file support Aaron Merey via Gdb-patches
2021-11-10  1:47 ` [PATCH 3/3] PR gdb/27570: missing support for debuginfod in core_target::build_file_mappings Aaron Merey via Gdb-patches
2021-11-14  2:56   ` Simon Marchi via Gdb-patches
2021-11-17  3:28     ` Aaron Merey via Gdb-patches
2022-01-26  1:42       ` Aaron Merey via Gdb-patches
2022-02-09  2:31         ` Aaron Merey via Gdb-patches

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox