data breakpoint

eronnen · eronnen · commit 54782ac3dc0d · 2025-04-26T19:41:24.000+02:00
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
@@ -45,6 +45,7 @@
 #include <chrono>
 #include <condition_variable>
 #include <cstdarg>
+#include <cstdint>
 #include <cstdio>
 #include <fstream>
 #include <future>
@@ -514,15 +515,18 @@ lldb::SBThread DAP::GetLLDBThread(const llvm::json::Object &arguments) {
   return target.GetProcess().GetThreadByID(tid);
 }
 
-lldb::SBFrame DAP::GetLLDBFrame(const llvm::json::Object &arguments) {
-  const uint64_t frame_id =
-      GetInteger<uint64_t>(arguments, "frameId").value_or(UINT64_MAX);
+lldb::SBFrame DAP::GetLLDBFrame(std::optional<uint64_t> frame_id) {
+  uint64_t id = frame_id.value_or(0);
   lldb::SBProcess process = target.GetProcess();
   // Upper 32 bits is the thread index ID
-  lldb::SBThread thread =
-      process.GetThreadByIndexID(GetLLDBThreadIndexID(frame_id));
+  lldb::SBThread thread = process.GetThreadByIndexID(GetLLDBThreadIndexID(id));
   // Lower 32 bits is the frame index
-  return thread.GetFrameAtIndex(GetLLDBFrameID(frame_id));
+  return thread.GetFrameAtIndex(GetLLDBFrameID(id));
+}
+
+lldb::SBFrame DAP::GetLLDBFrame(const llvm::json::Object &arguments) {
+  const auto frame_id = GetInteger<uint64_t>(arguments, "frameId");
+  return GetLLDBFrame(frame_id);
 }
 
 llvm::json::Value DAP::CreateTopLevelScopes() {
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
@@ -275,6 +275,7 @@ struct DAP {
   lldb::SBThread GetLLDBThread(lldb::tid_t id);
   lldb::SBThread GetLLDBThread(const llvm::json::Object &arguments);
 
+  lldb::SBFrame GetLLDBFrame(std::optional<uint64_t> frame_id);
   lldb::SBFrame GetLLDBFrame(const llvm::json::Object &arguments);
 
   llvm::json::Value CreateTopLevelScopes();
diff --git a/lldb/tools/lldb-dap/Handler/DataBreakpointInfoRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/DataBreakpointInfoRequestHandler.cpp
@@ -8,144 +8,49 @@
 
 #include "DAP.h"
 #include "EventHelper.h"
-#include "JSONUtils.h"
+#include "Protocol/ProtocolTypes.h"
 #include "RequestHandler.h"
 #include "lldb/API/SBMemoryRegionInfo.h"
 #include "llvm/ADT/StringExtras.h"
+#include <optional>
 
 namespace lldb_dap {
 
-// "DataBreakpointInfoRequest": {
-//   "allOf": [ { "$ref": "#/definitions/Request" }, {
-//     "type": "object",
-//     "description": "Obtains information on a possible data breakpoint that
-//     could be set on an expression or variable.\nClients should only call this
-//     request if the corresponding capability `supportsDataBreakpoints` is
-//     true.", "properties": {
-//       "command": {
-//         "type": "string",
-//         "enum": [ "dataBreakpointInfo" ]
-//       },
-//       "arguments": {
-//         "$ref": "#/definitions/DataBreakpointInfoArguments"
-//       }
-//     },
-//     "required": [ "command", "arguments"  ]
-//   }]
-// },
-// "DataBreakpointInfoArguments": {
-//   "type": "object",
-//   "description": "Arguments for `dataBreakpointInfo` request.",
-//   "properties": {
-//     "variablesReference": {
-//       "type": "integer",
-//       "description": "Reference to the variable container if the data
-//       breakpoint is requested for a child of the container. The
-//       `variablesReference` must have been obtained in the current suspended
-//       state. See 'Lifetime of Object References' in the Overview section for
-//       details."
-//     },
-//     "name": {
-//       "type": "string",
-//       "description": "The name of the variable's child to obtain data
-//       breakpoint information for.\nIf `variablesReference` isn't specified,
-//       this can be an expression."
-//     },
-//     "frameId": {
-//       "type": "integer",
-//       "description": "When `name` is an expression, evaluate it in the scope
-//       of this stack frame. If not specified, the expression is evaluated in
-//       the global scope. When `variablesReference` is specified, this property
-//       has no effect."
-//     }
-//   },
-//   "required": [ "name" ]
-// },
-// "DataBreakpointInfoResponse": {
-//   "allOf": [ { "$ref": "#/definitions/Response" }, {
-//     "type": "object",
-//     "description": "Response to `dataBreakpointInfo` request.",
-//     "properties": {
-//       "body": {
-//         "type": "object",
-//         "properties": {
-//           "dataId": {
-//             "type": [ "string", "null" ],
-//             "description": "An identifier for the data on which a data
-//             breakpoint can be registered with the `setDataBreakpoints`
-//             request or null if no data breakpoint is available. If a
-//             `variablesReference` or `frameId` is passed, the `dataId` is
-//             valid in the current suspended state, otherwise it's valid
-//             indefinitely. See 'Lifetime of Object References' in the Overview
-//             section for details. Breakpoints set using the `dataId` in the
-//             `setDataBreakpoints` request may outlive the lifetime of the
-//             associated `dataId`."
-//           },
-//           "description": {
-//             "type": "string",
-//             "description": "UI string that describes on what data the
-//             breakpoint is set on or why a data breakpoint is not available."
-//           },
-//           "accessTypes": {
-//             "type": "array",
-//             "items": {
-//               "$ref": "#/definitions/DataBreakpointAccessType"
-//             },
-//             "description": "Attribute lists the available access types for a
-//             potential data breakpoint. A UI client could surface this
-//             information."
-//           },
-//           "canPersist": {
-//             "type": "boolean",
-//             "description": "Attribute indicates that a potential data
-//             breakpoint could be persisted across sessions."
-//           }
-//         },
-//         "required": [ "dataId", "description" ]
-//       }
-//     },
-//     "required": [ "body" ]
-//   }]
-// }
-void DataBreakpointInfoRequestHandler::operator()(
-    const llvm::json::Object &request) const {
-  llvm::json::Object response;
-  FillResponse(request, response);
-  llvm::json::Object body;
-  lldb::SBError error;
-  llvm::json::Array accessTypes{"read", "write", "readWrite"};
-  const auto *arguments = request.getObject("arguments");
-  const auto variablesReference =
-      GetInteger<uint64_t>(arguments, "variablesReference").value_or(0);
-  llvm::StringRef name = GetString(arguments, "name").value_or("");
-  lldb::SBFrame frame = dap.GetLLDBFrame(*arguments);
-  lldb::SBValue variable = dap.variables.FindVariable(variablesReference, name);
+/// Obtains information on a possible data breakpoint that could be set on an
+/// expression or variable. Clients should only call this request if the
+/// corresponding capability supportsDataBreakpoints is true.
+llvm::Expected<protocol::DataBreakpointInfoResponseBody>
+DataBreakpointInfoRequestHandler::Run(
+    const protocol::DataBreakpointInfoArguments &args) const {
+  protocol::DataBreakpointInfoResponseBody response;
+  lldb::SBFrame frame = dap.GetLLDBFrame(args.frameId);
+  lldb::SBValue variable = dap.variables.FindVariable(
+      args.variablesReference.value_or(0), args.name);
   std::string addr, size;
 
   if (variable.IsValid()) {
     lldb::addr_t load_addr = variable.GetLoadAddress();
     size_t byte_size = variable.GetByteSize();
     if (load_addr == LLDB_INVALID_ADDRESS) {
-      body.try_emplace("dataId", nullptr);
-      body.try_emplace("description",
-                       "does not exist in memory, its location is " +
-                           std::string(variable.GetLocation()));
+      response.dataId = std::nullopt;
+      response.description = "does not exist in memory, its location is " +
+                             std::string(variable.GetLocation());
     } else if (byte_size == 0) {
-      body.try_emplace("dataId", nullptr);
-      body.try_emplace("description", "variable size is 0");
+      response.dataId = std::nullopt;
+      response.description = "variable size is 0";
     } else {
       addr = llvm::utohexstr(load_addr);
       size = llvm::utostr(byte_size);
     }
-  } else if (variablesReference == 0 && frame.IsValid()) {
-    lldb::SBValue value = frame.EvaluateExpression(name.data());
+  } else if (args.variablesReference.value_or(0) == 0 && frame.IsValid()) {
+    lldb::SBValue value = frame.EvaluateExpression(args.name.c_str());
     if (value.GetError().Fail()) {
       lldb::SBError error = value.GetError();
       const char *error_cstr = error.GetCString();
-      body.try_emplace("dataId", nullptr);
-      body.try_emplace("description", error_cstr && error_cstr[0]
-                                          ? std::string(error_cstr)
-                                          : "evaluation failed");
+      response.dataId = std::nullopt;
+      response.description = error_cstr && error_cstr[0]
+                                 ? std::string(error_cstr)
+                                 : "evaluation failed";
     } else {
       uint64_t load_addr = value.GetValueAsUnsigned();
       lldb::SBData data = value.GetPointeeData();
@@ -159,32 +64,31 @@ void DataBreakpointInfoRequestHandler::operator()(
         // request if SBProcess::GetMemoryRegionInfo returns error.
         if (err.Success()) {
           if (!(region.IsReadable() || region.IsWritable())) {
-            body.try_emplace("dataId", nullptr);
-            body.try_emplace("description",
-                             "memory region for address " + addr +
-                                 " has no read or write permissions");
+            response.dataId = std::nullopt;
+            response.description = "memory region for address " + addr +
+                                   " has no read or write permissions";
           }
         }
       } else {
-        body.try_emplace("dataId", nullptr);
-        body.try_emplace("description",
-                         "unable to get byte size for expression: " +
-                             name.str());
+        response.dataId = std::nullopt;
+        response.description =
+            "unable to get byte size for expression: " + args.name;
       }
     }
   } else {
-    body.try_emplace("dataId", nullptr);
-    body.try_emplace("description", "variable not found: " + name.str());
+    response.dataId = std::nullopt;
+    response.description = "variable not found: " + args.name;
   }
 
-  if (!body.getObject("dataId")) {
-    body.try_emplace("dataId", addr + "/" + size);
-    body.try_emplace("accessTypes", std::move(accessTypes));
-    body.try_emplace("description",
-                     size + " bytes at " + addr + " " + name.str());
+  if (!response.dataId.has_value()) {
+    response.dataId = addr + "/" + size;
+    response.accessTypes = {protocol::eDataBreakpointAccessTypeRead,
+                            protocol::eDataBreakpointAccessTypeWrite,
+                            protocol::eDataBreakpointAccessTypeReadWrite};
+    response.description = size + " bytes at " + addr + " " + args.name;
   }
-  response.try_emplace("body", std::move(body));
-  dap.SendJSON(llvm::json::Value(std::move(response)));
+
+  return response;
 }
 
 } // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp
@@ -212,7 +212,7 @@ static void EventThreadFunction(DAP &dap) {
             // avoids sending paths that should be source mapped. Note that
             // CreateBreakpoint doesn't apply source mapping and certain
             // implementation ignore the source part of this event anyway.
-            llvm::json::Value source_bp = CreateBreakpoint(&bp);
+            llvm::json::Value source_bp = bp.ToProtocolBreakpoint();
             source_bp.getAsObject()->erase("source");
 
             llvm::json::Object body;
diff --git a/lldb/tools/lldb-dap/Handler/TestGetTargetBreakpointsRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/TestGetTargetBreakpointsRequestHandler.cpp
@@ -20,7 +20,7 @@ void TestGetTargetBreakpointsRequestHandler::operator()(
   llvm::json::Array response_breakpoints;
   for (uint32_t i = 0; dap.target.GetBreakpointAtIndex(i).IsValid(); ++i) {
     auto bp = Breakpoint(dap, dap.target.GetBreakpointAtIndex(i));
-    AppendBreakpoint(&bp, response_breakpoints);
+    response_breakpoints.push_back(bp.ToProtocolBreakpoint());
   }
   llvm::json::Object body;
   body.try_emplace("breakpoints", std::move(response_breakpoints));
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
@@ -506,7 +506,7 @@ struct DataBreakpointInfoArguments {
   /// When `name` is an expression, evaluate it in the scope of this stack
   /// frame. If not specified, the expression is evaluated in the global scope.
   /// When `asAddress` is true, the `frameId` is ignored.
-  std::optional<int64_t> frameId;
+  std::optional<uint64_t> frameId;
 
   /// If specified, a debug adapter should return information for the range of
   /// memory extending `bytes` number of bytes from the address or variable
@@ -539,7 +539,7 @@ struct DataBreakpointInfoResponseBody {
   /// for details. Breakpoints set using the `dataId` in the
   /// `setDataBreakpoints` request may outlive the lifetime of the associated
   /// `dataId`.
-  std::optional<std::string> dataId;
+  std::optional<std::optional<std::string>> dataId;
 
   /// UI string that describes on what data the breakpoint is set on or why a
   /// data breakpoint is not available.
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp
@@ -43,13 +43,28 @@ bool fromJSON(const json::Value &Params, Source &S, json::Path P) {
          O.map("sourceReference", S.sourceReference);
 }
 
+static llvm::json::Value ToString(PresentationHint hint) {
+  switch (hint) {
+  case ePresentationHintNormal:
+    return "normal";
+  case ePresentationHintEmphasize:
+    return "emphasize";
+  case ePresentationHintDeemphasize:
+    return "deemphasize";
+  }
+  llvm_unreachable("unhandled presentation hint.");
+}
+
 llvm::json::Value toJSON(const Source &S) {
-  json::Object result{
-      {"name", S.name},
-      {"path", S.path},
-      {"sourceReference", S.sourceReference},
-      {"presentationHint", S.presentationHint},
-  };
+  json::Object result;
+  if (S.name)
+    result.insert({"name", *S.name});
+  if (S.path)
+    result.insert({"path", *S.path});
+  if (S.name)
+    result.insert({"sourceReference", *S.sourceReference});
+  if (S.presentationHint)
+    result.insert({"sourceReference", ToString(*S.presentationHint)});
 
   return result;
 }
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h
@@ -273,6 +273,7 @@ enum PresentationHint : unsigned {
   ePresentationHintEmphasize,
   ePresentationHintDeemphasize,
 };
+llvm::json::Value toJSON(const PresentationHint &);
 
 /// A `Source` is a descriptor for source code. It is returned from the debug
 /// adapter as part of a `StackFrame` and it is used by clients when specifying
diff --git a/lldb/tools/lldb-dap/Watchpoint.h b/lldb/tools/lldb-dap/Watchpoint.h
@@ -28,7 +28,7 @@ class Watchpoint : public BreakpointBase {
   void SetCondition() override;
   void SetHitCondition() override;
 
-  protocol::Breakpoint ToProtocolBreakpoint();
+  protocol::Breakpoint ToProtocolBreakpoint() override;
 
   void SetWatchpoint();
 

Original file line number	Diff line number	Diff line change
`@@ -20,7 +20,7 @@ void TestGetTargetBreakpointsRequestHandler::operator()(`
`20`	`20`	`llvm::json::Array response_breakpoints;`
`21`	`21`	`for (uint32_t i = 0; dap.target.GetBreakpointAtIndex(i).IsValid(); ++i) {`
`22`	`22`	`auto bp = Breakpoint(dap, dap.target.GetBreakpointAtIndex(i));`
`23`		`- AppendBreakpoint(&bp, response_breakpoints);`
	`23`	`+ response_breakpoints.push_back(bp.ToProtocolBreakpoint());`
`24`	`24`	`}`
`25`	`25`	`llvm::json::Object body;`
`26`	`26`	`body.try_emplace("breakpoints", std::move(response_breakpoints));`