[PATCH 1/2] pdb: Allow loading by gdb

[PATCH 1/2] pdb: Allow loading by gdb

[Mark Harmstone]

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

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.  */
     }
-- 
2.39.2

[PATCH 2/2] gdb: Allow reading of enum definitions in PDB files

[Mark Harmstone]

Adds a new file pdbread.c, which parses LF_ENUM records in PDB files.

---
 gdb/Makefile.in                |   2 +
 gdb/configure.tgt              |   4 +-
 gdb/pdbread.c                  | 573 +++++++++++++++++++++++++++++++++
 gdb/pdbread.h                  | 105 ++++++
 gdb/testsuite/gdb.pdb/enum.exp |  39 +++
 gdb/testsuite/gdb.pdb/enum1.s  | 116 +++++++
 6 files changed, 837 insertions(+), 2 deletions(-)
 create mode 100644 gdb/pdbread.c
 create mode 100644 gdb/pdbread.h
 create mode 100644 gdb/testsuite/gdb.pdb/enum.exp
 create mode 100644 gdb/testsuite/gdb.pdb/enum1.s

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 93f506ee391..ac0d7e35a02 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -838,6 +838,7 @@ ALL_TARGET_OBS = \
 	obsd-tdep.o \
 	or1k-linux-tdep.o \
 	or1k-tdep.o \
+	pdbread.o \
 	ppc-fbsd-tdep.o \
 	ppc-linux-tdep.o \
 	ppc-netbsd-tdep.o \
@@ -1770,6 +1771,7 @@ ALLDEPFILES = \
 	obsd-nat.c \
 	obsd-tdep.c \
 	or1k-linux-nat.c \
+	pdbread.c \
 	posix-hdep.c \
 	ppc-fbsd-nat.c \
 	ppc-fbsd-tdep.c \
diff --git a/gdb/configure.tgt b/gdb/configure.tgt
index d5b7dd1e7d7..899acd67ae6 100644
--- a/gdb/configure.tgt
+++ b/gdb/configure.tgt
@@ -335,7 +335,7 @@ i[34567]86-*-cygwin*)
 	;;
 i[34567]86-*-mingw32*)
 	# Target: Intel 386 running win32
-	gdb_target_obs="i386-windows-tdep.o windows-tdep.o"
+	gdb_target_obs="i386-windows-tdep.o windows-tdep.o pdbread.o"
 	;;
 i[34567]86-*-go32* | i[34567]86-*-msdosdjgpp*)
 	# Target: i386 running DJGPP/go32.
@@ -728,7 +728,7 @@ x86_64-*-mingw* | x86_64-*-cygwin*)
         # Target: MingW/amd64
 	gdb_target_obs="amd64-windows-tdep.o \
                         ${i386_tobjs} i386-windows-tdep.o \
-                        windows-tdep.o"
+                        windows-tdep.o pdbread.o"
         ;;
 x86_64-*-netbsd* | x86_64-*-knetbsd*-gnu)
 	# Target: NetBSD/amd64
