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

[Andrew Burgess <aburgess@redhat.com>]

Tom Tromey <tom@tromey.com> writes:

>>>>>> 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.
>
> If the remote reports the packet as unsupported, fall back to the 'm'
> packet and carry on.

The alternative to this would be to add a qSupported feature for the 'x'
packet and choose m/x based on the qSupported reply.

I mention this only for completeness really.  I've replied in another
thread that I think we should adopt a prefix character to avoid needing
to escape 'E', but this would also mean we never get an empty reply, so
probing would work fine.

> I think the docs should be explicit about what a 0-length read means.
> I tend to think this should just be an error.

I don't have a firm opinion about whether this _new_ packet should give
a success or failure for zero length access.  And I'm not convinced it
really matters, so long as it's documented!!  (I haven't checked your
patch, so you might have already done so).

Success would have the benefit of matching existing behaviour, so I'd be
slightly more inclined to take that approach.

Thanks,
Andrew

>
> 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.
>
> thanks,
> Tom