Skip to content

[lldb-dap] Support column breakpoints #125347

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 4, 2025

Conversation

vogelsgesang
Copy link
Member

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.

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.

See http://jonasdevlieghere.com/post/lldb-column-breakpoints/ for a
high-level introduction to column breakpoints.
@llvmbot
Copy link
Member

llvmbot commented Feb 1, 2025

@llvm/pr-subscribers-lldb

Author: Adrian Vogelsgesang (vogelsgesang)

Changes

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 --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]

@vogelsgesang vogelsgesang requested review from walter-erquinigo, labath and Michael137 and removed request for JDevlieghere February 1, 2025 15:40
Copy link
Member

@walter-erquinigo walter-erquinigo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM, but before merging, could you share more about the differences between the these cases and the actual logic between this patch and the previous attempt?

@vogelsgesang
Copy link
Member Author

vogelsgesang commented Feb 3, 2025

share more about the differences between the these cases and the actual logic between this patch and the previous attempt?

There are 3 separate commits in this PR. The first commit is literally the same as the originally reverted one.
The other two commits show the additional differences. 76af5fc is the most relevant change

Copy link
Member

@walter-erquinigo walter-erquinigo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!!

@vogelsgesang
Copy link
Member Author

@Michael137 Do you know if there is any way I could manually trigger the lldb-aarch64-ubuntu and the MacOS CI before merging this, to get CI feedback before merging this?

Also, from what I can tell, all test failures in Buildkite seem to be due to Bolt and unrelated to my PR. Is it safe to merge despite those failures? Or could they hide actual LLDB issues potentially introduced by my commit?

@Michael137
Copy link
Member

@Michael137 Do you know if there is any way I could manually trigger the lldb-aarch64-ubuntu and the MacOS CI before merging this, to get CI feedback before merging this?

Also, from what I can tell, all test failures in Buildkite seem to be due to Bolt and unrelated to my PR. Is it safe to merge despite those failures? Or could they hide actual LLDB issues potentially introduced by my commit?

There's currently no way to trigger the macOS CI unfortunately. Based on the CI monolithic-linux.sh invocation it looks like it does run check-lldb. If you download the whole log and search for lldb-api ::/lldb-shell ::/lldb-unit ::, you'll see that they passed

@Michael137
Copy link
Member

Feels a bit silly that we ran all LLVM/Clang/etc. tests just because you amended the LLVM release notes

@vogelsgesang
Copy link
Member Author

There's currently no way to trigger the macOS CI unfortunately.

Is a manual trigger possible for the lldb-aarch64-ubuntu build? Or is it same as for the macOS CI?

Is another round of "merge, pray, revert" our only option here, or is there anything else I can do to merge this with more confidence?

@Michael137
Copy link
Member

There's currently no way to trigger the macOS CI unfortunately.

Is a manual trigger possible for the lldb-aarch64-ubuntu build? Or is it same as for the macOS CI?

Not sure tbh. You can re-run failed runs, but not sure if you can trigger specific test-suites to run. I'd suggest you just merge this since the LLDB linux tests passed

@vogelsgesang vogelsgesang force-pushed the lldb-dap-column-breakpoints-2 branch from 6c2118d to 8e09ad8 Compare February 4, 2025 00:17
@vogelsgesang vogelsgesang merged commit 8e6fa15 into llvm:main Feb 4, 2025
5 of 7 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Feb 4, 2025

LLVM Buildbot has detected a new failure on builder clang-aarch64-quick running on linaro-clang-aarch64-quick while building lldb,llvm at step 5 "ninja check 1".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/65/builds/11710

Here is the relevant piece of the build log for the reference
Step 5 (ninja check 1) failure: stage 1 checked (failure)
******************** TEST 'Clangd Unit Tests :: ./ClangdTests/22/160' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:/home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/tools/clang/tools/extra/clangd/unittests/./ClangdTests-Clangd Unit Tests-1276915-22-160.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=160 GTEST_SHARD_INDEX=22 /home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/tools/clang/tools/extra/clangd/unittests/./ClangdTests
--

Note: This is test shard 23 of 160.
[==========] Running 8 tests from 8 test suites.
[----------] Global test environment set-up.
[----------] 1 test from BackgroundIndexRebuilderTest
[ RUN      ] BackgroundIndexRebuilderTest.IndexingTUs
BackgroundIndex: building version 1 after indexing enough files
BackgroundIndex: serving version 1 (6932 bytes)
BackgroundIndex: building version 2 after indexing enough files
BackgroundIndex: serving version 2 (6964 bytes)
[       OK ] BackgroundIndexRebuilderTest.IndexingTUs (0 ms)
[----------] 1 test from BackgroundIndexRebuilderTest (0 ms total)

[----------] 1 test from CompletionTest
[ RUN      ] CompletionTest.ObjectiveCProtocolFromIndexSpeculation
ASTWorker building file /clangd-test/Foo.m version null with command 
[/clangd-test]
clang -ffreestanding /clangd-test/Foo.m
Driver produced command: cc1 -cc1 -triple aarch64-unknown-linux-gnu -fsyntax-only -disable-free -clear-ast-before-backend -main-file-name Foo.m -mrelocation-model pic -pic-level 2 -pic-is-pie -mframe-pointer=non-leaf -fmath-errno -ffp-contract=on -fno-rounding-math -mconstructor-aliases -ffreestanding -enable-tlsdesc -target-cpu generic -target-feature +v8a -target-feature +fp-armv8 -target-feature +neon -target-abi aapcs -debugger-tuning=gdb -fdebug-compilation-dir=/clangd-test -fcoverage-compilation-dir=/clangd-test -resource-dir lib/clang/21 -internal-isystem lib/clang/21/include -internal-isystem /usr/local/include -internal-externc-isystem /include -internal-externc-isystem /usr/include -ferror-limit 19 -fno-signed-char -fgnuc-version=4.2.1 -fskip-odr-check-in-gmf -fobjc-runtime=gcc -fobjc-encode-cxx-class-template-spec -fobjc-exceptions -no-round-trip-args -target-feature -fmv -faddrsig -x objective-c /clangd-test/Foo.m
Building first preamble for /clangd-test/Foo.m version null
Built preamble of size 712288 for file /clangd-test/Foo.m version null in 10.04 seconds
not idle after addDocument
UNREACHABLE executed at ../llvm/clang-tools-extra/clangd/unittests/SyncAPI.cpp:22!
 #0 0x0000aaaad71ae918 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/tools/clang/tools/extra/clangd/unittests/./ClangdTests+0x101d918)
 #1 0x0000aaaad71ac920 llvm::sys::RunSignalHandlers() (/home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/tools/clang/tools/extra/clangd/unittests/./ClangdTests+0x101b920)
 #2 0x0000aaaad71af064 SignalHandler(int) Signals.cpp:0:0
 #3 0x0000ffffabd1d598 (linux-vdso.so.1+0x598)
 #4 0x0000ffffab87f200 __pthread_kill_implementation ./nptl/./nptl/pthread_kill.c:44:76
 #5 0x0000ffffab83a67c gsignal ./signal/../sysdeps/posix/raise.c:27:6
 #6 0x0000ffffab827130 abort ./stdlib/./stdlib/abort.c:81:7
 #7 0x0000aaaad715c6d8 llvm::RTTIRoot::anchor() (/home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/tools/clang/tools/extra/clangd/unittests/./ClangdTests+0xfcb6d8)
 #8 0x0000aaaad70168a4 clang::clangd::runCodeComplete(clang::clangd::ClangdServer&, llvm::StringRef, clang::clangd::Position, clang::clangd::CodeCompleteOptions) (/home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/tools/clang/tools/extra/clangd/unittests/./ClangdTests+0xe858a4)
 #9 0x0000aaaad6c394a0 clang::clangd::(anonymous namespace)::CompletionTest_ObjectiveCProtocolFromIndexSpeculation_Test::TestBody() CodeCompleteTests.cpp:0:0
#10 0x0000aaaad720a064 testing::Test::Run() (/home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/tools/clang/tools/extra/clangd/unittests/./ClangdTests+0x1079064)
#11 0x0000aaaad720b320 testing::TestInfo::Run() (/home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/tools/clang/tools/extra/clangd/unittests/./ClangdTests+0x107a320)
#12 0x0000aaaad720befc testing::TestSuite::Run() (/home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/tools/clang/tools/extra/clangd/unittests/./ClangdTests+0x107aefc)
#13 0x0000aaaad721bf04 testing::internal::UnitTestImpl::RunAllTests() (/home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/tools/clang/tools/extra/clangd/unittests/./ClangdTests+0x108af04)
#14 0x0000aaaad721b86c testing::UnitTest::Run() (/home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/tools/clang/tools/extra/clangd/unittests/./ClangdTests+0x108a86c)
#15 0x0000aaaad71f6a48 main (/home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/tools/clang/tools/extra/clangd/unittests/./ClangdTests+0x1065a48)
#16 0x0000ffffab8273fc __libc_start_call_main ./csu/../sysdeps/nptl/libc_start_call_main.h:74:3
#17 0x0000ffffab8274cc call_init ./csu/../csu/libc-start.c:128:20
#18 0x0000ffffab8274cc __libc_start_main ./csu/../csu/libc-start.c:379:5
#19 0x0000aaaad69f63b0 _start (/home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/tools/clang/tools/extra/clangd/unittests/./ClangdTests+0x8653b0)

--
...

@vogelsgesang
Copy link
Member Author

On clang-aarch64-quick the test CompletionTest.ObjectiveCProtocolFromIndexSpeculation from the clangd test suite failed - unrelated to this commit

@llvm-ci
Copy link
Collaborator

llvm-ci commented Feb 10, 2025

LLVM Buildbot has detected a new failure on builder openmp-offload-libc-amdgpu-runtime running on omp-vega20-1 while building lldb,llvm at step 7 "Add check check-offload".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/12955

Here is the relevant piece of the build log for the reference
Step 7 (Add check check-offload) failure: test (failure)
******************** TEST 'libomptarget :: amdgcn-amd-amdhsa :: api/omp_host_call.c' FAILED ********************
Exit Code: 2

Command Output (stdout):
--
# RUN: at line 1
/home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./bin/clang -fopenmp    -I /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test -I /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -L /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -L /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./lib -L /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src  -nogpulib -Wl,-rpath,/home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -Wl,-rpath,/home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -Wl,-rpath,/home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./lib  -fopenmp-targets=amdgcn-amd-amdhsa /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test/api/omp_host_call.c -o /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/api/Output/omp_host_call.c.tmp -Xoffload-linker -lc -Xoffload-linker -lm /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./lib/libomptarget.devicertl.a && /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/api/Output/omp_host_call.c.tmp | /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./bin/FileCheck /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test/api/omp_host_call.c
# executed command: /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./bin/clang -fopenmp -I /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test -I /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -L /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -L /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./lib -L /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -nogpulib -Wl,-rpath,/home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -Wl,-rpath,/home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -Wl,-rpath,/home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./lib -fopenmp-targets=amdgcn-amd-amdhsa /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test/api/omp_host_call.c -o /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/api/Output/omp_host_call.c.tmp -Xoffload-linker -lc -Xoffload-linker -lm /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./lib/libomptarget.devicertl.a
# executed command: /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/api/Output/omp_host_call.c.tmp
# note: command had no output on stdout or stderr
# error: command failed with exit status: -11
# executed command: /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./bin/FileCheck /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test/api/omp_host_call.c
# .---command stderr------------
# | FileCheck error: '<stdin>' is empty.
# | FileCheck command line:  /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./bin/FileCheck /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test/api/omp_host_call.c
# `-----------------------------
# error: command failed with exit status: 2

--

********************


Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025
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 llvm#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.
JDevlieghere pushed a commit to swiftlang/llvm-project that referenced this pull request Feb 25, 2025
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 llvm#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.

(cherry picked from commit 8e6fa15)
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request Mar 8, 2025
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 llvm#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.
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request Mar 20, 2025
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 llvm#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.
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request Apr 2, 2025
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 llvm#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.
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request Apr 17, 2025
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 llvm#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.
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request Apr 30, 2025
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 llvm#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.
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request May 15, 2025
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 llvm#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.
@vogelsgesang vogelsgesang deleted the lldb-dap-column-breakpoints-2 branch May 23, 2025 00:05
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request May 29, 2025
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 llvm#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.
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request Jun 13, 2025
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 llvm#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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants