Re: [PATCH 1/6 v2] gdb/debuginfod: Add debuginfod_section_query

[Andrew Burgess <aburgess@redhat.com>]

Aaron Merey <amerey@redhat.com> writes:

> v1: https://sourceware.org/pipermail/gdb-patches/2023-February/197455.html
>
> v2 improves the description of AC_DEBUGINFOD_SECTION and associated
> macro names.  An if-statement in debuginfod_section_query is also
> replaced with a gdb_assert.
>
> Commit message:
>
> Add new function debuginfod_section_query.  This function queries
> debuginfod servers for an individual ELF/DWARF section associated with
> a given build-id.
>
> Also check for libdebuginfod version >= 0.188 at configure time.
> debuginfod_section_query simply returns -ENOSYS if this condition
> is not met.

This LGTM.

I don't think this should be merged until there's a use for the new GDB
functions (when later patches are also approved).

Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew


> ---
>  config/debuginfod.m4     |  33 +++++++++++
>  gdb/config.in            |   3 +
>  gdb/configure            | 116 +++++++++++++++++++++++++++++++++++++--
>  gdb/configure.ac         |   2 +-
>  gdb/debuginfod-support.c |  60 ++++++++++++++++++++
>  gdb/debuginfod-support.h |  24 ++++++++
>  6 files changed, 232 insertions(+), 6 deletions(-)
>
> diff --git a/config/debuginfod.m4 b/config/debuginfod.m4
> index 2c1bfbdb544..4c443b77d3a 100644
> --- a/config/debuginfod.m4
> +++ b/config/debuginfod.m4
> @@ -26,3 +26,36 @@ else
>    AC_MSG_WARN([debuginfod support disabled; some features may be unavailable.])
>  fi
>  ])
> +
> +AC_DEFUN([AC_DEBUGINFOD_SECTION],
> +[
> +# Handle optional debuginfod support as well as optional section
> +# downloading support.
> +#
> +# Define HAVE_LIBDEBUGINFOD if libdebuginfod is found with version >= 0.179.
> +#
> +# Define HAVE_LIBDEBUGINFOD_FIND_SECTION if libdebuginfod is found with
> +# version >= 0.188.
> +AC_ARG_WITH([debuginfod],
> +  AC_HELP_STRING([--with-debuginfod], [Enable debuginfo lookups with debuginfod (auto/yes/no)]),
> +  [], [with_debuginfod=auto])
> +AC_MSG_CHECKING([whether to use debuginfod])
> +AC_MSG_RESULT([$with_debuginfod])
> +
> +if test "x$with_debuginfod" != xno; then
> +  PKG_CHECK_MODULES([DEBUGINFOD], [libdebuginfod >= 0.188],
> +    [AC_DEFINE([HAVE_LIBDEBUGINFOD_FIND_SECTION], [1],
> +	       [Define to 1 if debuginfod section downloading is supported.])],
> +    [AC_MSG_WARN([libdebuginfod is missing or some features may be unavailable.])])
> +
> +  PKG_CHECK_MODULES([DEBUGINFOD], [libdebuginfod >= 0.179],
> +    [AC_DEFINE([HAVE_LIBDEBUGINFOD], [1], [Define to 1 if debuginfod is enabled.])],
> +    [if test "x$with_debuginfod" = xyes; then
> +      AC_MSG_ERROR(["--with-debuginfod was given, but libdebuginfod is missing or unusable."])
> +     else
> +      AC_MSG_WARN([libdebuginfod is missing or unusable; some features may be unavailable.])
> +     fi])
> +else
> +  AC_MSG_WARN([debuginfod support disabled; some features may be unavailable.])
> +fi
> +])
> diff --git a/gdb/config.in b/gdb/config.in
> index a7da88b92d7..95dd2fa22cb 100644
> --- a/gdb/config.in
> +++ b/gdb/config.in
> @@ -244,6 +244,9 @@
>  /* Define to 1 if debuginfod is enabled. */
>  #undef HAVE_LIBDEBUGINFOD
>  
> +/* Define to 1 if debuginfod section downloading is supported. */
> +#undef HAVE_LIBDEBUGINFOD_FIND_SECTION
> +
>  /* Define if you have the expat library. */
>  #undef HAVE_LIBEXPAT
>  
> diff --git a/gdb/configure b/gdb/configure
> index 5bb2a0795e5..f58a2b6b92a 100755
> --- a/gdb/configure
> +++ b/gdb/configure
> @@ -18349,7 +18349,13 @@ esac
>  
>  # Handle optional debuginfod support
>  
> -# Handle optional debuginfod support
> +# Handle optional debuginfod support as well as optional section
> +# downloading support.
> +#
> +# Define HAVE_LIBDEBUGINFOD if libdebuginfod is found with version >= 0.179.
> +#
> +# Define HAVE_LIBDEBUGINFOD_FIND_SECTION if libdebuginfod is found with
> +# version >= 0.188.
>  
>  # Check whether --with-debuginfod was given.
>  if test "${with_debuginfod+set}" = set; then :
> @@ -18365,6 +18371,106 @@ $as_echo "$with_debuginfod" >&6; }
>  
>  if test "x$with_debuginfod" != xno; then
>  
> +pkg_failed=no
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for libdebuginfod >= 0.188" >&5
> +$as_echo_n "checking for libdebuginfod >= 0.188... " >&6; }
> +
> +if test -n "$DEBUGINFOD_CFLAGS"; then
> +    pkg_cv_DEBUGINFOD_CFLAGS="$DEBUGINFOD_CFLAGS"
> + elif test -n "$PKG_CONFIG"; then
> +    if test -n "$PKG_CONFIG" && \
> +    { { $as_echo "$as_me:${as_lineno-$LINENO}: \$PKG_CONFIG --exists --print-errors \"libdebuginfod >= 0.188\""; } >&5
> +  ($PKG_CONFIG --exists --print-errors "libdebuginfod >= 0.188") 2>&5
> +  ac_status=$?
> +  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
> +  test $ac_status = 0; }; then
> +  pkg_cv_DEBUGINFOD_CFLAGS=`$PKG_CONFIG --cflags "libdebuginfod >= 0.188" 2>/dev/null`
> +		      test "x$?" != "x0" && pkg_failed=yes
> +else
> +  pkg_failed=yes
> +fi
> + else
> +    pkg_failed=untried
> +fi
> +if test -n "$DEBUGINFOD_LIBS"; then
> +    pkg_cv_DEBUGINFOD_LIBS="$DEBUGINFOD_LIBS"
> + elif test -n "$PKG_CONFIG"; then
> +    if test -n "$PKG_CONFIG" && \
> +    { { $as_echo "$as_me:${as_lineno-$LINENO}: \$PKG_CONFIG --exists --print-errors \"libdebuginfod >= 0.188\""; } >&5
> +  ($PKG_CONFIG --exists --print-errors "libdebuginfod >= 0.188") 2>&5
> +  ac_status=$?
> +  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
> +  test $ac_status = 0; }; then
> +  pkg_cv_DEBUGINFOD_LIBS=`$PKG_CONFIG --libs "libdebuginfod >= 0.188" 2>/dev/null`
> +		      test "x$?" != "x0" && pkg_failed=yes
> +else
> +  pkg_failed=yes
> +fi
> + else
> +    pkg_failed=untried
> +fi
> +
> +if test $pkg_failed = no; then
> +  pkg_save_LDFLAGS="$LDFLAGS"
> +  LDFLAGS="$LDFLAGS $pkg_cv_DEBUGINFOD_LIBS"
> +  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
> +/* end confdefs.h.  */
> +
> +int
> +main ()
> +{
> +
> +  ;
> +  return 0;
> +}
> +_ACEOF
> +if ac_fn_c_try_link "$LINENO"; then :
> +
> +else
> +  pkg_failed=yes
> +fi
> +rm -f core conftest.err conftest.$ac_objext \
> +    conftest$ac_exeext conftest.$ac_ext
> +  LDFLAGS=$pkg_save_LDFLAGS
> +fi
> +
> +
> +
> +if test $pkg_failed = yes; then
> +        { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
> +$as_echo "no" >&6; }
> +
> +if $PKG_CONFIG --atleast-pkgconfig-version 0.20; then
> +        _pkg_short_errors_supported=yes
> +else
> +        _pkg_short_errors_supported=no
> +fi
> +        if test $_pkg_short_errors_supported = yes; then
> +	        DEBUGINFOD_PKG_ERRORS=`$PKG_CONFIG --short-errors --print-errors --cflags --libs "libdebuginfod >= 0.188" 2>&1`
> +        else
> +	        DEBUGINFOD_PKG_ERRORS=`$PKG_CONFIG --print-errors --cflags --libs "libdebuginfod >= 0.188" 2>&1`
> +        fi
> +	# Put the nasty error message in config.log where it belongs
> +	echo "$DEBUGINFOD_PKG_ERRORS" >&5
> +
> +	{ $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: libdebuginfod is missing or some features may be unavailable." >&5
> +$as_echo "$as_me: WARNING: libdebuginfod is missing or some features may be unavailable." >&2;}
> +elif test $pkg_failed = untried; then
> +        { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
> +$as_echo "no" >&6; }
> +	{ $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: libdebuginfod is missing or some features may be unavailable." >&5
> +$as_echo "$as_me: WARNING: libdebuginfod is missing or some features may be unavailable." >&2;}
> +else
> +	DEBUGINFOD_CFLAGS=$pkg_cv_DEBUGINFOD_CFLAGS
> +	DEBUGINFOD_LIBS=$pkg_cv_DEBUGINFOD_LIBS
> +        { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5
> +$as_echo "yes" >&6; }
> +
> +$as_echo "#define HAVE_LIBDEBUGINFOD_FIND_SECTION 1" >>confdefs.h
> +
> +fi
> +
> +
>  pkg_failed=no
>  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for libdebuginfod >= 0.179" >&5
>  $as_echo_n "checking for libdebuginfod >= 0.179... " >&6; }
> @@ -18448,18 +18554,18 @@ fi
>  	echo "$DEBUGINFOD_PKG_ERRORS" >&5
>  
>  	if test "x$with_debuginfod" = xyes; then
> -       as_fn_error $? "\"--with-debuginfod was given, but libdebuginfod is missing or unusable.\"" "$LINENO" 5
> +      as_fn_error $? "\"--with-debuginfod was given, but libdebuginfod is missing or unusable.\"" "$LINENO" 5
>       else
> -       { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: libdebuginfod is missing or unusable; some features may be unavailable." >&5
> +      { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: libdebuginfod is missing or unusable; some features may be unavailable." >&5
>  $as_echo "$as_me: WARNING: libdebuginfod is missing or unusable; some features may be unavailable." >&2;}
>       fi
>  elif test $pkg_failed = untried; then
>          { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
>  $as_echo "no" >&6; }
>  	if test "x$with_debuginfod" = xyes; then
> -       as_fn_error $? "\"--with-debuginfod was given, but libdebuginfod is missing or unusable.\"" "$LINENO" 5
> +      as_fn_error $? "\"--with-debuginfod was given, but libdebuginfod is missing or unusable.\"" "$LINENO" 5
>       else
> -       { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: libdebuginfod is missing or unusable; some features may be unavailable." >&5
> +      { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: libdebuginfod is missing or unusable; some features may be unavailable." >&5
>  $as_echo "$as_me: WARNING: libdebuginfod is missing or unusable; some features may be unavailable." >&2;}
>       fi
>  else
> diff --git a/gdb/configure.ac b/gdb/configure.ac
> index fb43cd10d6c..499802bb4c9 100644
> --- a/gdb/configure.ac
> +++ b/gdb/configure.ac
> @@ -342,7 +342,7 @@ case $host_os in
>  esac
>  
>  # Handle optional debuginfod support
> -AC_DEBUGINFOD
> +AC_DEBUGINFOD_SECTION
>  
>  # Libunwind support for ia64.
>  AC_ARG_WITH(libunwind-ia64,
> diff --git a/gdb/debuginfod-support.c b/gdb/debuginfod-support.c
> index 5853f420a18..8be43a91dcc 100644
> --- a/gdb/debuginfod-support.c
> +++ b/gdb/debuginfod-support.c
> @@ -80,6 +80,15 @@ debuginfod_exec_query (const unsigned char *build_id,
>    return scoped_fd (-ENOSYS);
>  }
>  
> +scoped_fd
> +debuginfod_section_query (const unsigned char *build_id,
> +			  int build_id_len,
> +			  const char *filename,
> +			  const char *section_name,
> +			  gdb::unique_xmalloc_ptr<char> *destname)
> +{
> +  return scoped_fd (-ENOSYS);
> +}
>  #define NO_IMPL _("Support for debuginfod is not compiled into GDB.")
>  
>  #else
> @@ -401,6 +410,57 @@ debuginfod_exec_query (const unsigned char *build_id,
>  
>    return fd;
>  }
> +
> +/* See debuginfod-support.h  */
> +
> +scoped_fd
> +debuginfod_section_query (const unsigned char *build_id,
> +			  int build_id_len,
> +			  const char *filename,
> +			  const char *section_name,
> +			  gdb::unique_xmalloc_ptr<char> *destname)
> +{
> +#if !defined (HAVE_LIBDEBUGINFOD_FIND_SECTION)
> +  return scoped_fd (-ENOSYS);
> +#else
> +
> + if (!debuginfod_is_enabled ())
> +    return scoped_fd (-ENOSYS);
> +
> +  debuginfod_client *c = get_debuginfod_client ();
> +
> +  if (c == nullptr)
> +    return scoped_fd (-ENOMEM);
> +
> +  char *dname = nullptr;
> +  std::string desc = std::string ("section ") + section_name + " for";
> +  scoped_fd fd;
> +  gdb::optional<target_terminal::scoped_restore_terminal_state> term_state;
> +
> +  {
> +    user_data data (desc.c_str (), filename);
> +    debuginfod_set_user_data (c, &data);
> +    if (target_supports_terminal_ours ())
> +      {
> +	term_state.emplace ();
> +	target_terminal::ours ();
> +      }
> +
> +    fd = scoped_fd (debuginfod_find_section (c, build_id, build_id_len,
> +					     section_name, &dname));
> +    debuginfod_set_user_data (c, nullptr);
> +  }
> +
> +  print_outcome (fd.get (), desc.c_str (), filename);
> +  gdb_assert (destname != nullptr);
> +
> +  if (fd.get () >= 0)
> +    destname->reset (dname);
> +
> +  return fd;
> +#endif /* HAVE_LIBDEBUGINFOD_FIND_SECTION */
> +}
> +
>  #endif
>  
>  /* Set callback for "set debuginfod enabled".  */
> diff --git a/gdb/debuginfod-support.h b/gdb/debuginfod-support.h
> index 633600a79da..9701e3b4685 100644
> --- a/gdb/debuginfod-support.h
> +++ b/gdb/debuginfod-support.h
> @@ -81,4 +81,28 @@ extern scoped_fd debuginfod_exec_query (const unsigned char *build_id,
>  					const char *filename,
>  					gdb::unique_xmalloc_ptr<char>
>  					  *destname);
> +
> +/* Query debuginfod servers for the binary contents of a ELF/DWARF section
> +   from a file matching BUILD_ID.  BUILD_ID can be given as a binary blob
> +   or a null-terminated string.  If given as a binary blob, BUILD_ID_LEN
> +   should be the number of bytes.  If given as a null-terminated string,
> +   BUILD_ID_LEN should be 0.
> +
> +   FILENAME should be the name or path associated with the file matching
> +   BUILD_ID.  It is used for printing messages to the user.
> +
> +   SECTION_NAME should be the name of an ELF/DWARF section.
> +
> +   If the file is successfully retrieved, return a file descriptor and store
> +   the file's local path in DESTNAME.  If unsuccessful, print an error message
> +   and return a negative errno.  If GDB is not built with debuginfod or
> +   libdebuginfod does not support section queries, this function returns
> +   -ENOSYS.  */
> +
> +extern scoped_fd debuginfod_section_query (const unsigned char *build_id,
> +					   int build_id_len,
> +					   const char *filename,
> +					   const char *section_name,
> +					   gdb::unique_xmalloc_ptr<char>
> +					     *destname);
>  #endif /* DEBUGINFOD_SUPPORT_H */
> -- 
> 2.40.1