llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Adrian Vogelsgesang (vogelsgesang) <details> <summary>Changes</summary> 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. We no longer set the `unreadableBytes` in the result. But this is optional, anyway, and afaik the VS Code UI would not yet make good use of `unreadableBytes`, anyway. --- Full diff: https://github.com/llvm/llvm-project/pull/109485.diff 2 Files Affected: - (modified) lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py (+2-3) - (modified) lldb/tools/lldb-dap/lldb-dap.cpp (+19-50) ``````````diff 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..9c4bb794dccb61 100644 --- a/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py +++ b/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py @@ -93,7 +93,6 @@ 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") # Use an offset @@ -101,7 +100,7 @@ def test_readMemory(self): 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 +108,4 @@ 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") + self.assertEqual(mem["message"], "memory read failed for 0x0") diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index 2fb86f675b4516..1ee04f0089179e 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -4405,14 +4405,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()) { @@ -4422,57 +4414,34 @@ void request_readMemory(const llvm::json::Object &request) { g_dap.SendJSON(llvm::json::Value(std::move(response))); return; } - lldb::addr_t addr = *addr_opt; + lldb::addr_t addr_int = *addr_opt; + addr_int += GetSigned(arguments, "offset", 0); + const uint64_t count_requested = GetUnsigned(arguments, "count", 0); - 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::SBAddress addr = + g_dap.target.ResolveLoadAddress(addr_int); + + // 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; + size_t count_result = + g_dap.target.ReadMemory(addr, buf.data(), count_read, error); + if (error.Fail()) { 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))); } `````````` </details> https://github.com/llvm/llvm-project/pull/109485 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
