[lldb-dap] Migrate disassemble request to structured handler (#140482)

Author: eronnen

lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp

@@ -9,113 +9,37 @@
 #include "DAP.h"
 #include "EventHelper.h"
 #include "JSONUtils.h"
+#include "Protocol/ProtocolRequests.h"
+#include "Protocol/ProtocolTypes.h"
 #include "RequestHandler.h"
 #include "lldb/API/SBInstruction.h"
+#include "lldb/lldb-types.h"
 #include "llvm/ADT/StringExtras.h"
+#include <optional>
+
+using namespace lldb_dap::protocol;
 
 namespace lldb_dap {
 
-// "DisassembleRequest": {
-//   "allOf": [ { "$ref": "#/definitions/Request" }, {
-//     "type": "object",
-//     "description": "Disassembles code stored at the provided
-//     location.\nClients should only call this request if the corresponding
-//     capability `supportsDisassembleRequest` is true.", "properties": {
-//       "command": {
-//         "type": "string",
-//         "enum": [ "disassemble" ]
-//       },
-//       "arguments": {
-//         "$ref": "#/definitions/DisassembleArguments"
-//       }
-//     },
-//     "required": [ "command", "arguments" ]
-//   }]
-// },
-// "DisassembleArguments": {
-//   "type": "object",
-//   "description": "Arguments for `disassemble` request.",
-//   "properties": {
-//     "memoryReference": {
-//       "type": "string",
-//       "description": "Memory reference to the base location containing the
-//       instructions to disassemble."
-//     },
-//     "offset": {
-//       "type": "integer",
-//       "description": "Offset (in bytes) to be applied to the reference
-//       location before disassembling. Can be negative."
-//     },
-//     "instructionOffset": {
-//       "type": "integer",
-//       "description": "Offset (in instructions) to be applied after the byte
-//       offset (if any) before disassembling. Can be negative."
-//     },
-//     "instructionCount": {
-//       "type": "integer",
-//       "description": "Number of instructions to disassemble starting at the
-//       specified location and offset.\nAn adapter must return exactly this
-//       number of instructions - any unavailable instructions should be
-//       replaced with an implementation-defined 'invalid instruction' value."
-//     },
-//     "resolveSymbols": {
-//       "type": "boolean",
-//       "description": "If true, the adapter should attempt to resolve memory
-//       addresses and other values to symbolic names."
-//     }
-//   },
-//   "required": [ "memoryReference", "instructionCount" ]
-// },
-// "DisassembleResponse": {
-//   "allOf": [ { "$ref": "#/definitions/Response" }, {
-//     "type": "object",
-//     "description": "Response to `disassemble` request.",
-//     "properties": {
-//       "body": {
-//         "type": "object",
-//         "properties": {
-//           "instructions": {
-//             "type": "array",
-//             "items": {
-//               "$ref": "#/definitions/DisassembledInstruction"
-//             },
-//             "description": "The list of disassembled instructions."
-//           }
-//         },
-//         "required": [ "instructions" ]
-//       }
-//     }
-//   }]
-// }
-void DisassembleRequestHandler::operator()(
-    const llvm::json::Object &request) const {
-  llvm::json::Object response;
-  FillResponse(request, response);
-  auto *arguments = request.getObject("arguments");
-
-  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_ptr = *addr_opt;
+/// Disassembles code stored at the provided location.
+/// Clients should only call this request if the corresponding capability
+/// `supportsDisassembleRequest` is true.
+llvm::Expected<DisassembleResponseBody>
+DisassembleRequestHandler::Run(const DisassembleArguments &args) const {
+  std::vector<DisassembledInstruction> instructions;
 
-  addr_ptr += GetInteger<int64_t>(arguments, "instructionOffset").value_or(0);
-  lldb::SBAddress addr(addr_ptr, dap.target);
-  if (!addr.IsValid()) {
-    response["success"] = false;
-    response["message"] = "Memory reference not found in the current binary.";
-    dap.SendJSON(llvm::json::Value(std::move(response)));
-    return;
-  }
+  std::optional<lldb::addr_t> addr_opt =
+      DecodeMemoryReference(args.memoryReference);
+  if (!addr_opt.has_value())
+    return llvm::make_error<DAPError>("Malformed memory reference: " +
+                                      args.memoryReference);
 
-  const auto inst_count =
-      GetInteger<int64_t>(arguments, "instructionCount").value_or(0);
+  lldb::addr_t addr_ptr = *addr_opt;
+  addr_ptr += args.instructionOffset.value_or(0);
+  lldb::SBAddress addr(addr_ptr, dap.target);
+  if (!addr.IsValid())
+    return llvm::make_error<DAPError>(
+        "Memory reference not found in the current binary.");
 
   std::string flavor_string;
   const auto target_triple = llvm::StringRef(dap.target.GetTriple());
@@ -132,19 +56,14 @@ void DisassembleRequestHandler::operator()(
     }
   }
 
-  lldb::SBInstructionList insts =
-      dap.target.ReadInstructions(addr, inst_count, flavor_string.c_str());
+  lldb::SBInstructionList insts = dap.target.ReadInstructions(
+      addr, args.instructionCount, flavor_string.c_str());
 
-  if (!insts.IsValid()) {
-    response["success"] = false;
-    response["message"] = "Failed to find instructions for memory address.";
-    dap.SendJSON(llvm::json::Value(std::move(response)));
-    return;
-  }
+  if (!insts.IsValid())
+    return llvm::make_error<DAPError>(
+        "Failed to find instructions for memory address.");
 
-  const bool resolveSymbols =
-      GetBoolean(arguments, "resolveSymbols").value_or(false);
-  llvm::json::Array instructions;
+  const bool resolve_symbols = args.resolveSymbols.value_or(false);
   const auto num_insts = insts.GetSize();
   for (size_t i = 0; i < num_insts; ++i) {
     lldb::SBInstruction inst = insts.GetInstructionAtIndex(i);
@@ -165,11 +84,10 @@ void DisassembleRequestHandler::operator()(
       }
     }
 
-    llvm::json::Object disassembled_inst{
-        {"address", "0x" + llvm::utohexstr(inst_addr)},
-        {"instructionBytes",
-         bytes.size() > 0 ? bytes.substr(0, bytes.size() - 1) : ""},
-    };
+    DisassembledInstruction disassembled_inst;
+    disassembled_inst.address = inst_addr;
+    disassembled_inst.instructionBytes =
+        bytes.size() > 0 ? bytes.substr(0, bytes.size() - 1) : "";
 
     std::string instruction;
     llvm::raw_string_ostream si(instruction);
@@ -185,59 +103,53 @@ void DisassembleRequestHandler::operator()(
                                                 : symbol.GetName())
          << ": ";
 
-      if (resolveSymbols) {
-        disassembled_inst.try_emplace("symbol", symbol.GetDisplayName());
-      }
+      if (resolve_symbols)
+        disassembled_inst.symbol = symbol.GetDisplayName();
     }
 
     si << llvm::formatv("{0,7} {1,12}", m, o);
     if (c && c[0]) {
       si << " ; " << c;
     }
 
-    disassembled_inst.try_emplace("instruction", instruction);
+    disassembled_inst.instruction = instruction;
 
     auto line_entry = addr.GetLineEntry();
     // If the line number is 0 then the entry represents a compiler generated
     // location.
     if (line_entry.GetStartAddress() == addr && line_entry.IsValid() &&
         line_entry.GetFileSpec().IsValid() && line_entry.GetLine() != 0) {
       auto source = CreateSource(line_entry);
-      disassembled_inst.try_emplace("location", source);
+      disassembled_inst.location = std::move(source);
 
       const auto line = line_entry.GetLine();
-      if (line && line != LLDB_INVALID_LINE_NUMBER) {
-        disassembled_inst.try_emplace("line", line);
-      }
+      if (line != 0 && line != LLDB_INVALID_LINE_NUMBER)
+        disassembled_inst.line = line;
+
       const auto column = line_entry.GetColumn();
-      if (column && column != LLDB_INVALID_COLUMN_NUMBER) {
-        disassembled_inst.try_emplace("column", column);
-      }
+      if (column != 0 && column != LLDB_INVALID_COLUMN_NUMBER)
+        disassembled_inst.column = column;
 
       auto end_line_entry = line_entry.GetEndAddress().GetLineEntry();
       if (end_line_entry.IsValid() &&
           end_line_entry.GetFileSpec() == line_entry.GetFileSpec()) {
         const auto end_line = end_line_entry.GetLine();
-        if (end_line && end_line != LLDB_INVALID_LINE_NUMBER &&
+        if (end_line != 0 && end_line != LLDB_INVALID_LINE_NUMBER &&
             end_line != line) {
-          disassembled_inst.try_emplace("endLine", end_line);
+          disassembled_inst.endLine = end_line;
 
           const auto end_column = end_line_entry.GetColumn();
-          if (end_column && end_column != LLDB_INVALID_COLUMN_NUMBER &&
-              end_column != column) {
-            disassembled_inst.try_emplace("endColumn", end_column - 1);
-          }
+          if (end_column != 0 && end_column != LLDB_INVALID_COLUMN_NUMBER &&
+              end_column != column)
+            disassembled_inst.endColumn = end_column - 1;
         }
       }
     }
 
-    instructions.emplace_back(std::move(disassembled_inst));
+    instructions.push_back(std::move(disassembled_inst));
   }
 
-  llvm::json::Object body;
-  body.try_emplace("instructions", std::move(instructions));
-  response.try_emplace("body", std::move(body));
-  dap.SendJSON(llvm::json::Value(std::move(response)));
+  return DisassembleResponseBody{std::move(instructions)};
 }
 
 } // namespace lldb_dap

lldb/tools/lldb-dap/Handler/RequestHandler.h

@@ -534,14 +534,17 @@ class LocationsRequestHandler : public LegacyRequestHandler {
   void operator()(const llvm::json::Object &request) const override;
 };
 
-class DisassembleRequestHandler : public LegacyRequestHandler {
+class DisassembleRequestHandler final
+    : public RequestHandler<protocol::DisassembleArguments,
+                            llvm::Expected<protocol::DisassembleResponseBody>> {
 public:
-  using LegacyRequestHandler::LegacyRequestHandler;
+  using RequestHandler::RequestHandler;
   static llvm::StringLiteral GetCommand() { return "disassemble"; }
   FeatureSet GetSupportedFeatures() const override {
     return {protocol::eAdapterFeatureDisassembleRequest};
   }
-  void operator()(const llvm::json::Object &request) const override;
+  llvm::Expected<protocol::DisassembleResponseBody>
+  Run(const protocol::DisassembleArguments &args) const override;
 };
 
 class ReadMemoryRequestHandler : public LegacyRequestHandler {

lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp

@@ -460,4 +460,22 @@ llvm::json::Value toJSON(const SetDataBreakpointsResponseBody &SDBR) {
   return result;
 }
 
+bool fromJSON(const llvm::json::Value &Params, DisassembleArguments &DA,
+              llvm::json::Path P) {
+  json::ObjectMapper O(Params, P);
+  return O && O.map("memoryReference", DA.memoryReference) &&
+         O.mapOptional("offset", DA.offset) &&
+         O.mapOptional("instructionOffset", DA.instructionOffset) &&
+         O.map("instructionCount", DA.instructionCount) &&
+         O.mapOptional("resolveSymbols", DA.resolveSymbols);
+}
+
+llvm::json::Value toJSON(const DisassembleResponseBody &DRB) {
+  llvm::json::Array instructions;
+  for (const auto &instruction : DRB.instructions) {
+    instructions.push_back(toJSON(instruction));
+  }
+  return llvm::json::Object{{"instructions", std::move(instructions)}};
+}
+
 } // namespace lldb_dap::protocol

lldb/tools/lldb-dap/Protocol/ProtocolRequests.h

@@ -726,6 +726,44 @@ struct SetDataBreakpointsResponseBody {
 };
 llvm::json::Value toJSON(const SetDataBreakpointsResponseBody &);
 
+/// Arguments to `disassemble` request.
+struct DisassembleArguments {
+  /// Memory reference to the base location containing the instructions to
+  /// disassemble.
+  std::string memoryReference;
+
+  /// Offset (in bytes) to be applied to the reference location before
+  /// disassembling. Can be negative.
+  std::optional<int64_t> offset;
+
+  /// Offset (in instructions) to be applied after the byte offset (if any)
+  /// before disassembling. Can be negative.
+  std::optional<int64_t> instructionOffset;
+
+  /// Number of instructions to disassemble starting at the specified location
+  /// and offset.
+  /// An adapter must return exactly this number of instructions - any
+  /// unavailable instructions should be replaced with an implementation-defined
+  /// 'invalid instruction' value.
+  uint32_t instructionCount;
+
+  /// If true, the adapter should attempt to resolve memory addresses and other
+  /// values to symbolic names.
+  std::optional<bool> resolveSymbols;
+};
+bool fromJSON(const llvm::json::Value &, DisassembleArguments &,
+              llvm::json::Path);
+llvm::json::Value toJSON(const DisassembleArguments &);
+
+/// Response to `disassemble` request.
+struct DisassembleResponseBody {
+  /// The list of disassembled instructions.
+  std::vector<DisassembledInstruction> instructions;
+};
+bool fromJSON(const llvm::json::Value &, DisassembleResponseBody &,
+              llvm::json::Path);
+llvm::json::Value toJSON(const DisassembleResponseBody &);
+
 } // namespace lldb_dap::protocol
 
 #endif