Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [rfc] Autoselect x86_64 or i386 based on the remote g packet size
@ 2006-11-09 20:58 Daniel Jacobowitz
  2006-11-09 21:36 ` Mark Kettenis
  0 siblings, 1 reply; 4+ messages in thread
From: Daniel Jacobowitz @ 2006-11-09 20:58 UTC (permalink / raw)
  To: gdb-patches

The first useful thing to go on top of my previous infrastructure patches.
This patch is handy when using GDB without a binary; it will be even handier
if someone (I'll probably do this eventually) gets around to warning
when a binary and target have incompatible architectures.

Today:

This GDB was configured as "x86_64-linux-gnu".
(gdb) show architecture 
The target architecture is set automatically (currently i386)
(gdb) tar rem :1234
Remote debugging using :1234
Remote register badly formatted:
T0506:0000000000000000;07:f0e1ffffff7f0000;10:80baaaaaaa2a0000;
here: 0000000;07:f0e1ffffff7f0000;10:80baaaaaaa2a0000;

After the patch:

(gdb) show architecture 
The target architecture is set automatically (currently i386)
(gdb) tar rem :1234
Remote debugging using :1234
0x00002aaaaaaaba80 in ?? ()
(gdb) show architecture 
The target architecture is set automatically (currently i386:x86-64)

Personally, I think that's pretty cool.  I do this surprisingly
often, e.g. to kill an errant gdbserver; you have to connect to it
using the right architecture or you won't get far enough to send a
'k' packet.

The operating principle is that a target is free to send back a "short"
g packet, but some sizes are plausible and others are not.  So I
enhanced the i386 backend to register the sizes of likely register
sets: core registers, core + i387, core + i387 + SSE.  The Linux
backend adds the %orig_eax and %orig_rax pseudo-"registers" too
(which, if I were inventing them today, might be target objects like
the sparc WCOOKIE, but are currently in the g packet).

So if you have a GDB which defaults to i386-linux, and it connects to
an amd64 target and gets exactly the right number of bytes to be
a 64-bit register set, it'll assume that's what it's got.

There's deliberately no mechanism to say "more than X bytes must be amd64",
because I feel that's unsafe.  The current registered guesses are
high-confidence, since e.g the amd64 guesses would be quite odd sizes for
an i386 target to return.  For instance, the amd64 non-FP registers cut
off between fioff and foseg if interpreted as a 32-bit register set: not
very likely.

All comments welcome!  Tested on x86_64-pc-linux-gnu, using gdbserver for
the testsuite and by hand.

-- 
Daniel Jacobowitz
CodeSourcery

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

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

	* Makefile.in (amd64-linux-nat.o, i386-linux-tdep.o, i386-tdep.o):
	Update.
	* i386-tdep.c (tdesc_i386, tdesc_amd64): New variables.
	(i386_register_g_packet_guesses): New function.
	(i386_gdbarch_init): Call it.
	* amd64-linux-tdep.c (amd64_linux_init_abi): Call
	i386_linux_register_g_packet_guesses.
	* i386-linux-tdep.c (i386_linux_register_g_packet_guesses): New.
	(i386_linux_init_abi): Call it.
	* i386-linux-tdep.h (i386_linux_register_g_packet_guesses): New
	prototype.
	* i386-tdep.h (tdesc_i386, tdesc_amd64): Declare.
	* remote.h (register_remote_g_packet_guess): Declare.

---
 gdb/Makefile.in        |    8 +-
 gdb/amd64-linux-tdep.c |    3 +
 gdb/i386-linux-tdep.c  |   25 ++++++++
 gdb/i386-linux-tdep.h  |    4 +
 gdb/i386-tdep.c        |   48 +++++++++++++++++
 gdb/i386-tdep.h        |    6 ++
 gdb/remote.c           |  137 ++++++++++++++++++++++++++++++++++++++++++-------
 gdb/remote.h           |    5 +
 8 files changed, 214 insertions(+), 22 deletions(-)

Index: src/gdb/Makefile.in
===================================================================
--- src.orig/gdb/Makefile.in	2006-11-09 10:38:30.000000000 -0500
+++ src/gdb/Makefile.in	2006-11-09 10:40:16.000000000 -0500
@@ -1753,7 +1753,8 @@ amd64-linux-nat.o: amd64-linux-nat.c $(d
 	$(i386_linux_tdep_h) $(amd64_nat_h) $(target_h) $(amd64_linux_tdep_h)
 amd64-linux-tdep.o: amd64-linux-tdep.c $(defs_h) $(frame_h) $(gdbcore_h) \
 	$(regcache_h) $(osabi_h) $(symtab_h) $(gdb_string_h) $(amd64_tdep_h) \
-	$(solib_svr4_h) $(gdbtypes_h) $(reggroups_h) $(amd64_linux_tdep_h)
+	$(solib_svr4_h) $(gdbtypes_h) $(reggroups_h) $(amd64_linux_tdep_h) \
+	$(i386_linux_tdep_h)
 amd64-nat.o: amd64-nat.c $(defs_h) $(gdbarch_h) $(regcache_h) \
 	$(gdb_assert_h) $(gdb_string_h) $(i386_tdep_h) $(amd64_tdep_h)
 amd64nbsd-nat.o: amd64nbsd-nat.c $(defs_h) $(target_h) $(gdb_assert_h) \
@@ -2116,7 +2117,8 @@ i386-linux-nat.o: i386-linux-nat.c $(def
 i386-linux-tdep.o: i386-linux-tdep.c $(defs_h) $(gdbcore_h) $(frame_h) \
 	$(value_h) $(regcache_h) $(inferior_h) $(osabi_h) $(reggroups_h) \
 	$(dwarf2_frame_h) $(gdb_string_h) $(i386_tdep_h) \
-	$(i386_linux_tdep_h) $(glibc_tdep_h) $(solib_svr4_h)
+	$(i386_linux_tdep_h) $(glibc_tdep_h) $(solib_svr4_h) \
+	$(remote_h)
 i386-nat.o: i386-nat.c $(defs_h) $(breakpoint_h) $(command_h) $(gdbcmd_h)
 i386nbsd-nat.o: i386nbsd-nat.c $(defs_h) $(gdbcore_h) $(regcache_h) \
 	$(target_h) $(i386_tdep_h) $(i386bsd_nat_h) $(bsd_kvm_h)
@@ -2144,7 +2146,7 @@ i386-tdep.o: i386-tdep.c $(defs_h) $(arc
 	$(gdbcmd_h) $(gdbcore_h) $(objfiles_h) $(osabi_h) $(regcache_h) \
 	$(reggroups_h) $(regset_h) $(symfile_h) $(symtab_h) $(target_h) \
 	$(value_h) $(dis_asm_h) $(gdb_assert_h) $(gdb_string_h) \
-	$(i386_tdep_h) $(i387_tdep_h)
+	$(i386_tdep_h) $(i387_tdep_h) $(remote_h) $(target_descriptions_h)
 i386v4-nat.o: i386v4-nat.c $(defs_h) $(value_h) $(inferior_h) $(regcache_h) \
 	$(i386_tdep_h) $(i387_tdep_h) $(gregset_h)
 i386v-nat.o: i386v-nat.c $(defs_h) $(frame_h) $(inferior_h) $(language_h) \
Index: src/gdb/i386-tdep.c
===================================================================
--- src.orig/gdb/i386-tdep.c	2006-11-09 10:34:53.000000000 -0500
+++ src/gdb/i386-tdep.c	2006-11-09 10:39:26.000000000 -0500
@@ -44,6 +44,8 @@
 #include "target.h"
 #include "value.h"
 #include "dis-asm.h"
+#include "remote.h"
+#include "target-descriptions.h"
 
 #include "gdb_assert.h"
 #include "gdb_string.h"
@@ -2253,6 +2255,49 @@ i386_fetch_pointer_argument (struct fram
   return read_memory_unsigned_integer (sp + (4 * (argi + 1)), 4);
 }
 
+struct target_desc *tdesc_i386, *tdesc_amd64;
+
+static void
+i386_register_g_packet_guesses (struct gdbarch *gdbarch)
+{
+  int regs32, regs64, regs387, regs_sse32, regs_sse64;
+
+  if (tdesc_i386 == NULL)
+    {
+      const struct bfd_arch_info *arch;
+
+      tdesc_i386 = allocate_target_description ();
+      arch = bfd_lookup_arch (bfd_arch_i386, bfd_mach_i386_i386);
+      set_tdesc_architecture (tdesc_i386, arch);
+
+      tdesc_amd64 = allocate_target_description ();
+      arch = bfd_lookup_arch (bfd_arch_i386, bfd_mach_x86_64);
+      set_tdesc_architecture (tdesc_amd64, arch);
+    }
+
+  /* If the size matches the set of 32-bit or 64-bit integer registers,
+     assume that's what we've got.  */
+  regs32 = 16 * 4;
+  register_remote_g_packet_guess (gdbarch, regs32, tdesc_i386);
+
+  regs64 = 17 * 8 + 7 * 4;
+  register_remote_g_packet_guess (gdbarch, regs64, tdesc_amd64);
+
+  /* Check for that plus i387 registers.  */
+  regs387 = 8 * 10 + 8 * 4;
+  register_remote_g_packet_guess (gdbarch, regs32 + regs387, tdesc_i386);
+  register_remote_g_packet_guess (gdbarch, regs64 + regs387, tdesc_amd64);
+
+  /* Check for that plus SSE.  */
+  regs_sse32 = 8 * 16 + 4;
+  register_remote_g_packet_guess (gdbarch, regs32 + regs387 + regs_sse32,
+				  tdesc_i386);
+
+  regs_sse64 = 16 * 16 + 4;
+  register_remote_g_packet_guess (gdbarch, regs64 + regs387 + regs_sse64,
+				  tdesc_amd64);
+}
+
 \f
 static struct gdbarch *
 i386_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
@@ -2418,6 +2463,9 @@ i386_gdbarch_init (struct gdbarch_info i
   /* Helper for function argument information.  */
   set_gdbarch_fetch_pointer_argument (gdbarch, i386_fetch_pointer_argument);
 
+  /* Register likely sizes of the remote protocol 'g' packet.  */
+  i386_register_g_packet_guesses (gdbarch);
+
   /* Hook in the DWARF CFI frame unwinder.  */
   frame_unwind_append_sniffer (gdbarch, dwarf2_frame_sniffer);
 
Index: src/gdb/remote.c
===================================================================
--- src.orig/gdb/remote.c	2006-11-09 10:38:30.000000000 -0500
+++ src/gdb/remote.c	2006-11-09 10:39:26.000000000 -0500
@@ -3466,11 +3466,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;
@@ -3478,11 +3477,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)
@@ -3519,20 +3548,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.  */
@@ -3580,6 +3595,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 ();
@@ -5983,6 +6005,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)
 {
@@ -6034,6 +6133,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
@@ -6166,6 +6266,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
@@ -6246,6 +6347,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/amd64-linux-tdep.c
===================================================================
--- src.orig/gdb/amd64-linux-tdep.c	2006-11-09 10:34:53.000000000 -0500
+++ src/gdb/amd64-linux-tdep.c	2006-11-09 10:39:26.000000000 -0500
@@ -29,6 +29,7 @@
 #include "gdbtypes.h"
 #include "reggroups.h"
 #include "amd64-linux-tdep.h"
+#include "i386-linux-tdep.h"
 
 #include "gdb_string.h"
 
@@ -287,6 +288,8 @@ amd64_linux_init_abi (struct gdbarch_inf
   /* Enable TLS support.  */
   set_gdbarch_fetch_tls_load_module_address (gdbarch,
                                              svr4_fetch_objfile_link_map);
+
+  i386_linux_register_g_packet_guesses (gdbarch);
 }
 \f
 
Index: src/gdb/i386-linux-tdep.c
===================================================================
--- src.orig/gdb/i386-linux-tdep.c	2006-11-09 10:34:53.000000000 -0500
+++ src/gdb/i386-linux-tdep.c	2006-11-09 10:39:26.000000000 -0500
@@ -1,6 +1,6 @@
 /* Target-dependent code for GNU/Linux i386.
 
-   Copyright (C) 2000, 2001, 2002, 2003, 2004, 2005
+   Copyright (C) 2000, 2001, 2002, 2003, 2004, 2005, 2006
    Free Software Foundation, Inc.
 
    This file is part of GDB.
@@ -28,6 +28,7 @@
 #include "inferior.h"
 #include "osabi.h"
 #include "reggroups.h"
+#include "remote.h"
 #include "dwarf2-frame.h"
 #include "gdb_string.h"
 
@@ -403,6 +404,26 @@ static int i386_linux_sc_reg_offset[] =
   0 * 4				/* %gs */
 };
 
+void
+i386_linux_register_g_packet_guesses (struct gdbarch *gdbarch)
+{
+  int regs32, regs64, regs387, regs_sse32, regs_sse64;
+
+  /* Recognize the GNU/Linux %orig_eax and %orig_rax "registers".  */
+  regs32 = 16 * 4;
+  regs64 = 17 * 8 + 7 * 4;
+
+  regs387 = 8 * 10 + 8 * 4;
+
+  regs_sse32 = 8 * 16 + 4;
+  regs_sse64 = 16 * 16 + 4;
+
+  register_remote_g_packet_guess (gdbarch, regs32 + regs387 + regs_sse32 + 4,
+				  tdesc_i386);
+  register_remote_g_packet_guess (gdbarch, regs64 + regs387 + regs_sse64 + 8,
+				  tdesc_amd64);
+}
+
 static void
 i386_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
@@ -442,6 +463,8 @@ i386_linux_init_abi (struct gdbarch_info
   /* Enable TLS support.  */
   set_gdbarch_fetch_tls_load_module_address (gdbarch,
                                              svr4_fetch_objfile_link_map);
+
+  i386_linux_register_g_packet_guesses (gdbarch);
 }
 
 /* Provide a prototype to silence -Wmissing-prototypes.  */
Index: src/gdb/i386-linux-tdep.h
===================================================================
--- src.orig/gdb/i386-linux-tdep.h	2006-11-09 10:34:53.000000000 -0500
+++ src/gdb/i386-linux-tdep.h	2006-11-09 10:39:26.000000000 -0500
@@ -1,6 +1,6 @@
 /* Target-dependent code for GNU/Linux x86.
 
-   Copyright (C) 2002, 2003 Free Software Foundation, Inc.
+   Copyright (C) 2002, 2003, 2006 Free Software Foundation, Inc.
 
    This file is part of GDB.
 
@@ -36,4 +36,6 @@
 /* Total number of registers for GNU/Linux.  */
 #define I386_LINUX_NUM_REGS (I386_LINUX_ORIG_EAX_REGNUM + 1)
 
+void i386_linux_register_g_packet_guesses (struct gdbarch *gdbarch);
+
 #endif /* i386-linux-tdep.h */
Index: src/gdb/i386-tdep.h
===================================================================
--- src.orig/gdb/i386-tdep.h	2006-11-09 10:34:53.000000000 -0500
+++ src/gdb/i386-tdep.h	2006-11-09 10:39:26.000000000 -0500
@@ -28,6 +28,7 @@ struct gdbarch;
 struct reggroup;
 struct regset;
 struct regcache;
+struct target_desc;
 
 /* GDB's i386 target supports both the 32-bit Intel Architecture
    (IA-32) and the 64-bit AMD x86-64 architecture.  Internally it uses
@@ -156,6 +157,11 @@ extern struct type *i386_mmx_type;
 extern struct type *i386_sse_type;
 extern struct type *i386_mxcsr_type;
 
+/* Minimal target descriptions for i386 and amd64 targets, which only
+   specify the architecture.  */
+extern struct target_desc *tdesc_i386;
+extern struct target_desc *tdesc_amd64;
+
 /* Segment selectors.  */
 #define I386_SEL_RPL	0x0003  /* Requester's Privilege Level mask.  */
 #define I386_SEL_UPL	0x0003	/* User Privilige Level. */
Index: src/gdb/remote.h
===================================================================
--- src.orig/gdb/remote.h	2006-11-09 10:34:53.000000000 -0500
+++ src/gdb/remote.h	2006-11-09 10:39:26.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


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

* Re: [rfc] Autoselect x86_64 or i386 based on the remote g packet       size
  2006-11-09 20:58 [rfc] Autoselect x86_64 or i386 based on the remote g packet size Daniel Jacobowitz
@ 2006-11-09 21:36 ` Mark Kettenis
  2006-11-09 21:49   ` Daniel Jacobowitz
  0 siblings, 1 reply; 4+ messages in thread
From: Mark Kettenis @ 2006-11-09 21:36 UTC (permalink / raw)
  To: gdb-patches

>  The operating principle is that a target is free to send back a "short"
>  g packet, but some sizes are plausible and others are not.  So I
>  enhanced the i386 backend to register the sizes of likely register
>  sets: core registers, core + i387, core + i387 + SSE.  The Linux
>  backend adds the %orig_eax and %orig_rax pseudo-"registers" too
>  (which, if I were inventing them today, might be target objects like
>  the sparc WCOOKIE, but are currently in the g packet).
>
>  So if you have a GDB which defaults to i386-linux, and it connects to
>  an amd64 target and gets exactly the right number of bytes to be
>  a 64-bit register set, it'll assume that's what it's got.
>
>  There's deliberately no mechanism to say "more than X bytes must be
>  amd64",
>  because I feel that's unsafe.  The current registered guesses are
>  high-confidence, since e.g the amd64 guesses would be quite odd sizes for
>  an i386 target to return.  For instance, the amd64 non-FP registers cut
>  off between fioff and foseg if interpreted as a 32-bit register set: not
>  very likely.
>
>  All comments welcome!  Tested on x86_64-pc-linux-gnu, using gdbserver for
>  the testsuite and by hand.

Hi Daniel,

I can't say I'm very enthousiastic about this patch.  It feels like a
kludge to avoid adding proper support to the remote protocol that allows
gdb to interrogate the target about this.  A very clever kludge, but still
a kludge.

Mark


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

* Re: [rfc] Autoselect x86_64 or i386 based on the remote g packet       size
  2006-11-09 21:36 ` Mark Kettenis
