Message ID | 20200221002559.6768-1-changbin.du@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v2] gdbstub: Fix single-step issue by confirming 'vContSupported+' feature to gdb | expand |
On 2/21/20 1:25 AM, 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> Reviewed-by: Luc Michel <luc.michel@greensocs.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); > } > >
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 >
Changbin Du <changbin.du@gmail.com> writes: > 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). OK I was initially confused because I couldn't replicate the failure with the aarch64 tests. With or without the vContSupported reply I saw the exact same set of messages go between gdbstub and gdb. However I did replicate with: ./arm-softmmu/qemu-system-arm -monitor none -display none \ -chardev file,path=test-armv6m-undef.out,id=output \ -semihosting -M microbit \ -kernel tests/tcg/arm-softmmu/test-armv6m-undef \ -s -S -d trace:gdbstub\* -D gdbstub.dump So I have applied it to me tree.
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); }