Re: [Qemu-devel] [PULL 10/37] gdbstub: add multiprocess support to Xfer:features:read:

[Peter Maydell]

On Mon, 7 Jan 2019 at 16:31, Peter Maydell <address@hidden> wrote:
>
> From: Luc Michel <address@hidden>
>
> Change the Xfer:features:read: packet handling to support the
> multiprocess extension. This packet is used to request the XML
> description of the CPU. In multiprocess mode, different descriptions can
> be sent for different processes.
>
> This function now takes the process to send the description for as a
> parameter, and use a buffer in the process structure to store the
> generated description.
>
> It takes the first CPU of the process to generate the description.

> @@ -1650,14 +1655,15 @@ static int gdb_handle_packet(GDBState *s, const char 
> *line_buf)
>              const char *xml;
>              target_ulong total_len;
>
> -            cc = CPU_GET_CLASS(first_cpu);
> +            process = gdb_get_cpu_process(s, s->g_cpu);
> +            cc = CPU_GET_CLASS(s->g_cpu);
>              if (cc->gdb_core_xml_file == NULL) {
>                  goto unknown_command;
>              }
>
>              gdb_has_xml = true;
>              p += 19;
> -            xml = get_feature_xml(p, &p, cc);
> +            xml = get_feature_xml(s, p, &p, process);
>              if (!xml) {
>                  snprintf(buf, sizeof(buf), "E00");
>                  put_packet(s, buf);

I've just run in to a segv in the gdbstub code in this bit,
because s->g_cpu is NULL.

Looking at the protocol trace from the gdb end:
(gdb) set debug remote 1
(gdb) target remote :1234
Remote debugging using :1234
Sending packet:
$qSupported:multiprocess+;swbreak+;hwbreak+;qRelocInsn+;fork-events+;vfork-events+;exec-events+;vContSupported+;QThreadEvents+;no-resumed+;xmlRegisters=i386#6a...Ack
Packet received: PacketSize=1000;qXfer:features:read+;multiprocess+
Packet qSupported (supported-packets) is supported
Sending packet: $vMustReplyEmpty#3a...Ack
Packet received:
Sending packet: $Hgp0.0#ad...Ack
Packet received: E22
Sending packet: $qXfer:features:read:target.xml:0,ffb#79...Ack
[here is where QEMU crashes]

What seems to happen is that GDB's attempt to set the
current thread with the "Hg" command fails because
gdb_get_cpu() fails[*], and so we return an E22 error status.
GDB (incorrectly) ignores this and issues a general command
anyway, and then QEMU crashes because we don't handle s->g_cpu
being NULL in this command's handling code.

[*] This happens with an out-of-tree board model I'm working
on that uses the cpu cluster stuff, so it's probably a bug in
my code that causes it.

I think it would be nice to do something more robust in these
cases, even if it does only happen when a QEMU bug and a GDB
bug collide :-)

thanks
-- PMM