diff --git a/gdb/pdbread.c b/gdb/pdbread.c
new file mode 100644
index 00000000000..faabbcf2240
--- /dev/null
+++ b/gdb/pdbread.c
@@ -0,0 +1,573 @@
+/* GDB code to parse PDB debug files.
+   Copyright (C) 2023 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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/>.  */
+
+#include "defs.h"
+#include "symtab.h"
+#include "symfile.h"
+#include "objfiles.h"
+#include "buildsym.h"
+#include "complaints.h"
+#include "coff/i386.h"
+#include "coff/external.h"
+#include "pdbread.h"
+
+struct pdb_type
+{
+  file_ptr offset;
+  uint16_t kind;
+};
+
+static void
+pdb_new_init (struct objfile *ignore)
+{
+}
+
+static void
+pdb_symfile_init (struct objfile *objfile)
+{
+}
+
+/* Map the PDB builtin types to GDB's.  */
+static struct type *
+pdb_builtin_type (const struct builtin_type *builtin, uint32_t t)
+{
+  switch (t)
+    {
+    case T_CHAR:
+    case T_INT1:
+      return builtin->builtin_char;
+
+    case T_UCHAR:
+    case T_UINT1:
+      return builtin->builtin_unsigned_char;
+
+    case T_SHORT:
+    case T_INT2:
+      return builtin->builtin_short;
+
+    case T_USHORT:
+    case T_UINT2:
+      return builtin->builtin_unsigned_short;
+
+    case T_LONG:
+      return builtin->builtin_long;
+
+    case T_ULONG:
+      return builtin->builtin_unsigned_long;
+
+    case T_INT4:
+      return builtin->builtin_int;
+
+    case T_UINT4:
+      return builtin->builtin_unsigned_int;
+
+    case T_QUAD:
+    case T_INT8:
+      return builtin->builtin_long_long;
+
+    case T_UQUAD:
+    case T_UINT8:
+      return builtin->builtin_unsigned_long_long;
+    }
+
+  return NULL;
+}
+
+/* Return the size in bytes of a PDB builtin type.  */
+static ULONGEST
+pdb_builtin_type_length (uint32_t t)
+{
+  switch (t)
+    {
+    case T_CHAR:
+    case T_INT1:
+    case T_UCHAR:
+    case T_UINT1:
+      return 1;
+
+    case T_SHORT:
+    case T_INT2:
+    case T_USHORT:
+    case T_UINT2:
+      return 2;
+
+    case T_LONG:
+    case T_ULONG:
+    case T_INT4:
+    case T_UINT4:
+      return 4;
+
+    case T_QUAD:
+    case T_INT8:
+    case T_UQUAD:
+    case T_UINT8:
+      return 8;
+    }
+
+  return 0;
+}
+
+/* Some integers, such as enum values, get stored as an extended value if
+   they're too large to fit into two bytes.  The two bytes that would normally
+   be the value are instead a type indicator, and the actual value follows.  */
+static bool
+pdb_read_extended_value (uint16_t kind, char **ptr, uint16_t *length,
+			 LONGEST *ret)
+{
+  switch (kind)
+  {
+  case LF_CHAR:
+    if (*length < 1)
+      return false;
+
+    *ret = *(signed char*) *ptr;
+    (*ptr)++;
+    (*length)--;
+
+    break;
+
+  case LF_SHORT:
+    if (*length < sizeof (int16_t))
+      return false;
+
+    *ret = (int16_t) bfd_getl16 (*ptr);
+    *ptr += sizeof (int16_t);
+    *length -= sizeof (int16_t);
+
+    break;
+
+  case LF_USHORT:
+    if (*length < sizeof (uint16_t))
+      return false;
+
+    *ret = bfd_getl16 (*ptr);
+    *ptr += sizeof (uint16_t);
+    *length -= sizeof (uint16_t);
+
+    break;
+
+  case LF_LONG:
+    if (*length < sizeof (int32_t))
+      return false;
+
+    *ret = (int32_t) bfd_getl32 (*ptr);
+    *ptr += sizeof (int32_t);
+    *length -= sizeof (int32_t);
+
+    break;
+
+  case LF_ULONG:
+    if (*length < sizeof (uint32_t))
+      return false;
+
+    *ret = bfd_getl32 (*ptr);
+    *ptr += sizeof (uint32_t);
+    *length -= sizeof (uint32_t);
+
+    break;
+
+  case LF_QUADWORD:
+    if (*length < sizeof (int64_t))
+      return false;
+
+    *ret = (int64_t) bfd_getl64 (*ptr);
+    *ptr += sizeof (int64_t);
+    *length -= sizeof (int64_t);
+
+    break;
+
+  case LF_UQUADWORD:
+    if (*length < sizeof (uint64_t))
+      return false;
+
+    *ret = bfd_getl64 (*ptr);
+    *ptr += sizeof (uint64_t);
+    *length -= sizeof (uint64_t);
+
+    break;
+
+  default:
+    return false;
+  }
+
+  return true;
+}
+
+/* Read an LF_FIELDLIST entry for an enum.  This can have two possible subtypes:
+   LF_ENUMERATE, for a value, and LF_INDEX, which points to an overflow
+   fieldlist if this one gets too large.  */
+static void
+pdb_read_enum_fieldlist (std::vector<struct field> &fields, bfd *tpi,
+			 struct pdb_type *types, uint32_t fieldlist_type,
+			 uint32_t first_type, uint32_t last_type)
+{
+  struct pdb_type *type;
+  char int_buf [sizeof (uint16_t)];
+  uint16_t length;
+  char *buf, *ptr;
+
+  if (fieldlist_type < first_type || fieldlist_type > last_type)
+    return;
+
+  type = &types [fieldlist_type - first_type];
+
+  if (type->kind != LF_FIELDLIST)
+    return;
+
+  if (bfd_seek (tpi, type->offset, SEEK_SET) != 0)
+    return;
+
+  if (bfd_bread (int_buf, sizeof (int_buf), tpi) != sizeof (int_buf))
+    return;
+
+  length = bfd_getl16 (int_buf);
+
+  if (length <= sizeof (uint16_t))
+    return;
+
+  buf = (char *) xmalloc (length);
+
+  if (bfd_bread (buf, length, tpi) != length)
+    goto end;
+
+  /* Skip LF_FIELDLIST value.  */
+  ptr = buf + sizeof (uint16_t);
+  length -= sizeof (uint16_t);
+
+  while (length >= sizeof (uint16_t))
+    {
+      uint16_t sub_type;
+
+      sub_type = bfd_getl16 (ptr);
+      ptr += sizeof (uint16_t);
+      length -= sizeof (uint16_t);
+
+      switch (sub_type)
+	{
+	case LF_ENUMERATE: {
+	  struct lf_enumerate *en;
+	  LONGEST val;
+	  size_t name_len;
+	  char *name;
+
+	  if (length < sizeof (*en))
+	    goto end;
+
+	  en = (struct lf_enumerate *) ptr;
+
+	  ptr += sizeof (*en);
+	  length -= sizeof (*en);
+
+	  val = bfd_getl16 (&en->value);
+
+	  /* Extended value - actual value follows.  */
+	  if (val >= 0x8000)
+	    {
+	      if (!pdb_read_extended_value (val, &ptr, &length, &val))
+		goto end;
+	    }
+
+	  name_len = strnlen (ptr, length);
+
+	  /* Should be zero-terminated.  */
+	  if (name_len == length)
+	    goto end;
+
+	  name = ptr;
+	  ptr += name_len + 1;
+	  length -= name_len + 1;
+
+	  if (name_len != 0)
+	    {
+	      fields.emplace_back ();
+	      struct field &field = fields.back ();
+	      field.set_name (xstrdup (name));
+	      field.set_loc_enumval (val);
+	    }
+
+	  /* Round to 4-byte boundary.  */
+	  if ((ptr - buf + 2) % 4)
+	    {
+	      if (length < 4)
+		goto end;
+
+	      length -= 4 - ((ptr - buf + 2) % 4);
+	      ptr += 4 - ((ptr - buf + 2) % 4);
+	    }
+
+	    break;
+	  }
+
+	case LF_INDEX: {
+	  struct lf_index *ind;
+	  uint32_t fl_type;
+
+	  if (length < sizeof (*ind))
+	    goto end;
+
+	  ind = (struct lf_index *) ptr;
+
+	  ptr += sizeof (*ind);
+	  length -= sizeof (*ind);
+
+	  fl_type = bfd_getl32 (&ind->index);
+
+	  pdb_read_enum_fieldlist (fields, tpi, types, fl_type, first_type,
+				   last_type);
+
+	  break;
+	}
+
+	default:
+	  goto end;
+	}
+    }
+
+end:
+  free (buf);
+}
+
+/* Parse an LF_ENUM record, and add it as an enum symbol.  */
+static void
+pdb_add_enum (struct objfile *objfile, buildsym_compunit &builder,
+	      bfd *tpi, struct pdb_type *types, uint32_t type_off,
+	      uint32_t first_type, uint32_t last_type)
+{
+  char int_buf [sizeof (uint16_t)];
+  uint16_t length;
+  struct lf_enum en;
+  struct type *t;
+  struct symbol *sym;
+  size_t name_len;
+  char *name;
+  std::vector<struct field> fields;
+  struct pdb_type *type = &types [type_off];
+
+  if (bfd_seek (tpi, type->offset, SEEK_SET) != 0)
+    return;
+
+  if (bfd_bread (int_buf, sizeof (int_buf), tpi) != sizeof (int_buf))
+    return;
+
+  length = bfd_getl16 (int_buf);
+
+  if (length <= sizeof (en))
+    return;
+
+  if (bfd_bread (&en, sizeof (en), tpi) != sizeof (en))
+    return;
+
+  if (bfd_getl16 (&en.properties) & CV_PROP_FORWARD_REF)
+    return;
+
+  name_len = length - sizeof (en);
+  name = (char * ) xmalloc (name_len + 1);
+
+  if (bfd_bread (name, name_len, tpi) != name_len)
+    {
+      free (name);
+      return;
+    }
+
+  /* Should be zero-terminated, but just in case.  */
+  name[name_len] = 0;
+
+  t = type_allocator (objfile).new_type ();
+  t->set_code (TYPE_CODE_ENUM);
+  t->set_name (name);
+
+  t->set_target_type (pdb_builtin_type (builtin_type (objfile),
+					bfd_getl32 (&en.underlying_type)));
+  t->set_length (pdb_builtin_type_length (bfd_getl32 (&en.underlying_type)));
+
+  pdb_read_enum_fieldlist (fields, tpi, types, bfd_getl32 (&en.field_list),
+			   first_type, last_type);
+
+  if (!fields.empty ())
+    {
+      t->set_num_fields (fields.size ());
+      t->set_fields
+	((struct field *)
+	 TYPE_ALLOC (t, sizeof (struct field) * fields.size ()));
+      memcpy (t->fields (), fields.data (),
+	      sizeof (struct field) * fields.size ());
+    }
+
+  sym = new (&objfile->objfile_obstack) symbol;
+  sym->set_language (language_c, &objfile->objfile_obstack);
+  sym->set_domain (STRUCT_DOMAIN);
+  sym->set_aclass_index (LOC_TYPEDEF);
+  sym->set_linkage_name (name);
+  sym->set_type (t);
+
+  add_symbol_to_list (sym, builder.get_global_symbols ());
+}
+
+/* Read and parse the TPI stream, which contains the type definitions.  */
+static bool
+pdb_read_tpi_stream (struct objfile *objfile, buildsym_compunit &builder)
+{
+  bfd *tpi;
+  struct pdb_tpi_stream_header h;
+  uint32_t first_type, last_type, num_types;
+  struct pdb_type *types;
+
+  tpi = bfd_get_elt_at_index (objfile->obfd.get (), PDB_TPI_STREAM_NUM);
+
+  if (!tpi)
+    return false;
+
+  if (bfd_seek (tpi, 0, SEEK_SET) != 0)
+    {
+      bfd_close (tpi);
+      return false;
+    }
+
+  if (bfd_bread (&h, sizeof (h), tpi) != sizeof (h))
+    {
+      bfd_close (tpi);
+      return false;
+    }
+
+  if (bfd_getl32 (&h.version) != TPI_STREAM_VERSION_80)
+    {
+      bfd_close (tpi);
+      return false;
+    }
+
+  if (bfd_seek (tpi, bfd_getl32 (&h.header_size), SEEK_SET) != 0)
+    {
+      bfd_close (tpi);
+      return false;
+    }
+
+  first_type = bfd_getl32 (&h.type_index_begin);
+  last_type = bfd_getl32 (&h.type_index_end) - 1;
+
+  if (last_type <= first_type)
+    {
+      bfd_close (tpi);
+      return true;
+    }
+
+  num_types = last_type - first_type + 1;
+  types = (struct pdb_type *) xmalloc (sizeof (*types) * num_types);
+
+  for (unsigned int i = 0; i < num_types; i++)
+    {
+      char int_buf[sizeof (uint16_t)];
+      uint16_t length;
+
+      types[i].offset = bfd_tell(tpi);
+
+      if (bfd_bread (int_buf, sizeof (int_buf), tpi) != sizeof (int_buf))
+	{
+	  free (types);
+	  bfd_close (tpi);
+	  return false;
+	}
+
+      length = bfd_getl16 (int_buf);
+
+      if (length < sizeof (uint16_t))
+	{
+	  free (types);
+	  bfd_close (tpi);
+	  return false;
+	}
+
+      if (bfd_bread (int_buf, sizeof (int_buf), tpi) != sizeof (int_buf))
+	{
+	  free (types);
+	  bfd_close (tpi);
+	  return false;
+	}
+
+      types[i].kind = bfd_getl16 (int_buf);
+
+      if (bfd_seek (tpi, length - sizeof (uint16_t), SEEK_CUR) != 0)
+	{
+	  free (types);
+	  bfd_close (tpi);
+	  return false;
+	}
+    }
+
+  for (unsigned int i = 0; i < num_types; i++)
+    {
+      switch (types[i].kind)
+	{
+	case LF_ENUM:
+	  pdb_add_enum (objfile, builder, tpi, types, i, first_type, last_type);
+	  break;
+
+	default:
+	  break;
+	}
+    }
+
+  free (types);
+
+  bfd_close (tpi);
+
+  return true;
+}
+
+static void
+pdb_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
+{
+  buildsym_compunit builder(objfile, "", nullptr, language_c, 0);
+
+  if (!pdb_read_tpi_stream (objfile, builder))
+    complaint (_("Failed to read types stream"));
+
+  builder.end_compunit_symtab (0);
+}
+
+static void
+pdb_symfile_finish (struct objfile *objfile)
+{
+}
+
+static const struct sym_fns pdb_sym_fns =
+{
+  pdb_new_init,			/* sym_new_init: init anything gbl to
+				   entire symtab */
+  pdb_symfile_init,		/* sym_init: read initial info, setup
+				   for sym_read() */
+  pdb_symfile_read,		/* sym_read: read a symbol file into
+				   symtab */
+  pdb_symfile_finish,		/* sym_finish: finished with file,
+				   cleanup */
+  default_symfile_offsets,	/* sym_offsets: xlate external to
+				   internal form */
+  default_symfile_segments,	/* sym_segments: Get segment
+				   information from a file */
+  NULL,                         /* sym_read_linetable  */
+
+  default_symfile_relocate,	/* sym_relocate: Relocate a debug
+				   section.  */
+  NULL,				/* sym_probe_fns */
+};
+
+void _initialize_pdbread ();
+void
+_initialize_pdbread ()
+{
+  add_symtab_fns (bfd_target_pdb_flavour, &pdb_sym_fns);
+}
diff --git a/gdb/pdbread.h b/gdb/pdbread.h
new file mode 100644
index 00000000000..dd5982ed9d8
--- /dev/null
+++ b/gdb/pdbread.h
@@ -0,0 +1,105 @@
+/* Include file for PDB debugging format support.
+   Copyright (C) 2023 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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/>.  */
+
+/* Header files referred to below can be found in Microsoft's PDB
+   repository: https://github.com/microsoft/microsoft-pdb.  */
+
+#ifndef PDBREAD_H
+#define PDBREAD_H
+
+#define PDB_TPI_STREAM_NUM	2
+
+#define LF_FIELDLIST		0x1203
+#define LF_INDEX		0x1404
+#define LF_ENUMERATE		0x1502
+#define LF_ENUM			0x1507
+#define LF_CHAR			0x8000
+#define LF_SHORT		0x8001
+#define LF_USHORT		0x8002
+#define LF_LONG			0x8003
+#define LF_ULONG		0x8004
+#define LF_QUADWORD		0x8009
+#define LF_UQUADWORD		0x800a
+
+#define T_CHAR			0x0010
+#define T_UCHAR			0x0020
+#define T_INT1			0x0068
+#define T_UINT1			0x0069
+#define T_SHORT			0x0011
+#define T_USHORT		0x0021
+#define T_INT2			0x0072
+#define T_UINT2			0x0073
+#define T_LONG			0x0012
+#define T_ULONG			0x0022
+#define T_INT4			0x0074
+#define T_UINT4			0x0075
+#define T_QUAD			0x0013
+#define T_UQUAD			0x0023
+#define T_INT8			0x0076
+#define T_UINT8			0x0077
+
+/* from bitfield structure CV_prop_t in cvinfo.h */
+#define CV_PROP_FORWARD_REF	0x80
+
+/* HDR in tpi.h */
+struct pdb_tpi_stream_header
+{
+  uint32_t version;
+  uint32_t header_size;
+  uint32_t type_index_begin;
+  uint32_t type_index_end;
+  uint32_t type_record_bytes;
+  uint16_t hash_stream_index;
+  uint16_t hash_aux_stream_index;
+  uint32_t hash_key_size;
+  uint32_t num_hash_buckets;
+  uint32_t hash_value_buffer_offset;
+  uint32_t hash_value_buffer_length;
+  uint32_t index_offset_buffer_offset;
+  uint32_t index_offset_buffer_length;
+  uint32_t hash_adj_buffer_offset;
+  uint32_t hash_adj_buffer_length;
+};
+
+#define TPI_STREAM_VERSION_80		20040203
+
+/* lfEnum in cvinfo.h */
+struct lf_enum
+{
+  uint16_t kind;
+  uint16_t num_elements;
+  uint16_t properties;
+  uint32_t underlying_type;
+  uint32_t field_list;
+} ATTRIBUTE_PACKED;
+
+/* lfEnumerate in cvinfo.h */
+struct lf_enumerate
+{
+  uint16_t attributes;
+  uint16_t value;
+} ATTRIBUTE_PACKED;
+
+/* lfIndex in cvinfo.h */
+struct lf_index
+{
+  uint16_t padding;
+  uint32_t index;
+} ATTRIBUTE_PACKED;
+
+#endif /* PDBREAD_H */
diff --git a/gdb/testsuite/gdb.pdb/enum.exp b/gdb/testsuite/gdb.pdb/enum.exp
new file mode 100644
index 00000000000..ab2e2cff3b8
--- /dev/null
+++ b/gdb/testsuite/gdb.pdb/enum.exp
@@ -0,0 +1,39 @@
+# Copyright 2023 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/>.
+
+if {![istarget i*86-*-mingw*]
+  && ![istarget x86_64-*-mingw*]} {
+    return 0
+}
+
+set obj [standard_output_file enum1.o]
+
+if {[target_assemble ${srcdir}/${subdir}/enum1.s ${obj} ""] != ""} then {
+  untested "failed to assemble"
+  return -1
+}
+
+set exe [standard_output_file enum1.exe]
+set pdb [standard_output_file enum1.pdb]
+
+if {[target_link ${obj} ${exe} "--pdb=${pdb}"] != "" } then {
+  untested "failed to link"
+  return -1
+}
+
+clean_restart ${pdb}
+
+gdb_test "ptype enum enum1" "type = enum enum1 \{red, green, blue = -1, yellow = 32768, purple = 4294967296\}.*" "ptype enum enum1"
+gdb_test "ptype enum enum2" "type = enum enum2 \{foo = 1, bar = 1, baz, xyzzy, plugh = 5\}.*" "ptype enum enum2"
diff --git a/gdb/testsuite/gdb.pdb/enum1.s b/gdb/testsuite/gdb.pdb/enum1.s
new file mode 100644
index 00000000000..4d4b2ed4f6a
--- /dev/null
+++ b/gdb/testsuite/gdb.pdb/enum1.s
@@ -0,0 +1,116 @@
+.equ CV_SIGNATURE_C13, 4
+
+.equ T_UQUAD, 0x0023
+
+.equ LF_FIELDLIST, 0x1203
+.equ LF_INDEX, 0x1404
+.equ LF_ENUMERATE, 0x1502
+.equ LF_ENUM, 0x1507
+
+.equ LF_USHORT, 0x8002
+.equ LF_LONG, 0x8003
+.equ LF_UQUADWORD, 0x800a
+
+.section ".debug$T", "rn"
+
+.long CV_SIGNATURE_C13
+
+# Type 1000, field list for enum enum1 (red = 0, green = 1, blue = -1, yellow = 0x8000, purple = 0x100000000)
+.fieldlist1:
+.short .enum1 - .fieldlist1 - 2
+.short LF_FIELDLIST
+.short LF_ENUMERATE
+.short 3 # public
+.short 0 # value
+.asciz "red"
+.byte 0xf2 # padding
+.byte 0xf1 # padding
+.short LF_ENUMERATE
+.short 3 # public
+.short 1 # value
+.asciz "green"
+.short LF_ENUMERATE
+.short 3 # public
+.short LF_LONG
+.long 0xffffffff # value
+.asciz "blue"
+.byte 0xf1 # padding
+.short LF_ENUMERATE
+.short 3 # public
+.short LF_USHORT
+.short 0x8000 # value
+.asciz "yellow"
+.byte 0xf1 # padding
+.short LF_ENUMERATE
+.short 3 # public
+.short LF_UQUADWORD
+.quad 0x100000000 # value
+.asciz "purple"
+.byte 0xf3 # padding
+.byte 0xf2 # padding
+.byte 0xf1 # padding
+
+# Type 1001, enum enum1
+.enum1:
+.short .fieldlist2 - .enum1 - 2
+.short LF_ENUM
+.short 3 # no. elements
+.short 0 # property
+.long T_UQUAD # underlying type
+.long 0x1000 # field list
+.asciz "enum1"
+.byte 0xf2 # padding
+.byte 0xf1 # padding
+
+# Type 1002, continuation field list for enum enum2 (xyzzy = 3, plugh = 5)
+.fieldlist2:
+.short .fieldlist3 - .fieldlist2 - 2
+.short LF_FIELDLIST
+.short LF_ENUMERATE
+.short 3 # public
+.short 3 # value
+.asciz "xyzzy"
+.short LF_ENUMERATE
+.short 3 # public
+.short 5 # value
+.asciz "plugh"
+
+# Type 1003, field list for enum enum2 (foo = 1, bar = 1, baz = 2, ...)
+.fieldlist3:
+.short .enum2 - .fieldlist3 - 2
+.short LF_FIELDLIST
+.short LF_ENUMERATE
+.short 3 # public
+.short 1 # value
+.asciz "foo"
+.byte 0xf2 # padding
+.byte 0xf1 # padding
+.short LF_ENUMERATE
+.short 3 # public
+.short 1 # value
+.asciz "bar"
+.byte 0xf2 # padding
+.byte 0xf1 # padding
+.short LF_ENUMERATE
+.short 3 # public
+.short 2 # value
+.asciz "baz"
+.byte 0xf2 # padding
+.byte 0xf1 # padding
+.short LF_INDEX
+.short 0 # padding
+.long 0x1002
+
+# Type 1004, enum enum2
+.enum2:
+.short .types_end - .enum2 - 2
+.short LF_ENUM
+.short 5 # no. elements
+.short 0 # property
+.long T_UINT4 # underlying type
+.long 0x1003 # field list
+.asciz "enum2"
+.byte 0xf2 # padding
+.byte 0xf1 # padding
+
+.types_end:
-- 
2.39.2

Re: [PATCH 1/2] pdb: Allow loading by gdb

[Alan Modra]

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.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 2/2] gdb: Allow reading of enum definitions in PDB files

[Tom Tromey]

>>>>> "Mark" == Mark Harmstone <mark@harmstone.com> writes:

Mark> Adds a new file pdbread.c, which parses LF_ENUM records in PDB files.

Hi.  Thank you for the patch.  I'm happy to see this happening.

Normally, gdb patches should be sent to the gdb-patches list.  It's fine
IMO to just CC one like this, in the middle of a series, to gdb-patches.

I don't know anything about pdb, so I won't comment much on that.

Mark> --- a/gdb/configure.tgt
Mark> +++ b/gdb/configure.tgt
Mark> @@ -335,7 +335,7 @@ i[34567]86-*-cygwin*)
Mark>  	;;
Mark>  i[34567]86-*-mingw32*)
Mark>  	# Target: Intel 386 running win32
Mark> -	gdb_target_obs="i386-windows-tdep.o windows-tdep.o"
Mark> +	gdb_target_obs="i386-windows-tdep.o windows-tdep.o pdbread.o"

I don't think this should be handled here.

Object and debuginfo readers can either be compiled in unconditionally,
or by seeing whether any necessary support is present in BFD.  For the
latter, see how elfread.o is handled in gdb/configure.ac.

