This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [rfc] Auto-detect MIPS 32-bit and 64-bit register packets


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 */


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]