[lldb][lldb-dap] Support breakpoint info bytes

lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py

@@ -1042,16 +1042,33 @@ def request_setFunctionBreakpoints(self, names, condition=None, hitCondition=Non
         return self.send_recv(command_dict)
 
     def request_dataBreakpointInfo(
-        self, variablesReference, name, frameIndex=0, threadId=None
+        self,
+        name: str,
+        variablesReference: Optional[int] = None,
+        frameIndex: Optional[int] = 0,
+        bytes_: Optional[int] = None,
+        asAddress: Optional[bool] = None,
     ):
-        stackFrame = self.get_stackFrame(frameIndex=frameIndex, threadId=threadId)
-        if stackFrame is None:
-            return []
-        args_dict = {
-            "variablesReference": variablesReference,
-            "name": name,
-            "frameId": stackFrame["id"],
-        }
+        args_dict = {}
+        if asAddress is not None:
+            args_dict = {
+                "name": name,
+                "asAddress": asAddress,
+                "bytes": bytes_,
+            }
+        else:
+            thread_id = self.get_thread_id()
+            stackFrame = self.get_stackFrame(frameIndex=frameIndex, threadId=thread_id)
+            if stackFrame is None:
+                raise ValueError(
+                    f"could not get stackframe for frameIndex: {frameIndex} and threadId: {thread_id}"
+                )
+            args_dict = {
+                "variablesReference": variablesReference,
+                "name": name,
+                "frameId": stackFrame["id"],
+            }
+
         command_dict = {
             "command": "dataBreakpointInfo",
             "type": "request",

lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py

@@ -104,7 +104,9 @@ def waitUntil(self, condition_callback):
             time.sleep(0.5)
         return False
 
-    def verify_breakpoint_hit(self, breakpoint_ids, timeout=DEFAULT_TIMEOUT):
+    def verify_breakpoint_hit(
+        self, breakpoint_ids, timeout=DEFAULT_TIMEOUT, reason: Optional[str] = None
+    ):
         """Wait for the process we are debugging to stop, and verify we hit
         any breakpoint location in the "breakpoint_ids" array.
         "breakpoint_ids" should be a list of breakpoint ID strings
@@ -116,9 +118,10 @@ def verify_breakpoint_hit(self, breakpoint_ids, timeout=DEFAULT_TIMEOUT):
                 body = stopped_event["body"]
                 if "reason" not in body:
                     continue
-                if (
-                    body["reason"] != "breakpoint"
-                    and body["reason"] != "instruction breakpoint"
+                if body["reason"] not in (
+                    "breakpoint",
+                    "instruction breakpoint",
+                    "data breakpoint",
                 ):
                     continue
                 if "description" not in body:
@@ -131,9 +134,10 @@ def verify_breakpoint_hit(self, breakpoint_ids, timeout=DEFAULT_TIMEOUT):
                 # So when looking at the description we just want to make sure
                 # the right breakpoint matches and not worry about the actual
                 # location.
+                type_name = reason or "breakpoint"
                 description = body["description"]
                 for breakpoint_id in breakpoint_ids:
-                    match_desc = f"breakpoint {breakpoint_id}."
+                    match_desc = f"{type_name} {breakpoint_id}"
                     if match_desc in description:
                         return
         self.assertTrue(False, f"breakpoint not hit, stopped_events={stopped_events}")
@@ -329,12 +333,16 @@ def continue_to_next_stop(self, timeout=DEFAULT_TIMEOUT):
         self.do_continue()
         return self.dap_server.wait_for_stopped(timeout)
 
-    def continue_to_breakpoint(self, breakpoint_id: str, timeout=DEFAULT_TIMEOUT):
-        self.continue_to_breakpoints((breakpoint_id), timeout)
+    def continue_to_breakpoint(
+        self, breakpoint_id: str, timeout=DEFAULT_TIMEOUT, reason: Optional[str] = None
+    ):
+        self.continue_to_breakpoints([breakpoint_id], timeout, reason)
 
-    def continue_to_breakpoints(self, breakpoint_ids, timeout=DEFAULT_TIMEOUT):
+    def continue_to_breakpoints(
+        self, breakpoint_ids, timeout=DEFAULT_TIMEOUT, reason: Optional[str] = None
+    ):
         self.do_continue()
-        self.verify_breakpoint_hit(breakpoint_ids, timeout)
+        self.verify_breakpoint_hit(breakpoint_ids, timeout, reason)
 
     def continue_to_exception_breakpoint(self, filter_label, timeout=DEFAULT_TIMEOUT):
         self.do_continue()

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

@@ -1725,6 +1725,8 @@ Status GDBRemoteCommunicationClient::GetMemoryRegionInfo(
     if (region_info.GetRange() == qXfer_region_info.GetRange()) {
       region_info.SetFlash(qXfer_region_info.GetFlash());
       region_info.SetBlocksize(qXfer_region_info.GetBlocksize());
+      region_info.SetReadable(qXfer_region_info.GetReadable());
+      region_info.SetWritable(qXfer_region_info.GetWritable());
     }
   }
   return error;

lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py

@@ -23,8 +23,8 @@ def test_duplicate_start_addresses(self):
         self.continue_to_next_stop()
         self.dap_server.get_stackFrame()
         # Test setting write watchpoint using expressions: &x, arr+2
-        response_x = self.dap_server.request_dataBreakpointInfo(0, "&x")
-        response_arr_2 = self.dap_server.request_dataBreakpointInfo(0, "arr+2")
+        response_x = self.dap_server.request_dataBreakpointInfo("&x", 0)
+        response_arr_2 = self.dap_server.request_dataBreakpointInfo("arr+2", 0)
         # Test response from dataBreakpointInfo request.
         self.assertEqual(response_x["body"]["dataId"].split("/")[1], "4")
         self.assertEqual(response_x["body"]["accessTypes"], self.accessTypes)
@@ -56,6 +56,47 @@ def test_duplicate_start_addresses(self):
         self.assertEqual(arr_2["value"], "42")
         self.assertEqual(i_val, "2")
 
+    @skipIfWindows
+    def test_breakpoint_info_bytes(self):
+        """Test supportBreakpointInfoBytes
+        Set the watchpoint on `var` variable address + 6 characters.
+        """
+        program = self.getBuildArtifact("a.out")
+        self.build_and_launch(program)
+        source = "main.cpp"
+        first_loop_break_line = line_number(source, "// first loop breakpoint")
+        self.set_source_breakpoints(source, [first_loop_break_line])
+        self.continue_to_next_stop()
+
+        # get the address of `var` variable
+        eval_response = self.dap_server.request_evaluate("&var", context="watch")
+        self.assertTrue(eval_response["success"])
+        var_address = eval_response["body"]["result"]
+
+        var_byte_watch_size = 5
+        bp_resp = self.dap_server.request_dataBreakpointInfo(
+            var_address, asAddress=True, bytes_=var_byte_watch_size
+        )
+        self.assertTrue(
+            bp_resp["success"], f"dataBreakpointInfo request failed: {bp_resp}"
+        )
+        resp_data_id = bp_resp["body"]["dataId"]
+        self.assertEqual(resp_data_id.split("/")[1], str(var_byte_watch_size))
+
+        data_breakpoints = [{"dataId": resp_data_id, "accessType": "write"}]
+        self.dap_server.request_setDataBreakpoint(data_breakpoints)
+
+        self.continue_to_breakpoint(breakpoint_id=1, reason="data breakpoint")
+        eval_response = self.dap_server.request_evaluate("var", context="watch")
+        self.assertTrue(eval_response["success"])
+        var_value = eval_response["body"]["result"]
+        self.assertEqual(var_value, '"HALLO"')
+
+        # Remove the watchpoint because once it leaves this function scope, the address can be
+        # be used by another variable or register.
+        self.dap_server.request_setDataBreakpoint([])
+        self.continue_to_exit()
+
     @skipIfWindows
     def test_expression(self):
         """Tests setting data breakpoints on expression."""
@@ -67,8 +108,8 @@ def test_expression(self):
         self.continue_to_next_stop()
         self.dap_server.get_stackFrame()
         # Test setting write watchpoint using expressions: &x, arr+2
-        response_x = self.dap_server.request_dataBreakpointInfo(0, "&x")
-        response_arr_2 = self.dap_server.request_dataBreakpointInfo(0, "arr+2")
+        response_x = self.dap_server.request_dataBreakpointInfo("&x", 0)
+        response_arr_2 = self.dap_server.request_dataBreakpointInfo("arr+2", 0)
         # Test response from dataBreakpointInfo request.
         self.assertEqual(response_x["body"]["dataId"].split("/")[1], "4")
         self.assertEqual(response_x["body"]["accessTypes"], self.accessTypes)
@@ -107,10 +148,10 @@ def test_functionality(self):
         self.continue_to_next_stop()
         self.dap_server.get_local_variables()
         # Test write watchpoints on x, arr[2]
-        response_x = self.dap_server.request_dataBreakpointInfo(1, "x")
+        response_x = self.dap_server.request_dataBreakpointInfo("x", 1)
         arr = self.dap_server.get_local_variable("arr")
         response_arr_2 = self.dap_server.request_dataBreakpointInfo(
-            arr["variablesReference"], "[2]"
+            "[2]", arr["variablesReference"]
         )
 
         # Test response from dataBreakpointInfo request.

lldb/test/API/tools/lldb-dap/databreakpoint/main.cpp

@@ -1,5 +1,6 @@
 int main(int argc, char const *argv[]) {
   // Test for data breakpoint
+  char var[6] = "HELLO";
   int x = 0;
   int arr[4] = {1, 2, 3, 4};
   for (int i = 0; i < 5; ++i) { // first loop breakpoint
@@ -10,6 +11,8 @@ int main(int argc, char const *argv[]) {
     }
   }
 
+  var[1] = 'A';
+
   x = 1;
   for (int i = 0; i < 10; ++i) { // second loop breakpoint
     ++x;

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

@@ -7,7 +7,6 @@
 //===----------------------------------------------------------------------===//
 
 #include "DAP.h"
-#include "EventHelper.h"
 #include "Protocol/ProtocolTypes.h"
 #include "RequestHandler.h"
 #include "lldb/API/SBMemoryRegionInfo.h"
@@ -16,12 +15,80 @@
 
 namespace lldb_dap {
 
+namespace {
+std::vector<protocol::DataBreakpointAccessType>
+GetBreakpointAccessTypes(lldb::SBMemoryRegionInfo region) {
+  std::vector<protocol::DataBreakpointAccessType> types;
+  if (region.IsReadable())
+    types.emplace_back(protocol::eDataBreakpointAccessTypeRead);
+  if (region.IsWritable())
+    types.emplace_back(protocol::eDataBreakpointAccessTypeWrite);
+  if (region.IsReadable() && region.IsWritable())
+    types.emplace_back(protocol::eDataBreakpointAccessTypeReadWrite);
+
+  return types;
+}
+
+llvm::Expected<protocol::DataBreakpointInfoResponseBody>
+HandleDataBreakpointBytes(DAP &dap,
+                          const protocol::DataBreakpointInfoArguments &args) {
+  const llvm::StringRef raw_address = args.name;
+
+  lldb::addr_t load_addr = LLDB_INVALID_ADDRESS;
+  if (raw_address.getAsInteger<lldb::addr_t>(0, load_addr)) {
+    return llvm::make_error<DAPError>(llvm::formatv("invalid address"),
+                                      llvm::inconvertibleErrorCode(), false);
+  }
+
+  if (lldb::SBAddress address(load_addr, dap.target); !address.IsValid()) {
+    return llvm::make_error<DAPError>(
+        llvm::formatv("address {:x} does not exist in the debuggee", load_addr),
+        llvm::inconvertibleErrorCode(), false);
+  }
+
+  const uint32_t byte_size =
+      args.bytes.value_or(dap.target.GetAddressByteSize());
+
+  lldb::SBMemoryRegionInfo region;
+  lldb::SBError err =
+      dap.target.GetProcess().GetMemoryRegionInfo(load_addr, region);
+  std::vector<protocol::DataBreakpointAccessType> access_types =
+      GetBreakpointAccessTypes(region);
+
+  protocol::DataBreakpointInfoResponseBody response;
+  if (err.Fail()) {
+    response.dataId = std::nullopt;
+    response.description = err.GetCString();
+    return response;
+  }
+
+  if (access_types.empty()) {
+    response.dataId = std::nullopt;
+    response.description = llvm::formatv(
+        "memory region for address {} has no read or write permissions",
+        load_addr);
+    return response;
+  }
+
+  response.dataId = llvm::formatv("{:x-}/{}", load_addr, byte_size);
+  response.description =
+      llvm::formatv("{} bytes at {:x}", byte_size, load_addr);
+  response.accessTypes = std::move(access_types);
+
+  return response;
+}
+} // namespace
+
 /// 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 {
+
+  if (args.asAddress.value_or(false))
+    return HandleDataBreakpointBytes(dap, args);
+
   protocol::DataBreakpointInfoResponseBody response;
   lldb::SBFrame frame = dap.GetLLDBFrame(args.frameId.value_or(UINT64_MAX));
   lldb::SBValue variable = dap.variables.FindVariable(
@@ -40,7 +107,7 @@ DataBreakpointInfoRequestHandler::Run(
       is_data_ok = false;
       response.description = "variable size is 0";
     } else {
-      addr = llvm::utohexstr(load_addr);
+      addr = llvm::utohexstr(load_addr, /*lowerCase=*/true);
       size = llvm::utostr(byte_size);
     }
   } else if (args.variablesReference.value_or(0) == 0 && frame.IsValid()) {
@@ -57,7 +124,7 @@ DataBreakpointInfoRequestHandler::Run(
       lldb::SBData data = value.GetPointeeData();
       if (data.IsValid()) {
         size = llvm::utostr(data.GetByteSize());
-        addr = llvm::utohexstr(load_addr);
+        addr = llvm::utohexstr(load_addr, /*lowerCase=*/true);
         lldb::SBMemoryRegionInfo region;
         lldb::SBError err =
             dap.target.GetProcess().GetMemoryRegionInfo(load_addr, region);
@@ -86,7 +153,7 @@ DataBreakpointInfoRequestHandler::Run(
     response.accessTypes = {protocol::eDataBreakpointAccessTypeRead,
                             protocol::eDataBreakpointAccessTypeWrite,
                             protocol::eDataBreakpointAccessTypeReadWrite};
-    response.description = size + " bytes at " + addr + " " + args.name;
+    response.description = size + " bytes at 0x" + addr + " " + args.name;
   }
 
   return response;

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

@@ -420,6 +420,9 @@ class DataBreakpointInfoRequestHandler
 public:
   using RequestHandler::RequestHandler;
   static llvm::StringLiteral GetCommand() { return "dataBreakpointInfo"; }
+  FeatureSet GetSupportedFeatures() const override {
+    return {protocol::eAdapterFeatureDataBreakpointBytes};
+  }
   llvm::Expected<protocol::DataBreakpointInfoResponseBody>
   Run(const protocol::DataBreakpointInfoArguments &args) const override;
 };

lldb/tools/lldb-dap/JSONUtils.cpp

@@ -893,7 +893,16 @@ llvm::json::Value CreateThreadStopped(DAP &dap, lldb::SBThread &thread,
       EmplaceSafeString(body, "description", desc_str);
     }
   } break;
-  case lldb::eStopReasonWatchpoint:
+  case lldb::eStopReasonWatchpoint: {
+    lldb::break_id_t bp_id = thread.GetStopReasonDataAtIndex(0);
+    lldb::break_id_t bp_loc_id = thread.GetStopReasonDataAtIndex(1);
+    std::string desc_str =
+        llvm::formatv("data breakpoint {0}.{1}", bp_id, bp_loc_id);
+    body.try_emplace("hitBreakpointIds",
+                     llvm::json::Array{llvm::json::Value(bp_id)});
+    body.try_emplace("reason", "data breakpoint");
+    EmplaceSafeString(body, "description", desc_str);
+  } break;
   case lldb::eStopReasonInstrumentation:
     body.try_emplace("reason", "breakpoint");
     break;

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

@@ -668,7 +668,7 @@ struct DataBreakpointInfoArguments {
   /// pause on data access anywhere within that range.
   /// Clients may set this property only if the `supportsDataBreakpointBytes`
   /// capability is true.
-  std::optional<int64_t> bytes;
+  std::optional<uint64_t> bytes;
 
   /// If `true`, the `name` is a memory address and the debugger should
   /// interpret it as a decimal value, or hex value if it is prefixed with `0x`.