Re: [PULL 29/38] gdbstub: Permit reverse step/break to provide stop response

["Alex Bennée" <alex.bennee@linaro.org>]

Michael Tokarev <mjt@tls.msk.ru> writes:

> 03.07.2023 16:44, Alex Bennée wrote:
>> From: Nicholas Piggin <npiggin@gmail.com>
>> 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
>
> Hi!
>
> Are you guys sure this needs to be in -stable?
>
> To me it looks a sort of "partial revert" of a previous commit:
>
> commit 758370052fb602f9f23c3b8ae26a6133373c78e6
> Author: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
> Date:   Thu May 4 12:37:31 2023 -0300
> Subject: gdbstub: only send stop-reply packets when allowed to
>
> which introduced `allow_stop_reply' field in GdbCmdParseEntry.
> This change ("gdbstub: Permit..") does not work in 8.0 without
> the above mentioned "gdbstub: only send" commit, and I guess
> it is *not* supposed to be in stable. Or is it?
>
> I'm not applying this one to stable for now.

Good catch - your right it's purely fixing something that has been
merged in the current cycle.

>
> Thanks,
>
> /mjt
>
>> 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>
>> Acked-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
>> Message-Id: <20230623035304.279833-1-npiggin@gmail.com>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Message-Id: <20230630180423.558337-30-alex.bennee@linaro.org>
>> 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;


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro