[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.

(cherry picked from commit 786dc5a)

Author: vogelsgesang

lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py

@@ -93,9 +93,12 @@ 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")
@@ -105,12 +108,11 @@ def test_readMemory(self):
         self.assertEqual(b64decode(mem["data"])[1:], b"dead\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"], "")
 
         # 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")

lldb/tools/lldb-dap/lldb-dap.cpp

@@ -4425,14 +4425,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()) {
@@ -4442,57 +4434,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)));
 }