Message ID | 20190827223317.8614-1-sandra@codesourcery.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [V3] gdbstub: Fix handler for 'F' packet | expand |
On 8/27/19 3:33 PM, Sandra Loosemore wrote: > Handling of the 'F' packet has been broken since commit > 4b20fab101b9e2d0fb47454209637a17fc7a13d5, which converted it to use > the new packet parsing infrastructure. Per the GDB RSP specification > > https://sourceware.org/gdb/current/onlinedocs/gdb/The-F-Reply-Packet.html > > the second parameter may be omitted, but the rewritten implementation > was failing to recognize this case. The result was that QEMU was > repeatedly resending the fileio request and ignoring GDB's replies of > successful completion. This patch restores the behavior of the > previous code in allowing the errno parameter to be omitted and > passing 0 to the callback in that case. > > Signed-off-by: Sandra Loosemore <sandra@codesourcery.com> > --- > gdbstub.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
Sandra Loosemore <sandra@codesourcery.com> writes: > Handling of the 'F' packet has been broken since commit > 4b20fab101b9e2d0fb47454209637a17fc7a13d5, which converted it to use > the new packet parsing infrastructure. Per the GDB RSP specification > > https://sourceware.org/gdb/current/onlinedocs/gdb/The-F-Reply-Packet.html > > the second parameter may be omitted, but the rewritten implementation > was failing to recognize this case. The result was that QEMU was > repeatedly resending the fileio request and ignoring GDB's replies of > successful completion. This patch restores the behavior of the > previous code in allowing the errno parameter to be omitted and > passing 0 to the callback in that case. > > Signed-off-by: Sandra Loosemore <sandra@codesourcery.com> Queued to gdbstub/next, thanks. > --- > gdbstub.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/gdbstub.c b/gdbstub.c > index b92ba59..3e8bcd0 100644 > --- a/gdbstub.c > +++ b/gdbstub.c > @@ -1820,11 +1820,15 @@ static void handle_read_all_regs(GdbCmdContext *gdb_ctx, void *user_ctx) > > static void handle_file_io(GdbCmdContext *gdb_ctx, void *user_ctx) > { > - if (gdb_ctx->num_params >= 2 && gdb_ctx->s->current_syscall_cb) { > + if (gdb_ctx->num_params >= 1 && gdb_ctx->s->current_syscall_cb) { > target_ulong ret, err; > > ret = (target_ulong)gdb_ctx->params[0].val_ull; > - err = (target_ulong)gdb_ctx->params[1].val_ull; > + if (gdb_ctx->num_params >= 2) { > + err = (target_ulong)gdb_ctx->params[1].val_ull; > + } else { > + err = 0; > + } > gdb_ctx->s->current_syscall_cb(gdb_ctx->s->c_cpu, ret, err); > gdb_ctx->s->current_syscall_cb = NULL; > } -- Alex Bennée
diff --git a/gdbstub.c b/gdbstub.c index b92ba59..3e8bcd0 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -1820,11 +1820,15 @@ static void handle_read_all_regs(GdbCmdContext *gdb_ctx, void *user_ctx) static void handle_file_io(GdbCmdContext *gdb_ctx, void *user_ctx) { - if (gdb_ctx->num_params >= 2 && gdb_ctx->s->current_syscall_cb) { + if (gdb_ctx->num_params >= 1 && gdb_ctx->s->current_syscall_cb) { target_ulong ret, err; ret = (target_ulong)gdb_ctx->params[0].val_ull; - err = (target_ulong)gdb_ctx->params[1].val_ull; + if (gdb_ctx->num_params >= 2) { + err = (target_ulong)gdb_ctx->params[1].val_ull; + } else { + err = 0; + } gdb_ctx->s->current_syscall_cb(gdb_ctx->s->c_cpu, ret, err); gdb_ctx->s->current_syscall_cb = NULL; }
Handling of the 'F' packet has been broken since commit 4b20fab101b9e2d0fb47454209637a17fc7a13d5, which converted it to use the new packet parsing infrastructure. Per the GDB RSP specification https://sourceware.org/gdb/current/onlinedocs/gdb/The-F-Reply-Packet.html the second parameter may be omitted, but the rewritten implementation was failing to recognize this case. The result was that QEMU was repeatedly resending the fileio request and ignoring GDB's replies of successful completion. This patch restores the behavior of the previous code in allowing the errno parameter to be omitted and passing 0 to the callback in that case. Signed-off-by: Sandra Loosemore <sandra@codesourcery.com> --- gdbstub.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)