llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Ely Ronnen (eronnen) <details> <summary>Changes</summary> - 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` --- Patch is 42.14 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/148061.diff 23 Files Affected: - (modified) lldb/include/lldb/API/SBTarget.h (+6) - (modified) lldb/include/lldb/Breakpoint/BreakpointResolver.h (+7-3) - (modified) lldb/include/lldb/Breakpoint/BreakpointResolverAddress.h (+4) - (modified) lldb/include/lldb/Core/Disassembler.h (+2) - (modified) lldb/include/lldb/Target/Target.h (+10-5) - (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py (+34-5) - (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py (+10-12) - (modified) lldb/source/API/SBTarget.cpp (+20-23) - (modified) lldb/source/Breakpoint/BreakpointResolver.cpp (+18-6) - (modified) lldb/source/Breakpoint/BreakpointResolverAddress.cpp (+13) - (modified) lldb/source/Core/Disassembler.cpp (+10) - (modified) lldb/source/Target/Target.cpp (+33-5) - (modified) lldb/test/API/tools/lldb-dap/breakpoint-assembly/TestDAP_breakpointAssembly.py (+76) - (modified) lldb/tools/lldb-dap/Breakpoint.cpp (+35-6) - (modified) lldb/tools/lldb-dap/CMakeLists.txt (+2-1) - (modified) lldb/tools/lldb-dap/DAP.cpp (+10-4) - (modified) lldb/tools/lldb-dap/Handler/SetBreakpointsRequestHandler.cpp (-1) - (added) lldb/tools/lldb-dap/Protocol/DAPTypes.cpp (+37) - (added) lldb/tools/lldb-dap/Protocol/DAPTypes.h (+53) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp (+4-1) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolTypes.h (+7-1) - (modified) lldb/tools/lldb-dap/SourceBreakpoint.cpp (+63-33) - (modified) lldb/tools/lldb-dap/SourceBreakpoint.h (+7) ``````````diff diff --git a/lldb/include/lldb/API/SBTarget.h b/lldb/include/lldb/API/SBTarget.h index 2776a8f9010fe..a135e3ce4db58 100644 --- a/lldb/include/lldb/API/SBTarget.h +++ b/lldb/include/lldb/API/SBTarget.h @@ -23,6 +23,7 @@ #include "lldb/API/SBValue.h" #include "lldb/API/SBWatchpoint.h" #include "lldb/API/SBWatchpointOptions.h" +#include "lldb/lldb-types.h" namespace lldb_private { namespace python { @@ -737,6 +738,11 @@ class LLDB_API SBTarget { lldb::SBBreakpoint BreakpointCreateBySBAddress(SBAddress &address); + lldb::SBBreakpoint + BreakpointCreateByFileAddress(const SBFileSpec &file_spec, addr_t file_addr, + addr_t offset = 0, + addr_t instructions_offset = 0); + /// Create a breakpoint using a scripted resolver. /// /// \param[in] class_name diff --git a/lldb/include/lldb/Breakpoint/BreakpointResolver.h b/lldb/include/lldb/Breakpoint/BreakpointResolver.h index 52cd70e934e6d..adb577e2600b9 100644 --- a/lldb/include/lldb/Breakpoint/BreakpointResolver.h +++ b/lldb/include/lldb/Breakpoint/BreakpointResolver.h @@ -16,6 +16,7 @@ #include "lldb/Utility/FileSpec.h" #include "lldb/Utility/RegularExpression.h" #include "lldb/lldb-private.h" +#include "lldb/lldb-types.h" #include <optional> namespace lldb_private { @@ -45,9 +46,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, + lldb::addr_t instructions_offset = 0); /// The Destructor is virtual, all significant breakpoint resolvers derive /// from this class. @@ -182,6 +183,7 @@ class BreakpointResolver : public Searcher { SearchDepth, SkipPrologue, SymbolNameArray, + InstructionsOffset, LastOptionName }; static const char @@ -220,6 +222,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. + lldb::addr_t m_instructions_offset; // Number of instructions to add to the + // resolved breakpoint address. // Subclass identifier (for llvm isa/dyn_cast) const unsigned char SubclassID; diff --git a/lldb/include/lldb/Breakpoint/BreakpointResolverAddress.h b/lldb/include/lldb/Breakpoint/BreakpointResolverAddress.h index 3a09892f3f194..01318bbfe0d4a 100644 --- a/lldb/include/lldb/Breakpoint/BreakpointResolverAddress.h +++ b/lldb/include/lldb/Breakpoint/BreakpointResolverAddress.h @@ -28,6 +28,10 @@ class BreakpointResolverAddress : public BreakpointResolver { const Address &addr, const FileSpec &module_spec); + BreakpointResolverAddress(const lldb::BreakpointSP &bkpt, const Address &addr, + const FileSpec &module_spec, lldb::addr_t offset, + lldb::addr_t instructions_offset); + ~BreakpointResolverAddress() override = default; static lldb::BreakpointResolverSP 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 0d4e11b65339e..5730f7b369cd6 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -18,6 +18,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" @@ -723,11 +724,11 @@ class Target : public std::enable_shared_from_this<Target>, 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 - lldb::BreakpointSP CreateAddressInModuleBreakpoint(lldb::addr_t file_addr, - bool internal, - const FileSpec &file_spec, - bool request_hardware); + // 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, + bool request_hardware, lldb::addr_t offset = 0, + lldb::addr_t instructions_offset = 0); // Use this to create Address breakpoints: lldb::BreakpointSP CreateBreakpoint(const Address &addr, bool internal, @@ -1328,6 +1329,10 @@ class Target : public std::enable_shared_from_this<Target>, const lldb_private::RegisterFlags &flags, uint32_t byte_size); + lldb::DisassemblerSP 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 68f58bf1349a7..6c6a674f664ad 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 00b01d4bdb1c5..1cb885b149dfd 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 f26f7951edc6f..84478ffe22601 100644 --- a/lldb/source/API/SBTarget.cpp +++ b/lldb/source/API/SBTarget.cpp @@ -949,6 +949,24 @@ SBBreakpoint SBTarget::BreakpointCreateBySBAddress(SBAddress &sb_address) { return sb_bp; } +SBBreakpoint +SBTarget::BreakpointCreateByFileAddress(const SBFileSpec &file_spec, + addr_t file_addr, addr_t offset, + addr_t instructions_offset) { + LLDB_INSTRUMENT_VA(this, file_spec, file_addr); + + SBBreakpoint sb_bp; + if (TargetSP target_sp = GetSP()) { + std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex()); + const bool hardware = false; + sb_bp = target_sp->CreateAddressInModuleBreakpoint( + file_addr, false, *file_spec.get(), hardware, offset, + instructions_offset); + } + + return sb_bp; +} + lldb::SBBreakpoint SBTarget::BreakpointCreateBySourceRegex(const char *source_regex, const lldb::SBFileSpec &source_file, @@ -1955,29 +1973,8 @@ 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(); - } - 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)); + sb_instructions.SetDisassembler( + target_sp->ReadInstructions(*addr_ptr, count, flavor_string)); } } diff --git a/lldb/source/Breakpoint/BreakpointResolver.cpp b/lldb/source/Breakpoint/BreakpointResolver.cpp index 91fdff4a455da..0bca1337e3cff 100644 --- a/lldb/source/Breakpoint/BreakpointResolver.cpp +++ b/lldb/source/Breakpoint/BreakpointResolver.cpp @@ -29,6 +29,7 @@ #include "lldb/Utility/Log.h" #include "lldb/Utility/Stream.h" #include "lldb/Utility/StreamString.h" +#include "lldb/lldb-forward.h" #include <optional> using namespace lldb_private; @@ -42,10 +43,12 @@ const char *BreakpointResolver::g_ty_to_name[] = {"FileAndLine", "Address", const char *BreakpointResolver::g_option_names[static_cast<uint32_t>( BreakpointResolver::OptionNames::LastOptionName)] = { - "AddressOffset", "Exact", "FileName", "Inlines", "Language", - "LineNumber", "Column", "ModuleName", "NameMask", "Offset", - "PythonClass", "Regex", "ScriptArgs", "SectionName", "SearchDepth", - "SkipPrologue", "SymbolNames"}; + "AddressOffset", "Exact", "FileName", + "Inlines", "Language", "LineNumber", + "Column", "ModuleName", "NameMask", + "Offset", "PythonClass", "Regex", + "ScriptArgs", "SectionName", "SearchDepth", + "SkipPrologue", "SymbolNames", "InstructionsOffset"}; const char *BreakpointResolver::ResolverTyToName(enum ResolverTy type) { if (type > LastKnownResolverType) @@ -65,8 +68,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, + lldb::addr_t instructions_offset) + : m_breakpoint(bkpt), m_offset(offset), + m_instructions_offset(instructions_offset), SubclassID(resolverTy) {} BreakpointResolver::~BreakpointResolver() = default; @@ -364,6 +369,13 @@ void BreakpointResolver::AddLocation(SearchFilter &filter, BreakpointLocationSP BreakpointResolver::AddLocation(Address loc_addr, bool *new_location) { + if (m_instructions_offset != 0) { + Target &target = GetBreakpoint()->GetTarget(); + const DisassemblerSP instructions = + target.ReadInstructions(loc_addr, m_instructions_offset); + loc_addr.Slide(instructions->GetInstructionList().GetTotalByteSize()); + } + 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..fae2022220ba3 100644 --- a/lldb/source/Breakpoint/BreakpointResolverAddress.cpp +++ b/lldb/source/Breakpoint/BreakpointResolverAddress.cpp @@ -30,6 +30,14 @@ BreakpointResolverAddress::BreakpointResolverAddress(const BreakpointSP &bkpt, : BreakpointResolver(bkpt, BreakpointResolver::AddressResolver), m_addr(addr), m_resolved_addr(LLDB_INVALID_ADDRESS) {} +BreakpointResolverAddress::BreakpointResolverAddress( + const BreakpointSP &bkpt, const Address &addr, const FileSpec &module_spec, + lldb::addr_t offset, lldb::addr_t instructions_offset) + : BreakpointResolver(bkpt, BreakpointResolver::AddressResolver, offset, + instructions_offset), + m_addr(addr), m_resolved_addr(LLDB_INVALID_ADDRESS), + m_module_filespec(module_spec) {} + BreakpointResolverSP BreakpointResolverAddress::CreateFromStructuredData( const StructuredData::Dictionary &options_dict, Status &error) { llvm::StringRef module_name; @@ -133,6 +141,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/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp index 833e327579a29..90394cc9a5569 100644 --- a/lldb/source/Core/Disassembler.cpp +++ b/lldb/source/Core/Disassembler.cpp @@ -1014,6 +1014,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/Target/Target.cpp b/lldb/source/Target/Target.cpp index 7f569173eba20..b1d6fdf8f7537 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -65,6 +65,7 @@ #include "lldb/Utility/State.h" #include "lldb/Utility/StreamString.h" #include "lldb/Utility/Timer.h" +#include "lldb/lldb-forward.h" #include "llvm/ADT/ScopeExit.h" #include "llvm/ADT/SetVector.h" @@ -564,14 +565,14 @@ BreakpointSP Target::CreateBreakpoint(const Address &addr, bool internal, return CreateBreakpoint(filter_sp, resolver_sp, internal, hardware, false); } -lldb::BreakpointSP -Target::CreateAddressInModuleBreakpoint(lldb::addr_t file_addr, bool internal, - const FileSpec &file_spec, - bool request_hardware) { +lldb::BreakpointSP Target::CreateAddressInModuleBreakpoint( + lldb::addr_t file_addr, bool internal, const FileSpec &file_spec, + bool request_hardware, lldb::addr_t offset, + lldb::addr_t instructions_offset) { SearchFilterSP filter_sp( new SearchFilterForUnconstrainedSearches(shared_from_this())); BreakpointResolverSP resolver_sp(new BreakpointResolverAddress( - nullptr, file_addr, file_spec)); + nullptr, file_addr, file_spec, offset, instructions_offset)); return CreateBreakpoint(filter_sp, resolver_sp, internal, request_hardware, false); } @@ -2990,6 +2991,33 @@ lldb::addr_t Target::GetBr... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/148061 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
