diff mbox

[1/1] Add debuginfod support to GDB

Message ID CAPTJ0XHYb_WL_Tb479sbqN1Vz0VAGLbPA44hP855t=4h5c9nfw@mail.gmail.com
State New
Headers show

Commit Message

Doug Evans via gdb-patches Jan. 9, 2020, 11:27 p.m. UTC 
On Thu, Jan 9, 2020 at 4:41 PM Aaron Merey <amerey@redhat.com> wrote:
>
> On Thu, Jan 9, 2020 at 10:27 AM Tom Tromey <tom@tromey.com> wrote:
> > Aaron>         * config/debuginfod.m4: New file. Add macro AC_DEBUGINFOD.
> > Aaron>         Adds new configure option --with-debuginfod.
> > Aaron>         * configure: Regenerate.
> > Aaron>         * configure.ac: Call AC_DEBUGINFOD.
> >
> > Does the top-level configure really need AC_DEBUGINFOD?
> >
> > If so, then this part of the patch has to go to gcc-patches first.
> > But, I suspect it's not needed, in which case dropping it is more
> > convenient, because we can have debuginfod.m4 locally and not involve
> > gcc at all.
>
> The reason for the top-level AC_DEBUGINFOD is to prevent the top-level
> configure check from succeeding in cases where --with-debuginfod is
> given but the debuginfod library or header cannot be found.

Why is debuginfo special in that way? There are a lot of other
libraries in the same situation.


Other comments:

Comments

Aaron Merey Jan. 10, 2020, 4:31 p.m. UTC | #1 
On Thu, Jan 9, 2020 at 6:28 PM Christian Biesinger
<cbiesinger@google.com> wrote:
> On Thu, Jan 9, 2020 at 4:41 PM Aaron Merey <amerey@redhat.com> wrote:
> > The reason for the top-level AC_DEBUGINFOD is to prevent the top-level
> > configure check from succeeding in cases where --with-debuginfod is
> > given but the debuginfod library or header cannot be found.
>
> Why is debuginfo special in that way? There are a lot of other
> libraries in the same situation.

It was recommended to me on binutils@ that the top-level configure check
should fail if --with-debuginfod is given but not installed.

https://sourceware.org/ml/binutils/2019-11/msg00371.html

Despite that I can remove the top-level AC_DEBUGINFOD to match the
behavior of other configure options.

> Now that GDB is C++, I'd move the declarations to where the variables
> are used, e.g.:
>   debuginfod_client *client = debuginfod_begin ();
>
> +      if (client != NULL)
>
> nullptr
>
> +++ b/gdb/elfread.c
>
> same here

Ok.

> Don't you also need to update gdb/README (the `configure' options
> section) and doc/gdb.texinfo (@node Configure Options)?

gdb.texinfo was updated in this patch but not README, will fix that.

Aaron
diff mbox

Patch

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 04979f3d12..f9c06b8cc6 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -2708,6 +2711,45 @@  dwarf2_get_dwz_file (struct dwarf2_per_objfile
*dwarf2_per_objfile)
+      char *alt_filename;
+      debuginfod_client *client;
+
+      client = debuginfod_begin ();

Now that GDB is C++, I'd move the declarations to where the variables
are used, e.g.:
  debuginfod_client *client = debuginfod_begin ();

+      if (client != NULL)

nullptr

+++ b/gdb/elfread.c

same here

Don't you also need to update gdb/README (the `configure' options
section) and doc/gdb.texinfo (@node Configure Options)?

Christian