https://github.com/eronnen updated https://github.com/llvm/llvm-project/pull/139969
>From ae324862f516ebb5be3206485476b586b22488b7 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/4] 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 eaebaf6619bbd..89faced78c847 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" @@ -217,6 +218,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 e57b4c00c1669c5fa25ce1ecb097e7d6b475f685 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/4] 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 89faced78c847..cf12828aef8fc 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.h +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h @@ -372,6 +372,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 53ac2b4eb7b1afaca28aca3ca5c26300586a55f4 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/4] 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 cf12828aef8fc..1ad6069c20b4b 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.h +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h @@ -374,11 +374,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 226363193267986cee34c130baa125749ae0a2f9 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/4] 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 { _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits