https://github.com/eronnen updated https://github.com/llvm/llvm-project/pull/139969
>From a705fec9e42d209ff64be3588ca74567d4319361 Mon Sep 17 00:00:00 2001 From: Ely Ronnen <elyron...@gmail.com> Date: Sat, 10 May 2025 20:45:17 +0200 Subject: [PATCH 1/5] support assembly in BreakpointLocationsRequestHandler --- .../breakpoint/TestDAP_setBreakpoints.py | 1 - .../TestDAP_setExceptionBreakpoints.py | 1 - .../TestDAP_setFunctionBreakpoints.py | 1 - lldb/tools/lldb-dap/DAP.h | 3 + .../Handler/BreakpointLocationsHandler.cpp | 77 +++++++++++++++---- lldb/tools/lldb-dap/Handler/RequestHandler.h | 11 +++ .../lldb-dap/Handler/SourceRequestHandler.cpp | 4 +- 7 files changed, 76 insertions(+), 22 deletions(-) diff --git a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py index aae1251b17c93..26df2573555df 100644 --- a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py +++ b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py @@ -12,7 +12,6 @@ import os -@skip("Temporarily disable the breakpoint tests") class TestDAP_setBreakpoints(lldbdap_testcase.DAPTestCaseBase): def setUp(self): lldbdap_testcase.DAPTestCaseBase.setUp(self) diff --git a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setExceptionBreakpoints.py b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setExceptionBreakpoints.py index 4dc8c5b3c7ded..92ac66cd44c5d 100644 --- a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setExceptionBreakpoints.py +++ b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setExceptionBreakpoints.py @@ -10,7 +10,6 @@ import lldbdap_testcase -@skip("Temporarily disable the breakpoint tests") class TestDAP_setExceptionBreakpoints(lldbdap_testcase.DAPTestCaseBase): @skipIfWindows def test_functionality(self): diff --git a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setFunctionBreakpoints.py b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setFunctionBreakpoints.py index baaca4d974d5d..946595f639edc 100644 --- a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setFunctionBreakpoints.py +++ b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setFunctionBreakpoints.py @@ -10,7 +10,6 @@ import lldbdap_testcase -@skip("Temporarily disable the breakpoint tests") class TestDAP_setFunctionBreakpoints(lldbdap_testcase.DAPTestCaseBase): @skipIfWindows def test_set_and_clear(self): diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index c1a1130b1e59f..587d15891530e 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -219,6 +219,9 @@ struct DAP { llvm::StringSet<> modules; /// @} + /// Number of lines of assembly code to show when no debug info is available. + uint32_t number_of_assembly_lines_for_nodebug = 32; + /// Creates a new DAP sessions. /// /// \param[in] log diff --git a/lldb/tools/lldb-dap/Handler/BreakpointLocationsHandler.cpp b/lldb/tools/lldb-dap/Handler/BreakpointLocationsHandler.cpp index 2ac886c3a5d2c..9eea549d72b00 100644 --- a/lldb/tools/lldb-dap/Handler/BreakpointLocationsHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/BreakpointLocationsHandler.cpp @@ -7,7 +7,7 @@ //===----------------------------------------------------------------------===// #include "DAP.h" -#include "JSONUtils.h" +#include "LLDBUtils.h" #include "RequestHandler.h" #include <vector> @@ -19,19 +19,50 @@ namespace lldb_dap { llvm::Expected<protocol::BreakpointLocationsResponseBody> BreakpointLocationsRequestHandler::Run( const protocol::BreakpointLocationsArguments &args) const { - std::string path = args.source.path.value_or(""); uint32_t start_line = args.line; uint32_t start_column = args.column.value_or(LLDB_INVALID_COLUMN_NUMBER); uint32_t end_line = args.endLine.value_or(start_line); uint32_t end_column = args.endColumn.value_or(std::numeric_limits<uint32_t>::max()); + // Find all relevant lines & columns + llvm::SmallVector<std::pair<uint32_t, uint32_t>, 8> locations; + if (args.source.sourceReference) { + AddAssemblyBreakpointLocations(locations, *args.source.sourceReference, + start_line, end_line); + } else { + std::string path = args.source.path.value_or(""); + AddSourceBreakpointLocations(locations, std::move(path), start_line, + start_column, end_line, end_column); + } + + // The line entries are sorted by addresses, but we must return the list + // ordered by line / column position. + std::sort(locations.begin(), locations.end()); + locations.erase(llvm::unique(locations), locations.end()); + + std::vector<protocol::BreakpointLocation> breakpoint_locations; + for (auto &l : locations) { + protocol::BreakpointLocation lc; + lc.line = l.first; + lc.column = l.second; + breakpoint_locations.push_back(std::move(lc)); + } + + return protocol::BreakpointLocationsResponseBody{ + /*breakpoints=*/std::move(breakpoint_locations)}; +} + +template <unsigned N> +void BreakpointLocationsRequestHandler::AddSourceBreakpointLocations( + llvm::SmallVector<std::pair<uint32_t, uint32_t>, N> &locations, + std::string path, uint32_t start_line, uint32_t start_column, + uint32_t end_line, uint32_t end_column) const { + lldb::SBFileSpec file_spec(path.c_str(), true); lldb::SBSymbolContextList compile_units = dap.target.FindCompileUnits(file_spec); - // Find all relevant lines & columns - llvm::SmallVector<std::pair<uint32_t, uint32_t>, 8> locations; for (uint32_t c_idx = 0, c_limit = compile_units.GetSize(); c_idx < c_limit; ++c_idx) { const lldb::SBCompileUnit &compile_unit = @@ -71,22 +102,34 @@ BreakpointLocationsRequestHandler::Run( locations.emplace_back(line, column); } } +} - // The line entries are sorted by addresses, but we must return the list - // ordered by line / column position. - std::sort(locations.begin(), locations.end()); - locations.erase(llvm::unique(locations), locations.end()); +template <unsigned N> +void BreakpointLocationsRequestHandler::AddAssemblyBreakpointLocations( + llvm::SmallVector<std::pair<uint32_t, uint32_t>, N> &locations, + int64_t sourceReference, uint32_t start_line, uint32_t end_line) const { + lldb::SBProcess process = dap.target.GetProcess(); + lldb::SBThread thread = + process.GetThreadByIndexID(GetLLDBThreadIndexID(sourceReference)); + lldb::SBFrame frame = thread.GetFrameAtIndex(GetLLDBFrameID(sourceReference)); - std::vector<protocol::BreakpointLocation> breakpoint_locations; - for (auto &l : locations) { - protocol::BreakpointLocation lc; - lc.line = l.first; - lc.column = l.second; - breakpoint_locations.push_back(std::move(lc)); - } + if (!frame.IsValid()) + return; - return protocol::BreakpointLocationsResponseBody{ - /*breakpoints=*/std::move(breakpoint_locations)}; + lldb::SBSymbol symbol = frame.GetSymbol(); + if (symbol.IsValid()) { + lldb::SBInstructionList insts = symbol.GetInstructions(dap.target); + for (uint32_t i = start_line - 1; i < insts.GetSize() && i < (end_line - 1); + ++i) { + locations.emplace_back(i, 0); + } + } else { + for (uint32_t i = start_line - 1; + i < dap.number_of_assembly_lines_for_nodebug && i < (end_line - 1); + ++i) { + locations.emplace_back(i, 0); + } + } } } // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h index 383f9e24a729a..34f2423e01f60 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.h +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h @@ -16,6 +16,7 @@ #include "Protocol/ProtocolRequests.h" #include "Protocol/ProtocolTypes.h" #include "llvm/ADT/DenseSet.h" +#include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Error.h" #include "llvm/Support/JSON.h" @@ -224,6 +225,16 @@ class BreakpointLocationsRequestHandler } llvm::Expected<protocol::BreakpointLocationsResponseBody> Run(const protocol::BreakpointLocationsArguments &args) const override; + + template <unsigned N> + void AddSourceBreakpointLocations( + llvm::SmallVector<std::pair<uint32_t, uint32_t>, N> &locations, + std::string path, uint32_t start_line, uint32_t start_column, + uint32_t end_line, uint32_t end_column) const; + template <unsigned N> + void AddAssemblyBreakpointLocations( + llvm::SmallVector<std::pair<uint32_t, uint32_t>, N> &locations, + int64_t sourceReference, uint32_t start_line, uint32_t end_line) const; }; class CompletionsRequestHandler : public LegacyRequestHandler { diff --git a/lldb/tools/lldb-dap/Handler/SourceRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/SourceRequestHandler.cpp index 0ddd87881a164..fb396a3dc8862 100644 --- a/lldb/tools/lldb-dap/Handler/SourceRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/SourceRequestHandler.cpp @@ -52,8 +52,8 @@ SourceRequestHandler::Run(const protocol::SourceArguments &args) const { insts.GetDescription(stream, exe_ctx); } else { // No valid symbol, just return the disassembly. - lldb::SBInstructionList insts = - dap.target.ReadInstructions(frame.GetPCAddress(), 32); + lldb::SBInstructionList insts = dap.target.ReadInstructions( + frame.GetPCAddress(), dap.number_of_assembly_lines_for_nodebug); insts.GetDescription(stream, exe_ctx); } >From 9e305534dae0d0e21d827cc3e4bb4d302dcaab23 Mon Sep 17 00:00:00 2001 From: Ely Ronnen <elyron...@gmail.com> Date: Sun, 11 May 2025 19:49:03 +0200 Subject: [PATCH 2/5] support assembly in SetBreakpointsRequestHandler --- lldb/tools/lldb-dap/DAP.h | 1 + .../Handler/BreakpointLocationsHandler.cpp | 20 ++--- lldb/tools/lldb-dap/Handler/RequestHandler.h | 9 ++ .../Handler/SetBreakpointsRequestHandler.cpp | 90 ++++++++++++++++++- lldb/tools/lldb-dap/SourceBreakpoint.cpp | 22 +++++ lldb/tools/lldb-dap/SourceBreakpoint.h | 1 + 6 files changed, 127 insertions(+), 16 deletions(-) diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 587d15891530e..0bc9063e1266f 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -169,6 +169,7 @@ struct DAP { Variables variables; lldb::SBBroadcaster broadcaster; llvm::StringMap<SourceBreakpointMap> source_breakpoints; + llvm::DenseMap<int64_t, SourceBreakpointMap> assembly_breakpoints; FunctionBreakpointMap function_breakpoints; InstructionBreakpointMap instruction_breakpoints; std::optional<std::vector<ExceptionBreakpoint>> exception_breakpoints; diff --git a/lldb/tools/lldb-dap/Handler/BreakpointLocationsHandler.cpp b/lldb/tools/lldb-dap/Handler/BreakpointLocationsHandler.cpp index 9eea549d72b00..be02c47056310 100644 --- a/lldb/tools/lldb-dap/Handler/BreakpointLocationsHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/BreakpointLocationsHandler.cpp @@ -117,18 +117,14 @@ void BreakpointLocationsRequestHandler::AddAssemblyBreakpointLocations( return; lldb::SBSymbol symbol = frame.GetSymbol(); - if (symbol.IsValid()) { - lldb::SBInstructionList insts = symbol.GetInstructions(dap.target); - for (uint32_t i = start_line - 1; i < insts.GetSize() && i < (end_line - 1); - ++i) { - locations.emplace_back(i, 0); - } - } else { - for (uint32_t i = start_line - 1; - i < dap.number_of_assembly_lines_for_nodebug && i < (end_line - 1); - ++i) { - locations.emplace_back(i, 0); - } + if (!symbol.IsValid()) + return; + + // start_line is relative to the symbol's start address + lldb::SBInstructionList insts = symbol.GetInstructions(dap.target); + for (uint32_t i = start_line - 1; i < insts.GetSize() && i < (end_line - 1); + ++i) { + locations.emplace_back(i, 0); } } diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h index 34f2423e01f60..ddd01ffe33823 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.h +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h @@ -379,6 +379,15 @@ class SetBreakpointsRequestHandler } llvm::Expected<protocol::SetBreakpointsResponseBody> Run(const protocol::SetBreakpointsArguments &args) const override; + + std::vector<protocol::Breakpoint> SetSourceBreakpoints( + const std::string &path, + const std::optional<std::vector<protocol::SourceBreakpoint>> &breakpoints) + const; + std::vector<protocol::Breakpoint> SetAssemblyBreakpoints( + int64_t sourceReference, + const std::optional<std::vector<protocol::SourceBreakpoint>> &breakpoints) + const; }; class SetExceptionBreakpointsRequestHandler : public LegacyRequestHandler { diff --git a/lldb/tools/lldb-dap/Handler/SetBreakpointsRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/SetBreakpointsRequestHandler.cpp index 86e090b66afe9..71f9e5578ef08 100644 --- a/lldb/tools/lldb-dap/Handler/SetBreakpointsRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/SetBreakpointsRequestHandler.cpp @@ -9,8 +9,11 @@ #include "DAP.h" #include "EventHelper.h" #include "JSONUtils.h" +#include "LLDBUtils.h" #include "Protocol/ProtocolRequests.h" #include "RequestHandler.h" +#include <cstdint> +#include <utility> #include <vector> namespace lldb_dap { @@ -23,15 +26,30 @@ llvm::Expected<protocol::SetBreakpointsResponseBody> SetBreakpointsRequestHandler::Run( const protocol::SetBreakpointsArguments &args) const { const auto &source = args.source; - const auto path = source.path.value_or(""); + std::vector<protocol::Breakpoint> response_breakpoints; + if (source.sourceReference) + response_breakpoints = SetAssemblyBreakpoints( + source.sourceReference.value(), args.breakpoints); + else if (source.path) + response_breakpoints = + SetSourceBreakpoints(source.path.value(), args.breakpoints); + + return protocol::SetBreakpointsResponseBody{std::move(response_breakpoints)}; +} + +std::vector<protocol::Breakpoint> +SetBreakpointsRequestHandler::SetSourceBreakpoints( + const std::string &path, + const std::optional<std::vector<protocol::SourceBreakpoint>> &breakpoints) + const { std::vector<protocol::Breakpoint> response_breakpoints; // Decode the source breakpoint infos for this "setBreakpoints" request SourceBreakpointMap request_bps; // "breakpoints" may be unset, in which case we treat it the same as being set // to an empty array. - if (args.breakpoints) { - for (const auto &bp : *args.breakpoints) { + if (breakpoints) { + for (const auto &bp : *breakpoints) { SourceBreakpoint src_bp(dap, bp); std::pair<uint32_t, uint32_t> bp_pos(src_bp.GetLine(), src_bp.GetColumn()); @@ -73,7 +91,71 @@ SetBreakpointsRequestHandler::Run( } } - return protocol::SetBreakpointsResponseBody{std::move(response_breakpoints)}; + return response_breakpoints; +} + +std::vector<protocol::Breakpoint> +SetBreakpointsRequestHandler::SetAssemblyBreakpoints( + int64_t sourceReference, + const std::optional<std::vector<protocol::SourceBreakpoint>> &breakpoints) + const { + std::vector<protocol::Breakpoint> response_breakpoints; + + lldb::SBProcess process = dap.target.GetProcess(); + lldb::SBThread thread = + process.GetThreadByIndexID(GetLLDBThreadIndexID(sourceReference)); + lldb::SBFrame frame = thread.GetFrameAtIndex(GetLLDBFrameID(sourceReference)); + + if (!frame.IsValid()) + return response_breakpoints; + + lldb::SBSymbol symbol = frame.GetSymbol(); + if (!symbol.IsValid()) + return response_breakpoints; // Not yet supporting breakpoints in assembly + // without a valid symbol + + SourceBreakpointMap request_bps; + if (breakpoints) { + for (const auto &bp : *breakpoints) { + SourceBreakpoint src_bp(dap, bp); + std::pair<uint32_t, uint32_t> bp_pos(src_bp.GetLine(), 0); + request_bps.try_emplace(bp_pos, src_bp); + const auto [iv, inserted] = + dap.assembly_breakpoints[sourceReference].try_emplace(bp_pos, src_bp); + // We check if this breakpoint already exists to update it + if (inserted) + iv->getSecond().SetBreakpoint(symbol); + else + iv->getSecond().UpdateBreakpoint(src_bp); + + protocol::Breakpoint response_bp = iv->getSecond().ToProtocolBreakpoint(); + protocol::Source source; + source.sourceReference = sourceReference; + source.name = symbol.GetName(); + response_bp.source = std::move(source); + + if (!response_bp.line) + response_bp.line = src_bp.GetLine(); + if (!response_bp.column) + response_bp.column = src_bp.GetColumn(); + response_breakpoints.push_back(response_bp); + } + } + + // Delete existing breakpoints for this sourceReference that are not in the + // request_bps set. + auto old_src_bp_pos = dap.assembly_breakpoints.find(sourceReference); + if (old_src_bp_pos != dap.assembly_breakpoints.end()) { + for (auto &old_bp : old_src_bp_pos->second) { + auto request_pos = request_bps.find(old_bp.first); + if (request_pos == request_bps.end()) { + dap.target.BreakpointDelete(old_bp.second.GetID()); + old_src_bp_pos->second.erase(old_bp.first); + } + } + } + + return response_breakpoints; } } // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/SourceBreakpoint.cpp b/lldb/tools/lldb-dap/SourceBreakpoint.cpp index 4581c995b4260..938b8fb8bcdda 100644 --- a/lldb/tools/lldb-dap/SourceBreakpoint.cpp +++ b/lldb/tools/lldb-dap/SourceBreakpoint.cpp @@ -13,7 +13,9 @@ #include "lldb/API/SBBreakpoint.h" #include "lldb/API/SBFileSpecList.h" #include "lldb/API/SBFrame.h" +#include "lldb/API/SBInstruction.h" #include "lldb/API/SBMutex.h" +#include "lldb/API/SBSymbol.h" #include "lldb/API/SBTarget.h" #include "lldb/API/SBThread.h" #include "lldb/API/SBValue.h" @@ -45,6 +47,26 @@ void SourceBreakpoint::SetBreakpoint(const llvm::StringRef source_path) { Breakpoint::SetBreakpoint(); } +void SourceBreakpoint::SetBreakpoint(lldb::SBSymbol &symbol) { + lldb::SBMutex lock = m_dap.GetAPIMutex(); + std::lock_guard<lldb::SBMutex> guard(lock); + + if (m_line == 0) + return; + + lldb::SBInstructionList inst_list = + m_dap.target.ReadInstructions(symbol.GetStartAddress(), m_line); + if (inst_list.GetSize() < m_line) + return; + lldb::SBAddress address = + inst_list.GetInstructionAtIndex(m_line - 1).GetAddress(); + + m_bp = m_dap.target.BreakpointCreateBySBAddress(address); + if (!m_log_message.empty()) + SetLogMessage(); + Breakpoint::SetBreakpoint(); +} + void SourceBreakpoint::UpdateBreakpoint(const SourceBreakpoint &request_bp) { if (m_log_message != request_bp.m_log_message) { m_log_message = request_bp.m_log_message; diff --git a/lldb/tools/lldb-dap/SourceBreakpoint.h b/lldb/tools/lldb-dap/SourceBreakpoint.h index 5b15296f861c5..8589800e50983 100644 --- a/lldb/tools/lldb-dap/SourceBreakpoint.h +++ b/lldb/tools/lldb-dap/SourceBreakpoint.h @@ -26,6 +26,7 @@ class SourceBreakpoint : public Breakpoint { // Set this breakpoint in LLDB as a new breakpoint void SetBreakpoint(const llvm::StringRef source_path); + void SetBreakpoint(lldb::SBSymbol &symbol); void UpdateBreakpoint(const SourceBreakpoint &request_bp); void SetLogMessage(); >From cb47dbc97fbbc1e2db9bd9c8989dcce425c796ee Mon Sep 17 00:00:00 2001 From: Ely Ronnen <elyron...@gmail.com> Date: Wed, 14 May 2025 23:51:41 +0200 Subject: [PATCH 3/5] fix resolving of assembly source breakpoints --- lldb/tools/lldb-dap/Breakpoint.cpp | 45 ++++++++++++++++--- lldb/tools/lldb-dap/DAP.h | 3 +- .../Handler/BreakpointLocationsHandler.cpp | 4 +- lldb/tools/lldb-dap/Handler/RequestHandler.h | 4 +- .../Handler/SetBreakpointsRequestHandler.cpp | 30 ++++++------- lldb/tools/lldb-dap/package-lock.json | 4 +- lldb/tools/lldb-dap/package.json | 5 ++- 7 files changed, 63 insertions(+), 32 deletions(-) diff --git a/lldb/tools/lldb-dap/Breakpoint.cpp b/lldb/tools/lldb-dap/Breakpoint.cpp index 26d633d1d172e..87fcd15b0a568 100644 --- a/lldb/tools/lldb-dap/Breakpoint.cpp +++ b/lldb/tools/lldb-dap/Breakpoint.cpp @@ -9,10 +9,12 @@ #include "Breakpoint.h" #include "DAP.h" #include "JSONUtils.h" +#include "LLDBUtils.h" #include "lldb/API/SBAddress.h" #include "lldb/API/SBBreakpointLocation.h" #include "lldb/API/SBLineEntry.h" #include "lldb/API/SBMutex.h" +#include "lldb/lldb-enumerations.h" #include "llvm/ADT/StringExtras.h" #include <cstddef> #include <cstdint> @@ -63,14 +65,43 @@ protocol::Breakpoint Breakpoint::ToProtocolBreakpoint() { std::string formatted_addr = "0x" + llvm::utohexstr(bp_addr.GetLoadAddress(m_bp.GetTarget())); breakpoint.instructionReference = formatted_addr; + + lldb::StopDisassemblyType stop_disassembly_display = + GetStopDisassemblyDisplay(m_dap.debugger); auto line_entry = bp_addr.GetLineEntry(); - const auto line = line_entry.GetLine(); - if (line != UINT32_MAX) - breakpoint.line = line; - const auto column = line_entry.GetColumn(); - if (column != 0) - breakpoint.column = column; - breakpoint.source = CreateSource(line_entry); + if (!ShouldDisplayAssemblySource(line_entry, stop_disassembly_display)) { + const auto line = line_entry.GetLine(); + if (line != UINT32_MAX) + breakpoint.line = line; + const auto column = line_entry.GetColumn(); + if (column != 0) + breakpoint.column = column; + breakpoint.source = CreateSource(line_entry); + } else { + // Breakpoint made by assembly + auto symbol_context = bp_addr.GetSymbolContext( + lldb::eSymbolContextSymbol | lldb::eSymbolContextModule); + if (symbol_context.IsValid()) { + auto symbol = symbol_context.GetSymbol(); + breakpoint.line = + m_bp.GetTarget() + .ReadInstructions(symbol.GetStartAddress(), bp_addr, nullptr) + .GetSize() + + 1; + protocol::Source source; + source.name = symbol.GetName(); + + auto module = symbol_context.GetModule(); + if (module.IsValid()) { + std::string path = module.GetFileSpec().GetDirectory(); + path += "/"; + path += module.GetFileSpec().GetFilename(); + source.path = std::move(path); + } + + breakpoint.source = std::move(source); + } + } } return breakpoint; diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 0bc9063e1266f..54b233077f0c3 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -169,7 +169,8 @@ struct DAP { Variables variables; lldb::SBBroadcaster broadcaster; llvm::StringMap<SourceBreakpointMap> source_breakpoints; - llvm::DenseMap<int64_t, SourceBreakpointMap> assembly_breakpoints; + llvm::DenseMap<int64_t, llvm::DenseMap<uint32_t, SourceBreakpoint>> + assembly_breakpoints; FunctionBreakpointMap function_breakpoints; InstructionBreakpointMap instruction_breakpoints; std::optional<std::vector<ExceptionBreakpoint>> exception_breakpoints; diff --git a/lldb/tools/lldb-dap/Handler/BreakpointLocationsHandler.cpp b/lldb/tools/lldb-dap/Handler/BreakpointLocationsHandler.cpp index be02c47056310..06ada47a6f27f 100644 --- a/lldb/tools/lldb-dap/Handler/BreakpointLocationsHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/BreakpointLocationsHandler.cpp @@ -122,9 +122,9 @@ void BreakpointLocationsRequestHandler::AddAssemblyBreakpointLocations( // start_line is relative to the symbol's start address lldb::SBInstructionList insts = symbol.GetInstructions(dap.target); - for (uint32_t i = start_line - 1; i < insts.GetSize() && i < (end_line - 1); + for (uint32_t i = start_line - 1; i < insts.GetSize() && i <= (end_line - 1); ++i) { - locations.emplace_back(i, 0); + locations.emplace_back(i, 1); } } diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h index ddd01ffe33823..fec304979995e 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.h +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h @@ -381,11 +381,11 @@ class SetBreakpointsRequestHandler Run(const protocol::SetBreakpointsArguments &args) const override; std::vector<protocol::Breakpoint> SetSourceBreakpoints( - const std::string &path, + const protocol::Source &source, const std::optional<std::vector<protocol::SourceBreakpoint>> &breakpoints) const; std::vector<protocol::Breakpoint> SetAssemblyBreakpoints( - int64_t sourceReference, + const protocol::Source &source, const std::optional<std::vector<protocol::SourceBreakpoint>> &breakpoints) const; }; diff --git a/lldb/tools/lldb-dap/Handler/SetBreakpointsRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/SetBreakpointsRequestHandler.cpp index 71f9e5578ef08..4fefd8b440c7d 100644 --- a/lldb/tools/lldb-dap/Handler/SetBreakpointsRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/SetBreakpointsRequestHandler.cpp @@ -28,21 +28,20 @@ SetBreakpointsRequestHandler::Run( const auto &source = args.source; std::vector<protocol::Breakpoint> response_breakpoints; if (source.sourceReference) - response_breakpoints = SetAssemblyBreakpoints( - source.sourceReference.value(), args.breakpoints); + response_breakpoints = SetAssemblyBreakpoints(source, args.breakpoints); else if (source.path) - response_breakpoints = - SetSourceBreakpoints(source.path.value(), args.breakpoints); + response_breakpoints = SetSourceBreakpoints(source, args.breakpoints); return protocol::SetBreakpointsResponseBody{std::move(response_breakpoints)}; } std::vector<protocol::Breakpoint> SetBreakpointsRequestHandler::SetSourceBreakpoints( - const std::string &path, + const protocol::Source &source, const std::optional<std::vector<protocol::SourceBreakpoint>> &breakpoints) const { std::vector<protocol::Breakpoint> response_breakpoints; + std::string path = source.path.value_or(""); // Decode the source breakpoint infos for this "setBreakpoints" request SourceBreakpointMap request_bps; @@ -96,10 +95,11 @@ SetBreakpointsRequestHandler::SetSourceBreakpoints( std::vector<protocol::Breakpoint> SetBreakpointsRequestHandler::SetAssemblyBreakpoints( - int64_t sourceReference, + const protocol::Source &source, const std::optional<std::vector<protocol::SourceBreakpoint>> &breakpoints) const { std::vector<protocol::Breakpoint> response_breakpoints; + int64_t sourceReference = source.sourceReference.value_or(0); lldb::SBProcess process = dap.target.GetProcess(); lldb::SBThread thread = @@ -114,14 +114,14 @@ SetBreakpointsRequestHandler::SetAssemblyBreakpoints( return response_breakpoints; // Not yet supporting breakpoints in assembly // without a valid symbol - SourceBreakpointMap request_bps; + llvm::DenseMap<uint32_t, SourceBreakpoint> request_bps; if (breakpoints) { for (const auto &bp : *breakpoints) { SourceBreakpoint src_bp(dap, bp); - std::pair<uint32_t, uint32_t> bp_pos(src_bp.GetLine(), 0); - request_bps.try_emplace(bp_pos, src_bp); + request_bps.try_emplace(src_bp.GetLine(), src_bp); const auto [iv, inserted] = - dap.assembly_breakpoints[sourceReference].try_emplace(bp_pos, src_bp); + dap.assembly_breakpoints[sourceReference].try_emplace( + src_bp.GetLine(), src_bp); // We check if this breakpoint already exists to update it if (inserted) iv->getSecond().SetBreakpoint(symbol); @@ -129,15 +129,11 @@ SetBreakpointsRequestHandler::SetAssemblyBreakpoints( iv->getSecond().UpdateBreakpoint(src_bp); protocol::Breakpoint response_bp = iv->getSecond().ToProtocolBreakpoint(); - protocol::Source source; - source.sourceReference = sourceReference; - source.name = symbol.GetName(); - response_bp.source = std::move(source); - + response_bp.source = source; if (!response_bp.line) response_bp.line = src_bp.GetLine(); - if (!response_bp.column) - response_bp.column = src_bp.GetColumn(); + if (bp.column) + response_bp.column = *bp.column; response_breakpoints.push_back(response_bp); } } diff --git a/lldb/tools/lldb-dap/package-lock.json b/lldb/tools/lldb-dap/package-lock.json index 0a2b9e764067e..af90a9573aee6 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.13", + "version": "0.2.14", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "lldb-dap", - "version": "0.2.13", + "version": "0.2.14", "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 d5ca604798799..73e70cd961f4f 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.13", + "version": "0.2.14", "publisher": "llvm-vs-code-extensions", "homepage": "https://lldb.llvm.org", "description": "Debugging with LLDB in Visual Studio Code", @@ -265,6 +265,9 @@ ] }, "breakpoints": [ + { + "language": "lldb.disassembly" + }, { "language": "ada" }, >From 6e1135deac7c1a58a85fc6f25a032d4c61e09d66 Mon Sep 17 00:00:00 2001 From: Ely Ronnen <elyron...@gmail.com> Date: Thu, 15 May 2025 00:36:10 +0200 Subject: [PATCH 4/5] remove include --- lldb/tools/lldb-dap/Handler/SetBreakpointsRequestHandler.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/lldb/tools/lldb-dap/Handler/SetBreakpointsRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/SetBreakpointsRequestHandler.cpp index 4fefd8b440c7d..d69da5bd02c1e 100644 --- a/lldb/tools/lldb-dap/Handler/SetBreakpointsRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/SetBreakpointsRequestHandler.cpp @@ -12,8 +12,6 @@ #include "LLDBUtils.h" #include "Protocol/ProtocolRequests.h" #include "RequestHandler.h" -#include <cstdint> -#include <utility> #include <vector> namespace lldb_dap { >From 3807a12724feb3fb2dc9b46db20611e8813cb77b Mon Sep 17 00:00:00 2001 From: Ely Ronnen <elyron...@gmail.com> Date: Sat, 17 May 2025 21:05:27 +0200 Subject: [PATCH 5/5] use load address as sourceReference --- lldb/include/lldb/API/SBFileSpec.h | 3 + lldb/source/API/SBFileSpec.cpp | 8 ++ lldb/tools/lldb-dap/Breakpoint.cpp | 18 +---- .../Handler/BreakpointLocationsHandler.cpp | 11 +-- .../Handler/SetBreakpointsRequestHandler.cpp | 11 +-- .../lldb-dap/Handler/SourceRequestHandler.cpp | 18 ++--- lldb/tools/lldb-dap/JSONUtils.cpp | 75 +++++++++++++------ lldb/tools/lldb-dap/JSONUtils.h | 14 ++++ 8 files changed, 96 insertions(+), 62 deletions(-) diff --git a/lldb/include/lldb/API/SBFileSpec.h b/lldb/include/lldb/API/SBFileSpec.h index 36641843aabeb..303cb7d712cbf 100644 --- a/lldb/include/lldb/API/SBFileSpec.h +++ b/lldb/include/lldb/API/SBFileSpec.h @@ -10,6 +10,7 @@ #define LLDB_API_SBFILESPEC_H #include "lldb/API/SBDefines.h" +#include "lldb/API/SBStream.h" namespace lldb { @@ -53,6 +54,8 @@ class LLDB_API SBFileSpec { uint32_t GetPath(char *dst_path, size_t dst_len) const; + bool GetPath(lldb::SBStream &dst_path) const; + static int ResolvePath(const char *src_path, char *dst_path, size_t dst_len); bool GetDescription(lldb::SBStream &description) const; diff --git a/lldb/source/API/SBFileSpec.cpp b/lldb/source/API/SBFileSpec.cpp index a7df9afc4b8eb..cb44dac1d4fcc 100644 --- a/lldb/source/API/SBFileSpec.cpp +++ b/lldb/source/API/SBFileSpec.cpp @@ -19,6 +19,7 @@ #include <cinttypes> #include <climits> +#include <string> using namespace lldb; using namespace lldb_private; @@ -147,6 +148,13 @@ uint32_t SBFileSpec::GetPath(char *dst_path, size_t dst_len) const { return result; } +bool SBFileSpec::GetPath(SBStream &dst_path) const { + LLDB_INSTRUMENT_VA(this, dst_path); + + std::string path = m_opaque_up->GetPath(); + return dst_path->PutCString(path.c_str()) > 0; +} + const lldb_private::FileSpec *SBFileSpec::operator->() const { return m_opaque_up.get(); } diff --git a/lldb/tools/lldb-dap/Breakpoint.cpp b/lldb/tools/lldb-dap/Breakpoint.cpp index 87fcd15b0a568..a54a34e0f936d 100644 --- a/lldb/tools/lldb-dap/Breakpoint.cpp +++ b/lldb/tools/lldb-dap/Breakpoint.cpp @@ -79,27 +79,15 @@ protocol::Breakpoint Breakpoint::ToProtocolBreakpoint() { breakpoint.source = CreateSource(line_entry); } else { // Breakpoint made by assembly - auto symbol_context = bp_addr.GetSymbolContext( - lldb::eSymbolContextSymbol | lldb::eSymbolContextModule); - if (symbol_context.IsValid()) { - auto symbol = symbol_context.GetSymbol(); + auto symbol = bp_addr.GetSymbol(); + if (symbol.IsValid()) { breakpoint.line = m_bp.GetTarget() .ReadInstructions(symbol.GetStartAddress(), bp_addr, nullptr) .GetSize() + 1; - protocol::Source source; - source.name = symbol.GetName(); - auto module = symbol_context.GetModule(); - if (module.IsValid()) { - std::string path = module.GetFileSpec().GetDirectory(); - path += "/"; - path += module.GetFileSpec().GetFilename(); - source.path = std::move(path); - } - - breakpoint.source = std::move(source); + breakpoint.source = CreateAssemblySource(m_dap.target, bp_addr); } } } diff --git a/lldb/tools/lldb-dap/Handler/BreakpointLocationsHandler.cpp b/lldb/tools/lldb-dap/Handler/BreakpointLocationsHandler.cpp index 06ada47a6f27f..c4d658caeee2d 100644 --- a/lldb/tools/lldb-dap/Handler/BreakpointLocationsHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/BreakpointLocationsHandler.cpp @@ -7,7 +7,6 @@ //===----------------------------------------------------------------------===// #include "DAP.h" -#include "LLDBUtils.h" #include "RequestHandler.h" #include <vector> @@ -108,15 +107,11 @@ template <unsigned N> void BreakpointLocationsRequestHandler::AddAssemblyBreakpointLocations( llvm::SmallVector<std::pair<uint32_t, uint32_t>, N> &locations, int64_t sourceReference, uint32_t start_line, uint32_t end_line) const { - lldb::SBProcess process = dap.target.GetProcess(); - lldb::SBThread thread = - process.GetThreadByIndexID(GetLLDBThreadIndexID(sourceReference)); - lldb::SBFrame frame = thread.GetFrameAtIndex(GetLLDBFrameID(sourceReference)); - - if (!frame.IsValid()) + lldb::SBAddress address(sourceReference, dap.target); + if (!address.IsValid()) return; - lldb::SBSymbol symbol = frame.GetSymbol(); + lldb::SBSymbol symbol = address.GetSymbol(); if (!symbol.IsValid()) return; diff --git a/lldb/tools/lldb-dap/Handler/SetBreakpointsRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/SetBreakpointsRequestHandler.cpp index d69da5bd02c1e..7b401f06e9a85 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 "EventHelper.h" #include "JSONUtils.h" -#include "LLDBUtils.h" #include "Protocol/ProtocolRequests.h" #include "RequestHandler.h" #include <vector> @@ -99,15 +98,11 @@ SetBreakpointsRequestHandler::SetAssemblyBreakpoints( std::vector<protocol::Breakpoint> response_breakpoints; int64_t sourceReference = source.sourceReference.value_or(0); - lldb::SBProcess process = dap.target.GetProcess(); - lldb::SBThread thread = - process.GetThreadByIndexID(GetLLDBThreadIndexID(sourceReference)); - lldb::SBFrame frame = thread.GetFrameAtIndex(GetLLDBFrameID(sourceReference)); - - if (!frame.IsValid()) + lldb::SBAddress address(sourceReference, dap.target); + if (!address.IsValid()) return response_breakpoints; - lldb::SBSymbol symbol = frame.GetSymbol(); + lldb::SBSymbol symbol = address.GetSymbol(); if (!symbol.IsValid()) return response_breakpoints; // Not yet supporting breakpoints in assembly // without a valid symbol diff --git a/lldb/tools/lldb-dap/Handler/SourceRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/SourceRequestHandler.cpp index fb396a3dc8862..9249e2aa6fef7 100644 --- a/lldb/tools/lldb-dap/Handler/SourceRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/SourceRequestHandler.cpp @@ -11,6 +11,7 @@ #include "LLDBUtils.h" #include "Protocol/ProtocolRequests.h" #include "Protocol/ProtocolTypes.h" +#include "lldb/API/SBAddress.h" #include "lldb/API/SBExecutionContext.h" #include "lldb/API/SBFrame.h" #include "lldb/API/SBInstructionList.h" @@ -19,6 +20,7 @@ #include "lldb/API/SBSymbol.h" #include "lldb/API/SBTarget.h" #include "lldb/API/SBThread.h" +#include "lldb/lldb-types.h" #include "llvm/Support/Error.h" namespace lldb_dap { @@ -34,18 +36,14 @@ SourceRequestHandler::Run(const protocol::SourceArguments &args) const { return llvm::make_error<DAPError>( "invalid arguments, expected source.sourceReference to be set"); - lldb::SBProcess process = dap.target.GetProcess(); - // Upper 32 bits is the thread index ID - lldb::SBThread thread = - process.GetThreadByIndexID(GetLLDBThreadIndexID(source)); - // Lower 32 bits is the frame index - lldb::SBFrame frame = thread.GetFrameAtIndex(GetLLDBFrameID(source)); - if (!frame.IsValid()) + lldb::SBAddress address(source, dap.target); + if (!address.IsValid()) return llvm::make_error<DAPError>("source not found"); + lldb::SBSymbol symbol = address.GetSymbol(); + lldb::SBStream stream; - lldb::SBExecutionContext exe_ctx(frame); - lldb::SBSymbol symbol = frame.GetSymbol(); + lldb::SBExecutionContext exe_ctx(dap.target); if (symbol.IsValid()) { lldb::SBInstructionList insts = symbol.GetInstructions(dap.target); @@ -53,7 +51,7 @@ SourceRequestHandler::Run(const protocol::SourceArguments &args) const { } else { // No valid symbol, just return the disassembly. lldb::SBInstructionList insts = dap.target.ReadInstructions( - frame.GetPCAddress(), dap.number_of_assembly_lines_for_nodebug); + address, dap.number_of_assembly_lines_for_nodebug); insts.GetDescription(stream, exe_ctx); } diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp index a8bd672583a5d..54e45cfc3319c 100644 --- a/lldb/tools/lldb-dap/JSONUtils.cpp +++ b/lldb/tools/lldb-dap/JSONUtils.cpp @@ -490,6 +490,13 @@ CreateExceptionBreakpointFilter(const ExceptionBreakpoint &bp) { return filter; } +static std::string GetLoadAddressString(const lldb::addr_t addr) { + std::string result; + llvm::raw_string_ostream os(result); + os << llvm::format_hex(addr, 18); + return result; +} + protocol::Source CreateSource(const lldb::SBFileSpec &file) { protocol::Source source; if (file.IsValid()) { @@ -516,6 +523,43 @@ protocol::Source CreateSource(llvm::StringRef source_path) { return source; } +protocol::Source CreateAssemblySource(const lldb::SBTarget &target, + lldb::SBAddress &address) { + protocol::Source source; + + auto symbol = address.GetSymbol(); + std::string name; + if (symbol.IsValid()) { + source.sourceReference = symbol.GetStartAddress().GetLoadAddress(target); + name = symbol.GetName(); + } else { + const auto load_addr = address.GetLoadAddress(target); + source.sourceReference = load_addr; + name = GetLoadAddressString(load_addr); + } + + lldb::SBModule module = address.GetModule(); + if (module.IsValid()) { + lldb::SBFileSpec file_spec = module.GetFileSpec(); + if (file_spec.IsValid()) { + lldb::SBStream module_path; + if (file_spec.GetPath(module_path)) { + std::string path = module_path.GetData(); + source.path = path + '`' + name; + } + } + } + + source.name = std::move(name); + + // Mark the source as deemphasized since users will only be able to view + // assembly for these frames. + source.presentationHint = + protocol::Source::PresentationHint::eSourcePresentationHintDeemphasize; + + return source; +} + bool ShouldDisplayAssemblySource( const lldb::SBLineEntry &line_entry, lldb::StopDisassemblyType stop_disassembly_display) { @@ -619,12 +663,10 @@ CreateStackFrame(lldb::SBFrame &frame, lldb::SBFormat &format, frame_name = name; } - if (frame_name.empty()) { + if (frame_name.empty()) // If the function name is unavailable, display the pc address as a 16-digit // hex string, e.g. "0x0000000000012345" - llvm::raw_string_ostream os(frame_name); - os << llvm::format_hex(frame.GetPC(), 18); - } + frame_name = GetLoadAddressString(frame.GetPC()); // We only include `[opt]` if a custom frame format is not specified. if (!format && frame.GetFunction().GetIsOptimized()) @@ -641,17 +683,10 @@ CreateStackFrame(lldb::SBFrame &frame, lldb::SBFormat &format, } else if (frame.GetSymbol().IsValid()) { // If no source is associated with the frame, use the DAPFrameID to track // the 'source' and generate assembly. - llvm::json::Object source; - EmplaceSafeString(source, "name", frame_name); - char buf[PATH_MAX] = {0}; - size_t size = frame.GetModule().GetFileSpec().GetPath(buf, PATH_MAX); - EmplaceSafeString(source, "path", - std::string(buf, size) + '`' + frame_name); - source.try_emplace("sourceReference", MakeDAPFrameID(frame)); - // Mark the source as deemphasized since users will only be able to view - // assembly for these frames. - EmplaceSafeString(source, "presentationHint", "deemphasize"); - object.try_emplace("source", std::move(source)); + auto frame_address = frame.GetPCAddress(); + object.try_emplace("source", CreateAssemblySource( + frame.GetThread().GetProcess().GetTarget(), + frame_address)); // Calculate the line of the current PC from the start of the current // symbol. @@ -665,12 +700,10 @@ CreateStackFrame(lldb::SBFrame &frame, lldb::SBFormat &format, object.try_emplace("column", 1); } else { // No valid line entry or symbol. - llvm::json::Object source; - EmplaceSafeString(source, "name", frame_name); - source.try_emplace("sourceReference", MakeDAPFrameID(frame)); - EmplaceSafeString(source, "presentationHint", "deemphasize"); - object.try_emplace("source", std::move(source)); - + auto frame_address = frame.GetPCAddress(); + object.try_emplace("source", CreateAssemblySource( + frame.GetThread().GetProcess().GetTarget(), + frame_address)); object.try_emplace("line", 1); object.try_emplace("column", 1); } diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h index 783f291338d8c..ac9b39739104f 100644 --- a/lldb/tools/lldb-dap/JSONUtils.h +++ b/lldb/tools/lldb-dap/JSONUtils.h @@ -269,6 +269,20 @@ protocol::Source CreateSource(const lldb::SBLineEntry &line_entry); /// definition outlined by Microsoft. protocol::Source CreateSource(llvm::StringRef source_path); +/// Create a "Source" object for a given frame, using its assembly for source. +/// +/// \param[in] target +/// The relevant target. +/// +/// \param[in] address +/// The address to use when creating the "Source" object. +/// +/// \return +/// A "Source" JSON object that follows the formal JSON +/// definition outlined by Microsoft. +protocol::Source CreateAssemblySource(const lldb::SBTarget &target, + lldb::SBAddress &address); + /// Return true if the given line entry should be displayed as assembly. /// /// \param[in] line_entry _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits