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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
