diff mbox series

[1/2] pdb: Allow loading by gdb

Message ID 20230509003247.24156-1-mark@harmstone.com
State Accepted
Headers
Series [1/2] pdb: Allow loading by gdb |

Checks

Context Check Description
snail/binutils-gdb-check success Github commit url

Commit Message

Mark Harmstone May 9, 2023, 12:32 a.m. UTC 
  These are the first patches adding support to allow GDB to load Microsoft's
PDB debugging files.

Add a new bfd_flavour value, and expose PDB files as objects, so that
they get accepted by add-symbol-file.

---
 bfd/bfd-in2.h | 3 ++-
 bfd/pdb.c     | 6 ++++--
 bfd/targets.c | 4 +++-
 3 files changed, 9 insertions(+), 4 deletions(-)

Comments

Alan Modra May 10, 2023, 12:56 a.m. UTC | #1 
On Tue, May 09, 2023 at 01:32:46AM +0100, Mark Harmstone wrote:
> Add a new bfd_flavour value, and expose PDB files as objects, so that
> they get accepted by add-symbol-file.

By equating object_p and archive_p you are going to get whichever of
bfd_archive or bfd_object is tried first as the argument of
bfd_check_format (or bfd_check_format_matches).  This seems fragile to
me.  We have multiple binary utilities, ld, and gbd all calling
bfd_check_format.  Do they all work correctly with this change, and
will they continue to work correctly with future changes?

I think you'd be better off staying with just one format, and
bfd_archive probably fits pdb files better than bfd_object.
Mark Harmstone May 15, 2023, 1:04 a.m. UTC | #2 
On 10/5/23 01:56, Alan Modra wrote:
> I think you'd be better off staying with just one format, and
> bfd_archive probably fits pdb files better than bfd_object.

Thanks Alan. But `add-symbol-file` is set up to accept object
files - does this imply that I should change it to treat PDB
archives as a special case?

Mark
Alan Modra May 15, 2023, 1:26 a.m. UTC | #3 
On Mon, May 15, 2023 at 02:04:29AM +0100, Mark Harmstone wrote:
> On 10/5/23 01:56, Alan Modra wrote:
> > I think you'd be better off staying with just one format, and
> > bfd_archive probably fits pdb files better than bfd_object.
> 
> Thanks Alan. But `add-symbol-file` is set up to accept object
> files - does this imply that I should change it to treat PDB
> archives as a special case?

I expect that is what should happen, but I'm not a gdb maintainer and
probably shouldn't be giving gdb advice.
Tom Tromey May 16, 2023, 3:03 p.m. UTC | #4 
>>>>> "Mark" == Mark Harmstone <mark@harmstone.com> writes:

Mark> On 10/5/23 01:56, Alan Modra wrote:
>> I think you'd be better off staying with just one format, and
>> bfd_archive probably fits pdb files better than bfd_object.

Mark> Thanks Alan. But `add-symbol-file` is set up to accept object
Mark> files - does this imply that I should change it to treat PDB
Mark> archives as a special case?

It seems fine to me as long as add-symbol-file doesn't end up letting
users supply ".a" files.

I guess I'd expect the usual approach to be something more automatic
though?

Tom
diff mbox series

Patch

diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index 7be18db20a8..e9b3e8e9a21 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -7410,7 +7410,8 @@  enum bfd_flavour
   bfd_target_mach_o_flavour,
   bfd_target_pef_flavour,
   bfd_target_pef_xlib_flavour,
-  bfd_target_sym_flavour
+  bfd_target_sym_flavour,
+  bfd_target_pdb_flavour
 };
 
 enum bfd_endian { BFD_ENDIAN_BIG, BFD_ENDIAN_LITTLE, BFD_ENDIAN_UNKNOWN };
diff --git a/bfd/pdb.c b/bfd/pdb.c
index 7fefd8140fa..c48fb459b9f 100644
--- a/bfd/pdb.c
+++ b/bfd/pdb.c
@@ -62,6 +62,8 @@  pdb_archive_p (bfd *abfd)
   return _bfd_no_cleanup;
 }
 
+#define pdb_object_p pdb_archive_p
+
 static bfd *
 pdb_get_elt_at_index (bfd *abfd, symindex sym_index)
 {
@@ -774,7 +776,7 @@  pdb_write_contents (bfd *abfd)
 const bfd_target pdb_vec =
 {
   "pdb",
-  bfd_target_unknown_flavour,
+  bfd_target_pdb_flavour,
   BFD_ENDIAN_LITTLE,		/* target byte order */
   BFD_ENDIAN_LITTLE,		/* target headers byte order */
   0,				/* object flags */
@@ -793,7 +795,7 @@  const bfd_target pdb_vec =
 
   {				/* bfd_check_format */
     _bfd_dummy_target,
-    _bfd_dummy_target,
+    pdb_object_p,
     pdb_archive_p,
     _bfd_dummy_target
   },
diff --git a/bfd/targets.c b/bfd/targets.c
index 3dbcd088966..a36e6f0f439 100644
--- a/bfd/targets.c
+++ b/bfd/targets.c
@@ -169,7 +169,8 @@  EXTERNAL
 .  bfd_target_mach_o_flavour,
 .  bfd_target_pef_flavour,
 .  bfd_target_pef_xlib_flavour,
-.  bfd_target_sym_flavour
+.  bfd_target_sym_flavour,
+.  bfd_target_pdb_flavour
 .};
 .
 .enum bfd_endian { BFD_ENDIAN_BIG, BFD_ENDIAN_LITTLE, BFD_ENDIAN_UNKNOWN };
@@ -1859,6 +1860,7 @@  bfd_flavour_name (enum bfd_flavour flavour)
     case bfd_target_pef_flavour: return "PEF";
     case bfd_target_pef_xlib_flavour: return "PEF_XLIB";
     case bfd_target_sym_flavour: return "SYM";
+    case bfd_target_pdb_flavour: return "PDB";
     /* There is no "default" case here so that -Wswitch (part of -Wall)
        catches missing entries.  */
     }