Hi, Nick. > Nicholas Piggin <npig...@gmail.com> wrote: > > On Tue Jul 11, 2023 at 9:03 PM AEST, Matheus Tavares Bernardino wrote: > > > Nicholas Piggin <npig...@gmail.com> wrote: > > > > > > diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c > > > index 6911b73c07..ce8b42eb15 100644 > > > --- a/gdbstub/gdbstub.c > > > +++ b/gdbstub/gdbstub.c > > > @@ -2051,8 +2051,17 @@ void gdb_read_byte(uint8_t ch) > > > return; > > > } > > > if (runstate_is_running()) { > > > - /* when the CPU is running, we cannot do anything except stop > > > - it when receiving a char */ > > > + /* > > > + * When the CPU is running, we cannot do anything except stop > > > + * it when receiving a char. This is expected on a Ctrl-C in the > > > + * gdb client. Because we are in all-stop mode, gdb sends a > > > + * 0x03 byte which is not a usual packet, so we handle it > > > specially > > > + * here, but it does expect a stop reply. > > > + */ > > > + if (ch != 0x03) { > > > + warn_report("gdbstub: client sent packet while target > > > running\n"); > > > + } > > > + gdbserver_state.allow_stop_reply = true; > > > vm_stop(RUN_STATE_PAUSED); > > > } else > > > #endif > > > > Makes sense to me, but shouldn't we send the stop-reply packet only for > > Ctrl+C/0x03? > > Good question. > > I think if we get a character here that's not a 3, we're already in > trouble, and we eat it so even worse. Since we only send a stop packet > back when the vm stops, then if we don't send one now we might never > send it. At least if we send one then the client might have some chance > to get back to a sane state.
I just noticed now (as I was integrating the latest upstream patches with our downstream qemu-system-hexagon) that this causes the gdbstub-untimely-packet tcg test to fail. My first thought was that, if 0x3 is the only valid case where we will read a char when the cpu is running, perhaps not issuing the stop-reply isn't that bad as GDB would ignore it anyways. E.g. from a `set debug remote 1` output: Sending packet: $qSupported:multiprocess+;swbreak+;hwbreak+;qRelocInsn+; fork-events+;vfork-events+;exec-events+;vContSupported+; QThreadEvents+;no-resumed+; xmlRegisters=i386#6a... Packet instead of Ack, ignoring it So, perhaps, we could do: diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c index f123b40ce7..8af066301a 100644 --- a/gdbstub/gdbstub.c +++ b/gdbstub/gdbstub.c @@ -2055,8 +2055,9 @@ void gdb_read_byte(uint8_t ch) */ if (ch != 0x03) { warn_report("gdbstub: client sent packet while target running\n"); + } else { + gdbserver_state.allow_stop_reply = true; } - gdbserver_state.allow_stop_reply = true; vm_stop(RUN_STATE_PAUSED); } else #endif -- >8 -- Alternatively, since GDB ignores the packet anyways, should we just let this be and refactor/remove the test?