[PATCH] gdbstub: Permit reverse step/break to provide stop response

[PATCH] gdbstub: Permit reverse step/break to provide stop response

[Nicholas Piggin]

The final part of the reverse step and break handling is to bring
the machine back to a debug stop state. gdb expects a response.

A gdb 'rsi' command hangs forever because the gdbstub filters out
the response (also observable with reverse_debugging.py avocado
tests).

Fix by setting allow_stop_reply for the gdb backward packets.

Fixes: 758370052fb ("gdbstub: only send stop-reply packets when allowed to")
Cc: qemu-stable@nongnu.org
Cc: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
Cc: Alex Bennée <alex.bennee@linaro.org>
Cc: Taylor Simpson <tsimpson@quicinc.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 gdbstub/gdbstub.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index be18568d0a..9496d7b175 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -1814,6 +1814,7 @@ static int gdb_handle_packet(const char *line_buf)
                 .handler = handle_backward,
                 .cmd = "b",
                 .cmd_startswith = 1,
+                .allow_stop_reply = true,
                 .schema = "o0"
             };
             cmd_parser = &backward_cmd_desc;
-- 
2.40.1

Re: [PATCH] gdbstub: Permit reverse step/break to provide stop response

[Matheus Tavares Bernardino]

> Nicholas Piggin <npiggin@gmail.com> wrote:
>
> The final part of the reverse step and break handling is to bring
> the machine back to a debug stop state. gdb expects a response.
> 
> A gdb 'rsi' command hangs forever because the gdbstub filters out
> the response (also observable with reverse_debugging.py avocado
> tests).
> 
> Fix by setting allow_stop_reply for the gdb backward packets.

Ah, it's interesting that [1] doesn't include 'bc' and 'bs' in the list
of cmds that may respond with a stop-reply packet:

    "The 'C', 'c', 'S', 's', 'vCont', 'vAttach', 'vRun', 'vStopped', and
    '?' packets can receive any of the below as a reply."

But their definitions at [2] do say the following:

    'bc' (and 'bc')
    [...]
    Reply: See Stop Reply Packets, for the reply specifications.

So I guess the list from [1] is not exhaustive. Anyway, thanks for the
fix!

Acked-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>

[1]: https://sourceware.org/gdb/onlinedocs/gdb/Stop-Reply-Packets.html#Stop-Reply-Packets
[2]: https://sourceware.org/gdb/onlinedocs/gdb/Packets.html#Packets

Re: [PATCH] gdbstub: Permit reverse step/break to provide stop response

[Peter Maydell]

On Fri, 23 Jun 2023 at 13:19, Matheus Tavares Bernardino
<quic_mathbern@quicinc.com> wrote:
>
> > Nicholas Piggin <npiggin@gmail.com> wrote:
> >
> > The final part of the reverse step and break handling is to bring
> > the machine back to a debug stop state. gdb expects a response.
> >
> > A gdb 'rsi' command hangs forever because the gdbstub filters out
> > the response (also observable with reverse_debugging.py avocado
> > tests).
> >
> > Fix by setting allow_stop_reply for the gdb backward packets.
>
> Ah, it's interesting that [1] doesn't include 'bc' and 'bs' in the list
> of cmds that may respond with a stop-reply packet:
>
>     "The 'C', 'c', 'S', 's', 'vCont', 'vAttach', 'vRun', 'vStopped', and
>     '?' packets can receive any of the below as a reply."
>
> But their definitions at [2] do say the following:
>
>     'bc' (and 'bc')
>     [...]
>     Reply: See Stop Reply Packets, for the reply specifications.
>
> So I guess the list from [1] is not exhaustive. Anyway, thanks for the
> fix!

That looks like it's probably a gdb docs bug (forgetting to
update that list when the bc/bs packets were added); we
should probably report that to upstream gdb.

thanks
-- PMM

Re: [PATCH] gdbstub: Permit reverse step/break to provide stop response

[Matheus Tavares Bernardino]

Peter Maydell <peter.maydell@linaro.org> wrote:
> 
> On Fri, 23 Jun 2023 at 13:19, Matheus Tavares Bernardino <quic_mathbern@quicinc.com> wrote:
> >
> > Ah, it's interesting that [1] doesn't include 'bc' and 'bs' in the list
> > of cmds that may respond with a stop-reply packet:
> >
> >     "The 'C', 'c', 'S', 's', 'vCont', 'vAttach', 'vRun', 'vStopped', and
> >     '?' packets can receive any of the below as a reply."
> >
> > But their definitions at [2] do say the following:
> >
> >     'bc' (and 'bc')
> >     [...]
> >     Reply: See Stop Reply Packets, for the reply specifications.
> >
> > So I guess the list from [1] is not exhaustive. Anyway, thanks for the
> > fix!
> 
> That looks like it's probably a gdb docs bug (forgetting to
> update that list when the bc/bs packets were added); we
> should probably report that to upstream gdb.

Good idea, done: https://sourceware.org/pipermail/gdb/2023-June/050804.html

Re: [PATCH] gdbstub: Permit reverse step/break to provide stop response

[Alex Bennée]

Nicholas Piggin <npiggin@gmail.com> writes:

> The final part of the reverse step and break handling is to bring
> the machine back to a debug stop state. gdb expects a response.
>
> A gdb 'rsi' command hangs forever because the gdbstub filters out
> the response (also observable with reverse_debugging.py avocado
> tests).
>
> Fix by setting allow_stop_reply for the gdb backward packets.
>
> Fixes: 758370052fb ("gdbstub: only send stop-reply packets when allowed to")
> Cc: qemu-stable@nongnu.org
> Cc: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
> Cc: Alex Bennée <alex.bennee@linaro.org>
> Cc: Taylor Simpson <tsimpson@quicinc.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Queued to gdbstub/next, thanks.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro