patch 1/2 debuginfod client

Mark Wielaard mark@klomp.org
Wed Nov 13 13:57:00 GMT 2019


Hi,

On Mon, 2019-10-28 at 15:06 -0400, Frank Ch. Eigler wrote:
> Subject: [PATCH 1/2] debuginfod 1/2: client side
> 
> Introduce the debuginfod/ subdirectory, containing the client for a
> new debuginfo-over-http service, in shared-library and command-line
> forms.  Two functions in libdwfl make calls into the client library to
> fetch elf/dwarf files by buildid, as a fallback.  Instead of normal
> dynamic linking (thus pulling in a variety of curl dependencies),
> the libdwfl hooks use dlopen/dlsym.  Server & tests coming in patch 2.

And finally the libdw, libdwfl and m4 dirs.
Some of which was already discussed and changed.

> diff --git a/libdw/ChangeLog b/libdw/ChangeLog
> index 394c0df293f0..f0af348aff6e 100644
> --- a/libdw/ChangeLog
> +++ b/libdw/ChangeLog
> @@ -1,3 +1,7 @@
> +2019-10-28  Aaron Merey  <amerey@redhat.com>
> +
> +	* Makefile.am (libdw_so_LDLIBS): Add -ldl for libdebuginfod.so dlopen.
> +
>  2019-08-26  Jonathon Anderson  <jma14@rice.edu>
>  
>  	* libdw_alloc.c (__libdw_allocate): Added thread-safe stack allocator.
> diff --git a/libdw/Makefile.am b/libdw/Makefile.am
> index ce793e903b88..33b5838dc4e1 100644
> --- a/libdw/Makefile.am
> +++ b/libdw/Makefile.am
> @@ -109,7 +109,7 @@ libdw_so_LIBS = ../libebl/libebl_pic.a ../backends/libebl_backends_pic.a \
>  		../libcpu/libcpu_pic.a libdw_pic.a ../libdwelf/libdwelf_pic.a \
>  		../libdwfl/libdwfl_pic.a
>  libdw_so_DEPS = ../lib/libeu.a ../libelf/libelf.so
> -libdw_so_LDLIBS = $(libdw_so_DEPS) -lz $(argp_LDADD) $(zip_LIBS) -pthread
> +libdw_so_LDLIBS = $(libdw_so_DEPS) -ldl -lz $(argp_LDADD) $(zip_LIBS) -pthread
>  libdw_so_SOURCES =
>  libdw.so$(EXEEXT): $(srcdir)/libdw.map $(libdw_so_LIBS) $(libdw_so_DEPS)
>  	$(AM_V_CCLD)$(LINK) $(dso_LDFLAGS) -o $@ \

Yes, ok.

> diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
> index 04a39637e9a4..be29cc00bc7e 100644
> --- a/libdwfl/ChangeLog
> +++ b/libdwfl/ChangeLog
> @@ -1,3 +1,9 @@
> +2019-10-28  Aaron Merey  <amerey@redhat.com>
> +
> +	* dwfl_build_id_find_elf.c (dwfl_build_id_find_elf): Call debuginfod
> +	client functions via dlopen to look for elf/dwarf files as fallback.
> +	* find-debuginfo.c (dwfl_standard_find_debuginfo): Ditto.
> +
>  2019-08-12  Mark Wielaard  <mark@klomp.org>
>  
>  	* gzip.c (open_stream): Return DWFL_E_ERRNO on bad file operation.
> diff --git a/libdwfl/Makefile.am b/libdwfl/Makefile.am
> index 89ca92ed8110..29046e9e5e85 100644
> --- a/libdwfl/Makefile.am
> +++ b/libdwfl/Makefile.am
> @@ -31,7 +31,7 @@
>  ##
>  include $(top_srcdir)/config/eu.am
>  AM_CPPFLAGS += -I$(srcdir) -I$(srcdir)/../libelf -I$(srcdir)/../libebl \
> -	   -I$(srcdir)/../libdw -I$(srcdir)/../libdwelf
> +	   -I$(srcdir)/../libdw -I$(srcdir)/../libdwelf -I$(srcdir)/../debuginfod
>  VERSION = 1
>  
>  noinst_LIBRARIES = libdwfl.a
> @@ -39,6 +39,7 @@ noinst_LIBRARIES += libdwfl_pic.a
>  
>  pkginclude_HEADERS = libdwfl.h
>  
> +
>  libdwfl_a_SOURCES = dwfl_begin.c dwfl_end.c dwfl_error.c dwfl_version.c \
>  		    dwfl_module.c dwfl_report_elf.c relocate.c \
>  		    dwfl_module_build_id.c dwfl_module_report_build_id.c \

AM_CPPFLAGS change is ok. There is a stray whitespace change.

> diff --git a/libdwfl/dwfl_build_id_find_elf.c b/libdwfl/dwfl_build_id_find_elf.c
> index cc6c3f62d276..1f3834180c4a 100644
> --- a/libdwfl/dwfl_build_id_find_elf.c
> +++ b/libdwfl/dwfl_build_id_find_elf.c

This part looks good on the debuginfod-submit branch
(although I am slightly sad about the > 80 char lines)

