diff mbox

[RFA] Small improvements to the remote protocol manual

Message ID 1473819514-18403-1-git-send-email-tom@tromey.com
State New
Headers show

Commit Message

Tom Tromey Sept. 14, 2016, 2:18 a.m. UTC
I was reading the gdb RSP manual recently and I found a number of
small problems in the documentation.  This patch attempts to improve
these areas.  Specfically:

* The term "memory breakpoint" is used only in this section of the
  manual, and there inconsistently.  I found this term confusing --
  initially I thought it might be a watchpoint.  This patch changes it
  to use the term "software breakpoint", which is used in the rest of
  the manual.

* The z0 packet didn't document how "kind" was written.  And, it had a
  stray link to the architecture-specific protocol details node.  This
  patch moves this link to a better spot.

* The z1 patch didn't document that it accepts cmd_list.

* I couldn't find any text saying what response is given to a command
  like vCont in non-stop mode.  The answer is that OK is sent, and
  then a stop reply is sent as a notification.  This patch adds a note
  about this.

* The "create" stop reply did not document that the "R" argument is
  ignored.

* The "W", "X", and "w" packets did not document how the "AA" part is
  formatted.

* The %Stop notification example said "%%Stop", but I think this is
  incorrect.

2016-09-13  Tom Tromey  <tom@tromey.com>

	* gdb.texinfo (Packets) <z0>: Use "software breakpoint" rather
	than "memory breakpoint".  Further document "kind".  Move
	protocol-details link earlier.
	<z1>: Document the cmd_list argument.  Fix typo.
	<g>: Remove incorrect sentence.
	(Stop Reply Packets): Document "OK" response to requests when in
	non-stop mode.
	<swbreak>: Use "software breakpoint" rather than "memory
	breakpoint".
	<create>: Document that "R" is ignored.
	<W, X, w>: Document formatting of "AA".
	(Notification Packets): Use "%Stop", not "%%Stop".
---
 gdb/doc/ChangeLog   | 15 +++++++++++++
 gdb/doc/gdb.texinfo | 63 ++++++++++++++++++++++++++++++-----------------------
 2 files changed, 51 insertions(+), 27 deletions(-)

Comments

Tom Tromey Sept. 14, 2016, 10:58 p.m. UTC | #1
>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Tom> I was reading the gdb RSP manual recently and I found a number of
Tom> small problems in the documentation.  This patch attempts to improve
Tom> these areas.

Also, I forgot to note here that I didn't understand the documentation
for the "A" packet; but when I looked at the code I couldn't find it in
either gdb or gdbserver.  So, I wonder if it ought to be deleted.

Another oddity I noticed is that it's unclear what a client ought to do
if a stub sends a notification that it doesn't understand.  The ACK for
a notification depends on the contents (which implies that they've been
understood), and there doesn't seem to be a general notification NAK.  I
think this might be a useful addition.

Tom
Yao Qi Sept. 15, 2016, 4:09 p.m. UTC | #2
On Wed, Sep 14, 2016 at 11:58 PM, Tom Tromey <tom@tromey.com> wrote:
> Also, I forgot to note here that I didn't understand the documentation
> for the "A" packet; but when I looked at the code I couldn't find it in
> either gdb or gdbserver.  So, I wonder if it ought to be deleted.
>

The "A" packet was added in 1999 as a "reserved" packet,

+@item set program arguments @strong{(reserved)} @emph{(optional)}
+@tab @code{A}@var{arglen}@code{,}@var{argnum}@code{,}@var{arg}@code{,...}

but the "reserved" marker is removed in
b8ff78cefa335846b4303f303847a4d69e652795 (2005),

        - Remove "(reserved)" markers on 'A' and 'I' packets; it's unclear
          what this ever meant.

Nowadays, we still can't pass arguments in remote debugging.  If "A" still
meets our needs, we can use it in the future to pass arguments.
Tom Tromey Sept. 15, 2016, 4:14 p.m. UTC | #3
>>>>> "Yao" == Yao Qi <qiyaoltc@gmail.com> writes:

Yao> Nowadays, we still can't pass arguments in remote debugging.  If "A" still
Yao> meets our needs, we can use it in the future to pass arguments.

I didn't understand the current documentation for the "A" packet.  If
someone can explain it, I'd be happy to write it up.

However - I think vRun supports arguments.  See extended_remote_run.
So maybe "A" isn't needed at all.

Tom
Tom Tromey Sept. 27, 2016, 4:43 p.m. UTC | #4
>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Tom> I was reading the gdb RSP manual recently and I found a number of
Tom> small problems in the documentation.  This patch attempts to improve
Tom> these areas.

Ping.

