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

Adrian Vogelsgesang via lldb-commits lldb-commits at lists.llvm.org
Thu Oct 31 04:55:09 PDT 2024


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

>From af45bc2e24623d7225d24a4680a28630d67d636e Mon Sep 17 00:00:00 2001
From: Adrian Vogelsgesang <avogelsgesang at salesforce.com>
Date: Sat, 26 Oct 2024 14:34:31 +0000
Subject: [PATCH 1/4] [lldb-dap] Support column breakpoints

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.
---
 .../test/tools/lldb-dap/dap_server.py         |  30 ++-
 .../API/tools/lldb-dap/breakpoint/Makefile    |   2 +-
 .../breakpoint/TestDAP_breakpointLocations.py |  74 +++++++
 .../breakpoint/TestDAP_setBreakpoints.py      | 172 +++++++++------
 lldb/tools/lldb-dap/DAP.h                     |   2 +-
 lldb/tools/lldb-dap/lldb-dap.cpp              | 197 +++++++++++++++++-
 .../llvm/DebugInfo/DWARF/DWARFDebugLine.h     |   2 +-
 7 files changed, 393 insertions(+), 86 deletions(-)
 create mode 100644 lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_breakpointLocations.py

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
+//     possible locations for source breakpoints.",
+//     "properties": {
+//       "body": {
+//         "type": "object",
+//         "properties": {
+//           "breakpoints": {
+//             "type": "array",
+//             "items": {
+//               "$ref": "#/definitions/BreakpointLocation"
+//             },
+//             "description": "Sorted set of possible breakpoint locations."
+//           }
+//         },
+//         "required": [ "breakpoints" ]
+//       }
+//     },
+//     "required": [ "body" ]
+//   }]
+// },
+// "BreakpointLocation": {
+//   "type": "object",
+//   "description": "Properties of a breakpoint location returned from the
+//   `breakpointLocations` request.",
+//   "properties": {
+//     "line": {
+//       "type": "integer",
+//       "description": "Start line of breakpoint location."
+//     },
+//     "column": {
+//       "type": "integer",
+//       "description": "The start position of a breakpoint location. Position
+//       is measured in UTF-16 code units and the client capability
+//       `columnsStartAt1` determines whether it is 0- or 1-based."
+//     },
+//     "endLine": {
+//       "type": "integer",
+//       "description": "The end line of breakpoint location if the location
+//       covers a range."
+//     },
+//     "endColumn": {
+//       "type": "integer",
+//       "description": "The end position of a breakpoint location (if the
+//       location covers a range). Position is measured in UTF-16 code units and
+//       the client capability `columnsStartAt1` determines whether it is 0- or
+//       1-based."
+//     }
+//   },
+//   "required": [ "line" ]
+// },
+void request_breakpointLocations(const llvm::json::Object &request) {
+  llvm::json::Object response;
+  FillResponse(request, response);
+  auto *arguments = request.getObject("arguments");
+  auto *source = arguments->getObject("source");
+  std::string path = GetString(source, "path").str();
+  uint64_t start_line = GetUnsigned(arguments, "line", 0);
+  uint64_t start_column = GetUnsigned(arguments, "column", 0);
+  uint64_t end_line = GetUnsigned(arguments, "endLine", start_line);
+  uint64_t end_column =
+      GetUnsigned(arguments, "endColumn", std::numeric_limits<uint64_t>::max());
+
+  lldb::SBFileSpec file_spec(path.c_str(), true);
+  lldb::SBSymbolContextList compile_units =
+      g_dap.target.FindCompileUnits(file_spec);
+
+  // Find all relevant lines & columns
+  llvm::SmallVector<std::pair<uint32_t, uint32_t>, 8> locations;
+  for (uint32_t c_idx = 0, c_limit = compile_units.GetSize(); c_idx < c_limit;
+       ++c_idx) {
+    const lldb::SBCompileUnit &compile_unit =
+        compile_units.GetContextAtIndex(c_idx).GetCompileUnit();
+    if (!compile_units.IsValid())
+      continue;
+
+    // Go through the line table and find all matching lines / columns
+    for (uint32_t l_idx = 0, l_limit = compile_unit.GetNumLineEntries();
+         l_idx < l_limit; ++l_idx) {
+      lldb::SBLineEntry line_entry = compile_unit.GetLineEntryAtIndex(l_idx);
+
+      // Filter by line / column
+      uint32_t line = line_entry.GetLine();
+      if (line < start_line || line > end_line)
+        continue;
+      uint32_t column = line_entry.GetColumn();
+      if (column == LLDB_INVALID_COLUMN_NUMBER)
+        continue;
+      if (line == start_line && column < start_column)
+        continue;
+      if (line == end_line && column > end_column)
+        continue;
+      locations.emplace_back(line, column);
+    }
+  }
+
+  // The line entries are sorted by addresses, but we must return the list
+  // ordered by line / column position.
+  std::sort(locations.begin(), locations.end());
+  locations.erase(std::unique(locations.begin(), locations.end()),
+                  locations.end());
+
+  llvm::json::Array locations_json;
+  for (auto &l : locations) {
+    llvm::json::Object location;
+    location.try_emplace("line", l.first);
+    location.try_emplace("column", l.second);
+    locations_json.emplace_back(std::move(location));
+  }
+
+  llvm::json::Object body;
+  body.try_emplace("breakpoints", std::move(locations_json));
+  response.try_emplace("body", std::move(body));
+  g_dap.SendJSON(llvm::json::Value(std::move(response)));
+}
+
 // "ContinueRequest": {
 //   "allOf": [ { "$ref": "#/definitions/Request" }, {
 //     "type": "object",
