Author: Ely Ronnen Date: 2025-08-08T22:29:47+02:00 New Revision: 4d3feaea66f43758518d15e07a975e3492317b1c
URL: https://github.com/llvm/llvm-project/commit/4d3feaea66f43758518d15e07a975e3492317b1c DIFF: https://github.com/llvm/llvm-project/commit/4d3feaea66f43758518d15e07a975e3492317b1c.diff LOG: [lldb-dap] persistent assembly breakpoints (#148061) Resolves #141955 - Adds data to breakpoints `Source` object, in order for assembly breakpoints, which rely on a temporary `sourceReference` value, to be able to resolve in future sessions like normal path+line breakpoints - Adds optional `instructions_offset` parameter to `BreakpointResolver` Added: lldb/tools/lldb-dap/Protocol/DAPTypes.cpp lldb/tools/lldb-dap/Protocol/DAPTypes.h Modified: lldb/include/lldb/API/SBTarget.h lldb/include/lldb/Breakpoint/BreakpointResolver.h lldb/include/lldb/Breakpoint/BreakpointResolverName.h lldb/include/lldb/Core/Disassembler.h lldb/include/lldb/Target/Target.h lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py lldb/source/API/SBTarget.cpp lldb/source/Breakpoint/BreakpointResolver.cpp lldb/source/Breakpoint/BreakpointResolverAddress.cpp lldb/source/Breakpoint/BreakpointResolverName.cpp lldb/source/Core/Disassembler.cpp lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp lldb/source/Target/Target.cpp lldb/test/API/tools/lldb-dap/breakpoint-assembly/TestDAP_breakpointAssembly.py lldb/tools/lldb-dap/Breakpoint.cpp lldb/tools/lldb-dap/CMakeLists.txt lldb/tools/lldb-dap/DAP.cpp lldb/tools/lldb-dap/Handler/SetBreakpointsRequestHandler.cpp lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp lldb/tools/lldb-dap/Protocol/ProtocolTypes.h lldb/tools/lldb-dap/SourceBreakpoint.cpp lldb/tools/lldb-dap/SourceBreakpoint.h Removed: ################################################################################ diff --git a/lldb/include/lldb/API/SBTarget.h b/lldb/include/lldb/API/SBTarget.h index 2776a8f9010fe..22b6c63ed5b97 100644 --- a/lldb/include/lldb/API/SBTarget.h +++ b/lldb/include/lldb/API/SBTarget.h @@ -658,6 +658,14 @@ class LLDB_API SBTarget { lldb::LanguageType symbol_language, const SBFileSpecList &module_list, const SBFileSpecList &comp_unit_list); + lldb::SBBreakpoint BreakpointCreateByName( + const char *symbol_name, + uint32_t + name_type_mask, // Logical OR one or more FunctionNameType enum bits + lldb::LanguageType symbol_language, lldb::addr_t offset, + bool offset_is_insn_count, const SBFileSpecList &module_list, + const SBFileSpecList &comp_unit_list); + #ifdef SWIG lldb::SBBreakpoint BreakpointCreateByNames( const char **symbol_name, uint32_t num_names, diff --git a/lldb/include/lldb/Breakpoint/BreakpointResolver.h b/lldb/include/lldb/Breakpoint/BreakpointResolver.h index 52cd70e934e6d..243aceeb6fd6a 100644 --- a/lldb/include/lldb/Breakpoint/BreakpointResolver.h +++ b/lldb/include/lldb/Breakpoint/BreakpointResolver.h @@ -45,9 +45,9 @@ class BreakpointResolver : public Searcher { /// The breakpoint that owns this resolver. /// \param[in] resolverType /// The concrete breakpoint resolver type for this breakpoint. - BreakpointResolver(const lldb::BreakpointSP &bkpt, - unsigned char resolverType, - lldb::addr_t offset = 0); + BreakpointResolver(const lldb::BreakpointSP &bkpt, unsigned char resolverType, + lldb::addr_t offset = 0, + bool offset_is_insn_count = false); /// The Destructor is virtual, all significant breakpoint resolvers derive /// from this class. @@ -76,6 +76,7 @@ class BreakpointResolver : public Searcher { void SetOffset(lldb::addr_t offset); lldb::addr_t GetOffset() const { return m_offset; } + lldb::addr_t GetOffsetIsInsnCount() const { return m_offset_is_insn_count; } /// In response to this method the resolver scans all the modules in the /// breakpoint's target, and adds any new locations it finds. @@ -220,6 +221,8 @@ class BreakpointResolver : public Searcher { lldb::BreakpointWP m_breakpoint; // This is the breakpoint we add locations to. lldb::addr_t m_offset; // A random offset the user asked us to add to any // breakpoints we set. + bool m_offset_is_insn_count; // Use the offset as an instruction count + // instead of an address offset. // Subclass identifier (for llvm isa/dyn_cast) const unsigned char SubclassID; diff --git a/lldb/include/lldb/Breakpoint/BreakpointResolverName.h b/lldb/include/lldb/Breakpoint/BreakpointResolverName.h index c83814c174e88..48b3edabd808f 100644 --- a/lldb/include/lldb/Breakpoint/BreakpointResolverName.h +++ b/lldb/include/lldb/Breakpoint/BreakpointResolverName.h @@ -27,7 +27,7 @@ class BreakpointResolverName : public BreakpointResolver { lldb::FunctionNameType name_type_mask, lldb::LanguageType language, Breakpoint::MatchType type, lldb::addr_t offset, - bool skip_prologue); + bool offset_is_insn_count, bool skip_prologue); // This one takes an array of names. It is always MatchType = Exact. BreakpointResolverName(const lldb::BreakpointSP &bkpt, const char *names[], diff --git a/lldb/include/lldb/Core/Disassembler.h b/lldb/include/lldb/Core/Disassembler.h index 21bacb14f9b25..50a5d87835844 100644 --- a/lldb/include/lldb/Core/Disassembler.h +++ b/lldb/include/lldb/Core/Disassembler.h @@ -291,6 +291,8 @@ class InstructionList { size_t GetSize() const; + size_t GetTotalByteSize() const; + uint32_t GetMaxOpcocdeByteSize() const; lldb::InstructionSP GetInstructionAtIndex(size_t idx) const; diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index 7b23c8abe8d2f..14a09f29094d5 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,7 +724,7 @@ 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 + // 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, @@ -752,8 +753,8 @@ class Target : public std::enable_shared_from_this<Target>, const FileSpecList *containingModules, const FileSpecList *containingSourceFiles, const char *func_name, lldb::FunctionNameType func_name_type_mask, lldb::LanguageType language, - lldb::addr_t offset, LazyBool skip_prologue, bool internal, - bool request_hardware); + lldb::addr_t offset, bool offset_is_insn_count, LazyBool skip_prologue, + bool internal, bool request_hardware); lldb::BreakpointSP CreateExceptionBreakpoint(enum lldb::LanguageType language, bool catch_bp, @@ -1334,6 +1335,10 @@ class Target : public std::enable_shared_from_this<Target>, const lldb_private::RegisterFlags &flags, uint32_t byte_size); + llvm::Expected<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 0b09893c7ed5b..939be9941a49d 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py @@ -107,17 +107,23 @@ def dump_dap_log(log_file): class Source(object): def __init__( - self, path: Optional[str] = None, source_reference: Optional[int] = None + self, + path: Optional[str] = None, + source_reference: Optional[int] = None, + raw_dict: Optional[dict[str, Any]] = None, ): self._name = None self._path = None self._source_reference = None + self._raw_dict = None if path is not None: self._name = os.path.basename(path) self._path = path elif source_reference is not None: self._source_reference = source_reference + elif raw_dict is not None: + self._raw_dict = raw_dict else: raise ValueError("Either path or source_reference must be provided") @@ -125,6 +131,9 @@ def __str__(self): return f"Source(name={self.name}, path={self.path}), source_reference={self.source_reference})" def as_dict(self): + if self._raw_dict is not None: + return self._raw_dict + source_dict = {} if self._name is not None: source_dict["name"] = self._name @@ -135,6 +144,19 @@ def as_dict(self): return source_dict +class Breakpoint(object): + def __init__(self, obj): + self._breakpoint = obj + + def is_verified(self): + """Check if the breakpoint is verified.""" + return self._breakpoint.get("verified", False) + + def source(self): + """Get the source of the breakpoint.""" + return self._breakpoint.get("source", {}) + + class NotSupportedError(KeyError): """Raised if a feature is not supported due to its capabilities.""" @@ -170,7 +192,7 @@ def __init__( self.initialized = False self.frame_scopes = {} self.init_commands = init_commands - self.resolved_breakpoints = {} + self.resolved_breakpoints: dict[str, Breakpoint] = {} @classmethod def encode_content(cls, s: str) -> bytes: @@ -326,8 +348,8 @@ def _process_continued(self, all_threads_continued: bool): def _update_verified_breakpoints(self, breakpoints: list[Event]): for breakpoint in breakpoints: if "id" in breakpoint: - self.resolved_breakpoints[str(breakpoint["id"])] = breakpoint.get( - "verified", False + self.resolved_breakpoints[str(breakpoint["id"])] = Breakpoint( + breakpoint ) def send_packet(self, command_dict: Request, set_sequence=True): @@ -484,7 +506,14 @@ def wait_for_breakpoints_to_be_verified( if breakpoint_event is None: break - return [id for id in breakpoint_ids if id not in self.resolved_breakpoints] + return [ + id + for id in breakpoint_ids + if ( + id not in self.resolved_breakpoints + or not self.resolved_breakpoints[id].is_verified() + ) + ] def wait_for_exited(self, timeout: Optional[float] = None): event_dict = self.wait_for_event("exited", timeout=timeout) diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py index 1567462839748..c51b4b1892951 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py @@ -59,24 +59,22 @@ def set_source_breakpoints( Each object in data is 1:1 mapping with the entry in lines. It contains optional location/hitCondition/logMessage parameters. """ - response = self.dap_server.request_setBreakpoints( - Source(source_path), lines, data + return self.set_source_breakpoints_from_source( + Source(path=source_path), lines, data, wait_for_resolve ) - if response is None or not response["success"]: - return [] - breakpoints = response["body"]["breakpoints"] - breakpoint_ids = [] - for breakpoint in breakpoints: - breakpoint_ids.append("%i" % (breakpoint["id"])) - if wait_for_resolve: - self.wait_for_breakpoints_to_resolve(breakpoint_ids) - return breakpoint_ids def set_source_breakpoints_assembly( self, source_reference, lines, data=None, wait_for_resolve=True + ): + return self.set_source_breakpoints_from_source( + Source(source_reference=source_reference), lines, data, wait_for_resolve + ) + + def set_source_breakpoints_from_source( + self, source: Source, lines, data=None, wait_for_resolve=True ): response = self.dap_server.request_setBreakpoints( - Source(source_reference=source_reference), + source, lines, data, ) diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp index f26f7951edc6f..6aa41c52f3731 100644 --- a/lldb/source/API/SBTarget.cpp +++ b/lldb/source/API/SBTarget.cpp @@ -766,16 +766,19 @@ SBBreakpoint SBTarget::BreakpointCreateByName(const char *symbol_name, const bool hardware = false; const LazyBool skip_prologue = eLazyBoolCalculate; const lldb::addr_t offset = 0; + const bool offset_is_insn_count = false; if (module_name && module_name[0]) { FileSpecList module_spec_list; module_spec_list.Append(FileSpec(module_name)); sb_bp = target_sp->CreateBreakpoint( &module_spec_list, nullptr, symbol_name, eFunctionNameTypeAuto, - eLanguageTypeUnknown, offset, skip_prologue, internal, hardware); + eLanguageTypeUnknown, offset, offset_is_insn_count, skip_prologue, + internal, hardware); } else { sb_bp = target_sp->CreateBreakpoint( nullptr, nullptr, symbol_name, eFunctionNameTypeAuto, - eLanguageTypeUnknown, offset, skip_prologue, internal, hardware); + eLanguageTypeUnknown, offset, offset_is_insn_count, skip_prologue, + internal, hardware); } } @@ -811,6 +814,17 @@ lldb::SBBreakpoint SBTarget::BreakpointCreateByName( const SBFileSpecList &comp_unit_list) { LLDB_INSTRUMENT_VA(this, symbol_name, name_type_mask, symbol_language, module_list, comp_unit_list); + return BreakpointCreateByName(symbol_name, name_type_mask, symbol_language, 0, + false, module_list, comp_unit_list); +} + +lldb::SBBreakpoint SBTarget::BreakpointCreateByName( + const char *symbol_name, uint32_t name_type_mask, + LanguageType symbol_language, lldb::addr_t offset, + bool offset_is_insn_count, const SBFileSpecList &module_list, + const SBFileSpecList &comp_unit_list) { + LLDB_INSTRUMENT_VA(this, symbol_name, name_type_mask, symbol_language, offset, + offset_is_insn_count, module_list, comp_unit_list); SBBreakpoint sb_bp; if (TargetSP target_sp = GetSP(); @@ -821,7 +835,8 @@ lldb::SBBreakpoint SBTarget::BreakpointCreateByName( std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex()); FunctionNameType mask = static_cast<FunctionNameType>(name_type_mask); sb_bp = target_sp->CreateBreakpoint(module_list.get(), comp_unit_list.get(), - symbol_name, mask, symbol_language, 0, + symbol_name, mask, symbol_language, + offset, offset_is_insn_count, skip_prologue, internal, hardware); } @@ -1955,29 +1970,10 @@ lldb::SBInstructionList SBTarget::ReadInstructions(lldb::SBAddress base_addr, if (TargetSP target_sp = GetSP()) { if (Address *addr_ptr = base_addr.get()) { - DataBufferHeap data( - target_sp->GetArchitecture().GetMaximumOpcodeByteSize() * count, 0); - bool force_live_memory = true; - lldb_private::Status error; - lldb::addr_t load_addr = LLDB_INVALID_ADDRESS; - const size_t bytes_read = - target_sp->ReadMemory(*addr_ptr, data.GetBytes(), data.GetByteSize(), - error, force_live_memory, &load_addr); - - const bool data_from_file = load_addr == LLDB_INVALID_ADDRESS; - if (!flavor_string || flavor_string[0] == '\0') { - // FIXME - we don't have the mechanism in place to do per-architecture - // settings. But since we know that for now we only support flavors on - // x86 & x86_64, - const llvm::Triple::ArchType arch = - target_sp->GetArchitecture().GetTriple().getArch(); - if (arch == llvm::Triple::x86 || arch == llvm::Triple::x86_64) - flavor_string = target_sp->GetDisassemblyFlavor(); + if (llvm::Expected<DisassemblerSP> disassembler = + target_sp->ReadInstructions(*addr_ptr, count, flavor_string)) { + sb_instructions.SetDisassembler(*disassembler); } - sb_instructions.SetDisassembler(Disassembler::DisassembleBytes( - target_sp->GetArchitecture(), nullptr, flavor_string, - target_sp->GetDisassemblyCPU(), target_sp->GetDisassemblyFeatures(), - *addr_ptr, data.GetBytes(), bytes_read, count, data_from_file)); } } diff --git a/lldb/source/Breakpoint/BreakpointResolver.cpp b/lldb/source/Breakpoint/BreakpointResolver.cpp index 91fdff4a455da..4ac40501a5df5 100644 --- a/lldb/source/Breakpoint/BreakpointResolver.cpp +++ b/lldb/source/Breakpoint/BreakpointResolver.cpp @@ -42,9 +42,9 @@ const char *BreakpointResolver::g_ty_to_name[] = {"FileAndLine", "Address", const char *BreakpointResolver::g_option_names[static_cast<uint32_t>( BreakpointResolver::OptionNames::LastOptionName)] = { - "AddressOffset", "Exact", "FileName", "Inlines", "Language", - "LineNumber", "Column", "ModuleName", "NameMask", "Offset", - "PythonClass", "Regex", "ScriptArgs", "SectionName", "SearchDepth", + "AddressOffset", "Exact", "FileName", "Inlines", "Language", + "LineNumber", "Column", "ModuleName", "NameMask", "Offset", + "PythonClass", "Regex", "ScriptArgs", "SectionName", "SearchDepth", "SkipPrologue", "SymbolNames"}; const char *BreakpointResolver::ResolverTyToName(enum ResolverTy type) { @@ -65,8 +65,10 @@ BreakpointResolver::NameToResolverTy(llvm::StringRef name) { BreakpointResolver::BreakpointResolver(const BreakpointSP &bkpt, const unsigned char resolverTy, - lldb::addr_t offset) - : m_breakpoint(bkpt), m_offset(offset), SubclassID(resolverTy) {} + lldb::addr_t offset, + bool offset_is_insn_count) + : m_breakpoint(bkpt), m_offset(offset), + m_offset_is_insn_count(offset_is_insn_count), SubclassID(resolverTy) {} BreakpointResolver::~BreakpointResolver() = default; @@ -364,7 +366,32 @@ void BreakpointResolver::AddLocation(SearchFilter &filter, BreakpointLocationSP BreakpointResolver::AddLocation(Address loc_addr, bool *new_location) { - loc_addr.Slide(m_offset); + if (m_offset_is_insn_count) { + Target &target = GetBreakpoint()->GetTarget(); + llvm::Expected<DisassemblerSP> expected_instructions = + target.ReadInstructions(loc_addr, m_offset); + if (!expected_instructions) { + LLDB_LOG_ERROR(GetLog(LLDBLog::Breakpoints), + expected_instructions.takeError(), + "error: Unable to read instructions at address 0x{0:x}", + loc_addr.GetLoadAddress(&target)); + return BreakpointLocationSP(); + } + + const DisassemblerSP instructions = *expected_instructions; + if (!instructions || + instructions->GetInstructionList().GetSize() != m_offset) { + LLDB_LOG(GetLog(LLDBLog::Breakpoints), + "error: Unable to read {0} instructions at address 0x{1:x}", + m_offset, loc_addr.GetLoadAddress(&target)); + return BreakpointLocationSP(); + } + + loc_addr.Slide(instructions->GetInstructionList().GetTotalByteSize()); + } else { + loc_addr.Slide(m_offset); + } + return GetBreakpoint()->AddLocation(loc_addr, new_location); } diff --git a/lldb/source/Breakpoint/BreakpointResolverAddress.cpp b/lldb/source/Breakpoint/BreakpointResolverAddress.cpp index 828647ceef637..70360d9d960ec 100644 --- a/lldb/source/Breakpoint/BreakpointResolverAddress.cpp +++ b/lldb/source/Breakpoint/BreakpointResolverAddress.cpp @@ -133,6 +133,11 @@ Searcher::CallbackReturn BreakpointResolverAddress::SearchCallback( Address tmp_address; if (module_sp->ResolveFileAddress(m_addr.GetOffset(), tmp_address)) m_addr = tmp_address; + else + return Searcher::eCallbackReturnStop; + } else { + // If we didn't find the module, then we can't resolve the address. + return Searcher::eCallbackReturnStop; } } diff --git a/lldb/source/Breakpoint/BreakpointResolverName.cpp b/lldb/source/Breakpoint/BreakpointResolverName.cpp index 21024a4198e1d..6372595a0f21f 100644 --- a/lldb/source/Breakpoint/BreakpointResolverName.cpp +++ b/lldb/source/Breakpoint/BreakpointResolverName.cpp @@ -24,11 +24,13 @@ using namespace lldb; using namespace lldb_private; -BreakpointResolverName::BreakpointResolverName(const BreakpointSP &bkpt, - const char *name_cstr, FunctionNameType name_type_mask, - LanguageType language, Breakpoint::MatchType type, lldb::addr_t offset, +BreakpointResolverName::BreakpointResolverName( + const BreakpointSP &bkpt, const char *name_cstr, + FunctionNameType name_type_mask, LanguageType language, + Breakpoint::MatchType type, lldb::addr_t offset, bool offset_is_insn_count, bool skip_prologue) - : BreakpointResolver(bkpt, BreakpointResolver::NameResolver, offset), + : BreakpointResolver(bkpt, BreakpointResolver::NameResolver, offset, + offset_is_insn_count), m_match_type(type), m_language(language), m_skip_prologue(skip_prologue) { if (m_match_type == Breakpoint::Regexp) { m_regex = RegularExpression(name_cstr); @@ -81,7 +83,7 @@ BreakpointResolverName::BreakpointResolverName(const BreakpointSP &bkpt, BreakpointResolverName::BreakpointResolverName( const BreakpointResolverName &rhs) : BreakpointResolver(rhs.GetBreakpoint(), BreakpointResolver::NameResolver, - rhs.GetOffset()), + rhs.GetOffset(), rhs.GetOffsetIsInsnCount()), m_lookups(rhs.m_lookups), m_class_name(rhs.m_class_name), m_regex(rhs.m_regex), m_match_type(rhs.m_match_type), m_language(rhs.m_language), m_skip_prologue(rhs.m_skip_prologue) {} @@ -177,7 +179,8 @@ BreakpointResolverSP BreakpointResolverName::CreateFromStructuredData( std::shared_ptr<BreakpointResolverName> resolver_sp = std::make_shared<BreakpointResolverName>( nullptr, names[0].c_str(), name_masks[0], language, - Breakpoint::MatchType::Exact, offset, skip_prologue); + Breakpoint::MatchType::Exact, offset, + /*offset_is_insn_count = */ false, skip_prologue); for (size_t i = 1; i < num_elem; i++) { resolver_sp->AddNameLookup(ConstString(names[i]), name_masks[i]); } diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp index 925de2a5c836c..e0a7d69345706 100644 --- a/lldb/source/Core/Disassembler.cpp +++ b/lldb/source/Core/Disassembler.cpp @@ -1016,6 +1016,16 @@ uint32_t InstructionList::GetMaxOpcocdeByteSize() const { return max_inst_size; } +size_t InstructionList::GetTotalByteSize() const { + size_t total_byte_size = 0; + collection::const_iterator pos, end; + for (pos = m_instructions.begin(), end = m_instructions.end(); pos != end; + ++pos) { + total_byte_size += (*pos)->GetOpcode().GetByteSize(); + } + return total_byte_size; +} + InstructionSP InstructionList::GetInstructionAtIndex(size_t idx) const { InstructionSP inst_sp; if (idx < m_instructions.size()) diff --git a/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp b/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp index fe9f5d086da2c..1d210ea78df1a 100644 --- a/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp +++ b/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp @@ -1561,7 +1561,8 @@ void DynamicLoaderDarwinKernel::SetNotificationBreakpointIfNeeded() { .CreateBreakpoint(&module_spec_list, nullptr, "OSKextLoadedKextSummariesUpdated", eFunctionNameTypeFull, eLanguageTypeUnknown, 0, - skip_prologue, internal_bp, hardware) + /*offset_is_insn_count = */ false, skip_prologue, + internal_bp, hardware) .get(); bp->SetCallback(DynamicLoaderDarwinKernel::BreakpointHitCallback, this, diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp index 08bef4999eb9a..efb9ccc76b507 100644 --- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp +++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp @@ -530,7 +530,7 @@ bool DynamicLoaderMacOS::SetNotificationBreakpoint() { m_process->GetTarget() .CreateBreakpoint(&dyld_filelist, source_files, "lldb_image_notifier", eFunctionNameTypeFull, - eLanguageTypeUnknown, 0, skip_prologue, + eLanguageTypeUnknown, 0, false, skip_prologue, internal, hardware) .get(); breakpoint->SetCallback(DynamicLoaderMacOS::NotifyBreakpointHit, this, @@ -546,8 +546,9 @@ bool DynamicLoaderMacOS::SetNotificationBreakpoint() { m_process->GetTarget() .CreateBreakpoint(&dyld_filelist, source_files, "gdb_image_notifier", eFunctionNameTypeFull, - eLanguageTypeUnknown, 0, skip_prologue, - internal, hardware) + eLanguageTypeUnknown, 0, + /*offset_is_insn_count = */ false, + skip_prologue, internal, hardware) .get(); breakpoint->SetCallback(DynamicLoaderMacOS::NotifyBreakpointHit, this, true); diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp index 24a73717266a4..b1f2a661c4af6 100644 --- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp +++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp @@ -102,7 +102,7 @@ AppleObjCRuntimeV1::CreateExceptionResolver(const BreakpointSP &bkpt, resolver_sp = std::make_shared<BreakpointResolverName>( bkpt, std::get<1>(GetExceptionThrowLocation()).AsCString(), eFunctionNameTypeBase, eLanguageTypeUnknown, Breakpoint::Exact, 0, - eLazyBoolNo); + /*offset_is_insn_count = */ false, eLazyBoolNo); // FIXME: don't do catch yet. return resolver_sp; } diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp index cca721e842f0d..9beb133f5595f 100644 --- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp +++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp @@ -1163,7 +1163,7 @@ AppleObjCRuntimeV2::CreateExceptionResolver(const BreakpointSP &bkpt, resolver_sp = std::make_shared<BreakpointResolverName>( bkpt, std::get<1>(GetExceptionThrowLocation()).AsCString(), eFunctionNameTypeBase, eLanguageTypeUnknown, Breakpoint::Exact, 0, - eLazyBoolNo); + /*offset_is_insn_count = */ false, eLazyBoolNo); // FIXME: We don't do catch breakpoints for ObjC yet. // Should there be some way for the runtime to specify what it can do in this // regard? diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp index a4b3e26474a55..8dc5f511c6291 100644 --- a/lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp +++ b/lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp @@ -169,7 +169,8 @@ GNUstepObjCRuntime::CreateExceptionResolver(const BreakpointSP &bkpt, if (throw_bp) resolver_sp = std::make_shared<BreakpointResolverName>( bkpt, "objc_exception_throw", eFunctionNameTypeBase, - eLanguageTypeUnknown, Breakpoint::Exact, 0, eLazyBoolNo); + eLanguageTypeUnknown, Breakpoint::Exact, 0, + /*offset_is_insn_count = */ false, eLazyBoolNo); return resolver_sp; } diff --git a/lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp b/lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp index 867f6a6393bca..70093c9624f09 100644 --- a/lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp +++ b/lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp @@ -1601,6 +1601,7 @@ void StructuredDataDarwinLog::AddInitCompletionHook(Process &process) { const char *func_name = "_libtrace_init"; const lldb::addr_t offset = 0; + const bool offset_is_insn_count = false; const LazyBool skip_prologue = eLazyBoolCalculate; // This is an internal breakpoint - the user shouldn't see it. const bool internal = true; @@ -1608,7 +1609,8 @@ void StructuredDataDarwinLog::AddInitCompletionHook(Process &process) { auto breakpoint_sp = target.CreateBreakpoint( &module_spec_list, source_spec_list, func_name, eFunctionNameTypeFull, - eLanguageTypeC, offset, skip_prologue, internal, hardware); + eLanguageTypeC, offset, offset_is_insn_count, skip_prologue, internal, + hardware); if (!breakpoint_sp) { // Huh? Bail here. LLDB_LOGF(log, diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index 4f39f6018e624..fa98c24606492 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -558,10 +558,11 @@ BreakpointSP Target::CreateBreakpoint(lldb::addr_t addr, bool internal, BreakpointSP Target::CreateBreakpoint(const Address &addr, bool internal, bool hardware) { - SearchFilterSP filter_sp( - new SearchFilterForUnconstrainedSearches(shared_from_this())); - BreakpointResolverSP resolver_sp( - new BreakpointResolverAddress(nullptr, addr)); + SearchFilterSP filter_sp = + std::make_shared<SearchFilterForUnconstrainedSearches>( + shared_from_this()); + BreakpointResolverSP resolver_sp = + std::make_shared<BreakpointResolverAddress>(nullptr, addr); return CreateBreakpoint(filter_sp, resolver_sp, internal, hardware, false); } @@ -569,10 +570,12 @@ lldb::BreakpointSP Target::CreateAddressInModuleBreakpoint(lldb::addr_t file_addr, bool internal, const FileSpec &file_spec, bool request_hardware) { - SearchFilterSP filter_sp( - new SearchFilterForUnconstrainedSearches(shared_from_this())); - BreakpointResolverSP resolver_sp(new BreakpointResolverAddress( - nullptr, file_addr, file_spec)); + SearchFilterSP filter_sp = + std::make_shared<SearchFilterForUnconstrainedSearches>( + shared_from_this()); + BreakpointResolverSP resolver_sp = + std::make_shared<BreakpointResolverAddress>(nullptr, file_addr, + file_spec); return CreateBreakpoint(filter_sp, resolver_sp, internal, request_hardware, false); } @@ -581,7 +584,8 @@ BreakpointSP Target::CreateBreakpoint( const FileSpecList *containingModules, const FileSpecList *containingSourceFiles, const char *func_name, FunctionNameType func_name_type_mask, LanguageType language, - lldb::addr_t offset, LazyBool skip_prologue, bool internal, bool hardware) { + lldb::addr_t offset, bool offset_is_insn_count, LazyBool skip_prologue, + bool internal, bool hardware) { BreakpointSP bp_sp; if (func_name) { SearchFilterSP filter_sp(GetSearchFilterForModuleAndCUList( @@ -594,7 +598,7 @@ BreakpointSP Target::CreateBreakpoint( BreakpointResolverSP resolver_sp(new BreakpointResolverName( nullptr, func_name, func_name_type_mask, language, Breakpoint::Exact, - offset, skip_prologue)); + offset, offset_is_insn_count, skip_prologue)); bp_sp = CreateBreakpoint(filter_sp, resolver_sp, internal, hardware, true); } return bp_sp; @@ -2996,6 +3000,38 @@ lldb::addr_t Target::GetBreakableLoadAddress(lldb::addr_t addr) { return arch_plugin ? arch_plugin->GetBreakableLoadAddress(addr, *this) : addr; } +llvm::Expected<lldb::DisassemblerSP> +Target::ReadInstructions(const Address &start_addr, uint32_t count, + const char *flavor_string) { + DataBufferHeap data(GetArchitecture().GetMaximumOpcodeByteSize() * count, 0); + bool force_live_memory = true; + lldb_private::Status error; + lldb::addr_t load_addr = LLDB_INVALID_ADDRESS; + const size_t bytes_read = + ReadMemory(start_addr, data.GetBytes(), data.GetByteSize(), error, + force_live_memory, &load_addr); + + if (error.Fail()) + return llvm::createStringError( + error.AsCString("Target::ReadInstructions failed to read memory at %s"), + start_addr.GetLoadAddress(this)); + + const bool data_from_file = load_addr == LLDB_INVALID_ADDRESS; + if (!flavor_string || flavor_string[0] == '\0') { + // FIXME - we don't have the mechanism in place to do per-architecture + // settings. But since we know that for now we only support flavors on + // x86 & x86_64, + const llvm::Triple::ArchType arch = GetArchitecture().GetTriple().getArch(); + if (arch == llvm::Triple::x86 || arch == llvm::Triple::x86_64) + flavor_string = GetDisassemblyFlavor(); + } + + return Disassembler::DisassembleBytes( + GetArchitecture(), nullptr, flavor_string, GetDisassemblyCPU(), + GetDisassemblyFeatures(), start_addr, data.GetBytes(), bytes_read, count, + data_from_file); +} + SourceManager &Target::GetSourceManager() { if (!m_source_manager_up) m_source_manager_up = std::make_unique<SourceManager>(shared_from_this()); diff --git a/lldb/test/API/tools/lldb-dap/breakpoint-assembly/TestDAP_breakpointAssembly.py b/lldb/test/API/tools/lldb-dap/breakpoint-assembly/TestDAP_breakpointAssembly.py index 674bfe4199b4a..7552a77d2280a 100644 --- a/lldb/test/API/tools/lldb-dap/breakpoint-assembly/TestDAP_breakpointAssembly.py +++ b/lldb/test/API/tools/lldb-dap/breakpoint-assembly/TestDAP_breakpointAssembly.py @@ -83,3 +83,79 @@ def test_break_on_invalid_source_reference(self): break_point["message"], "Invalid sourceReference.", ) + + @skipIfWindows + def test_persistent_assembly_breakpoint(self): + """Tests that assembly breakpoints are working persistently across sessions""" + self.build() + program = self.getBuildArtifact("a.out") + self.create_debug_adapter() + + # Run the first session and set a persistent assembly breakpoint + try: + self.dap_server.request_initialize() + self.dap_server.request_launch(program) + + assmebly_func_breakpoints = self.set_function_breakpoints(["assembly_func"]) + self.continue_to_breakpoints(assmebly_func_breakpoints) + + assembly_func_frame = self.get_stackFrames()[0] + source_reference = assembly_func_frame["source"]["sourceReference"] + + # Set an assembly breakpoint in the middle of the assembly function + persistent_breakpoint_line = 4 + persistent_breakpoint_ids = self.set_source_breakpoints_assembly( + source_reference, [persistent_breakpoint_line] + ) + + self.assertEqual( + len(persistent_breakpoint_ids), + 1, + "Expected one assembly breakpoint to be set", + ) + + persistent_breakpoint_source = self.dap_server.resolved_breakpoints[ + persistent_breakpoint_ids[0] + ].source() + self.assertIn( + "adapterData", + persistent_breakpoint_source, + "Expected assembly breakpoint to have persistent information", + ) + self.assertIn( + "persistenceData", + persistent_breakpoint_source["adapterData"], + "Expected assembly breakpoint to have persistent information", + ) + + self.continue_to_breakpoints(persistent_breakpoint_ids) + finally: + self.dap_server.request_disconnect(terminateDebuggee=True) + self.dap_server.terminate() + + # Restart the session and verify the breakpoint is still there + self.create_debug_adapter() + try: + self.dap_server.request_initialize() + self.dap_server.request_launch(program) + new_session_breakpoints_ids = self.set_source_breakpoints_from_source( + Source(raw_dict=persistent_breakpoint_source), + [persistent_breakpoint_line], + ) + + self.assertEqual( + len(new_session_breakpoints_ids), + 1, + "Expected one breakpoint to be set in the new session", + ) + + self.continue_to_breakpoints(new_session_breakpoints_ids) + current_line = self.get_stackFrames()[0]["line"] + self.assertEqual( + current_line, + persistent_breakpoint_line, + "Expected to hit the persistent assembly breakpoint at the same line", + ) + finally: + self.dap_server.request_disconnect(terminateDebuggee=True) + self.dap_server.terminate() diff --git a/lldb/tools/lldb-dap/Breakpoint.cpp b/lldb/tools/lldb-dap/Breakpoint.cpp index b4e593eb83d27..c8039576b29bd 100644 --- a/lldb/tools/lldb-dap/Breakpoint.cpp +++ b/lldb/tools/lldb-dap/Breakpoint.cpp @@ -8,10 +8,14 @@ #include "Breakpoint.h" #include "DAP.h" +#include "LLDBUtils.h" +#include "Protocol/DAPTypes.h" #include "ProtocolUtils.h" #include "lldb/API/SBAddress.h" #include "lldb/API/SBBreakpointLocation.h" +#include "lldb/API/SBFileSpec.h" #include "lldb/API/SBLineEntry.h" +#include "lldb/API/SBModule.h" #include "lldb/API/SBMutex.h" #include "llvm/ADT/StringExtras.h" #include <cstddef> @@ -21,6 +25,22 @@ using namespace lldb_dap; +static std::optional<protocol::PersistenceData> +GetPersistenceDataForSymbol(lldb::SBSymbol &symbol) { + protocol::PersistenceData persistence_data; + lldb::SBModule module = symbol.GetStartAddress().GetModule(); + if (!module.IsValid()) + return std::nullopt; + + lldb::SBFileSpec file_spec = module.GetFileSpec(); + if (!file_spec.IsValid()) + return std::nullopt; + + persistence_data.module_path = GetSBFileSpecPath(file_spec); + persistence_data.symbol_name = symbol.GetName(); + return persistence_data; +} + void Breakpoint::SetCondition() { m_bp.SetCondition(m_condition.c_str()); } void Breakpoint::SetHitCondition() { @@ -73,7 +93,7 @@ protocol::Breakpoint Breakpoint::ToProtocolBreakpoint() { const auto column = line_entry.GetColumn(); if (column != LLDB_INVALID_COLUMN_NUMBER) breakpoint.column = column; - } else { + } else if (source) { // Assembly breakpoint. auto symbol = bp_addr.GetSymbol(); if (symbol.IsValid()) { @@ -82,6 +102,15 @@ protocol::Breakpoint Breakpoint::ToProtocolBreakpoint() { .ReadInstructions(symbol.GetStartAddress(), bp_addr, nullptr) .GetSize() + 1; + + // Add persistent data so that the breakpoint can be resolved + // in future sessions. + std::optional<protocol::PersistenceData> persistence_data = + GetPersistenceDataForSymbol(symbol); + if (persistence_data) { + source->adapterData = + protocol::SourceLLDBData{std::move(persistence_data)}; + } } } diff --git a/lldb/tools/lldb-dap/CMakeLists.txt b/lldb/tools/lldb-dap/CMakeLists.txt index 4cddfb1bea1c2..5e0ad53b82f89 100644 --- a/lldb/tools/lldb-dap/CMakeLists.txt +++ b/lldb/tools/lldb-dap/CMakeLists.txt @@ -66,7 +66,8 @@ add_lldb_library(lldbDAP Handler/ThreadsRequestHandler.cpp Handler/VariablesRequestHandler.cpp Handler/WriteMemoryRequestHandler.cpp - + + Protocol/DAPTypes.cpp Protocol/ProtocolBase.cpp Protocol/ProtocolEvents.cpp Protocol/ProtocolTypes.cpp diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index cbd3b14463e25..849712f724c69 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -1406,11 +1406,15 @@ void DAP::EventThread() { // avoids sending paths that should be source mapped. Note that // CreateBreakpoint doesn't apply source mapping and certain // implementation ignore the source part of this event anyway. - llvm::json::Value source_bp = bp.ToProtocolBreakpoint(); - source_bp.getAsObject()->erase("source"); + protocol::Breakpoint protocol_bp = bp.ToProtocolBreakpoint(); + + // "source" is not needed here, unless we add adapter data to be + // saved by the client. + if (protocol_bp.source && !protocol_bp.source->adapterData) + protocol_bp.source = std::nullopt; llvm::json::Object body; - body.try_emplace("breakpoint", source_bp); + body.try_emplace("breakpoint", protocol_bp); body.try_emplace("reason", "changed"); llvm::json::Object bp_event = CreateEventObject("breakpoint"); @@ -1491,8 +1495,9 @@ std::vector<protocol::Breakpoint> DAP::SetSourceBreakpoints( protocol::Breakpoint response_breakpoint = iv->second.ToProtocolBreakpoint(); - response_breakpoint.source = source; + if (!response_breakpoint.source) + response_breakpoint.source = source; if (!response_breakpoint.line && src_bp.GetLine() != LLDB_INVALID_LINE_NUMBER) response_breakpoint.line = src_bp.GetLine(); diff --git a/lldb/tools/lldb-dap/Handler/SetBreakpointsRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/SetBreakpointsRequestHandler.cpp index 5d336af740c99..142351fd62179 100644 --- a/lldb/tools/lldb-dap/Handler/SetBreakpointsRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/SetBreakpointsRequestHandler.cpp @@ -9,7 +9,6 @@ #include "DAP.h" #include "Protocol/ProtocolRequests.h" #include "RequestHandler.h" -#include <vector> namespace lldb_dap { diff --git a/lldb/tools/lldb-dap/Protocol/DAPTypes.cpp b/lldb/tools/lldb-dap/Protocol/DAPTypes.cpp new file mode 100644 index 0000000000000..ecb4baef56e80 --- /dev/null +++ b/lldb/tools/lldb-dap/Protocol/DAPTypes.cpp @@ -0,0 +1,36 @@ +#include "Protocol/DAPTypes.h" + +using namespace llvm; + +namespace lldb_dap::protocol { + +bool fromJSON(const llvm::json::Value &Params, PersistenceData &PD, + llvm::json::Path P) { + json::ObjectMapper O(Params, P); + return O && O.mapOptional("module_path", PD.module_path) && + O.mapOptional("symbol_name", PD.symbol_name); +} + +llvm::json::Value toJSON(const PersistenceData &PD) { + json::Object result{ + {"module_path", PD.module_path}, + {"symbol_name", PD.symbol_name}, + }; + + return result; +} + +bool fromJSON(const llvm::json::Value &Params, SourceLLDBData &SLD, + llvm::json::Path P) { + json::ObjectMapper O(Params, P); + return O && O.mapOptional("persistenceData", SLD.persistenceData); +} + +llvm::json::Value toJSON(const SourceLLDBData &SLD) { + json::Object result; + if (SLD.persistenceData) + result.insert({"persistenceData", SLD.persistenceData}); + return result; +} + +} // namespace lldb_dap::protocol \ No newline at end of file diff --git a/lldb/tools/lldb-dap/Protocol/DAPTypes.h b/lldb/tools/lldb-dap/Protocol/DAPTypes.h new file mode 100644 index 0000000000000..716d8b491b258 --- /dev/null +++ b/lldb/tools/lldb-dap/Protocol/DAPTypes.h @@ -0,0 +1,53 @@ +//===-- ProtocolTypes.h ---------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// This file contains private DAP types used in the protocol. +// +// Each struct has a toJSON and fromJSON function, that converts between +// the struct and a JSON representation. (See JSON.h) +// +//===----------------------------------------------------------------------===// + +#ifndef LLDB_TOOLS_LLDB_DAP_PROTOCOL_DAP_TYPES_H +#define LLDB_TOOLS_LLDB_DAP_PROTOCOL_DAP_TYPES_H + +#include "lldb/lldb-types.h" +#include "llvm/Support/JSON.h" +#include <optional> +#include <string> + +namespace lldb_dap::protocol { + +/// Data used to help lldb-dap resolve breakpoints persistently across diff erent +/// sessions. This information is especially useful for assembly breakpoints, +/// because `sourceReference` can change across sessions. For regular source +/// breakpoints the path and line are the same For each session. +struct PersistenceData { + /// The source module path. + std::string module_path; + + /// The symbol name of the Source. + std::string symbol_name; +}; +bool fromJSON(const llvm::json::Value &, PersistenceData &, llvm::json::Path); +llvm::json::Value toJSON(const PersistenceData &); + +/// Custom source data used by lldb-dap. +/// This data should help lldb-dap identify sources correctly across diff erent +/// sessions. +struct SourceLLDBData { + /// Data that helps lldb resolve this source persistently across diff erent + /// sessions. + std::optional<PersistenceData> persistenceData; +}; +bool fromJSON(const llvm::json::Value &, SourceLLDBData &, llvm::json::Path); +llvm::json::Value toJSON(const SourceLLDBData &); + +} // namespace lldb_dap::protocol + +#endif diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp index fe8bb5252cc23..369858c3a5f4b 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp +++ b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp @@ -46,7 +46,8 @@ bool fromJSON(const json::Value &Params, Source &S, json::Path P) { json::ObjectMapper O(Params, P); return O && O.map("name", S.name) && O.map("path", S.path) && O.map("presentationHint", S.presentationHint) && - O.map("sourceReference", S.sourceReference); + O.map("sourceReference", S.sourceReference) && + O.map("adapterData", S.adapterData); } llvm::json::Value toJSON(Source::PresentationHint hint) { @@ -71,6 +72,8 @@ llvm::json::Value toJSON(const Source &S) { result.insert({"sourceReference", *S.sourceReference}); if (S.presentationHint) result.insert({"presentationHint", *S.presentationHint}); + if (S.adapterData) + result.insert({"adapterData", *S.adapterData}); return result; } diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h index 89122c8f66307..c4be7911a662b 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h @@ -20,6 +20,7 @@ #ifndef LLDB_TOOLS_LLDB_DAP_PROTOCOL_PROTOCOL_TYPES_H #define LLDB_TOOLS_LLDB_DAP_PROTOCOL_PROTOCOL_TYPES_H +#include "Protocol/DAPTypes.h" #include "lldb/lldb-defines.h" #include "llvm/ADT/DenseSet.h" #include "llvm/Support/JSON.h" @@ -336,7 +337,12 @@ struct Source { /// skipped on stepping. std::optional<PresentationHint> presentationHint; - // unsupported keys: origin, sources, adapterData, checksums + /// Additional data that a debug adapter might want to loop through the + /// client. The client should leave the data intact and persist it across + /// sessions. The client should not interpret the data. + std::optional<SourceLLDBData> adapterData; + + // unsupported keys: origin, sources, checksums }; bool fromJSON(const llvm::json::Value &, Source::PresentationHint &, llvm::json::Path); diff --git a/lldb/tools/lldb-dap/SourceBreakpoint.cpp b/lldb/tools/lldb-dap/SourceBreakpoint.cpp index 5fce9fe0ddbb3..843a5eb09c7ae 100644 --- a/lldb/tools/lldb-dap/SourceBreakpoint.cpp +++ b/lldb/tools/lldb-dap/SourceBreakpoint.cpp @@ -10,7 +10,9 @@ #include "BreakpointBase.h" #include "DAP.h" #include "JSONUtils.h" +#include "ProtocolUtils.h" #include "lldb/API/SBBreakpoint.h" +#include "lldb/API/SBFileSpec.h" #include "lldb/API/SBFileSpecList.h" #include "lldb/API/SBFrame.h" #include "lldb/API/SBInstruction.h" @@ -46,41 +48,20 @@ llvm::Error SourceBreakpoint::SetBreakpoint(const protocol::Source &source) { if (source.sourceReference) { // Breakpoint set by assembly source. - std::optional<lldb::addr_t> raw_addr = - m_dap.GetSourceReferenceAddress(*source.sourceReference); - if (!raw_addr) - return llvm::createStringError(llvm::inconvertibleErrorCode(), - "Invalid sourceReference."); - - lldb::SBAddress source_address(*raw_addr, m_dap.target); - if (!source_address.IsValid()) - return llvm::createStringError(llvm::inconvertibleErrorCode(), - "Invalid sourceReference."); - - lldb::SBSymbol symbol = source_address.GetSymbol(); - if (!symbol.IsValid()) { - // FIXME: Support assembly breakpoints without a valid symbol. - return llvm::createStringError(llvm::inconvertibleErrorCode(), - "Breakpoints in assembly without a valid " - "symbol are not supported yet."); + if (source.adapterData && source.adapterData->persistenceData) { + // Prefer use the adapter persitence data, because this could be a + // breakpoint from a previous session where the `sourceReference` is not + // valid anymore. + if (llvm::Error error = CreateAssemblyBreakpointWithPersistenceData( + *source.adapterData->persistenceData)) + return error; + } else { + if (llvm::Error error = CreateAssemblyBreakpointWithSourceReference( + *source.sourceReference)) + return error; } - - lldb::SBInstructionList inst_list = - m_dap.target.ReadInstructions(symbol.GetStartAddress(), m_line); - if (inst_list.GetSize() < m_line) - return llvm::createStringError(llvm::inconvertibleErrorCode(), - "Invalid instruction list size."); - - lldb::SBAddress address = - inst_list.GetInstructionAtIndex(m_line - 1).GetAddress(); - - m_bp = m_dap.target.BreakpointCreateBySBAddress(address); } else { - // Breakpoint set by a regular source file. - const auto source_path = source.path.value_or(""); - lldb::SBFileSpecList module_list; - m_bp = m_dap.target.BreakpointCreateByLocation(source_path.c_str(), m_line, - m_column, 0, module_list); + CreatePathBreakpoint(source); } if (!m_log_message.empty()) @@ -97,6 +78,60 @@ void SourceBreakpoint::UpdateBreakpoint(const SourceBreakpoint &request_bp) { BreakpointBase::UpdateBreakpoint(request_bp); } +void SourceBreakpoint::CreatePathBreakpoint(const protocol::Source &source) { + const auto source_path = source.path.value_or(""); + lldb::SBFileSpecList module_list; + m_bp = m_dap.target.BreakpointCreateByLocation(source_path.c_str(), m_line, + m_column, 0, module_list); +} + +llvm::Error SourceBreakpoint::CreateAssemblyBreakpointWithSourceReference( + int64_t source_reference) { + std::optional<lldb::addr_t> raw_addr = + m_dap.GetSourceReferenceAddress(source_reference); + if (!raw_addr) + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "Invalid sourceReference."); + + lldb::SBAddress source_address(*raw_addr, m_dap.target); + if (!source_address.IsValid()) + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "Invalid sourceReference."); + + lldb::SBSymbol symbol = source_address.GetSymbol(); + if (!symbol.IsValid()) { + // FIXME: Support assembly breakpoints without a valid symbol. + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "Breakpoints in assembly without a valid " + "symbol are not supported yet."); + } + + lldb::SBInstructionList inst_list = + m_dap.target.ReadInstructions(symbol.GetStartAddress(), m_line); + if (inst_list.GetSize() < m_line) + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "Invalid instruction list size."); + + lldb::SBAddress address = + inst_list.GetInstructionAtIndex(m_line - 1).GetAddress(); + + m_bp = m_dap.target.BreakpointCreateBySBAddress(address); + return llvm::Error::success(); +} + +llvm::Error SourceBreakpoint::CreateAssemblyBreakpointWithPersistenceData( + const protocol::PersistenceData &persistence_data) { + lldb::SBFileSpec file_spec(persistence_data.module_path.c_str()); + lldb::SBFileSpecList comp_unit_list; + lldb::SBFileSpecList file_spec_list; + file_spec_list.Append(file_spec); + m_bp = m_dap.target.BreakpointCreateByName( + persistence_data.symbol_name.c_str(), lldb::eFunctionNameTypeFull, + lldb::eLanguageTypeUnknown, m_line - 1, true, file_spec_list, + comp_unit_list); + return llvm::Error::success(); +} + lldb::SBError SourceBreakpoint::AppendLogMessagePart(llvm::StringRef part, bool is_expr) { if (is_expr) { diff --git a/lldb/tools/lldb-dap/SourceBreakpoint.h b/lldb/tools/lldb-dap/SourceBreakpoint.h index 857ac4286d59d..34054a8dcfd5f 100644 --- a/lldb/tools/lldb-dap/SourceBreakpoint.h +++ b/lldb/tools/lldb-dap/SourceBreakpoint.h @@ -11,6 +11,7 @@ #include "Breakpoint.h" #include "DAPForward.h" +#include "Protocol/DAPTypes.h" #include "Protocol/ProtocolTypes.h" #include "lldb/API/SBError.h" #include "llvm/ADT/StringRef.h" @@ -50,6 +51,12 @@ class SourceBreakpoint : public Breakpoint { uint32_t GetColumn() const { return m_column; } protected: + void CreatePathBreakpoint(const protocol::Source &source); + llvm::Error + CreateAssemblyBreakpointWithSourceReference(int64_t source_reference); + llvm::Error CreateAssemblyBreakpointWithPersistenceData( + const protocol::PersistenceData &persistence_data); + // logMessage part can be either a raw text or an expression. struct LogMessagePart { LogMessagePart(llvm::StringRef text, bool is_expr) _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
