[Lldb-commits] [lldb] [llvm] [lldb-dap] Support column breakpoints (PR #125347)
via lldb-commits
lldb-commits at lists.llvm.org
Sat Feb 1 07:39:05 PST 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Adrian Vogelsgesang (vogelsgesang)
<details>
<summary>Changes</summary>
This commit adds support for column breakpoints to lldb-dap.
To do so, support for the `breakpointLocations` request 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.
This was previously submitted as #<!-- -->113787, but got reverted due to failures
on ARM and macOS. This second attempt has less strict test case
expectations. Also, I added a release note.
---
Patch is 28.44 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/125347.diff
8 Files Affected:
- (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py (+29-9)
- (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py (+2-1)
- (modified) lldb/test/API/tools/lldb-dap/breakpoint/Makefile (+1-1)
- (added) lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_breakpointLocations.py (+88)
- (modified) lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py (+102-68)
- (modified) lldb/tools/lldb-dap/DAP.h (+2-1)
- (modified) lldb/tools/lldb-dap/lldb-dap.cpp (+197-2)
- (modified) llvm/docs/ReleaseNotes.md (+4)
``````````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 c29992ce9c7848..043d82e2e2c7d1 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,28 @@ 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",
@@ -851,6 +873,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,
@@ -911,18 +935,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/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
index a25466f07fa557..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
@@ -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
)
+ self.assertTrue(response["success"])
if waitForStop:
return self.dap_server.wait_for_stopped()
return None
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..1058157e2c6683
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_breakpointLocations.py
@@ -0,0 +1,88 @@
+"""
+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_breakpointLocations(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"])
+ # On some systems, there is an additional breakpoint available
+ # at line 41, column 3, i.e. at the end of the loop. To make this
+ # test more portable, only check that all expected breakpoints are
+ # presented, but also accept additional breakpoints.
+ expected_breakpoints = [
+ {"column": 39, "line": 40},
+ {"column": 51, "line": 40},
+ {"column": 3, "line": 42},
+ {"column": 18, "line": 42},
+ ]
+ for bp in expected_breakpoints:
+ self.assertIn(bp, response["body"]["breakpoints"])
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..c62feda64a1254 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,66 @@ 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",
)
- 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"
- )
+ 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",
)
- 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"
- )
+ 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 +270,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 +344,49 @@ 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(int)")
+
+ # 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(int)")
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index 846300cb945b0d..b23be68ea002fd 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -50,7 +50,8 @@
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 9e0e7f21ce4fc7..e323990d8b6ed0 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -912,6 +912,196 @@ void request_attach(DAP &dap, 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 i...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/125347
More information about the lldb-commits
mailing list