https://github.com/eronnen updated https://github.com/llvm/llvm-project/pull/140470
>From 2ee16e3911bd1c93618f63f5068dcdcaf389e46c Mon Sep 17 00:00:00 2001 From: Ely Ronnen <elyron...@gmail.com> Date: Sun, 18 May 2025 20:56:47 +0200 Subject: [PATCH 1/3] [lldb-dap] Attempt to synchronously wait for breakpoints resolve in lldb-dap tests in order to stabilize the tests --- .../test/tools/lldb-dap/dap_server.py | 6 +++-- .../test/tools/lldb-dap/lldbdap_testcase.py | 26 +++++++++++++++++-- .../breakpoint/TestDAP_setBreakpoints.py | 1 - .../TestDAP_setFunctionBreakpoints.py | 1 - .../tools/lldb-dap/module/TestDAP_module.py | 4 ++- .../TestDAP_terminatedEvent.py | 6 +++-- ...TestGetTargetBreakpointsRequestHandler.cpp | 10 +++++-- 7 files changed, 43 insertions(+), 11 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 9937618a2cf54..4676f03eb6bd5 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 @@ -1188,7 +1188,7 @@ def request_locations(self, locationReference): } return self.send_recv(command_dict) - def request_testGetTargetBreakpoints(self): + def request_testGetTargetBreakpoints(self, only_resolved=False): """A request packet used in the LLDB test suite to get all currently set breakpoint infos for all breakpoints currently set in the target. @@ -1196,7 +1196,9 @@ def request_testGetTargetBreakpoints(self): command_dict = { "command": "_testGetTargetBreakpoints", "type": "request", - "arguments": {}, + "arguments": { + "onlyResolved": only_resolved, + }, } return self.send_recv(command_dict) 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 4028ae4a2525f..67774be7af750 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 @@ -48,7 +48,7 @@ def build_and_create_debug_adapter_for_attach(self): self.build_and_create_debug_adapter(dictionary={"EXE": unique_name}) return self.getBuildArtifact(unique_name) - def set_source_breakpoints(self, source_path, lines, data=None): + def set_source_breakpoints(self, source_path, lines, data=None, wait_for_resolve=True): """Sets source breakpoints and returns an array of strings containing the breakpoint IDs ("1", "2") for each breakpoint that was set. Parameter data is array of data objects for breakpoints. @@ -62,9 +62,11 @@ def set_source_breakpoints(self, source_path, lines, data=None): breakpoint_ids = [] for breakpoint in breakpoints: breakpoint_ids.append("%i" % (breakpoint["id"])) + if wait_for_resolve: + self.wait_for_breakpoints_to_resolve(breakpoint_ids, timeout=10) return breakpoint_ids - def set_function_breakpoints(self, functions, condition=None, hitCondition=None): + def set_function_breakpoints(self, functions, condition=None, hitCondition=None, wait_for_resolve=True): """Sets breakpoints by function name given an array of function names and returns an array of strings containing the breakpoint IDs ("1", "2") for each breakpoint that was set. @@ -78,7 +80,27 @@ def set_function_breakpoints(self, functions, condition=None, hitCondition=None) breakpoint_ids = [] for breakpoint in breakpoints: breakpoint_ids.append("%i" % (breakpoint["id"])) + if wait_for_resolve: + self.wait_for_breakpoints_to_resolve(breakpoint_ids, timeout=10) return breakpoint_ids + + def wait_for_breakpoints_to_resolve(self, breakpoint_ids: list[str], timeout: Optional[float] = None): + unresolved_breakpoints = set(breakpoint_ids) + + # Check already resolved breakpoints + resolved_breakpoints = self.dap_server.request_testGetTargetBreakpoints(only_resolved=True)["body"]["breakpoints"] + for resolved_breakpoint in resolved_breakpoints: + unresolved_breakpoints.discard(str(resolved_breakpoint["id"])) + + while len(unresolved_breakpoints) > 0: + breakpoint_event = self.dap_server.wait_for_event("breakpoint", timeout=timeout) + if breakpoint_event is None: + break + + if breakpoint_event["body"]["reason"] in ["changed", "new"]: + unresolved_breakpoints.discard(str(breakpoint_event["body"]["breakpoint"]["id"])) + + self.assertEqual(len(unresolved_breakpoints), 0, f"Expected to resolve all breakpoints. Unresolved breakpoint ids: {unresolved_breakpoints}") def waitUntil(self, condition_callback): for _ in range(20): 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 aae1251b17c93..26df2573555df 100644 --- a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py +++ b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py @@ -12,7 +12,6 @@ import os -@skip("Temporarily disable the breakpoint tests") class TestDAP_setBreakpoints(lldbdap_testcase.DAPTestCaseBase): def setUp(self): lldbdap_testcase.DAPTestCaseBase.setUp(self) diff --git a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setFunctionBreakpoints.py b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setFunctionBreakpoints.py index baaca4d974d5d..946595f639edc 100644 --- a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setFunctionBreakpoints.py +++ b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setFunctionBreakpoints.py @@ -10,7 +10,6 @@ import lldbdap_testcase -@skip("Temporarily disable the breakpoint tests") class TestDAP_setFunctionBreakpoints(lldbdap_testcase.DAPTestCaseBase): @skipIfWindows def test_set_and_clear(self): diff --git a/lldb/test/API/tools/lldb-dap/module/TestDAP_module.py b/lldb/test/API/tools/lldb-dap/module/TestDAP_module.py index 3fc0f752ee39e..2cea2a94adbbd 100644 --- a/lldb/test/API/tools/lldb-dap/module/TestDAP_module.py +++ b/lldb/test/API/tools/lldb-dap/module/TestDAP_module.py @@ -16,7 +16,9 @@ def run_test(self, symbol_basename, expect_debug_info_size): program = self.getBuildArtifact(program_basename) self.build_and_launch(program) functions = ["foo"] - breakpoint_ids = self.set_function_breakpoints(functions) + + # This breakpoint will be resolved only when the libfoo module is loaded + breakpoint_ids = self.set_function_breakpoints(functions, wait_for_resolve=False) self.assertEqual(len(breakpoint_ids), len(functions), "expect one breakpoint") self.continue_to_breakpoints(breakpoint_ids) active_modules = self.dap_server.get_modules() diff --git a/lldb/test/API/tools/lldb-dap/terminated-event/TestDAP_terminatedEvent.py b/lldb/test/API/tools/lldb-dap/terminated-event/TestDAP_terminatedEvent.py index b0abe2a38dac4..a93a4a2fa77cb 100644 --- a/lldb/test/API/tools/lldb-dap/terminated-event/TestDAP_terminatedEvent.py +++ b/lldb/test/API/tools/lldb-dap/terminated-event/TestDAP_terminatedEvent.py @@ -35,10 +35,12 @@ def test_terminated_event(self): self.build_and_launch(program) # Set breakpoints functions = ["foo"] - breakpoint_ids = self.set_function_breakpoints(functions) + + # This breakpoint will be resolved only when the libfoo module is loaded + breakpoint_ids = self.set_function_breakpoints(functions, wait_for_resolve=False) self.assertEqual(len(breakpoint_ids), len(functions), "expect one breakpoint") main_bp_line = line_number("main.cpp", "// main breakpoint 1") - breakpoint_ids.append(self.set_source_breakpoints("main.cpp", [main_bp_line])) + breakpoint_ids.append(self.set_source_breakpoints("main.cpp", [main_bp_line], wait_for_resolve=False)) self.continue_to_breakpoints(breakpoint_ids) self.continue_to_exit() diff --git a/lldb/tools/lldb-dap/Handler/TestGetTargetBreakpointsRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/TestGetTargetBreakpointsRequestHandler.cpp index 5f4f016f6a1ef..129eb31b8356b 100644 --- a/lldb/tools/lldb-dap/Handler/TestGetTargetBreakpointsRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/TestGetTargetBreakpointsRequestHandler.cpp @@ -15,12 +15,18 @@ namespace lldb_dap { void TestGetTargetBreakpointsRequestHandler::operator()( const llvm::json::Object &request) const { + const auto *arguments = request.getObject("arguments"); + bool only_resolved = GetBoolean(arguments, "onlyResolved").value_or(false); + llvm::json::Object response; FillResponse(request, response); llvm::json::Array response_breakpoints; for (uint32_t i = 0; dap.target.GetBreakpointAtIndex(i).IsValid(); ++i) { - auto bp = Breakpoint(dap, dap.target.GetBreakpointAtIndex(i)); - response_breakpoints.push_back(bp.ToProtocolBreakpoint()); + const auto target_bp = dap.target.GetBreakpointAtIndex(i); + if (!only_resolved || target_bp.GetNumResolvedLocations() > 0) { + auto bp = Breakpoint(dap, target_bp); + response_breakpoints.push_back(bp.ToProtocolBreakpoint()); + } } llvm::json::Object body; body.try_emplace("breakpoints", std::move(response_breakpoints)); >From 3b9659bf3e5f9f0815810bee51a087d52945b2d6 Mon Sep 17 00:00:00 2001 From: Ely Ronnen <elyron...@gmail.com> Date: Sun, 18 May 2025 21:11:25 +0200 Subject: [PATCH 2/3] python format --- .../test/tools/lldb-dap/lldbdap_testcase.py | 34 ++++++++++++++----- .../tools/lldb-dap/module/TestDAP_module.py | 6 ++-- .../TestDAP_terminatedEvent.py | 12 +++++-- 3 files changed, 38 insertions(+), 14 deletions(-) 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 67774be7af750..372da89d670d5 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 @@ -48,7 +48,9 @@ def build_and_create_debug_adapter_for_attach(self): self.build_and_create_debug_adapter(dictionary={"EXE": unique_name}) return self.getBuildArtifact(unique_name) - def set_source_breakpoints(self, source_path, lines, data=None, wait_for_resolve=True): + def set_source_breakpoints( + self, source_path, lines, data=None, wait_for_resolve=True + ): """Sets source breakpoints and returns an array of strings containing the breakpoint IDs ("1", "2") for each breakpoint that was set. Parameter data is array of data objects for breakpoints. @@ -66,7 +68,9 @@ def set_source_breakpoints(self, source_path, lines, data=None, wait_for_resolve self.wait_for_breakpoints_to_resolve(breakpoint_ids, timeout=10) return breakpoint_ids - def set_function_breakpoints(self, functions, condition=None, hitCondition=None, wait_for_resolve=True): + def set_function_breakpoints( + self, functions, condition=None, hitCondition=None, wait_for_resolve=True + ): """Sets breakpoints by function name given an array of function names and returns an array of strings containing the breakpoint IDs ("1", "2") for each breakpoint that was set. @@ -83,24 +87,36 @@ def set_function_breakpoints(self, functions, condition=None, hitCondition=None, if wait_for_resolve: self.wait_for_breakpoints_to_resolve(breakpoint_ids, timeout=10) return breakpoint_ids - - def wait_for_breakpoints_to_resolve(self, breakpoint_ids: list[str], timeout: Optional[float] = None): + + def wait_for_breakpoints_to_resolve( + self, breakpoint_ids: list[str], timeout: Optional[float] = None + ): unresolved_breakpoints = set(breakpoint_ids) - + # Check already resolved breakpoints - resolved_breakpoints = self.dap_server.request_testGetTargetBreakpoints(only_resolved=True)["body"]["breakpoints"] + resolved_breakpoints = self.dap_server.request_testGetTargetBreakpoints( + only_resolved=True + )["body"]["breakpoints"] for resolved_breakpoint in resolved_breakpoints: unresolved_breakpoints.discard(str(resolved_breakpoint["id"])) while len(unresolved_breakpoints) > 0: - breakpoint_event = self.dap_server.wait_for_event("breakpoint", timeout=timeout) + breakpoint_event = self.dap_server.wait_for_event( + "breakpoint", timeout=timeout + ) if breakpoint_event is None: break if breakpoint_event["body"]["reason"] in ["changed", "new"]: - unresolved_breakpoints.discard(str(breakpoint_event["body"]["breakpoint"]["id"])) + unresolved_breakpoints.discard( + str(breakpoint_event["body"]["breakpoint"]["id"]) + ) - self.assertEqual(len(unresolved_breakpoints), 0, f"Expected to resolve all breakpoints. Unresolved breakpoint ids: {unresolved_breakpoints}") + self.assertEqual( + len(unresolved_breakpoints), + 0, + f"Expected to resolve all breakpoints. Unresolved breakpoint ids: {unresolved_breakpoints}", + ) def waitUntil(self, condition_callback): for _ in range(20): diff --git a/lldb/test/API/tools/lldb-dap/module/TestDAP_module.py b/lldb/test/API/tools/lldb-dap/module/TestDAP_module.py index 2cea2a94adbbd..4fc221668a8ee 100644 --- a/lldb/test/API/tools/lldb-dap/module/TestDAP_module.py +++ b/lldb/test/API/tools/lldb-dap/module/TestDAP_module.py @@ -16,9 +16,11 @@ def run_test(self, symbol_basename, expect_debug_info_size): program = self.getBuildArtifact(program_basename) self.build_and_launch(program) functions = ["foo"] - + # This breakpoint will be resolved only when the libfoo module is loaded - breakpoint_ids = self.set_function_breakpoints(functions, wait_for_resolve=False) + breakpoint_ids = self.set_function_breakpoints( + functions, wait_for_resolve=False + ) self.assertEqual(len(breakpoint_ids), len(functions), "expect one breakpoint") self.continue_to_breakpoints(breakpoint_ids) active_modules = self.dap_server.get_modules() diff --git a/lldb/test/API/tools/lldb-dap/terminated-event/TestDAP_terminatedEvent.py b/lldb/test/API/tools/lldb-dap/terminated-event/TestDAP_terminatedEvent.py index a93a4a2fa77cb..7de85bd1589cd 100644 --- a/lldb/test/API/tools/lldb-dap/terminated-event/TestDAP_terminatedEvent.py +++ b/lldb/test/API/tools/lldb-dap/terminated-event/TestDAP_terminatedEvent.py @@ -35,12 +35,18 @@ def test_terminated_event(self): self.build_and_launch(program) # Set breakpoints functions = ["foo"] - + # This breakpoint will be resolved only when the libfoo module is loaded - breakpoint_ids = self.set_function_breakpoints(functions, wait_for_resolve=False) + breakpoint_ids = self.set_function_breakpoints( + functions, wait_for_resolve=False + ) self.assertEqual(len(breakpoint_ids), len(functions), "expect one breakpoint") main_bp_line = line_number("main.cpp", "// main breakpoint 1") - breakpoint_ids.append(self.set_source_breakpoints("main.cpp", [main_bp_line], wait_for_resolve=False)) + breakpoint_ids.append( + self.set_source_breakpoints( + "main.cpp", [main_bp_line], wait_for_resolve=False + ) + ) self.continue_to_breakpoints(breakpoint_ids) self.continue_to_exit() >From bee389042d88d4fdde5ec39be5ec354dcdd2a4cc Mon Sep 17 00:00:00 2001 From: Ely Ronnen <elyron...@gmail.com> Date: Wed, 21 May 2025 07:52:49 +0200 Subject: [PATCH 3/3] keep track of verified breakpoints in tests --- .../test/tools/lldb-dap/dap_server.py | 39 ++++++++++++++++--- .../test/tools/lldb-dap/lldbdap_testcase.py | 26 ++----------- ...TestGetTargetBreakpointsRequestHandler.cpp | 10 +---- 3 files changed, 40 insertions(+), 35 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 4676f03eb6bd5..06827ee7a82e0 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 @@ -136,6 +136,7 @@ def __init__( self.initialized = False self.frame_scopes = {} self.init_commands = init_commands + self.resolved_breakpoints = set([]) @classmethod def encode_content(cls, s: str) -> bytes: @@ -279,6 +280,14 @@ def _process_continued(self, all_threads_continued: bool): if all_threads_continued: self.thread_stop_reasons = {} + def _update_verified_breakpoints(self, breakpoints): + for breakpoint in breakpoints: + if "verified" in breakpoint: + if breakpoint["verified"]: + self.resolved_breakpoints.add(str(breakpoint["id"])) + else: + self.resolved_breakpoints.discard(str(breakpoint["id"])) + def send_packet(self, command_dict: Request, set_sequence=True): """Take the "command_dict" python dictionary and encode it as a JSON string and send the contents as a packet to the VSCode debug @@ -422,8 +431,27 @@ def wait_for_breakpoint_events(self, timeout: Optional[float] = None): if not event: break breakpoint_events.append(event) + + self._update_verified_breakpoints( + [event["body"]["breakpoint"] for event in breakpoint_events] + ) return breakpoint_events + def wait_for_breakpoints_to_be_verified( + self, breakpoint_ids: list[str], timeout: Optional[float] = None + ): + """Wait for all breakpoints to be verified. Return all unverified breakpoints.""" + unresolved_breakpoints = set(breakpoint_ids) + unresolved_breakpoints -= self.resolved_breakpoints + while len(unresolved_breakpoints) > 0: + breakpoint_event = self.wait_for_event("breakpoint", timeout=timeout) + if breakpoint_event is None: + break + + self._update_verified_breakpoints([breakpoint_event["body"]["breakpoint"]]) + unresolved_breakpoints -= self.resolved_breakpoints + return unresolved_breakpoints + def wait_for_exited(self, timeout: Optional[float] = None): event_dict = self.wait_for_event("exited", timeout=timeout) if event_dict is None: @@ -985,7 +1013,10 @@ def request_setBreakpoints(self, file_path, line_array, data=None): "type": "request", "arguments": args_dict, } - return self.send_recv(command_dict) + response = self.send_recv(command_dict) + breakpoints = response["body"]["breakpoints"] + self._update_verified_breakpoints(breakpoints) + return response def request_setExceptionBreakpoints(self, filters): args_dict = {"filters": filters} @@ -1188,7 +1219,7 @@ def request_locations(self, locationReference): } return self.send_recv(command_dict) - def request_testGetTargetBreakpoints(self, only_resolved=False): + def request_testGetTargetBreakpoints(self): """A request packet used in the LLDB test suite to get all currently set breakpoint infos for all breakpoints currently set in the target. @@ -1196,9 +1227,7 @@ def request_testGetTargetBreakpoints(self, only_resolved=False): command_dict = { "command": "_testGetTargetBreakpoints", "type": "request", - "arguments": { - "onlyResolved": only_resolved, - }, + "arguments": {}, } return self.send_recv(command_dict) 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 372da89d670d5..6ada319967079 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 @@ -89,29 +89,11 @@ def set_function_breakpoints( return breakpoint_ids def wait_for_breakpoints_to_resolve( - self, breakpoint_ids: list[str], timeout: Optional[float] = None + self, breakpoint_ids: list[str], timeout: Optional[float] = DEFAULT_TIMEOUT ): - unresolved_breakpoints = set(breakpoint_ids) - - # Check already resolved breakpoints - resolved_breakpoints = self.dap_server.request_testGetTargetBreakpoints( - only_resolved=True - )["body"]["breakpoints"] - for resolved_breakpoint in resolved_breakpoints: - unresolved_breakpoints.discard(str(resolved_breakpoint["id"])) - - while len(unresolved_breakpoints) > 0: - breakpoint_event = self.dap_server.wait_for_event( - "breakpoint", timeout=timeout - ) - if breakpoint_event is None: - break - - if breakpoint_event["body"]["reason"] in ["changed", "new"]: - unresolved_breakpoints.discard( - str(breakpoint_event["body"]["breakpoint"]["id"]) - ) - + unresolved_breakpoints = self.dap_server.wait_for_breakpoints_to_be_verified( + breakpoint_ids, timeout + ) self.assertEqual( len(unresolved_breakpoints), 0, diff --git a/lldb/tools/lldb-dap/Handler/TestGetTargetBreakpointsRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/TestGetTargetBreakpointsRequestHandler.cpp index 129eb31b8356b..5f4f016f6a1ef 100644 --- a/lldb/tools/lldb-dap/Handler/TestGetTargetBreakpointsRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/TestGetTargetBreakpointsRequestHandler.cpp @@ -15,18 +15,12 @@ namespace lldb_dap { void TestGetTargetBreakpointsRequestHandler::operator()( const llvm::json::Object &request) const { - const auto *arguments = request.getObject("arguments"); - bool only_resolved = GetBoolean(arguments, "onlyResolved").value_or(false); - llvm::json::Object response; FillResponse(request, response); llvm::json::Array response_breakpoints; for (uint32_t i = 0; dap.target.GetBreakpointAtIndex(i).IsValid(); ++i) { - const auto target_bp = dap.target.GetBreakpointAtIndex(i); - if (!only_resolved || target_bp.GetNumResolvedLocations() > 0) { - auto bp = Breakpoint(dap, target_bp); - response_breakpoints.push_back(bp.ToProtocolBreakpoint()); - } + auto bp = Breakpoint(dap, dap.target.GetBreakpointAtIndex(i)); + response_breakpoints.push_back(bp.ToProtocolBreakpoint()); } llvm::json::Object body; body.try_emplace("breakpoints", std::move(response_breakpoints)); _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits