[lldb-dap] Add data breakpoints for bytes

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

@@ -1234,16 +1234,18 @@ def request_setFunctionBreakpoints(self, names, condition=None, hitCondition=Non
         return response
 
     def request_dataBreakpointInfo(
-        self, variablesReference, name, frameIndex=0, threadId=None
+        self, variablesReference, name, size=None, frameIndex=0, threadId=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 = {"name": name}
+        if size is None:
+            args_dict["variablesReference"] = variablesReference
+            args_dict["frameId"] = stackFrame["id"]
+        else:
+            args_dict["asAddress"] = True
+            args_dict["bytes"] = size
         command_dict = {
             "command": "dataBreakpointInfo",
             "type": "request",

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

@@ -169,6 +169,7 @@ def verify_breakpoint_hit(self, breakpoint_ids: List[Union[int, str]]):
                 if (
                     body["reason"] != "breakpoint"
                     and body["reason"] != "instruction breakpoint"
+                    and body["reason"] != "data breakpoint"
                 ):
                     continue
                 if "hitBreakpointIds" not in body:

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

@@ -39,18 +39,21 @@ def test_duplicate_start_addresses(self):
             {"dataId": response_x["body"]["dataId"], "accessType": "write"},
         ]
         set_response = self.dap_server.request_setDataBreakpoint(dataBreakpoints)
-        self.assertEqual(
-            set_response["body"]["breakpoints"],
-            [{"verified": False}, {"verified": True}, {"verified": True}],
-        )
+        breakpoints = set_response["body"]["breakpoints"]
+        self.assertEqual(len(breakpoints), 3)
+        self.assertFalse(breakpoints[0]["verified"])
+        self.assertTrue(breakpoints[1]["verified"])
+        self.assertTrue(breakpoints[2]["verified"])
 
-        self.continue_to_next_stop()
+        self.dap_server.request_continue()
+        self.verify_breakpoint_hit([breakpoints[2]["id"]])
         x_val = self.dap_server.get_local_variable_value("x")
         i_val = self.dap_server.get_local_variable_value("i")
         self.assertEqual(x_val, "2")
         self.assertEqual(i_val, "1")
 
-        self.continue_to_next_stop()
+        self.dap_server.request_continue()
+        self.verify_breakpoint_hit([breakpoints[1]["id"]])
         arr_2 = self.dap_server.get_local_variable_child("arr", "[2]")
         i_val = self.dap_server.get_local_variable_value("i")
         self.assertEqual(arr_2["value"], "42")
@@ -79,18 +82,20 @@ def test_expression(self):
             {"dataId": response_arr_2["body"]["dataId"], "accessType": "write"},
         ]
         set_response = self.dap_server.request_setDataBreakpoint(dataBreakpoints)
-        self.assertEqual(
-            set_response["body"]["breakpoints"],
-            [{"verified": True}, {"verified": True}],
-        )
+        breakpoints = set_response["body"]["breakpoints"]
+        self.assertEqual(len(breakpoints), 2)
+        self.assertTrue(breakpoints[0]["verified"])
+        self.assertTrue(breakpoints[1]["verified"])
 
-        self.continue_to_next_stop()
+        self.dap_server.request_continue()
+        self.verify_breakpoint_hit([breakpoints[0]["id"]])
         x_val = self.dap_server.get_local_variable_value("x")
         i_val = self.dap_server.get_local_variable_value("i")
         self.assertEqual(x_val, "2")
         self.assertEqual(i_val, "1")
 
-        self.continue_to_next_stop()
+        self.dap_server.request_continue()
+        self.verify_breakpoint_hit([breakpoints[1]["id"]])
         arr_2 = self.dap_server.get_local_variable_child("arr", "[2]")
         i_val = self.dap_server.get_local_variable_value("i")
         self.assertEqual(arr_2["value"], "42")
@@ -123,18 +128,20 @@ def test_functionality(self):
             {"dataId": response_arr_2["body"]["dataId"], "accessType": "write"},
         ]
         set_response = self.dap_server.request_setDataBreakpoint(dataBreakpoints)
-        self.assertEqual(
-            set_response["body"]["breakpoints"],
-            [{"verified": True}, {"verified": True}],
-        )
+        breakpoints = set_response["body"]["breakpoints"]
+        self.assertEqual(len(breakpoints), 2)
+        self.assertTrue(breakpoints[0]["verified"])
+        self.assertTrue(breakpoints[1]["verified"])
 