@ 2006-11-09 21:49   ` Daniel Jacobowitz
  2006-11-20  4:26     ` Daniel Jacobowitz
  0 siblings, 1 reply; 4+ messages in thread
From: Daniel Jacobowitz @ 2006-11-09 21:49 UTC (permalink / raw)
  To: gdb-patches

On Thu, Nov 09, 2006 at 10:36:29PM +0100, Mark Kettenis wrote:
> Hi Daniel,
> 
> I can't say I'm very enthousiastic about this patch.  It feels like a
> kludge to avoid adding proper support to the remote protocol that allows
> gdb to interrogate the target about this.  A very clever kludge, but still
> a kludge.

True.  I'm not avoiding it, though; that interrogation happens to be
mentioned here:
  http://sourceware.org/ml/gdb-patches/2006-11/msg00055.html

  Something I haven't implemented yet, but probably will before posting
  the XML bits: this is enough to implement a remote target which can
  tell you its architecture!  Pretty neat, though a little tricky to get
  right.  What we ought to do at that point is validate that the remote
  target and the selected executable have sufficiently compatible
  architectures.  Then GDB will be able to warn me when I connect a MIPS
  cross GDB to an i386 gdbserver by accident (which I do probably once a
  week).

Honestly, this clever auto-detection is much more useful for MIPS than
it is for i386/AMD64; for MIPS, an executable file doesn't necessarily
contain the answer to the question, but for i386 it always does.  I
implemented the AMD64 bits after I had it working for MIPS, as a demo
when Jan K. posted an even kludgier patch to warn about this case a
couple of weeks ago.  If you don't like this bit, I'd be happy to drop
it, and make gdbserver report the architecture manually when I advance
to the XML stage of these target descriptions.  I won't mourn the i386
parts of this patch.  Would you rather I did that?

