[Lldb-commits] [lldb] 786dc5a - [lldb-dap] Simplify `readMemory` (#109485)
via lldb-commits
lldb-commits at lists.llvm.org
Wed Sep 25 04:49:46 PDT 2024
Author: Adrian Vogelsgesang
Date: 2024-09-25T13:49:42+02:00
New Revision: 786dc5a2da9bb55d98c65d018de25d9bd31485ff
URL: https://github.com/llvm/llvm-project/commit/786dc5a2da9bb55d98c65d018de25d9bd31485ff
DIFF: https://github.com/llvm/llvm-project/commit/786dc5a2da9bb55d98c65d018de25d9bd31485ff.diff
LOG: [lldb-dap] Simplify `readMemory` (#109485)
The `readMemory` request used the `MemoryRegionInfo` so it could also
support short reads. Since #106532, this is no longer necessary, as
mentioned by @labath in a comment on #104317.
With this commit, we no longer set the `unreadableBytes` in the result.
But this is optional, anyway, according to the spec, and afaik the
VS Code UI does not make good use of `unreadableBytes`, anyway.
We prefer `SBTarget::ReadMemory` over `SBProcess::ReadMemory`, because
the `memory read` command also reads memory through the target instead
of the process, and because users would expect the UI view and the
results from memory read to be in-sync.
Added:
Modified:
lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py
lldb/tools/lldb-dap/lldb-dap.cpp
Removed:
################################################################################
diff --git a/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py b/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py
index 1082541aebcf7c..ea43fccf016a7f 100644
--- a/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py
+++ b/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py
@@ -93,15 +93,18 @@ def test_readMemory(self):
# We can read the complete string
mem = self.dap_server.request_readMemory(memref, 0, 5)["body"]
- self.assertEqual(mem["unreadableBytes"], 0)
self.assertEqual(b64decode(mem["data"]), b"dead\0")
+ # We can read large chunks, potentially returning partial results
+ mem = self.dap_server.request_readMemory(memref, 0, 4096)["body"]
+ self.assertEqual(b64decode(mem["data"])[0:5], b"dead\0")
+
# Use an offset
mem = self.dap_server.request_readMemory(memref, 2, 3)["body"]
self.assertEqual(b64decode(mem["data"]), b"ad\0")
# Reads of size 0 are successful
- # VS-Code sends those in order to check if a `memoryReference` can actually be dereferenced.
+ # VS Code sends those in order to check if a `memoryReference` can actually be dereferenced.
mem = self.dap_server.request_readMemory(memref, 0, 0)
self.assertEqual(mem["success"], True)
self.assertEqual(mem["body"]["data"], "")
@@ -109,4 +112,3 @@ def test_readMemory(self):
# Reads at offset 0x0 fail
mem = self.dap_server.request_readMemory("0x0", 0, 6)
self.assertEqual(mem["success"], False)
- self.assertEqual(mem["message"], "Memory region is not readable")
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index c7653fed2def4e..f692d77347038c 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -4422,14 +4422,6 @@ void request_readMemory(const llvm::json::Object &request) {
FillResponse(request, response);
auto *arguments = request.getObject("arguments");
- lldb::SBProcess process = g_dap.target.GetProcess();
- if (!process.IsValid()) {
- response["success"] = false;
- response["message"] = "No process running";
- g_dap.SendJSON(llvm::json::Value(std::move(response)));
- return;
- }
-
llvm::StringRef memoryReference = GetString(arguments, "memoryReference");
auto addr_opt = DecodeMemoryReference(memoryReference);
if (!addr_opt.has_value()) {
@@ -4439,57 +4431,32 @@ void request_readMemory(const llvm::json::Object &request) {
g_dap.SendJSON(llvm::json::Value(std::move(response)));
return;
}
- lldb::addr_t addr = *addr_opt;
-
- addr += GetSigned(arguments, "offset", 0);
- const uint64_t requested_count = GetUnsigned(arguments, "count", 0);
- lldb::SBMemoryRegionInfo region_info;
- lldb::SBError memreg_error = process.GetMemoryRegionInfo(addr, region_info);
- if (memreg_error.Fail()) {
- response["success"] = false;
- EmplaceSafeString(response, "message",
- "Unable to find memory region: " +
- std::string(memreg_error.GetCString()));
- g_dap.SendJSON(llvm::json::Value(std::move(response)));
- return;
- }
- if (!region_info.IsReadable()) {
+ lldb::addr_t addr_int = *addr_opt;
+ addr_int += GetSigned(arguments, "offset", 0);
+ const uint64_t count_requested = GetUnsigned(arguments, "count", 0);
+
+ // We also need support reading 0 bytes
+ // VS Code sends those requests to check if a `memoryReference`
+ // can be dereferenced.
+ const uint64_t count_read = std::max<uint64_t>(count_requested, 1);
+ std::vector<uint8_t> buf;
+ buf.resize(count_read);
+ lldb::SBError error;
+ lldb::SBAddress addr{addr_int, g_dap.target};
+ size_t count_result =
+ g_dap.target.ReadMemory(addr, buf.data(), count_read, error);
+ if (count_result == 0) {
response["success"] = false;
- response.try_emplace("message", "Memory region is not readable");
+ EmplaceSafeString(response, "message", error.GetCString());
g_dap.SendJSON(llvm::json::Value(std::move(response)));
return;
}
- const uint64_t available_count =
- std::min(requested_count, region_info.GetRegionEnd() - addr);
- const uint64_t unavailable_count = requested_count - available_count;
-
- std::vector<uint8_t> buf;
- buf.resize(available_count);
- if (available_count > 0) {
- lldb::SBError memread_error;
- uint64_t bytes_read =
- process.ReadMemory(addr, buf.data(), available_count, memread_error);
- if (memread_error.Fail()) {
- response["success"] = false;
- EmplaceSafeString(response, "message",
- "Unable to read memory: " +
- std::string(memread_error.GetCString()));
- g_dap.SendJSON(llvm::json::Value(std::move(response)));
- return;
- }
- if (bytes_read != available_count) {
- response["success"] = false;
- EmplaceSafeString(response, "message", "Unexpected, short read");
- g_dap.SendJSON(llvm::json::Value(std::move(response)));
- return;
- }
- }
+ buf.resize(std::min(count_result, count_requested));
llvm::json::Object body;
- std::string formatted_addr = "0x" + llvm::utohexstr(addr);
+ std::string formatted_addr = "0x" + llvm::utohexstr(addr_int);
body.try_emplace("address", formatted_addr);
body.try_emplace("data", llvm::encodeBase64(buf));
- body.try_emplace("unreadableBytes", unavailable_count);
response.try_emplace("body", std::move(body));
g_dap.SendJSON(llvm::json::Value(std::move(response)));
}
More information about the lldb-commits
mailing list