Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [rfc] Auto-detect MIPS 32-bit and 64-bit register packets
@ 2006-11-09 21:08 Daniel Jacobowitz
  2006-11-28 22:17 ` Daniel Jacobowitz
  0 siblings, 1 reply; 3+ messages in thread
From: Daniel Jacobowitz @ 2006-11-09 21:08 UTC (permalink / raw)
  To: gdb-patches

Similar to the last patch, but for MIPS64.

This is even more useful than the amd64 patch.  If you have an i386 or amd64
ELF file, it's obvious which register layout to expect.  If you have a MIPS
binary, it may be considerably less clear.  For one thing, mips_isa_regsize
is currently set to return 64-bit for binaries based on the BFD architecture
rather than the ELF class; so o32 binaries optimized for a 64-bit processor
cause GDB to expect 64-bit registers!  For another, I have a gdbserver port
for MIPS64 coming up next; if GDB can handle receiving the 64-bit register
layout from it, then a single gdbserver can be used (in most cases) for
all three MIPS ABIs.

For MIPS the problem addressed by this patch is encountered considerably
more frequently than the amd64 equivalent.  It got to be a FAQ on the
linux-mips list at one point, for using kgdb - kernels are often an example
of a 32-bit binary optimized for a particular 64-bit processor, when a
64-bit kernel isn't justified.

I'd like to thank MontaVista for funding this bit of work, which I hope
will be useful to MIPS GNU/Linux developers!  And hopefully stop me
having to explain the manual "set architecture" workaround on linux-mips,
too.

Tested manually on mips-linux and mips64-linux with various combinations
of stubs.  Depends on the earlier patches in this group, of course - I'll
wait to commit this until I see how those are received.

-- 
Daniel Jacobowitz
CodeSourcery

2006-11-09  Daniel Jacobowitz  <dan@codesourcery.com>

	* Makefile.in (mips-tdep.o, target-descriptions.o): Update.
	* target-descriptions.c (struct property): New.
	(struct target_desc): Add properties member.
	(tdesc_property, set_tdesc_property): New.
	* target-descriptions.h (tdesc_property, set_tdesc_property):
	Declare.
	* mips-tdep.c (PROPERTY_GP32, PROPERTY_GP64): New constants.
	(struct gdbarch_tdep): Add register_size_valid_p and register_size.
	(mips_isa_regsize): Use them.
	(mips_register_g_packet_guesses): New.
	(mips_gdbarch_init): Call it.  If a target description is supplied,
	check for internal properties.

---
 gdb/Makefile.in           |    4 +-
 gdb/mips-tdep.c           |   71 ++++++++++++++++++++++++++++++++++++++++++++++
 gdb/target-descriptions.c |   48 +++++++++++++++++++++++++++++++
 gdb/target-descriptions.h |    8 +++++
 4 files changed, 129 insertions(+), 2 deletions(-)

Index: src/gdb/Makefile.in
===================================================================
--- src.orig/gdb/Makefile.in	2006-11-09 10:41:32.000000000 -0500
+++ src/gdb/Makefile.in	2006-11-09 10:42:40.000000000 -0500
@@ -2377,7 +2377,7 @@ mips-tdep.o: mips-tdep.c $(defs_h) $(gdb
 	$(block_h) $(reggroups_h) $(opcode_mips_h) $(elf_mips_h) \
 	$(elf_bfd_h) $(symcat_h) $(sim_regno_h) $(dis_asm_h) \
 	$(frame_unwind_h) $(frame_base_h) $(trad_frame_h) $(infcall_h) \
-	$(floatformat_h)
+	$(floatformat_h) $(remote_h) $(target_descriptions_h)
 mipsv4-nat.o: mipsv4-nat.c $(defs_h) $(inferior_h) $(gdbcore_h) $(target_h) \
 	$(regcache_h) $(gregset_h)
 memory-map.o: memory-map.c $(defs_h) $(memory_map_h) $(xml_support_h) \
@@ -2765,7 +2765,7 @@ target.o: target.c $(defs_h) $(gdb_strin
 	$(gdb_wait_h) $(dcache_h) $(regcache_h) $(gdb_assert_h) $(gdbcore_h) \
 	$(exceptions_h) $(target_descriptions_h)
 target-descriptions.o: target-descriptions.c $(defs_h) $(arch_utils_h) \
-	$(target_h) $(target_descriptions_h) $(gdb_assert_h)
+	$(target_h) $(target_descriptions_h) $(vec_h) $(gdb_assert_h)
 target-memory.o: target-memory.c $(defs_h) $(vec_h) $(target_h) \
 	$(memory_map_h) $(gdb_assert_h)
 thread.o: thread.c $(defs_h) $(symtab_h) $(frame_h) $(inferior_h) \
Index: src/gdb/target-descriptions.c
===================================================================
--- src.orig/gdb/target-descriptions.c	2006-11-09 10:41:24.000000000 -0500
+++ src/gdb/target-descriptions.c	2006-11-09 10:41:46.000000000 -0500
@@ -26,15 +26,26 @@
 #include "arch-utils.h"
 #include "target.h"
 #include "target-descriptions.h"
+#include "vec.h"
 
 #include "gdb_assert.h"
 
 /* Types.  */
 
+typedef struct property
+{
+  const char *key;
+  const char *value;
+} property_s;
+DEF_VEC_O(property_s);
+
 struct target_desc
 {
   /* The architecture reported by the target, if any.  */
   const struct bfd_arch_info *arch;
+
+  /* Any architecture-specific properties specified by the target.  */
+  VEC(property_s) *properties;
 };
 
 /* Global state.  These variables are associated with the current
@@ -133,6 +144,23 @@ tdesc_architecture (const struct target_
   return target_desc->arch;
 }
 
+/* Return the string value of a property named KEY, or NULL if the
+   property was not specified.  */
+
+const char *
+tdesc_property (const struct target_desc *target_desc, const char *key)
+{
+  struct property *prop;
+  int ix;
+
+  for (ix = 0; VEC_iterate (property_s, target_desc->properties, ix, prop);
+       ix++)
+    if (strcmp (prop->key, key) == 0)
+      return prop->value;
+
+  return NULL;
+}
+
 /* Methods for constructing a target description.  */
 
 struct target_desc *
@@ -147,3 +175,23 @@ set_tdesc_architecture (struct target_de
 {
   target_desc->arch = arch;
 }
+
+void
+set_tdesc_property (struct target_desc *target_desc,
+		    const char *key, const char *value)
+{
+  struct property *prop, new_prop;
+  int ix;
+
+  gdb_assert (key != NULL && value != NULL);
+
+  for (ix = 0; VEC_iterate (property_s, target_desc->properties, ix, prop);
+       ix++)
+    if (strcmp (prop->key, key) == 0)
+      internal_error (__FILE__, __LINE__,
+		      _("Attempted to add duplicate property \"%s\""), key);
+
+  new_prop.key = key;
+  new_prop.value = value;
+  VEC_safe_push (property_s, target_desc->properties, &new_prop);
+}
Index: src/gdb/target-descriptions.h
===================================================================
--- src.orig/gdb/target-descriptions.h	2006-11-09 10:41:24.000000000 -0500
+++ src/gdb/target-descriptions.h	2006-11-09 10:41:46.000000000 -0500
@@ -51,10 +51,18 @@ const struct target_desc *target_current
 const struct bfd_arch_info *tdesc_architecture
   (const struct target_desc *);
 
+/* Return the string value of a property named KEY, or NULL if the
+   property was not specified.  */
+
+const char *tdesc_property (const struct target_desc *,
+			    const char *key);
+
 /* Methods for constructing a target description.  */
 
 struct target_desc *allocate_target_description (void);
 void set_tdesc_architecture (struct target_desc *,
 			     const struct bfd_arch_info *);
+void set_tdesc_property (struct target_desc *,
+			 const char *key, const char *value);
 
 #endif /* TARGET_DESCRIPTIONS_H */
Index: src/gdb/mips-tdep.c
===================================================================
--- src.orig/gdb/mips-tdep.c	2006-11-09 10:34:47.000000000 -0500
+++ src/gdb/mips-tdep.c	2006-11-09 10:41:46.000000000 -0500
@@ -55,6 +55,8 @@
 #include "trad-frame.h"
 #include "infcall.h"
 #include "floatformat.h"
+#include "remote.h"
+#include "target-descriptions.h"
 
 static const struct objfile_data *mips_pdr_data;
 
@@ -119,6 +121,11 @@ static enum mips_fpu_type mips_fpu_type 
 
 static int mips_debug = 0;
 
+/* Properties (for struct target_desc) describing the g/G packet
+   layout.  */
+#define PROPERTY_GP32 "internal: transfers-32bit-registers"
+#define PROPERTY_GP64 "internal: transfers-64bit-registers"
+
 /* MIPS specific per-architecture information */
 struct gdbarch_tdep
 {
@@ -141,6 +148,13 @@ struct gdbarch_tdep
   const struct mips_regnum *regnum;
   /* Register names table for the current register set.  */
   const char **mips_processor_reg_names;
+
+  /* The size of register data available from the target, if known.
+     This doesn't quite obsolete the manual
+     mips64_transfers_32bit_regs_p, since that is documented to force
+     left alignment even for big endian (very strange).  */
+  int register_size_valid_p;
+  int register_size;
 };
 
 static int
@@ -245,6 +259,13 @@ mips_abi (struct gdbarch *gdbarch)
 int
 mips_isa_regsize (struct gdbarch *gdbarch)
 {
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
+  /* If we know how big the registers are, use that size.  */
+  if (tdep->register_size_valid_p)
+    return tdep->register_size;
+
+  /* Fall back to the previous behavior.  */
   return (gdbarch_bfd_arch_info (gdbarch)->bits_per_word
 	  / gdbarch_bfd_arch_info (gdbarch)->bits_per_byte);
 }
@@ -4711,6 +4732,37 @@ global_mips_abi (void)
   internal_error (__FILE__, __LINE__, _("unknown ABI string"));
 }
 
+static void
+mips_register_g_packet_guesses (struct gdbarch *gdbarch)
+{
+  static struct target_desc *tdesc_gp32, *tdesc_gp64;
+
+  if (tdesc_gp32 == NULL)
+    {
+      /* Create feature sets with the appropriate properties.  The values
+	 are not important.  */
+
+      tdesc_gp32 = allocate_target_description ();
+      set_tdesc_property (tdesc_gp32, PROPERTY_GP32, "");
+
+      tdesc_gp64 = allocate_target_description ();
+      set_tdesc_property (tdesc_gp64, PROPERTY_GP64, "");
+    }
+
+  /* If the size matches the set of 32-bit or 64-bit integer registers,
+     assume that's what we've got.  */
+  register_remote_g_packet_guess (gdbarch, 38 * 4, tdesc_gp32);
+  register_remote_g_packet_guess (gdbarch, 38 * 8, tdesc_gp64);
+
+  /* If the size matches the full set of registers GDB traditionally
+     knows about, including floating point, for either 32-bit or
+     64-bit, assume that's what we've got.  */
+  register_remote_g_packet_guess (gdbarch, 90 * 4, tdesc_gp32);
+  register_remote_g_packet_guess (gdbarch, 90 * 8, tdesc_gp64);
+
+  /* Otherwise we don't have a useful guess.  */
+}
+
 static struct gdbarch *
 mips_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 {
@@ -4885,6 +4937,23 @@ mips_gdbarch_init (struct gdbarch_info i
   tdep->found_abi = found_abi;
   tdep->mips_abi = mips_abi;
   tdep->mips_fpu_type = fpu_type;
+  tdep->register_size_valid_p = 0;
+  tdep->register_size = 0;
+
+  if (info.target_desc)
+    {
+      /* Some useful properties can be inferred from the target.  */
+      if (tdesc_property (info.target_desc, PROPERTY_GP32) != NULL)
+	{
+	  tdep->register_size_valid_p = 1;
+	  tdep->register_size = 4;
+	}
+      else if (tdesc_property (info.target_desc, PROPERTY_GP64) != NULL)
+	{
+	  tdep->register_size_valid_p = 1;
+	  tdep->register_size = 8;
+	}
+    }
 
   /* Initially set everything according to the default ABI/ISA.  */
   set_gdbarch_short_bit (gdbarch, 16);
@@ -5152,6 +5221,8 @@ mips_gdbarch_init (struct gdbarch_info i
 
   set_gdbarch_single_step_through_delay (gdbarch, mips_single_step_through_delay);
 
+  mips_register_g_packet_guesses (gdbarch);
+
   /* Hook in OS ABI-specific overrides, if they have been registered.  */
   gdbarch_init_osabi (info, gdbarch);
 


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [rfc] Auto-detect MIPS 32-bit and 64-bit register packets
  2006-11-09 21:08 [rfc] Auto-detect MIPS 32-bit and 64-bit register packets Daniel Jacobowitz
@ 2006-11-28 22:17 ` Daniel Jacobowitz
  2006-11-28 22:18   ` Daniel Jacobowitz
  0 siblings, 1 reply; 3+ messages in thread
From: Daniel Jacobowitz @ 2006-11-28 22:17 UTC (permalink / raw)
  To: gdb-patches

On Thu, Nov 09, 2006 at 04:08:38PM -0500, Daniel Jacobowitz wrote:
> This is even more useful than the amd64 patch.  If you have an i386 or amd64
> ELF file, it's obvious which register layout to expect.  If you have a MIPS
> binary, it may be considerably less clear.  For one thing, mips_isa_regsize
> is currently set to return 64-bit for binaries based on the BFD architecture
> rather than the ELF class; so o32 binaries optimized for a 64-bit processor
> cause GDB to expect 64-bit registers!  For another, I have a gdbserver port
> for MIPS64 coming up next; if GDB can handle receiving the 64-bit register
> layout from it, then a single gdbserver can be used (in most cases) for
> all three MIPS ABIs.

Accordingly, though I've dropped the amd64 patch, I've committed this
one.  I'll follow up with a NEWS entry for this and mips64 gdbserver
in a few minutes.

There are only two changes from the last version: some common bits
which were formerly part of the i386/amd64 patch are now part of this
one, and there is now a more sensible warning if you load an n32 binary
and connect to an o32 debugger, instead of an internal error.

-- 
Daniel Jacobowitz
CodeSourcery

2006-11-09  Daniel Jacobowitz  <dan@codesourcery.com>

	* Makefile.in (mips-tdep.o, target-descriptions.o): Update.
	* target-descriptions.c (struct property): New.
	(struct target_desc): Add properties member.
	(tdesc_property, set_tdesc_property): New.
	* target-descriptions.h (tdesc_property, set_tdesc_property):
	Declare.
	* mips-tdep.c (PROPERTY_GP32, PROPERTY_GP64): New constants.
	(struct gdbarch_tdep): Add register_size_valid_p and register_size.
	(mips_isa_regsize): Use them.
	(mips_register_g_packet_guesses): New.
	(mips_gdbarch_init): Call it.  If a target description is supplied,
	check for internal properties.

---
 gdb/Makefile.in           |    4 +-
 gdb/mips-tdep.c           |   71 ++++++++++++++++++++++++++++++++++++++++++++++
 gdb/target-descriptions.c |   48 +++++++++++++++++++++++++++++++
 gdb/target-descriptions.h |    8 +++++
 4 files changed, 129 insertions(+), 2 deletions(-)

Index: src/gdb/Makefile.in
===================================================================
--- src.orig/gdb/Makefile.in	2006-11-09 10:41:32.000000000 -0500
+++ src/gdb/Makefile.in	2006-11-09 10:42:40.000000000 -0500
@@ -2377,7 +2377,7 @@ mips-tdep.o: mips-tdep.c $(defs_h) $(gdb
 	$(block_h) $(reggroups_h) $(opcode_mips_h) $(elf_mips_h) \
 	$(elf_bfd_h) $(symcat_h) $(sim_regno_h) $(dis_asm_h) \
 	$(frame_unwind_h) $(frame_base_h) $(trad_frame_h) $(infcall_h) \
-	$(floatformat_h)
+	$(floatformat_h) $(remote_h) $(target_descriptions_h)
 mipsv4-nat.o: mipsv4-nat.c $(defs_h) $(inferior_h) $(gdbcore_h) $(target_h) \
 	$(regcache_h) $(gregset_h)
 memory-map.o: memory-map.c $(defs_h) $(memory_map_h) $(xml_support_h) \
@@ -2765,7 +2765,7 @@ target.o: target.c $(defs_h) $(gdb_strin
 	$(gdb_wait_h) $(dcache_h) $(regcache_h) $(gdb_assert_h) $(gdbcore_h) \
 	$(exceptions_h) $(target_descriptions_h)
 target-descriptions.o: target-descriptions.c $(defs_h) $(arch_utils_h) \
-	$(target_h) $(target_descriptions_h) $(gdb_assert_h)
+	$(target_h) $(target_descriptions_h) $(vec_h) $(gdb_assert_h)
 target-memory.o: target-memory.c $(defs_h) $(vec_h) $(target_h) \
 	$(memory_map_h) $(gdb_assert_h)
 thread.o: thread.c $(defs_h) $(symtab_h) $(frame_h) $(inferior_h) \
Index: src/gdb/target-descriptions.c
===================================================================
--- src.orig/gdb/target-descriptions.c	2006-11-09 10:41:24.000000000 -0500
+++ src/gdb/target-descriptions.c	2006-11-09 10:41:46.000000000 -0500
@@ -26,15 +26,26 @@
 #include "arch-utils.h"
 #include "target.h"
 #include "target-descriptions.h"
+#include "vec.h"
 
 #include "gdb_assert.h"
 
 /* Types.  */
 
+typedef struct property
+{
+  const char *key;
+  const char *value;
+} property_s;
+DEF_VEC_O(property_s);
+
 struct target_desc
 {
   /* The architecture reported by the target, if any.  */
   const struct bfd_arch_info *arch;
+
+  /* Any architecture-specific properties specified by the target.  */
+  VEC(property_s) *properties;
 };
 
 /* Global state.  These variables are associated with the current
@@ -133,6 +144,23 @@ tdesc_architecture (const struct target_
   return target_desc->arch;
 }
 
+/* Return the string value of a property named KEY, or NULL if the
+   property was not specified.  */
+
+const char *
+tdesc_property (const struct target_desc *target_desc, const char *key)
+{
+  struct property *prop;
+  int ix;
+
+  for (ix = 0; VEC_iterate (property_s, target_desc->properties, ix, prop);
+       ix++)
+    if (strcmp (prop->key, key) == 0)
+      return prop->value;
+
+  return NULL;
+}
+
 /* Methods for constructing a target description.  */
 
 struct target_desc *
@@ -147,3 +175,23 @@ set_tdesc_architecture (struct target_de
 {
   target_desc->arch = arch;
 }
+
+void
+set_tdesc_property (struct target_desc *target_desc,
+		    const char *key, const char *value)
+{
+  struct property *prop, new_prop;
+  int ix;
+
+  gdb_assert (key != NULL && value != NULL);
+
+  for (ix = 0; VEC_iterate (property_s, target_desc->properties, ix, prop);
+       ix++)
+    if (strcmp (prop->key, key) == 0)
+      internal_error (__FILE__, __LINE__,
+		      _("Attempted to add duplicate property \"%s\""), key);
+
+  new_prop.key = key;
+  new_prop.value = value;
+  VEC_safe_push (property_s, target_desc->properties, &new_prop);
+}
Index: src/gdb/target-descriptions.h
===================================================================
--- src.orig/gdb/target-descriptions.h	2006-11-09 10:41:24.000000000 -0500
+++ src/gdb/target-descriptions.h	2006-11-09 10:41:46.000000000 -0500
@@ -51,10 +51,18 @@ const struct target_desc *target_current
 const struct bfd_arch_info *tdesc_architecture
   (const struct target_desc *);
 
+/* Return the string value of a property named KEY, or NULL if the
+   property was not specified.  */
+
+const char *tdesc_property (const struct target_desc *,
+			    const char *key);
+
 /* Methods for constructing a target description.  */
 
 struct target_desc *allocate_target_description (void);
 void set_tdesc_architecture (struct target_desc *,
 			     const struct bfd_arch_info *);
+void set_tdesc_property (struct target_desc *,
+			 const char *key, const char *value);
 
 #endif /* TARGET_DESCRIPTIONS_H */
Index: src/gdb/mips-tdep.c
===================================================================
--- src.orig/gdb/mips-tdep.c	2006-11-09 10:34:47.000000000 -0500
+++ src/gdb/mips-tdep.c	2006-11-09 10:41:46.000000000 -0500
@@ -55,6 +55,8 @@
 #include "trad-frame.h"
 #include "infcall.h"
 #include "floatformat.h"
+#include "remote.h"
+#include "target-descriptions.h"
 
 static const struct objfile_data *mips_pdr_data;
 
@@ -119,6 +121,11 @@ static enum mips_fpu_type mips_fpu_type 
 
 static int mips_debug = 0;
 
+/* Properties (for struct target_desc) describing the g/G packet
+   layout.  */
+#define PROPERTY_GP32 "internal: transfers-32bit-registers"
+#define PROPERTY_GP64 "internal: transfers-64bit-registers"
+
 /* MIPS specific per-architecture information */
 struct gdbarch_tdep
 {
@@ -141,6 +148,13 @@ struct gdbarch_tdep
   const struct mips_regnum *regnum;
   /* Register names table for the current register set.  */
   const char **mips_processor_reg_names;
+
+  /* The size of register data available from the target, if known.
+     This doesn't quite obsolete the manual
+     mips64_transfers_32bit_regs_p, since that is documented to force
+     left alignment even for big endian (very strange).  */
+  int register_size_valid_p;
+  int register_size;
 };
 
 static int
@@ -245,6 +259,13 @@ mips_abi (struct gdbarch *gdbarch)
 int
 mips_isa_regsize (struct gdbarch *gdbarch)
 {
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
+  /* If we know how big the registers are, use that size.  */
+  if (tdep->register_size_valid_p)
+    return tdep->register_size;
+
+  /* Fall back to the previous behavior.  */
   return (gdbarch_bfd_arch_info (gdbarch)->bits_per_word
 	  / gdbarch_bfd_arch_info (gdbarch)->bits_per_byte);
 }
@@ -4711,6 +4732,37 @@ global_mips_abi (void)
   internal_error (__FILE__, __LINE__, _("unknown ABI string"));
 }
 
+static void
+mips_register_g_packet_guesses (struct gdbarch *gdbarch)
+{
+  static struct target_desc *tdesc_gp32, *tdesc_gp64;
+
+  if (tdesc_gp32 == NULL)
+    {
+      /* Create feature sets with the appropriate properties.  The values
+	 are not important.  */
+
+      tdesc_gp32 = allocate_target_description ();
+      set_tdesc_property (tdesc_gp32, PROPERTY_GP32, "");
+
+      tdesc_gp64 = allocate_target_description ();
+      set_tdesc_property (tdesc_gp64, PROPERTY_GP64, "");
+    }
+
+  /* If the size matches the set of 32-bit or 64-bit integer registers,
+     assume that's what we've got.  */
+  register_remote_g_packet_guess (gdbarch, 38 * 4, tdesc_gp32);
+  register_remote_g_packet_guess (gdbarch, 38 * 8, tdesc_gp64);
+
+  /* If the size matches the full set of registers GDB traditionally
+     knows about, including floating point, for either 32-bit or
+     64-bit, assume that's what we've got.  */
+  register_remote_g_packet_guess (gdbarch, 90 * 4, tdesc_gp32);
+  register_remote_g_packet_guess (gdbarch, 90 * 8, tdesc_gp64);
+
+  /* Otherwise we don't have a useful guess.  */
+}
+
 static struct gdbarch *
 mips_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 {
@@ -4885,6 +4937,23 @@ mips_gdbarch_init (struct gdbarch_info i
   tdep->found_abi = found_abi;
   tdep->mips_abi = mips_abi;
   tdep->mips_fpu_type = fpu_type;
+  tdep->register_size_valid_p = 0;
+  tdep->register_size = 0;
+
+  if (info.target_desc)
+    {
+      /* Some useful properties can be inferred from the target.  */
+      if (tdesc_property (info.target_desc, PROPERTY_GP32) != NULL)
+	{
+	  tdep->register_size_valid_p = 1;
+	  tdep->register_size = 4;
+	}
+      else if (tdesc_property (info.target_desc, PROPERTY_GP64) != NULL)
+	{
+	  tdep->register_size_valid_p = 1;
+	  tdep->register_size = 8;
+	}
+    }
 
   /* Initially set everything according to the default ABI/ISA.  */
   set_gdbarch_short_bit (gdbarch, 16);
@@ -5152,6 +5221,8 @@ mips_gdbarch_init (struct gdbarch_info i
 
   set_gdbarch_single_step_through_delay (gdbarch, mips_single_step_through_delay);
 
+  mips_register_g_packet_guesses (gdbarch);
+
   /* Hook in OS ABI-specific overrides, if they have been registered.  */
   gdbarch_init_osabi (info, gdbarch);
 


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [rfc] Auto-detect MIPS 32-bit and 64-bit register packets
  2006-11-28 22:17 ` Daniel Jacobowitz
