Re: [PATCH] Add gdbarch_sizeof_g_packet to "struct remote_arch_state" to handle the issue that get error "Remote 'g' packet reply is too long" with qemu-system-x86_64

[Hui Zhu <teawater at gmail dot com>]

On Mon, Nov 17, 2014 at 3:23 PM, Doug Evans <xdje42@gmail.com> wrote:
> On Sun, Nov 16, 2014 at 11:08 PM, Hui Zhu <teawater@gmail.com> wrote:
>> Ping.
>>
>> Or I just enjoy it with myself.  :P
>>
>> Thanks,
>> Hui
>>
>> On Tue, Sep 2, 2014 at 11:26 AM, Hui Zhu <teawater@gmail.com> wrote:
>>> Ping.
>>>
>>> Thanks,
>>> Hui
>>>
>>> On Thu, Aug 28, 2014 at 11:11 PM, Hui Zhu <teawater@gmail.com> wrote:
>>>> If qemu-system-x86_64 start with -S, gdb will get error "Remote 'g' packet reply
>>>> is too long", for example:
>>>> sudo qemu-system-x86_64 -nographic -k en-us -kernel /boot/vmlinuz-3.13.0-34-generic -gdb tcp::12345 -append "console=ttyS0,115200 root=/dev/sda rw kmemleak=off" -S -m 512 -smp 1
>>>> gdb /usr/lib/debug/boot/vmlinux-$(uname -r)
>>>> (gdb) set debug remote 1
>>>> (gdb) target remote :12345
>>>> ...
>>>> ...
>>>> Sending packet: $g#67...Ack
>>>> Packet received: 0000000000000000630600000000000000000000000000000000000
>>>> 000000000f0ff00000200000000f00000000000000000000000000000000000000000000
>>>> 000000000000000000000000000000000000000000000000000000000000000000000000
>>>> 000000000000000000000000000000000000000000000000000000000000000000000000
>>>> 000000000000000007f03000000000000000000000000000000000000000000000000000
>>>> 000000000000000000000000000000000000000000000000000000000000000000000000
>>>> 000000000000000000000000000000000000000000000000000000000000000000000000
>>>> 000000000000000000000000000000000000000000000000000000000000000000000000
>>>> 0000000000000000000000000000000000000000000000000801f0000
>>>> (gdb) b start_kernel
>>>> (gdb) c
>>>> Continuing.
>>>> ...
>>>> ...
>>>> Sending packet: $g#67...Ack
>>>> Packet received: 0000000000000000d381ffffffff000040000000000000000010060
>>>> 00000000000f009000000000010f4ce81ffffffff981fc081ffffffff901fc081fffffff
>>>> f0030c1010000000000000000000000000000000000000000a869270200000000fffffff
>>>> f0000000020000000000000002051d381ffffffffb01fc081ffffffff385bd381fffffff
>>>> f96000000100000000000000000000000000000000000000000000000000000000000000
>>>> 000000000000000000000000000000000000000000000000000000000000000000000000
>>>> 000000000000000000000000000000000000000000000000000000000000000000000000
>>>> 07f030000000000000000000000000000000000000000000000000000000000000000000
>>>> 000000000000000000000000000000000000000000000000000000000000000000000000
>>>> 000000000000000000000000000000000000000000000000000000000000000000000000
>>>> 000000000000000000000000000000000000000000000000000000000000000000000000
>>>> 000000000000000000000000000000000000000000000000000000000000000000000000
>>>> 000000000000000000000000000000000000000000000000000000000000000000000000
>>>> 000000000000000000000000000000000000000000000000000000000000000000000000
>>>> 000000000000000000000000000000000000000000000000000000000000000000000000
>>>> 0801f0000
>>>> Remote 'g' packet reply is too long: 0000000000000000d381ffffffff0000400
>>>> 0000000000000001006000000000000f009000000000010f4ce81ffffffff981fc081fff
>>>> fffff901fc081ffffffff0030c1010000000000000000000000000000000000000000a86
>>>> 9270200000000ffffffff0000000020000000000000002051d381ffffffffb01fc081fff
>>>> fffff385bd381ffffffff960000001000000000000000000000000000000000000000000
>>>> 000000000000000000000000000000000000000000000000000000000000000000000000
>>>> 000000000000000000000000000000000000000000000000000000000000000000000000
>>>> 0000000000000000000007f0300000000000000000000000000000000000000000000000
>>>> 000000000000000000000000000000000000000000000000000000000000000000000000
>>>> 000000000000000000000000000000000000000000000000000000000000000000000000
>>>> 000000000000000000000000000000000000000000000000000000000000000000000000
>>>> 000000000000000000000000000000000000000000000000000000000000000000000000
>>>> 000000000000000000000000000000000000000000000000000000000000000000000000
>>>> 000000000000000000000000000000000000000000000000000000000000000000000000
>>>> 000000000000000000000000000000000000000000000000000000000000000000000000
>>>> 000000000000000000000801f0000
>>>> (gdb) c
>>>> Continuing.
>>>> Cannot execute this command while the selected thread is running.
>>>>
>>>> The root cause of this issue is:
>>>> When GDB load a debug binary, function init_remote_state will be called and
>>>> rsa->sizeof_g_packet will be initialized.
>>>>   /* Record the maximum possible size of the g packet - it may turn out
>>>>      to be smaller.  */
>>>>   rsa->sizeof_g_packet = map_regcache_remote_table (gdbarch, rsa->regs);
>>>> When remote.c get first reply of g that its size is smaller than
>>>> rsa->sizeof_g_packet in function process_g_packet, sizeof_g_packet will be
>>>> updated to this smaller size.
>>>>       rsa->sizeof_g_packet = buf_len / 2;
>>>> When inferior is breaked by breakpoint, because some reason architecture reason,
>>>> QEMU send a packet for "g" that is more bigger than previous one.  Then GDB will
>>>> get error in function process_g_packet.
>>>>   /* Further sanity checks, with knowledge of the architecture.  */
>>>>   if (buf_len > 2 * rsa->sizeof_g_packet)
>>>>     error (_("Remote 'g' packet reply is too long: %s"), rs->buf);
>>>>
>>>> To handle this issue:
>>>> This patch add "gdbarch_sizeof_g_packet" to "struct remote_arch_state", it will
>>>> keep the size of g packet that get from gdbarch.  And function process_g_packet
>>>> use it do the check for the reply of "g".  And update sizeof_g_packet if it is
>>>> not same with reply of "g".
>>>>
>>>> 2014-08-21  Hui Zhu  <teawater@gmail.com>
>>>>
>>>>         * remote.c (struct remote_arch_state): Add gdbarch_sizeof_g_packet.
>>>>         (init_remote_state): Initialize rsa->gdbarch_sizeof_g_packet.
>>>>         (process_g_packet): Check buf_len with rsa->gdbarch_sizeof_g_packet.
>>>>         Update code to update rsa->sizeof_g_packet if buf_len is not for it.
>>>> ---
>>>>  gdb/remote.c | 18 ++++++++++++------
>>>>  1 file changed, 12 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/gdb/remote.c b/gdb/remote.c
>>>> index 357e9f2..9727915 100644
>>>> --- a/gdb/remote.c
>>>> +++ b/gdb/remote.c
>>>> @@ -439,6 +439,11 @@ struct remote_arch_state
>>>>    /* Description of the remote protocol registers.  */
>>>>    long sizeof_g_packet;
>>>>
>>>> +  /* The sizeof_g_packet will be changed in some GDBstub (QEMU) when it executes.
>>>> +     Description of the remote protocol registers that get form current
>>>> +     gdbarch.  */
>>>> +  long gdbarch_sizeof_g_packet;
>>>> +
>>>>    /* Description of the remote protocol registers indexed by REGNUM
>>>>       (making an array gdbarch_num_regs in size).  */
>>>>    struct packet_reg *regs;
>>>> @@ -673,7 +678,8 @@ init_remote_state (struct gdbarch *gdbarch)
>>>>
>>>>    /* Record the maximum possible size of the g packet - it may turn out
>>>>       to be smaller.  */
>>>> -  rsa->sizeof_g_packet = map_regcache_remote_table (gdbarch, rsa->regs);
>>>> +  rsa->gdbarch_sizeof_g_packet = map_regcache_remote_table (gdbarch, rsa->regs);
>>>> +  rsa->sizeof_g_packet = rsa->gdbarch_sizeof_g_packet;
>>>>
>>>>    /* Default maximum number of characters in a packet body.  Many
>>>>       remote stubs have a hardwired buffer size of 400 bytes
>>>> @@ -693,8 +699,8 @@ init_remote_state (struct gdbarch *gdbarch)
>>>>       header / footer.  NOTE: cagney/1999-10-26: I suspect that 8
>>>>       (``$NN:G...#NN'') is a better guess, the below has been padded a
>>>>       little.  */
>>>> -  if (rsa->sizeof_g_packet > ((rsa->remote_packet_size - 32) / 2))
>>>> -    rsa->remote_packet_size = (rsa->sizeof_g_packet * 2 + 32);
>>>> +  if (rsa->gdbarch_sizeof_g_packet > ((rsa->remote_packet_size - 32) / 2))
>>>> +    rsa->remote_packet_size = (rsa->gdbarch_sizeof_g_packet * 2 + 32);
>>>>
>>>>    /* Make sure that the packet buffer is plenty big enough for
>>>>       this architecture.  */
>>>> @@ -6070,7 +6076,7 @@ process_g_packet (struct regcache *regcache)
>>>>    buf_len = strlen (rs->buf);
>>>>
>>>>    /* Further sanity checks, with knowledge of the architecture.  */
>>>> -  if (buf_len > 2 * rsa->sizeof_g_packet)
>>>> +  if (buf_len > 2 * rsa->gdbarch_sizeof_g_packet)
>>>>      error (_("Remote 'g' packet reply is too long: %s"), rs->buf);
>>>>
>>>>    /* Save the size of the packet sent to us by the target.  It is used
>>>> @@ -6079,11 +6085,11 @@ process_g_packet (struct regcache *regcache)
>>>>    if (rsa->actual_register_packet_size == 0)
>>>>      rsa->actual_register_packet_size = buf_len;
>>>>
>>>> -  /* If this is smaller than we guessed the 'g' packet would be,
>>>> +  /* If this is not same with we guessed the 'g' packet would be,
>>>>       update our records.  A 'g' reply that doesn't include a register's
>>>>       value implies either that the register is not available, or that
>>>>       the 'p' packet must be used.  */
>>>> -  if (buf_len < 2 * rsa->sizeof_g_packet)
>>>> +  if (buf_len != 2 * rsa->sizeof_g_packet)
>>>>      {
>>>>        rsa->sizeof_g_packet = buf_len / 2;
>>>>
>>>> --
>>>> 1.9.3
>>>>
>
> Hi.
>
> When I've used qemu and seen this issue, it's because the inferior has
> stopped with the current arch different than what gdb thinks.
> E.g., if I boot linux on qemu and then Ctrl-C, then when qemu talks to
> gdb it either presents a 32bit or a 64bit arch to gdb, depending on
> what program qemu was currently running (the kernel may be 64 bit but
> the kernel can be running a mix of 32bit and 64bit programs).
>
> Thus gdb is being presented, randomly, with 32bit or 64bit arches over
> the same remote connection, which obviously isn't going to work.
>
> So I guess my first question is, is this patch just a bandaid?
> IOW,  the problem here is more than just packet size, isn't it?

Will GDB be hurted if it support bigger size?  Maybe GDB can output a
warning like "the size that you use is bigger than xxxxxx", And keep
work.
But not just tell the user "your target is cheater.  It changed the
size.  I hate that.  I will not work with it."
GDB is a debugger but not a judge, right?

> GDB currently doesn't support debugging of i386 and amd64 arches over
> one remote debug connection.

https://github.com/teawater/binutils-gdb/commit/c50396c9d0d07be08e3fae5248c30d5b81def602

Try it if you meet the issue.(For the people that search this issue.)

Thanks,
Hui