Mark> +/* Return the size in bytes of a PDB builtin type.  */
Mark> +static ULONGEST
Mark> +pdb_builtin_type_length (uint32_t t)
Mark> +{

Probably this can be removed in favor.  It's only used here:

+  t->set_target_type (pdb_builtin_type (builtin_type (objfile),
+					bfd_getl32 (&en.underlying_type)));
+  t->set_length (pdb_builtin_type_length (bfd_getl32 (&en.underlying_type)));

but this will do something weird if the builtin type and
pdb_builtin_type_length disagree about the size.  Instead you can do
something like:

   struct type *targ_type = pdb_builtin_type (...);
   t->set_target_type (targ_type);
   t->set_length (targ_type->length ());

Mark> +/* Some integers, such as enum values, get stored as an extended value if
Mark> +   they're too large to fit into two bytes.  The two bytes that would normally
Mark> +   be the value are instead a type indicator, and the actual value follows.  */
Mark> +static bool
Mark> +pdb_read_extended_value (uint16_t kind, char **ptr, uint16_t *length,
Mark> +			 LONGEST *ret)

gdb normally puts 'const' where it makes sense, so for instance 'const char **ptr'.
This requires a change in the caller but I was going to request that anyway.

Mark> +  buf = (char *) xmalloc (length);
Mark> +
Mark> +  if (bfd_bread (buf, length, tpi) != length)
Mark> +    goto end;

gdb generally uses RAII now for cleanups.  'buf' could either be a
unique_xmalloc_ptr<char>, or a gdb::char_vector (or even byte_vector
depending on if you want to use gdb_byte).  This will ensure proper
cleanup even in the face of future refactorings, and instead of 'goto end',
the code can just 'return'.

Mark> +	  /* Round to 4-byte boundary.  */
Mark> +	  if ((ptr - buf + 2) % 4)
Mark> +	    {
Mark> +	      if (length < 4)
Mark> +		goto end;
Mark> +
Mark> +	      length -= 4 - ((ptr - buf + 2) % 4);
Mark> +	      ptr += 4 - ((ptr - buf + 2) % 4);

I thought gdb had some macro for this but I can't find it now.

Mark> +	case LF_INDEX: {
Mark> +	  struct lf_index *ind;
Mark> +	  uint32_t fl_type;
Mark> +
Mark> +	  if (length < sizeof (*ind))
Mark> +	    goto end;
Mark> +
Mark> +	  ind = (struct lf_index *) ptr;

Not sure if this is really guaranteed to have correct alignment; if not
it could just be memcpy'd out.

Mark> +  name_len = length - sizeof (en);
Mark> +  name = (char * ) xmalloc (name_len + 1);

Either unique_xmalloc_ptr<char> or std::string here.

Mark> +  t = type_allocator (objfile).new_type ();
Mark> +  t->set_code (TYPE_CODE_ENUM);
Mark> +  t->set_name (name);

This will leak 'name'.

In gdb, types are allocated on an obstack; in this case, the objfile
obstack.  When the objfile is destroyed, the obstack is also freed --
but this does not run any destructors or free any data associated with
objects on the obstack.

Instead, if a string must be preserved, it should be copied to the
objfile obstack.  If the string is likely to be duplicated in the
objfile (if it might be needed multiple times), then objfile::intern can
be used.

Mark> +  num_types = last_type - first_type + 1;
Mark> +  types = (struct pdb_type *) xmalloc (sizeof (*types) * num_types);

Use std::vector here.

Mark> +
Mark> +  for (unsigned int i = 0; i < num_types; i++)

You can use C++ "foreach".

Mark> +	  bfd_close (tpi);

gdb has a complicated BFD reference-counting setup, but that might be
overkill for this use.  However, it's not difficult to make a deleter
type that wraps bfd_close, so that explicit closes on error paths aren't
needed.  This would be more idiomatic in gdb.  If you look for typedefs
of unique_ptr, you'll find some of these.

Mark> +static void
Mark> +pdb_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
Mark> +{
Mark> +  buildsym_compunit builder(objfile, "", nullptr, language_c, 0);

Assuming this is the first of several PDB-reading patches, it may be
worth considering the overall approach.

Other debug info readers in gdb work in two passes.  First they make
some kind of "quick" index.  Then, when a symbol lookup finds a match,
the full debug info for a given compilation unit is read in.

The code here only does the second step.  I don't know PDB, so maybe
that's the only way it can be done?

In gdb, the symbols are normally associated with a given compilation
unit.  This isn't done in this patch... I'm wondering if that's possible
to do?

Mark> +if {![istarget i*86-*-mingw*]
Mark> +  && ![istarget x86_64-*-mingw*]} {
Mark> +    return 0

Probably can use 'require {is_any_target ...}'

Mark> +gdb_test "ptype enum enum1" "type = enum enum1 \{red, green, blue = -1, yellow = 32768, purple = 4294967296\}.*" "ptype enum enum1"

Normally we'd split the lines so they fit in 80 columns.

Tom

Re: [PATCH 1/2] pdb: Allow loading by gdb

[Mark Harmstone]

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

Re: [PATCH 1/2] pdb: Allow loading by gdb

[Alan Modra]

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.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 1/2] pdb: Allow loading by gdb

[Tom Tromey]

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