[RFC PATCH] Support debuginfo and source file fetching via debuginfo server

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

[RFC PATCH] Support debuginfo and source file fetching via debuginfo server

Aaron Merey
Debuginfo server is a lightweight web service that indexes debuginfo
and source files by build-id and serves them over HTTP. Debuginfo server
is able to index unpackaged, locally-built software in addition to RPM
files. Debuginfo server is packaged with a shared library, libdbgserver,
that provides a small set of client functions for fetching files from
debuginfo server. This patch adds debuginfo server support to GDB. In
case a source file or separate debuginfo file cannot be found locally,
GDB can use libdbgserver to query debuginfo server for the file in
question, if enabled to do so.

We plan on packaging debuginfo server with elfutils. For more information
please see the 'dbgserver' branch of the elfutils git repo
(https://sourceware.org/git/?p=elfutils.git;a=shortlog;h=refs/heads/dbgserver).

This patch was tested on x86-64 Fedora 29. The testsuite was run both
with and without this patch applied and the summaries indicated no
regressions. Debuginfo server tests were also added to the testsuite
under gdb/testsuite/gdb.dbgserver/.

gdb/ChangeLog:

        * configure.ac: Add --with-dbgserver option.
        * config.in: Add libdbgserver.
        * elfread.c (elf_symfile_read): Query debuginfo server if
        separate debuginfo file cannot be found.
        * Makefile.in (CLIBS): Add libdbgserver.
        * source.c (open_source_file): Query debuginfo server if source
        file cannot be found.
        * top.c (print_gdb_configuration): Print "--with-dbgserver" or
        "--without-dbgserver" depending on whether GDB was
        configured with debuginfo server.

gdb/testsuite/ChangeLog:

        * gdb.dbgserver/: New directory for debuginfo server tests.
        * gdb.dbgserver/fetch_src_and_symbols.c: New file.
        * gdb.dbgserver/fetch_src_and_symbols.exp: Test debuginfo
        server's ability to fetch source and separate debuginfo files.
---
 gdb/Makefile.in                               |  5 +-
 gdb/config.in                                 |  3 +
 gdb/configure.ac                              | 26 ++++++
 gdb/elfread.c                                 | 27 ++++++
 gdb/source.c                                  | 56 +++++++++++
 .../gdb.dbgserver/fetch_src_and_symbols.c     |  1 +
 .../gdb.dbgserver/fetch_src_and_symbols.exp   | 93 +++++++++++++++++++
 gdb/top.c                                     |  9 ++
 8 files changed, 219 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.dbgserver/fetch_src_and_symbols.c
 create mode 100644 gdb/testsuite/gdb.dbgserver/fetch_src_and_symbols.exp

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index d5d095aae4..3149e0ea53 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -210,6 +210,9 @@ INTL = @LIBINTL@
 INTL_DEPS = @LIBINTL_DEP@
 INTL_CFLAGS = @INCINTL@
 
+# Where is the dbgserver library, if any?
+LIBDBGSERVER = @LIBDBGSERVER@
+
 # Where is the ICONV library?  This will be empty if in libc or not available.
 LIBICONV = @LIBICONV@
 
@@ -593,7 +596,7 @@ CLIBS = $(SIM) $(READLINE) $(OPCODES) $(BFD) $(ZLIB) $(INTL) $(LIBIBERTY) $(LIBD
  @LIBS@ @GUILE_LIBS@ @PYTHON_LIBS@ \
  $(LIBEXPAT) $(LIBLZMA) $(LIBBABELTRACE) $(LIBIPT) \
  $(LIBIBERTY) $(WIN32LIBS) $(LIBGNU) $(LIBICONV) $(LIBMPFR) \
- $(SRCHIGH_LIBS)
+ $(SRCHIGH_LIBS) $(LIBDBGSERVER)
 CDEPS = $(NAT_CDEPS) $(SIM) $(BFD) $(READLINE_DEPS) \
  $(OPCODES) $(INTL_DEPS) $(LIBIBERTY) $(CONFIG_DEPS) $(LIBGNU)
 
diff --git a/gdb/config.in b/gdb/config.in
index 26ca02f6a3..f2f4b38bd7 100644
--- a/gdb/config.in
+++ b/gdb/config.in
@@ -231,6 +231,9 @@
 /* Define if you have the expat library. */
 #undef HAVE_LIBEXPAT
 
+/* Define if you have the dbgserver library. */
+#undef HAVE_LIBDBGSERVER
+
 /* Define to 1 if you have the `libiconvlist' function. */
 #undef HAVE_LIBICONVLIST
 
diff --git a/gdb/configure.ac b/gdb/configure.ac
index 5a18c16405..a224c7df81 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -327,6 +327,32 @@ case $host_os in
     enable_gdbtk=no ;;
 esac
 
+# Enable dbgserver
+AC_ARG_WITH([dbgserver],
+        AC_HELP_STRING([--with-dbgserver],
+                       [Enable debuginfo and source-file lookups with dbgserver (auto/yes/no)]),
+        [], [with_dbgserver=auto])
+AC_MSG_CHECKING([whether to use dbgserver])
+AC_MSG_RESULT([$with_dbgserver])
+
+if test "${with_dbgserver}" = no; then
+  AC_MSG_WARN([dbgserver support disabled; some features may be unavailable.])
+  HAVE_LIBDBGSERVER=no
+else
+  AC_LIB_HAVE_LINKFLAGS([dbgserver], [], [#include <elfutils/dbgserver-client.h>],
+                                     [const unsigned char buildid;
+                                     int buildid_len = 1;
+                                     char *path;
+                                     path = dbgserver_find_debuginfo (& buildid, buildid_len, &path);])
+  if test "$HAVE_LIBDBGSERVER" != yes; then
+    if test "$with_dbgserver" = yes; then
+      AC_MSG_ERROR([dbgserver is missing or unusable])
+    else
+      AC_MSG_WARN([dbgserver is missing or unusable; some features may be unavailable.])
+    fi
+  fi
+fi
+
 # Libunwind support for ia64.
 
 AC_ARG_WITH(libunwind-ia64,
diff --git a/gdb/elfread.c b/gdb/elfread.c
index 630550b80d..aa81ddf3c3 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -47,6 +47,9 @@
 #include "location.h"
 #include "auxv.h"
 #include "mdebugread.h"
+#if HAVE_LIBDBGSERVER
+#include <elfutils/dbgserver-client.h>
+#endif
 
 /* Forward declarations.  */
 extern const struct sym_fns elf_sym_fns_gdb_index;
@@ -1296,6 +1299,30 @@ elf_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
   symbol_file_add_separate (debug_bfd.get (), debugfile.c_str (),
     symfile_flags, objfile);
  }
+#if HAVE_LIBDBGSERVER
+      else
+        {
+          const struct bfd_build_id *build_id;
+          char *debugfile_path;
+
+          build_id = build_id_bfd_get (objfile->obfd);
+          int fd = dbgserver_find_debuginfo (build_id->data,
+                                             build_id->size,
+                                             &debugfile_path);
+
+          if (fd >= 0)
+            {
+              /* debuginfo successfully retrieved from server, reopen
+                 the file as a bfd instead.  */
+              gdb_bfd_ref_ptr debug_bfd (symfile_bfd_open (debugfile_path));
+
+              symbol_file_add_separate(debug_bfd.get (), debugfile_path,
+                                       symfile_flags, objfile);
+              close(fd);
+              free(debugfile_path);
+            }
+        }
+#endif /* LIBDBGSERVER */
     }
 }
 
diff --git a/gdb/source.c b/gdb/source.c
index b27f210802..0150fd7c42 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -30,6 +30,10 @@
 
 #include <sys/types.h>
 #include <fcntl.h>
+#include "build-id.h"
+#ifdef HAVE_LIBDBGSERVER
+#include <elfutils/dbgserver-client.h>
+#endif
 #include "gdbcore.h"
 #include "gdb_regex.h"
 #include "symfile.h"
@@ -1060,6 +1064,58 @@ open_source_file (struct symtab *s)
   s->fullname = NULL;
   scoped_fd fd = find_and_open_source (s->filename, SYMTAB_DIRNAME (s),
        &fullname);
+
+#if HAVE_LIBDBGSERVER
+  if (fd.get() < 0)
+    {
+      if (SYMTAB_COMPUNIT(s) != NULL)
+        {
+          const struct bfd_build_id *build_id;
+          const objfile *ofp = COMPUNIT_OBJFILE (SYMTAB_COMPUNIT (s));
+
+          /* prefix the comp_dir to relative file names */
+          const char* dirname = SYMTAB_DIRNAME (s);
+          int suffname_len = strlen(dirname) + strlen(s->filename) + 2;
+          char *suffname = (char *) alloca(suffname_len);
+          if (IS_DIR_SEPARATOR (s->filename[0]))
+            xsnprintf (suffname, suffname_len, "%s", s->filename);
+          else
+            {
+              xsnprintf (suffname, suffname_len, "%s%s%s", dirname,
+                         SLASH_STRING, s->filename);
+            }
+
+          build_id = build_id_bfd_get (ofp->obfd);
+
+          /* Query debuginfo-server for the source file.  */
+          if (build_id != NULL)
+            {
+              char *name_in_cache;
+              int dbgsrv_rc = dbgserver_find_source (build_id->data,
+                                                     build_id->size,
+                                                     suffname,
+                                                     &name_in_cache);
+              if (dbgsrv_rc >= 0)
+                {
+                  fullname.reset (xstrdup(name_in_cache));
+                  free (name_in_cache);
+                }
+              else if (dbgsrv_rc == -ENOSYS)
+                {
+                  /* -ENOSYS indicates that libdbgserver could not find
+                     any dbgserver URLs to query due to $DBGSERVER_URLS
+                     not being defined. Replace -ENOSYS with -ENOENT so
+                     that users who have not configured dbgserver see the
+                     usual error message when a source file cannot be found.  */
+                  dbgsrv_rc = -ENOENT;
+                }
+              s->fullname = fullname.release ();
+              return scoped_fd(dbgsrv_rc);
+            }
+        }
+    }
+#endif /* HAVE_LIBDBGSERVER */
+
   s->fullname = fullname.release ();
   return fd;
 }