-        self.continue_to_next_stop()
+        self.dap_server.request_continue()
+        self.verify_breakpoint_hit([breakpoints[0]["id"]])
         x_val = self.dap_server.get_local_variable_value("x")
         i_val = self.dap_server.get_local_variable_value("i")
         self.assertEqual(x_val, "2")
         self.assertEqual(i_val, "1")
 
-        self.continue_to_next_stop()
+        self.dap_server.request_continue()
+        self.verify_breakpoint_hit([breakpoints[1]["id"]])
         arr_2 = self.dap_server.get_local_variable_child("arr", "[2]")
         i_val = self.dap_server.get_local_variable_value("i")
         self.assertEqual(arr_2["value"], "42")
@@ -153,8 +160,11 @@ def test_functionality(self):
             }
         ]
         set_response = self.dap_server.request_setDataBreakpoint(dataBreakpoints)
-        self.assertEqual(set_response["body"]["breakpoints"], [{"verified": True}])
-        self.continue_to_next_stop()
+        breakpoints = set_response["body"]["breakpoints"]
+        self.assertEqual(len(breakpoints), 1)
+        self.assertTrue(breakpoints[0]["verified"])
+        self.dap_server.request_continue()
+        self.verify_breakpoint_hit([breakpoints[0]["id"]])
         x_val = self.dap_server.get_local_variable_value("x")
         self.assertEqual(x_val, "3")
 
@@ -167,7 +177,64 @@ def test_functionality(self):
             }
         ]
         set_response = self.dap_server.request_setDataBreakpoint(dataBreakpoints)
-        self.assertEqual(set_response["body"]["breakpoints"], [{"verified": True}])
-        self.continue_to_next_stop()
+        breakpoints = set_response["body"]["breakpoints"]
+        self.assertEqual(len(breakpoints), 1)
+        self.assertTrue(breakpoints[0]["verified"])
+        self.dap_server.request_continue()
+        self.verify_breakpoint_hit([breakpoints[0]["id"]])
         x_val = self.dap_server.get_local_variable_value("x")
         self.assertEqual(x_val, "10")
+
+    @skipIfWindows
+    def test_bytes(self):
+        """Tests setting data breakpoints on memory range."""
+        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()
+        # Test write watchpoints on x, arr[2]
+        x = self.dap_server.get_local_variable("x")
+        response_x = self.dap_server.request_dataBreakpointInfo(
+            0, x["memoryReference"], 4
+        )
+        arr_2 = self.dap_server.get_local_variable_child("arr", "[2]")
+        response_arr_2 = self.dap_server.request_dataBreakpointInfo(
+            0, arr_2["memoryReference"], 4
+        )
+
+        # Test response from dataBreakpointInfo request.
+        self.assertEqual(
+            response_x["body"]["dataId"].split("/"), [x["memoryReference"][2:], "4"]
+        )
+        self.assertEqual(response_x["body"]["accessTypes"], self.accessTypes)
+        self.assertEqual(
+            response_arr_2["body"]["dataId"].split("/"),
+            [arr_2["memoryReference"][2:], "4"],
+        )
+        self.assertEqual(response_arr_2["body"]["accessTypes"], self.accessTypes)
+        dataBreakpoints = [
+            {"dataId": response_x["body"]["dataId"], "accessType": "write"},
+            {"dataId": response_arr_2["body"]["dataId"], "accessType": "write"},
+        ]
+        set_response = self.dap_server.request_setDataBreakpoint(dataBreakpoints)
+        breakpoints = set_response["body"]["breakpoints"]
+        self.assertEqual(len(breakpoints), 2)
+        self.assertTrue(breakpoints[0]["verified"])
+        self.assertTrue(breakpoints[1]["verified"])
+
+        self.dap_server.request_continue()
+        self.verify_breakpoint_hit([breakpoints[0]["id"]])
+        x_val = self.dap_server.get_local_variable_value("x")
+        i_val = self.dap_server.get_local_variable_value("i")
+        self.assertEqual(x_val, "2")
+        self.assertEqual(i_val, "1")
+
+        self.dap_server.request_continue()
+        self.verify_breakpoint_hit([breakpoints[1]["id"]])
+        arr_2 = self.dap_server.get_local_variable_child("arr", "[2]")
+        i_val = self.dap_server.get_local_variable_value("i")
+        self.assertEqual(arr_2["value"], "42")
+        self.assertEqual(i_val, "2")
+        self.dap_server.request_setDataBreakpoint([])

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

