🍒 [lldb] Cherry pick patches for MultiMemoryRead packet p2 (6.2) #11697
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit introduces a base-class implementation for a method that
reads memory from multiple ranges at once. This implementation simply
calls the underlying `ReadMemoryFromInferior` method on each requested
range, intentionally bypassing the memory caching mechanism (though this
may be easily changed in the future).
`Process` implementations that can be perform this operation more
efficiently - e.g. with the MultiMemPacket described in [1] - are
expected to override this method.
As an example, this commit changes AppleObjCClassDescriptorV2 to use the
new API.
Note about the API
------------------
In the RFC, we discussed having the API return some kind of class
`ReadMemoryRangesResult`. However, while writing such a class, it became
clear that it was merely wrapping a vector, without providing anything
useful. For example, this class:
```
struct ReadMemoryRangesResult {
ReadMemoryRangesResult(
llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> ranges)
: ranges(std::move(ranges)) {}
llvm::ArrayRef<llvm::MutableArrayRef<uint8_t>> getRanges() const {
return ranges;
}
private:
llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> ranges;
};
```
As can be seen in the added test and in the added use-case
(AppleObjCClassDescriptorV2), users of this API will just iterate over
the vector of memory buffers. So they want a return type that can be
iterated over, and the vector seems more natural than creating a new
class and defining iterators for it.
Likewise, in the RFC, we discussed wrapping the result into an
`Expected`. Upon experimenting with the code, this feels like it limits
what the API is able to do as the base class implementation never needs
to fail the entire result, it's the individual reads that may fail and
this is expressed through a zero-length result. Any derived classes
overriding `ReadMemoryRanges` should also never produce a top level
failure: if they did, they can just fall back to the base class
implementation, which would produce a better result.
The choice of having the caller allocate a buffer and pass it to
`Process::ReadMemoryRanges` is done mostly to follow conventions already
done in the Process class.
[1]:
https://discourse.llvm.org/t/rfc-a-new-vectorized-memory-read-packet/
(cherry picked from commit f2cb997)
(cherry picked from commit bccb899)
…164311) This commit makes use of the newly created MultiMemRead packet to provide an efficient implementation of MultiMemRead inside ProcessGDBRemote. Testing is tricky, but it is accomplished two ways: 1. Some Objective-C tests would fail if this were implemented incorrectly, as there is already an in-tree use of the base class implementation of MultiMemRead, which is now getting replaced by the derived class. 2. One Objective-C test is modified so that we ensure the packet is being sent by looking at the packet logs. While not the most elegant solution, it is a strategy adopted in other tests as well. This gets around the fact that we cannot instantiate / unittest a mock ProcessGDBRemote. Depends on llvm#163651 (cherry picked from commit 276bccd) (cherry picked from commit 8e1255f)
Once the proposal [1] is implemented, the Process class will have the ability to read multiple memory addresses at once. This patch is an NFC refactor to make the code more amenable to that future. [1]: https://discourse.llvm.org/t/rfc-a-new-vectorized-memory-read-packet/ (cherry picked from commit fb7c49e)
…eads part 2 This NFC commit converts the helper function FindTaskId into FindTaskIds, operating on an array of task addresses instead of a single task address. This will make the subsequent diff to use a vectorized memory read much smaller. (cherry picked from commit fcab7f1)
|
@swift-ci test |
These are methods that use the MultiMemRead packet to speed up reading unsigned numbers / pointers from memory. As the comments say, they should go upstream once we find a use for them there (and the timing constraints are a bit more favourable). (cherry picked from commit 7ef61a7)
This concludes the work to remove the extra packets created by OperatingSystemSwiftTasks. (cherry picked from commit ce39e63)
felipepiovezan
force-pushed
the
felipe/multimem_cherry_picks_p2_62
branch
from
acc5765 to
a4f99fb
Compare
|
@swift-ci test |
This branch is missing some ADT / iterator / ArrayRef functionality from recent versions of LLVM.
|
@swift-ci test |
|
@swift-ci test macos platform |
JDevlieghere
approved these changes
Oct 25, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the last set
rdar://163364181