diff --git a/gdb/testsuite/gdb.dbgserver/fetch_src_and_symbols.c b/gdb/testsuite/gdb.dbgserver/fetch_src_and_symbols.c
new file mode 100644
index 0000000000..76e8197013
--- /dev/null
+++ b/gdb/testsuite/gdb.dbgserver/fetch_src_and_symbols.c
@@ -0,0 +1 @@
+int main() { return 0; }
diff --git a/gdb/testsuite/gdb.dbgserver/fetch_src_and_symbols.exp b/gdb/testsuite/gdb.dbgserver/fetch_src_and_symbols.exp
new file mode 100644
index 0000000000..9d21d6a96d
--- /dev/null
+++ b/gdb/testsuite/gdb.dbgserver/fetch_src_and_symbols.exp
@@ -0,0 +1,93 @@
+# Copyright 2010-2019 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# test dbgserver functionality
+
+standard_testfile
+
+# skip testing if dbgserver cannot be found
+if { [which dbgserver] == 0 } {
+    untested "cannot find dbgserver"
+    return -1
+}
+
+# skip testing if gdb was not configured with dbgserver
+if { [string first "with-dbgserver" [exec $GDB --configuration]] == -1 } {
+    untested "GDB not configured with dbgserver"
+    return -1
+}
+
+set cache [file join [standard_output_file {}] ".client_cache"]
+
+# make sure there isn't an old client cache lying around
+file delete -force $cache
+
+# make a copy source file that we can move around
+if { [catch {file copy -force ${srcdir}/${subdir}/${srcfile} \
+                              [standard_output_file tmp-${srcfile}]}] != 0 } {
+    error "Could not create temporary file"
+    return -1
+}
+
+set sourcetmp [standard_output_file tmp-${srcfile}]
+set outputdir [standard_output_file {}]
+
+if { [gdb_compile "${sourcetmp}" "${binfile}" executable {debug}] != "" } {
+    return -1
+}
+
+set port 58002
+set ::env(DBGSERVER_URLS) "localhost:$port"
+set ::env(DBGSERVER_TIMEOUT) 10
+set ::env(DBGSERVER_CACHE_PATH) $cache
+
+# test that gdb cannot find source without dbgserver
+gdb_start
+gdb_load $binfile
+gdb_test_no_output "set substitute-path $outputdir /dev/null"
+gdb_test "l" ".*Connection refused\."
+gdb_exit
+
+# strip symbols into separate file and move it so gdb cannot find it without dbgserver
+gdb_gnu_strip_debug $binfile
+
+set debugdir [file join [standard_output_file {}] "debug"]
+set debuginfo [file join [standard_output_file {}] "fetch_src_and_symbols.debug"]
+
+file mkdir -force $debugdir
+file copy -force $debuginfo $debugdir
+file delete -force $debuginfo
+
+# test that gdb cannot find symbols without dbgserver
+gdb_start
+gdb_load $binfile
+gdb_test "file" ".*No symbol file.*"
+gdb_exit
+
+# start up dbgserver
+
+set dbgserver_pid [exec dbgserver -p $port -F $debugdir >/dev/null 2>&1 &]
+sleep 5
+
+# gdb should now be able to find the symbol and source files
+gdb_start
+gdb_load $binfile
+gdb_test_no_output "set substitute-path $outputdir /dev/null"
+gdb_test "br main" "Breakpoint 1 at.*file.*"
+gdb_test "l" "int main().*return 0;.*"
+gdb_exit
+
+file delete -force $cache
+exec kill $dbgserver_pid
diff --git a/gdb/top.c b/gdb/top.c
index 9d4ce1fa3b..a1715a9910 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -1480,6 +1480,15 @@ This GDB was configured as follows:\n\
              --without-python\n\
 "));
 #endif