@@ -7,23 +7,40 @@
 //===----------------------------------------------------------------------===//
 
 #include "DAP.h"
+#include "DAPError.h"
 #include "EventHelper.h"
 #include "Protocol/ProtocolTypes.h"
 #include "RequestHandler.h"
+#include "lldb/API/SBAddress.h"
 #include "lldb/API/SBMemoryRegionInfo.h"
 #include "llvm/ADT/StringExtras.h"
 #include <optional>
 
 namespace lldb_dap {
 
+static bool IsRW(DAP &dap, lldb::addr_t load_addr) {
+  if (!lldb::SBAddress(load_addr, dap.target).IsValid())
+    return false;
+  lldb::SBMemoryRegionInfo region;
+  lldb::SBError err =
+      dap.target.GetProcess().GetMemoryRegionInfo(load_addr, region);
+  // Only lldb-server supports "qMemoryRegionInfo". So, don't fail this
+  // request if SBProcess::GetMemoryRegionInfo returns error.
+  if (err.Success()) {
+    if (!(region.IsReadable() || region.IsWritable())) {
+      return false;
+    }
+  }
+  return true;
+}
+
 /// 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;
@@ -43,7 +60,8 @@ DataBreakpointInfoRequestHandler::Run(
       addr = llvm::utohexstr(load_addr);
       size = llvm::utostr(byte_size);
     }
-  } else if (args.variablesReference.value_or(0) == 0 && frame.IsValid()) {
+  } else if (lldb::SBFrame frame = dap.GetLLDBFrame(args.frameId);
+             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();
@@ -58,24 +76,28 @@ DataBreakpointInfoRequestHandler::Run(
       if (data.IsValid()) {
         size = llvm::utostr(data.GetByteSize());
         addr = llvm::utohexstr(load_addr);
-        lldb::SBMemoryRegionInfo region;
-        lldb::SBError err =
-            dap.target.GetProcess().GetMemoryRegionInfo(load_addr, region);
-        // Only lldb-server supports "qMemoryRegionInfo". So, don't fail this
-        // request if SBProcess::GetMemoryRegionInfo returns error.
-        if (err.Success()) {
-          if (!(region.IsReadable() || region.IsWritable())) {
-            is_data_ok = false;
-            response.description = "memory region for address " + addr +
-                                   " has no read or write permissions";
-          }
+        if (!IsRW(dap, load_addr)) {
+          is_data_ok = false;
+          response.description = "memory region for address " + addr +
+                                 " has no read or write permissions";
         }
       } else {
         is_data_ok = false;
         response.description =
             "unable to get byte size for expression: " + args.name;
       }
     }
+  } else if (args.asAddress) {
+    size = llvm::utostr(args.bytes.value_or(dap.target.GetAddressByteSize()));
+    lldb::addr_t load_addr = LLDB_INVALID_ADDRESS;
+    if (llvm::StringRef(args.name).getAsInteger<lldb::addr_t>(0, load_addr))
+      return llvm::make_error<DAPError>(args.name + " is not a valid address",
+                                        llvm::inconvertibleErrorCode(), false);
+    addr = llvm::utohexstr(load_addr);
+    if (!IsRW(dap, load_addr))
+      return llvm::make_error<DAPError>("memory region for address " + addr +
+                                            " has no read or write permissions",
+                                        llvm::inconvertibleErrorCode(), false);
   } else {
     is_data_ok = false;
     response.description = "variable not found: " + args.name;
@@ -86,7 +108,10 @@ DataBreakpointInfoRequestHandler::Run(
     response.accessTypes = {protocol::eDataBreakpointAccessTypeRead,
                             protocol::eDataBreakpointAccessTypeWrite,
                             protocol::eDataBreakpointAccessTypeReadWrite};
-    response.description = size + " bytes at " + addr + " " + args.name;
+    if (args.asAddress)
+      response.description = size + " bytes at " + addr;
+    else
+      response.description = size + " bytes at " + addr + " " + args.name;
   }
 
   return response;

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

@@ -432,6 +432,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

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

lldb/tools/lldb-dap/Watchpoint.cpp

@@ -45,6 +45,7 @@ protocol::Breakpoint Watchpoint::ToProtocolBreakpoint() {
       breakpoint.message = m_error.GetCString();
   } else {
     breakpoint.verified = true;
+    breakpoint.id = m_wp.GetID();
   }
 
   return breakpoint;