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

eronnen · 2025-05-18T21:52:40Z

No description provided.

llvmbot · 2025-05-18T21:53:15Z

@llvm/pr-subscribers-lldb

Author: Ely Ronnen (eronnen)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/140482.diff

6 Files Affected:

(modified) lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp (+39-137)
(modified) lldb/tools/lldb-dap/Handler/RequestHandler.h (+4-3)
(modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp (+18)
(modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.h (+31)
(modified) lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp (+34)
(modified) lldb/tools/lldb-dap/Protocol/ProtocolTypes.h (+53)

diff --git a/lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp
index d738f54ff1a9f..9df831c8b8c61 100644
--- a/lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp
@@ -9,113 +9,30 @@
 #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 "llvm/ADT/StringExtras.h"
 
+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);
+  auto addr_opt = DecodeMemoryReference(args.memoryReference);
+  if (!addr_opt.has_value())
+    return llvm::make_error<DAPError>("Malformed memory reference: " + args.memoryReference);
+  
+  lldb::addr_t addr_ptr = *addr_opt;
+  addr_ptr += args.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;
-  }
-
-  const auto inst_count =
-      GetInteger<int64_t>(arguments, "instructionCount").value_or(0);
+  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());
@@ -133,18 +50,12 @@ void DisassembleRequestHandler::operator()(
   }
 
   lldb::SBInstructionList insts =
-      dap.target.ReadInstructions(addr, inst_count, flavor_string.c_str());
+      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 resolveSymbols = 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 +76,9 @@ 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 = "0x" + llvm::utohexstr(inst_addr);
+    disassembled_inst.instructionBytes = bytes.size() > 0 ? bytes.substr(0, bytes.size() - 1) : "";
 
     std::string instruction;
     llvm::raw_string_ostream si(instruction);
@@ -185,9 +94,8 @@ void DisassembleRequestHandler::operator()(
                                                 : symbol.GetName())
          << ": ";
 
-      if (resolveSymbols) {
-        disassembled_inst.try_emplace("symbol", symbol.GetDisplayName());
-      }
+      if (resolveSymbols)
+        disassembled_inst.symbol = symbol.GetDisplayName();
     }
 
     si << llvm::formatv("{0,7} {1,12}", m, o);
