RE: [PATCH 3/3] gdb, gdbserver: introduce the 'x' RSP packet for binary memory read

["Aktemur, Tankut Baris" <tankut.baris.aktemur@intel.com>]

Hi Tom,

On Tuesday, March 19, 2024 5:01 PM, Tom Tromey wrote:
> >>>>> Aktemur, Tankut Baris <tankut.baris.aktemur@intel.com> writes:
> 
> >> I think the probing idea is that you simply try the 'x' command the
> >> first time it is needed.  If remote sends an empty response (not an 'E'
> >> response), then the packet isn't supported, so you disable it and retry
> >> with 'm'.
> >>
> >> This is documented in the Overview node:
> 
> > Yes, I get this idea, and the 'X' packet actually uses it.  My concern there is
> > that (1) sending this packet with an arbitrary address and length 0 seems too
> > artificial; (2) using the proposed reply format, the target would send an empty reply
> > both when it does not recognize the packet and when it recognizes because the length
> > argument in the probe is 0 (the same problem would exist with the 'm', 'p', 'g' packets
> > if we were to probe them).  This would not be an issue if we decide to force the reply
> > to always start with a marker character, as suggested in Ciaran's email.
> 
> I definitely think that gdb should not send an 'x' packet with an
> arbitrary address and a length of 0.
> 
> Instead the idea is that if the packet is not explicitly forbidden
> (either via a gdb command or by previous probe failure), just attempt to
> use this packet for the first memory read.

You mean, just as "x\0", without any arguments?  (Please also see the end
of the email about this)

> If the remote reports the packet as unsupported, fall back to the 'm'
> packet and carry on.
> 
> I think the docs should be explicit about what a 0-length read means.

I agree that the document should be clear for this.

> I tend to think this should just be an error.

My first reaction would be that it depends on the given address.  If the
address is accessible, it seems viable that a zero-length read is trivially
a success, akin to defining an empty string.  If the address is not
accessible, however, it would be an error.  So, a zero-length read attempt
could also be considered as a probe to check if the address is accessible
by the context.

Checking the code, I see in `target_write_memory`:

  /* GDB may send X packets with LEN==0, for probing packet support.
     If we let such a request go through, then buffer.data() below may
     return NULL, which may confuse target implementations.  Handle it
     here to avoid lower levels having to care about this case.  */
  if (len == 0)
    return 0;

(A return value of 0 denotes success, by the way.)
And then in `read_inferior_memory`:

  /* At the time of writing, GDB only sends write packets with LEN==0,
     not read packets (see comment in target_write_memory), but it
     doesn't hurt to prevent problems if it ever does, or we're
     connected to some client other than GDB that does.  */
  if (len == 0)
    return 0;

I had mentioned `check_binary_download` in a previous email.  In that function
GDB probes for 'X' using the first memory address it receives with a length
of 0.  From that perspective, the address is arbitrary, and I, too, find that
unpleasant.

Because of these reasons, I had preferred not probing and chose announcing
the support instead.  But if probing is still the desired approach, how about this:

We can send a plain "x\0" packet, with no arguments.  Because this doesn't conform
to the format, it would be an error, but that's sufficient to indicate that
the server supports the packet.  Would that be ok?

> I also prefer Ciaran's idea for the response packet.  I didn't realize
> other packets already did this or perhaps I would have suggested it.

Ok, thank you.  I'll make the reply start with a marker character in the next
revision.

> thanks,
> Tom

Regards
-Baris


Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928