llvmbot wrote:

<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Zequan Wu (ZequanWu)

<details>
<summary>Changes</summary>

This implements functionality to handle DataBreakpointInfo request and 
SetDataBreakpoints request.

Previous commit 
https://github.com/llvm/llvm-project/commit/8c56e78ec531f0e2460213c20fff869b6b7add99
 was reverted because setting 1 byte watchpoint failed in the new test on 
ARM64. So, I changed the test to setting 4 byte watchpoint instead, and hope 
this won't break it again. It also adds the fixes from 
https://github.com/llvm/llvm-project/pull/81680.

---

Patch is 29.35 KiB, truncated to 20.00 KiB below, full version: 
https://github.com/llvm/llvm-project/pull/81909.diff


9 Files Affected:

- (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py 
(+47) 
- (added) lldb/test/API/tools/lldb-dap/databreakpoint/Makefile (+3) 
- (added) 
lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py 
(+133) 
- (added) lldb/test/API/tools/lldb-dap/databreakpoint/main.cpp (+17) 
- (modified) lldb/tools/lldb-dap/CMakeLists.txt (+1) 
- (modified) lldb/tools/lldb-dap/DAPForward.h (+2) 
- (added) lldb/tools/lldb-dap/Watchpoint.cpp (+48) 
- (added) lldb/tools/lldb-dap/Watchpoint.h (+34) 
- (modified) lldb/tools/lldb-dap/lldb-dap.cpp (+302-34) 


``````````diff
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index bb863bb8719176..27a76a652f4063 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -501,6 +501,18 @@ def get_local_variable_value(self, name, frameIndex=0, 
threadId=None):
             return variable["value"]
         return None
 
+    def get_local_variable_child(self, name, child_name, frameIndex=0, 
threadId=None):
+        local = self.get_local_variable(name, frameIndex, threadId)
+        if local["variablesReference"] == 0:
+            return None
+        children = self.request_variables(local["variablesReference"])["body"][
+            "variables"
+        ]
+        for child in children:
+            if child["name"] == child_name:
+                return child
+        return None
+
     def replay_packets(self, replay_file_path):
         f = open(replay_file_path, "r")
         mode = "invalid"
@@ -895,6 +907,41 @@ 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
+    ):
+        stackFrame = self.get_stackFrame(frameIndex=frameIndex, 
threadId=threadId)
+        if stackFrame is None:
+            return []
+        args_dict = {
+            "variablesReference": variablesReference,
+            "name": name,
+            "frameId": stackFrame["id"],
+        }
+        command_dict = {
+            "command": "dataBreakpointInfo",
+            "type": "request",
+            "arguments": args_dict,
+        }
+        return self.send_recv(command_dict)
+
+    def request_setDataBreakpoint(self, dataBreakpoints):
+        """dataBreakpoints is a list of dictionary with following fields:
+        {
+            dataId: (address in hex)/(size in bytes)
+            accessType: read/write/readWrite
+            [condition]: string
+            [hitCondition]: string
+        }
+        """
+        args_dict = {"breakpoints": dataBreakpoints}
+        command_dict = {
+            "command": "setDataBreakpoints",
+            "type": "request",
+            "arguments": args_dict,
+        }
+        return self.send_recv(command_dict)
+
     def request_compileUnits(self, moduleId):
         args_dict = {"moduleId": moduleId}
         command_dict = {
diff --git a/lldb/test/API/tools/lldb-dap/databreakpoint/Makefile 
b/lldb/test/API/tools/lldb-dap/databreakpoint/Makefile
new file mode 100644
index 00000000000000..99998b20bcb050
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/databreakpoint/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git 
a/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py 
b/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py
new file mode 100644
index 00000000000000..02afbe4180378d
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py
@@ -0,0 +1,133 @@
+"""
+Test lldb-dap dataBreakpointInfo and setDataBreakpoints requests
+"""
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+import lldbdap_testcase
+
+
+class TestDAP_setDataBreakpoints(lldbdap_testcase.DAPTestCaseBase):
+    def setUp(self):
+        lldbdap_testcase.DAPTestCaseBase.setUp(self)
+        self.accessTypes = ["read", "write", "readWrite"]
+
+    @skipIfWindows
+    @skipIfRemote
+    def test_expression(self):
+        """Tests setting data breakpoints on expression."""
+        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, "4@&x")
+        response_arr_2 = self.dap_server.request_dataBreakpointInfo(0, 
"4@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)
+        dataBreakpoints = [
+            {"dataId": response_x["body"]["dataId"], "accessType": "write"},
+            {"dataId": response_arr_2["body"]["dataId"], "accessType": 
"write"},
+        ]
+        set_response = self.dap_server.request_setDataBreakpoint(
+            dataBreakpoints)
+        self.assertEquals(set_response["body"]["breakpoints"], [
+                          {"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_functionality(self):
+        """Tests setting data breakpoints on variable."""
+        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_local_variables()
+        # Test write watchpoints on x, arr[2]
+        response_x = self.dap_server.request_dataBreakpointInfo(1, "x")
+        arr = self.dap_server.get_local_variable("arr")
+        response_arr_2 = self.dap_server.request_dataBreakpointInfo(
+            arr["variablesReference"], "[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)
+        dataBreakpoints = [
+            {"dataId": response_x["body"]["dataId"], "accessType": "write"},
+            {"dataId": response_arr_2["body"]["dataId"], "accessType": 
"write"},
+        ]
+        set_response = self.dap_server.request_setDataBreakpoint(
+            dataBreakpoints)
+        self.assertEquals(set_response["body"]["breakpoints"], [
+                          {"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")
+        self.dap_server.request_setDataBreakpoint([])
+
+        # Test hit condition
+        second_loop_break_line = line_number(source, "// second loop 
breakpoint")
+        breakpoint_ids = self.set_source_breakpoints(source, 
[second_loop_break_line])
+        self.continue_to_breakpoints(breakpoint_ids)
+        dataBreakpoints = [
+            {
+                "dataId": response_x["body"]["dataId"],
+                "accessType": "write",
+                "hitCondition": "2",
+            }
+        ]
+        set_response = self.dap_server.request_setDataBreakpoint(
+            dataBreakpoints)
+        self.assertEquals(set_response["body"]["breakpoints"], [
+                          {"verified": True}])
+        self.continue_to_next_stop()
+        x_val = self.dap_server.get_local_variable_value("x")
+        self.assertEquals(x_val, "3")
+
+        # Test condition
+        dataBreakpoints = [
+            {
+                "dataId": response_x["body"]["dataId"],
+                "accessType": "write",
+                "condition": "x==10",
+            }
+        ]
+        set_response = self.dap_server.request_setDataBreakpoint(
+            dataBreakpoints)
+        self.assertEquals(set_response["body"]["breakpoints"], [
+                          {"verified": True}])
+        self.continue_to_next_stop()
+        x_val = self.dap_server.get_local_variable_value("x")
+        self.assertEquals(x_val, "10")
diff --git a/lldb/test/API/tools/lldb-dap/databreakpoint/main.cpp 
b/lldb/test/API/tools/lldb-dap/databreakpoint/main.cpp
new file mode 100644
index 00000000000000..bef09c203845e4
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/databreakpoint/main.cpp
@@ -0,0 +1,17 @@
+int main(int argc, char const *argv[]) {
+  // Test for data breakpoint
+  int x = 0;
+  int arr[4] = {1, 2, 3, 4};
+  for (int i = 0; i < 5; ++i) { // first loop breakpoint
+    if (i == 1) {
+      x = i + 1;
+    } else if (i == 2) {
+      arr[i] = 42;
+    }
+  }
+
+  x = 1;
+  for (int i = 0; i < 10; ++i) { // second loop breakpoint
+    ++x;
+  }
+}
diff --git a/lldb/tools/lldb-dap/CMakeLists.txt 
b/lldb/tools/lldb-dap/CMakeLists.txt
index f8c0e4ecf36c2f..f8f0d86453f585 100644
--- a/lldb/tools/lldb-dap/CMakeLists.txt
+++ b/lldb/tools/lldb-dap/CMakeLists.txt
@@ -37,6 +37,7 @@ add_lldb_tool(lldb-dap
   RunInTerminal.cpp
   SourceBreakpoint.cpp
   DAP.cpp
+  Watchpoint.cpp
 
   LINK_LIBS
     liblldb
diff --git a/lldb/tools/lldb-dap/DAPForward.h b/lldb/tools/lldb-dap/DAPForward.h
index fffff1e3f79020..8c79488fae8dbf 100644
--- a/lldb/tools/lldb-dap/DAPForward.h
+++ b/lldb/tools/lldb-dap/DAPForward.h
@@ -14,6 +14,7 @@ struct BreakpointBase;
 struct ExceptionBreakpoint;
 struct FunctionBreakpoint;
 struct SourceBreakpoint;
+struct Watchpoint;
 } // namespace lldb_dap
 
 namespace lldb {
@@ -39,6 +40,7 @@ class SBStringList;
 class SBTarget;
 class SBThread;
 class SBValue;
+class SBWatchpoint;
 } // namespace lldb
 
 #endif
diff --git a/lldb/tools/lldb-dap/Watchpoint.cpp 
b/lldb/tools/lldb-dap/Watchpoint.cpp
new file mode 100644
index 00000000000000..2f176e0da84f15
--- /dev/null
+++ b/lldb/tools/lldb-dap/Watchpoint.cpp
@@ -0,0 +1,48 @@
+//===-- Watchpoint.cpp ------------------------------------------*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "Watchpoint.h"
+#include "DAP.h"
+#include "JSONUtils.h"
+#include "llvm/ADT/StringExtras.h"
+
+namespace lldb_dap {
+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()); }
+
+void Watchpoint::SetHitCondition() {
+  uint64_t hitCount = 0;
+  if (llvm::to_integer(hitCondition, hitCount))
+    wp.SetIgnoreCount(hitCount - 1);
+}
+
+void Watchpoint::CreateJsonObject(llvm::json::Object &object) {
+  if (error.Success()) {
+    object.try_emplace("verified", true);
+  } else {
+    object.try_emplace("verified", false);
+    EmplaceSafeString(object, "message", error.GetCString());
+  }
+}
+} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Watchpoint.h b/lldb/tools/lldb-dap/Watchpoint.h
new file mode 100644
index 00000000000000..026b07d67241ce
--- /dev/null
+++ b/lldb/tools/lldb-dap/Watchpoint.h
@@ -0,0 +1,34 @@
+//===-- Watchpoint.h --------------------------------------------*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_TOOLS_LLDB_DAP_WATCHPOINT_H
+#define LLDB_TOOLS_LLDB_DAP_WATCHPOINT_H
+
+#include "BreakpointBase.h"
+#include "lldb/API/SBError.h"
+#include "lldb/API/SBWatchpoint.h"
+#include "lldb/API/SBWatchpointOptions.h"
+
+namespace lldb_dap {
+
+struct Watchpoint : public BreakpointBase {
+  // The LLDB breakpoint associated wit this watchpoint.
+  lldb::SBWatchpoint wp;
+  lldb::SBError error;
+
+  Watchpoint() = default;
+  Watchpoint(const llvm::json::Object &obj);
+  Watchpoint(lldb::SBWatchpoint wp) : wp(wp) {}
+
+  void SetCondition() override;
+  void SetHitCondition() override;
+  void CreateJsonObject(llvm::json::Object &object) override;
+};
+} // namespace lldb_dap
+
+#endif
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 67022347e6d624..bc2f08486da616 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -7,6 +7,8 @@
 
//===----------------------------------------------------------------------===//
 
 #include "DAP.h"
+#include "Watchpoint.h"
+#include "lldb/API/SBMemoryRegionInfo.h"
 
 #include <cassert>
 #include <climits>
@@ -560,6 +562,46 @@ void EventThreadFunction() {
   }
 }
 
+lldb::SBValue FindVariable(uint64_t variablesReference, llvm::StringRef name) {
+  lldb::SBValue variable;
+  if (lldb::SBValueList *top_scope = GetTopLevelScope(variablesReference)) {
+    bool is_duplicated_variable_name = name.contains(" @");
+    // variablesReference is one of our scopes, not an actual variable it is
+    // asking for a variable in locals or globals or registers
+    int64_t end_idx = top_scope->GetSize();
+    // Searching backward so that we choose the variable in closest scope
+    // among variables of the same name.
+    for (int64_t i = end_idx - 1; i >= 0; --i) {
+      lldb::SBValue curr_variable = top_scope->GetValueAtIndex(i);
+      std::string variable_name = CreateUniqueVariableNameForDisplay(
+          curr_variable, is_duplicated_variable_name);
+      if (variable_name == name) {
+        variable = curr_variable;
+        break;
+      }
+    }
+  } else {
+    // This is not under the globals or locals scope, so there are no 
duplicated
+    // names.
+
+    // We have a named item within an actual variable so we need to find it
+    // withing the container variable by name.
+    lldb::SBValue container = g_dap.variables.GetVariable(variablesReference);
+    variable = container.GetChildMemberWithName(name.data());
+    if (!variable.IsValid()) {
+      if (name.starts_with("[")) {
+        llvm::StringRef index_str(name.drop_front(1));
+        uint64_t index = 0;
+        if (!index_str.consumeInteger(0, index)) {
+          if (index_str == "]")
+            variable = container.GetChildAtIndex(index);
+        }
+      }
+    }
+  }
+  return variable;
+}
+
 // Both attach and launch take a either a sourcePath or sourceMap
 // argument (or neither), from which we need to set the target.source-map.
 void SetSourceMapFromArguments(const llvm::json::Object &arguments) {
@@ -1647,6 +1689,8 @@ void request_initialize(const llvm::json::Object 
&request) {
   body.try_emplace("supportsProgressReporting", true);
   // The debug adapter supports 'logMessage' in breakpoint.
   body.try_emplace("supportsLogPoints", true);
+  // The debug adapter supports data watchpoints.
+  body.try_emplace("supportsDataBreakpoints", true);
 
   response.try_emplace("body", std::move(body));
   g_dap.SendJSON(llvm::json::Value(std::move(response)));
@@ -2591,6 +2635,259 @@ void request_setFunctionBreakpoints(const 
llvm::json::Object &request) {
   g_dap.SendJSON(llvm::json::Value(std::move(response)));
 }
 
+// "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 th...
[truncated]

``````````

</details>


https://github.com/llvm/llvm-project/pull/81909
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to