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

[Simon Marchi <simon.marchi@ericsson.com>]

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