[lldb] [llvm] [lldb-dap] Support column breakpoints (PR #113787)

via llvm-commits llvm-commits at lists.llvm.org
Sat Oct 26 20:39:47 PDT 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb

@llvm/pr-subscribers-debuginfo

Author: Adrian Vogelsgesang (vogelsgesang)

<details>
<summary>Changes</summary>

This commit adds support for column breakpoints to lldb-dap.

To do so, support for `breakpointLocations` was added. To find all available breakpoint positions, we iterate over the line table.

The `setBreakpoints` request already forwarded the column correctly to `SBTarget::BreakpointCreateByLocation`. However, `SourceBreakpointMap` did not keep track of multiple breakpoints in the same line. To do so, the `SourceBreakpointMap` is now indexed by line+column instead of by line only.

See http://jonasdevlieghere.com/post/lldb-column-breakpoints/ for a high-level introduction to column breakpoints.

---

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


7 Files Affected:

- (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py (+21-9) 
- (modified) lldb/test/API/tools/lldb-dap/breakpoint/Makefile (+1-1) 
- (added) lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_breakpointLocations.py (+74) 
- (modified) lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py (+104-68) 
- (modified) lldb/tools/lldb-dap/DAP.h (+1-1) 
- (modified) lldb/tools/lldb-dap/lldb-dap.cpp (+191-6) 
- (modified) llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLine.h (+1-1) 


``````````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 63748a71f1122d..659408c709a249 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
@@ -612,6 +612,22 @@ def request_attach(
         command_dict = {"command": "attach", "type": "request", "arguments": args_dict}
         return self.send_recv(command_dict)
 
+    def request_breakpointLocations(self, file_path, line, end_line=None, column=None, end_column=None):
+        (dir, base) = os.path.split(file_path)
+        source_dict = {"name": base, "path": file_path}
+        args_dict = {}
+        args_dict["source"] = source_dict
+        if line is not None:
+           args_dict["line"] = line
+        if end_line is not None:
+            args_dict["endLine"] = end_line
+        if column is not None:
+            args_dict["column"] = column
+        if end_column is not None:
+            args_dict["endColumn"] = end_column
+        command_dict = {"command": "breakpointLocations", "type": "request", "arguments": args_dict}
+        return self.send_recv(command_dict)
+
     def request_configurationDone(self):
         command_dict = {
             "command": "configurationDone",
@@ -912,18 +928,14 @@ def request_setBreakpoints(self, file_path, line_array, data=None):
                     breakpoint_data = data[i]
                 bp = {"line": line}
                 if breakpoint_data is not None:
-                    if "condition" in breakpoint_data and breakpoint_data["condition"]:
+                    if breakpoint_data.get("condition"):
                         bp["condition"] = breakpoint_data["condition"]
-                    if (
-                        "hitCondition" in breakpoint_data
-                        and breakpoint_data["hitCondition"]
-                    ):
+                    if breakpoint_data.get("hitCondition"):
                         bp["hitCondition"] = breakpoint_data["hitCondition"]
-                    if (
-                        "logMessage" in breakpoint_data
-                        and breakpoint_data["logMessage"]
-                    ):
+                    if breakpoint_data.get("logMessage"):
                         bp["logMessage"] = breakpoint_data["logMessage"]
+                    if breakpoint_data.get("column"):
+                        bp["column"] = breakpoint_data["column"]
                 breakpoints.append(bp)
             args_dict["breakpoints"] = breakpoints
 
diff --git a/lldb/test/API/tools/lldb-dap/breakpoint/Makefile b/lldb/test/API/tools/lldb-dap/breakpoint/Makefile
index 7634f513e85233..06438b3e6e3139 100644
--- a/lldb/test/API/tools/lldb-dap/breakpoint/Makefile
+++ b/lldb/test/API/tools/lldb-dap/breakpoint/Makefile
@@ -16,4 +16,4 @@ main-copy.cpp: main.cpp
 # The following shared library will be used to test breakpoints under dynamic loading
 libother:  other-copy.c
 	"$(MAKE)" -f $(MAKEFILE_RULES) \
-		DYLIB_ONLY=YES DYLIB_C_SOURCES=other-copy.c DYLIB_NAME=other 
+		DYLIB_ONLY=YES DYLIB_C_SOURCES=other-copy.c DYLIB_NAME=other
diff --git a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_breakpointLocations.py b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_breakpointLocations.py
new file mode 100644
index 00000000000000..d84ce843e3fba8
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_breakpointLocations.py
@@ -0,0 +1,74 @@
+"""
+Test lldb-dap breakpointLocations request
+"""
+
+
+import dap_server
+import shutil
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+import lldbdap_testcase
+import os
+
+
+class TestDAP_setBreakpoints(lldbdap_testcase.DAPTestCaseBase):
+    def setUp(self):
+        lldbdap_testcase.DAPTestCaseBase.setUp(self)
+
+        self.main_basename = "main-copy.cpp"
+        self.main_path = os.path.realpath(self.getBuildArtifact(self.main_basename))
+
+    @skipIfWindows
+    def test_column_breakpoints(self):
+        """Test retrieving the available breakpoint locations."""
+        program = self.getBuildArtifact("a.out")
+        self.build_and_launch(program, stopOnEntry=True)
+        loop_line = line_number(self.main_path, "// break loop")
+        self.dap_server.request_continue()
+
+        # Ask for the breakpoint locations based only on the line number
+        response = self.dap_server.request_breakpointLocations(self.main_path, loop_line)
+        self.assertTrue(response["success"])
+        self.assertEqual(response["body"]["breakpoints"], [
+            { "line": loop_line, "column": 9 },
+            { "line": loop_line, "column": 13 },
+            { "line": loop_line, "column": 20 },
+            { "line": loop_line, "column": 23 },
+            { "line": loop_line, "column": 25 },
+            { "line": loop_line, "column": 34 },
+            { "line": loop_line, "column": 37 },
+            { "line": loop_line, "column": 39 },
+            { "line": loop_line, "column": 51 }
+        ])
+
+        # Ask for the breakpoint locations for a column range
+        response = self.dap_server.request_breakpointLocations(
+            self.main_path,
+            loop_line,
+            column = 24,
+            end_column = 46,
+        )
+        self.assertTrue(response["success"])
+        self.assertEqual(response["body"]["breakpoints"], [
+            { "line": loop_line, "column": 25 },
+            { "line": loop_line, "column": 34 },
+            { "line": loop_line, "column": 37 },
+            { "line": loop_line, "column": 39 },
+        ])
+
+        # Ask for the breakpoint locations for a range of line numbers
+        response = self.dap_server.request_breakpointLocations(
+            self.main_path,
+            line = loop_line,
+            end_line = loop_line + 2,
+            column = 39,
+        )
+        self.maxDiff=None
+        self.assertTrue(response["success"])
+        self.assertEqual(response["body"]["breakpoints"], [
+            {'column': 39, 'line': 40},
+            {'column': 51, 'line': 40},
+            {'column': 3, 'line': 42},
+            {'column': 18, 'line': 42}
+        ])
diff --git a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py
index 123fea79c5cda8..2a2b7bbf6d29f2 100644
--- a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py
+++ b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py
@@ -125,20 +125,18 @@ def test_set_and_clear(self):
         # Set 3 breakpoints and verify that they got set correctly
         response = self.dap_server.request_setBreakpoints(self.main_path, lines)
         line_to_id = {}
-        if response:
-            breakpoints = response["body"]["breakpoints"]
-            self.assertEqual(
-                len(breakpoints),
-                len(lines),
-                "expect %u source breakpoints" % (len(lines)),
-            )
-            for breakpoint, index in zip(breakpoints, range(len(lines))):
-                line = breakpoint["line"]
-                self.assertTrue(line, lines[index])
-                # Store the "id" of the breakpoint that was set for later
-                line_to_id[line] = breakpoint["id"]
-                self.assertIn(line, lines, "line expected in lines array")
-                self.assertTrue(breakpoint["verified"], "expect breakpoint verified")
+        breakpoints = response["body"]["breakpoints"]
+        self.assertEqual(
+            len(breakpoints),
+            len(lines),
+            "expect %u source breakpoints" % (len(lines)),
+        )
+        for index, breakpoint in enumerate(breakpoints):
+            line = breakpoint["line"]
+            self.assertEqual(line, lines[index])
+            # Store the "id" of the breakpoint that was set for later
+            line_to_id[line] = breakpoint["id"]
+            self.assertTrue(breakpoint["verified"], "expect breakpoint verified")
 
         # There is no breakpoint delete packet, clients just send another
         # setBreakpoints packet with the same source file with fewer lines.
@@ -151,75 +149,70 @@ def test_set_and_clear(self):
         # Set 2 breakpoints and verify that the previous breakpoints that were
         # set above are still set.
         response = self.dap_server.request_setBreakpoints(self.main_path, lines)
-        if response:
-            breakpoints = response["body"]["breakpoints"]
+        breakpoints = response["body"]["breakpoints"]
+        self.assertEqual(
+            len(breakpoints),
+            len(lines),
+            "expect %u source breakpoints" % (len(lines)),
+        )
+        for index, breakpoint in enumerate(breakpoints):
+            line = breakpoint["line"]
+            self.assertEqual(line, lines[index])
+            # Verify the same breakpoints are still set within LLDB by
+            # making sure the breakpoint ID didn't change
             self.assertEqual(
-                len(breakpoints),
-                len(lines),
-                "expect %u source breakpoints" % (len(lines)),
+                line_to_id[line],
+                breakpoint["id"],
+                "verify previous breakpoints stayed the same",
+            )
+            self.assertTrue(
+                breakpoint["verified"], "expect breakpoint still verified"
             )
-            for breakpoint, index in zip(breakpoints, range(len(lines))):
-                line = breakpoint["line"]
-                self.assertTrue(line, lines[index])
-                # Verify the same breakpoints are still set within LLDB by
-                # making sure the breakpoint ID didn't change
-                self.assertEqual(
-                    line_to_id[line],
-                    breakpoint["id"],
-                    "verify previous breakpoints stayed the same",
-                )
-                self.assertIn(line, lines, "line expected in lines array")
-                self.assertTrue(
-                    breakpoint["verified"], "expect breakpoint still verified"
-                )
 
         # Now get the full list of breakpoints set in the target and verify
         # we have only 2 breakpoints set. The response above could have told
         # us about 2 breakpoints, but we want to make sure we don't have the
         # third one still set in the target
         response = self.dap_server.request_testGetTargetBreakpoints()
-        if response:
-            breakpoints = response["body"]["breakpoints"]
+        breakpoints = response["body"]["breakpoints"]
+        self.assertEqual(
+            len(breakpoints),
+            len(lines),
+            "expect %u source breakpoints" % (len(lines)),
+        )
+        for breakpoint in breakpoints:
+            line = breakpoint["line"]
+            # Verify the same breakpoints are still set within LLDB by
+            # making sure the breakpoint ID didn't change
             self.assertEqual(
-                len(breakpoints),
-                len(lines),
-                "expect %u source breakpoints" % (len(lines)),
+                line_to_id[line],
+                breakpoint["id"],
+                "verify previous breakpoints stayed the same",
+            )
+            self.assertIn(line, lines, "line expected in lines array")
+            self.assertTrue(
+                breakpoint["verified"], "expect breakpoint still verified"
             )
-            for breakpoint in breakpoints:
-                line = breakpoint["line"]
-                # Verify the same breakpoints are still set within LLDB by
-                # making sure the breakpoint ID didn't change
-                self.assertEqual(
-                    line_to_id[line],
-                    breakpoint["id"],
-                    "verify previous breakpoints stayed the same",
-                )
-                self.assertIn(line, lines, "line expected in lines array")
-                self.assertTrue(
-                    breakpoint["verified"], "expect breakpoint still verified"
-                )
 
         # Now clear all breakpoints for the source file by passing down an
         # empty lines array
         lines = []
         response = self.dap_server.request_setBreakpoints(self.main_path, lines)
-        if response:
-            breakpoints = response["body"]["breakpoints"]
-            self.assertEqual(
-                len(breakpoints),
-                len(lines),
-                "expect %u source breakpoints" % (len(lines)),
-            )
+        breakpoints = response["body"]["breakpoints"]
+        self.assertEqual(
+            len(breakpoints),
+            len(lines),
+            "expect %u source breakpoints" % (len(lines)),
+        )
 
         # Verify with the target that all breakpoints have been cleared
         response = self.dap_server.request_testGetTargetBreakpoints()
-        if response:
-            breakpoints = response["body"]["breakpoints"]
-            self.assertEqual(
-                len(breakpoints),
-                len(lines),
-                "expect %u source breakpoints" % (len(lines)),
-            )
+        breakpoints = response["body"]["breakpoints"]
+        self.assertEqual(
+            len(breakpoints),
+            len(lines),
+            "expect %u source breakpoints" % (len(lines)),
+        )
 
         # Now set a breakpoint again in the same source file and verify it
         # was added.
@@ -281,12 +274,11 @@ def test_clear_breakpoints_unset_breakpoints(self):
         self.assertEqual(
             len(breakpoints), len(lines), "expect %u source breakpoints" % (len(lines))
         )
-        for breakpoint, index in zip(breakpoints, range(len(lines))):
+        for index, breakpoint in enumerate(breakpoints):
             line = breakpoint["line"]
-            self.assertTrue(line, lines[index])
+            self.assertEqual(line, lines[index])
             # Store the "id" of the breakpoint that was set for later
             line_to_id[line] = breakpoint["id"]
-            self.assertIn(line, lines, "line expected in lines array")
             self.assertTrue(breakpoint["verified"], "expect breakpoint verified")
 
         # Now clear all breakpoints for the source file by not setting the
@@ -356,3 +348,47 @@ def test_functionality(self):
         self.continue_to_breakpoints(breakpoint_ids)
         i = int(self.dap_server.get_local_variable_value("i"))
         self.assertEqual(i, 7, "i != 7 showing post hitCondition hits every time")
+
+    @skipIfWindows
+    def test_column_breakpoints(self):
+        """Test setting multiple breakpoints in the same line at different columns."""
+        loop_line = line_number("main.cpp", "// break loop")
+
+        program = self.getBuildArtifact("a.out")
+        self.build_and_launch(program)
+
+        # Set two breakpoints on the loop line at different columns.
+        columns = [13, 39]
+        response = self.dap_server.request_setBreakpoints(self.main_path, [loop_line, loop_line], list({"column": c} for c in columns))
+
+        # Verify the breakpoints were set correctly
+        breakpoints = response["body"]["breakpoints"]
+        breakpoint_ids = []
+        self.assertEqual(
+            len(breakpoints),
+            len(columns),
+            "expect %u source breakpoints" % (len(columns)),
+        )
+        for index, breakpoint in enumerate(breakpoints):
+            self.assertEqual(breakpoint["line"], loop_line)
+            self.assertEqual(breakpoint["column"], columns[index])
+            self.assertTrue(breakpoint["verified"], "expect breakpoint verified")
+            breakpoint_ids.append(breakpoint["id"])
+
+        # Continue to the first breakpoint,
+        self.continue_to_breakpoints([breakpoint_ids[0]])
+
+        # We should have stopped right before the call to `twelve`.
+        # Step into and check we are inside `twelve`.
+        self.stepIn()
+        func_name = self.get_stackFrames()[0]["name"]
+        self.assertEqual(func_name, "twelve")
+
+        # Continue to the second breakpoint.
+        self.continue_to_breakpoints([breakpoint_ids[1]])
+
+        # We should have stopped right before the call to `fourteen`.
+        # Step into and check we are inside `fourteen`.
+        self.stepIn()
+        func_name = self.get_stackFrames()[0]["name"]
+        self.assertEqual(func_name, "a::fourteen")
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index acc10ade75fd14..49c2b5b34afc3d 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -67,7 +67,7 @@
 
 namespace lldb_dap {
 
-typedef llvm::DenseMap<uint32_t, SourceBreakpoint> SourceBreakpointMap;
+typedef llvm::DenseMap<std::pair<uint32_t, uint32_t>, SourceBreakpoint> SourceBreakpointMap;
 typedef llvm::StringMap<FunctionBreakpoint> FunctionBreakpointMap;
 typedef llvm::DenseMap<lldb::addr_t, InstructionBreakpoint>
     InstructionBreakpointMap;
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index f70b0d3d4cbee0..9b84da08d442a9 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -9,7 +9,10 @@
 #include "DAP.h"
 #include "Watchpoint.h"
 #include "lldb/API/SBDeclaration.h"
+#include "lldb/API/SBFileSpec.h"
+#include "lldb/API/SBLineEntry.h"
 #include "lldb/API/SBMemoryRegionInfo.h"
+#include "lldb/lldb-defines.h"
 #include "llvm/Support/Base64.h"
 
 #include <cassert>
@@ -18,6 +21,7 @@
 #include <cstdio>
 #include <cstdlib>
 #include <cstring>
+#include <limits>
 #include <optional>
 #include <sys/stat.h>
 #include <sys/types.h>
@@ -910,6 +914,183 @@ void request_attach(const llvm::json::Object &request) {
   }
 }
 
+// "BreakpointLocationsRequest": {
+//   "allOf": [ { "$ref": "#/definitions/Request" }, {
+//     "type": "object",
+//     "description": "The `breakpointLocations` request returns all possible
+//     locations for source breakpoints in a given range.\nClients should only
+//     call this request if the corresponding capability
+//     `supportsBreakpointLocationsRequest` is true.",
+//     "properties": {
+//       "command": {
+//         "type": "string",
+//         "enum": [ "breakpointLocations" ]
+//       },
+//       "arguments": {
+//         "$ref": "#/definitions/BreakpointLocationsArguments"
+//       }
+//     },
+//     "required": [ "command" ]
+//   }]
+// },
+// "BreakpointLocationsArguments": {
+//   "type": "object",
+//   "description": "Arguments for `breakpointLocations` request.",
+//   "properties": {
+//     "source": {
+//       "$ref": "#/definitions/Source",
+//       "description": "The source location of the breakpoints; either
+//       `source.path` or `source.sourceReference` must be specified."
+//     },
+//     "line": {
+//       "type": "integer",
+//       "description": "Start line of range to search possible breakpoint
+//       locations in. If only the line is specified, the request returns all
+//       possible locations in that line."
+//     },
+//     "column": {
+//       "type": "integer",
+//       "description": "Start position within `line` to search possible
+//       breakpoint locations in. It is measured in UTF-16 code units and the
+//       client capability `columnsStartAt1` determines whether it is 0- or
+//       1-based. If no column is given, the first position in the start line is
+//       assumed."
+//     },
+//     "endLine": {
+//       "type": "integer",
+//       "description": "End line of range to search possible breakpoint
+//       locations in. If no end line is given, then the end line is assumed to
+//       be the start line."
+//     },
+//     "endColumn": {
+//       "type": "integer",
+//       "description": "End position within `endLine` to search possible
+//       breakpoint locations in. It is measured in UTF-16 code units and the
+//       client capability `columnsStartAt1` determines whether it is 0- or
+//       1-based. If no end column is given, the last position in the end line
+//       is assumed."
+//     }
+//   },
+//   "required": [ "source", "line" ]
+// },
+// "BreakpointLocationsResponse": {
+//   "allOf": [ { "$ref": "#/definitions/Response" }, {
+//     "type": "object",
+//     "description": "Response to `breakpointLocations` request.\nContains
+//     p...
[truncated]

``````````

</details>


https://github.com/llvm/llvm-project/pull/113787


More information about the llvm-commits mailing list