diff --git a/libdwfl/find-debuginfo.c b/libdwfl/find-debuginfo.c
> index 9267788d2d19..d36ec3cc39b8 100644
> --- a/libdwfl/find-debuginfo.c
> +++ b/libdwfl/find-debuginfo.c
> @@ -1,5 +1,5 @@
>  /* Standard find_debuginfo callback for libdwfl.
> -   Copyright (C) 2005-2010, 2014, 2015 Red Hat, Inc.
> +   Copyright (C) 2005-2010, 2014, 2015, 2019 Red Hat, Inc.
>     This file is part of elfutils.
>  
>     This file is free software; you can redistribute it and/or modify
> @@ -31,9 +31,13 @@
>  #endif
>  
>  #include "libdwflP.h"
> +#ifdef ENABLE_DEBUGINFOD
> +#include "debuginfod.h"
> +#endif
>  #include <stdio.h>
>  #include <fcntl.h>
>  #include <unistd.h>
> +#include <dlfcn.h>
>  #include <sys/stat.h>
>  #include "system.h"
>  
> @@ -359,7 +363,8 @@ dwfl_standard_find_debuginfo (Dwfl_Module *mod,
>       other than just by finding nothing, that's all we do.  */
>    const unsigned char *bits;
>    GElf_Addr vaddr;
> -  if (INTUSE(dwfl_module_build_id) (mod, &bits, &vaddr) > 0)
> +  int bits_len;
> +  if ((bits_len = INTUSE(dwfl_module_build_id) (mod, &bits, &vaddr)) > 0)
>      {
>        /* Dropping most arguments means we cannot rely on them in
>  	 dwfl_build_id_find_debuginfo.  But leave it that way since
> @@ -397,6 +402,28 @@ dwfl_standard_find_debuginfo (Dwfl_Module *mod,
>        free (canon);
>      }
>  
> +#if ENABLE_DEBUGINFOD
> +  {
> +    static void *debuginfod_so;
> +    static __typeof__ (debuginfod_find_debuginfo) *fp_debuginfod_find_debuginfo;
> +
> +    if (debuginfod_so == NULL)
> +      debuginfod_so = dlopen("libdebuginfod-" VERSION ".so", RTLD_LAZY);
> +    if (debuginfod_so == NULL)
> +      debuginfod_so = dlopen("libdebuginfod.so", RTLD_LAZY);
> +    if (debuginfod_so != NULL && fp_debuginfod_find_debuginfo == NULL)
> +      fp_debuginfod_find_debuginfo = dlsym (debuginfod_so, "debuginfod_find_debuginfo");
> +
> +    if (fp_debuginfod_find_debuginfo != NULL)
> +      {
> +        /* If all else fails and a build-id is available, query the
> +           debuginfo-server if enabled.  */
> +        if (fd < 0 && bits_len > 0)
> +          fd = (*fp_debuginfod_find_debuginfo) (bits, bits_len, NULL);
> +      }
> +  }
> +#endif /* ENABLE_DEBUGINFOD */
> +
>    return fd;
>  }

So this part was also changed as in the above case. Now always build
and caching failure to find the library. Good.

But given that they are almost similar, I would suggest to move both
into their own file sharing most of the code to do the dlopen dance.

>  INTDEF (dwfl_standard_find_debuginfo)
> diff --git a/m4/ChangeLog b/m4/ChangeLog
> index 9ee06d750a1e..8ab0ff39610a 100644
> --- a/m4/ChangeLog
> +++ b/m4/ChangeLog
> @@ -1,3 +1,7 @@
> +2019-10-28  Aaron Merey  <amerey@redhat.com>
> +
> +	* ax_check_compile_flag.m4, ax_cxx_compile_stdcxx.m4: New files.
> +
>  2015-05-01  Mark Wielaard  <mjw@redhat.com>
>  
>  	* zip.m4: Explicitly set with_ to no, if not yes.
> diff --git a/m4/Makefile.am b/m4/Makefile.am
> index 3b0e11458748..ae7a565777e8 100644
> --- a/m4/Makefile.am
> +++ b/m4/Makefile.am
> @@ -18,4 +18,4 @@
>  ##
>  
>  ##m4-files-begin
> -EXTRA_DIST = codeset.m4 gettext.m4 iconv.m4 lcmessage.m4 progtest.m4 zip.m4
> +EXTRA_DIST = codeset.m4 gettext.m4 iconv.m4 lcmessage.m4 progtest.m4 zip.m4 ax_check_compile_flag.m4 ax_cxx_compile_stdcxx.m4
> diff --git a/m4/ax_check_compile_flag.m4 b/m4/ax_check_compile_flag.m4
> new file mode 100644
> index 000000000000..ca3639715e72
> --- /dev/null
> +++ b/m4/ax_check_compile_flag.m4
> @@ -0,0 +1,74 @@
> +# ===========================================================================
> +#   http://www.gnu.org/software/autoconf-archive/ax_check_compile_flag.html
> +# ===========================================================================
> [...]
> diff --git a/m4/ax_cxx_compile_stdcxx.m4 b/m4/ax_cxx_compile_stdcxx.m4
> new file mode 100644
> index 000000000000..8adc76569aa7
> --- /dev/null
> +++ b/m4/ax_cxx_compile_stdcxx.m4
> @@ -0,0 +1,556 @@
> +# ===========================================================================
> +#   http://www.gnu.org/software/autoconf-archive/ax_cxx_compile_stdcxx.html
> +# ===========================================================================
> [...]

All the m4 stuff looks OK.

Thanks,

Mark



More information about the Elfutils-devel mailing list