[gdbremote] Document MultiMemRead packet in protocol extensions

[felipepiovezan]

This adds a specification for the new packet discussed in the RFC 1.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Felipe de Azevedo Piovezan (felipepiovezan)

Changes

This adds a specification for the new packet discussed in the RFC 1.

---

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

1 Files Affected:

• (modified) lldb/docs/resources/lldbgdbremote.md (+38)

diff --git a/lldb/docs/resources/lldbgdbremote.md b/lldb/docs/resources/lldbgdbremote.md
index 36b95f1073ebc..d5e42890e14b1 100644
--- a/lldb/docs/resources/lldbgdbremote.md
+++ b/lldb/docs/resources/lldbgdbremote.md
@@ -2530,3 +2530,41 @@ read packet: $e0030100#b9
 
 **Priority to Implement:** Only required for Wasm support. Necessary to show
 variables.
+
+### MultiMemRead
+
+Read memory from multiple memory addresses.
+
+There are two arguments to the request:
+
+* `ranges`: a list of base-16 pairs of numbers. Each pair is separated by a
+`,`, as is each number in the pair. The first number of the pair denotes the
+base address of the memory read, the second denotes the number of bytes to be
+read.
+* `options`: an optional string of options. If present, it may be empty. If
+present, it is the last argument of the request.
+
+Both arguments must end with a `;`.
+
+The reply packet starts with a comma-separated list of base-16 numbers,
+denoting how many bytes were read from each address, followed by a `;`,
+followed by a sequence of bytes containing the memory data. The length of this
+sequence must be equal to the sum of the numbers provided at the start of the
+reply.
+
+If the stub is unable to read from any individual address, it should return a
+length of "zero" for that address in the reply packet.
+
+A stub that supports this packet should return `MultiMemRead+` in the reply to
+`qSupported`.
+
+```
+send packet: $MultiMemRead:ranges:100a00,4,200200,a0,400000,4;
+read packet: $4,0,2;<binary encoding of abcd1000><binary encoding of eeff>
+```
+
+In the example above, the first read produced `abcd1000`, the read of `a0`
+bytes from address `200200` failed, and the third read produced two bytes –
+`eeff` – out of the four requested.
+
+**Priority to Implement:** Only required for performance.

[jasonmolenda]

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.

[felipepiovezan]

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.

I think we should remove this field entirely and let extensions be done through extra information in the reply to qSupported

[felipepiovezan]

Addressed review comments. Please have a look again!

[jasonmolenda]

Looks good to me with the edits, but David's feedback was more critical, I'd like to get his sign-off.

[DavidSpickett]

This is a bit ambiguous because "unable to read any" could mean they all failed or just one.

You need a way to say "if by applying the rules above, the stub has read 0 bytes from the range...".

[DavidSpickett]

Actually I'm the one that said to say "any" but I was wrong :) Zero is clearer.

[felipepiovezan]

Addressed review comments

[DavidSpickett]

LGTM