@@ -1941,6 +2122,8 @@ void request_initialize(const llvm::json::Object &request) {
   body.try_emplace("supportsCompletionsRequest", true);
   // The debug adapter supports the disassembly request.
   body.try_emplace("supportsDisassembleRequest", true);
+  // The debug adapter supports the `breakpointLocations` request.
+  body.try_emplace("supportsBreakpointLocationsRequest", true);
   // The debug adapter supports stepping granularities (argument `granularity`)
   // for the stepping requests.
   body.try_emplace("supportsSteppingGranularity", true);
@@ -2701,16 +2884,16 @@ void request_setBreakpoints(const llvm::json::Object &request) {
   // to an empty array.
   if (breakpoints) {
     for (const auto &bp : *breakpoints) {
-      auto bp_obj = bp.getAsObject();
+      auto *bp_obj = bp.getAsObject();
       if (bp_obj) {
         SourceBreakpoint src_bp(*bp_obj);
-        request_bps[src_bp.line] = src_bp;
+        std::pair<uint32_t, uint32_t> bp_pos(src_bp.line, src_bp.column);
+        request_bps[bp_pos] = src_bp;
 
         // We check if this breakpoint already exists to update it
         auto existing_source_bps = g_dap.source_breakpoints.find(path);
         if (existing_source_bps != g_dap.source_breakpoints.end()) {
-          const auto &existing_bp =
-              existing_source_bps->second.find(src_bp.line);
+          const auto &existing_bp = existing_source_bps->second.find(bp_pos);
           if (existing_bp != existing_source_bps->second.end()) {
             existing_bp->second.UpdateBreakpoint(src_bp);
             AppendBreakpoint(&existing_bp->second, response_breakpoints, path,
@@ -2719,8 +2902,8 @@ void request_setBreakpoints(const llvm::json::Object &request) {
           }
         }
         // At this point the breakpoint is new
-        g_dap.source_breakpoints[path][src_bp.line] = src_bp;
-        SourceBreakpoint &new_bp = g_dap.source_breakpoints[path][src_bp.line];
+        g_dap.source_breakpoints[path][bp_pos] = src_bp;
+        SourceBreakpoint &new_bp = g_dap.source_breakpoints[path][bp_pos];
         new_bp.SetBreakpoint(path.data());
         AppendBreakpoint(&new_bp, response_breakpoints, path, new_bp.line);
       }
@@ -4810,6 +4993,8 @@ void request_setInstructionBreakpoints(const llvm::json::Object &request) {
 
 void RegisterRequestCallbacks() {
   g_dap.RegisterRequestCallback("attach", request_attach);
+  g_dap.RegisterRequestCallback("breakpointLocations",
+                                request_breakpointLocations);
   g_dap.RegisterRequestCallback("completions", request_completions);
   g_dap.RegisterRequestCallback("continue", request_continue);
   g_dap.RegisterRequestCallback("configurationDone", request_configurationDone);
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLine.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLine.h
index ff7bf87d8e6b5a..81c19e0acf602f 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLine.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLine.h
@@ -78,7 +78,7 @@ class DWARFDebugLine {
     /// The maximum number of individual operations that may be encoded in an
     /// instruction.
     uint8_t MaxOpsPerInst;
-    /// The initial value of theis_stmtregister.
+    /// The initial value of the is_stmt register.
     uint8_t DefaultIsStmt;
     /// This parameter affects the meaning of the special opcodes. See below.
     int8_t LineBase;

>From 69376869bfbe8caff51e937f2ca39926d7522b48 Mon Sep 17 00:00:00 2001
From: Adrian Vogelsgesang <avogelsgesang at salesforce.com>
Date: Sun, 27 Oct 2024 03:50:27 +0000
Subject: [PATCH 2/4] Fix formatting

---
 .../test/tools/lldb-dap/dap_server.py         | 12 +++-
 .../breakpoint/TestDAP_breakpointLocations.py | 71 +++++++++++--------
 .../breakpoint/TestDAP_setBreakpoints.py      | 12 ++--
 lldb/tools/lldb-dap/DAP.h                     |  3 +-
 4 files changed, 57 insertions(+), 41 deletions(-)

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 659408c709a249..0384e257a6868d 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,20 +612,26 @@ 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):
+    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
+            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}
+        command_dict = {
+            "command": "breakpointLocations",
+            "type": "request",
+            "arguments": args_dict,
+        }
         return self.send_recv(command_dict)
 
     def request_configurationDone(self):
diff --git a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_breakpointLocations.py b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_breakpointLocations.py
index d84ce843e3fba8..4803451bdc8aaa 100644
--- a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_breakpointLocations.py
+++ b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_breakpointLocations.py
@@ -28,47 +28,58 @@ def test_column_breakpoints(self):
         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)
+        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 }
-        ])
+        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,
+            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 },
-        ])
+        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,
+            line=loop_line,
+            end_line=loop_line + 2,
+            column=39,
         )
-        self.maxDiff=None
+        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}
-        ])
+        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 2a2b7bbf6d29f2..d408b72dc4975b 100644
--- a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py
+++ b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py
@@ -165,9 +165,7 @@ def test_set_and_clear(self):
                 breakpoint["id"],
                 "verify previous breakpoints stayed the same",
             )
-            self.assertTrue(
-                breakpoint["verified"], "expect breakpoint still verified"
-            )
+            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
@@ -190,9 +188,7 @@ def test_set_and_clear(self):
                 "verify previous breakpoints stayed the same",
             )
             self.assertIn(line, lines, "line expected in lines array")
-            self.assertTrue(
-                breakpoint["verified"], "expect breakpoint still verified"
-            )
+            self.assertTrue(breakpoint["verified"], "expect breakpoint still verified")
 
         # Now clear all breakpoints for the source file by passing down an
         # empty lines array
@@ -359,7 +355,9 @@ def test_column_breakpoints(self):
 
         # 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))
+        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"]
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index 49c2b5b34afc3d..8ca46a7ea7337b 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -67,7 +67,8 @@
 
 namespace lldb_dap {
 
-typedef llvm::DenseMap<std::pair<uint32_t, 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;

>From 1f655ecf35add94b4c194119e324a48c05cd9a76 Mon Sep 17 00:00:00 2001
From: Adrian Vogelsgesang <avogelsgesang at salesforce.com>
Date: Sun, 27 Oct 2024 13:21:00 +0000
Subject: [PATCH 3/4] Fix test case

---
 .../Python/lldbsuite/test/tools/lldb-dap/dap_server.py        | 2 ++
 .../Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py  | 3 ++-
 .../API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py   | 4 ++--
 3 files changed, 6 insertions(+), 3 deletions(-)

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 0384e257a6868d..aaa1c93098b025 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
@@ -874,6 +874,8 @@ def request_next(self, threadId, granularity="statement"):
     def request_stepIn(self, threadId, targetId, granularity="statement"):
         if self.exit_status is not None:
             raise ValueError("request_stepIn called after process exited")
+        if threadId is None:
+            threadId = self.get_thread_id()
         args_dict = {
             "threadId": threadId,
             "targetId": targetId,
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
index a25466f07fa557..d3faab99059c86 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
@@ -238,9 +238,10 @@ def set_global(self, name, value, id=None):
     def stepIn(
         self, threadId=None, targetId=None, waitForStop=True, granularity="statement"
     ):
-        self.dap_server.request_stepIn(
+        response = self.dap_server.request_stepIn(
             threadId=threadId, targetId=targetId, granularity=granularity
         )
+        assert response["success"] is True
         if waitForStop:
             return self.dap_server.wait_for_stopped()
         return None
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 d408b72dc4975b..c62feda64a1254 100644
--- a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py
+++ b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py
@@ -380,7 +380,7 @@ def test_column_breakpoints(self):
         # Step into and check we are inside `twelve`.
         self.stepIn()
         func_name = self.get_stackFrames()[0]["name"]
-        self.assertEqual(func_name, "twelve")
+        self.assertEqual(func_name, "twelve(int)")
 
         # Continue to the second breakpoint.
         self.continue_to_breakpoints([breakpoint_ids[1]])
@@ -389,4 +389,4 @@ def test_column_breakpoints(self):
         # Step into and check we are inside `fourteen`.
         self.stepIn()
         func_name = self.get_stackFrames()[0]["name"]
-        self.assertEqual(func_name, "a::fourteen")
+        self.assertEqual(func_name, "a::fourteen(int)")

>From 46ce2bf10a95f649aeca5f46ec614cc7c8570db5 Mon Sep 17 00:00:00 2001
From: Adrian Vogelsgesang <avogelsgesang at salesforce.com>
Date: Thu, 31 Oct 2024 11:54:34 +0000
Subject: [PATCH 4/4] Addres review comment re `self.assertTrue`

---
 .../Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py    | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
index d3faab99059c86..34e9b96dbcc3f5 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
@@ -241,7 +241,7 @@ def stepIn(
         response = self.dap_server.request_stepIn(
             threadId=threadId, targetId=targetId, granularity=granularity
         )
-        assert response["success"] is True
+        self.assertTrue(response["success"])
         if waitForStop:
             return self.dap_server.wait_for_stopped()
         return None



More information about the lldb-commits mailing list