+#if HAVE_LIBDBGSERVER
+  fprintf_filtered (stream, _("\
+             --with-dbgserver\n\
+"));
+#else
+   fprintf_filtered (stream, _("\
+             --without-dbgserver\n\
+"));
+#endif
 #if HAVE_GUILE
   fprintf_filtered (stream, _("\
              --with-guile\n\
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

Re: [RFC PATCH] Support debuginfo and source file fetching via debuginfo server

Tom Tromey-2
>>>>> "Aaron" == Aaron Merey <[hidden email]> writes:

Aaron> Debuginfo server is a lightweight web service that indexes debuginfo
Aaron> and source files by build-id and serves them over HTTP.

Thank you for the patch.

I think the idea is fine for gdb, so all that's left is some nits in the
patch.

Aaron> +#if HAVE_LIBDBGSERVER
Aaron> +      else
Aaron> +        {
Aaron> +          const struct bfd_build_id *build_id;
Aaron> +          char *debugfile_path;
Aaron> +
Aaron> +          build_id = build_id_bfd_get (objfile->obfd);
Aaron> +          int fd = dbgserver_find_debuginfo (build_id->data,
Aaron> +                                             build_id->size,
Aaron> +                                             &debugfile_path);

I was wondering what the fd represents.  If it's open on the file, can
we simply reuse it rather than trying to reopen the file?

Instead of "int", using scoped_fd would be better.

Aaron> +              symbol_file_add_separate(debug_bfd.get (), debugfile_path,

GNU style is a space before parens - there are a few instances.

Aaron> +              free(debugfile_path);

xfree instead of free.

Aaron> +#if HAVE_LIBDBGSERVER
Aaron> +  if (fd.get() < 0)
Aaron> +    {
Aaron> +      if (SYMTAB_COMPUNIT(s) != NULL)
Aaron> +        {
Aaron> +          const struct bfd_build_id *build_id;
Aaron> +          const objfile *ofp = COMPUNIT_OBJFILE (SYMTAB_COMPUNIT (s));
Aaron> +
Aaron> +          /* prefix the comp_dir to relative file names */
Aaron> +          const char* dirname = SYMTAB_DIRNAME (s);
Aaron> +          int suffname_len = strlen(dirname) + strlen(s->filename) + 2;
Aaron> +          char *suffname = (char *) alloca(suffname_len);

I think it's better to just use std::string for this kind of thing.

Probably this area will need some refactoring since other patches have
touched this.

Aaron> +              char *name_in_cache;
Aaron> +              int dbgsrv_rc = dbgserver_find_source (build_id->data,
Aaron> +                                                     build_id->size,
Aaron> +                                                     suffname,
Aaron> +                                                     &name_in_cache);
Aaron> +              if (dbgsrv_rc >= 0)
Aaron> +                {
Aaron> +                  fullname.reset (xstrdup(name_in_cache));
Aaron> +                  free (name_in_cache);

It seems like you could just use

    fullname.reset (name_in_cache);

here.

Aaron> +                }
Aaron> +              else if (dbgsrv_rc == -ENOSYS)
Aaron> +                {
Aaron> +                  /* -ENOSYS indicates that libdbgserver could not find
Aaron> +                     any dbgserver URLs to query due to $DBGSERVER_URLS
Aaron> +                     not being defined. Replace -ENOSYS with -ENOENT so
Aaron> +                     that users who have not configured dbgserver see the
Aaron> +                     usual error message when a source file cannot be found.  */
Aaron> +                  dbgsrv_rc = -ENOENT;

This assignment doesn't seem useful here.

Tom