Re: [PATCH v2 0/7] Support reading/writing memory on architectures with non 8-bits addressable memory

[Simon Marchi <simon dot marchi at ericsson dot com>]

On 15-06-11 05:06 PM, Simon Marchi wrote:
> On 15-05-21 01:44 PM, Pedro Alves wrote:
>> This confuses me and gives me lots of pause.  My immediate reaction
>> is, "well, that's odd. what's different compared to MI here?".  I'm
>> not imagining what exactly ends up being easier?
>>
>> But anyway, I guess it's a small detail.  I'll review ignoring
>> that.
> 
> I was trying to reply and explain the logic behind my choice, but the
> more I developed, the more I felt it was not really founded.
> 
> My reasoning about the difference was the following:
> 
> """
> In RSP, the size is there for GDB to tell the target "here is how much data
> I am handing you". Its function is to describe the data found in the packet.
> It's more there as a matter convenience, saying how much data follows, than
> representing an amount of target memory units.
> 
> In MI, the size is not describing the amount of data you are handing GDB. It
> represents a size in the context of the memory of the target.
> """
> 
> However, the paragraph about RSP doesn't make much sense. We are the ones
> defining the protocol, so the field can mean whatever we want. If we want it
> to be in target addressable memory units, then there is nothing that prevents
> that. It just needs to be well documented. Also, I realize that writing memory
> would be the only place where we would be talking about the target's memory in
> bytes, so it's not so really consistent.
> 
> About the "it is easier" part, I initially found that the common part of gdbserver
> would be less encumbered with code related to this if those sizes were in bytes.
> Namely,
> 
>  * decode_M_packet
>  * decode_X_packet
>  * write_inferior_memory
>  * read_inferior_memory
>  * code related to this in process_serial_event
> 
> could just operate on bytes and work without knowledge of the addressable memory unit.
> However, upon closer inspection, it seems like other functions will need to be aware
> of it anyway, such as check_mem_read and check_mem_write. These two functions do
> computations on addresses, so they need to have the length of the read/write in
> addressable units and not bytes. If we do it for these two functions, then it's
> not much more work to do it cleanly for the other functions as well.
> 
> Here is a draft of how the changes would look like in gdbserver when using addressable
> memory units. It's really not that bad I think.
> 
> https://github.com/simark/binutils-gdb/commit/2ecb2f054a288053e3726e92fb6126dd4c782a15
> 
> So in the end, it might be more consistent to use addressable memory units everywhere
> in the RSP, and not more complicated to implement. Of course, that's only for things
> related to the target memory, things that fetch an XML file would still be in bytes.
> 
> What is your opinion on this?
> 
>>>
>>>    -> $m1000,8#??
>>>    <- aaaabbbbccccdddd
>>>
>>>    -> $M1000,6:eeeeffffeeee#??
>>>    <- OK
>>>
>>>    -> $m1000,8#??
>>>    <- eeeeffffeeeedddd
>>>
>>> If there are any other RSP packets or MI commands that need such
>>> clarification, it will be on a case-by-case basis, whatever makes more
>>> sense for each particular one.
>>
>> Off hand, I thought of qCRC and qSearch:memory.  The latter is
>> more interesting:
>>
>> - Would you allow searching for an 1 8-bit byte pattern?
> 
> Hmm I don't know. To be safe I'd say no. If we do, it means we need to
> search with a granularity of a byte. What if you search for the pattern
> 0x2345 in this memory:
> 
> 0x100 0123
> 0x101 4567
> 0x102 89ab
> 0x103 cdef
> 
> Should there be a match that spans halves of two addresses? Unless we only
> search with a byte granularity in the special case where the pattern is
> one byte long? But then what about 3-bytes patterns?
> 
> I think it's a lot of corner cases for not much value. I think it could be
> enhanced later to support it if somebody needs it.
> 
>> - So what length would you use for that one?  Host byte
>>   or addressable units?
> 
> Length here would be in addressable units.
> 
>> Thanks,
>> Pedro Alves
>>
> 
> Thanks,
> 
> Simon

Oh, and I acknowledged your comments in the individual patches, they are all
clear, thanks a lot.

Simon