| Message ID | 20190705160421.19015-6-alex.bennee@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tcg tests and gdbstub fixes | expand |
On 7/5/19 6:04 PM, Alex Bennée wrote: > The refactoring of handle_set_reg missed the fact we previously had > responded with an empty packet when we were not using XML based > protocols. This broke the fallback behaviour for architectures that > don't have registers defined in QEMU's gdb-xml directory. > > Revert to the previous behaviour and clean up the commentary for what > is going on. > > Fixes: 62b3320bddd > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > Cc: Jon Doron <arilou@gmail.com> > Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > --- > gdbstub.c | 18 ++++++++++++------ > 1 file changed, 12 insertions(+), 6 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On Jul 5, 2019 6:08 PM, "Alex Bennée" <alex.bennee@linaro.org> wrote: > > The refactoring of handle_set_reg missed the fact we previously had > responded with an empty packet when we were not using XML based > protocols. This broke the fallback behaviour for architectures that > don't have registers defined in QEMU's gdb-xml directory. > > Revert to the previous behaviour and clean up the commentary for what > is going on. > > Fixes: 62b3320bddd > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > Cc: Jon Doron <arilou@gmail.com> > Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > --- Do you plan to integrate this patch in 4.1? Thanks, Aleksandar > gdbstub.c | 18 ++++++++++++------ > 1 file changed, 12 insertions(+), 6 deletions(-) > > diff --git a/gdbstub.c b/gdbstub.c > index ea3349d1aa..b6df7ee25a 100644 > --- a/gdbstub.c > +++ b/gdbstub.c > @@ -1669,12 +1669,23 @@ static void handle_remove_bp(GdbCmdContext *gdb_ctx, void *user_ctx) > put_packet(gdb_ctx->s, "E22"); > } > > +/* > + * handle_set/get_reg > + * > + * Older gdb are really dumb, and don't use 'G/g' if 'P/p' is available. > + * This works, but can be very slow. Anything new enough to understand > + * XML also knows how to use this properly. However to use this we > + * need to define a local XML file as well as be talking to a > + * reasonably modern gdb. Responding with an empty packet will cause > + * the remote gdb to fallback to older methods. > + */ > + > static void handle_set_reg(GdbCmdContext *gdb_ctx, void *user_ctx) > { > int reg_size; > > if (!gdb_has_xml) { > - put_packet(gdb_ctx->s, "E00"); > + put_packet(gdb_ctx->s, ""); > return; > } > > @@ -1694,11 +1705,6 @@ static void handle_get_reg(GdbCmdContext *gdb_ctx, void *user_ctx) > { > int reg_size; > > - /* > - * Older gdb are really dumb, and don't use 'g' if 'p' is avaialable. > - * This works, but can be very slow. Anything new enough to > - * understand XML also knows how to use this properly. > - */ > if (!gdb_has_xml) { > put_packet(gdb_ctx->s, ""); > return; > -- > 2.20.1 > >
Aleksandar Markovic <aleksandar.m.mail@gmail.com> writes: > On Jul 5, 2019 6:08 PM, "Alex Bennée" <alex.bennee@linaro.org> wrote: >> >> The refactoring of handle_set_reg missed the fact we previously had >> responded with an empty packet when we were not using XML based >> protocols. This broke the fallback behaviour for architectures that >> don't have registers defined in QEMU's gdb-xml directory. >> >> Revert to the previous behaviour and clean up the commentary for what >> is going on. >> >> Fixes: 62b3320bddd >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> Cc: Jon Doron <arilou@gmail.com> >> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> >> --- > > Do you plan to integrate this patch in 4.1? > > Thanks, Aleksandar Yes - I'm putting together a PR today. > >> gdbstub.c | 18 ++++++++++++------ >> 1 file changed, 12 insertions(+), 6 deletions(-) >> >> diff --git a/gdbstub.c b/gdbstub.c >> index ea3349d1aa..b6df7ee25a 100644 >> --- a/gdbstub.c >> +++ b/gdbstub.c >> @@ -1669,12 +1669,23 @@ static void handle_remove_bp(GdbCmdContext > *gdb_ctx, void *user_ctx) >> put_packet(gdb_ctx->s, "E22"); >> } >> >> +/* >> + * handle_set/get_reg >> + * >> + * Older gdb are really dumb, and don't use 'G/g' if 'P/p' is available. >> + * This works, but can be very slow. Anything new enough to understand >> + * XML also knows how to use this properly. However to use this we >> + * need to define a local XML file as well as be talking to a >> + * reasonably modern gdb. Responding with an empty packet will cause >> + * the remote gdb to fallback to older methods. >> + */ >> + >> static void handle_set_reg(GdbCmdContext *gdb_ctx, void *user_ctx) >> { >> int reg_size; >> >> if (!gdb_has_xml) { >> - put_packet(gdb_ctx->s, "E00"); >> + put_packet(gdb_ctx->s, ""); >> return; >> } >> >> @@ -1694,11 +1705,6 @@ static void handle_get_reg(GdbCmdContext *gdb_ctx, > void *user_ctx) >> { >> int reg_size; >> >> - /* >> - * Older gdb are really dumb, and don't use 'g' if 'p' is avaialable. >> - * This works, but can be very slow. Anything new enough to >> - * understand XML also knows how to use this properly. >> - */ >> if (!gdb_has_xml) { >> put_packet(gdb_ctx->s, ""); >> return; >> -- >> 2.20.1 >> >> -- Alex Bennée
On Jul 10, 2019 11:30 AM, "Alex Bennée" <alex.bennee@linaro.org> wrote: > > > Aleksandar Markovic <aleksandar.m.mail@gmail.com> writes: > > > On Jul 5, 2019 6:08 PM, "Alex Bennée" <alex.bennee@linaro.org> wrote: > >> > >> The refactoring of handle_set_reg missed the fact we previously had > >> responded with an empty packet when we were not using XML based > >> protocols. This broke the fallback behaviour for architectures that > >> don't have registers defined in QEMU's gdb-xml directory. > >> > >> Revert to the previous behaviour and clean up the commentary for what > >> is going on. > >> > >> Fixes: 62b3320bddd > >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > >> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > >> Cc: Jon Doron <arilou@gmail.com> > >> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > >> --- > > > > Do you plan to integrate this patch in 4.1? > > > > Thanks, Aleksandar > > Yes - I'm putting together a PR today. > That's great, thanks!! Aleksandar > > > >> gdbstub.c | 18 ++++++++++++------ > >> 1 file changed, 12 insertions(+), 6 deletions(-) > >> > >> diff --git a/gdbstub.c b/gdbstub.c > >> index ea3349d1aa..b6df7ee25a 100644 > >> --- a/gdbstub.c > >> +++ b/gdbstub.c > >> @@ -1669,12 +1669,23 @@ static void handle_remove_bp(GdbCmdContext > > *gdb_ctx, void *user_ctx) > >> put_packet(gdb_ctx->s, "E22"); > >> } > >> > >> +/* > >> + * handle_set/get_reg > >> + * > >> + * Older gdb are really dumb, and don't use 'G/g' if 'P/p' is available. > >> + * This works, but can be very slow. Anything new enough to understand > >> + * XML also knows how to use this properly. However to use this we > >> + * need to define a local XML file as well as be talking to a > >> + * reasonably modern gdb. Responding with an empty packet will cause > >> + * the remote gdb to fallback to older methods. > >> + */ > >> + > >> static void handle_set_reg(GdbCmdContext *gdb_ctx, void *user_ctx) > >> { > >> int reg_size; > >> > >> if (!gdb_has_xml) { > >> - put_packet(gdb_ctx->s, "E00"); > >> + put_packet(gdb_ctx->s, ""); > >> return; > >> } > >> > >> @@ -1694,11 +1705,6 @@ static void handle_get_reg(GdbCmdContext *gdb_ctx, > > void *user_ctx) > >> { > >> int reg_size; > >> > >> - /* > >> - * Older gdb are really dumb, and don't use 'g' if 'p' is avaialable. > >> - * This works, but can be very slow. Anything new enough to > >> - * understand XML also knows how to use this properly. > >> - */ > >> if (!gdb_has_xml) { > >> put_packet(gdb_ctx->s, ""); > >> return; > >> -- > >> 2.20.1 > >> > >> > > > -- > Alex Bennée
diff --git a/gdbstub.c b/gdbstub.c index ea3349d1aa..b6df7ee25a 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -1669,12 +1669,23 @@ static void handle_remove_bp(GdbCmdContext *gdb_ctx, void *user_ctx) put_packet(gdb_ctx->s, "E22"); } +/* + * handle_set/get_reg + * + * Older gdb are really dumb, and don't use 'G/g' if 'P/p' is available. + * This works, but can be very slow. Anything new enough to understand + * XML also knows how to use this properly. However to use this we + * need to define a local XML file as well as be talking to a + * reasonably modern gdb. Responding with an empty packet will cause + * the remote gdb to fallback to older methods. + */ + static void handle_set_reg(GdbCmdContext *gdb_ctx, void *user_ctx) { int reg_size; if (!gdb_has_xml) { - put_packet(gdb_ctx->s, "E00"); + put_packet(gdb_ctx->s, ""); return; } @@ -1694,11 +1705,6 @@ static void handle_get_reg(GdbCmdContext *gdb_ctx, void *user_ctx) { int reg_size; - /* - * Older gdb are really dumb, and don't use 'g' if 'p' is avaialable. - * This works, but can be very slow. Anything new enough to - * understand XML also knows how to use this properly. - */ if (!gdb_has_xml) { put_packet(gdb_ctx->s, ""); return;