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 <ame...@redhat.com>
> +
> + * Makefile.am (libdw_so_LDLIBS): Add -ldl for libdebuginfod.so dlopen.
> +
> 2019-08-26 Jonathon Anderson <jm...@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 <ame...@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 <m...@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 <ame...@redhat.com>
> +
> + * ax_check_compile_flag.m4, ax_cxx_compile_stdcxx.m4: New files.
> +
> 2015-05-01 Mark Wielaard <m...@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
