'g' packet reply is too long error when target changes number of registers

'g' packet reply is too long error when target changes number of registers

[alexandru.sardan]

Hello,

I'm trying to debug an ARM Aarch64 target with gdb-cross and I get a
'g' packet reply is too long error in the following scenario:

* I debug my ARM target through a probe that has a gdbserver running on it
(gdbproxy).
* First I load a custom target description in gdb that reflects the current 
hardware I am debugging
* I connect to the probe with "target remote probe_ip"
* Then I configure the probe to connect to the target (using the same target
description as the one loaded in GDB)
* When I ask for the register Info (info reg), I get the 'g' packet reply 
error.

Because the probe had no knowledge of the target that will be debugged 
beforehand, the "target remote" command will force the probe to reply with 
info about a smaller number of registers (according to the default description)
than gdb expects.
This causes rsa->sizeof_g_packet to be reduced.

After configuration, the probe will send all the register information of the 
target. This causes the 'g'packet reply error because now, more registers 
are being sent.

Why is rsa->sizeof_g_packet shrunk when a smaller packet is received.
Shouldn't this reflect the size in accordance with the target description?

I've made a small patch to address this issue. Do you think it's a proper fix?


Kind regards,
Alex


Patch follows:

From 8cae18e62d544094b160878459624605c0491534 Mon Sep 17 00:00:00 2001
From: Alexandru-Cezar Sardan <alexandru.sardan@freescale.com>
Date: Tue, 21 Jan 2014 17:45:20 +0200
Subject: [PATCH] Fix remote 'g' packet reply is too long error

Fix remote 'g' packet reply is too long error
when target architecture is modified.
As long as the target description is the same as
the hardware, this error will no longer appeear

Signed-off-by: Alexandru-Cezar Sardan <alexandru.sardan@freescale.com>
---
 gdb/remote.c |   14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index 7761e00..c29d88a 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -6103,7 +6103,7 @@ process_g_packet (struct regcache *regcache)
   struct gdbarch *gdbarch = get_regcache_arch (regcache);
   struct remote_state *rs = get_remote_state ();
   struct remote_arch_state *rsa = get_remote_arch_state ();
-  int i, buf_len;
+  int i, buf_len, reg_pack_sz;
   char *p;
   char *regs;
 
@@ -6125,31 +6125,33 @@ process_g_packet (struct regcache *regcache)
      the 'p' packet must be used.  */
   if (buf_len < 2 * rsa->sizeof_g_packet)
     {
-      rsa->sizeof_g_packet = buf_len / 2;
+      reg_pack_sz = buf_len / 2;
 
       for (i = 0; i < gdbarch_num_regs (gdbarch); i++)
 	{
 	  if (rsa->regs[i].pnum == -1)
 	    continue;
 
-	  if (rsa->regs[i].offset >= rsa->sizeof_g_packet)
+	  if (rsa->regs[i].offset >= reg_pack_sz )
 	    rsa->regs[i].in_g_packet = 0;
 	  else
 	    rsa->regs[i].in_g_packet = 1;
 	}
     }
+  else
+      reg_pack_sz = rsa->sizeof_g_packet;
 
-  regs = alloca (rsa->sizeof_g_packet);
+  regs = alloca (reg_pack_sz);
 
   /* Unimplemented registers read as all bits zero.  */
-  memset (regs, 0, rsa->sizeof_g_packet);
+  memset (regs, 0, reg_pack_sz);
 
   /* Reply describes registers byte by byte, each byte encoded as two
      hex characters.  Suck them all up, then supply them to the
      register cacheing/storage mechanism.  */
 
   p = rs->buf;