@@ -195,7 +103,7 @@ void DisassembleRequestHandler::operator()(
       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
@@ -203,41 +111,35 @@ void DisassembleRequestHandler::operator()(
     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 &&
-            end_line != line) {
-          disassembled_inst.try_emplace("endLine", end_line);
+        if (end_line != 0 && end_line != LLDB_INVALID_LINE_NUMBER && end_line != 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
diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h
index e6bccfe12f402..09bedb88d6b09 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.h
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h
@@ -534,14 +534,15 @@ 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 {
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
index 7efab87d39986..4160077d419e1 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
+++ b/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
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
index b421c631344de..d48a3a3a14fb7 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
@@ -726,6 +726,37 @@ 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);
+
+/// Response to `disassemble` request.
+struct DisassembleResponseBody {
+  /// The list of disassembled instructions.
+  std::vector<DisassembledInstruction> instructions;
+};
+llvm::json::Value toJSON(const DisassembleResponseBody &);
+
 } // namespace lldb_dap::protocol
 
 #endif
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp
index ce7519e3b16b8..94fe236d952a7 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp
@@ -782,4 +782,38 @@ bool fromJSON(const llvm::json::Value &Params, InstructionBreakpoint &IB,
          O.mapOptional("mode", IB.mode);
 }
 
+llvm::json::Value toJSON(const DisassembledInstruction::PresentationHint &PH) {
+  switch (PH) {
+  case DisassembledInstruction::eSourcePresentationHintNormal:
+    return "normal";
+  case DisassembledInstruction::eSourcePresentationHintInvalid:
+    return "invalid";
+  }
+  llvm_unreachable("unhandled presentation hint.");
+}
+
+llvm::json::Value toJSON(const DisassembledInstruction &DI) {
+  llvm::json::Object result{{"address", DI.address},
+                            {"instruction", DI.instruction}};
+
+  if (DI.instructionBytes)
+    result.insert({"instructionBytes", *DI.instructionBytes});
+  if (DI.symbol)
+    result.insert({"symbol", *DI.symbol});
+  if (DI.location)
+    result.insert({"location", *DI.location});
+  if (DI.line)
+    result.insert({"line", *DI.line});
+  if (DI.column)
+    result.insert({"column", *DI.column});
+  if (DI.endLine)
+    result.insert({"endLine", *DI.endLine});
+  if (DI.endColumn)
+    result.insert({"endColumn", *DI.endColumn});
+  if (DI.presentationHint)
+    result.insert({"presentationHint", *DI.presentationHint});
+
+  return result;
+}
+
 } // namespace lldb_dap::protocol
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h
index 3df77ee7374a7..262ac240a773f 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h
@@ -627,6 +627,59 @@ struct InstructionBreakpoint {
 bool fromJSON(const llvm::json::Value &, InstructionBreakpoint &,
               llvm::json::Path);
 
+/// Properties of a single disassembled instruction, returned by `disassemble` request.
+struct DisassembledInstruction {
+  enum PresentationHint : unsigned {
+    eSourcePresentationHintNormal,
+    eSourcePresentationHintInvalid,
+  };
+
+  /// The address of the instruction. Treated as a hex value if prefixed with
+  /// `0x`, or as a decimal value otherwise.
+  std::string address;
+
+  /// Raw bytes representing the instruction and its operands, in an
+  /// implementation-defined format.
+  std::optional<std::string> instructionBytes;
+
+  /// Text representing the instruction and its operands, in an
+  /// implementation-defined format.
+  std::string instruction;
+
+  /// Name of the symbol that corresponds with the location of this instruction,
+  /// if any.
+  std::optional<std::string> symbol;
+
+  /// Source location that corresponds to this instruction, if any.
+  /// Should always be set (if available) on the first instruction returned,
+  /// but can be omitted afterwards if this instruction maps to the same source
+  /// file as the previous instruction.
+  std::optional<protocol::Source> location;
+
+  /// The line within the source location that corresponds to this instruction,
+  /// if any.
+  std::optional<uint32_t> line;
+
+  /// The column within the line that corresponds to this instruction, if any.
+  std::optional<uint32_t> column;
+
+  /// The end line of the range that corresponds to this instruction, if any.
+  std::optional<uint32_t> endLine;
+
+  /// The end column of the range that corresponds to this instruction, if any.
+  std::optional<uint32_t> endColumn;
+
+  /// A hint for how to present the instruction in the UI.
+  ///
+  /// A value of `invalid` may be used to indicate this instruction is 'filler'
+  /// and cannot be reached by the program. For example, unreadable memory
+  /// addresses may be presented is 'invalid.'
+  /// Values: 'normal', 'invalid'
+  std::optional<PresentationHint> presentationHint;
+};
+llvm::json::Value toJSON(const DisassembledInstruction::PresentationHint &);
+llvm::json::Value toJSON(const DisassembledInstruction &);
+
 } // namespace lldb_dap::protocol
 
 #endif

ashgti

Should we also add unit tests to https://github.com/llvm/llvm-project/blob/main/lldb/unittests/DAP/ProtocolTypesTest.cpp for these new types as well?

ashgti · 2025-05-18T23:54:47Z

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

+/// request.
+struct DisassembledInstruction {
+  enum PresentationHint : unsigned {
+    eSourcePresentationHintNormal,


Should this be eDisassembledInstructionPresentationHintNormal? (That does feel really long...). Maybe eInstructionPresentationHintNormal? and same for Invalid below.

ashgti · 2025-05-18T23:57:38Z

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

+
+  /// The address of the instruction. Treated as a hex value if prefixed with
+  /// `0x`, or as a decimal value otherwise.
+  std::string address;


I wonder, should we use an lldb::addr_t for the type here and encode it to a hex string in the toJSON()?

JDevlieghere

+1 on the unit test

JDevlieghere · 2025-05-19T02:07:00Z

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

-    dap.SendJSON(llvm::json::Value(std::move(response)));
-    return;
-  }
+  auto addr_opt = DecodeMemoryReference(args.memoryReference);


I know this is existing code, but since you're touching it, let's use the actual type here. (auto in llvm)

eronnen · 2025-05-19T09:51:15Z

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

+
+bool fromJSON(const llvm::json::Value &Params, DisassembledInstruction &DI,
+              llvm::json::Path P) {
+  std::optional<llvm::StringRef> raw_address =


Maybe it's better to implement fromJSON and toJSON for addr_t, although then it means it will be converted to a hex string for all other addr_t usages too

I think that may run into issues with the c++ type system since the type is just a typedef like typedef uint64_t addr_t; I think c++ doesn't make a distinction between other uint64_t types and addr_t. I could be wrong on that though, but that may be an an issue with generalizing this to much.

I think the current approach of special casing this per request is fine because it forces us to think about the representation. Also addr_t is just a typedef for uint64_t so adding an overload would require a separate type.

ashgti

LGTM!

ashgti · 2025-05-19T16:03:01Z

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

+
+bool fromJSON(const llvm::json::Value &Params, DisassembledInstruction &DI,
+              llvm::json::Path P) {
+  std::optional<llvm::StringRef> raw_address =


I think that may run into issues with the c++ type system since the type is just a typedef like typedef uint64_t addr_t; I think c++ doesn't make a distinction between other uint64_t types and addr_t. I could be wrong on that though, but that may be an an issue with generalizing this to much.

JDevlieghere · 2025-05-19T16:00:16Z

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

+bool fromJSON(const llvm::json::Value &, DisassembleArguments &,
+              llvm::json::Path);


Add the toJSON forward declaration.

JDevlieghere · 2025-05-19T16:00:27Z

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

+  /// The list of disassembled instructions.
+  std::vector<DisassembledInstruction> instructions;
+};
+llvm::json::Value toJSON(const DisassembleResponseBody &);


Add the fromJSON forward declaration.

JDevlieghere · 2025-05-19T16:01:44Z

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

-  const bool resolveSymbols =
-      GetBoolean(arguments, "resolveSymbols").value_or(false);
-  llvm::json::Array instructions;
+  const bool resolveSymbols = args.resolveSymbols.value_or(false);


Suggested change

const bool resolveSymbols = args.resolveSymbols.value_or(false);

const bool resolve_symbols = args.resolveSymbols.value_or(false);

JDevlieghere · 2025-05-19T16:03:28Z

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

+
+bool fromJSON(const llvm::json::Value &Params, DisassembledInstruction &DI,
+              llvm::json::Path P) {
+  std::optional<llvm::StringRef> raw_address =


I think the current approach of special casing this per request is fine because it forces us to think about the representation. Also addr_t is just a typedef for uint64_t so adding an overload would require a separate type.

JDevlieghere · 2025-05-19T16:05:40Z

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

+  std::optional<llvm::StringRef> raw_address =
+      Params.getAsObject()->getString("address");
+  if (!raw_address) {
+    P.report("missing `address` field");


We're using single quotes (') rather than backticks for the existing error messages so let's stay consistent with that.

Suggested change

P.report("missing `address` field");

P.report("missing 'address' field");

JDevlieghere · 2025-05-19T16:05:51Z

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

+
+  std::optional<lldb::addr_t> address = DecodeMemoryReference(*raw_address);
+  if (!address) {
+    P.report("invalid `address`");


Suggested change

P.report("invalid `address`");

P.report("invalid 'address'");

eronnen · 2025-05-19T19:17:21Z

Fixed comments

JDevlieghere

LGTM

eronnen requested review from ashgti, JDevlieghere and da-viper May 18, 2025 21:52

llvmbot added lldb lldb-dap labels May 18, 2025

[lldb-dap] Migrate disassemble request to structured handler

1014235

eronnen force-pushed the lldb-dap-migrate-disassembly-request branch from b187a83 to 1014235 Compare May 18, 2025 21:53

ashgti reviewed May 19, 2025

View reviewed changes

JDevlieghere reviewed May 19, 2025

View reviewed changes

eronnen added 2 commits May 19, 2025 10:29

adding DisassembledInstruction unit tests

304c28f

remove auto

7d45343

eronnen force-pushed the lldb-dap-migrate-disassembly-request branch from 1e18aaf to 7d45343 Compare May 19, 2025 08:32

eronnen requested review from JDevlieghere and ashgti May 19, 2025 08:32

eronnen commented May 19, 2025

View reviewed changes

ashgti approved these changes May 19, 2025

View reviewed changes

JDevlieghere reviewed May 19, 2025

View reviewed changes

small CR fixes

08c9172

eronnen requested a review from JDevlieghere May 19, 2025 19:17

JDevlieghere approved these changes May 19, 2025

View reviewed changes

eronnen merged commit 30c9909 into llvm:main May 19, 2025
10 checks passed

		bool fromJSON(const llvm::json::Value &, DisassembleArguments &,
		llvm::json::Path);

	const bool resolveSymbols = args.resolveSymbols.value_or(false);
	const bool resolve_symbols = args.resolveSymbols.value_or(false);

	P.report("missing `address` field");
	P.report("missing 'address' field");

	P.report("invalid `address`");
	P.report("invalid 'address'");

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

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

Uh oh!

Conversation

eronnen commented May 18, 2025

Uh oh!

llvmbot commented May 18, 2025

Uh oh!

ashgti left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JDevlieghere left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ashgti left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eronnen commented May 19, 2025

Uh oh!

JDevlieghere left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!