(I'd like to hold on to the MIPS bits, because they're solving a
slightly different problem, and because I know they're useful with
existing non-gdbserver stubs that would be harder to fix.)

-- 
Daniel Jacobowitz
CodeSourcery


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

* Re: [rfc] Autoselect x86_64 or i386 based on the remote g packet       size
  2006-11-09 21:49   ` Daniel Jacobowitz
@ 2006-11-20  4:26     ` Daniel Jacobowitz
  0 siblings, 0 replies; 4+ messages in thread
From: Daniel Jacobowitz @ 2006-11-20  4:26 UTC (permalink / raw)
  To: gdb-patches

On Thu, Nov 09, 2006 at 04:49:24PM -0500, Daniel Jacobowitz wrote:
> Honestly, this clever auto-detection is much more useful for MIPS than
> it is for i386/AMD64; for MIPS, an executable file doesn't necessarily
> contain the answer to the question, but for i386 it always does.  I
> implemented the AMD64 bits after I had it working for MIPS, as a demo
> when Jan K. posted an even kludgier patch to warn about this case a
> couple of weeks ago.  If you don't like this bit, I'd be happy to drop
> it, and make gdbserver report the architecture manually when I advance
> to the XML stage of these target descriptions.  I won't mourn the i386
> parts of this patch.  Would you rather I did that?
> 
> (I'd like to hold on to the MIPS bits, because they're solving a
> slightly different problem, and because I know they're useful with
> existing non-gdbserver stubs that would be harder to fix.)

Hi Mark,

For avoidance of doubt, I'm definitely dropping the i386/amd64 patch.
It was a stretch to begin with.  I've already begun working on a patch
to specifiy the architecture explicitly and I really hope I'll have it
finished this week (but the way this entire project's been dragging
out, I'm not sure...).

Could you let me know if using the g packet size approach for the
clearer-cut MIPS situation is OK with you?  The difference, in my
opinion, is that it doesn't reflect a change in architecture; you can
run the same o32 ABI program on a 32-bit or 64-bit system, so it would
be nice if GDB could handle 32-bit or 64-bit regsets automatically.
This prevents a mips64 gdbserver having to figure out the child's ABI.

-- 
Daniel Jacobowitz
CodeSourcery


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

end of thread, other threads:[~2006-11-20  4:26 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-11-09 20:58 [rfc] Autoselect x86_64 or i386 based on the remote g packet size Daniel Jacobowitz
2006-11-09 21:36 ` Mark Kettenis
2006-11-09 21:49   ` Daniel Jacobowitz
2006-11-20  4:26     ` Daniel Jacobowitz

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