[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] gdbstub: Fix client Ctrl-C handling
From: |
Matheus Tavares Bernardino |
Subject: |
Re: [PATCH] gdbstub: Fix client Ctrl-C handling |
Date: |
Tue, 1 Aug 2023 15:40:36 -0300 |
Hi, Nick.
> Nicholas Piggin <npiggin@gmail.com> wrote:
>
> On Tue Jul 11, 2023 at 9:03 PM AEST, Matheus Tavares Bernardino wrote:
> > > Nicholas Piggin <npiggin@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?
- Re: [PATCH] gdbstub: Fix client Ctrl-C handling,
Matheus Tavares Bernardino <=