public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Andrew Burgess <aburgess@redhat.com>
To: gdb-patches@sourceware.org
Cc: Andrew Burgess <aburgess@redhat.com>, Tom Tromey <tom@tromey.com>
Subject: [PATCHv2 2/5] gdb: merge debug symbol file lookup code from coffread & elfread paths
Date: Wed,  8 Nov 2023 15:48:56 +0000	[thread overview]
Message-ID: <9872e349b990347597086865520991dbb023feaa.1699458402.git.aburgess@redhat.com> (raw)
In-Reply-To: <cover.1699458402.git.aburgess@redhat.com>

This commit merges the code that looks for and loads the separate
debug symbol files from coffread.c and elfread.c.  The factored out
code is moved into a new objfile::find_and_add_separate_symbol_file()
method.

For the elfread.c path there should be no user visible changes after
this commit.

For the coffread.c path GDB will now attempt to perform a debuginfod
lookup for the missing debug information, assuming that GDB can find a
build-id in the COFF file.

I don't know if COFF files can include a build-id, but I the existing
coffread.c code already includes a call to
find_separate_debug_file_by_build-id, so I know that it is at least OK
for GDB to ask a COFF file for a build-id.  If the COFF file doesn't
include a build-id then the debuginfod lookup code will not trigger
and the new code is harmless.

If the COFF file does include a build-id, then we're going to end up
asking debuginfod for the debug file.  As build-ids should be unique,
this should be harmless, even if debuginfod doesn't contain any
suitable debug data, it just costs us one debuginfod lookup, so I'm
not too worried about this for now.

As with the previous commit, I've done some minimal testing using the
mingw toolchain on a Linux machine, GDB seems to still access the
split debug information just fine.
---
 gdb/coffread.c      | 24 ++---------------
 gdb/elfread.c       | 57 +++------------------------------------
 gdb/objfiles.h      | 10 +++++++
 gdb/symfile-debug.c | 66 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 81 insertions(+), 76 deletions(-)

diff --git a/gdb/coffread.c b/gdb/coffread.c
index e1415d6b258..5898b3a8e08 100644
--- a/gdb/coffread.c
+++ b/gdb/coffread.c
@@ -40,8 +40,6 @@
 
 #include "coff-pe-read.h"
 
-#include "build-id.h"
-
 /* The objfile we are currently reading.  */
 
 static struct objfile *coffread_objfile;
@@ -729,26 +727,8 @@ coff_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
 	   && objfile->separate_debug_objfile == NULL
 	   && objfile->separate_debug_objfile_backlink == NULL)
     {
-      deferred_warnings warnings;
-      std::string debugfile
-	= find_separate_debug_file_by_buildid (objfile, &warnings);
-
-      if (debugfile.empty ())
-	debugfile
-	  = find_separate_debug_file_by_debuglink (objfile, &warnings);
-
-      if (!debugfile.empty ())
-	{
-	  gdb_bfd_ref_ptr debug_bfd (symfile_bfd_open (debugfile.c_str ()));
-
-	  symbol_file_add_separate (debug_bfd, debugfile.c_str (),
-				    symfile_flags, objfile);
-	}
-      /* If all the methods to collect the debuginfo failed, print any
-	 warnings that were collected, this is a no-op if there are no
-	 warnings.  */
-      if (debugfile.empty ())
-	warnings.emit ();
+      if (objfile->find_and_add_separate_symbol_file (symfile_flags))
+	gdb_assert (objfile->separate_debug_objfile != nullptr);
     }
 }
 
diff --git a/gdb/elfread.c b/gdb/elfread.c
index 7900dfbc388..86e7f61586e 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -41,14 +41,12 @@
 #include "regcache.h"
 #include "bcache.h"
 #include "gdb_bfd.h"
-#include "build-id.h"
 #include "location.h"
 #include "auxv.h"
 #include "mdebugread.h"
 #include "ctfread.h"
 #include "gdbsupport/gdb_string_view.h"
 #include "gdbsupport/scoped_fd.h"
-#include "debuginfod-support.h"
 #include "dwarf2/public.h"
 #include "cli/cli-cmds.h"
 
@@ -1218,59 +1216,10 @@ elf_symfile_read_dwarf2 (struct objfile *objfile,
 	   && objfile->separate_debug_objfile == NULL
 	   && objfile->separate_debug_objfile_backlink == NULL)
     {
-      deferred_warnings warnings;
-
-      std::string debugfile
-	= find_separate_debug_file_by_buildid (objfile, &warnings);
-
-      if (debugfile.empty ())
-	debugfile = find_separate_debug_file_by_debuglink (objfile, &warnings);
-
-      if (!debugfile.empty ())
-	{
-	  gdb_bfd_ref_ptr debug_bfd
-	    (symfile_bfd_open_no_error (debugfile.c_str ()));
-
-	  if (debug_bfd != nullptr)
-	    symbol_file_add_separate (debug_bfd, debugfile.c_str (),
-				      symfile_flags, objfile);
-	}
+      if (objfile->find_and_add_separate_symbol_file (symfile_flags))
+	gdb_assert (objfile->separate_debug_objfile != nullptr);
       else
-	{
-	  has_dwarf2 = false;
-	  const struct bfd_build_id *build_id
-	    = build_id_bfd_get (objfile->obfd.get ());
-	  const char *filename = bfd_get_filename (objfile->obfd.get ());
-
-	  if (build_id != nullptr)
-	    {
-	      gdb::unique_xmalloc_ptr<char> symfile_path;
-	      scoped_fd fd (debuginfod_debuginfo_query (build_id->data,
-							build_id->size,
-							filename,
-							&symfile_path));
-
-	      if (fd.get () >= 0)
-		{
-		  /* File successfully retrieved from server.  */
-		  gdb_bfd_ref_ptr debug_bfd
-		    (symfile_bfd_open_no_error (symfile_path.get ()));
-
-		  if (debug_bfd != nullptr
-		      && build_id_verify (debug_bfd.get (), build_id->size,
-					  build_id->data))
-		    {
-		      symbol_file_add_separate (debug_bfd, symfile_path.get (),
-						symfile_flags, objfile);
-		      has_dwarf2 = true;
-		    }
-		}
-	    }
-	}
-      /* If all the methods to collect the debuginfo failed, print the
-	 warnings, this is a no-op if there are no warnings.  */
-      if (debugfile.empty () && !has_dwarf2)
-	warnings.emit ();
+	has_dwarf2 = false;
     }
 
   return has_dwarf2;
diff --git a/gdb/objfiles.h b/gdb/objfiles.h
index 4b8aa9bfcec..ec9d354e4a7 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -513,6 +513,16 @@ struct objfile
 
   bool has_partial_symbols ();
 
+  /* Look for a separate debug symbol file for this objfile, make use of
+     build-id, debug-link, and debuginfod as necessary.  If a suitable
+     separate debug symbol file is found then it is loaded using a call to
+     symbol_file_add_separate (SYMFILE_FLAGS is passed through unmodified
+     to this call) and this function returns true.  If no suitable separate
+     debug symbol file is found and loaded then this function returns
+     false.  */
+
+  bool find_and_add_separate_symbol_file (symfile_add_flags symfile_flags);
+
   /* Return true if this objfile has any unexpanded symbols.  A return
      value of false indicates either, that this objfile has all its
      symbols fully expanded (i.e. fully read in), or that this objfile has
diff --git a/gdb/symfile-debug.c b/gdb/symfile-debug.c
index 850da4147a3..961ae2327f7 100644
--- a/gdb/symfile-debug.c
+++ b/gdb/symfile-debug.c
@@ -35,6 +35,8 @@
 #include "block.h"
 #include "filenames.h"
 #include "cli/cli-style.h"
+#include "build-id.h"
+#include "debuginfod-support.h"
 
 /* We need to save a pointer to the real symbol functions.
    Plus, the debug versions are malloc'd because we have to NULL out the
@@ -558,6 +560,70 @@ objfile::require_partial_symbols (bool verbose)
     }
 }
 
+/* See objfiles.h.  */
+
+bool
+objfile::find_and_add_separate_symbol_file (symfile_add_flags symfile_flags)
+{
+  bool has_dwarf2 = true;
+
+  deferred_warnings warnings;
+
+  std::string debugfile
+    = find_separate_debug_file_by_buildid (this, &warnings);
+
+  if (debugfile.empty ())
+    debugfile = find_separate_debug_file_by_debuglink (this, &warnings);
+
+  if (!debugfile.empty ())
+    {
+      gdb_bfd_ref_ptr debug_bfd
+	(symfile_bfd_open_no_error (debugfile.c_str ()));
+
+      if (debug_bfd != nullptr)
+	symbol_file_add_separate (debug_bfd, debugfile.c_str (),
+				  symfile_flags, this);
+    }
+  else
+    {
+      has_dwarf2 = false;
+      const struct bfd_build_id *build_id
+	= build_id_bfd_get (this->obfd.get ());
+      const char *filename = bfd_get_filename (this->obfd.get ());
+
+      if (build_id != nullptr)
+	{
+	  gdb::unique_xmalloc_ptr<char> symfile_path;
+	  scoped_fd fd (debuginfod_debuginfo_query (build_id->data,
+						    build_id->size,
+						    filename,
+						    &symfile_path));
+
+	  if (fd.get () >= 0)
+	    {
+	      /* File successfully retrieved from server.  */
+	      gdb_bfd_ref_ptr debug_bfd
+		(symfile_bfd_open_no_error (symfile_path.get ()));
+
+	      if (debug_bfd != nullptr
+		  && build_id_verify (debug_bfd.get (), build_id->size,
+				      build_id->data))
+		{
+		  symbol_file_add_separate (debug_bfd, symfile_path.get (),
+					    symfile_flags, this);
+		  has_dwarf2 = true;
+		}
+	    }
+	}
+    }
+  /* If all the methods to collect the debuginfo failed, print the
+     warnings, this is a no-op if there are no warnings.  */
+  if (debugfile.empty () && !has_dwarf2)
+    warnings.emit ();
+
+  return has_dwarf2;
+}
+
 \f
 /* Debugging version of struct sym_probe_fns.  */
 
-- 
2.25.4


  parent reply	other threads:[~2023-11-08 15:49 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-18 10:53 [PATCH 0/5] New Python hook for missing debug information Andrew Burgess
2023-10-18 10:53 ` [PATCH 1/5] gdb/coffread: bring separate debug file logic into line with elfread.c Andrew Burgess
2023-10-20 17:35   ` Tom Tromey
2023-10-24 11:59     ` Andrew Burgess
2023-10-18 10:53 ` [PATCH 2/5] gdb: merge debug symbol file lookup code from coffread & elfread paths Andrew Burgess
2023-10-18 13:18   ` Jon Turney
2023-10-18 17:25     ` Andrew Burgess
2023-10-18 10:53 ` [PATCH 3/5] gdb: refactor objfile::find_and_add_separate_symbol_file Andrew Burgess
2023-10-20 17:50   ` Tom Tromey
2023-10-24 12:03     ` Andrew Burgess
2023-11-08 15:22     ` Andrew Burgess
2023-10-18 10:53 ` [PATCH 4/5] gdb: add an extension language hook for missing debug info Andrew Burgess
2023-10-18 10:53 ` [PATCH 5/5] gdb: implement missing debug handler hook for Python Andrew Burgess
2023-10-18 12:08   ` Eli Zaretskii
2023-11-08 15:48 ` [PATCHv2 0/5] New Python hook for missing debug information Andrew Burgess
2023-11-08 15:48   ` [PATCHv2 1/5] gdb/coffread: bring separate debug file logic into line with elfread.c Andrew Burgess
2023-11-08 15:48   ` Andrew Burgess [this message]
2023-11-08 15:48   ` [PATCHv2 3/5] gdb: refactor objfile::find_and_add_separate_symbol_file Andrew Burgess
2023-11-08 15:48   ` [PATCHv2 4/5] gdb: add an extension language hook for missing debug info Andrew Burgess
2023-11-08 15:48   ` [PATCHv2 5/5] gdb: implement missing debug handler hook for Python Andrew Burgess
2023-11-12 22:38     ` Tom Tromey
2023-11-15 12:36     ` Tom de Vries
2023-11-16 10:59       ` Andrew Burgess
2023-11-16 11:16         ` Tom de Vries
2023-11-16 17:21           ` Andrew Burgess
2023-11-16 15:26         ` Tom Tromey
2023-11-12 22:39   ` [PATCHv2 0/5] New Python hook for missing debug information Tom Tromey
2023-11-13 16:04     ` Andrew Burgess
2023-11-13 17:18       ` Tom Tromey
2023-11-12 22:40   ` Tom Tromey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9872e349b990347597086865520991dbb023feaa.1699458402.git.aburgess@redhat.com \
    --to=aburgess@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=tom@tromey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).