Tom
Eli Zaretskii Sept. 27, 2016, 5:43 p.m. UTC | #5
> From: Tom Tromey <tom@tromey.com>
> Cc: gdb-patches@sourceware.org
> Date: Tue, 27 Sep 2016 10:43:54 -0600
> 
> >>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
> 
> Tom> I was reading the gdb RSP manual recently and I found a number of
> Tom> small problems in the documentation.  This patch attempts to improve
> Tom> these areas.
> 
> Ping.

I'm sorry, but the original patch was followed by discussions and
suggestions for changes/improvements, so I concluded the jury was
still out on this one.  Could you send the final version of the patch?

Thanks.
Tom Tromey Sept. 27, 2016, 6:06 p.m. UTC | #6
>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

Eli> I'm sorry, but the original patch was followed by discussions and
Eli> suggestions for changes/improvements, so I concluded the jury was
Eli> still out on this one.  Could you send the final version of the patch?

There was some minor discussion about questions I had but AFAICT no
requests for changes; so I haven't made any.

I think dealing with the "A" packet or the notification oddities, if
necessary, can be done in a follow-up.  I still don't know what to do
here.

Tom
Eli Zaretskii Sept. 27, 2016, 7:39 p.m. UTC | #7
> From: Tom Tromey <tom@tromey.com>
> Cc: Tom Tromey <tom@tromey.com>,  gdb-patches@sourceware.org
> Date: Tue, 27 Sep 2016 12:06:02 -0600
> 
> There was some minor discussion about questions I had but AFAICT no
> requests for changes; so I haven't made any.
> 
> I think dealing with the "A" packet or the notification oddities, if
> necessary, can be done in a follow-up.  I still don't know what to do
> here.

OK.

Apart of 2 typos ("px@ref", "@var{GDBN}") and the use of @ref instead
of @pxref in parentheses, I have no other comments to the patch.

Thanks, and sorry for the delay in reviewing.
Tom Tromey Sept. 28, 2016, 5:04 p.m. UTC | #8
>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

Eli> Apart of 2 typos ("px@ref", "@var{GDBN}") and the use of @ref instead
Eli> of @pxref in parentheses, I have no other comments to the patch.

I'm happy to report that 2 out of 3 of these were pre-existing :)

Eli> Thanks, and sorry for the delay in reviewing.

No problem.  I've fixed up the errors and I'm going to check it in shortly.

Tom
Pedro Alves Dec. 2, 2016, 11:58 a.m. UTC | #9
On 09/14/2016 11:58 PM, Tom Tromey wrote:

> Another oddity I noticed is that it's unclear what a client ought to do
> if a stub sends a notification that it doesn't understand.  

The idea so far was that that shouldn't happen, so it's undefined.
The client should be able to tell whether a client supports some feature
that requires some notification, based on the initial qSupported negotiation,
or on some mode (such as non-stop) being explicitly activated by the
client.

> The ACK for
> a notification depends on the contents (which implies that they've been
> understood), and there doesn't seem to be a general notification NAK.  I
> think this might be a useful addition.

Thanks,
Pedro Alves
Pedro Alves Dec. 2, 2016, 11:59 a.m. UTC | #10
Took me this long to get to reading this email...

On 09/14/2016 03:18 AM, Tom Tromey wrote:

> @@ -36047,10 +36053,11 @@ also the @samp{w} (@ref{thread exit event}) remote reply below.
>  The process exited, and @var{AA} is the exit status.  This is only
>  applicable to certain targets.
>  
> -The second form of the response, including the process ID of the exited
> -process, can be used only when @value{GDBN} has reported support for
> -multiprocess protocol extensions; see @ref{multiprocess extensions}.
> -The @var{pid} is formatted as a big-endian hex string.
> +The second form of the response, including the process ID of the
> +exited process, can be used only when @value{GDBN} has reported
> +support for multiprocess protocol extensions; see @ref{multiprocess
> +extensions}.  Both @var{AA} and @var{pid} are formatted as big-endian
> +hex strings.

For the record, note that:

- the format of @var{AA} used to be exactly two hex chars:

	/* GDB used to accept only 2 hex chars here.  Stubs should
	   only send more if they detect GDB supports multi-process
	   support.  */
	p = unpack_varlen_hex (&buf[1], &value);

- The RSP overview states that "Except where otherwise noted all numbers
  are represented in hex with leading zeros suppressed."

I've pondered before about removing all the explicit mentions of hex
encoding, in order to make the exceptions stand out.  My idea would be
that if packets don't mention the formatting of numbers, then people
will naturally look for that in general packet formatting description.

But in the current state of the manual, it certainly doesn't hurt to
be explicit.

