remote protocol support for TARGET_OBJECT_AUXV

remote protocol support for TARGET_OBJECT_AUXV

[Roland McGrath]

I am proposing a new query packet `qAuxVector' to support reading
TARGET_OBJECT_AUXV data.  Here's the proposed docs addition describing it.
After that, I've appended (short) patches implementing it in gdb and
gdbserver so you can see concretely what I mean.


Thanks,
Roland


Index: gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.192
diff -u -b -p -r1.192 gdb.texinfo
--- gdb.texinfo	4 Feb 2004 23:24:43 -0000	1.192
+++ gdb.texinfo	4 Feb 2004 23:56:55 -0000
@@ -20420,6 +20420,23 @@ The target does not need to look up any 
 The target requests the value of a new symbol @var{sym_name} (hex
 encoded).  @value{GDBN} will continue to supply the values of symbols
 (if available), until the target ceases to request them.
+@end table
+
+@item @code{qAuxVector}:@var{offset},@var{length} --- auxiliary vector data
+
+Read from the target's @dfn{auxiliary vector}, treated as an
+uninterpreted block of bytes.  (@xref{Auxiliary Vector}.)
+Request @var{length} bytes starting at @var{offset} bytes into the data.
+
+Reply:
+@table @samp
+@item @code{=}@var{data}
+@var{data} (hex encoded) contains the data bytes read.
+There is no more auxiliary data after what was requested.
+
+@item @code{+}@var{data}
+@var{data} (hex encoded) contains the data bytes read.
+There may be additional data that can be read with another request.
 @end table
 
 @end table



2004-02-01  Roland McGrath  <roland@redhat.com>

	* remote.c (remote_protocol_qAuxVector): New variable.
	(init_all_packet_configs): Initialize it.
	(set_remote_protocol_qAuxVector_packet_cmd): New function.
	(show_remote_protocol_qAuxVector_packet_cmd): New function.
	(show_remote_cmd): Call it.
	(_initialize_remote): Initialize commands.
	(remote_xfer_partial): If enabled, use qAuxVector query to service
	TARGET_OBJECT_AUXV requests.

Index: remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.128
diff -u -b -p -r1.128 remote.c
--- remote.c	26 Jan 2004 23:07:00 -0000	1.128
+++ remote.c	4 Feb 2004 23:56:37 -0000
@@ -981,6 +981,23 @@ show_remote_protocol_binary_download_cmd
   show_packet_config_cmd (&remote_protocol_binary_download);
 }
 
+/* Should we try the 'qAuxVector' (target auxiliary vector read) request? */
+static struct packet_config remote_protocol_qAuxVector;
+
+static void
+set_remote_protocol_qAuxVector_packet_cmd (char *args, int from_tty,
+					   struct cmd_list_element *c)
+{
+  update_packet_config (&remote_protocol_qAuxVector);
+}
+
+static void
+show_remote_protocol_qAuxVector_packet_cmd (char *args, int from_tty,
+					    struct cmd_list_element *c)
+{
+  show_packet_config_cmd (&remote_protocol_qAuxVector);
+}
+
 
 /* Tokens for use by the asynchronous signal handlers for SIGINT */
 static void *sigint_remote_twice_token;
@@ -2070,6 +2087,7 @@ init_all_packet_configs (void)
   /* Force remote_write_bytes to check whether target supports binary
      downloading. */
   update_packet_config (&remote_protocol_binary_download);
+  update_packet_config (&remote_protocol_qAuxVector);
 }
 
 /* Symbol look-up. */
@@ -4872,6 +4890,50 @@ remote_xfer_partial (struct target_ops *
     case TARGET_OBJECT_AVR:
       query_type = 'R';
       break;
+
+    case TARGET_OBJECT_AUXV:
+      if (remote_protocol_qAuxVector.support != PACKET_DISABLE)
+	{
+	  unsigned int total = 0;
+	  while (len > 0)
+	    {
+	      LONGEST n = min ((rs->remote_packet_size - 2) / 2, len);
+	      snprintf (buf2, rs->remote_packet_size,
+			"qAuxVector:%s,%s",
+			phex_nz (offset, sizeof offset),
+			phex_nz (n, sizeof n));
+	      i = putpkt (buf2);
+	      if (i < 0)
+		return total > 0 ? total : i;
+	      buf2[0] = '\0';
+	      getpkt (buf2, rs->remote_packet_size, 0);
+	      if (packet_ok (buf2, &remote_protocol_qAuxVector) != PACKET_OK)
+		return total > 0 ? total : -1;
+	      switch (buf2[0])
+		{
+		case '=':	/* Final data.  */
+		  return total + hex2bin (&buf2[1], readbuf, len);
+
+		case '+':	/* Some data here, but there is more.  */
+		  i = hex2bin (&buf2[2], readbuf, len);
+		  if (i > 0)
+		    {
+		      readbuf = (void *) ((char *) readbuf + i);
+		      offset += i;
+		      len -= i;
+		      total += i;
+		      break;
+		    }
+
+		default:
+		  warning ("Malformed response to qAuxVector: %s", buf2);
+		  return total > 0 ? total : i;
+		}
+	    }
+	  return total;
+	}
+      return -1;
+
     default:
       return -1;
     }
@@ -5369,6 +5431,7 @@ show_remote_cmd (char *args, int from_tt
   show_remote_protocol_qSymbol_packet_cmd (args, from_tty, NULL);
   show_remote_protocol_vcont_packet_cmd (args, from_tty, NULL);
   show_remote_protocol_binary_download_cmd (args, from_tty, NULL);
+  show_remote_protocol_qAuxVector_packet_cmd (args, from_tty, NULL);
 }
 
 static void
@@ -5610,6 +5673,13 @@ in a memory packet.\n",
 			 "Z4", "access-watchpoint",
 			 set_remote_protocol_Z_access_wp_packet_cmd,
 			 show_remote_protocol_Z_access_wp_packet_cmd,
+			 &remote_set_cmdlist, &remote_show_cmdlist,
+			 0);
+
+  add_packet_config_cmd (&remote_protocol_qAuxVector,
+			 "qAuxVector", "read-aux-vector",
+			 set_remote_protocol_qAuxVector_packet_cmd,
+			 show_remote_protocol_qAuxVector_packet_cmd,
 			 &remote_set_cmdlist, &remote_show_cmdlist,
 			 0);
 


2004-02-01  Roland McGrath  <roland@redhat.com>

	* target.h (struct target_ops): New member `read_auxv'.
	* server.c (handle_query): Handle qAuxVector query using that hook.
	* linux-low.c (linux_read_auxv): New function.
	(linux_target_ops): Initialize `read_auxv' member to that.

Index: gdbserver/linux-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/linux-low.c,v
retrieving revision 1.27
diff -b -p -u -r1.27 linux-low.c
--- gdbserver/linux-low.c	31 Jan 2004 22:19:31 -0000	1.27
+++ gdbserver/linux-low.c	4 Feb 2004 23:55:48 -0000
@@ -1381,6 +1381,32 @@ linux_send_signal (int signum)
     kill (signal_pid, signum);
 }
 
+/* Copy LEN bytes from inferior's auxiliary vector starting at OFFSET
+   to debugger memory starting at MYADDR.  */
+
+static int
+linux_read_auxv (CORE_ADDR offset, char *myaddr, unsigned int len)
+{
+  char filename[PATH_MAX];
+  int fd, n;
+
+  snprintf (filename, sizeof filename, "/proc/%d/auxv", inferior_pid);
+
+  fd = open (filename, O_RDONLY);
+  if (fd < 0)
+    return -1;
+
+  if (offset != (CORE_ADDR) 0
+      && lseek (fd, (off_t) offset, SEEK_SET) != (off_t) offset)
+    n = -1;
+  else
+    n = read (fd, myaddr, len);
+
+  close (fd);
+
+  return n;
+}
+
 \f
 static struct target_ops linux_target_ops = {
   linux_create_inferior,
@@ -1396,6 +1422,7 @@ static struct target_ops linux_target_op
   linux_write_memory,
   linux_look_up_symbols,
   linux_send_signal,
+  linux_read_auxv,
 };
 
 static void
Index: gdbserver/server.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/server.c,v
retrieving revision 1.16
diff -b -p -u -r1.16 server.c
--- gdbserver/server.c	13 Oct 2003 16:17:21 -0000	1.16
+++ gdbserver/server.c	4 Feb 2004 23:55:48 -0000
@@ -120,6 +120,31 @@ handle_query (char *own_buf)
 	}
     }
       
+  if (strncmp ("qAuxVector:", own_buf, 11) == 0)
+    {
+      if (the_target->read_auxv == NULL)
+	strcpy (own_buf, "E00");
+      else
+	{
+	  char data[(PBUFSIZ - 2) / 2];
+	  CORE_ADDR ofs;
+	  unsigned int len;
+	  int n;
+	  decode_m_packet (&own_buf[11], &ofs, &len); /* "OFS,LEN" */
+	  if (len > sizeof data)
+	    len = sizeof data;
+	  n = (*the_target->read_auxv) (ofs, data, len);
+	  if (n < 0)
+	    strcpy (own_buf, "E00");
+	  else
+	    {
+	      own_buf[0] = n < len ? '=' : '+';
+	      convert_int_to_ascii (data, &own_buf[1], n);
+	    }
+	}
+      return;
+    }
+
   /* Otherwise we didn't know what packet it was.  Say we didn't
      understand it.  */
   own_buf[0] = 0;
Index: gdbserver/target.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/target.h,v
retrieving revision 1.8
diff -b -p -u -r1.8 target.h
--- gdbserver/target.h	13 Oct 2003 16:17:21 -0000	1.8
+++ gdbserver/target.h	4 Feb 2004 23:55:48 -0000
@@ -125,6 +125,12 @@ struct target_ops
 
   /* Send a signal to the inferior process, however is appropriate.  */
   void (*send_signal) (int);
+
+  /* Read auxiliary vector data from the inferior process.
+
+     Read LEN bytes at OFFSET into a buffer at MYADDR.  */
+
+  int (*read_auxv) (CORE_ADDR offset, char *myaddr, unsigned int len);
 };
 
 extern struct target_ops *the_target;

Re: remote protocol support for TARGET_OBJECT_AUXV

[Eli Zaretskii]

> Date: Wed, 4 Feb 2004 15:58:46 -0800
> From: Roland McGrath <roland@redhat.com>
> 
> I am proposing a new query packet `qAuxVector' to support reading
> TARGET_OBJECT_AUXV data.  Here's the proposed docs addition describing it.
> After that, I've appended (short) patches implementing it in gdb and
> gdbserver so you can see concretely what I mean.

