llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Ely Ronnen (eronnen) <details> <summary>Changes</summary> * Support assembly source breakpoints * Change `sourceReference` to be the load address for simplicity and consistency across threads/frames [Screencast From 2025-05-17 23-57-30.webm](https://github.com/user-attachments/assets/2e7c181d-42c1-4121-8f13-b180c19d0e33) --- Patch is 29.52 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/139969.diff 19 Files Affected: - (modified) lldb/include/lldb/API/SBFileSpec.h (+3) - (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py (+7) - (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py (+10) - (modified) lldb/source/API/SBFileSpec.cpp (+8) - (added) lldb/test/API/tools/lldb-dap/breakpoint-assembly/Makefile (+3) - (added) lldb/test/API/tools/lldb-dap/breakpoint-assembly/TestDAP_breakpointAssembly.py (+42) - (added) lldb/test/API/tools/lldb-dap/breakpoint-assembly/main.c (+14) - (modified) lldb/tools/lldb-dap/Breakpoint.cpp (+26-7) - (modified) lldb/tools/lldb-dap/DAP.h (+5) - (modified) lldb/tools/lldb-dap/Handler/BreakpointLocationsHandler.cpp (+51-17) - (modified) lldb/tools/lldb-dap/Handler/RequestHandler.h (+20) - (modified) lldb/tools/lldb-dap/Handler/SetBreakpointsRequestHandler.cpp (+75-4) - (modified) lldb/tools/lldb-dap/Handler/SourceRequestHandler.cpp (+9-11) - (modified) lldb/tools/lldb-dap/JSONUtils.cpp (+54-21) - (modified) lldb/tools/lldb-dap/JSONUtils.h (+14) - (modified) lldb/tools/lldb-dap/SourceBreakpoint.cpp (+22) - (modified) lldb/tools/lldb-dap/SourceBreakpoint.h (+1) - (modified) lldb/tools/lldb-dap/package-lock.json (+2-2) - (modified) lldb/tools/lldb-dap/package.json (+4-1) ``````````diff 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/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py index 70fd0b0c419db..4a907a5e36901 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 @@ -955,6 +955,13 @@ def request_setBreakpoints(self, file_path, line_array, data=None): """ (dir, base) = os.path.split(file_path) source_dict = {"name": base, "path": file_path} + return self.request_setBreakpoints_with_source(source_dict, line_array, data) + + def request_setBreakpointsAssembly(self, sourceReference, line_array, data=None): + source_dict = {"sourceReference": sourceReference} + return self.request_setBreakpoints_with_source(source_dict, line_array, data) + + def request_setBreakpoints_with_source(self, source_dict, line_array, data=None): args_dict = { "source": source_dict, "sourceModified": False, 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 afdc746ed0d0d..427f66a7da0c8 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 @@ -63,6 +63,16 @@ def set_source_breakpoints(self, source_path, lines, data=None): for breakpoint in breakpoints: breakpoint_ids.append("%i" % (breakpoint["id"])) return breakpoint_ids + + def set_source_breakpoints_assembly(self, source_reference, lines, data=None): + response = self.dap_server.request_setBreakpointsAssembly(source_reference, lines, data) + if response is None: + return [] + breakpoints = response["body"]["breakpoints"] + breakpoint_ids = [] + for breakpoint in breakpoints: + breakpoint_ids.append("%i" % (breakpoint["id"])) + return breakpoint_ids def set_function_breakpoints(self, functions, condition=None, hitCondition=None): """Sets breakpoints by function name given an array of function names 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/test/API/tools/lldb-dap/breakpoint-assembly/Makefile b/lldb/test/API/tools/lldb-dap/breakpoint-assembly/Makefile new file mode 100644 index 0000000000000..10495940055b6 --- /dev/null +++ b/lldb/test/API/tools/lldb-dap/breakpoint-assembly/Makefile @@ -0,0 +1,3 @@ +C_SOURCES := main.c + +include Makefile.rules 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 new file mode 100644 index 0000000000000..ba9df3a18590b --- /dev/null +++ b/lldb/test/API/tools/lldb-dap/breakpoint-assembly/TestDAP_breakpointAssembly.py @@ -0,0 +1,42 @@ +""" +Test lldb-dap setBreakpoints request +""" + + +import dap_server +import shutil +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil +import lldbdap_testcase +import os + + +class TestDAP_setBreakpointsAssembly(lldbdap_testcase.DAPTestCaseBase): + # @skipIfWindows + def test_functionality(self): + """Tests hitting assembly source breakpoints""" + program = self.getBuildArtifact("a.out") + self.build_and_launch(program) + + self.dap_server.request_evaluate( + "`settings set stop-disassembly-display no-debuginfo", context="repl" + ) + + assmebly_func_breakpoints = self.set_function_breakpoints(["assembly_func"]) + self.continue_to_breakpoints(assmebly_func_breakpoints) + + assembly_func_frame = self.get_stackFrames()[0] + self.assertIn( + "sourceReference", + assembly_func_frame.get("source"), + "Expected assembly source frame", + ) + + line = assembly_func_frame["line"] + + # Set an assembly breakpoint in the next line and check that it's hit + assembly_breakpoint_ids = self.set_source_breakpoints_assembly( + assembly_func_frame["source"]["sourceReference"], [line + 1] + ) + self.continue_to_breakpoints(assembly_breakpoint_ids) diff --git a/lldb/test/API/tools/lldb-dap/breakpoint-assembly/main.c b/lldb/test/API/tools/lldb-dap/breakpoint-assembly/main.c new file mode 100644 index 0000000000000..350739006f903 --- /dev/null +++ b/lldb/test/API/tools/lldb-dap/breakpoint-assembly/main.c @@ -0,0 +1,14 @@ +#include <stddef.h> + +__attribute__((nodebug)) int assembly_func(int n) { + n += 1; + n += 2; + n += 3; + + return n; +} + +int main(int argc, char const *argv[]) { + assembly_func(10); + return 0; +} diff --git a/lldb/tools/lldb-dap/Breakpoint.cpp b/lldb/tools/lldb-dap/Breakpoint.cpp index 26d633d1d172e..a54a34e0f936d 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,31 @@ 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 = bp_addr.GetSymbol(); + if (symbol.IsValid()) { + breakpoint.line = + m_bp.GetTarget() + .ReadInstructions(symbol.GetStartAddress(), bp_addr, nullptr) + .GetSize() + + 1; + + breakpoint.source = CreateAssemblySource(m_dap.target, bp_addr); + } + } } return breakpoint; diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 8f24c6cf82924..b0fe265b7bca1 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -169,6 +169,8 @@ struct DAP { Variables variables; lldb::SBBroadcaster broadcaster; llvm::StringMap<SourceBreakpointMap> source_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; @@ -219,6 +221,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..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 "JSONUtils.h" #include "RequestHandler.h" #include <vector> @@ -19,19 +18,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 +101,26 @@ 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::SBAddress address(sourceReference, dap.target); + if (!address.IsValid()) + return; - 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)); - } + lldb::SBSymbol symbol = address.GetSymbol(); + if (!symbol.IsValid()) + return; - return protocol::BreakpointLocationsResponseBody{ - /*breakpoints=*/std::move(breakpoint_locations)}; + // 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, 1); + } } } // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h index e6bccfe12f402..80898d1ee5ef1 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" @@ -232,6 +233,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 { @@ -378,6 +389,15 @@ class SetBreakpointsRequestHandler } llvm::Expected<protocol::SetBreakpointsResponseBody> Run(const protocol::SetBreakpointsArguments &args) const override; + + std::vector<protocol::Breakpoint> SetSourceBreakpoints( + const protocol::Source &source, + const std::optional<std::vector<protocol::SourceBreakpoint>> &breakpoints) + const; + std::vector<protocol::Breakpoint> SetAssemblyBreakpoints( + const protocol::Source &source, + 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..7b401f06e9a85 100644 --- a/lldb/tools/lldb-dap/Handler/SetBreakpointsRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/SetBreakpointsRequestHandler.cpp @@ -23,15 +23,29 @@ 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, args.breakpoints); + else if (source.path) + response_breakpoints = SetSourceBreakpoints(source, args.breakpoints); + + return protocol::SetBreakpointsResponseBody{std::move(response_breakpoints)}; +} + +std::vector<protocol::Breakpoint> +SetBreakpointsRequestHandler::SetSourceBreakpoints( + 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; // "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 +87,64 @@ SetBreakpointsRequestHandler::Run( } } - return protocol::SetBreakpointsResponseBody{std::move(response_breakpoints)}; + return response_breakpoints; +} + +std::vector<protocol::Breakpoint> +SetBreakpointsRequestHandler::SetAssemblyBreakpoints( + 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::SBAddress address(sourceReference, dap.target); + if (!address.IsValid()) + return response_breakpoints; + + lldb::SBSymbol symbol = address.GetSymbol(); + if (!symbol.IsValid()) + return response_breakpoints; // Not yet supporting breakpoints in assembly + // without a valid symbol + + llvm::DenseMap<uint32_t, SourceBreakpoint> request_bps; + if (breakpoints) { + for (const auto &bp : *breakpoints) { + SourceBreakpoint src_bp(dap, bp); + request_bps.try_emplace(src_bp.GetLine(), src_bp); + const auto [iv, inserted] = + 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); + else + iv->getSecond().UpdateBreakpoint(src_bp); + + protocol::Breakpoint response_bp = iv->getSecond().ToProtocolBreakpoint(); + response_bp.source = source; + if (!response_bp.line) + response_bp.line = src_bp.GetLine(); + if (bp.column) + response_bp.column = *bp.column; + 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/Handler/SourceRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/SourceRequestHandler.cpp index 0ddd87881a164..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,26 +36,22 @@ 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)); - ... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/139969 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
