[Lldb-commits] [lldb] [lldb-dap] Use protocol types for ReadMemory request (PR #144552)

[John Harrison via lldb-commits]

================
@@ -7,136 +7,57 @@
 //===----------------------------------------------------------------------===//
 
 #include "DAP.h"
-#include "EventHelper.h"
 #include "JSONUtils.h"
 #include "RequestHandler.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Base64.h"
 
 namespace lldb_dap {
 
-// "ReadMemoryRequest": {
-//   "allOf": [ { "$ref": "#/definitions/Request" }, {
-//     "type": "object",
-//     "description": "Reads bytes from memory at the provided location. Clients
-//                     should only call this request if the corresponding
-//                     capability `supportsReadMemoryRequest` is true.",
-//     "properties": {
-//       "command": {
-//         "type": "string",
-//         "enum": [ "readMemory" ]
-//       },
-//       "arguments": {
-//         "$ref": "#/definitions/ReadMemoryArguments"
-//       }
-//     },
-//     "required": [ "command", "arguments" ]
-//   }]
-// },
-// "ReadMemoryArguments": {
-//   "type": "object",
-//   "description": "Arguments for `readMemory` request.",
-//   "properties": {
-//     "memoryReference": {
-//       "type": "string",
-//       "description": "Memory reference to the base location from which data
-//                       should be read."
-//     },
-//     "offset": {
-//       "type": "integer",
-//       "description": "Offset (in bytes) to be applied to the reference
-//                       location before reading data. Can be negative."
-//     },
-//     "count": {
-//       "type": "integer",
-//       "description": "Number of bytes to read at the specified location and
-//                       offset."
-//     }
-//   },
-//   "required": [ "memoryReference", "count" ]
-// },
-// "ReadMemoryResponse": {
-//   "allOf": [ { "$ref": "#/definitions/Response" }, {
-//     "type": "object",
-//     "description": "Response to `readMemory` request.",
-//     "properties": {
-//       "body": {
-//         "type": "object",
-//         "properties": {
-//           "address": {
-//             "type": "string",
-//             "description": "The address of the first byte of data returned.
-//                             Treated as a hex value if prefixed with `0x`, or
-//                             as a decimal value otherwise."
-//           },
-//           "unreadableBytes": {
-//             "type": "integer",
-//             "description": "The number of unreadable bytes encountered after
-//                             the last successfully read byte.\nThis can be
-//                             used to determine the number of bytes that should
-//                             be skipped before a subsequent
-//             `readMemory` request succeeds."
-//           },
-//           "data": {
-//             "type": "string",
-//             "description": "The bytes read from memory, encoded using base64.
-//                             If the decoded length of `data` is less than the
-//                             requested `count` in the original `readMemory`
-//                             request, and `unreadableBytes` is zero or
-//                             omitted, then the client should assume it's
-//                             reached the end of readable memory."
-//           }
-//         },
-//         "required": [ "address" ]
-//       }
-//     }
-//   }]
-// },
-void ReadMemoryRequestHandler::operator()(
-    const llvm::json::Object &request) const {
-  llvm::json::Object response;
-  FillResponse(request, response);
-  auto *arguments = request.getObject("arguments");
+// Reads bytes from memory at the provided location.
+//
+// Clients should only call this request if the corresponding capability
+// `supportsReadMemoryRequest` is true
+llvm::Expected<protocol::ReadMemoryResponse>
+ReadMemoryRequestHandler::Run(const protocol::ReadMemoryArguments &args) const {
 
-  llvm::StringRef memoryReference =
-      GetString(arguments, "memoryReference").value_or("");
-  auto addr_opt = DecodeMemoryReference(memoryReference);
-  if (!addr_opt.has_value()) {
-    response["success"] = false;
-    response["message"] =
-        "Malformed memory reference: " + memoryReference.str();
-    dap.SendJSON(llvm::json::Value(std::move(response)));
-    return;
-  }
-  lldb::addr_t addr_int = *addr_opt;
-  addr_int += GetInteger<uint64_t>(arguments, "offset").value_or(0);
-  const uint64_t count_requested =
-      GetInteger<uint64_t>(arguments, "count").value_or(0);
+  const std::optional<lldb::addr_t> addr_opt =
+      DecodeMemoryReference(args.memoryReference);
+  if (!addr_opt)
+    return llvm::make_error<DAPError>(
+        llvm::formatv("Malformed memory reference: {}", args.memoryReference));
+
+  const lldb::addr_t raw_address = *addr_opt + args.offset.value_or(0);
+
+  lldb::SBProcess process = dap.target.GetProcess();
+  if (!lldb::SBDebugger::StateIsStoppedState(process.GetState()))
+    return llvm::make_error<NotStoppedError>();
 
+  const uint64_t count_read = std::max<uint64_t>(args.count, 1);
   // 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);
+  auto buffer = std::vector<uint8_t>(count_read);
----------------
ashgti wrote:

Should we use `std::vector<std::byte>` for the raw memory? I think its really the same type under the hood, but I think its a little more clear.

https://github.com/llvm/llvm-project/pull/144552