[lldb-dap] Deduplicate watchpoints starting at the same address on SetDataBreakpointsRequest.

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

@@ -12,6 +12,51 @@ def setUp(self):
         lldbdap_testcase.DAPTestCaseBase.setUp(self)
         self.accessTypes = ["read", "write", "readWrite"]
 
+    @skipIfWindows
+    @skipIfRemote
+    def test_duplicate_start_addresses(self):
+        """Test setDataBreakpoints with multiple watchpoints starting at the same addresses."""
+        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()
+        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")
+        # Test response from dataBreakpointInfo request.
+        self.assertEquals(response_x["body"]["dataId"].split("/")[1], "4")
+        self.assertEquals(response_x["body"]["accessTypes"], self.accessTypes)
+        self.assertEquals(response_arr_2["body"]["dataId"].split("/")[1], "4")
+        self.assertEquals(response_arr_2["body"]["accessTypes"], self.accessTypes)
+        # The first one should be overwritten by the third one as they start at
+        # the same address. This is indicated by returning {verified: False} for
+        # the first one.
+        dataBreakpoints = [
+            {"dataId": response_x["body"]["dataId"], "accessType": "read"},
+            {"dataId": response_arr_2["body"]["dataId"], "accessType": "write"},
+            {"dataId": response_x["body"]["dataId"], "accessType": "write"},
+        ]
+        set_response = self.dap_server.request_setDataBreakpoint(dataBreakpoints)
+        self.assertEquals(
+            set_response["body"]["breakpoints"],
+            [{"verified": False}, {"verified": True}, {"verified": True}],
+        )
+
+        self.continue_to_next_stop()
+        x_val = self.dap_server.get_local_variable_value("x")
+        i_val = self.dap_server.get_local_variable_value("i")
+        self.assertEquals(x_val, "2")
+        self.assertEquals(i_val, "1")
+
+        self.continue_to_next_stop()
+        arr_2 = self.dap_server.get_local_variable_child("arr", "[2]")
+        i_val = self.dap_server.get_local_variable_value("i")
+        self.assertEquals(arr_2["value"], "42")
+        self.assertEquals(i_val, "2")
+
     @skipIfWindows
     @skipIfRemote
     def test_expression(self):

lldb/tools/lldb-dap/Watchpoint.cpp

@@ -16,17 +16,11 @@ Watchpoint::Watchpoint(const llvm::json::Object &obj) : BreakpointBase(obj) {
   llvm::StringRef dataId = GetString(obj, "dataId");
   std::string accessType = GetString(obj, "accessType").str();
   auto [addr_str, size_str] = dataId.split('/');
-  lldb::addr_t addr;
-  size_t size;
   llvm::to_integer(addr_str, addr, 16);
   llvm::to_integer(size_str, size);
-  lldb::SBWatchpointOptions options;
   options.SetWatchpointTypeRead(accessType != "write");
   if (accessType != "read")
     options.SetWatchpointTypeWrite(lldb::eWatchpointWriteTypeOnModify);
-  wp = g_dap.target.WatchpointCreateByAddress(addr, size, options, error);
-  SetCondition();
-  SetHitCondition();
 }
 
 void Watchpoint::SetCondition() { wp.SetCondition(condition.c_str()); }
@@ -38,11 +32,20 @@ void Watchpoint::SetHitCondition() {
 }
 
 void Watchpoint::CreateJsonObject(llvm::json::Object &object) {
-  if (error.Success()) {
-    object.try_emplace("verified", true);
-  } else {
+  if (!error.IsValid() || error.Fail()) {
     object.try_emplace("verified", false);
-    EmplaceSafeString(object, "message", error.GetCString());
+    if (error.Fail())
+      EmplaceSafeString(object, "message", error.GetCString());
+  } else {
+    object.try_emplace("verified", true);
   }
 }
+
+void Watchpoint::SetWatchpoint() {
+  wp = g_dap.target.WatchpointCreateByAddress(addr, size, options, error);
+  if (!condition.empty())
+    SetCondition();
+  if (!hitCondition.empty())
+    SetHitCondition();
+}
 } // namespace lldb_dap

lldb/tools/lldb-dap/Watchpoint.h

@@ -17,6 +17,9 @@
 namespace lldb_dap {
 
 struct Watchpoint : public BreakpointBase {
+  lldb::addr_t addr;
+  size_t size;
+  lldb::SBWatchpointOptions options;
   // The LLDB breakpoint associated wit this watchpoint.
   lldb::SBWatchpoint wp;
   lldb::SBError error;
@@ -28,6 +31,8 @@ struct Watchpoint : public BreakpointBase {
   void SetCondition() override;
   void SetHitCondition() override;
   void CreateJsonObject(llvm::json::Object &object) override;
+
+  void SetWatchpoint();
 };
 } // namespace lldb_dap
 

lldb/tools/lldb-dap/lldb-dap.cpp

@@ -2880,15 +2880,29 @@ void request_setDataBreakpoints(const llvm::json::Object &request) {
   const auto *breakpoints = arguments->getArray("breakpoints");
   llvm::json::Array response_breakpoints;
   g_dap.target.DeleteAllWatchpoints();
+  std::vector<Watchpoint> watchpoints;
   if (breakpoints) {
     for (const auto &bp : *breakpoints) {
       const auto *bp_obj = bp.getAsObject();
       if (bp_obj) {
         Watchpoint wp(*bp_obj);
-        AppendBreakpoint(&wp, response_breakpoints);
+        watchpoints.push_back(wp);
       }
     }
   }
+  // If two watchpoints start at the same address, the latter overwrite the
+  // former. So, we only enable those at first-seen addresses when iterating
+  // backward.
+  std::set<lldb::addr_t> addresses;
+  for (auto iter = watchpoints.rbegin(); iter != watchpoints.rend(); ++iter) {
+    if (addresses.count(iter->addr) == 0) {
+      iter->SetWatchpoint();
+      addresses.insert(iter->addr);
+    }
+  }
+  for (auto wp : watchpoints)
+    AppendBreakpoint(&wp, response_breakpoints);
+
   llvm::json::Object body;
   body.try_emplace("breakpoints", std::move(response_breakpoints));
   response.try_emplace("body", std::move(body));