The manual change is okay.  Thanks.

Re: remote protocol support for TARGET_OBJECT_AUXV

[Andrew Cagney]

> I am proposing a new query packet `qAuxVector' to support reading
> TARGET_OBJECT_AUXV data.  Here's the proposed docs addition describing it.
> After that, I've appended (short) patches implementing it in gdb and
> gdbserver so you can see concretely what I mean.

(roland, in case you're confused - this gets discussed as a protocol 
change and also approved as a doco change)

> Index: gdb.texinfo
> ===================================================================
> RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
> retrieving revision 1.192
> diff -u -b -p -r1.192 gdb.texinfo
> --- gdb.texinfo	4 Feb 2004 23:24:43 -0000	1.192
> +++ gdb.texinfo	4 Feb 2004 23:56:55 -0000
> @@ -20420,6 +20420,23 @@ The target does not need to look up any 
>  The target requests the value of a new symbol @var{sym_name} (hex
>  encoded).  @value{GDBN} will continue to supply the values of symbols
>  (if available), until the target ceases to request them.
> +@end table
> +
> +@item @code{qAuxVector}:@var{offset},@var{length} --- auxiliary vector data

The underlying target methods that this is adding support for are:

/* Request the transfer of up to LEN 8-bit bytes of the target's
    OBJECT.  The OFFSET, for a seekable object, specifies the starting
    point.  The ANNEX can be used to provide additional data-specific
    information to the target.

    Return the number of bytes actually transfered, zero when no
    further transfer is possible, and -1 when the transfer is not
    supported.

    NOTE: cagney/2003-10-17: The current interface does not support a
    "retry" mechanism.  Instead it assumes that at least one byte will
    be transfered on each call.

    NOTE: cagney/2003-10-17: The current interface can lead to
    fragmented transfers.  Lower target levels should not implement
    hacks, such as enlarging the transfer, in an attempt to compensate
    for this.  Instead, the target stack should be extended so that it
    implements supply/collect methods and a look-aside object cache.
    With that available, the lowest target can safely and freely "push"
    data up the stack.

    NOTE: cagney/2003-10-17: Unlike the old query and the memory
    transfer mechanisms, these methods are explicitly parameterized by
    the target that it should be applied to.

    NOTE: cagney/2003-10-17: Just like the old query and memory xfer
    methods, these new methods perform partial transfers.  The only
    difference is that these new methods thought to include "partial"
    in the name.  The old code's failure to do this lead to much
    confusion and duplication of effort as each target object attempted
    to locally take responsibility for something it didn't have to
    worry about.

    NOTE: cagney/2003-10-17: With a TARGET_OBJECT_KOD object, for
    backward compatibility with the "target_query" method that this
    replaced, when OFFSET and LEN are both zero, return the "minimum"
    buffer size.  See "remote.c" for further information.  */

enum target_object
{
   /* Kernel Object Display transfer.  See "kod.c" and "remote.c".  */
   TARGET_OBJECT_KOD,
   /* AVR target specific transfer.  See "avr-tdep.c" and "remote.c".  */
   TARGET_OBJECT_AVR,
   /* Transfer up-to LEN bytes of memory starting at OFFSET.  */
   TARGET_OBJECT_MEMORY,
   /* Kernel Unwind Table.  See "ia64-tdep.c".  */
   TARGET_OBJECT_UNWIND_TABLE,
   /* Transfer auxilliary vector.  */
   TARGET_OBJECT_AUXV,
   /* StackGhost cookie.  See "sparc-tdep.c".  */
   TARGET_OBJECT_WCOOKIE

   /* Possible future objects: TARGET_OBJECT_FILE, TARGET_OBJECT_PROC, 
... */
};

extern LONGEST target_read_partial (struct target_ops *ops,
                                     enum target_object object,
                                     const char *annex, void *buf,
                                     ULONGEST offset, LONGEST len);

extern LONGEST target_write_partial (struct target_ops *ops,
                                      enum target_object object,
                                      const char *annex, const void *buf,
                                      ULONGEST offset, LONGEST len);

I think it would be better to specify something that accomodates this 
(to my suprise) rapidly expanding interface.
Otherwize, in a month or so, we'll find ourselves discussing the exact 
saome problem for "unwind-table", "wcookie", "avr", and "kod".

I also think that the protocol semantics should reflect this interfaces 
semantics (or a superset of it).

> +Read from the target's @dfn{auxiliary vector}, treated as an
> +uninterpreted block of bytes.  (@xref{Auxiliary Vector}.)
> +Request @var{length} bytes starting at @var{offset} bytes into the data.
> +
> +Reply:
> +@table @samp
> +@item @code{=}@var{data}
> +@var{data} (hex encoded) contains the data bytes read.
> +There is no more auxiliary data after what was requested.
> +
> +@item @code{+}@var{data}
> +@var{data} (hex encoded) contains the data bytes read.
> +There may be additional data that can be read with another request.
>  @end table

(technical nit "+" can't be used.  For want of a better term, it acts as 
a [re]sync character and hence can't appear in the middle of a packet. 
Also see note at end.)

Lets start with this:

-> qPartial:<obj>:read:<x-annex>:<x-offset>,<x-length>
	<- OK -- implicit zero length returned EOF
	<- <x-data> -- must contain the byte @<x-offset>
	<- "" -- packet not recognized
	<- Enn - something wrong with the request

-> qPartial:<obj>:write:<x-annex>:<x-offset>:<x-data>
	<- <x-be-length> -- number of bytes actially written
	<- "" -- packet not recognized
	<- Enn -- something wrong with the request

-> qPartial:<obj>?
	<- OK -- queries of type <obj> are supported
	<- "" -- queries of type <obj> are not supported

a variation being something like:
qPartial:obj=<obj>:op=write[:annex=<x-annex>]:offset=<x-offset>:data=<x-data>
which makes it more extensible.

Anyway, this would give us:
-> qPartial:auxv?
OK
-> qPartial:auxv:read::0,ff
123456....

And closely parallels the read/write partial methods.

Once that's been used for a bit, and its performance evaluated, we can 
look to refine it.  Perhaphs by adding additional replies to the read 
side (and even the write side).

--

Looking more closely at this fragment of the committed code:

> LONGEST
> target_auxv_read (struct target_ops *ops, char **data)
> {
>   size_t auxv_alloc = 512, auxv_pos = 0;
>   char *auxv = xmalloc (auxv_alloc);
>   int n;
> 
>   while (1)
>     {
>       n = target_read_partial (ops, TARGET_OBJECT_AUXV,
>                                NULL, &auxv[auxv_pos], 0,
>                                auxv_alloc - auxv_pos);
>       if (n <= 0)
>         break;
>       auxv_pos += n;
>       if (auxv_pos < auxv_alloc) /* Read all there was.  */
>         break;

the it must assume that the transfer was fragmented, and attempt a re-fetch.

Andrew

Re: remote protocol support for TARGET_OBJECT_AUXV

[Roland McGrath]

> I think it would be better to specify something that accomodates this 
> (to my suprise) rapidly expanding interface.
> Otherwize, in a month or so, we'll find ourselves discussing the exact 
> saome problem for "unwind-table", "wcookie", "avr", and "kod".

In the gdb-patches thread, I suggested such an idea as well.  The details
of transmitting offset/length and getting back data/eof is something we
should decide once and do the same for all new cases, whether they are
technically the same query or not.

> I also think that the protocol semantics should reflect this interfaces 
> semantics (or a superset of it).

That seems reasonable to me, hence my prior similar suggestion.  However, I
also sympathize with the opinion Dan expressed.  I will paraphrase that as
that the stub's job should be kept simple whenever possible.  The stubs are
after all, stubs (i.e. small things), and there are many of them, whereas
gdb is big and hairy and so extra layers of well-organizedness are good in
gdb but sometimes bad for stubs.

> (technical nit "+" can't be used.

Right!  Thanks.  It's been a long time since I was in the habit of typing
gdb remote protocol by hand, so I'd forgotten what is and isn't encoded how.
Your having reminded me, I went and re-read the elisp I used to do that.

> -> qPartial:<obj>:read:<x-annex>:<x-offset>,<x-length>
> 	<- OK -- implicit zero length returned EOF
> 	<- <x-data> -- must contain the byte @<x-offset>
> 	<- "" -- packet not recognized
> 	<- Enn - something wrong with the request
> 
> -> qPartial:<obj>:write:<x-annex>:<x-offset>:<x-data>
> 	<- <x-be-length> -- number of bytes actially written
> 	<- "" -- packet not recognized
> 	<- Enn -- something wrong with the request

This looks fine to me, basically.  But I do think it is worth starting out
with the one more wrinkle of EOF reporting along with data return.  With
this simpler protocol, there are necessarily two request/reply transactions
for every time you want to read "all the data", even if there is as little
as one byte of it to read.  That seems unnecessary and undesireable.
(Perhaps I've spent too long doing serial debugging when serial ports were
slower than they usually are these days.  But all the remote protocol
traffic adds up to slow sessions hacking kernels.)

This is only worthwhile if the target_read_partial usage conventions are
able to express it.  But I think that is a change worth making as well.
This goes to the issue with target_auxv_read that you cited.  If it must
make additional target_read_partial requests until one returns zero, then
all target implementations necessarily have their analogues of the remote
protocol's extra packet here.  To wit, the Linux implementation would do
open+read+close+open+lseek+read+close instead of just open+read+close.
That can obviously be internally optimized to make the worst case be
open+pread+pread+close, but that is still one more system call than is
really required to answer the whole question the caller of
target_read_partial is asking.

I think these improvements are worth pursuing without delay.  However, I
don't see any reason not to go ahead with the protocol you have proposed in
the interim, since I can implement that in a few minutes without perturbing
anything else.  

> -> qPartial:<obj>?
> 	<- OK -- queries of type <obj> are supported
> 	<- "" -- queries of type <obj> are not supported

What does "supported" here mean exactly?  A stub will e.g. support reading
but not writing, and a very constrained range of "annex" values.  If an OK
response to "qPartial:foo?" just means that you have some chance if you try
some particular read/write query, is there a particular benefit to that
over just trying the first query you wanted and taking the empty reply as
"not supported"?

> a variation being something like:
> qPartial:obj=<obj>:op=write[:annex=<x-annex>]:offset=<x-offset>:data=<x-data>
> which makes it more extensible.

That is not really any more extensible, I'd say.  You can just define new
tokens after the first or second : to change the details later.  This is
just more verbose, unless it's free form as to the order of parameters.
Being free form is less desireable because it requires hairy parsing in the
stub instead of just matching fixed leading strings for the few particular
requests that are actually supported.


Thanks,
Roland

Re: remote protocol support for TARGET_OBJECT_AUXV

[Andrew Cagney]

> I think these improvements are worth pursuing without delay.  However, I
> don't see any reason not to go ahead with the protocol you have proposed in
> the interim, since I can implement that in a few minutes without perturbing
> anything else.  

OK.  Once we've seen the numbers we can persue this.

>> -> qPartial:<obj>?
>> 	<- OK -- queries of type <obj> are supported
>> 	<- "" -- queries of type <obj> are not supported

> What does "supported" here mean exactly?  A stub will e.g. support reading
> but not writing, and a very constrained range of "annex" values.  If an OK
> response to "qPartial:foo?" just means that you have some chance if you try
> some particular read/write query, is there a particular benefit to that
> over just trying the first query you wanted and taking the empty reply as
> "not supported"?

Good question.  In the past GDB has encountered situtations where it 
needed to differentiate between the questions:
	- operation not supported
	- error during supported operation
for instance, with breakpoints, with the current protocol there's no 
well defined mechanism for determining if:
	-> Z2,0,0     or -> Z2
	<- E00
(insert watchpoint,addr,len) fails because:
	- 0,0 is an illegal addr,len
	- Z2 is, in that stub, considered an illegal Z packet
	- the operation is legal, but the hardware failed
The proposal is to fix it with:
	-> Z2?
	<- OK
so that GDB can explictly differentiate between:
	- hardware watchpoints not supported
	- error inserting hardware watchpoint
consequently, since then, care as been taken to define a "supported" 
query with new packets.

For here, the cases I can think of are:

- new gdb, old stub, packet not recognized
"" must always be returned

- new gdb, new stub, no /proc, or no read(write?) /proc access
Operation attempted, Enn returned

- new gdb, new stub, /proc
Operation attempted, valid response returned

so perhaphs clearly specifying this may prove sufficient.  I think that 
the main thing is that the spec has to be clear that:

	-> qPartial:asdfasdf:...
	<- ""

where "asdfasdf" is not "supported" must return "" and not "Enn".

>> a variation being something like:
>> qPartial:obj=<obj>:op=write[:annex=<x-annex>]:offset=<x-offset>:data=<x-data>
>> which makes it more extensible.
> 
> 
> That is not really any more extensible, I'd say.  You can just define new
> tokens after the first or second : to change the details later.  This is
> just more verbose, unless it's free form as to the order of parameters.
> Being free form is less desireable because it requires hairy parsing in the
> stub instead of just matching fixed leading strings for the few particular
> requests that are actually supported.

Should additional parameters be needed, it might help. However, yes, 
simply specifying a new packet is easier.

Andrew

PS: A contraction would be 'qPart:...".

Re: remote protocol support for TARGET_OBJECT_AUXV

[Roland McGrath]

> > I think these improvements are worth pursuing without delay.  However, I
> > don't see any reason not to go ahead with the protocol you have proposed in
> > the interim, since I can implement that in a few minutes without perturbing
> > anything else.  
> 
> OK.  Once we've seen the numbers we can persue this.

What numbers are you referring to?  We already know the number of system
calls, number of protocol transactions, number of protocol bytes, and such
numbers involved in having or not having EOF-detection optimized.

> For here, the cases I can think of are:
> 
> - new gdb, old stub, packet not recognized
> "" must always be returned
> 
> - new gdb, new stub, no /proc, or no read(write?) /proc access
> Operation attempted, Enn returned
> 
> - new gdb, new stub, /proc
> Operation attempted, valid response returned
> 
> so perhaphs clearly specifying this may prove sufficient.  I think that 
> the main thing is that the spec has to be clear that:
> 
> 	-> qPartial:asdfasdf:...
> 	<- ""
> 
> where "asdfasdf" is not "supported" must return "" and not "Enn".

The other case is new gdb, new stub, no stub target support.
i.e., a new gdbserver on a platform other than Linux 2.6 (until some more
gdbserver support is written).  I figure "" is the right response here,
since it's a "no sense even asking me until I'm recompiled" kind of failure.

The last set of wrinkles is

	-> qPart:auxv:write:...		(known obj, writing not supported)
	-> qPart:auxv:badop:...		(known obj, unknown op)
	-> qPart:auxv:read:foobar:...	(known obj+op, unexpected "annex")

I tend toward "" for all these as well.  If there is an obj+op where
"annex" is meaningful in general and is a value rather than a selection
among keywords (it's some kind of file name or something like that), then
that would be a case to return "Enn" for a bad one.

Here is proposed wording for a protocol based on your comments.
I have the implementation ready to go as well.


Thanks,
Roland


Index: gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.193
diff -u -b -p -r1.193 gdb.texinfo
--- gdb.texinfo	6 Feb 2004 15:48:25 -0000	1.193
+++ gdb.texinfo	12 Feb 2004 02:09:07 -0000
@@ -20422,6 +20422,76 @@ encoded).  @value{GDBN} will continue to
 (if available), until the target ceases to request them.
 @end table
 
+@item @code{qPart}:@var{object}:@code{read}:@var{annex}:@var{offset},@var{length} --- read special data
+
+Read uninterpreted bytes from the target's special data area
+identified by the keyword @code{object}.
+Request @var{length} bytes starting at @var{offset} bytes into the data.
+The content and encoding of @var{annex} is specific to the object;
+it can supply additional details about what data to access.
+The following table shows the @var{object} keywords in use so far.
+
+@table @code
+@item auxv
+Access the target's @dfn{auxiliary vector}.  @xref{Auxiliary Vector}.
+@var{annex} must be empty.
+@end table
+
+Reply:
+@table @samp
+@item OK
+The @var{offset} in the request is at the end of the data.
+There is no more data to be read.
+
+@item @var{XX@dots{}}
+Hex encoded data bytes read.
+This may be fewer bytes than the @var{length} in the request.
+
+@item @code{E}@var{NN}
+The request is bad, the offset is invalid,
+or there was an error encountered reading the data.
+
+@item @code{}
+An empty reply indicates the @var{object} or @var{annex} string was not
+recognized by the stub.
+@end table
+
+@item @code{qPart}:@var{object}:@code{write}:@var{annex}:@var{offset}:@var{data@dots{}}
+
+Write uninterpreted bytes into the target's special data area
+identified by the keyword @code{object},
+starting at @var{offset} bytes into the data.
+@var{data@dots{}} is the hex-encoded data to be written.
+The content and encoding of @var{annex} is specific to the object;
+it can supply additional details about what data to access.
+The following table shows the @var{object} keywords in use so far.
+
+@table @code
+@item auxv
+Access the target's @dfn{auxiliary vector}.  @xref{Auxiliary Vector}.
+@var{annex} must be empty.
+@end table
+
+Reply:
+@table @samp
+@item @var{NN}
+@var{NN} (hex encoded) is the number of bytes written.
+This may be fewer bytes than supplied in the request.
+
+@item @code{E}@var{NN}
+The request is bad, the offset is invalid,
+or there was an error encountered writing the data.
+
+@item @code{}
+An empty reply indicates the @var{object} or @var{annex} string was not
+recognized by the stub, or that the object does not support writing.
+@end table
+
+@item @code{qPart}:@var{object}:@var{operation}:@dots{}
+Requests of this form may be added in the future.  When a stub does
+not recognize the @var{object} keyword, or its support for
+@var{object} does not recognize the @var{operation} keyword,
+the stub must respond with an empty packet.
 @end table
 
 @node Register Packet Format

Re: remote protocol support for TARGET_OBJECT_AUXV

[Eli Zaretskii]

> Date: Wed, 11 Feb 2004 18:11:58 -0800
> From: Roland McGrath <roland@redhat.com>
> 
> Here is proposed wording for a protocol based on your comments.
> I have the implementation ready to go as well.

If the implementation is approved, here are my comments about the
documentation patch:

> +@item @code{E}@var{NN}
> +The request is bad, the offset is invalid,
> +or there was an error encountered reading the data.

I suggest using @var{nn} (lower-case) instead of @var{NN}, since that
should look better in the printed manual.  Also, please explain in
the text what does @var{nn} stand for; I assume it's a number that
tells what kind of error happened, but I think the manual shouldn't
leave that to reader's guesses.

> +@table @samp

I don't think @samp is appropriate here: it puts every item in
quotes, which is a bit ugly.  Your @item's already have the necessary
markup (@code etc.), so I'd suggest to use "@table @asis" instead.

> +@item @var{NN}
> +@var{NN} (hex encoded) is the number of bytes written.

For consistency, I'd suggest to use @var{XX} here, as you did in a
previous table.

> +@item @code{E}@var{NN}
> +The request is bad, the offset is invalid,
> +or there was an error encountered writing the data.

See the comment above about @var{NN}.

> +@item @code{}
> +An empty reply indicates the @var{object} or @var{annex} string was not
> +recognized by the stub, or that the object does not support writing.

The empty item is confusing and will look awkward in print, I think.
How about this instead:

  @item @code{""} (empty)

?

Thanks.

Re: remote protocol support for TARGET_OBJECT_AUXV

[Andrew Cagney]

> I think these improvements are worth pursuing without delay.  However, I
>> > don't see any reason not to go ahead with the protocol you have proposed in
>> > the interim, since I can implement that in a few minutes without perturbing
>> > anything else.  
> 
>> 
>> OK.  Once we've seen the numbers we can persue this.
> 
> 
> What numbers are you referring to?  We already know the number of system
> calls, number of protocol transactions, number of protocol bytes, and such
> numbers involved in having or not having EOF-detection optimized.

The numbers showing if the user can see a problem, and if so where it 
is.  Without that we're needlessly micro-optimizing in a fog.


>> For here, the cases I can think of are:
>> 
>> - new gdb, old stub, packet not recognized
>> "" must always be returned
>> 
>> - new gdb, new stub, no /proc, or no read(write?) /proc access
>> Operation attempted, Enn returned
>> 
>> - new gdb, new stub, /proc
>> Operation attempted, valid response returned
>> 
>> so perhaphs clearly specifying this may prove sufficient.  I think that 
>> the main thing is that the spec has to be clear that:
>> 
>> 	-> qPartial:asdfasdf:...
>> 	<- ""
>> 
>> where "asdfasdf" is not "supported" must return "" and not "Enn".
> 
> 
> The other case is new gdb, new stub, no stub target support.
> i.e., a new gdbserver on a platform other than Linux 2.6 (until some more
> gdbserver support is written).  I figure "" is the right response here,
> since it's a "no sense even asking me until I'm recompiled" kind of failure.
> 
> The last set of wrinkles is
> 
> 	-> qPart:auxv:write:...		(known obj, writing not supported)
> 	-> qPart:auxv:badop:...		(known obj, unknown op)
> 	-> qPart:auxv:read:foobar:...	(known obj+op, unexpected "annex")
> 
> I tend toward "" for all these as well.  If there is an obj+op where
> "annex" is meaningful in general and is a value rather than a selection
> among keywords (it's some kind of file name or something like that), then
> that would be a case to return "Enn" for a bad one.

Ok.

> Here is proposed wording for a protocol based on your comments.
> I have the implementation ready to go as well.

Given the above, here:

> +The following table shows the @var{object} keywords in use so far.
> +
> +@table @code
> +@item auxv

spell out the entire packet vis:

@item @code{qPart}:@code{auxv}:@code{read}::@var{offset},@var{length}

so that its' clear that the annex is required to be empty.

Eli fyi,

The @var{NN} was picked up from other parts, and the lack of definition 
of @var{nn} is a long standing problem.

Roland suggest defining "@var{nn}" as "errno", we'll leave it to later 
to more formally define it as the host/target's errno (I suspect that it 
is the target).  And E00 for a syntax error.

Andrew

> +@item @code{E}@var{NN}
> +The request is bad, the offset is invalid,
> +or there was an error encountered reading the data.
> +
> +@item @code{}
> +An empty reply indicates the @var{object} or @var{annex} string was not
> +recognized by the stub.
> +@end table
> +
> +@item @code{qPart}:@var{object}:@code{write}:@var{annex}:@var{offset}:@var{data@dots{}}
> +
> +Write uninterpreted bytes into the target's special data area
> +identified by the keyword @code{object},
> +starting at @var{offset} bytes into the data.
> +@var{data@dots{}} is the hex-encoded data to be written.
> +The content and encoding of @var{annex} is specific to the object;
> +it can supply additional details about what data to access.
> +The following table shows the @var{object} keywords in use so far.
> +
> +@table @code
> +@item auxv
> +Access the target's @dfn{auxiliary vector}.  @xref{Auxiliary Vector}.
> +@var{annex} must be empty.
> +@end table
> +
> +Reply:
> +@table @samp
> +@item @var{NN}
> +@var{NN} (hex encoded) is the number of bytes written.
> +This may be fewer bytes than supplied in the request.
> +
> +@item @code{E}@var{NN}
> +The request is bad, the offset is invalid,
> +or there was an error encountered writing the data.
> +
> +@item @code{}
> +An empty reply indicates the @var{object} or @var{annex} string was not
> +recognized by the stub, or that the object does not support writing.
> +@end table
> +
> +@item @code{qPart}:@var{object}:@var{operation}:@dots{}
> +Requests of this form may be added in the future.  When a stub does
> +not recognize the @var{object} keyword, or its support for
> +@var{object} does not recognize the @var{operation} keyword,
> +the stub must respond with an empty packet.
>  @end table
>  
>  @node Register Packet Format
> 


Andrew

Re: remote protocol support for TARGET_OBJECT_AUXV

[Roland McGrath]

> I suggest using @var{nn} (lower-case) instead of @var{NN}, since that
> should look better in the printed manual.  Also, please explain in
> the text what does @var{nn} stand for; I assume it's a number that
> tells what kind of error happened, but I think the manual shouldn't
> leave that to reader's guesses.

I copied this part of the documentation, and protocol behavior, from the
other existing protocol requests.  As far as I can tell, they are all
underspecified.  What gdbserver seems to do is actually write "ENN" (two
literal 'N's).  So go figure.  I'd be happy to change my additions to
reference a standard explanation of what error packets really look like.
But the mess was here before me and I'm not the one who knows how it ought
to be different.

> > +@table @samp
> 
> I don't think @samp is appropriate here: it puts every item in
> quotes, which is a bit ugly.  Your @item's already have the necessary
> markup (@code etc.), so I'd suggest to use "@table @asis" instead.

Again, just copied another similar place in gdb.texinfo.  I'll happily
change this as you say.  I think the manual may already be inconsistent
with itself in the formatting style for packet text.

> > +@item @code{E}@var{NN}
> > +The request is bad, the offset is invalid,
> > +or there was an error encountered writing the data.
> 
> See the comment above about @var{NN}.

I've change dthese to @code{E}@var{nn} because you said so.
But every other existing appearance is @code{E}@var{NN}, what I copied.

> The empty item is confusing and will look awkward in print, I think.
> How about this instead:
> 
>   @item @code{""} (empty)

Whatever you say.  @code{} was in Andrew's example, so I copied it.

Re: remote protocol support for TARGET_OBJECT_AUXV

[Roland McGrath]

> spell out the entire packet vis:
> 
> @item @code{qPart}:@code{auxv}:@code{read}::@var{offset},@var{length}
> 
> so that its' clear that the annex is required to be empty.

That was the purpose of the sentence "@var{annex} must be empty."
But I've changed it as you asked.

Here is a new documentation patch.  If this is satisfactory, I have the
patches implementing it ready for submission as well.


Thanks,
Roland

Index: gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.196
diff -u -b -p -r1.196 gdb.texinfo
--- gdb.texinfo	24 Feb 2004 15:41:29 -0000	1.196
+++ gdb.texinfo	24 Feb 2004 23:28:29 -0000
@@ -20422,6 +20422,82 @@ encoded).  @value{GDBN} will continue to
 (if available), until the target ceases to request them.
 @end table
 
+@item @code{qPart}:@var{object}:@code{read}:@var{annex}:@var{offset},@var{length} --- read special data
+
+Read uninterpreted bytes from the target's special data area
+identified by the keyword @code{object}.
+Request @var{length} bytes starting at @var{offset} bytes into the data.
+The content and encoding of @var{annex} is specific to the object;
+it can supply additional details about what data to access.
+
+Here are the specific requests of this form defined so far.
+All @samp{@code{qPart}:@var{object}:@code{read}:@dots{}}
+requests use the same reply formats, listed below.
+
+@table @asis
+@item @code{qPart}:@code{auxv}:@code{read}::@var{offset},@var{length}
+Access the target's @dfn{auxiliary vector}.  @xref{Auxiliary Vector}.
+Note @var{annex} must be empty.
+@end table
+
+Reply:
+@table @asis
+@item @code{OK}
+The @var{offset} in the request is at the end of the data.
+There is no more data to be read.
+
+@item @var{XX@dots{}}
+Hex encoded data bytes read.
+This may be fewer bytes than the @var{length} in the request.
+
+@item @code{E00}
+The request was malformed, or @var{annex} was invalid.
+
+@item @code{E}@var{nn}
+The offset was invalid, or there was an error encountered reading the data.
+@var{nn} is a hex-encoded @code{errno} value.
+
+@item @code{""} (empty)
+An empty reply indicates the @var{object} or @var{annex} string was not
+recognized by the stub.
+@end table
+
+@item @code{qPart}:@var{object}:@code{write}:@var{annex}:@var{offset}:@var{data@dots{}}
+
+Write uninterpreted bytes into the target's special data area
+identified by the keyword @code{object},
+starting at @var{offset} bytes into the data.
+@var{data@dots{}} is the hex-encoded data to be written.
+The content and encoding of @var{annex} is specific to the object;
+it can supply additional details about what data to access.
+
+No requests of this form are presently in use.  This specification
+serves as a placeholder to document the common format that new
+specific request specifications ought to use.
+
+Reply:
+@table @asis
+@item @var{nn}
+@var{nn} (hex encoded) is the number of bytes written.
+This may be fewer bytes than supplied in the request.
+
+@item @code{E00}
+The request was malformed, or @var{annex} was invalid.
+
+@item @code{E}@var{nn}
+The offset was invalid, or there was an error encountered writing the data.
+@var{nn} is a hex-encoded @code{errno} value.
+
+@item @code{""} (empty)
+An empty reply indicates the @var{object} or @var{annex} string was not
+recognized by the stub, or that the object does not support writing.
+@end table
+
+@item @code{qPart}:@var{object}:@var{operation}:@dots{}
+Requests of this form may be added in the future.  When a stub does
+not recognize the @var{object} keyword, or its support for
+@var{object} does not recognize the @var{operation} keyword,
+the stub must respond with an empty packet.
 @end table
 
 @node Register Packet Format

Re: remote protocol support for TARGET_OBJECT_AUXV

[Eli Zaretskii]

> Date: Tue, 24 Feb 2004 15:21:29 -0800
> From: Roland McGrath <roland@redhat.com>
> 
> > I suggest using @var{nn} (lower-case) instead of @var{NN}, since that
> > should look better in the printed manual.  Also, please explain in
> > the text what does @var{nn} stand for; I assume it's a number that
> > tells what kind of error happened, but I think the manual shouldn't
> > leave that to reader's guesses.
> 
> I copied this part of the documentation, and protocol behavior, from the
> other existing protocol requests.  As far as I can tell, they are all
> underspecified.  What gdbserver seems to do is actually write "ENN" (two
> literal 'N's).  So go figure.  I'd be happy to change my additions to
> reference a standard explanation of what error packets really look like.

If gdbserver prints a literal "ENN", then @var is wrong.

Does anybody know what's the story here, why ENN is printed and what
it should be?  Is this perhaps a bug?

> I think the manual may already be inconsistent with itself in the
> formatting style for packet text.

True.  We are trying to fix the inconsistencies as we find them, but
there's still a lot of work.

Re: remote protocol support for TARGET_OBJECT_AUXV

[Eli Zaretskii]

> Date: Tue, 24 Feb 2004 15:29:34 -0800
> From: Roland McGrath <roland@redhat.com>
> 
> Here is a new documentation patch.  If this is satisfactory, I have the
> patches implementing it ready for submission as well.

Thanks, this patch is okay with me.

Re: remote protocol support for TARGET_OBJECT_AUXV

[Daniel Jacobowitz]

On Wed, Feb 25, 2004 at 07:48:42AM +0200, Eli Zaretskii wrote:
> > Date: Tue, 24 Feb 2004 15:21:29 -0800
> > From: Roland McGrath <roland@redhat.com>
> > 
> > > I suggest using @var{nn} (lower-case) instead of @var{NN}, since that
> > > should look better in the printed manual.  Also, please explain in
> > > the text what does @var{nn} stand for; I assume it's a number that
> > > tells what kind of error happened, but I think the manual shouldn't
> > > leave that to reader's guesses.
> > 
> > I copied this part of the documentation, and protocol behavior, from the
> > other existing protocol requests.  As far as I can tell, they are all
> > underspecified.  What gdbserver seems to do is actually write "ENN" (two
> > literal 'N's).  So go figure.  I'd be happy to change my additions to
> > reference a standard explanation of what error packets really look like.
> 
> If gdbserver prints a literal "ENN", then @var is wrong.
> 
> Does anybody know what's the story here, why ENN is printed and what
> it should be?  Is this perhaps a bug?

It predates my work with gdbserver.  My guess is that someone saw ENN
in the manual, realized that GDB didn't parse the error numbers to do
anything useful, and decided not to bother coming up with some.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: remote protocol support for TARGET_OBJECT_AUXV

[Eli Zaretskii]

> Date: Wed, 25 Feb 2004 09:34:16 -0500
> From: Daniel Jacobowitz <drow@false.org>
> > 
> > Does anybody know what's the story here, why ENN is printed and what
> > it should be?  Is this perhaps a bug?
> 
> It predates my work with gdbserver.  My guess is that someone saw ENN
> in the manual, realized that GDB didn't parse the error numbers to do
> anything useful, and decided not to bother coming up with some.

If so, I think we should simply remove the ENN thing (and update the
docs accordingly).  It doesn't make sense, IMHO, to print a string
that has no useful meaning.

Do you agree?

Re: remote protocol support for TARGET_OBJECT_AUXV

[Andrew Cagney]

>>Date: Wed, 25 Feb 2004 09:34:16 -0500
>>> From: Daniel Jacobowitz <drow@false.org>
>>
>>>> > 
>>>> > Does anybody know what's the story here, why ENN is printed and what
>>>> > it should be?  Is this perhaps a bug?
>>
>>> 
>>> It predates my work with gdbserver.  My guess is that someone saw ENN
>>> in the manual, realized that GDB didn't parse the error numbers to do
>>> anything useful, and decided not to bother coming up with some.
> 
> 
> If so, I think we should simply remove the ENN thing (and update the
> docs accordingly).  It doesn't make sense, IMHO, to print a string
> that has no useful meaning.

Er, no.  "Enn" as a reply packet is a fundamental part of the protocol 
and can't be removed.  Whats lacking is the formal specification of its 
contents.  For the moment I'd leave that part of roland's doco as is.

Andrew

Re: remote protocol support for TARGET_OBJECT_AUXV

[Daniel Jacobowitz]

On Wed, Feb 25, 2004 at 10:53:57AM -0500, Andrew Cagney wrote:
> >>Date: Wed, 25 Feb 2004 09:34:16 -0500
> >>>From: Daniel Jacobowitz <drow@false.org>
> >>
> >>>>> 
> >>>>> Does anybody know what's the story here, why ENN is printed and what
> >>>>> it should be?  Is this perhaps a bug?
> >>
> >>>
> >>>It predates my work with gdbserver.  My guess is that someone saw ENN
> >>>in the manual, realized that GDB didn't parse the error numbers to do
> >>>anything useful, and decided not to bother coming up with some.
> >
> >
> >If so, I think we should simply remove the ENN thing (and update the
> >docs accordingly).  It doesn't make sense, IMHO, to print a string
> >that has no useful meaning.
> 
> Er, no.  "Enn" as a reply packet is a fundamental part of the protocol 
> and can't be removed.  Whats lacking is the formal specification of its 
> contents.  For the moment I'd leave that part of roland's doco as is.

Yeah, I agree.  Eli, it does have a useful meaning: it means that an
error occured.  It just neglects to tell us _what_ :)

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: remote protocol support for TARGET_OBJECT_AUXV

[Andrew Cagney]

>>Date: Tue, 24 Feb 2004 15:29:34 -0800
>>> From: Roland McGrath <roland@redhat.com>
>>> 
>>> Here is a new documentation patch.  If this is satisfactory, I have the
>>> patches implementing it ready for submission as well.
> 
> 
> Thanks, this patch is okay with me.

Ditto.

Andrew

Re: remote protocol support for TARGET_OBJECT_AUXV

[Eli Zaretskii]

> Date: Wed, 25 Feb 2004 11:06:15 -0500
> From: Daniel Jacobowitz <drow@false.org>
> > 
> > Er, no.  "Enn" as a reply packet is a fundamental part of the protocol 
> > and can't be removed.  Whats lacking is the formal specification of its 
> > contents.  For the moment I'd leave that part of roland's doco as is.
> 
> Yeah, I agree.  Eli, it does have a useful meaning: it means that an
> error occured.  It just neglects to tell us _what_ :)

Sorry, I must be misunderstanding the issue.  So, once again, could
someone please explain to my confused self what is the meaning of "NN"
in "ENN"?

Re: remote protocol support for TARGET_OBJECT_AUXV

[Daniel Jacobowitz]

On Thu, Feb 26, 2004 at 08:15:00AM +0200, Eli Zaretskii wrote:
> > Date: Wed, 25 Feb 2004 11:06:15 -0500
> > From: Daniel Jacobowitz <drow@false.org>
> > > 
> > > Er, no.  "Enn" as a reply packet is a fundamental part of the protocol 
> > > and can't be removed.  Whats lacking is the formal specification of its 
> > > contents.  For the moment I'd leave that part of roland's doco as is.
> > 
> > Yeah, I agree.  Eli, it does have a useful meaning: it means that an
> > error occured.  It just neglects to tell us _what_ :)
> 
> Sorry, I must be misunderstanding the issue.  So, once again, could
> someone please explain to my confused self what is the meaning of "NN"
> in "ENN"?

Theoretically, it ought to be an error code.  i.e. it ought to tell GDB
what has gone wrong.

But GDB doesn't interpret the error code.  I just skimmed remote.c; in
some places it checks for starts-with-E; in some it checks for 'E' and
length 3; in some it checks for 'E' and length 3 and digits; in some it
checks for 'E' and length 3 and hex digits.  The last is, I think,
correct.

In any case, you're right that it ought to be corrected.  I'm not going
to correct it to the full extent of defining error codes, but I will
make it return E01 for all errors instead of ENN; that should fix a bug
I noticed yesterday.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: remote protocol support for TARGET_OBJECT_AUXV

[Nathan J. Williams]

Daniel Jacobowitz <drow@false.org> writes:

> But GDB doesn't interpret the error code.  I just skimmed remote.c; in
> some places it checks for starts-with-E; in some it checks for 'E' and
> length 3; in some it checks for 'E' and length 3 and digits; in some it
> checks for 'E' and length 3 and hex digits.  The last is, I think,
> correct.

I noticed this in some recent work. My approach to fixing it was to
make remote.c in GDB only check for 'E..' (not hex digits) and to make
gdbserver return E## (hex digits), for best compatibility between
gdb and gdbservers of differing vintage.

        - Nathan

Re: remote protocol support for TARGET_OBJECT_AUXV

[Eli Zaretskii]

> Date: Thu, 26 Feb 2004 09:57:26 -0500
> From: Daniel Jacobowitz <drow@false.org>
> 
> In any case, you're right that it ought to be corrected.  I'm not going
> to correct it to the full extent of defining error codes, but I will
> make it return E01 for all errors instead of ENN

Thank you.  And thanks for taking time to explain the issue.