Re: new gdb remote packet type

[Andrew Cagney <cagney at gnu dot org>]

> > > > Yes, the qPart packet was designed for exactly the situtation you encountered.  It can specify a length/offset and return a short transfer.
> > > >
> > > > Andrew
> > > Hi:
> > >
> > > Does this represent what you are expecting of the qPart packet?  I saw no reason to export the type because this
> > > packet is only useful in remote.c
> >
> >
> >
> > While not immediatly, it is going to eventually be exposed out side of remote.c, so we need to get what goes across the wire right up front. To that end:
> >
> > - use register sets
> >
> > Unlike the G-packet, where the byte layout is determined by GDB internal data structure, the byte layout needs to be specified by the inferior's regsets.  That in turn means specifying separate packets for fetching all or part of each of "gregs", "fpregs", and "xpregs"(?).
> >
> > - specify offset/length
> >
> > I think your code is specifying a register number, it should view the region as a sequence of bytes and fetch using offset/length.  This information can be obtained using "regset.h" (although that interface might need some massaging).
> >
> > - fetch lazy
> >
> > Just request the region for the requested register.  It's then up to the remote end to determine that more/less of the region can be supplied.
> >
> > Eventually this will also be added to the target vector, however, for the moment something more local is definitly fine.
> >
> > Andrew
> Hi:
>
> Well I must admit to being a little confused:)  My intent is to not introduce any architecture dependency in remote.c and just restructure the g-packet to match what the target wants.  I could be missing a point here but the regset stuff looks very architecture dependent and it would seem incorrect to use it in remote.c.
> Well for offset I am using the regnum.  We are inquirying whether the target wants the reg mapped in the g-packet .

There are two, equally correct, ways to view the registers being fetched:

- as array of values indexed by register number
- as flat byte addressable region with implied format

The current G packet format uses the latter, badly.  The implied format 
is determined somewhat arbitrarially by GDB-internals.  The qPart I'm 
suggesting, while still viewing registers as a flat sequence of bytes, 
is at least citing /proc and regsets for that bytes format (and a 
standard GDB is expected to understand anyway).

One way to think of qPart is that it provides a mechanism for accessing 
a number of separate address spaces - memory, gregset, auxv, etc.  For 
each, it always specifies an offset/length and gets back at least part 
of that request.  Passing a register index, doesn't fit into that model.

> We could fetch lazy but this isn't currently done by remote.c because most target g-packet payloads aren't very large. IA64 is the exception with a payload in excess of 10,000 bytes.
> I must be missing your larger objective here because  my limited knowledge of gdb is in remote.c.

It's still large enough to hurt - people are still debugging across very 
 low bandwidth links.

Anyway, it sounds like you might (dig dig) be more interested  in the 
attached patch+hack (fernando nasser gets credit for the patch, I deny 
responsibility for the hack bit :-).  It adds a P packet for fetching 
individual registers which is something the existing remote protocol 
spec doco hints at.

Andrew


--- /home/scratch/GDB/src/gdb/remote.c	Wed Apr 28 09:15:50 2004
+++ remote.c	Wed May  5 13:32:31 2004
@@ -267,7 +267,11 @@
       r->pnum = regnum;
       r->regnum = regnum;
       r->offset = DEPRECATED_REGISTER_BYTE (regnum);
+#ifdef GPACKET_UPPER_BOUND_HACK
+      r->in_g_packet = (regnum < GPACKET_UPPER_BOUND_HACK (gdbarch));
+#else
       r->in_g_packet = (regnum < NUM_REGS);
+#endif
       /* ...name = REGISTER_NAME (regnum); */
 
       /* Compute packet size by accumulating the size of all registers. */
@@ -3240,6 +3244,65 @@
 
 static int register_bytes_found;
 
+/* Helper for remote_fetch_registers.  Fetch a register using the
+   ``p'' packet.  */
+
+static void
+fetch_register_using_p (int regnum)
+{
+  struct remote_state *rs = get_remote_state ();
+  char *regs;
+  char *buf;
+  int i;
+  char *p;
+
+  buf = alloca (rs->remote_packet_size);
+  sprintf (buf, "p%x", regnum);
+  remote_send (buf, rs->remote_packet_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 ((buf[0] < '0' || buf[0] > '9')
+	 && (buf[0] < 'a' || buf[0] > 'f')
+	 && buf[0] != 'x')	/* New: unavailable register value */
+    {
+      if (remote_debug)
+	fprintf_unfiltered (gdb_stdlog,
+			    "Bad register packet; fetching a new packet\n");
+      getpkt (buf, rs->remote_packet_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.  */
+
+  regs = alloca (DEPRECATED_REGISTER_RAW_SIZE (regnum));
+  memset (regs, DEPRECATED_REGISTER_RAW_SIZE (regnum), 0);
+  p = buf;
+  for (i = 0; i < DEPRECATED_REGISTER_RAW_SIZE (regnum); i++)
+    {
+      if (p[0] == 0)
+	break;
+      if (p[1] == 0)
+	{
+	  warning ("Remote reply is of odd length: %s", buf);
+	  /* Don't change register_bytes_found in this case, and don't
+	     print a second warning.  */
+	  break;
+	}
+      if (p[0] == 'x' && p[1] == 'x')
+	regs[i] = 0;		/* 'x' */
+      else
+	regs[i] = fromhex (p[0]) * 16 + fromhex (p[1]);
+      p += 2;
+    }
+
+  supply_register (regnum, &regs[0]);
+  if (buf[0] == 'x')
+    deprecated_register_valid[i] = -1;	/* register value not available */
+}
+
 /* Read the remote registers into the block REGS.  */
 /* Currently we just read all the registers, so we don't use regnum.  */
 
@@ -3258,10 +3321,18 @@
     {
       struct packet_reg *reg = packet_reg_from_regnum (rs, regnum);
       gdb_assert (reg != NULL);
+#if 0
       if (!reg->in_g_packet)
 	internal_error (__FILE__, __LINE__,
 			"Attempt to fetch a non G-packet register when this "
 			"remote.c does not support the p-packet.");
+#else
+      if (!reg->in_g_packet)
+	{
+	  fetch_register_using_p (regnum);
+	  return;
+	}
+#endif
     }
 
   sprintf (buf, "g");