From e4156c5fbeb6829081693576552a8c7d4db56826 Mon Sep 17 00:00:00 2001 From: royitaqi Date: Wed, 16 Jul 2025 07:57:13 -0700 Subject: [PATCH 01/18] [lldb] Fix a crash in lldb-server during RemoveSoftwareBreakpoint() (#148738) # Lldb-server crash We have seen stacks like the following in lldb-server core dumps: ``` [ "__GI___pthread_kill at pthread_kill.c:46", "__GI_raise at raise.c:26", "__GI_abort at abort.c:100", "__assert_fail_base at assert.c:92", "__GI___assert_fail at assert.c:101", "lldb_private::NativeProcessProtocol::RemoveSoftwareBreakpoint(unsigned long) at /redacted/lldb-server:0" ] ``` # Hypothesis of root cause In `NativeProcessProtocol::RemoveSoftwareBreakpoint()` ([code](https://github.com/llvm/llvm-project/blob/19b2dd9d798c124406b0124a1b8debb711675281/lldb/source/Host/common/NativeProcessProtocol.cpp#L359-L423)), a `ref_count` is asserted and reduced. If it becomes zero, the code first go through a series of memory reads and writes to remove the breakpoint trap opcode and to restore the original process code, then, if everything goes fine, removes the entry from the map `m_software_breakpoints` at the end of the function. However, if any of the validations for the above reads and writes goes wrong, the code returns an error early, skipping the removal of the entry. This leaves the entry behind with a `ref_count` of zero. The next call to `NativeProcessProtocol::RemoveSoftwareBreakpoint()` for the same breakpoint[*] would violate the assertion about `ref_count > 0` ([here](https://github.com/llvm/llvm-project/blob/19b2dd9d798c124406b0124a1b8debb711675281/lldb/source/Host/common/NativeProcessProtocol.cpp#L365)), which would cause a crash. [*] We haven't found a *regular* way to repro such a next call in lldb or lldb-dap. This is because both of them remove the breakpoint from their internal list when they get any response from the lldb-server (OK or error). Asking the client to delete the breakpoint a second time doesn't trigger the client to send the `$z` gdb packet to lldb-server. We are able to trigger the crash by sending the `$z` packet directly, see "Manual test" below. # Fix Lift the removal of the map entry to be immediately after the decrement of `ref_count`, before the early returns. This ensures that the asserted case will never happen. The validation errors can still happen, and whether they happen or not, the breakpoint has been removed from the perspective of the lldb-server (same as that of lldb and lldb-dap). # Manual test & unit test See PR. (cherry picked from commit a13712ed88cf6fc37d3789d4c3b54ffdd1a05a1d) --- .../Host/common/NativeProcessProtocol.cpp | 18 ++-- .../Host/NativeProcessProtocolTest.cpp | 93 ++++++++++++++++++- 2 files changed, 104 insertions(+), 7 deletions(-) diff --git a/lldb/source/Host/common/NativeProcessProtocol.cpp b/lldb/source/Host/common/NativeProcessProtocol.cpp index 405acbb5662d6..196f54b93538d 100644 --- a/lldb/source/Host/common/NativeProcessProtocol.cpp +++ b/lldb/source/Host/common/NativeProcessProtocol.cpp @@ -366,12 +366,19 @@ Status NativeProcessProtocol::RemoveSoftwareBreakpoint(lldb::addr_t addr) { if (--it->second.ref_count > 0) return Status(); + // Remove the entry from m_software_breakpoints rightaway, so that we don't + // leave behind an entry with ref_count == 0 in case one of the following + // conditions returns an error. The breakpoint is moved so that it can be + // accessed below. + SoftwareBreakpoint bkpt = std::move(it->second); + m_software_breakpoints.erase(it); + // This is the last reference. Let's remove the breakpoint. Status error; // Clear a software breakpoint instruction - llvm::SmallVector curr_break_op( - it->second.breakpoint_opcodes.size(), 0); + llvm::SmallVector curr_break_op(bkpt.breakpoint_opcodes.size(), + 0); // Read the breakpoint opcode size_t bytes_read = 0; @@ -382,10 +389,10 @@ Status NativeProcessProtocol::RemoveSoftwareBreakpoint(lldb::addr_t addr) { "addr=0x%" PRIx64 ": tried to read %zu bytes but only read %zu", addr, curr_break_op.size(), bytes_read); } - const auto &saved = it->second.saved_opcodes; + const auto &saved = bkpt.saved_opcodes; // Make sure the breakpoint opcode exists at this address - if (llvm::ArrayRef(curr_break_op) != it->second.breakpoint_opcodes) { - if (curr_break_op != it->second.saved_opcodes) + if (llvm::ArrayRef(curr_break_op) != bkpt.breakpoint_opcodes) { + if (curr_break_op != bkpt.saved_opcodes) return Status::FromErrorString( "Original breakpoint trap is no longer in memory."); LLDB_LOG(log, @@ -418,7 +425,6 @@ Status NativeProcessProtocol::RemoveSoftwareBreakpoint(lldb::addr_t addr) { llvm::make_range(saved.begin(), saved.end())); } - m_software_breakpoints.erase(it); return Status(); } diff --git a/lldb/unittests/Host/NativeProcessProtocolTest.cpp b/lldb/unittests/Host/NativeProcessProtocolTest.cpp index a48e67c9213da..91c4fd69d6e54 100644 --- a/lldb/unittests/Host/NativeProcessProtocolTest.cpp +++ b/lldb/unittests/Host/NativeProcessProtocolTest.cpp @@ -73,6 +73,97 @@ TEST(NativeProcessProtocolTest, SetBreakpointFailVerify) { llvm::Failed()); } +TEST(NativeProcessProtocolTest, RemoveSoftwareBreakpoint) { + NiceMock DummyDelegate; + MockProcess Process(DummyDelegate, + ArchSpec("x86_64-pc-linux")); + auto Trap = cantFail(Process.GetSoftwareBreakpointTrapOpcode(1)); + auto Original = std::vector{0xbb}; + + // Set up a breakpoint. + { + InSequence S; + EXPECT_CALL(Process, ReadMemory(0x47, 1)) + .WillOnce(Return(ByMove(Original))); + EXPECT_CALL(Process, WriteMemory(0x47, Trap)).WillOnce(Return(ByMove(1))); + EXPECT_CALL(Process, ReadMemory(0x47, 1)).WillOnce(Return(ByMove(Trap))); + EXPECT_THAT_ERROR(Process.SetBreakpoint(0x47, 0, false).ToError(), + llvm::Succeeded()); + } + + // Remove the breakpoint for the first time. This should remove the breakpoint + // from m_software_breakpoints. + // + // Should succeed. + { + InSequence S; + EXPECT_CALL(Process, ReadMemory(0x47, 1)).WillOnce(Return(ByMove(Trap))); + EXPECT_CALL(Process, WriteMemory(0x47, llvm::ArrayRef(Original))) + .WillOnce(Return(ByMove(1))); + EXPECT_CALL(Process, ReadMemory(0x47, 1)) + .WillOnce(Return(ByMove(Original))); + EXPECT_THAT_ERROR(Process.RemoveBreakpoint(0x47, false).ToError(), + llvm::Succeeded()); + } + + // Remove the breakpoint for the second time. + // + // Should fail. None of the ReadMemory() or WriteMemory() should be called, + // because the function should early return when seeing that the breakpoint + // isn't in m_software_breakpoints. + { + EXPECT_CALL(Process, ReadMemory(_, _)).Times(0); + EXPECT_CALL(Process, WriteMemory(_, _)).Times(0); + EXPECT_THAT_ERROR(Process.RemoveBreakpoint(0x47, false).ToError(), + llvm::Failed()); + } +} + +TEST(NativeProcessProtocolTest, RemoveSoftwareBreakpointMemoryError) { + NiceMock DummyDelegate; + MockProcess Process(DummyDelegate, + ArchSpec("x86_64-pc-linux")); + auto Trap = cantFail(Process.GetSoftwareBreakpointTrapOpcode(1)); + auto Original = std::vector{0xbb}; + auto SomethingElse = std::vector{0xaa}; + + // Set up a breakpoint. + { + InSequence S; + EXPECT_CALL(Process, ReadMemory(0x47, 1)) + .WillOnce(Return(ByMove(Original))); + EXPECT_CALL(Process, WriteMemory(0x47, Trap)).WillOnce(Return(ByMove(1))); + EXPECT_CALL(Process, ReadMemory(0x47, 1)).WillOnce(Return(ByMove(Trap))); + EXPECT_THAT_ERROR(Process.SetBreakpoint(0x47, 0, false).ToError(), + llvm::Succeeded()); + } + + // Remove the breakpoint for the first time, with an unexpected value read by + // the first ReadMemory(). This should cause an early return, with the + // breakpoint removed from m_software_breakpoints. + // + // Should fail. + { + InSequence S; + EXPECT_CALL(Process, ReadMemory(0x47, 1)) + .WillOnce(Return(ByMove(SomethingElse))); + EXPECT_THAT_ERROR(Process.RemoveBreakpoint(0x47, false).ToError(), + llvm::Failed()); + } + + // Remove the breakpoint for the second time. + // + // Should fail. None of the ReadMemory() or WriteMemory() should be called, + // because the function should early return when seeing that the breakpoint + // isn't in m_software_breakpoints. + { + EXPECT_CALL(Process, ReadMemory(_, _)).Times(0); + EXPECT_CALL(Process, WriteMemory(_, _)).Times(0); + EXPECT_THAT_ERROR(Process.RemoveBreakpoint(0x47, false).ToError(), + llvm::Failed()); + } +} + TEST(NativeProcessProtocolTest, ReadMemoryWithoutTrap) { NiceMock DummyDelegate; MockProcess Process(DummyDelegate, @@ -146,4 +237,4 @@ TEST(NativeProcessProtocolTest, ReadCStringFromMemory_CrossPageBoundary) { bytes_read), llvm::HasValue(llvm::StringRef("hello"))); EXPECT_EQ(bytes_read, 6UL); -} \ No newline at end of file +} From af47436df78eb6e031f3074878eefc516716f96b Mon Sep 17 00:00:00 2001 From: Wanyi Date: Fri, 18 Jul 2025 01:14:28 -0400 Subject: [PATCH 02/18] [lldb-dap] Fix type req->arguments == "disconnect" (#149446) This typo was introduced in PR #140331. This branch will never get executed. We also set the `disconnecting = true` in the `DAP::Disconnect()` so I am not sure if we need it in both places. (cherry picked from commit de453e86977adf4f418b003b5c25931b8365c9cc) --- lldb/tools/lldb-dap/DAP.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index fd89f52595ec6..cbd3b14463e25 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -983,7 +983,7 @@ llvm::Error DAP::Loop() { if (const protocol::Request *req = std::get_if(&*next); - req && req->arguments == "disconnect") + req && req->command == "disconnect") disconnecting = true; const std::optional cancel_args = From 96a3aaf2403ebed61fcdb7ba2f398d0fc78ef961 Mon Sep 17 00:00:00 2001 From: Stephen Tozer Date: Tue, 22 Jul 2025 12:43:08 +0100 Subject: [PATCH 03/18] [lldb-dap] Allow returning multiple breakpoints in "stopped" event (#149133) Currently, the "stopped" event returned when a breakpoint is hit will always return only the ID of first breakpoint returned from `GetStopReasonDataAtIndex`. This is slightly different from the behaviour in `lldb`, where multiple breakpoints can exist at a single instruction address and all are returned as part of the stop reason when that address is hit. This patch allows all multiple hit breakpoints to be returned in the "stopped" event, both in the hitBreakpointIds field and in the description, using the same formatting as lldb e.g. "breakpoint 1.1 2.1". I'm not aware of any effect this will have on debugger plugins; as far as I can tell, it makes no difference within the VS Code UI - this just fixes a minor issue encountered while writing an `lldb-dap` backend for Dexter. (cherry picked from commit d54400559bb6181566030d5f99c6716ea2b2f0a9) --- .../test/tools/lldb-dap/lldbdap_testcase.py | 22 ++++++++++++++++++ .../breakpoint/TestDAP_setBreakpoints.py | 23 +++++++++++++++++++ .../API/tools/lldb-dap/breakpoint/main.cpp | 2 +- lldb/tools/lldb-dap/JSONUtils.cpp | 17 +++++++++----- 4 files changed, 57 insertions(+), 7 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 d823126e3e2fd..1567462839748 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 @@ -173,6 +173,28 @@ def verify_breakpoint_hit(self, breakpoint_ids, timeout=DEFAULT_TIMEOUT): return self.assertTrue(False, f"breakpoint not hit, stopped_events={stopped_events}") + def verify_all_breakpoints_hit(self, breakpoint_ids, timeout=DEFAULT_TIMEOUT): + """Wait for the process we are debugging to stop, and verify we hit + all of the breakpoint locations in the "breakpoint_ids" array. + "breakpoint_ids" should be a list of int breakpoint IDs ([1, 2]).""" + stopped_events = self.dap_server.wait_for_stopped(timeout) + for stopped_event in stopped_events: + if "body" in stopped_event: + body = stopped_event["body"] + if "reason" not in body: + continue + if ( + body["reason"] != "breakpoint" + and body["reason"] != "instruction breakpoint" + ): + continue + if "hitBreakpointIds" not in body: + continue + hit_bps = body["hitBreakpointIds"] + if all(breakpoint_id in hit_bps for breakpoint_id in breakpoint_ids): + return + self.assertTrue(False, f"breakpoints not hit, stopped_events={stopped_events}") + def verify_stop_exception_info(self, expected_description, timeout=DEFAULT_TIMEOUT): """Wait for the process we are debugging to stop, and verify the stop reason is 'exception' and that the description matches 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 831edd6494c1e..2e860ff5d5e17 100644 --- a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py +++ b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py @@ -398,3 +398,26 @@ def test_column_breakpoints(self): self.stepIn() func_name = self.get_stackFrames()[0]["name"] self.assertEqual(func_name, "a::fourteen(int)") + + @skipIfWindows + def test_hit_multiple_breakpoints(self): + """Test that if we hit multiple breakpoints at the same address, they + all appear in the stop reason.""" + breakpoint_lines = [ + line_number("main.cpp", "// break non-breakpointable line"), + line_number("main.cpp", "// before loop"), + ] + + program = self.getBuildArtifact("a.out") + self.build_and_launch(program) + + # Set a pair of breakpoints that will both resolve to the same address. + breakpoint_ids = [ + int(bp_id) + for bp_id in self.set_source_breakpoints(self.main_path, breakpoint_lines) + ] + self.assertEqual(len(breakpoint_ids), 2, "expected two breakpoints") + self.dap_server.request_continue() + print(breakpoint_ids) + # Verify we hit both of the breakpoints we just set + self.verify_all_breakpoints_hit(breakpoint_ids) diff --git a/lldb/test/API/tools/lldb-dap/breakpoint/main.cpp b/lldb/test/API/tools/lldb-dap/breakpoint/main.cpp index a84546a95af15..2206b07f19494 100644 --- a/lldb/test/API/tools/lldb-dap/breakpoint/main.cpp +++ b/lldb/test/API/tools/lldb-dap/breakpoint/main.cpp @@ -33,7 +33,7 @@ int main(int argc, char const *argv[]) { if (foo == nullptr) { fprintf(stderr, "%s\n", dlerror()); exit(2); - } + } // break non-breakpointable line foo(12); // before loop for (int i = 0; i < 10; ++i) { diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp index 41ca29a405ac9..f42c50236f19e 100644 --- a/lldb/tools/lldb-dap/JSONUtils.cpp +++ b/lldb/tools/lldb-dap/JSONUtils.cpp @@ -654,12 +654,17 @@ llvm::json::Value CreateThreadStopped(DAP &dap, lldb::SBThread &thread, } else { body.try_emplace("reason", "breakpoint"); } - lldb::break_id_t bp_id = thread.GetStopReasonDataAtIndex(0); - lldb::break_id_t bp_loc_id = thread.GetStopReasonDataAtIndex(1); - std::string desc_str = - llvm::formatv("breakpoint {0}.{1}", bp_id, bp_loc_id); - body.try_emplace("hitBreakpointIds", - llvm::json::Array{llvm::json::Value(bp_id)}); + std::vector bp_ids; + std::ostringstream desc_sstream; + desc_sstream << "breakpoint"; + for (size_t idx = 0; idx < thread.GetStopReasonDataCount(); idx += 2) { + lldb::break_id_t bp_id = thread.GetStopReasonDataAtIndex(idx); + lldb::break_id_t bp_loc_id = thread.GetStopReasonDataAtIndex(idx + 1); + bp_ids.push_back(bp_id); + desc_sstream << " " << bp_id << "." << bp_loc_id; + } + std::string desc_str = desc_sstream.str(); + body.try_emplace("hitBreakpointIds", llvm::json::Array(bp_ids)); EmplaceSafeString(body, "description", desc_str); } } break; From f984e487c64e1b70f433a5d2b8aa6c4aaba5cfe7 Mon Sep 17 00:00:00 2001 From: woruyu <99597449+woruyu@users.noreply.github.com> Date: Fri, 1 Aug 2025 20:43:13 +0800 Subject: [PATCH 04/18] [lldb-dap] support moduleId in the stackTrace response (#149774) This PR resolves https://github.com/llvm/llvm-project/issues/149316 --------- Co-authored-by: Ebuka Ezike (cherry picked from commit 0b9470b329103bcdfe3578d99664974d2a53bf8d) --- .../lldb-dap/coreFile/TestDAP_coreFile.py | 3 ++ .../lldb-dap/stackTrace/TestDAP_stackTrace.py | 33 +++++++++++++++++++ lldb/tools/lldb-dap/JSONUtils.cpp | 7 ++++ 3 files changed, 43 insertions(+) diff --git a/lldb/test/API/tools/lldb-dap/coreFile/TestDAP_coreFile.py b/lldb/test/API/tools/lldb-dap/coreFile/TestDAP_coreFile.py index db43dbaf515cf..1143cd93a70b3 100644 --- a/lldb/test/API/tools/lldb-dap/coreFile/TestDAP_coreFile.py +++ b/lldb/test/API/tools/lldb-dap/coreFile/TestDAP_coreFile.py @@ -26,6 +26,7 @@ def test_core_file(self): "column": 0, "id": 524288, "line": 4, + "moduleId": "01DF54A6-045E-657D-3F8F-FB9CE1118789-14F8BD6D", "name": "bar", "source": {"name": "main.c", "path": "/home/labath/test/main.c"}, "instructionPointerReference": "0x40011C", @@ -34,6 +35,7 @@ def test_core_file(self): "column": 0, "id": 524289, "line": 10, + "moduleId": "01DF54A6-045E-657D-3F8F-FB9CE1118789-14F8BD6D", "name": "foo", "source": {"name": "main.c", "path": "/home/labath/test/main.c"}, "instructionPointerReference": "0x400142", @@ -42,6 +44,7 @@ def test_core_file(self): "column": 0, "id": 524290, "line": 16, + "moduleId": "01DF54A6-045E-657D-3F8F-FB9CE1118789-14F8BD6D", "name": "_start", "source": {"name": "main.c", "path": "/home/labath/test/main.c"}, "instructionPointerReference": "0x40015F", diff --git a/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py b/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py index abd469274ffd4..fd2037b5762d1 100644 --- a/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py +++ b/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py @@ -242,3 +242,36 @@ def test_StackFrameFormat(self): frame = self.get_stackFrames(format={"parameters": False, "module": True})[0] self.assertEqual(frame["name"], "a.out recurse") + + @skipIfWindows + def test_stack_frame_module_id(self): + program = self.getBuildArtifact("a.out") + self.build_and_launch(program) + source = "main.c" + lines = [line_number(source, "recurse end")] + breakpoint_ids = self.set_source_breakpoints(source, lines) + self.assertEqual( + len(breakpoint_ids), len(lines), "expect correct number of breakpoints" + ) + + self.continue_to_breakpoints(breakpoint_ids) + + modules = self.dap_server.get_modules() + name_to_id = { + name: info["id"] for name, info in modules.items() if "id" in info + } + + stack_frames = self.get_stackFrames() + for frame in stack_frames: + module_id = frame.get("moduleId") + source_name = frame.get("source", {}).get("name") + if module_id is None or source_name is None: + continue + + if source_name in name_to_id: + expected_id = name_to_id[source_name] + self.assertEqual( + module_id, + expected_id, + f"Expected moduleId '{expected_id}' for {source_name}, got: {module_id}", + ) diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp index f42c50236f19e..4f26599a49bac 100644 --- a/lldb/tools/lldb-dap/JSONUtils.cpp +++ b/lldb/tools/lldb-dap/JSONUtils.cpp @@ -550,6 +550,13 @@ llvm::json::Value CreateStackFrame(DAP &dap, lldb::SBFrame &frame, if (frame.IsArtificial() || frame.IsHidden()) object.try_emplace("presentationHint", "subtle"); + lldb::SBModule module = frame.GetModule(); + if (module.IsValid()) { + std::string uuid = module.GetUUIDString(); + if (!uuid.empty()) + object.try_emplace("moduleId", uuid); + } + return llvm::json::Value(std::move(object)); } From aa1f117e1a24bdc0b8bc3db496bf768c3ecb9742 Mon Sep 17 00:00:00 2001 From: royitaqi Date: Mon, 4 Aug 2025 08:39:06 -0700 Subject: [PATCH 05/18] [vscode-lldb] Fix format (#151829) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generated by running the formatter: ``` royshi-mac-home ~/public_llvm/llvm-project/lldb/tools/lldb-dap % yarn format yarn run v1.22.22 warning package.json: License should be a valid SPDX license expression warning lldb-dap@0.2.15: The engine "vscode" appears to be invalid. $ npx prettier './src-ts/' --write src-ts/debug-adapter-factory.ts 65ms src-ts/debug-configuration-provider.ts 17ms (unchanged) src-ts/debug-session-tracker.ts 11ms (unchanged) src-ts/disposable-context.ts 2ms (unchanged) src-ts/extension.ts 3ms (unchanged) src-ts/lldb-dap-server.ts 7ms src-ts/ui/error-with-notification.ts 4ms (unchanged) src-ts/ui/modules-data-provider.ts 5ms (unchanged) src-ts/ui/show-error-message.ts 6ms (unchanged) src-ts/uri-launch-handler.ts 5ms (unchanged) ✨ Done in 0.99s. ``` (cherry picked from commit cd834449a6d551cace6afad798ffad318f4ff325) --- lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts | 4 +++- lldb/tools/lldb-dap/src-ts/lldb-dap-server.ts | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts b/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts index b5db45b56d6a6..015bcf77ddd29 100644 --- a/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts +++ b/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts @@ -169,7 +169,9 @@ export async function createDebugAdapterExecutable( workspaceFolder: vscode.WorkspaceFolder | undefined, configuration: vscode.DebugConfiguration, ): Promise { - const config = vscode.workspace.workspaceFile ? vscode.workspace.getConfiguration("lldb-dap") : vscode.workspace.getConfiguration("lldb-dap", workspaceFolder); + const config = vscode.workspace.workspaceFile + ? vscode.workspace.getConfiguration("lldb-dap") + : vscode.workspace.getConfiguration("lldb-dap", workspaceFolder); const log_path = config.get("log-path"); let env: { [key: string]: string } = {}; if (log_path) { diff --git a/lldb/tools/lldb-dap/src-ts/lldb-dap-server.ts b/lldb/tools/lldb-dap/src-ts/lldb-dap-server.ts index 79573ec7342b1..03a0fc9d0fef3 100644 --- a/lldb/tools/lldb-dap/src-ts/lldb-dap-server.ts +++ b/lldb/tools/lldb-dap/src-ts/lldb-dap-server.ts @@ -26,7 +26,7 @@ export class LLDBDapServer implements vscode.Disposable { args: string[], options?: child_process.SpawnOptionsWithoutStdio, ): Promise<{ host: string; port: number } | undefined> { - const dapArgs = [...args, "--connection", "listen://localhost:0" ]; + const dapArgs = [...args, "--connection", "listen://localhost:0"]; if (!(await this.shouldContinueStartup(dapPath, dapArgs))) { return undefined; } From 22ea1d59a13a56a36c71eb7f1ef086f141903070 Mon Sep 17 00:00:00 2001 From: award999 Date: Tue, 5 Aug 2025 11:14:55 -0400 Subject: [PATCH 06/18] Logging setup for lldb-dap extension (#146884) - ~Add `winston` dependency (MIT license) to handle logging setup~ - Have an `LogOutputChannel` to log user facing information, errors, warnings - Write a debug session logs under the provided `logUri` to capture further diagnostics when the `lldb-dap.captureSessionLogs` setting is enabled. *Note* the `lldb-dap.log-path` setting takes precedence when set Issue: #146880 --------- Co-authored-by: Jonas Devlieghere (cherry picked from commit ae7be39601496aa8f712672844de82285a227646) --- lldb/tools/lldb-dap/package-lock.json | 4 +- lldb/tools/lldb-dap/package.json | 13 +++- .../lldb-dap/src-ts/debug-adapter-factory.ts | 33 ++++++++- .../src-ts/debug-configuration-provider.ts | 22 +++++- .../lldb-dap/src-ts/debug-session-tracker.ts | 14 +++- lldb/tools/lldb-dap/src-ts/extension.ts | 26 +++++-- lldb/tools/lldb-dap/src-ts/logging.ts | 67 +++++++++++++++++++ 7 files changed, 167 insertions(+), 12 deletions(-) create mode 100644 lldb/tools/lldb-dap/src-ts/logging.ts diff --git a/lldb/tools/lldb-dap/package-lock.json b/lldb/tools/lldb-dap/package-lock.json index af90a9573aee6..1969b196accc6 100644 --- a/lldb/tools/lldb-dap/package-lock.json +++ b/lldb/tools/lldb-dap/package-lock.json @@ -1,12 +1,12 @@ { "name": "lldb-dap", - "version": "0.2.14", + "version": "0.2.15", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "lldb-dap", - "version": "0.2.14", + "version": "0.2.15", "license": "Apache 2.0 License with LLVM exceptions", "devDependencies": { "@types/node": "^18.19.41", diff --git a/lldb/tools/lldb-dap/package.json b/lldb/tools/lldb-dap/package.json index 801abe73edd7d..fa2d4daffaf27 100644 --- a/lldb/tools/lldb-dap/package.json +++ b/lldb/tools/lldb-dap/package.json @@ -81,10 +81,16 @@ "description": "The path to the lldb-dap binary, e.g. /usr/local/bin/lldb-dap" }, "lldb-dap.log-path": { + "scope": "machine-overridable", + "type": "string", + "description": "The log path for lldb-dap (if any)", + "markdownDeprecationMessage": "Use the `#lldb-dap.logFolder#` setting instead" + }, + "lldb-dap.logFolder": { "order": 0, "scope": "machine-overridable", "type": "string", - "description": "The log path for lldb-dap (if any)" + "markdownDescription": "The folder to persist lldb-dap logs. If no value is provided, logs will be persisted in the [Extension Logs Folder](command:workbench.action.openExtensionLogsFolder)." }, "lldb-dap.serverMode": { "order": 0, @@ -110,6 +116,11 @@ "additionalProperties": { "type": "string" } + }, + "lldb-dap.captureSessionLogs": { + "type": "boolean", + "description": "When enabled, LLDB-DAP session logs will be written to the Extension's log folder if the `lldb-dap.log-path` setting is not explicitly set.", + "default": false } } }, diff --git a/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts b/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts index 015bcf77ddd29..6e94400b09155 100644 --- a/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts +++ b/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts @@ -5,6 +5,7 @@ import * as child_process from "child_process"; import * as fs from "node:fs/promises"; import { ConfigureButton, OpenSettingsButton } from "./ui/show-error-message"; import { ErrorWithNotification } from "./ui/error-with-notification"; +import { LogFilePathProvider, LogType } from "./logging"; const exec = util.promisify(child_process.execFile); @@ -160,12 +161,16 @@ async function getDAPArguments( * Creates a new {@link vscode.DebugAdapterExecutable} based on the provided workspace folder and * debug configuration. Assumes that the given debug configuration is for a local launch of lldb-dap. * + * @param logger The {@link vscode.LogOutputChannel} to log setup diagnostics + * @param logFilePath The {@link LogFilePathProvider} for determining where to put session logs * @param workspaceFolder The {@link vscode.WorkspaceFolder} that the debug session will be launched within * @param configuration The {@link vscode.DebugConfiguration} that will be launched * @throws An {@link ErrorWithNotification} if something went wrong * @returns The {@link vscode.DebugAdapterExecutable} that can be used to launch lldb-dap */ export async function createDebugAdapterExecutable( + logger: vscode.LogOutputChannel, + logFilePath: LogFilePathProvider, workspaceFolder: vscode.WorkspaceFolder | undefined, configuration: vscode.DebugConfiguration, ): Promise { @@ -176,6 +181,10 @@ export async function createDebugAdapterExecutable( let env: { [key: string]: string } = {}; if (log_path) { env["LLDBDAP_LOG"] = log_path; + } else if ( + vscode.workspace.getConfiguration("lldb-dap").get("captureSessionLogs", false) + ) { + env["LLDBDAP_LOG"] = logFilePath.get(LogType.DEBUG_SESSION); } const configEnvironment = config.get<{ [key: string]: string }>("environment") || {}; @@ -190,6 +199,11 @@ export async function createDebugAdapterExecutable( }; const dbgArgs = await getDAPArguments(workspaceFolder, configuration); + logger.info(`lldb-dap path: ${dapPath}`); + logger.info(`lldb-dap args: ${dbgArgs}`); + logger.info(`cwd: ${dbgOptions.cwd}`); + logger.info(`env: ${JSON.stringify(dbgOptions.env)}`); + return new vscode.DebugAdapterExecutable(dapPath, dbgArgs, dbgOptions); } @@ -200,18 +214,33 @@ export async function createDebugAdapterExecutable( export class LLDBDapDescriptorFactory implements vscode.DebugAdapterDescriptorFactory { + constructor( + private readonly logger: vscode.LogOutputChannel, + private logFilePath: LogFilePathProvider, + ) {} + async createDebugAdapterDescriptor( session: vscode.DebugSession, executable: vscode.DebugAdapterExecutable | undefined, ): Promise { + this.logger.info(`Creating debug adapter for session "${session.name}"`); + this.logger.info( + `Session "${session.name}" debug configuration:\n` + + JSON.stringify(session.configuration, undefined, 2), + ); if (executable) { - throw new Error( + const error = new Error( "Setting the debug adapter executable in the package.json is not supported.", ); + this.logger.error(error); + throw error; } // Use a server connection if the debugAdapterPort is provided if (session.configuration.debugAdapterPort) { + this.logger.info( + `Spawning debug adapter server on port ${session.configuration.debugAdapterPort}`, + ); return new vscode.DebugAdapterServer( session.configuration.debugAdapterPort, session.configuration.debugAdapterHostname, @@ -219,6 +248,8 @@ export class LLDBDapDescriptorFactory } return createDebugAdapterExecutable( + this.logger, + this.logFilePath, session.workspaceFolder, session.configuration, ); diff --git a/lldb/tools/lldb-dap/src-ts/debug-configuration-provider.ts b/lldb/tools/lldb-dap/src-ts/debug-configuration-provider.ts index 316ffaf47c3d2..8c04ec2bdc9d3 100644 --- a/lldb/tools/lldb-dap/src-ts/debug-configuration-provider.ts +++ b/lldb/tools/lldb-dap/src-ts/debug-configuration-provider.ts @@ -5,6 +5,7 @@ import { LLDBDapServer } from "./lldb-dap-server"; import { createDebugAdapterExecutable } from "./debug-adapter-factory"; import { ConfigureButton, showErrorMessage } from "./ui/show-error-message"; import { ErrorWithNotification } from "./ui/error-with-notification"; +import { LogFilePathProvider } from "./logging"; const exec = util.promisify(child_process.execFile); @@ -71,13 +72,24 @@ const configurations: Record = { export class LLDBDapConfigurationProvider implements vscode.DebugConfigurationProvider { - constructor(private readonly server: LLDBDapServer) {} + constructor( + private readonly server: LLDBDapServer, + private readonly logger: vscode.LogOutputChannel, + private readonly logFilePath: LogFilePathProvider, + ) {} async resolveDebugConfiguration( folder: vscode.WorkspaceFolder | undefined, debugConfiguration: vscode.DebugConfiguration, token?: vscode.CancellationToken, ): Promise { + this.logger.info( + `Resolving debug configuration for "${debugConfiguration.name}"`, + ); + this.logger.debug( + "Initial debug configuration:\n" + + JSON.stringify(debugConfiguration, undefined, 2), + ); let config = vscode.workspace.getConfiguration("lldb-dap"); for (const [key, cfg] of Object.entries(configurations)) { if (Reflect.has(debugConfiguration, key)) { @@ -152,6 +164,8 @@ export class LLDBDapConfigurationProvider // Always try to create the debug adapter executable as this will show the user errors // if there are any. const executable = await createDebugAdapterExecutable( + this.logger, + this.logFilePath, folder, debugConfiguration, ); @@ -184,8 +198,14 @@ export class LLDBDapConfigurationProvider } } + this.logger.info( + "Resolved debug configuration:\n" + + JSON.stringify(debugConfiguration, undefined, 2), + ); + return debugConfiguration; } catch (error) { + this.logger.error(error as Error); // Show a better error message to the user if possible if (!(error instanceof ErrorWithNotification)) { throw error; diff --git a/lldb/tools/lldb-dap/src-ts/debug-session-tracker.ts b/lldb/tools/lldb-dap/src-ts/debug-session-tracker.ts index 50db1e1c3a7b0..7d7f73dbff92d 100644 --- a/lldb/tools/lldb-dap/src-ts/debug-session-tracker.ts +++ b/lldb/tools/lldb-dap/src-ts/debug-session-tracker.ts @@ -5,6 +5,7 @@ import * as vscode from "vscode"; // prettier-ignore interface EventMap { "module": DebugProtocol.ModuleEvent; + "exited": DebugProtocol.ExitedEvent; } /** A type assertion to check if a ProtocolMessage is an event or if it is a specific event. */ @@ -47,7 +48,7 @@ export class DebugSessionTracker onDidChangeModules: vscode.Event = this.modulesChanged.event; - constructor() { + constructor(private logger: vscode.LogOutputChannel) { this.onDidChangeModules(this.moduleChangedListener, this); vscode.debug.onDidChangeActiveDebugSession((session) => this.modulesChanged.fire(session), @@ -62,8 +63,12 @@ export class DebugSessionTracker createDebugAdapterTracker( session: vscode.DebugSession, ): vscode.ProviderResult { + this.logger.info(`Starting debug session "${session.name}"`); + let stopping = false; return { + onError: (error) => !stopping && this.logger.error(error), // Can throw benign read errors when shutting down. onDidSendMessage: (message) => this.onDidSendMessage(session, message), + onWillStopSession: () => (stopping = true), onExit: () => this.onExit(session), }; } @@ -134,6 +139,13 @@ export class DebugSessionTracker } this.modules.set(session, modules); this.modulesChanged.fire(session); + } else if (isEvent(message, "exited")) { + // The vscode.DebugAdapterTracker#onExit event is sometimes called with + // exitCode = undefined but the exit event from LLDB-DAP always has the "exitCode" + const { exitCode } = message.body; + this.logger.info( + `Session "${session.name}" exited with code ${exitCode}`, + ); } } } diff --git a/lldb/tools/lldb-dap/src-ts/extension.ts b/lldb/tools/lldb-dap/src-ts/extension.ts index c8e5146e29cea..4b7a35e6944c6 100644 --- a/lldb/tools/lldb-dap/src-ts/extension.ts +++ b/lldb/tools/lldb-dap/src-ts/extension.ts @@ -1,3 +1,4 @@ +import * as path from "path"; import * as vscode from "vscode"; import { LLDBDapDescriptorFactory } from "./debug-adapter-factory"; @@ -10,28 +11,35 @@ import { ModulesDataProvider, ModuleProperty, } from "./ui/modules-data-provider"; +import { LogFilePathProvider } from "./logging"; /** * This class represents the extension and manages its life cycle. Other extensions * using it as as library should use this class as the main entry point. */ export class LLDBDapExtension extends DisposableContext { - constructor() { + constructor( + logger: vscode.LogOutputChannel, + logFilePath: LogFilePathProvider, + outputChannel: vscode.OutputChannel, + ) { super(); const lldbDapServer = new LLDBDapServer(); - const sessionTracker = new DebugSessionTracker(); + const sessionTracker = new DebugSessionTracker(logger); this.pushSubscription( + logger, + outputChannel, lldbDapServer, sessionTracker, vscode.debug.registerDebugConfigurationProvider( "lldb-dap", - new LLDBDapConfigurationProvider(lldbDapServer), + new LLDBDapConfigurationProvider(lldbDapServer, logger, logFilePath), ), vscode.debug.registerDebugAdapterDescriptorFactory( "lldb-dap", - new LLDBDapDescriptorFactory(), + new LLDBDapDescriptorFactory(logger, logFilePath), ), vscode.debug.registerDebugAdapterTrackerFactory( "lldb-dap", @@ -54,6 +62,12 @@ export class LLDBDapExtension extends DisposableContext { /** * This is the entry point when initialized by VS Code. */ -export function activate(context: vscode.ExtensionContext) { - context.subscriptions.push(new LLDBDapExtension()); +export async function activate(context: vscode.ExtensionContext) { + const outputChannel = vscode.window.createOutputChannel("LLDB-DAP", { log: true }); + outputChannel.info("LLDB-DAP extension activating..."); + const logFilePath = new LogFilePathProvider(context, outputChannel); + context.subscriptions.push( + new LLDBDapExtension(outputChannel, logFilePath, outputChannel), + ); + outputChannel.info("LLDB-DAP extension activated"); } diff --git a/lldb/tools/lldb-dap/src-ts/logging.ts b/lldb/tools/lldb-dap/src-ts/logging.ts new file mode 100644 index 0000000000000..3b1c3c37ce1ce --- /dev/null +++ b/lldb/tools/lldb-dap/src-ts/logging.ts @@ -0,0 +1,67 @@ +import * as path from "path"; +import * as vscode from "vscode"; + +/** + * Formats the given date as a string in the form "YYYYMMddTHHMMSS". + * + * @param date The date to format as a string. + * @returns The formatted date. + */ +function formatDate(date: Date): string { + const year = date.getFullYear().toString().padStart(4, "0"); + const month = (date.getMonth() + 1).toString().padStart(2, "0"); + const day = date.getDate().toString().padStart(2, "0"); + const hour = date.getHours().toString().padStart(2, "0"); + const minute = date.getMinutes().toString().padStart(2, "0"); + const seconds = date.getSeconds().toString().padStart(2, "0"); + return `${year}${month}${day}T${hour}${minute}${seconds}`; +} + +export enum LogType { + DEBUG_SESSION, +} + +export class LogFilePathProvider { + private logFolder: string = ""; + + constructor( + private context: vscode.ExtensionContext, + private logger: vscode.LogOutputChannel, + ) { + this.updateLogFolder(); + context.subscriptions.push( + vscode.workspace.onDidChangeConfiguration(e => { + if ( + e.affectsConfiguration("lldb-dap.logFolder") + ) { + this.updateLogFolder(); + } + }) + ); + } + + get(type: LogType): string { + const logFolder = this.logFolder || this.context.logUri.fsPath; + switch(type) { + case LogType.DEBUG_SESSION: + return path.join(logFolder, `lldb-dap-session-${formatDate(new Date())}.log`); + break; + } + } + + private updateLogFolder() { + const config = vscode.workspace.getConfiguration("lldb-dap"); + let logFolder = + config.get("logFolder") || this.context.logUri.fsPath; + vscode.workspace.fs + .createDirectory(vscode.Uri.file(logFolder)) + .then(undefined, (error) => { + this.logger.error(`Failed to create log folder ${logFolder}: ${error}`); + logFolder = this.context.logUri.fsPath; + }) + .then(() => { + this.logFolder = logFolder; + this.logger.info(`Persisting lldb-dap logs to ${logFolder}`); + }); + } +} From 0890afc4b5141bc2ac086fcd5a833057bd473c2c Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Tue, 5 Aug 2025 09:15:19 -0700 Subject: [PATCH 07/18] [lldb-dap] Bump the version to 0.2.16 (cherry picked from commit dd0bb2c3a8675ff1e2b9de565a301c0a09d3063b) --- lldb/tools/lldb-dap/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/tools/lldb-dap/package.json b/lldb/tools/lldb-dap/package.json index fa2d4daffaf27..1ce27e7d62fa5 100644 --- a/lldb/tools/lldb-dap/package.json +++ b/lldb/tools/lldb-dap/package.json @@ -1,7 +1,7 @@ { "name": "lldb-dap", "displayName": "LLDB DAP", - "version": "0.2.15", + "version": "0.2.16", "publisher": "llvm-vs-code-extensions", "homepage": "https://lldb.llvm.org", "description": "Debugging with LLDB in Visual Studio Code", From 8a8ead60fc759d870648699eb4167d0c40034e35 Mon Sep 17 00:00:00 2001 From: royitaqi Date: Tue, 5 Aug 2025 10:54:07 -0700 Subject: [PATCH 08/18] [vscode-lldb] Fix `yarn package` (#152002) # Problem `yarn package` cannot be run twice in a row - the second time will error out: > Error: ENOENT: no such file or directory, stat '/llvm-project/lldb/tools/lldb-dap/out/lldb-dap.vsix' This error is also weird, because the file actually exists. See the end of this [full output](https://gist.github.com/royitaqi/f3f4838ed30d7ade846f53f0fb7d68f4). # Fix Delete the `lldb-dap.vsix` file at the start of each run. See consecutive runs [being successful](https://gist.github.com/royitaqi/9609181b4fe6a8a4e71880c36cd0c7c9). (cherry picked from commit 4882874ddc1017f2f1b9b11fb67065440b101701) --- lldb/tools/lldb-dap/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/tools/lldb-dap/package.json b/lldb/tools/lldb-dap/package.json index 1ce27e7d62fa5..d677a81cc7974 100644 --- a/lldb/tools/lldb-dap/package.json +++ b/lldb/tools/lldb-dap/package.json @@ -45,7 +45,7 @@ "vscode:prepublish": "tsc -p ./", "watch": "tsc -watch -p ./", "format": "npx prettier './src-ts/' --write", - "package": "vsce package --out ./out/lldb-dap.vsix", + "package": "rm -rf ./out/lldb-dap.vsix && vsce package --out ./out/lldb-dap.vsix", "publish": "vsce publish", "vscode-uninstall": "code --uninstall-extension llvm-vs-code-extensions.lldb-dap", "vscode-install": "code --install-extension ./out/lldb-dap.vsix" From cdadfab7c449c7bbf428c4e06070026370159188 Mon Sep 17 00:00:00 2001 From: David Spickett Date: Wed, 6 Aug 2025 09:31:09 +0100 Subject: [PATCH 09/18] [lldb] Treat address found via function name as a callable address (#151973) I discovered this building the lldb test suite with `-mthumb` set, so that all test programs are purely Arm Thumb code. When C++ expressions did a function lookup, they took a different path from C programs. That path happened to land on the line that I've changed. Where we try to look something up as a function, but don't then resolve the address as if it's a callable. With Thumb, if you do the non-callable lookup, the bottom bit won't be set. This means when lldb's expression wrapper function branches into the found function, it'll be in Arm mode trying to execute Thumb code. Thumb is the only instance where you'd notice this. Aside from maybe MicroMIPS or MIPS16 perhaps but I expect that there are 0 users of that and lldb. I have added a new test case that will simulate this situation in "normal" Arm builds so that it will get run on Linaro's buildbot. This change also fixes the following existing tests when `-mthumb` is used: ``` lldb-api :: commands/expression/anonymous-struct/TestCallUserAnonTypedef.py lldb-api :: commands/expression/argument_passing_restrictions/TestArgumentPassingRestrictions.py lldb-api :: commands/expression/call-function/TestCallStopAndContinue.py lldb-api :: commands/expression/call-function/TestCallUserDefinedFunction.py lldb-api :: commands/expression/char/TestExprsChar.py lldb-api :: commands/expression/class_template_specialization_empty_pack/TestClassTemplateSpecializationParametersHandling.py lldb-api :: commands/expression/context-object/TestContextObject.py lldb-api :: commands/expression/formatters/TestFormatters.py lldb-api :: commands/expression/import_base_class_when_class_has_derived_member/TestImportBaseClassWhenClassHasDerivedMember.py lldb-api :: commands/expression/inline-namespace/TestInlineNamespace.py lldb-api :: commands/expression/namespace-alias/TestInlineNamespaceAlias.py lldb-api :: commands/expression/no-deadlock/TestExprDoesntBlock.py lldb-api :: commands/expression/pr35310/TestExprsBug35310.py lldb-api :: commands/expression/static-initializers/TestStaticInitializers.py lldb-api :: commands/expression/test/TestExprs.py lldb-api :: commands/expression/timeout/TestCallWithTimeout.py lldb-api :: commands/expression/top-level/TestTopLevelExprs.py lldb-api :: commands/expression/unwind_expression/TestUnwindExpression.py lldb-api :: commands/expression/xvalue/TestXValuePrinting.py lldb-api :: functionalities/thread/main_thread_exit/TestMainThreadExit.py lldb-api :: lang/cpp/call-function/TestCallCPPFunction.py lldb-api :: lang/cpp/chained-calls/TestCppChainedCalls.py lldb-api :: lang/cpp/class-template-parameter-pack/TestClassTemplateParameterPack.py lldb-api :: lang/cpp/constructors/TestCppConstructors.py lldb-api :: lang/cpp/function-qualifiers/TestCppFunctionQualifiers.py lldb-api :: lang/cpp/function-ref-qualifiers/TestCppFunctionRefQualifiers.py lldb-api :: lang/cpp/global_operators/TestCppGlobalOperators.py lldb-api :: lang/cpp/llvm-style/TestLLVMStyle.py lldb-api :: lang/cpp/multiple-inheritance/TestCppMultipleInheritance.py lldb-api :: lang/cpp/namespace/TestNamespace.py lldb-api :: lang/cpp/namespace/TestNamespaceLookup.py lldb-api :: lang/cpp/namespace_conflicts/TestNamespaceConflicts.py lldb-api :: lang/cpp/operators/TestCppOperators.py lldb-api :: lang/cpp/overloaded-functions/TestOverloadedFunctions.py lldb-api :: lang/cpp/rvalue-references/TestRvalueReferences.py lldb-api :: lang/cpp/static_methods/TestCPPStaticMethods.py lldb-api :: lang/cpp/template/TestTemplateArgs.py lldb-api :: python_api/thread/TestThreadAPI.py ``` There are other failures that are due to different problems, and this change does not make those worse. (I have no plans to run the test suite with `-mthumb` regularly, I just did it to test some other refactoring) (cherry picked from commit 4077e66432a1d17543d0cef4b5a9280caff6e974) --- lldb/source/Expression/IRExecutionUnit.cpp | 5 +- .../test/API/arm/thumb-function-addr/Makefile | 3 + .../TestThumbFunctionAddr.py | 67 +++++++++++++++++++ lldb/test/API/arm/thumb-function-addr/main.c | 9 +++ 4 files changed, 82 insertions(+), 2 deletions(-) create mode 100644 lldb/test/API/arm/thumb-function-addr/Makefile create mode 100644 lldb/test/API/arm/thumb-function-addr/TestThumbFunctionAddr.py create mode 100644 lldb/test/API/arm/thumb-function-addr/main.c diff --git a/lldb/source/Expression/IRExecutionUnit.cpp b/lldb/source/Expression/IRExecutionUnit.cpp index 5746bf3eb7cbd..f1958ad947de0 100644 --- a/lldb/source/Expression/IRExecutionUnit.cpp +++ b/lldb/source/Expression/IRExecutionUnit.cpp @@ -721,8 +721,9 @@ class LoadAddressResolver { // If that didn't work, try the function. if (load_address == LLDB_INVALID_ADDRESS && candidate_sc.function) { Address addr = candidate_sc.function->GetAddress(); - load_address = m_target.GetProcessSP() ? addr.GetLoadAddress(&m_target) - : addr.GetFileAddress(); + load_address = m_target.GetProcessSP() + ? addr.GetCallableLoadAddress(&m_target) + : addr.GetFileAddress(); } // We found a load address. diff --git a/lldb/test/API/arm/thumb-function-addr/Makefile b/lldb/test/API/arm/thumb-function-addr/Makefile new file mode 100644 index 0000000000000..10495940055b6 --- /dev/null +++ b/lldb/test/API/arm/thumb-function-addr/Makefile @@ -0,0 +1,3 @@ +C_SOURCES := main.c + +include Makefile.rules diff --git a/lldb/test/API/arm/thumb-function-addr/TestThumbFunctionAddr.py b/lldb/test/API/arm/thumb-function-addr/TestThumbFunctionAddr.py new file mode 100644 index 0000000000000..d08099f6331e5 --- /dev/null +++ b/lldb/test/API/arm/thumb-function-addr/TestThumbFunctionAddr.py @@ -0,0 +1,67 @@ +""" +Test that addresses of functions compiled for Arm Thumb include the Thumb mode +bit (bit 0 of the address) when resolved and used in expressions. +""" + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class TestThumbFunctionAddr(TestBase): + def do_thumb_function_test(self, language): + self.build(dictionary={"CFLAGS_EXTRAS": f"-x {language} -mthumb"}) + + exe = self.getBuildArtifact("a.out") + line = line_number("main.c", "// Set break point at this line.") + self.runCmd("target create %s" % exe) + bpid = lldbutil.run_break_set_by_file_and_line(self, "main.c", line) + + self.runCmd("run") + self.assertIsNotNone( + lldbutil.get_one_thread_stopped_at_breakpoint_id(self.process(), bpid), + "Process is not stopped at breakpoint", + ) + + # The compiler set this, so the mode bit will be included here. + a_function_addr_var = ( + self.thread().GetFrameAtIndex(0).FindVariable("a_function_addr") + ) + self.assertTrue(a_function_addr_var.IsValid()) + a_function_addr = a_function_addr_var.GetValueAsUnsigned() + self.assertTrue(a_function_addr & 1) + + self.expect("p/x a_function_addr", substrs=[f"0x{a_function_addr:08x}"]) + # If lldb did not pay attention to the mode bit this would SIGILL trying + # to execute Thumb encodings in Arm mode. + self.expect("expression -- a_function()", substrs=["= 123"]) + + # We cannot call GetCallableLoadAdress via. the API, so we expect this + # to not have the bit set as it's treating it as a non-function symbol. + found_function = self.target().FindFunctions("a_function")[0] + self.assertTrue(found_function.IsValid()) + found_function = found_function.GetFunction() + self.assertTrue(found_function.IsValid()) + found_function_addr = found_function.GetStartAddress() + a_function_load_addr = found_function_addr.GetLoadAddress(self.target()) + self.assertEqual(a_function_load_addr, a_function_addr & ~1) + + # image lookup should not include the mode bit. + a_function_file_addr = found_function_addr.GetFileAddress() + self.expect( + "image lookup -n a_function", substrs=[f"0x{a_function_file_addr:08x}"] + ) + + # This test is run for C and C++ because the two will take different paths + # trying to resolve the function's address. + + @skipIf(archs=no_match(["arm$"])) + @skipIf(archs=["arm64"]) + def test_function_addr_c(self): + self.do_thumb_function_test("c") + + @skipIf(archs=no_match(["arm$"])) + @skipIf(archs=["arm64"]) + def test_function_addr_cpp(self): + self.do_thumb_function_test("c++") diff --git a/lldb/test/API/arm/thumb-function-addr/main.c b/lldb/test/API/arm/thumb-function-addr/main.c new file mode 100644 index 0000000000000..f3e01b78f575b --- /dev/null +++ b/lldb/test/API/arm/thumb-function-addr/main.c @@ -0,0 +1,9 @@ +#include + +int a_function() { return 123; } + +int main() { + const uintptr_t a_function_addr = (uintptr_t)a_function; + // Set break point at this line. + return a_function(); +} From b3e2b8555044b5de05f03bd221e626944b2fa659 Mon Sep 17 00:00:00 2001 From: royitaqi Date: Wed, 6 Aug 2025 10:24:09 -0700 Subject: [PATCH 10/18] [vscode-lldb] Fix race condition when changing lldb-dap arguments (#151828) # Problem When the user changes lldb-dap's arguments (e.g. path), there is a race condition, where the new lldb-dap process could be started first and have set the extension's `serverProcess` and `serverInfo` according to the new process, while the old lldb-dap process exits later and wipes out these two fields. Consequences: 1. This causes `getServerProcess()` to return `undefined` when it should return the new process. 2. This also causes wrong behavior when starting the next debug session that a new lldb-dap process will be started and the old not reused nor killed. # Fix When wiping the two fields, check if `serverProcess` equals to the process captured by the handler. If they equal, wipe the fields. If not, then the fields have already been updated (either new process has started, or the fields were already wiped out by another handler), and so the wiping should be skipped. (cherry picked from commit bb2642fab70fb4d59e431e01e319dcf6b90d88d8) --- lldb/tools/lldb-dap/src-ts/lldb-dap-server.ts | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/lldb/tools/lldb-dap/src-ts/lldb-dap-server.ts b/lldb/tools/lldb-dap/src-ts/lldb-dap-server.ts index 03a0fc9d0fef3..f360fa3adfa80 100644 --- a/lldb/tools/lldb-dap/src-ts/lldb-dap-server.ts +++ b/lldb/tools/lldb-dap/src-ts/lldb-dap-server.ts @@ -39,8 +39,7 @@ export class LLDBDapServer implements vscode.Disposable { const process = child_process.spawn(dapPath, dapArgs, options); process.on("error", (error) => { reject(error); - this.serverProcess = undefined; - this.serverInfo = undefined; + this.cleanUp(process); }); process.on("exit", (code, signal) => { let errorMessage = "Server process exited early"; @@ -50,8 +49,7 @@ export class LLDBDapServer implements vscode.Disposable { errorMessage += ` due to signal ${signal}`; } reject(new Error(errorMessage)); - this.serverProcess = undefined; - this.serverInfo = undefined; + this.cleanUp(process); }); process.stdout.setEncoding("utf8").on("data", (data) => { const connection = /connection:\/\/\[([^\]]+)\]:(\d+)/.exec( @@ -126,7 +124,16 @@ Restarting the server will interrupt any existing debug sessions and start a new return; } this.serverProcess.kill(); - this.serverProcess = undefined; - this.serverInfo = undefined; + this.cleanUp(this.serverProcess); + } + + cleanUp(process: child_process.ChildProcessWithoutNullStreams) { + // If the following don't equal, then the fields have already been updated + // (either a new process has started, or the fields were already cleaned + // up), and so the cleanup should be skipped. + if (this.serverProcess === process) { + this.serverProcess = undefined; + this.serverInfo = undefined; + } } } From dd49810f49a8dfe707c09ca366dd1c6f60da42d5 Mon Sep 17 00:00:00 2001 From: royitaqi Date: Wed, 6 Aug 2025 11:42:21 -0700 Subject: [PATCH 11/18] [vscode-lldb] Add VS Code commands for high level debug workflow (#151827) This allows other debugger extensions to leverage the `lldb-dap` extension's settings and logic (e.g. "Server Mode"). Other debugger extensions can invoke these commands to resolve configuration, create adapter descriptor, and get the `lldb-dap` process for state tracking, additional interaction, and telemetry. VS Code commands added: * `lldb-dap.resolveDebugConfiguration` * `lldb-dap.resolveDebugConfigurationWithSubstitutedVariables` * `lldb-dap.createDebugAdapterDescriptor` * `lldb-dap.getServerProcess` (cherry picked from commit 0e0ea714f3079341b9e11eb478eb85400423ee7c) --- lldb/docs/resources/lldbdap.md | 23 ++++++++++++++++++ .../lldb-dap/src-ts/debug-adapter-factory.ts | 10 +++++++- .../src-ts/debug-configuration-provider.ts | 24 ++++++++++++++++++- lldb/tools/lldb-dap/src-ts/lldb-dap-server.ts | 7 ++++++ 4 files changed, 62 insertions(+), 2 deletions(-) diff --git a/lldb/docs/resources/lldbdap.md b/lldb/docs/resources/lldbdap.md index 955713dd1b594..3838c82ab5dfb 100644 --- a/lldb/docs/resources/lldbdap.md +++ b/lldb/docs/resources/lldbdap.md @@ -170,3 +170,26 @@ This is also very simple, just run: ```bash npm run format ``` + +## Working with the VS Code extension from another extension + +The VS Code extension exposes the following [VS Code +commands](https://code.visualstudio.com/api/extension-guides/command), +which can be invoked by other debugger extensions to leverage this extension's +settings and logic. The commands help resolve configuration, create adapter +descriptor, and get the lldb-dap process for state tracking, additional +interaction, and telemetry. + +``` +// Resolve debug configuration +const resolvedConfiguration = await vscode.commands.executeCommand("lldb-dap.resolveDebugConfiguration", folder, configuration, token); + +// Resolve debug configuration with substituted variables +const resolvedConfigurationWithSubstitutedVariables = await vscode.commands.executeCommand("lldb-dap.resolveDebugConfigurationWithSubstitutedVariables", folder, configuration, token); + +// Create debug adapter descriptor +const adapterDescriptor = await vscode.commands.executeCommand("lldb-dap.createDebugAdapterDescriptor", session, executable); + +// Get DAP server process +const process = await vscode.commands.executeCommand("lldb-dap.getServerProcess"); +``` diff --git a/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts b/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts index 6e94400b09155..157aa2ac76a1f 100644 --- a/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts +++ b/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts @@ -217,7 +217,15 @@ export class LLDBDapDescriptorFactory constructor( private readonly logger: vscode.LogOutputChannel, private logFilePath: LogFilePathProvider, - ) {} + ) { + vscode.commands.registerCommand( + "lldb-dap.createDebugAdapterDescriptor", + ( + session: vscode.DebugSession, + executable: vscode.DebugAdapterExecutable | undefined, + ) => this.createDebugAdapterDescriptor(session, executable), + ); + } async createDebugAdapterDescriptor( session: vscode.DebugSession, diff --git a/lldb/tools/lldb-dap/src-ts/debug-configuration-provider.ts b/lldb/tools/lldb-dap/src-ts/debug-configuration-provider.ts index 8c04ec2bdc9d3..1e16dac031125 100644 --- a/lldb/tools/lldb-dap/src-ts/debug-configuration-provider.ts +++ b/lldb/tools/lldb-dap/src-ts/debug-configuration-provider.ts @@ -76,7 +76,29 @@ export class LLDBDapConfigurationProvider private readonly server: LLDBDapServer, private readonly logger: vscode.LogOutputChannel, private readonly logFilePath: LogFilePathProvider, - ) {} + ) { + vscode.commands.registerCommand( + "lldb-dap.resolveDebugConfiguration", + ( + folder: vscode.WorkspaceFolder | undefined, + debugConfiguration: vscode.DebugConfiguration, + token?: vscode.CancellationToken, + ) => this.resolveDebugConfiguration(folder, debugConfiguration, token), + ); + vscode.commands.registerCommand( + "lldb-dap.resolveDebugConfigurationWithSubstitutedVariables", + ( + folder: vscode.WorkspaceFolder | undefined, + debugConfiguration: vscode.DebugConfiguration, + token?: vscode.CancellationToken, + ) => + this.resolveDebugConfigurationWithSubstitutedVariables( + folder, + debugConfiguration, + token, + ), + ); + } async resolveDebugConfiguration( folder: vscode.WorkspaceFolder | undefined, diff --git a/lldb/tools/lldb-dap/src-ts/lldb-dap-server.ts b/lldb/tools/lldb-dap/src-ts/lldb-dap-server.ts index f360fa3adfa80..5f9d8efdcb3a3 100644 --- a/lldb/tools/lldb-dap/src-ts/lldb-dap-server.ts +++ b/lldb/tools/lldb-dap/src-ts/lldb-dap-server.ts @@ -12,6 +12,13 @@ export class LLDBDapServer implements vscode.Disposable { private serverProcess?: child_process.ChildProcessWithoutNullStreams; private serverInfo?: Promise<{ host: string; port: number }>; + constructor() { + vscode.commands.registerCommand( + "lldb-dap.getServerProcess", + () => this.serverProcess, + ); + } + /** * Starts the server with the provided options. The server will be restarted or reused as * necessary. From 3c6c6b2c9518bc5482ad517be42192c42f50aad6 Mon Sep 17 00:00:00 2001 From: David Spickett Date: Fri, 8 Aug 2025 09:07:36 +0000 Subject: [PATCH 12/18] [lldb][test] Disable TestDAP_memory.py on 32-bit Arm Linux This has been very flakey lately: ====================================================================== ERROR: test_writeMemory (TestDAP_memory.TestDAP_memory) Tests the 'writeMemory' request ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py", line 202, in test_writeMemory mem_response = self.writeMemory( File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py", line 545, in writeMemory response = self.dap_server.request_writeMemory( File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py", line 850, in request_writeMemory return self.send_recv(command_dict) File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py", line 403, in send_recv raise ValueError(desc) ValueError: no response for "writeMemory" Config=arm-/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/bin/clang ====================================================================== ERROR: test_writeMemory (TestDAP_memory.TestDAP_memory) Tests the 'writeMemory' request ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 2067, in tearDown Base.tearDown(self) File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 1105, in tearDown hook() # try the plain call and hope it works File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py", line 486, in cleanup self.dap_server.request_disconnect(terminateDebuggee=True) File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py", line 799, in request_disconnect return self.send_recv(command_dict) File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py", line 397, in send_recv self.send_packet(command) File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py", line 349, in send_packet self.send.flush() BrokenPipeError: [Errno 32] Broken pipe Config=arm-/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/bin/clang ---------------------------------------------------------------------- General tracking issue - https://github.com/llvm/llvm-project/issues/137660 (cherry picked from commit b800930db22d7735eec1d54cc66530ddab123a4d) --- lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py b/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py index 2c3c1fe9cae6b..1d7bbdb0c5a90 100644 --- a/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py +++ b/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py @@ -126,6 +126,8 @@ def test_readMemory(self): self.continue_to_exit() + # Flakey on 32-bit Arm Linux. + @skipif(oslist=["linux"], archs=["arm$"]) def test_writeMemory(self): """ Tests the 'writeMemory' request From 2612df6827f12e11b1968e27d698e82650976b3d Mon Sep 17 00:00:00 2001 From: Ely Ronnen Date: Fri, 8 Aug 2025 22:29:47 +0200 Subject: [PATCH 13/18] [lldb-dap] persistent assembly breakpoints (#148061) Resolves #141955 - Adds data to breakpoints `Source` object, in order for assembly breakpoints, which rely on a temporary `sourceReference` value, to be able to resolve in future sessions like normal path+line breakpoints - Adds optional `instructions_offset` parameter to `BreakpointResolver` (cherry picked from commit 4d3feaea66f43758518d15e07a975e3492317b1c) --- lldb/include/lldb/API/SBTarget.h | 8 ++ .../lldb/Breakpoint/BreakpointResolver.h | 9 +- .../lldb/Breakpoint/BreakpointResolverName.h | 2 +- lldb/include/lldb/Core/Disassembler.h | 2 + lldb/include/lldb/Target/Target.h | 11 +- .../test/tools/lldb-dap/dap_server.py | 39 ++++++- .../test/tools/lldb-dap/lldbdap_testcase.py | 22 ++-- lldb/source/API/SBTarget.cpp | 46 ++++---- lldb/source/Breakpoint/BreakpointResolver.cpp | 39 +++++-- .../Breakpoint/BreakpointResolverAddress.cpp | 5 + .../Breakpoint/BreakpointResolverName.cpp | 15 +-- lldb/source/Core/Disassembler.cpp | 10 ++ .../DynamicLoaderDarwinKernel.cpp | 3 +- .../MacOSX-DYLD/DynamicLoaderMacOS.cpp | 7 +- .../AppleObjCRuntime/AppleObjCRuntimeV1.cpp | 2 +- .../AppleObjCRuntime/AppleObjCRuntimeV2.cpp | 2 +- .../GNUstepObjCRuntime/GNUstepObjCRuntime.cpp | 3 +- .../DarwinLog/StructuredDataDarwinLog.cpp | 4 +- lldb/source/Target/Target.cpp | 56 ++++++++-- .../TestDAP_breakpointAssembly.py | 76 +++++++++++++ lldb/tools/lldb-dap/Breakpoint.cpp | 31 +++++- lldb/tools/lldb-dap/CMakeLists.txt | 3 +- lldb/tools/lldb-dap/DAP.cpp | 13 ++- .../Handler/SetBreakpointsRequestHandler.cpp | 1 - lldb/tools/lldb-dap/Protocol/DAPTypes.cpp | 36 +++++++ lldb/tools/lldb-dap/Protocol/DAPTypes.h | 53 +++++++++ .../tools/lldb-dap/Protocol/ProtocolTypes.cpp | 5 +- lldb/tools/lldb-dap/Protocol/ProtocolTypes.h | 8 +- lldb/tools/lldb-dap/SourceBreakpoint.cpp | 101 ++++++++++++------ lldb/tools/lldb-dap/SourceBreakpoint.h | 7 ++ 30 files changed, 498 insertions(+), 121 deletions(-) create mode 100644 lldb/tools/lldb-dap/Protocol/DAPTypes.cpp create mode 100644 lldb/tools/lldb-dap/Protocol/DAPTypes.h diff --git a/lldb/include/lldb/API/SBTarget.h b/lldb/include/lldb/API/SBTarget.h index fc57245a80254..4381781383075 100644 --- a/lldb/include/lldb/API/SBTarget.h +++ b/lldb/include/lldb/API/SBTarget.h @@ -660,6 +660,14 @@ class LLDB_API SBTarget { lldb::LanguageType symbol_language, const SBFileSpecList &module_list, const SBFileSpecList &comp_unit_list); + lldb::SBBreakpoint BreakpointCreateByName( + const char *symbol_name, + uint32_t + name_type_mask, // Logical OR one or more FunctionNameType enum bits + lldb::LanguageType symbol_language, lldb::addr_t offset, + bool offset_is_insn_count, const SBFileSpecList &module_list, + const SBFileSpecList &comp_unit_list); + #ifdef SWIG lldb::SBBreakpoint BreakpointCreateByNames( const char **symbol_name, uint32_t num_names, diff --git a/lldb/include/lldb/Breakpoint/BreakpointResolver.h b/lldb/include/lldb/Breakpoint/BreakpointResolver.h index 52cd70e934e6d..243aceeb6fd6a 100644 --- a/lldb/include/lldb/Breakpoint/BreakpointResolver.h +++ b/lldb/include/lldb/Breakpoint/BreakpointResolver.h @@ -45,9 +45,9 @@ class BreakpointResolver : public Searcher { /// The breakpoint that owns this resolver. /// \param[in] resolverType /// The concrete breakpoint resolver type for this breakpoint. - BreakpointResolver(const lldb::BreakpointSP &bkpt, - unsigned char resolverType, - lldb::addr_t offset = 0); + BreakpointResolver(const lldb::BreakpointSP &bkpt, unsigned char resolverType, + lldb::addr_t offset = 0, + bool offset_is_insn_count = false); /// The Destructor is virtual, all significant breakpoint resolvers derive /// from this class. @@ -76,6 +76,7 @@ class BreakpointResolver : public Searcher { void SetOffset(lldb::addr_t offset); lldb::addr_t GetOffset() const { return m_offset; } + lldb::addr_t GetOffsetIsInsnCount() const { return m_offset_is_insn_count; } /// In response to this method the resolver scans all the modules in the /// breakpoint's target, and adds any new locations it finds. @@ -220,6 +221,8 @@ class BreakpointResolver : public Searcher { lldb::BreakpointWP m_breakpoint; // This is the breakpoint we add locations to. lldb::addr_t m_offset; // A random offset the user asked us to add to any // breakpoints we set. + bool m_offset_is_insn_count; // Use the offset as an instruction count + // instead of an address offset. // Subclass identifier (for llvm isa/dyn_cast) const unsigned char SubclassID; diff --git a/lldb/include/lldb/Breakpoint/BreakpointResolverName.h b/lldb/include/lldb/Breakpoint/BreakpointResolverName.h index c83814c174e88..48b3edabd808f 100644 --- a/lldb/include/lldb/Breakpoint/BreakpointResolverName.h +++ b/lldb/include/lldb/Breakpoint/BreakpointResolverName.h @@ -27,7 +27,7 @@ class BreakpointResolverName : public BreakpointResolver { lldb::FunctionNameType name_type_mask, lldb::LanguageType language, Breakpoint::MatchType type, lldb::addr_t offset, - bool skip_prologue); + bool offset_is_insn_count, bool skip_prologue); // This one takes an array of names. It is always MatchType = Exact. BreakpointResolverName(const lldb::BreakpointSP &bkpt, const char *names[], diff --git a/lldb/include/lldb/Core/Disassembler.h b/lldb/include/lldb/Core/Disassembler.h index 21bacb14f9b25..50a5d87835844 100644 --- a/lldb/include/lldb/Core/Disassembler.h +++ b/lldb/include/lldb/Core/Disassembler.h @@ -291,6 +291,8 @@ class InstructionList { size_t GetSize() const; + size_t GetTotalByteSize() const; + uint32_t GetMaxOpcocdeByteSize() const; lldb::InstructionSP GetInstructionAtIndex(size_t idx) const; diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index dadd6d267bcf6..476715ebb9ce1 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -19,6 +19,7 @@ #include "lldb/Breakpoint/BreakpointList.h" #include "lldb/Breakpoint/BreakpointName.h" #include "lldb/Breakpoint/WatchpointList.h" +#include "lldb/Core/Address.h" #include "lldb/Core/Architecture.h" #include "lldb/Core/Disassembler.h" #include "lldb/Core/ModuleList.h" @@ -817,7 +818,7 @@ class Target : public std::enable_shared_from_this, lldb::BreakpointSP CreateBreakpoint(lldb::addr_t load_addr, bool internal, bool request_hardware); - // Use this to create a breakpoint from a load address and a module file spec + // Use this to create a breakpoint from a file address and a module file spec lldb::BreakpointSP CreateAddressInModuleBreakpoint(lldb::addr_t file_addr, bool internal, const FileSpec &file_spec, @@ -846,8 +847,8 @@ class Target : public std::enable_shared_from_this, const FileSpecList *containingModules, const FileSpecList *containingSourceFiles, const char *func_name, lldb::FunctionNameType func_name_type_mask, lldb::LanguageType language, - lldb::addr_t offset, LazyBool skip_prologue, bool internal, - bool request_hardware); + lldb::addr_t offset, bool offset_is_insn_count, LazyBool skip_prologue, + bool internal, bool request_hardware); lldb::BreakpointSP CreateExceptionBreakpoint(enum lldb::LanguageType language, bool catch_bp, @@ -1438,6 +1439,10 @@ class Target : public std::enable_shared_from_this, const lldb_private::RegisterFlags &flags, uint32_t byte_size); + llvm::Expected + ReadInstructions(const Address &start_addr, uint32_t count, + const char *flavor_string = nullptr); + // Target Stop Hooks class StopHook : public UserID { public: 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 0b09893c7ed5b..939be9941a49d 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 @@ -107,17 +107,23 @@ def dump_dap_log(log_file): class Source(object): def __init__( - self, path: Optional[str] = None, source_reference: Optional[int] = None + self, + path: Optional[str] = None, + source_reference: Optional[int] = None, + raw_dict: Optional[dict[str, Any]] = None, ): self._name = None self._path = None self._source_reference = None + self._raw_dict = None if path is not None: self._name = os.path.basename(path) self._path = path elif source_reference is not None: self._source_reference = source_reference + elif raw_dict is not None: + self._raw_dict = raw_dict else: raise ValueError("Either path or source_reference must be provided") @@ -125,6 +131,9 @@ def __str__(self): return f"Source(name={self.name}, path={self.path}), source_reference={self.source_reference})" def as_dict(self): + if self._raw_dict is not None: + return self._raw_dict + source_dict = {} if self._name is not None: source_dict["name"] = self._name @@ -135,6 +144,19 @@ def as_dict(self): return source_dict +class Breakpoint(object): + def __init__(self, obj): + self._breakpoint = obj + + def is_verified(self): + """Check if the breakpoint is verified.""" + return self._breakpoint.get("verified", False) + + def source(self): + """Get the source of the breakpoint.""" + return self._breakpoint.get("source", {}) + + class NotSupportedError(KeyError): """Raised if a feature is not supported due to its capabilities.""" @@ -170,7 +192,7 @@ def __init__( self.initialized = False self.frame_scopes = {} self.init_commands = init_commands - self.resolved_breakpoints = {} + self.resolved_breakpoints: dict[str, Breakpoint] = {} @classmethod def encode_content(cls, s: str) -> bytes: @@ -326,8 +348,8 @@ def _process_continued(self, all_threads_continued: bool): def _update_verified_breakpoints(self, breakpoints: list[Event]): for breakpoint in breakpoints: if "id" in breakpoint: - self.resolved_breakpoints[str(breakpoint["id"])] = breakpoint.get( - "verified", False + self.resolved_breakpoints[str(breakpoint["id"])] = Breakpoint( + breakpoint ) def send_packet(self, command_dict: Request, set_sequence=True): @@ -484,7 +506,14 @@ def wait_for_breakpoints_to_be_verified( if breakpoint_event is None: break - return [id for id in breakpoint_ids if id not in self.resolved_breakpoints] + return [ + id + for id in breakpoint_ids + if ( + id not in self.resolved_breakpoints + or not self.resolved_breakpoints[id].is_verified() + ) + ] def wait_for_exited(self, timeout: Optional[float] = None): event_dict = self.wait_for_event("exited", timeout=timeout) 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 1567462839748..c51b4b1892951 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 @@ -59,24 +59,22 @@ def set_source_breakpoints( Each object in data is 1:1 mapping with the entry in lines. It contains optional location/hitCondition/logMessage parameters. """ - response = self.dap_server.request_setBreakpoints( - Source(source_path), lines, data + return self.set_source_breakpoints_from_source( + Source(path=source_path), lines, data, wait_for_resolve ) - if response is None or not response["success"]: - return [] - breakpoints = response["body"]["breakpoints"] - breakpoint_ids = [] - for breakpoint in breakpoints: - breakpoint_ids.append("%i" % (breakpoint["id"])) - if wait_for_resolve: - self.wait_for_breakpoints_to_resolve(breakpoint_ids) - return breakpoint_ids def set_source_breakpoints_assembly( self, source_reference, lines, data=None, wait_for_resolve=True + ): + return self.set_source_breakpoints_from_source( + Source(source_reference=source_reference), lines, data, wait_for_resolve + ) + + def set_source_breakpoints_from_source( + self, source: Source, lines, data=None, wait_for_resolve=True ): response = self.dap_server.request_setBreakpoints( - Source(source_reference=source_reference), + source, lines, data, ) diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp index 0cc25d0139f74..34f3c261719b2 100644 --- a/lldb/source/API/SBTarget.cpp +++ b/lldb/source/API/SBTarget.cpp @@ -771,16 +771,19 @@ SBBreakpoint SBTarget::BreakpointCreateByName(const char *symbol_name, const bool hardware = false; const LazyBool skip_prologue = eLazyBoolCalculate; const lldb::addr_t offset = 0; + const bool offset_is_insn_count = false; if (module_name && module_name[0]) { FileSpecList module_spec_list; module_spec_list.Append(FileSpec(module_name)); sb_bp = target_sp->CreateBreakpoint( &module_spec_list, nullptr, symbol_name, eFunctionNameTypeAuto, - eLanguageTypeUnknown, offset, skip_prologue, internal, hardware); + eLanguageTypeUnknown, offset, offset_is_insn_count, skip_prologue, + internal, hardware); } else { sb_bp = target_sp->CreateBreakpoint( nullptr, nullptr, symbol_name, eFunctionNameTypeAuto, - eLanguageTypeUnknown, offset, skip_prologue, internal, hardware); + eLanguageTypeUnknown, offset, offset_is_insn_count, skip_prologue, + internal, hardware); } } @@ -816,6 +819,17 @@ lldb::SBBreakpoint SBTarget::BreakpointCreateByName( const SBFileSpecList &comp_unit_list) { LLDB_INSTRUMENT_VA(this, symbol_name, name_type_mask, symbol_language, module_list, comp_unit_list); + return BreakpointCreateByName(symbol_name, name_type_mask, symbol_language, 0, + false, module_list, comp_unit_list); +} + +lldb::SBBreakpoint SBTarget::BreakpointCreateByName( + const char *symbol_name, uint32_t name_type_mask, + LanguageType symbol_language, lldb::addr_t offset, + bool offset_is_insn_count, const SBFileSpecList &module_list, + const SBFileSpecList &comp_unit_list) { + LLDB_INSTRUMENT_VA(this, symbol_name, name_type_mask, symbol_language, offset, + offset_is_insn_count, module_list, comp_unit_list); SBBreakpoint sb_bp; if (TargetSP target_sp = GetSP(); @@ -826,7 +840,8 @@ lldb::SBBreakpoint SBTarget::BreakpointCreateByName( std::lock_guard guard(target_sp->GetAPIMutex()); FunctionNameType mask = static_cast(name_type_mask); sb_bp = target_sp->CreateBreakpoint(module_list.get(), comp_unit_list.get(), - symbol_name, mask, symbol_language, 0, + symbol_name, mask, symbol_language, + offset, offset_is_insn_count, skip_prologue, internal, hardware); } @@ -2013,29 +2028,10 @@ lldb::SBInstructionList SBTarget::ReadInstructions(lldb::SBAddress base_addr, if (TargetSP target_sp = GetSP()) { if (Address *addr_ptr = base_addr.get()) { - DataBufferHeap data( - target_sp->GetArchitecture().GetMaximumOpcodeByteSize() * count, 0); - bool force_live_memory = true; - lldb_private::Status error; - lldb::addr_t load_addr = LLDB_INVALID_ADDRESS; - const size_t bytes_read = - target_sp->ReadMemory(*addr_ptr, data.GetBytes(), data.GetByteSize(), - error, force_live_memory, &load_addr); - - const bool data_from_file = load_addr == LLDB_INVALID_ADDRESS; - if (!flavor_string || flavor_string[0] == '\0') { - // FIXME - we don't have the mechanism in place to do per-architecture - // settings. But since we know that for now we only support flavors on - // x86 & x86_64, - const llvm::Triple::ArchType arch = - target_sp->GetArchitecture().GetTriple().getArch(); - if (arch == llvm::Triple::x86 || arch == llvm::Triple::x86_64) - flavor_string = target_sp->GetDisassemblyFlavor(); + if (llvm::Expected disassembler = + target_sp->ReadInstructions(*addr_ptr, count, flavor_string)) { + sb_instructions.SetDisassembler(*disassembler); } - sb_instructions.SetDisassembler(Disassembler::DisassembleBytes( - target_sp->GetArchitecture(), nullptr, flavor_string, - target_sp->GetDisassemblyCPU(), target_sp->GetDisassemblyFeatures(), - *addr_ptr, data.GetBytes(), bytes_read, count, data_from_file)); } } diff --git a/lldb/source/Breakpoint/BreakpointResolver.cpp b/lldb/source/Breakpoint/BreakpointResolver.cpp index 91fdff4a455da..4ac40501a5df5 100644 --- a/lldb/source/Breakpoint/BreakpointResolver.cpp +++ b/lldb/source/Breakpoint/BreakpointResolver.cpp @@ -42,9 +42,9 @@ const char *BreakpointResolver::g_ty_to_name[] = {"FileAndLine", "Address", const char *BreakpointResolver::g_option_names[static_cast( BreakpointResolver::OptionNames::LastOptionName)] = { - "AddressOffset", "Exact", "FileName", "Inlines", "Language", - "LineNumber", "Column", "ModuleName", "NameMask", "Offset", - "PythonClass", "Regex", "ScriptArgs", "SectionName", "SearchDepth", + "AddressOffset", "Exact", "FileName", "Inlines", "Language", + "LineNumber", "Column", "ModuleName", "NameMask", "Offset", + "PythonClass", "Regex", "ScriptArgs", "SectionName", "SearchDepth", "SkipPrologue", "SymbolNames"}; const char *BreakpointResolver::ResolverTyToName(enum ResolverTy type) { @@ -65,8 +65,10 @@ BreakpointResolver::NameToResolverTy(llvm::StringRef name) { BreakpointResolver::BreakpointResolver(const BreakpointSP &bkpt, const unsigned char resolverTy, - lldb::addr_t offset) - : m_breakpoint(bkpt), m_offset(offset), SubclassID(resolverTy) {} + lldb::addr_t offset, + bool offset_is_insn_count) + : m_breakpoint(bkpt), m_offset(offset), + m_offset_is_insn_count(offset_is_insn_count), SubclassID(resolverTy) {} BreakpointResolver::~BreakpointResolver() = default; @@ -364,7 +366,32 @@ void BreakpointResolver::AddLocation(SearchFilter &filter, BreakpointLocationSP BreakpointResolver::AddLocation(Address loc_addr, bool *new_location) { - loc_addr.Slide(m_offset); + if (m_offset_is_insn_count) { + Target &target = GetBreakpoint()->GetTarget(); + llvm::Expected expected_instructions = + target.ReadInstructions(loc_addr, m_offset); + if (!expected_instructions) { + LLDB_LOG_ERROR(GetLog(LLDBLog::Breakpoints), + expected_instructions.takeError(), + "error: Unable to read instructions at address 0x{0:x}", + loc_addr.GetLoadAddress(&target)); + return BreakpointLocationSP(); + } + + const DisassemblerSP instructions = *expected_instructions; + if (!instructions || + instructions->GetInstructionList().GetSize() != m_offset) { + LLDB_LOG(GetLog(LLDBLog::Breakpoints), + "error: Unable to read {0} instructions at address 0x{1:x}", + m_offset, loc_addr.GetLoadAddress(&target)); + return BreakpointLocationSP(); + } + + loc_addr.Slide(instructions->GetInstructionList().GetTotalByteSize()); + } else { + loc_addr.Slide(m_offset); + } + return GetBreakpoint()->AddLocation(loc_addr, new_location); } diff --git a/lldb/source/Breakpoint/BreakpointResolverAddress.cpp b/lldb/source/Breakpoint/BreakpointResolverAddress.cpp index 828647ceef637..70360d9d960ec 100644 --- a/lldb/source/Breakpoint/BreakpointResolverAddress.cpp +++ b/lldb/source/Breakpoint/BreakpointResolverAddress.cpp @@ -133,6 +133,11 @@ Searcher::CallbackReturn BreakpointResolverAddress::SearchCallback( Address tmp_address; if (module_sp->ResolveFileAddress(m_addr.GetOffset(), tmp_address)) m_addr = tmp_address; + else + return Searcher::eCallbackReturnStop; + } else { + // If we didn't find the module, then we can't resolve the address. + return Searcher::eCallbackReturnStop; } } diff --git a/lldb/source/Breakpoint/BreakpointResolverName.cpp b/lldb/source/Breakpoint/BreakpointResolverName.cpp index edde1c91b789c..33ebbab302ed5 100644 --- a/lldb/source/Breakpoint/BreakpointResolverName.cpp +++ b/lldb/source/Breakpoint/BreakpointResolverName.cpp @@ -24,11 +24,13 @@ using namespace lldb; using namespace lldb_private; -BreakpointResolverName::BreakpointResolverName(const BreakpointSP &bkpt, - const char *name_cstr, FunctionNameType name_type_mask, - LanguageType language, Breakpoint::MatchType type, lldb::addr_t offset, +BreakpointResolverName::BreakpointResolverName( + const BreakpointSP &bkpt, const char *name_cstr, + FunctionNameType name_type_mask, LanguageType language, + Breakpoint::MatchType type, lldb::addr_t offset, bool offset_is_insn_count, bool skip_prologue) - : BreakpointResolver(bkpt, BreakpointResolver::NameResolver, offset), + : BreakpointResolver(bkpt, BreakpointResolver::NameResolver, offset, + offset_is_insn_count), m_match_type(type), m_language(language), m_skip_prologue(skip_prologue) { if (m_match_type == Breakpoint::Regexp) { m_regex = RegularExpression(name_cstr); @@ -81,7 +83,7 @@ BreakpointResolverName::BreakpointResolverName(const BreakpointSP &bkpt, BreakpointResolverName::BreakpointResolverName( const BreakpointResolverName &rhs) : BreakpointResolver(rhs.GetBreakpoint(), BreakpointResolver::NameResolver, - rhs.GetOffset()), + rhs.GetOffset(), rhs.GetOffsetIsInsnCount()), m_lookups(rhs.m_lookups), m_class_name(rhs.m_class_name), m_regex(rhs.m_regex), m_match_type(rhs.m_match_type), m_language(rhs.m_language), m_skip_prologue(rhs.m_skip_prologue) {} @@ -177,7 +179,8 @@ BreakpointResolverSP BreakpointResolverName::CreateFromStructuredData( std::shared_ptr resolver_sp = std::make_shared( nullptr, names[0].c_str(), name_masks[0], language, - Breakpoint::MatchType::Exact, offset, skip_prologue); + Breakpoint::MatchType::Exact, offset, + /*offset_is_insn_count = */ false, skip_prologue); for (size_t i = 1; i < num_elem; i++) { resolver_sp->AddNameLookup(ConstString(names[i]), name_masks[i]); } diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp index 925de2a5c836c..e0a7d69345706 100644 --- a/lldb/source/Core/Disassembler.cpp +++ b/lldb/source/Core/Disassembler.cpp @@ -1016,6 +1016,16 @@ uint32_t InstructionList::GetMaxOpcocdeByteSize() const { return max_inst_size; } +size_t InstructionList::GetTotalByteSize() const { + size_t total_byte_size = 0; + collection::const_iterator pos, end; + for (pos = m_instructions.begin(), end = m_instructions.end(); pos != end; + ++pos) { + total_byte_size += (*pos)->GetOpcode().GetByteSize(); + } + return total_byte_size; +} + InstructionSP InstructionList::GetInstructionAtIndex(size_t idx) const { InstructionSP inst_sp; if (idx < m_instructions.size()) diff --git a/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp b/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp index fe9f5d086da2c..1d210ea78df1a 100644 --- a/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp +++ b/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp @@ -1561,7 +1561,8 @@ void DynamicLoaderDarwinKernel::SetNotificationBreakpointIfNeeded() { .CreateBreakpoint(&module_spec_list, nullptr, "OSKextLoadedKextSummariesUpdated", eFunctionNameTypeFull, eLanguageTypeUnknown, 0, - skip_prologue, internal_bp, hardware) + /*offset_is_insn_count = */ false, skip_prologue, + internal_bp, hardware) .get(); bp->SetCallback(DynamicLoaderDarwinKernel::BreakpointHitCallback, this, diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp index 08bef4999eb9a..efb9ccc76b507 100644 --- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp +++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp @@ -530,7 +530,7 @@ bool DynamicLoaderMacOS::SetNotificationBreakpoint() { m_process->GetTarget() .CreateBreakpoint(&dyld_filelist, source_files, "lldb_image_notifier", eFunctionNameTypeFull, - eLanguageTypeUnknown, 0, skip_prologue, + eLanguageTypeUnknown, 0, false, skip_prologue, internal, hardware) .get(); breakpoint->SetCallback(DynamicLoaderMacOS::NotifyBreakpointHit, this, @@ -546,8 +546,9 @@ bool DynamicLoaderMacOS::SetNotificationBreakpoint() { m_process->GetTarget() .CreateBreakpoint(&dyld_filelist, source_files, "gdb_image_notifier", eFunctionNameTypeFull, - eLanguageTypeUnknown, 0, skip_prologue, - internal, hardware) + eLanguageTypeUnknown, 0, + /*offset_is_insn_count = */ false, + skip_prologue, internal, hardware) .get(); breakpoint->SetCallback(DynamicLoaderMacOS::NotifyBreakpointHit, this, true); diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp index 24a73717266a4..b1f2a661c4af6 100644 --- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp +++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp @@ -102,7 +102,7 @@ AppleObjCRuntimeV1::CreateExceptionResolver(const BreakpointSP &bkpt, resolver_sp = std::make_shared( bkpt, std::get<1>(GetExceptionThrowLocation()).AsCString(), eFunctionNameTypeBase, eLanguageTypeUnknown, Breakpoint::Exact, 0, - eLazyBoolNo); + /*offset_is_insn_count = */ false, eLazyBoolNo); // FIXME: don't do catch yet. return resolver_sp; } diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp index d8c6da1037c3f..d2b9a1477a234 100644 --- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp +++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp @@ -1176,7 +1176,7 @@ AppleObjCRuntimeV2::CreateExceptionResolver(const BreakpointSP &bkpt, resolver_sp = std::make_shared( bkpt, std::get<1>(GetExceptionThrowLocation()).AsCString(), eFunctionNameTypeBase, eLanguageTypeUnknown, Breakpoint::Exact, 0, - eLazyBoolNo); + /*offset_is_insn_count = */ false, eLazyBoolNo); // FIXME: We don't do catch breakpoints for ObjC yet. // Should there be some way for the runtime to specify what it can do in this // regard? diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp index a4b3e26474a55..8dc5f511c6291 100644 --- a/lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp +++ b/lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp @@ -169,7 +169,8 @@ GNUstepObjCRuntime::CreateExceptionResolver(const BreakpointSP &bkpt, if (throw_bp) resolver_sp = std::make_shared( bkpt, "objc_exception_throw", eFunctionNameTypeBase, - eLanguageTypeUnknown, Breakpoint::Exact, 0, eLazyBoolNo); + eLanguageTypeUnknown, Breakpoint::Exact, 0, + /*offset_is_insn_count = */ false, eLazyBoolNo); return resolver_sp; } diff --git a/lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp b/lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp index 82f18c5fe37a2..9537c842f4419 100644 --- a/lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp +++ b/lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp @@ -1603,6 +1603,7 @@ void StructuredDataDarwinLog::AddInitCompletionHook(Process &process) { const char *func_name = "_libtrace_init"; const lldb::addr_t offset = 0; + const bool offset_is_insn_count = false; const LazyBool skip_prologue = eLazyBoolCalculate; // This is an internal breakpoint - the user shouldn't see it. const bool internal = true; @@ -1610,7 +1611,8 @@ void StructuredDataDarwinLog::AddInitCompletionHook(Process &process) { auto breakpoint_sp = target.CreateBreakpoint( &module_spec_list, source_spec_list, func_name, eFunctionNameTypeFull, - eLanguageTypeC, offset, skip_prologue, internal, hardware); + eLanguageTypeC, offset, offset_is_insn_count, skip_prologue, internal, + hardware); if (!breakpoint_sp) { // Huh? Bail here. LLDB_LOGF(log, diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index a3414c89375aa..762b97868d6fb 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -581,10 +581,11 @@ BreakpointSP Target::CreateBreakpoint(lldb::addr_t addr, bool internal, BreakpointSP Target::CreateBreakpoint(const Address &addr, bool internal, bool hardware) { - SearchFilterSP filter_sp( - new SearchFilterForUnconstrainedSearches(shared_from_this())); - BreakpointResolverSP resolver_sp( - new BreakpointResolverAddress(nullptr, addr)); + SearchFilterSP filter_sp = + std::make_shared( + shared_from_this()); + BreakpointResolverSP resolver_sp = + std::make_shared(nullptr, addr); return CreateBreakpoint(filter_sp, resolver_sp, internal, hardware, false); } @@ -592,10 +593,12 @@ lldb::BreakpointSP Target::CreateAddressInModuleBreakpoint(lldb::addr_t file_addr, bool internal, const FileSpec &file_spec, bool request_hardware) { - SearchFilterSP filter_sp( - new SearchFilterForUnconstrainedSearches(shared_from_this())); - BreakpointResolverSP resolver_sp(new BreakpointResolverAddress( - nullptr, file_addr, file_spec)); + SearchFilterSP filter_sp = + std::make_shared( + shared_from_this()); + BreakpointResolverSP resolver_sp = + std::make_shared(nullptr, file_addr, + file_spec); return CreateBreakpoint(filter_sp, resolver_sp, internal, request_hardware, false); } @@ -604,7 +607,8 @@ BreakpointSP Target::CreateBreakpoint( const FileSpecList *containingModules, const FileSpecList *containingSourceFiles, const char *func_name, FunctionNameType func_name_type_mask, LanguageType language, - lldb::addr_t offset, LazyBool skip_prologue, bool internal, bool hardware) { + lldb::addr_t offset, bool offset_is_insn_count, LazyBool skip_prologue, + bool internal, bool hardware) { BreakpointSP bp_sp; if (func_name) { SearchFilterSP filter_sp(GetSearchFilterForModuleAndCUList( @@ -617,7 +621,7 @@ BreakpointSP Target::CreateBreakpoint( BreakpointResolverSP resolver_sp(new BreakpointResolverName( nullptr, func_name, func_name_type_mask, language, Breakpoint::Exact, - offset, skip_prologue)); + offset, offset_is_insn_count, skip_prologue)); bp_sp = CreateBreakpoint(filter_sp, resolver_sp, internal, hardware, true); } return bp_sp; @@ -3093,6 +3097,38 @@ lldb::addr_t Target::GetBreakableLoadAddress(lldb::addr_t addr) { return arch_plugin ? arch_plugin->GetBreakableLoadAddress(addr, *this) : addr; } +llvm::Expected +Target::ReadInstructions(const Address &start_addr, uint32_t count, + const char *flavor_string) { + DataBufferHeap data(GetArchitecture().GetMaximumOpcodeByteSize() * count, 0); + bool force_live_memory = true; + lldb_private::Status error; + lldb::addr_t load_addr = LLDB_INVALID_ADDRESS; + const size_t bytes_read = + ReadMemory(start_addr, data.GetBytes(), data.GetByteSize(), error, + force_live_memory, &load_addr); + + if (error.Fail()) + return llvm::createStringError( + error.AsCString("Target::ReadInstructions failed to read memory at %s"), + start_addr.GetLoadAddress(this)); + + const bool data_from_file = load_addr == LLDB_INVALID_ADDRESS; + if (!flavor_string || flavor_string[0] == '\0') { + // FIXME - we don't have the mechanism in place to do per-architecture + // settings. But since we know that for now we only support flavors on + // x86 & x86_64, + const llvm::Triple::ArchType arch = GetArchitecture().GetTriple().getArch(); + if (arch == llvm::Triple::x86 || arch == llvm::Triple::x86_64) + flavor_string = GetDisassemblyFlavor(); + } + + return Disassembler::DisassembleBytes( + GetArchitecture(), nullptr, flavor_string, GetDisassemblyCPU(), + GetDisassemblyFeatures(), start_addr, data.GetBytes(), bytes_read, count, + data_from_file); +} + SourceManager &Target::GetSourceManager() { if (!m_source_manager_up) m_source_manager_up = std::make_unique(shared_from_this()); diff --git a/lldb/test/API/tools/lldb-dap/breakpoint-assembly/TestDAP_breakpointAssembly.py b/lldb/test/API/tools/lldb-dap/breakpoint-assembly/TestDAP_breakpointAssembly.py index 674bfe4199b4a..7552a77d2280a 100644 --- a/lldb/test/API/tools/lldb-dap/breakpoint-assembly/TestDAP_breakpointAssembly.py +++ b/lldb/test/API/tools/lldb-dap/breakpoint-assembly/TestDAP_breakpointAssembly.py @@ -83,3 +83,79 @@ def test_break_on_invalid_source_reference(self): break_point["message"], "Invalid sourceReference.", ) + + @skipIfWindows + def test_persistent_assembly_breakpoint(self): + """Tests that assembly breakpoints are working persistently across sessions""" + self.build() + program = self.getBuildArtifact("a.out") + self.create_debug_adapter() + + # Run the first session and set a persistent assembly breakpoint + try: + self.dap_server.request_initialize() + self.dap_server.request_launch(program) + + assmebly_func_breakpoints = self.set_function_breakpoints(["assembly_func"]) + self.continue_to_breakpoints(assmebly_func_breakpoints) + + assembly_func_frame = self.get_stackFrames()[0] + source_reference = assembly_func_frame["source"]["sourceReference"] + + # Set an assembly breakpoint in the middle of the assembly function + persistent_breakpoint_line = 4 + persistent_breakpoint_ids = self.set_source_breakpoints_assembly( + source_reference, [persistent_breakpoint_line] + ) + + self.assertEqual( + len(persistent_breakpoint_ids), + 1, + "Expected one assembly breakpoint to be set", + ) + + persistent_breakpoint_source = self.dap_server.resolved_breakpoints[ + persistent_breakpoint_ids[0] + ].source() + self.assertIn( + "adapterData", + persistent_breakpoint_source, + "Expected assembly breakpoint to have persistent information", + ) + self.assertIn( + "persistenceData", + persistent_breakpoint_source["adapterData"], + "Expected assembly breakpoint to have persistent information", + ) + + self.continue_to_breakpoints(persistent_breakpoint_ids) + finally: + self.dap_server.request_disconnect(terminateDebuggee=True) + self.dap_server.terminate() + + # Restart the session and verify the breakpoint is still there + self.create_debug_adapter() + try: + self.dap_server.request_initialize() + self.dap_server.request_launch(program) + new_session_breakpoints_ids = self.set_source_breakpoints_from_source( + Source(raw_dict=persistent_breakpoint_source), + [persistent_breakpoint_line], + ) + + self.assertEqual( + len(new_session_breakpoints_ids), + 1, + "Expected one breakpoint to be set in the new session", + ) + + self.continue_to_breakpoints(new_session_breakpoints_ids) + current_line = self.get_stackFrames()[0]["line"] + self.assertEqual( + current_line, + persistent_breakpoint_line, + "Expected to hit the persistent assembly breakpoint at the same line", + ) + finally: + self.dap_server.request_disconnect(terminateDebuggee=True) + self.dap_server.terminate() diff --git a/lldb/tools/lldb-dap/Breakpoint.cpp b/lldb/tools/lldb-dap/Breakpoint.cpp index b4e593eb83d27..c8039576b29bd 100644 --- a/lldb/tools/lldb-dap/Breakpoint.cpp +++ b/lldb/tools/lldb-dap/Breakpoint.cpp @@ -8,10 +8,14 @@ #include "Breakpoint.h" #include "DAP.h" +#include "LLDBUtils.h" +#include "Protocol/DAPTypes.h" #include "ProtocolUtils.h" #include "lldb/API/SBAddress.h" #include "lldb/API/SBBreakpointLocation.h" +#include "lldb/API/SBFileSpec.h" #include "lldb/API/SBLineEntry.h" +#include "lldb/API/SBModule.h" #include "lldb/API/SBMutex.h" #include "llvm/ADT/StringExtras.h" #include @@ -21,6 +25,22 @@ using namespace lldb_dap; +static std::optional +GetPersistenceDataForSymbol(lldb::SBSymbol &symbol) { + protocol::PersistenceData persistence_data; + lldb::SBModule module = symbol.GetStartAddress().GetModule(); + if (!module.IsValid()) + return std::nullopt; + + lldb::SBFileSpec file_spec = module.GetFileSpec(); + if (!file_spec.IsValid()) + return std::nullopt; + + persistence_data.module_path = GetSBFileSpecPath(file_spec); + persistence_data.symbol_name = symbol.GetName(); + return persistence_data; +} + void Breakpoint::SetCondition() { m_bp.SetCondition(m_condition.c_str()); } void Breakpoint::SetHitCondition() { @@ -73,7 +93,7 @@ protocol::Breakpoint Breakpoint::ToProtocolBreakpoint() { const auto column = line_entry.GetColumn(); if (column != LLDB_INVALID_COLUMN_NUMBER) breakpoint.column = column; - } else { + } else if (source) { // Assembly breakpoint. auto symbol = bp_addr.GetSymbol(); if (symbol.IsValid()) { @@ -82,6 +102,15 @@ protocol::Breakpoint Breakpoint::ToProtocolBreakpoint() { .ReadInstructions(symbol.GetStartAddress(), bp_addr, nullptr) .GetSize() + 1; + + // Add persistent data so that the breakpoint can be resolved + // in future sessions. + std::optional persistence_data = + GetPersistenceDataForSymbol(symbol); + if (persistence_data) { + source->adapterData = + protocol::SourceLLDBData{std::move(persistence_data)}; + } } } diff --git a/lldb/tools/lldb-dap/CMakeLists.txt b/lldb/tools/lldb-dap/CMakeLists.txt index 4cddfb1bea1c2..5e0ad53b82f89 100644 --- a/lldb/tools/lldb-dap/CMakeLists.txt +++ b/lldb/tools/lldb-dap/CMakeLists.txt @@ -66,7 +66,8 @@ add_lldb_library(lldbDAP Handler/ThreadsRequestHandler.cpp Handler/VariablesRequestHandler.cpp Handler/WriteMemoryRequestHandler.cpp - + + Protocol/DAPTypes.cpp Protocol/ProtocolBase.cpp Protocol/ProtocolEvents.cpp Protocol/ProtocolTypes.cpp diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index cbd3b14463e25..849712f724c69 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -1406,11 +1406,15 @@ void DAP::EventThread() { // avoids sending paths that should be source mapped. Note that // CreateBreakpoint doesn't apply source mapping and certain // implementation ignore the source part of this event anyway. - llvm::json::Value source_bp = bp.ToProtocolBreakpoint(); - source_bp.getAsObject()->erase("source"); + protocol::Breakpoint protocol_bp = bp.ToProtocolBreakpoint(); + + // "source" is not needed here, unless we add adapter data to be + // saved by the client. + if (protocol_bp.source && !protocol_bp.source->adapterData) + protocol_bp.source = std::nullopt; llvm::json::Object body; - body.try_emplace("breakpoint", source_bp); + body.try_emplace("breakpoint", protocol_bp); body.try_emplace("reason", "changed"); llvm::json::Object bp_event = CreateEventObject("breakpoint"); @@ -1491,8 +1495,9 @@ std::vector DAP::SetSourceBreakpoints( protocol::Breakpoint response_breakpoint = iv->second.ToProtocolBreakpoint(); - response_breakpoint.source = source; + if (!response_breakpoint.source) + response_breakpoint.source = source; if (!response_breakpoint.line && src_bp.GetLine() != LLDB_INVALID_LINE_NUMBER) response_breakpoint.line = src_bp.GetLine(); diff --git a/lldb/tools/lldb-dap/Handler/SetBreakpointsRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/SetBreakpointsRequestHandler.cpp index 5d336af740c99..142351fd62179 100644 --- a/lldb/tools/lldb-dap/Handler/SetBreakpointsRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/SetBreakpointsRequestHandler.cpp @@ -9,7 +9,6 @@ #include "DAP.h" #include "Protocol/ProtocolRequests.h" #include "RequestHandler.h" -#include namespace lldb_dap { diff --git a/lldb/tools/lldb-dap/Protocol/DAPTypes.cpp b/lldb/tools/lldb-dap/Protocol/DAPTypes.cpp new file mode 100644 index 0000000000000..ecb4baef56e80 --- /dev/null +++ b/lldb/tools/lldb-dap/Protocol/DAPTypes.cpp @@ -0,0 +1,36 @@ +#include "Protocol/DAPTypes.h" + +using namespace llvm; + +namespace lldb_dap::protocol { + +bool fromJSON(const llvm::json::Value &Params, PersistenceData &PD, + llvm::json::Path P) { + json::ObjectMapper O(Params, P); + return O && O.mapOptional("module_path", PD.module_path) && + O.mapOptional("symbol_name", PD.symbol_name); +} + +llvm::json::Value toJSON(const PersistenceData &PD) { + json::Object result{ + {"module_path", PD.module_path}, + {"symbol_name", PD.symbol_name}, + }; + + return result; +} + +bool fromJSON(const llvm::json::Value &Params, SourceLLDBData &SLD, + llvm::json::Path P) { + json::ObjectMapper O(Params, P); + return O && O.mapOptional("persistenceData", SLD.persistenceData); +} + +llvm::json::Value toJSON(const SourceLLDBData &SLD) { + json::Object result; + if (SLD.persistenceData) + result.insert({"persistenceData", SLD.persistenceData}); + return result; +} + +} // namespace lldb_dap::protocol \ No newline at end of file diff --git a/lldb/tools/lldb-dap/Protocol/DAPTypes.h b/lldb/tools/lldb-dap/Protocol/DAPTypes.h new file mode 100644 index 0000000000000..716d8b491b258 --- /dev/null +++ b/lldb/tools/lldb-dap/Protocol/DAPTypes.h @@ -0,0 +1,53 @@ +//===-- ProtocolTypes.h ---------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// This file contains private DAP types used in the protocol. +// +// Each struct has a toJSON and fromJSON function, that converts between +// the struct and a JSON representation. (See JSON.h) +// +//===----------------------------------------------------------------------===// + +#ifndef LLDB_TOOLS_LLDB_DAP_PROTOCOL_DAP_TYPES_H +#define LLDB_TOOLS_LLDB_DAP_PROTOCOL_DAP_TYPES_H + +#include "lldb/lldb-types.h" +#include "llvm/Support/JSON.h" +#include +#include + +namespace lldb_dap::protocol { + +/// Data used to help lldb-dap resolve breakpoints persistently across different +/// sessions. This information is especially useful for assembly breakpoints, +/// because `sourceReference` can change across sessions. For regular source +/// breakpoints the path and line are the same For each session. +struct PersistenceData { + /// The source module path. + std::string module_path; + + /// The symbol name of the Source. + std::string symbol_name; +}; +bool fromJSON(const llvm::json::Value &, PersistenceData &, llvm::json::Path); +llvm::json::Value toJSON(const PersistenceData &); + +/// Custom source data used by lldb-dap. +/// This data should help lldb-dap identify sources correctly across different +/// sessions. +struct SourceLLDBData { + /// Data that helps lldb resolve this source persistently across different + /// sessions. + std::optional persistenceData; +}; +bool fromJSON(const llvm::json::Value &, SourceLLDBData &, llvm::json::Path); +llvm::json::Value toJSON(const SourceLLDBData &); + +} // namespace lldb_dap::protocol + +#endif diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp index fe8bb5252cc23..369858c3a5f4b 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp +++ b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp @@ -46,7 +46,8 @@ bool fromJSON(const json::Value &Params, Source &S, json::Path P) { json::ObjectMapper O(Params, P); return O && O.map("name", S.name) && O.map("path", S.path) && O.map("presentationHint", S.presentationHint) && - O.map("sourceReference", S.sourceReference); + O.map("sourceReference", S.sourceReference) && + O.map("adapterData", S.adapterData); } llvm::json::Value toJSON(Source::PresentationHint hint) { @@ -71,6 +72,8 @@ llvm::json::Value toJSON(const Source &S) { result.insert({"sourceReference", *S.sourceReference}); if (S.presentationHint) result.insert({"presentationHint", *S.presentationHint}); + if (S.adapterData) + result.insert({"adapterData", *S.adapterData}); return result; } diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h index 89122c8f66307..c4be7911a662b 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h @@ -20,6 +20,7 @@ #ifndef LLDB_TOOLS_LLDB_DAP_PROTOCOL_PROTOCOL_TYPES_H #define LLDB_TOOLS_LLDB_DAP_PROTOCOL_PROTOCOL_TYPES_H +#include "Protocol/DAPTypes.h" #include "lldb/lldb-defines.h" #include "llvm/ADT/DenseSet.h" #include "llvm/Support/JSON.h" @@ -336,7 +337,12 @@ struct Source { /// skipped on stepping. std::optional presentationHint; - // unsupported keys: origin, sources, adapterData, checksums + /// Additional data that a debug adapter might want to loop through the + /// client. The client should leave the data intact and persist it across + /// sessions. The client should not interpret the data. + std::optional adapterData; + + // unsupported keys: origin, sources, checksums }; bool fromJSON(const llvm::json::Value &, Source::PresentationHint &, llvm::json::Path); diff --git a/lldb/tools/lldb-dap/SourceBreakpoint.cpp b/lldb/tools/lldb-dap/SourceBreakpoint.cpp index 5fce9fe0ddbb3..843a5eb09c7ae 100644 --- a/lldb/tools/lldb-dap/SourceBreakpoint.cpp +++ b/lldb/tools/lldb-dap/SourceBreakpoint.cpp @@ -10,7 +10,9 @@ #include "BreakpointBase.h" #include "DAP.h" #include "JSONUtils.h" +#include "ProtocolUtils.h" #include "lldb/API/SBBreakpoint.h" +#include "lldb/API/SBFileSpec.h" #include "lldb/API/SBFileSpecList.h" #include "lldb/API/SBFrame.h" #include "lldb/API/SBInstruction.h" @@ -46,41 +48,20 @@ llvm::Error SourceBreakpoint::SetBreakpoint(const protocol::Source &source) { if (source.sourceReference) { // Breakpoint set by assembly source. - std::optional raw_addr = - m_dap.GetSourceReferenceAddress(*source.sourceReference); - if (!raw_addr) - return llvm::createStringError(llvm::inconvertibleErrorCode(), - "Invalid sourceReference."); - - lldb::SBAddress source_address(*raw_addr, m_dap.target); - if (!source_address.IsValid()) - return llvm::createStringError(llvm::inconvertibleErrorCode(), - "Invalid sourceReference."); - - lldb::SBSymbol symbol = source_address.GetSymbol(); - if (!symbol.IsValid()) { - // FIXME: Support assembly breakpoints without a valid symbol. - return llvm::createStringError(llvm::inconvertibleErrorCode(), - "Breakpoints in assembly without a valid " - "symbol are not supported yet."); + if (source.adapterData && source.adapterData->persistenceData) { + // Prefer use the adapter persitence data, because this could be a + // breakpoint from a previous session where the `sourceReference` is not + // valid anymore. + if (llvm::Error error = CreateAssemblyBreakpointWithPersistenceData( + *source.adapterData->persistenceData)) + return error; + } else { + if (llvm::Error error = CreateAssemblyBreakpointWithSourceReference( + *source.sourceReference)) + return error; } - - lldb::SBInstructionList inst_list = - m_dap.target.ReadInstructions(symbol.GetStartAddress(), m_line); - if (inst_list.GetSize() < m_line) - return llvm::createStringError(llvm::inconvertibleErrorCode(), - "Invalid instruction list size."); - - lldb::SBAddress address = - inst_list.GetInstructionAtIndex(m_line - 1).GetAddress(); - - m_bp = m_dap.target.BreakpointCreateBySBAddress(address); } else { - // Breakpoint set by a regular source file. - const auto source_path = source.path.value_or(""); - lldb::SBFileSpecList module_list; - m_bp = m_dap.target.BreakpointCreateByLocation(source_path.c_str(), m_line, - m_column, 0, module_list); + CreatePathBreakpoint(source); } if (!m_log_message.empty()) @@ -97,6 +78,60 @@ void SourceBreakpoint::UpdateBreakpoint(const SourceBreakpoint &request_bp) { BreakpointBase::UpdateBreakpoint(request_bp); } +void SourceBreakpoint::CreatePathBreakpoint(const protocol::Source &source) { + const auto source_path = source.path.value_or(""); + lldb::SBFileSpecList module_list; + m_bp = m_dap.target.BreakpointCreateByLocation(source_path.c_str(), m_line, + m_column, 0, module_list); +} + +llvm::Error SourceBreakpoint::CreateAssemblyBreakpointWithSourceReference( + int64_t source_reference) { + std::optional raw_addr = + m_dap.GetSourceReferenceAddress(source_reference); + if (!raw_addr) + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "Invalid sourceReference."); + + lldb::SBAddress source_address(*raw_addr, m_dap.target); + if (!source_address.IsValid()) + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "Invalid sourceReference."); + + lldb::SBSymbol symbol = source_address.GetSymbol(); + if (!symbol.IsValid()) { + // FIXME: Support assembly breakpoints without a valid symbol. + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "Breakpoints in assembly without a valid " + "symbol are not supported yet."); + } + + lldb::SBInstructionList inst_list = + m_dap.target.ReadInstructions(symbol.GetStartAddress(), m_line); + if (inst_list.GetSize() < m_line) + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "Invalid instruction list size."); + + lldb::SBAddress address = + inst_list.GetInstructionAtIndex(m_line - 1).GetAddress(); + + m_bp = m_dap.target.BreakpointCreateBySBAddress(address); + return llvm::Error::success(); +} + +llvm::Error SourceBreakpoint::CreateAssemblyBreakpointWithPersistenceData( + const protocol::PersistenceData &persistence_data) { + lldb::SBFileSpec file_spec(persistence_data.module_path.c_str()); + lldb::SBFileSpecList comp_unit_list; + lldb::SBFileSpecList file_spec_list; + file_spec_list.Append(file_spec); + m_bp = m_dap.target.BreakpointCreateByName( + persistence_data.symbol_name.c_str(), lldb::eFunctionNameTypeFull, + lldb::eLanguageTypeUnknown, m_line - 1, true, file_spec_list, + comp_unit_list); + return llvm::Error::success(); +} + lldb::SBError SourceBreakpoint::AppendLogMessagePart(llvm::StringRef part, bool is_expr) { if (is_expr) { diff --git a/lldb/tools/lldb-dap/SourceBreakpoint.h b/lldb/tools/lldb-dap/SourceBreakpoint.h index 857ac4286d59d..34054a8dcfd5f 100644 --- a/lldb/tools/lldb-dap/SourceBreakpoint.h +++ b/lldb/tools/lldb-dap/SourceBreakpoint.h @@ -11,6 +11,7 @@ #include "Breakpoint.h" #include "DAPForward.h" +#include "Protocol/DAPTypes.h" #include "Protocol/ProtocolTypes.h" #include "lldb/API/SBError.h" #include "llvm/ADT/StringRef.h" @@ -50,6 +51,12 @@ class SourceBreakpoint : public Breakpoint { uint32_t GetColumn() const { return m_column; } protected: + void CreatePathBreakpoint(const protocol::Source &source); + llvm::Error + CreateAssemblyBreakpointWithSourceReference(int64_t source_reference); + llvm::Error CreateAssemblyBreakpointWithPersistenceData( + const protocol::PersistenceData &persistence_data); + // logMessage part can be either a raw text or an expression. struct LogMessagePart { LogMessagePart(llvm::StringRef text, bool is_expr) From a2d0c7bd160f556c2cd98638c0749b2faf7ae61c Mon Sep 17 00:00:00 2001 From: Kazu Hirata Date: Fri, 8 Aug 2025 14:44:35 -0700 Subject: [PATCH 14/18] [lldb] Fix warnings This patch fixes: lldb/unittests/DAP/ProtocolTypesTest.cpp:112:67: error: missing field 'adapterData' initializer [-Werror,-Wmissing-field-initializers] lldb/unittests/DAP/ProtocolTypesTest.cpp:571:70: error: missing field 'adapterData' initializer [-Werror,-Wmissing-field-initializers] (cherry picked from commit f091b401844e3c689c06f5ea6c1674e1a0bb4e86) --- lldb/unittests/DAP/ProtocolTypesTest.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lldb/unittests/DAP/ProtocolTypesTest.cpp b/lldb/unittests/DAP/ProtocolTypesTest.cpp index 3a1b9402b40c9..4aab2dc223134 100644 --- a/lldb/unittests/DAP/ProtocolTypesTest.cpp +++ b/lldb/unittests/DAP/ProtocolTypesTest.cpp @@ -108,8 +108,9 @@ TEST(ProtocolTypesTest, Breakpoint) { breakpoint.id = 42; breakpoint.verified = true; breakpoint.message = "Breakpoint set successfully"; - breakpoint.source = Source{"test.cpp", "/path/to/test.cpp", 123, - Source::eSourcePresentationHintNormal}; + breakpoint.source = + Source{"test.cpp", "/path/to/test.cpp", 123, + Source::eSourcePresentationHintNormal, std::nullopt}; breakpoint.line = 10; breakpoint.column = 5; breakpoint.endLine = 15; @@ -567,8 +568,9 @@ TEST(ProtocolTypesTest, DisassembledInstruction) { instruction.instructionBytes = "0F 1F 00"; instruction.instruction = "mov eax, ebx"; instruction.symbol = "main"; - instruction.location = Source{"test.cpp", "/path/to/test.cpp", 123, - Source::eSourcePresentationHintNormal}; + instruction.location = + Source{"test.cpp", "/path/to/test.cpp", 123, + Source::eSourcePresentationHintNormal, std::nullopt}; instruction.line = 10; instruction.column = 5; instruction.endLine = 15; From 5534767388ea6c2ff55fd0eb4cd25c6eb795ea83 Mon Sep 17 00:00:00 2001 From: David Spickett Date: Fri, 15 Aug 2025 12:26:45 +0000 Subject: [PATCH 15/18] [lldb][lldb-dap][test] Disable part of TestDAP_launch on Arm 32-bit This test has been flakey on our bot: https://lab.llvm.org/buildbot/#/builders/18/builds/20410 ``` ====================================================================== FAIL: test_extra_launch_commands (TestDAP_launch.TestDAP_launch) Tests the "launchCommands" with extra launching settings ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py", line 482, in test_extra_launch_commands self.verify_commands("stopCommands", output, stopCommands) File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py", line 228, in verify_commands self.assertTrue( AssertionError: False is not true : verify 'frame variable' found in console output for 'stopCommands' Config=arm-/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/bin/clang ---------------------------------------------------------------------- ``` Likely a timing issue waiting for the command output on a slower machine. General tracking issue - https://github.com/llvm/llvm-project/issues/137660 (cherry picked from commit 4f65345ab5f2787a4704efb5828657c50be6d65a) --- lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py index 94a0ac698b80c..97c2bca3313ab 100644 --- a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py +++ b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py @@ -431,6 +431,8 @@ def test_commands(self): self.verify_commands("exitCommands", output, exitCommands) self.verify_commands("terminateCommands", output, terminateCommands) + # Flakey on 32-bit Arm Linux. + @skipif(oslist=["linux"], archs=["arm$"]) def test_extra_launch_commands(self): """ Tests the "launchCommands" with extra launching settings From 592682acc974fe2374276403acbde494014b5472 Mon Sep 17 00:00:00 2001 From: David Spickett Date: Fri, 15 Aug 2025 12:28:39 +0000 Subject: [PATCH 16/18] [lldb][lldb-dap][test] Correct skip in TestDAP_launch Fixes 4f65345ab5f2787a4704efb5828657c50be6d65a Yet again I forgot it's skip[I]f. (cherry picked from commit 0fca1e4e06445c98352fb3a034bf197bc7990f36) --- lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py index 97c2bca3313ab..2c0c5a583c58a 100644 --- a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py +++ b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py @@ -432,7 +432,7 @@ def test_commands(self): self.verify_commands("terminateCommands", output, terminateCommands) # Flakey on 32-bit Arm Linux. - @skipif(oslist=["linux"], archs=["arm$"]) + @skipIf(oslist=["linux"], archs=["arm$"]) def test_extra_launch_commands(self): """ Tests the "launchCommands" with extra launching settings From 6e115f094d757eb5f4f52fa401dff2dba54450cb Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Thu, 9 Oct 2025 11:18:19 -0700 Subject: [PATCH 17/18] [lldb] Adjust for CreateBreakpoint API change --- lldb/source/Plugins/ExpressionParser/Swift/SwiftREPL.cpp | 1 + lldb/source/Target/ThreadPlanCallFunction.cpp | 7 ++++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lldb/source/Plugins/ExpressionParser/Swift/SwiftREPL.cpp b/lldb/source/Plugins/ExpressionParser/Swift/SwiftREPL.cpp index 029c39444dd5a..7eb10de4ea3cb 100644 --- a/lldb/source/Plugins/ExpressionParser/Swift/SwiftREPL.cpp +++ b/lldb/source/Plugins/ExpressionParser/Swift/SwiftREPL.cpp @@ -173,6 +173,7 @@ lldb::REPLSP SwiftREPL::CreateInstanceFromDebugger(Status &err, eFunctionNameTypeAuto, // Name type eLanguageTypeUnknown, // Language 0, // offset + false, // offset_is_insn_count eLazyBoolYes, // skip_prologue, true, // internal false); // request_hardware diff --git a/lldb/source/Target/ThreadPlanCallFunction.cpp b/lldb/source/Target/ThreadPlanCallFunction.cpp index a3ef537c4b194..445d25177a729 100644 --- a/lldb/source/Target/ThreadPlanCallFunction.cpp +++ b/lldb/source/Target/ThreadPlanCallFunction.cpp @@ -451,10 +451,11 @@ void ThreadPlanCallFunction::SetBreakpoints() { const LazyBool skip_prologue = eLazyBoolNo; const bool is_internal = true; const bool is_hardware = false; + const bool offset_is_insn_count = false; m_error_backstop_bp_sp = m_process.GetTarget().CreateBreakpoint( - &stdlib_module_list, NULL, backstop_name.str().c_str(), - eFunctionNameTypeFull, eLanguageTypeUnknown, 0, skip_prologue, - is_internal, is_hardware); + &stdlib_module_list, NULL, backstop_name.str().c_str(), + eFunctionNameTypeFull, eLanguageTypeUnknown, 0, + offset_is_insn_count, skip_prologue, is_internal, is_hardware); } } } From 4078c201716bebfed81404ce9fd239c21117cd14 Mon Sep 17 00:00:00 2001 From: David Spickett Date: Fri, 8 Aug 2025 09:54:37 +0000 Subject: [PATCH 18/18] [lldb][test] Fix typo in decorator (cherry picked from commit 61d569ffc338834cc81cddf31023de41fd3a6be7) --- lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py b/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py index 1d7bbdb0c5a90..f51056d7020c6 100644 --- a/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py +++ b/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py @@ -127,7 +127,7 @@ def test_readMemory(self): self.continue_to_exit() # Flakey on 32-bit Arm Linux. - @skipif(oslist=["linux"], archs=["arm$"]) + @skipIf(oslist=["linux"], archs=["arm$"]) def test_writeMemory(self): """ Tests the 'writeMemory' request