[1/6,v2] gdb/debuginfod: Add debuginfod_section_query

  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.
---
 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(-)
  

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
  

Andrew Burgess <aburgess@redhat.com> writes:

> 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>

Actually, on thinking about this further, I'd like to withdraw this
approval, and ask a question / make a suggestion:

Why do we need AC_DEBUGINFOD and AC_DEBUGINFOD_SECTION?  Couldn't you
just replace AC_DEBUGINFOD with AC_DEBUGINFOD_SECTION?  The SECTION
version checks for 0.179 and 0.188 and sets the various flags based on
whatever is found.

You would need to update binutils by running autoreconf, which would set
the new define, but binutils would just ignore that define and all would
be good.

I think I would split this first patch in two, the first part would
update the autoconf macro and then run autoreconf in binutils and gdb.
Cross post this to both mailing lists, and get this merged.

Then rebase this GDB series on top of that, and now GDB can just start
making use of the new #define.

Thanks,
Andrew