-  for (i = 0; i < rsa->sizeof_g_packet; i++)
+  for (i = 0; i < reg_pack_sz; i++)
     {
       if (p[0] == 0 || p[1] == 0)
 	/* This shouldn't happen - we adjusted sizeof_g_packet above.  */
-- 
1.7.9.5

Re: 'g' packet reply is too long error when target changes number of registers

[Pedro Alves]

On 02/03/2014 02:11 PM, alexandru.sardan@freescale.com wrote:
> Hello,
> 
> I'm trying to debug an ARM Aarch64 target with gdb-cross and I get a
> 'g' packet reply is too long error in the following scenario:
> 
> * I debug my ARM target through a probe that has a gdbserver running on it
> (gdbproxy).
> * First I load a custom target description in gdb that reflects the current 
> hardware I am debugging
> * I connect to the probe with "target remote probe_ip"
> * Then I configure the probe to connect to the target (using the same target
> description as the one loaded in GDB)
> * When I ask for the register Info (info reg), I get the 'g' packet reply 
> error.
> 

> Because the probe had no knowledge of the target that will be debugged 
> beforehand, the "target remote" command will force the probe to reply with 
> info about a smaller number of registers (according to the default description)
> than gdb expects.

This sounds odd.  Why not?  Simply configure it before connecting
with GDB?  It sounds quite wrong to be changing the target behind
GDB's back when GDB is _already_ debugging it.  Not just the size
of the g/G packets may change inadvertently, but the layout as well.
If the target description changes with your re-configuration, it
sounds to me like GDB should fetch/recompute the whole target
description.  Today, I think that can only be done with a
disconnect/reconnect.

> This causes rsa->sizeof_g_packet to be reduced.
> 
> After configuration, the probe will send all the register information of the 
> target. This causes the 'g'packet reply error because now, more registers 
> are being sent.

> 
> Why is rsa->sizeof_g_packet shrunk when a smaller packet is received.

  /* If this is smaller than 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 we don't shrink it, then we'd send too much when writing
registers, for example, I think?

-- 
Pedro Alves

RE: 'g' packet reply is too long error when target changes number of registers

[alexandru.sardan]

Hi,

See my comments inline.

Thanks,
Alex

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Tuesday, February 04, 2014 8:45 PM
> To: Sardan Alexandru Cezar-B41700
> Cc: gdb@sourceware.org; Udma Catalin-Dan-B32721
> Subject: Re: 'g' packet reply is too long error when target changes
> number of registers
> 
> On 02/03/2014 02:11 PM, alexandru.sardan@freescale.com wrote:
> > Hello,
> >
> > I'm trying to debug an ARM Aarch64 target with gdb-cross and I get a
> > 'g' packet reply is too long error in the following scenario:
> >
> > * I debug my ARM target through a probe that has a gdbserver running on
> it
> > (gdbproxy).
> > * First I load a custom target description in gdb that reflects the
> current
> > hardware I am debugging
> > * I connect to the probe with "target remote probe_ip"
> > * Then I configure the probe to connect to the target (using the same
> target
> > description as the one loaded in GDB)
> > * When I ask for the register Info (info reg), I get the 'g' packet
> reply
> > error.
> >
> 
> > Because the probe had no knowledge of the target that will be debugged
> > beforehand, the "target remote" command will force the probe to reply
> with
> > info about a smaller number of registers (according to the default
> description)
> > than gdb expects.
> 
> This sounds odd.  Why not?  Simply configure it before connecting
> with GDB?  It sounds quite wrong to be changing the target behind
> GDB's back when GDB is _already_ debugging it.  Not just the size
> of the g/G packets may change inadvertently, but the layout as well.
> If the target description changes with your re-configuration, it
> sounds to me like GDB should fetch/recompute the whole target
> description.  Today, I think that can only be done with a
> disconnect/reconnect.
>
 
[Alex Sărdan] In our case "target remote" doesn't start the debug session,
it only connects to the probe. To start debugging we issue some monitor
commands in order to configure the probe.
Is there any other way to recompute the description without disconnect?
When using GDB with Eclipse DSF, issuing a disconnect terminates the DSF
session.

> > This causes rsa->sizeof_g_packet to be reduced.
> >
> > After configuration, the probe will send all the register information
> of the
> > target. This causes the 'g'packet reply error because now, more
> registers
> > are being sent.
> 
> >
> > Why is rsa->sizeof_g_packet shrunk when a smaller packet is received.
> 
>   /* If this is smaller than 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 we don't shrink it, then we'd send too much when writing
> registers, for example, I think?
> 

[Alex Sărdan] Shouldn't this check be handled in the server, since I'm loading
up an XML with the (presumably) correct hardware description?

> --
> Pedro Alves
> 
> 

Re: 'g' packet reply is too long error when target changes number of registers

[Pedro Alves]

On 02/05/2014 02:07 PM, alexandru.sardan@freescale.com wrote:
> Hi,
> 
> See my comments inline.
> 
> Thanks,
> Alex
> 
>> -----Original Message-----
>> From: Pedro Alves [mailto:palves@redhat.com]
>> Sent: Tuesday, February 04, 2014 8:45 PM
>> To: Sardan Alexandru Cezar-B41700
>> Cc: gdb@sourceware.org; Udma Catalin-Dan-B32721
>> Subject: Re: 'g' packet reply is too long error when target changes
>> number of registers
>>
>> On 02/03/2014 02:11 PM, alexandru.sardan@freescale.com wrote:
>>> Hello,
>>>
>>> I'm trying to debug an ARM Aarch64 target with gdb-cross and I get a
>>> 'g' packet reply is too long error in the following scenario:
>>>
>>> * I debug my ARM target through a probe that has a gdbserver running on
>> it
>>> (gdbproxy).
>>> * First I load a custom target description in gdb that reflects the
>> current
>>> hardware I am debugging
>>> * I connect to the probe with "target remote probe_ip"
>>> * Then I configure the probe to connect to the target (using the same
>> target
>>> description as the one loaded in GDB)
>>> * When I ask for the register Info (info reg), I get the 'g' packet
>> reply
>>> error.
>>>
>>

>>> Because the probe had no knowledge of the target that will be debugged
>>> beforehand, the "target remote" command will force the probe to reply
>> with
>>> info about a smaller number of registers (according to the default
>> description)
>>> than gdb expects.
>>
>> This sounds odd.  Why not?  Simply configure it before connecting
>> with GDB?  It sounds quite wrong to be changing the target behind
>> GDB's back when GDB is _already_ debugging it.  Not just the size
>> of the g/G packets may change inadvertently, but the layout as well.
>> If the target description changes with your re-configuration, it
>> sounds to me like GDB should fetch/recompute the whole target
>> description.  Today, I think that can only be done with a
>> disconnect/reconnect.
>>
>  
> [Alex SĂŁrdan] In our case "target remote" doesn't start the debug session,
> it only connects to the probe. 
>  To start debugging we issue some monitor
> commands in order to configure the probe.

That's just going against what "target remote" is designed to do.
It's in Just Don't Do That, territory.

By design, "target remote" assumes a debugging session is already ongoing.
It sounds to me extended-remote is a better map to what you want to
do.

> Is there any other way to recompute the description without disconnect?
> When using GDB with Eclipse DSF, issuing a disconnect terminates the DSF
> session.

Here's what I suggest you do:

 - connect with "target extended-remote" instead of "target remote".  The
   main difference here is that extended-remote allows connecting to a target
   that is not running yet.
 - if not configured yet, have the probe reply W00 to the status ("?") packet.
   (With that, GDB knows that nothing is running yet, but remains connected.)
 - Teach the probe about the vAttach packet (support for "attach")
 - after connecting and finding no program is running (probe is not
   configured), issue the necessary monitor commands for setup, and end with an
   attach -- e.g., "attach 1" ("1" being just a dummy pid so that GDB doesn't
   complain, but you can give it any meaning you want, if you want.
     - GDB issues the vAttach packet.
 - the probe reacts to vAttach the same way it's reacting to whatever
   monitor command you've implemented that starts the debug session.
 - GDB fetches the target description, etc. at this point, and fetches
   the current registers, etc.
 - debug session is active.

Absolutely no change to GDB is required then.

(you can play with extended-remote with gdbserver to get a feel -- pass
it the --multi switch)

>> If we don't shrink it, then we'd send too much when writing
>> registers, for example, I think?
>>
> 
> [Alex SĂŁrdan] Shouldn't this check be handled in the server, since I'm loading
> up an XML with the (presumably) correct hardware description?

What sort of check are you thinking?  All it could possibly do is
either error out, or ignore the extra registers.  The former
isn't a good indication to GDB that it should write with 'P',
and the latter is just, well, nasty.

-- 
Pedro Alves

RE: 'g' packet reply is too long error when target changes number of registers

[alexandru.sardan]

Hi,

Thanks for your advice and clarifications.
Your proposed solution seems the right way to go, I will try it out.

Regards,
Alex

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Wednesday, February 05, 2014 4:56 PM
> To: Sardan Alexandru Cezar-B41700
> Cc: gdb@sourceware.org; Udma Catalin-Dan-B32721
> Subject: Re: 'g' packet reply is too long error when target changes
> number of registers
> 
> On 02/05/2014 02:07 PM, alexandru.sardan@freescale.com wrote:
> > Hi,
> >
> > See my comments inline.
> >
> > Thanks,
> > Alex
> >
> >> -----Original Message-----
> >> From: Pedro Alves [mailto:palves@redhat.com]
> >> Sent: Tuesday, February 04, 2014 8:45 PM
> >> To: Sardan Alexandru Cezar-B41700
> >> Cc: gdb@sourceware.org; Udma Catalin-Dan-B32721
> >> Subject: Re: 'g' packet reply is too long error when target changes
> >> number of registers
> >>
> >> On 02/03/2014 02:11 PM, alexandru.sardan@freescale.com wrote:
> >>> Hello,
> >>>
> >>> I'm trying to debug an ARM Aarch64 target with gdb-cross and I get a
> >>> 'g' packet reply is too long error in the following scenario:
> >>>
> >>> * I debug my ARM target through a probe that has a gdbserver running
> on
> >> it
> >>> (gdbproxy).
> >>> * First I load a custom target description in gdb that reflects the
> >> current
> >>> hardware I am debugging
> >>> * I connect to the probe with "target remote probe_ip"
> >>> * Then I configure the probe to connect to the target (using the same
> >> target
> >>> description as the one loaded in GDB)
> >>> * When I ask for the register Info (info reg), I get the 'g' packet
> >> reply
> >>> error.
> >>>
> >>
> 
> >>> Because the probe had no knowledge of the target that will be
> debugged
> >>> beforehand, the "target remote" command will force the probe to reply
> >> with
> >>> info about a smaller number of registers (according to the default
> >> description)
> >>> than gdb expects.
> >>
> >> This sounds odd.  Why not?  Simply configure it before connecting
> >> with GDB?  It sounds quite wrong to be changing the target behind
> >> GDB's back when GDB is _already_ debugging it.  Not just the size
> >> of the g/G packets may change inadvertently, but the layout as well.
> >> If the target description changes with your re-configuration, it
> >> sounds to me like GDB should fetch/recompute the whole target
> >> description.  Today, I think that can only be done with a
> >> disconnect/reconnect.
> >>
> >
> > [Alex Sărdan] In our case "target remote" doesn't start the debug
> session,
> > it only connects to the probe.
> >  To start debugging we issue some monitor
> > commands in order to configure the probe.
> 
> That's just going against what "target remote" is designed to do.
> It's in Just Don't Do That, territory.
> 
> By design, "target remote" assumes a debugging session is already
> ongoing.
> It sounds to me extended-remote is a better map to what you want to
> do.
> 
> > Is there any other way to recompute the description without disconnect?
> > When using GDB with Eclipse DSF, issuing a disconnect terminates the
> DSF
> > session.
> 
> Here's what I suggest you do:
> 
>  - connect with "target extended-remote" instead of "target remote".  The
>    main difference here is that extended-remote allows connecting to a
> target
>    that is not running yet.
>  - if not configured yet, have the probe reply W00 to the status ("?")
> packet.
>    (With that, GDB knows that nothing is running yet, but remains
> connected.)
>  - Teach the probe about the vAttach packet (support for "attach")
>  - after connecting and finding no program is running (probe is not
>    configured), issue the necessary monitor commands for setup, and end
> with an
>    attach -- e.g., "attach 1" ("1" being just a dummy pid so that GDB
> doesn't
>    complain, but you can give it any meaning you want, if you want.
>      - GDB issues the vAttach packet.
>  - the probe reacts to vAttach the same way it's reacting to whatever
>    monitor command you've implemented that starts the debug session.
>  - GDB fetches the target description, etc. at this point, and fetches
>    the current registers, etc.
>  - debug session is active.
> 
> Absolutely no change to GDB is required then.
> 
> (you can play with extended-remote with gdbserver to get a feel -- pass
> it the --multi switch)
> 
> >> If we don't shrink it, then we'd send too much when writing
> >> registers, for example, I think?
> >>
> >
> > [Alex Sărdan] Shouldn't this check be handled in the server, since I'm
> loading
> > up an XML with the (presumably) correct hardware description?
> 
> What sort of check are you thinking?  All it could possibly do is
> either error out, or ignore the extra registers.  The former
> isn't a good indication to GDB that it should write with 'P',
> and the latter is just, well, nasty.
> 
> --
> Pedro Alves
> 
>