[PATCH][gdb/doc] Fix to manual for description of remote protocol

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH][gdb/doc] Fix to manual for description of remote protocol

Sourceware - gdb-patches mailing list
See the thread starting at
https://sourceware.org/pipermail/gdb/2020-July/048808.html
and in particular the analysis I give in
https://sourceware.org/pipermail/gdb/2020-July/048817.html

The manual states in the section E.1 about the remote protocol that remote
stubs must implement the 's' command. This appears to be false. Further, it
does not mention the '?' command, which does appear to be required.

The attached patch against git master at the time of writing fixes these
two nits.

--
https://rrt.sc3d.org

0001-Correct-an-error-in-the-remote-protocol-specificatio.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][gdb/doc] Fix to manual for description of remote protocol

Pedro Alves-2
On 7/21/20 8:14 PM, Reuben Thomas via Gdb-patches wrote:

> ---
>  gdb/ChangeLog       |  5 +++++
>  gdb/doc/gdb.texinfo | 13 +++++++------
>  2 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index fad4608002..f2f9302078 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,8 @@
> +2020-07-21  Reuben Thomas  <[hidden email]>
> +
> + * doc/gdb.texinfo: Correct the description of which remote
> + protocol commands are mandatory for a stub to implement.
> +

Note this should go to the gdb/doc/ChangeLog file instead.

>  2020-07-20  John Baldwin  <[hidden email]>
>  
>   * fbsd-tdep.c (fbsd_skip_solib_resolver): New function.
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index a002084d5b..1e72c0ed32 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -39134,12 +39134,13 @@ For any @var{command} not supported by the stub, an empty response
>  protocol.  A newer @value{GDBN} can tell if a packet is supported based
>  on that response.
>  
> -At a minimum, a stub is required to support the @samp{g} and @samp{G}
> -commands for register access, and the @samp{m} and @samp{M} commands
> -for memory access.  Stubs that only control single-threaded targets
> -can implement run control with the @samp{c} (continue), and @samp{s}
> -(step) commands.  Stubs that support multi-threading targets should
> -support the @samp{vCont} command.  All other commands are optional.
> +At a minimum, a stub is required to support the @samp{?} command to tell
> +@value{GDBN} the reason for halting, @samp{g} and @samp{G} commands for
> +register access, and the @samp{m} and @samp{M} commands for memory
> +access.  Stubs that only control single-threaded targets can implement
> +run control with the @samp{c} (continue) command.  Stubs that support
> +multi-threading targets should support the @samp{vCont} command.  All
> +other commands are optional.
>  

I see you're dropping the reference to the s (step) command.
However, either "s" or "vCont;s" _are_ mandatory if the target supports
hardware single-step.  E.g. an x86 stub that doesn't implement
s or vCont;s won't work.  GDB only implements software single-stepping for
architectures that don't do hardware stepping.

The choice of whether to require s is a little more complicated
than that, but I don't think we need to go into too much detail here.

Do you think the version below is clear enough?

From 68e60bc53a040ad2750277aaa5fc7019eb051c22 Mon Sep 17 00:00:00 2001
From: Reuben Thomas <[hidden email]>
Date: Wed, 22 Jul 2020 14:19:17 +0100
Subject: [PATCH] Correct an error in the remote protocol specification

The list of commands that a stub must implement was wrong.

gdb/ChangeLog:
2020-07-21  Reuben Thomas  <[hidden email]>

        * gdb.texinfo (Remote Protocol, Overview): Correct the description
        of which remote protocol commands are mandatory for a stub to
        implement.
---
 gdb/doc/gdb.texinfo | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index a002084d5b9..3bd8ddfd624 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -39134,12 +39134,15 @@ For any @var{command} not supported by the stub, an empty response
 protocol.  A newer @value{GDBN} can tell if a packet is supported based
 on that response.
 
-At a minimum, a stub is required to support the @samp{g} and @samp{G}
+At a minimum, a stub is required to support the @samp{?} command to
+tell @value{GDBN} the reason for halting, @samp{g} and @samp{G}
 commands for register access, and the @samp{m} and @samp{M} commands
 for memory access.  Stubs that only control single-threaded targets
-can implement run control with the @samp{c} (continue), and @samp{s}
-(step) commands.  Stubs that support multi-threading targets should
-support the @samp{vCont} command.  All other commands are optional.
+can implement run control with the @samp{c} (continue) command, and if
+the target architecture supports hardware-assisted single-stepping,
+the @samp{s} (step) command.  Stubs that support multi-threading
+targets should support the @samp{vCont} command.  All other commands
+are optional.
 
 @node Packets
 @section Packets

base-commit: 39fdda0744607575103b30ffbec3cdb99f8d2501
--
2.14.5

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][gdb/doc] Fix to manual for description of remote protocol

Sourceware - gdb-patches mailing list
On Wed, 22 Jul 2020 at 14:56, Pedro Alves <[hidden email]> wrote:

Thanks very much for the review!

Note this should go to the gdb/doc/ChangeLog file instead.
>

Apologies, I didn't see that file existed.

I see you're dropping the reference to the s (step) command.
> However, either "s" or "vCont;s" _are_ mandatory if the target supports
> hardware single-step.  E.g. an x86 stub that doesn't implement
> s or vCont;s won't work.


Thanks for the correction.

The choice of whether to require s is a little more complicated
> than that, but I don't think we need to go into too much detail here.
>
> Do you think the version below is clear enough?
>

That looks great to me.

--
https://rrt.sc3d.org
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][gdb/doc] Fix to manual for description of remote protocol

Pedro Alves-2
On 7/22/20 2:58 PM, Reuben Thomas wrote:

> On Wed, 22 Jul 2020 at 14:56, Pedro Alves <[hidden email] <mailto:[hidden email]>> wrote:
>
> Thanks very much for the review!
>
>     Note this should go to the gdb/doc/ChangeLog file instead.
>
>
> Apologies, I didn't see that file existed.
>
>     I see you're dropping the reference to the s (step) command.
>     However, either "s" or "vCont;s" _are_ mandatory if the target supports
>     hardware single-step.  E.g. an x86 stub that doesn't implement
>     s or vCont;s won't work.
>
>
> Thanks for the correction.
>
>     The choice of whether to require s is a little more complicated
>     than that, but I don't think we need to go into too much detail here.
>
>     Do you think the version below is clear enough?
>
>
> That looks great to me.

Alright, I've merged it.

Thanks!