[Lldb-commits] [lldb] [gdbremote] Document MultiMemRead packet in protocol extensions (PR #162675)

[Jason Molenda via lldb-commits]

jasonmolenda wrote:

I chatted with Felipe.  I hadn't followed the original RFC very well after it went up and missed the discussion about an `options` key-value. I think people are trying to make this packet more extensible than gdb remote serial packets normally are.

Extensibility can come in two forms:  adding additional behaviors, or changing existing behaviors.  The gdb remote serial protocol packets with key-value pairs or JSON arguments can add additional behaviors well.  They do not handle changing existing behaviors well -- or at all, usually, without adding some additional hint like in `qSupported` or something.

An example of where this has been a problem in the past would be the `QEnvironment` packet, which accepted an environment variable to set in a launched process, NAME=VALUE, in plain text.  Eventually someone tried to send a name or value with one of the critical gdb remote serial protocol characters like # or $, and it was a big problem.  And so we now have `QEnvironmentHexEncoded` which takes asciihex encoded NAME=value.

The documentation here (and impementation in the other PR) are trying to use the `options` field to allow for changing existing behavior.  For instance, the current packet uses the gdb remote serial protocol binary escape scheme, where the channel is assumed to be 8 bit clean and we need to escape 4 characters.  If we wanted to use this packet over a channel that was only 7 bit clean (this will not happen), we would need to change the packet to send its read bytes in asciihex instead of the binary escape protocol -- a fundamental change in the initial implementation behavior.  The scheme as implemented right now, is that if a debugger sends an `options:` key-value pair with any value in it, an existing stub will error out, assuming that a change in behavior is required.

In short, this is a versioning field.  If we want to do this, we might as well stick a `version:1` key-value in the packet.  We've never done this for a gdb remote serial protocol packet, but if we think there is a need/value in doing this, I'd say let's make it obvious what it is.

And as a far less important aside, don't hardcode the order of key-values.  It's a dictionary/map/hash/KV array, order is not significant.

https://github.com/llvm/llvm-project/pull/162675