@ 2006-11-28 22:18   ` Daniel Jacobowitz
  0 siblings, 0 replies; 3+ messages in thread
From: Daniel Jacobowitz @ 2006-11-28 22:18 UTC (permalink / raw)
  To: gdb-patches

On Tue, Nov 28, 2006 at 05:17:42PM -0500, Daniel Jacobowitz wrote:
> Accordingly, though I've dropped the amd64 patch, I've committed this
> one.  I'll follow up with a NEWS entry for this and mips64 gdbserver
> in a few minutes.
> 
> There are only two changes from the last version: some common bits
> which were formerly part of the i386/amd64 patch are now part of this
> one, and there is now a more sensible warning if you load an n32 binary
> and connect to an o32 debugger, instead of an internal error.

Oops.  Wrong attachment.  Here's the right one.

-- 
Daniel Jacobowitz
CodeSourcery

2006-11-28  Daniel Jacobowitz  <dan@codesourcery.com>

	* Makefile.in (mips-tdep.o, target-descriptions.o): Update.
	* target-descriptions.c (struct property): New.
	(struct target_desc): Add properties member.
	(tdesc_property, set_tdesc_property): New.
	* target-descriptions.h (tdesc_property, set_tdesc_property):
	Declare.
	* mips-tdep.c (PROPERTY_GP32, PROPERTY_GP64): New constants.
	(struct gdbarch_tdep): Add register_size_valid_p and register_size.
	(mips_isa_regsize): Use them.
	(mips_register_g_packet_guesses): New.
	(mips_gdbarch_init): Call it.  If a target description is supplied,
	check for internal properties.  Check for register size mismatches.
	* remote.c (send_g_packet, process_g_packet): New functions, split
	out from fetch_registers_using_g.
	(fetch_registers_using_g): Use them.
	(struct remote_g_packet_guess, remote_g_packet_guess_s)
	(struct remote_g_packet_data, remote_g_packet_data_handle)
	(remote_g_packet_data_init, register_remote_g_packet_guess)
	(remote_read_description): New.
	(init_remote_ops, init_remote_async_ops): Set to_read_description.
	(_initialize_remote): Register remote_g_packet_data_handle.
	* remote.h (register_remote_g_packet_guess): Declare.

---
 gdb/Makefile.in           |    4 -
 gdb/mips-tdep.c           |   81 +++++++++++++++++++++++++++
 gdb/remote.c              |  137 ++++++++++++++++++++++++++++++++++++++++------
 gdb/remote.h              |    5 +
 gdb/target-descriptions.c |   47 +++++++++++++++
 gdb/target-descriptions.h |    9 +++
 6 files changed, 264 insertions(+), 19 deletions(-)

Index: src/gdb/Makefile.in
===================================================================
--- src.orig/gdb/Makefile.in	2006-11-28 13:56:38.000000000 -0500
+++ src/gdb/Makefile.in	2006-11-28 15:49:05.000000000 -0500
@@ -2380,7 +2380,7 @@ mips-tdep.o: mips-tdep.c $(defs_h) $(gdb
 	$(block_h) $(reggroups_h) $(opcode_mips_h) $(elf_mips_h) \
 	$(elf_bfd_h) $(symcat_h) $(sim_regno_h) $(dis_asm_h) \
 	$(frame_unwind_h) $(frame_base_h) $(trad_frame_h) $(infcall_h) \
-	$(floatformat_h)
+	$(floatformat_h) $(remote_h) $(target_descriptions_h)
 mipsv4-nat.o: mipsv4-nat.c $(defs_h) $(inferior_h) $(gdbcore_h) $(target_h) \
 	$(regcache_h) $(gregset_h)
 memory-map.o: memory-map.c $(defs_h) $(memory_map_h) $(xml_support_h) \
@@ -2777,7 +2777,7 @@ target.o: target.c $(defs_h) $(gdb_strin
 	$(gdb_wait_h) $(dcache_h) $(regcache_h) $(gdb_assert_h) $(gdbcore_h) \
 	$(exceptions_h) $(target_descriptions_h)
 target-descriptions.o: target-descriptions.c $(defs_h) $(arch_utils_h) \
-	$(target_h) $(target_descriptions_h) $(gdb_assert_h)
+	$(target_h) $(target_descriptions_h) $(vec_h) $(gdb_assert_h)
 target-memory.o: target-memory.c $(defs_h) $(vec_h) $(target_h) \
 	$(memory_map_h) $(gdb_assert_h)
 thread.o: thread.c $(defs_h) $(symtab_h) $(frame_h) $(inferior_h) \
Index: src/gdb/mips-tdep.c
===================================================================
--- src.orig/gdb/mips-tdep.c	2006-11-28 13:56:36.000000000 -0500
+++ src/gdb/mips-tdep.c	2006-11-28 16:40:48.000000000 -0500
@@ -55,6 +55,8 @@
 #include "trad-frame.h"
 #include "infcall.h"
 #include "floatformat.h"
+#include "remote.h"
+#include "target-descriptions.h"
 
 static const struct objfile_data *mips_pdr_data;
 
@@ -119,6 +121,11 @@ static enum mips_fpu_type mips_fpu_type 
 
 static int mips_debug = 0;
 
+/* Properties (for struct target_desc) describing the g/G packet
+   layout.  */
+#define PROPERTY_GP32 "internal: transfers-32bit-registers"
+#define PROPERTY_GP64 "internal: transfers-64bit-registers"
+
 /* MIPS specific per-architecture information */
 struct gdbarch_tdep
 {
@@ -141,6 +148,13 @@ struct gdbarch_tdep
   const struct mips_regnum *regnum;
   /* Register names table for the current register set.  */
   const char **mips_processor_reg_names;
+
+  /* The size of register data available from the target, if known.
+     This doesn't quite obsolete the manual
+     mips64_transfers_32bit_regs_p, since that is documented to force
+     left alignment even for big endian (very strange).  */
+  int register_size_valid_p;
+  int register_size;
 };
 
 static int
@@ -245,6 +259,13 @@ mips_abi (struct gdbarch *gdbarch)
 int
 mips_isa_regsize (struct gdbarch *gdbarch)
 {
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
+  /* If we know how big the registers are, use that size.  */
+  if (tdep->register_size_valid_p)
+    return tdep->register_size;
+
+  /* Fall back to the previous behavior.  */
   return (gdbarch_bfd_arch_info (gdbarch)->bits_per_word
 	  / gdbarch_bfd_arch_info (gdbarch)->bits_per_byte);
 }
@@ -4711,6 +4732,37 @@ global_mips_abi (void)
   internal_error (__FILE__, __LINE__, _("unknown ABI string"));
 }
 
+static void
+mips_register_g_packet_guesses (struct gdbarch *gdbarch)
+{
+  static struct target_desc *tdesc_gp32, *tdesc_gp64;
+
+  if (tdesc_gp32 == NULL)
+    {
+      /* Create feature sets with the appropriate properties.  The values
+	 are not important.  */
+
+      tdesc_gp32 = allocate_target_description ();
+      set_tdesc_property (tdesc_gp32, PROPERTY_GP32, "");
+
+      tdesc_gp64 = allocate_target_description ();
+      set_tdesc_property (tdesc_gp64, PROPERTY_GP64, "");
+    }
+
+  /* If the size matches the set of 32-bit or 64-bit integer registers,
+     assume that's what we've got.  */
+  register_remote_g_packet_guess (gdbarch, 38 * 4, tdesc_gp32);
+  register_remote_g_packet_guess (gdbarch, 38 * 8, tdesc_gp64);
+
+  /* If the size matches the full set of registers GDB traditionally
+     knows about, including floating point, for either 32-bit or
+     64-bit, assume that's what we've got.  */
+  register_remote_g_packet_guess (gdbarch, 90 * 4, tdesc_gp32);
+  register_remote_g_packet_guess (gdbarch, 90 * 8, tdesc_gp64);
+
+  /* Otherwise we don't have a useful guess.  */
+}
+
 static struct gdbarch *
 mips_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 {
@@ -4855,6 +4907,16 @@ mips_gdbarch_init (struct gdbarch_info i
     fprintf_unfiltered (gdb_stdlog,
 			"mips_gdbarch_init: fpu_type = %d\n", fpu_type);
 
+  /* Check for blatant incompatibilities.  */
+
+  /* If we have only 32-bit registers, then we can't debug a 64-bit
+     ABI.  */
+  if (info.target_desc
+      && tdesc_property (info.target_desc, PROPERTY_GP32) != NULL
+      && mips_abi != MIPS_ABI_EABI32
+      && mips_abi != MIPS_ABI_O32)
+    return NULL;
+
   /* try to find a pre-existing architecture */
   for (arches = gdbarch_list_lookup_by_info (arches, &info);
        arches != NULL;
@@ -4885,6 +4947,23 @@ mips_gdbarch_init (struct gdbarch_info i
   tdep->found_abi = found_abi;
   tdep->mips_abi = mips_abi;
   tdep->mips_fpu_type = fpu_type;
+  tdep->register_size_valid_p = 0;
+  tdep->register_size = 0;
+
+  if (info.target_desc)
+    {
+      /* Some useful properties can be inferred from the target.  */
+      if (tdesc_property (info.target_desc, PROPERTY_GP32) != NULL)
+	{
+	  tdep->register_size_valid_p = 1;
+	  tdep->register_size = 4;
+	}
+      else if (tdesc_property (info.target_desc, PROPERTY_GP64) != NULL)
+	{
+	  tdep->register_size_valid_p = 1;
+	  tdep->register_size = 8;
+	}
+    }
 
   /* Initially set everything according to the default ABI/ISA.  */
   set_gdbarch_short_bit (gdbarch, 16);
@@ -5152,6 +5231,8 @@ mips_gdbarch_init (struct gdbarch_info i
 
   set_gdbarch_single_step_through_delay (gdbarch, mips_single_step_through_delay);
 
+  mips_register_g_packet_guesses (gdbarch);
+
   /* Hook in OS ABI-specific overrides, if they have been registered.  */
   gdbarch_init_osabi (info, gdbarch);
 
Index: src/gdb/remote.c
===================================================================
--- src.orig/gdb/remote.c	2006-11-28 13:56:38.000000000 -0500
+++ src/gdb/remote.c	2006-11-28 15:49:05.000000000 -0500
@@ -3535,11 +3535,10 @@ fetch_register_using_p (struct packet_re
 
 /* Fetch the registers included in the target's 'g' packet.  */
 
-static void
-fetch_registers_using_g (void)
+static int
+send_g_packet (void)
 {
   struct remote_state *rs = get_remote_state ();
-  struct remote_arch_state *rsa = get_remote_arch_state ();
   int i, buf_len;
   char *p;
   char *regs;
@@ -3547,11 +3546,41 @@ fetch_registers_using_g (void)
   sprintf (rs->buf, "g");
   remote_send (&rs->buf, &rs->buf_size);
 
+  /* We can get out of synch in various cases.  If the first character
+     in the buffer is not a hex character, assume that has happened
+     and try to fetch another packet to read.  */
+  while ((rs->buf[0] < '0' || rs->buf[0] > '9')
+	 && (rs->buf[0] < 'A' || rs->buf[0] > 'F')
+	 && (rs->buf[0] < 'a' || rs->buf[0] > 'f')
+	 && rs->buf[0] != 'x')	/* New: unavailable register value.  */
+    {
+      if (remote_debug)
+	fprintf_unfiltered (gdb_stdlog,
+			    "Bad register packet; fetching a new packet\n");
+      getpkt (&rs->buf, &rs->buf_size, 0);
+    }
+
   buf_len = strlen (rs->buf);
 
   /* Sanity check the received packet.  */
   if (buf_len % 2 != 0)
     error (_("Remote 'g' packet reply is of odd length: %s"), rs->buf);
+
+  return buf_len / 2;
+}
+
+static void
+process_g_packet (void)
+{
+  struct remote_state *rs = get_remote_state ();
+  struct remote_arch_state *rsa = get_remote_arch_state ();
+  int i, buf_len;
+  char *p;
+  char *regs;
+
+  buf_len = strlen (rs->buf);
+
+  /* Further sanity checks, with knowledge of the architecture.  */
   if (REGISTER_BYTES_OK_P () && !REGISTER_BYTES_OK (buf_len / 2))
     error (_("Remote 'g' packet reply is wrong length: %s"), rs->buf);
   if (buf_len > 2 * rsa->sizeof_g_packet)
@@ -3588,20 +3617,6 @@ fetch_registers_using_g (void)
   /* Unimplemented registers read as all bits zero.  */
   memset (regs, 0, rsa->sizeof_g_packet);
 
-  /* We can get out of synch in various cases.  If the first character
-     in the buffer is not a hex character, assume that has happened
-     and try to fetch another packet to read.  */
-  while ((rs->buf[0] < '0' || rs->buf[0] > '9')
-	 && (rs->buf[0] < 'A' || rs->buf[0] > 'F')
-	 && (rs->buf[0] < 'a' || rs->buf[0] > 'f')
-	 && rs->buf[0] != 'x')	/* New: unavailable register value.  */
-    {
-      if (remote_debug)
-	fprintf_unfiltered (gdb_stdlog,
-			    "Bad register packet; fetching a new packet\n");
-      getpkt (&rs->buf, &rs->buf_size, 0);
-    }
-
   /* Reply describes registers byte by byte, each byte encoded as two
      hex characters.  Suck them all up, then supply them to the
      register cacheing/storage mechanism.  */
@@ -3649,6 +3664,13 @@ fetch_registers_using_g (void)
 }
 
 static void
+fetch_registers_using_g (void)
+{
+  send_g_packet ();
+  process_g_packet ();
+}
+
+static void
 remote_fetch_registers (int regnum)
 {
   struct remote_state *rs = get_remote_state ();
@@ -6052,6 +6074,83 @@ remote_get_thread_local_address (ptid_t 
   return 0;
 }
 
+/* Support for inferring a target description based on the current
+   architecture and the size of a 'g' packet.  While the 'g' packet
+   can have any size (since optional registers can be left off the
+   end), some sizes are easily recognizable given knowledge of the
+   approximate architecture.  */
+
+struct remote_g_packet_guess
+{
+  int bytes;
+  const struct target_desc *tdesc;
+};
+typedef struct remote_g_packet_guess remote_g_packet_guess_s;
+DEF_VEC_O(remote_g_packet_guess_s);
+
+struct remote_g_packet_data
+{
+  VEC(remote_g_packet_guess_s) *guesses;
+};
+
+static struct gdbarch_data *remote_g_packet_data_handle;
+
+static void *
+remote_g_packet_data_init (struct obstack *obstack)
+{
+  return OBSTACK_ZALLOC (obstack, struct remote_g_packet_data);
+}
+
+void
+register_remote_g_packet_guess (struct gdbarch *gdbarch, int bytes,
+				const struct target_desc *tdesc)
+{
+  struct remote_g_packet_data *data
+    = gdbarch_data (gdbarch, remote_g_packet_data_handle);
+  struct remote_g_packet_guess new_guess, *guess;
+  int ix;
+
+  gdb_assert (tdesc != NULL);
+
+  for (ix = 0;
+       VEC_iterate (remote_g_packet_guess_s, data->guesses, ix, guess);
+       ix++)
+    if (guess->bytes == bytes)
+      internal_error (__FILE__, __LINE__,
+		      "Duplicate g packet description added for size %d",
+		      bytes);
+
+  new_guess.bytes = bytes;
+  new_guess.tdesc = tdesc;
+  VEC_safe_push (remote_g_packet_guess_s, data->guesses, &new_guess);
+}
+
+static const struct target_desc *
+remote_read_description (struct target_ops *target)
+{
+  struct remote_g_packet_data *data
+    = gdbarch_data (current_gdbarch, remote_g_packet_data_handle);
+
+  if (!VEC_empty (remote_g_packet_guess_s, data->guesses))
+    {
+      struct remote_g_packet_guess *guess;
+      int ix;
+      int bytes = send_g_packet ();
+
+      for (ix = 0;
+	   VEC_iterate (remote_g_packet_guess_s, data->guesses, ix, guess);
+	   ix++)
+	if (guess->bytes == bytes)
+	  return guess->tdesc;
+
+      /* We discard the g packet.  A minor optimization would be to
+	 hold on to it, and fill the register cache once we have selected
+	 an architecture, but it's too tricky to do safely.  */
+    }
+
+  return NULL;
+}
+
 static void
 init_remote_ops (void)
 {
@@ -6103,6 +6202,7 @@ Specify the serial device it is connecte
   remote_ops.to_memory_map = remote_memory_map;
   remote_ops.to_flash_erase = remote_flash_erase;
   remote_ops.to_flash_done = remote_flash_done;
+  remote_ops.to_read_description = remote_read_description;
 }
 
 /* Set up the extended remote vector by making a copy of the standard
@@ -6235,6 +6335,7 @@ Specify the serial device it is connecte
   remote_async_ops.to_memory_map = remote_memory_map;
   remote_async_ops.to_flash_erase = remote_flash_erase;
   remote_async_ops.to_flash_done = remote_flash_done;
+  remote_ops.to_read_description = remote_read_description;
 }
 
 /* Set up the async extended remote vector by making a copy of the standard
@@ -6326,6 +6427,8 @@ _initialize_remote (void)
   /* architecture specific data */
   remote_gdbarch_data_handle =
     gdbarch_data_register_post_init (init_remote_state);
+  remote_g_packet_data_handle =
+    gdbarch_data_register_pre_init (remote_g_packet_data_init);
 
   /* Old tacky stuff.  NOTE: This comes after the remote protocol so
      that the remote protocol has been initialized.  */
Index: src/gdb/remote.h
===================================================================
--- src.orig/gdb/remote.h	2006-11-28 13:56:36.000000000 -0500
+++ src/gdb/remote.h	2006-11-28 15:49:05.000000000 -0500
@@ -21,6 +21,8 @@
 #ifndef REMOTE_H
 #define REMOTE_H
 
+struct target_desc;
+
 /* FIXME?: move this interface down to tgt vector) */
 
 /* Read a packet from the remote machine, with error checking, and
@@ -63,4 +65,7 @@ extern int remote_read_bytes (CORE_ADDR 
 extern void (*deprecated_target_resume_hook) (void);
 extern void (*deprecated_target_wait_loop_hook) (void);
 
+void register_remote_g_packet_guess (struct gdbarch *gdbarch, int bytes,
+				     const struct target_desc *tdesc);
+
 #endif
Index: src/gdb/target-descriptions.c
===================================================================
--- src.orig/gdb/target-descriptions.c	2006-11-28 13:56:38.000000000 -0500
+++ src/gdb/target-descriptions.c	2006-11-28 15:49:05.000000000 -0500
@@ -26,13 +26,23 @@
 #include "arch-utils.h"
 #include "target.h"
 #include "target-descriptions.h"
+#include "vec.h"
 
 #include "gdb_assert.h"
 
 /* Types.  */
 
+typedef struct property
+{
+  const char *key;
+  const char *value;
+} property_s;
+DEF_VEC_O(property_s);
+
 struct target_desc
 {
+  /* Any architecture-specific properties specified by the target.  */
+  VEC(property_s) *properties;
 };
 
 /* Global state.  These variables are associated with the current
@@ -122,6 +132,23 @@ target_current_description (void)
   return NULL;
 }
 
+/* Return the string value of a property named KEY, or NULL if the
+   property was not specified.  */
+
+const char *
+tdesc_property (const struct target_desc *target_desc, const char *key)
+{
+  struct property *prop;
+  int ix;
+
+  for (ix = 0; VEC_iterate (property_s, target_desc->properties, ix, prop);
+       ix++)
+    if (strcmp (prop->key, key) == 0)
+      return prop->value;
+
+  return NULL;
+}
+
 /* Methods for constructing a target description.  */
 
 struct target_desc *
@@ -129,3 +156,23 @@ allocate_target_description (void)
 {
   return XZALLOC (struct target_desc);
 }
+
+void
+set_tdesc_property (struct target_desc *target_desc,
+		    const char *key, const char *value)
+{
+  struct property *prop, new_prop;
+  int ix;
+
+  gdb_assert (key != NULL && value != NULL);
+
+  for (ix = 0; VEC_iterate (property_s, target_desc->properties, ix, prop);
+       ix++)
+    if (strcmp (prop->key, key) == 0)
+      internal_error (__FILE__, __LINE__,
+		      _("Attempted to add duplicate property \"%s\""), key);
+
+  new_prop.key = key;
+  new_prop.value = value;
+  VEC_safe_push (property_s, target_desc->properties, &new_prop);
+}
Index: src/gdb/target-descriptions.h
===================================================================
--- src.orig/gdb/target-descriptions.h	2006-11-28 13:56:38.000000000 -0500
+++ src/gdb/target-descriptions.h	2006-11-28 15:49:05.000000000 -0500
@@ -45,8 +45,17 @@ const struct target_desc *target_current
 
 /* Accessors for target descriptions.  */
 
+/* Return the string value of a property named KEY, or NULL if the
+   property was not specified.  */
+
+const char *tdesc_property (const struct target_desc *,
+			    const char *key);
+
 /* Methods for constructing a target description.  */
 
 struct target_desc *allocate_target_description (void);
 
+void set_tdesc_property (struct target_desc *,
+			 const char *key, const char *value);
+
 #endif /* TARGET_DESCRIPTIONS_H */


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2006-11-28 22:18 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-11-09 21:08 [rfc] Auto-detect MIPS 32-bit and 64-bit register packets Daniel Jacobowitz
2006-11-28 22:17 ` Daniel Jacobowitz
2006-11-28 22:18   ` Daniel Jacobowitz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox