Re: [PATCH v2] gdbstub: Fix single-step issue by confirming 'vContSupported+' feature to gdb

[Changbin Du <changbin.du@gmail.com>]

hello, is this patch ready to merge now? Thanks!

On Fri, Feb 21, 2020 at 08:25:59AM +0800, Changbin Du wrote:
> Recently when debugging an arm32 system on qemu, I found sometimes the
> single-step command (stepi) is not working. This can be reproduced by
> below steps:
>  1) start qemu-system-arm -s -S .. and wait for gdb connection.
>  2) start gdb and connect to qemu. In my case, gdb gets a wrong value
>     (0x60) for PC, which is an another bug.
>  3) After connected, type 'stepi' and expect it will stop at next ins.
> 
> But, it has never stopped. This because:
>  1) We doesn't report ‘vContSupported’ feature to gdb explicitly and gdb
>     think we do not support it. In this case, gdb use a software breakpoint
>     to emulate single-step.
>  2) Since gdb gets a wrong initial value of PC, then gdb inserts a
>     breakpoint to wrong place (PC+4).
> 
> Not only for the arm target, Philippe has also encountered this on MIPS.
> Probably gdb has different assumption for different architectures.
> 
> Since we do support ‘vContSupported’ query command, so let's tell gdb that
> we support it.
> 
> Before this change, gdb send below 'Z0' packet to implement single-step:
> gdb_handle_packet: Z0,4,4
> 
> After this change, gdb send "vCont;s.." which is expected:
> gdb_handle_packet: vCont?
> put_packet: vCont;c;C;s;S
> gdb_handle_packet: vCont;s:p1.1;c:p1.-1
> 
> Signed-off-by: Changbin Du <changbin.du@gmail.com>
> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> ---
> v2: polish commit message.
> ---
>  gdbstub.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gdbstub.c b/gdbstub.c
> index ce304ff482..adccd938e2 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -2111,7 +2111,7 @@ static void handle_query_supported(GdbCmdContext *gdb_ctx, void *user_ctx)
>          gdb_ctx->s->multiprocess = true;
>      }
>  
> -    pstrcat(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), ";multiprocess+");
> +    pstrcat(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), ";vContSupported+;multiprocess+");
>      put_packet(gdb_ctx->s, gdb_ctx->str_buf);
>  }
>  
> -- 
> 2.25.0
> 

-- 
Cheers,
Changbin Du