Thanks for the fixes.

Thanks,
Pedro Alves
diff mbox

Patch

diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index c6fe47c..63c4f4b 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,18 @@ 
+2016-09-13  Tom Tromey  <tom@tromey.com>
+
+	* gdb.texinfo (Packets) <z0>: Use "software breakpoint" rather
+	than "memory breakpoint".  Further document "kind".  Move
+	protocol-details link earlier.
+	<z1>: Document the cmd_list argument.  Fix typo.
+	<g>: Remove incorrect sentence.
+	(Stop Reply Packets): Document "OK" response to requests when in
+	non-stop mode.
+	<swbreak>: Use "software breakpoint" rather than "memory
+	breakpoint".
+	<create>: Document that "R" is ignored.
+	<W, X, w>: Document formatting of "AA".
+	(Notification Packets): Use "%Stop", not "%%Stop".
+
 2016-08-24  Simon Marchi  <simon.marchi@ericsson.com>
 
 	* gdb.texinfo (Input/Output): Mention possibility to unset
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index d1a5e7c..5e2caf0 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -35225,8 +35225,7 @@  Each byte of register data is described by two hex digits.  The bytes
 with the register are transmitted in target byte order.  The size of
 each register and their position within the @samp{g} packet are
 determined by the @value{GDBN} internal gdbarch functions
-@code{DEPRECATED_REGISTER_RAW_SIZE} and @code{gdbarch_register_name}.  The
-specification of several standard @samp{g} packets is specified below.
+@code{DEPRECATED_REGISTER_RAW_SIZE} and @code{gdbarch_register_name}.
 
 When reading registers from a trace frame (@pxref{Analyze Collected
 Data,,Using the Collected Data}), the stub may also return a string of
@@ -35723,22 +35722,25 @@  be implemented in an idempotent way.}
 @itemx Z0,@var{addr},@var{kind}@r{[};@var{cond_list}@dots{}@r{]}@r{[};cmds:@var{persist},@var{cmd_list}@dots{}@r{]}
 @cindex @samp{z0} packet
 @cindex @samp{Z0} packet
-Insert (@samp{Z0}) or remove (@samp{z0}) a memory breakpoint at address
+Insert (@samp{Z0}) or remove (@samp{z0}) a software breakpoint at address
 @var{addr} of type @var{kind}.
 
-A memory breakpoint is implemented by replacing the instruction at
+A software breakpoint is implemented by replacing the instruction at
 @var{addr} with a software breakpoint or trap instruction.  The
-@var{kind} is target-specific and typically indicates the size of
-the breakpoint in bytes that should be inserted.  E.g., the @sc{arm}
-and @sc{mips} can insert either a 2 or 4 byte breakpoint.  Some
-architectures have additional meanings for @var{kind};
-@var{cond_list} is an optional list of conditional expressions in bytecode
-form that should be evaluated on the target's side.  These are the
-conditions that should be taken into consideration when deciding if
-the breakpoint trigger should be reported back to @var{GDBN}.
+@var{kind} is target-specific and typically indicates the size of the
+breakpoint in bytes that should be inserted.  E.g., the @sc{arm} and
+@sc{mips} can insert either a 2 or 4 byte breakpoint.  Some
+architectures have additional meanings for @var{kind}
+(px@ref{Architecture-Specific Protocol Details}); if no
+architecture-specific value is being used, it should be @samp{0}.
+@var{kind} is hex-encoded.  @var{cond_list} is an optional list of
+conditional expressions in bytecode form that should be evaluated on
+the target's side.  These are the conditions that should be taken into
+consideration when deciding if the breakpoint trigger should be
+reported back to @var{GDBN}.
 
 See also the @samp{swbreak} stop reason (@pxref{swbreak stop reason})
-for how to best report a memory breakpoint event to @value{GDBN}.
+for how to best report a software breakpoint event to @value{GDBN}.
 
 The @var{cond_list} parameter is comprised of a series of expressions,
 concatenated without separators. Each expression has the following form:
@@ -35767,10 +35769,8 @@  actual conditional expression in bytecode form.
 
 @end table
 
-see @ref{Architecture-Specific Protocol Details}.
-
 @emph{Implementation note: It is possible for a target to copy or move
-code that contains memory breakpoints (e.g., when implementing
+code that contains software breakpoints (e.g., when implementing
 overlays).  The behavior of this packet, in the presence of such a
 target, is not defined.}
 
@@ -35785,15 +35785,16 @@  for an error
 @end table
 
 @item z1,@var{addr},@var{kind}
-@itemx Z1,@var{addr},@var{kind}@r{[};@var{cond_list}@dots{}@r{]}
+@itemx Z1,@var{addr},@var{kind}@r{[};@var{cond_list}@dots{}@r{]}@r{[};cmds:@var{persist},@var{cmd_list}@dots{}@r{]}
 @cindex @samp{z1} packet
 @cindex @samp{Z1} packet
 Insert (@samp{Z1}) or remove (@samp{z1}) a hardware breakpoint at
 address @var{addr}.
 
 A hardware breakpoint is implemented using a mechanism that is not
-dependant on being able to modify the target's memory.  The @var{kind}
-and @var{cond_list} have the same meaning as in @samp{Z0} packets.
+dependent on being able to modify the target's memory.  The
+@var{kind}, @var{cond_list}, and @var{cmd_list} arguments have the
+same meaning as in @samp{Z0} packets.
 
 @emph{Implementation note: A hardware breakpoint is not affected by code
 movement.}
@@ -35873,6 +35874,10 @@  when the target halts.  In the below the exact meaning of @dfn{signal
 number} is defined by the header @file{include/gdb/signals.h} in the
 @value{GDBN} source code.
 
+In non-stop mode, the server will simply reply @samp{OK} to commands
+such as @samp{vCont}; any stop will be the subject of a future
+notification.  @xref{Remote Non-Stop}.
+
 As in the description of request packets, we include spaces in the
 reply templates for clarity; these are not part of the reply packet's
 syntax.  No @value{GDBN} stop reply packet uses spaces to separate its
@@ -35951,7 +35956,7 @@  for more information.
 
 @item swbreak
 @anchor{swbreak stop reason}
-The packet indicates a memory breakpoint instruction was executed,
+The packet indicates a software breakpoint instruction was executed,
 irrespective of whether it was @value{GDBN} that planted the
 breakpoint or the breakpoint is hardcoded in the program.  The @var{r}
 part must be left empty.
@@ -36038,7 +36043,8 @@  The packet indicates that the thread was just created.  The new thread
 is stopped until @value{GDBN} sets it running with a resumption packet
 (@pxref{vCont packet}).  This packet should not be sent by default;
 @value{GDBN} requests it with the @ref{QThreadEvents} packet.  See
-also the @samp{w} (@ref{thread exit event}) remote reply below.
+also the @samp{w} (@ref{thread exit event}) remote reply below.  The
+@var{r} part is ignored.
 
 @end table
 
@@ -36047,10 +36053,11 @@  also the @samp{w} (@ref{thread exit event}) remote reply below.
 The process exited, and @var{AA} is the exit status.  This is only
 applicable to certain targets.
 
-The second form of the response, including the process ID of the exited
-process, can be used only when @value{GDBN} has reported support for
-multiprocess protocol extensions; see @ref{multiprocess extensions}.
-The @var{pid} is formatted as a big-endian hex string.
+The second form of the response, including the process ID of the
+exited process, can be used only when @value{GDBN} has reported
+support for multiprocess protocol extensions; see @ref{multiprocess
+extensions}.  Both @var{AA} and @var{pid} are formatted as big-endian
+hex strings.
 
 @item X @var{AA}
 @itemx X @var{AA} ; process:@var{pid}
@@ -36059,7 +36066,8 @@  The process terminated with signal @var{AA}.
 The second form of the response, including the process ID of the
 terminated process, can be used only when @value{GDBN} has reported
 support for multiprocess protocol extensions; see @ref{multiprocess
-extensions}.  The @var{pid} is formatted as a big-endian hex string.
+extensions}.  Both @var{AA} and @var{pid} are formatted as big-endian
+hex strings.
 
 @anchor{thread exit event}
 @cindex thread exit event, remote reply
@@ -36068,6 +36076,7 @@  extensions}.  The @var{pid} is formatted as a big-endian hex string.
 The thread exited, and @var{AA} is the exit status.  This response
 should not be sent by default; @value{GDBN} requests it with the
 @ref{QThreadEvents} packet.  See also @ref{thread create event} above.
+@var{AA} is formatted as a big-endian hex string.
 
 @item N
 There are no resumed threads left in the target.  In other words, even
@@ -38639,7 +38648,7 @@  the process shall be repeated.
 The process of asynchronous notification can be illustrated by the
 following example:
 @smallexample
-<- @code{%%Stop:T0505:98e7ffbf;04:4ce6ffbf;08:b1b6e54c;thread:p7526.7526;core:0;}
+<- @code{%Stop:T0505:98e7ffbf;04:4ce6ffbf;08:b1b6e54c;thread:p7526.7526;core:0;}
 @code{...}
 -> @code{vStopped}
 <- @code{T0505:68f37db7;04:40f37db7;08:63850408;thread:p7526.7528;core:0;}