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))); } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
