Hi Alex,
There don't seem to be any other patches attached? I would NACK a patch
> that isn't actually used in-tree.
No there isn't ! I should have not been so prolix. Actually the patch
corrects a
possible null pointer dereference in the gdbserver code. That's all folks.
Below is how I discovered it and why it matters, imho, to be fixed
(out of a pending issue).
This null deref does not happen in normal operation because of the way
gdbserver is initialized. However what I was telling you, is that it may be
really interesting to be able to take benefit of some gdb features
internally
without starting a gdbserver and the overhead of the gdb protocol when
there is no need for a client/server interaction while debugging.
For instance, I developed a special purpose x86 board where I needed to
break at some instructions, do some automation (snapshots, vm memory
access, cpu analysis) and then resume the VM. Implementing a
"vm_change_state handler" and dealing with RUN_STATE_DEBUG was
perfect for my need.
The debug events (#BP, #DB) are tied to the gdbserver stub, at some point
when "cpu_handle_guest_debug()" is called, a call to "gdb_set_stop_cpu()"
triggers the NULL deref because the gdbserver is not initialized.
My little fix addresses this error and allows to make use of the following
Qemu
breakpoint functions without touching "cpus.c" or "exec.c":
- cpu_breakpoint_remove/insert()
- kvm_remove/insert_breakpoint()
Notice, that in the particular case of KVM, I had to async_run_on_cpu() my
debug handler instead of directly executing it in the context of
vm_change_state(). The vCPU started to significantly slow down
(low irq rate), while this never happens with the TCG.
Is that more clear to you Alex ? (and hopefully lead to patch ACK :)
Regards,
stephane
> Signed-off-by: Stephane Duverger <stephane.duver...@gmail.com>
> > ---
> > gdbstub.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/gdbstub.c b/gdbstub.c
> > index d6ab95006c..788fd625ab 100644
> > --- a/gdbstub.c
> > +++ b/gdbstub.c
> > @@ -1412,6 +1412,9 @@ static int gdb_handle_packet(GDBState *s, const
> char *line_buf)
> >
> > void gdb_set_stop_cpu(CPUState *cpu)
> > {
> > + if (!gdbserver_state) {
> > + return;
> > + }
> > gdbserver_state->c_cpu = cpu;
> > gdbserver_state->g_cpu = cpu;
> > }
>
>
> --
> Alex Bennée
>
