diff --git a/lldb/docs/resources/lldbdap.md b/lldb/docs/resources/lldbdap.md index 955713dd1b594..3838c82ab5dfb 100644 --- a/lldb/docs/resources/lldbdap.md +++ b/lldb/docs/resources/lldbdap.md @@ -170,3 +170,26 @@ This is also very simple, just run: ```bash npm run format ``` + +## Working with the VS Code extension from another extension + +The VS Code extension exposes the following [VS Code +commands](https://code.visualstudio.com/api/extension-guides/command), +which can be invoked by other debugger extensions to leverage this extension's +settings and logic. The commands help resolve configuration, create adapter +descriptor, and get the lldb-dap process for state tracking, additional +interaction, and telemetry. + +``` +// Resolve debug configuration +const resolvedConfiguration = await vscode.commands.executeCommand("lldb-dap.resolveDebugConfiguration", folder, configuration, token); + +// Resolve debug configuration with substituted variables +const resolvedConfigurationWithSubstitutedVariables = await vscode.commands.executeCommand("lldb-dap.resolveDebugConfigurationWithSubstitutedVariables", folder, configuration, token); + +// Create debug adapter descriptor +const adapterDescriptor = await vscode.commands.executeCommand("lldb-dap.createDebugAdapterDescriptor", session, executable); + +// Get DAP server process +const process = await vscode.commands.executeCommand("lldb-dap.getServerProcess"); +``` diff --git a/lldb/include/lldb/API/SBTarget.h b/lldb/include/lldb/API/SBTarget.h index fc57245a80254..4381781383075 100644 --- a/lldb/include/lldb/API/SBTarget.h +++ b/lldb/include/lldb/API/SBTarget.h @@ -660,6 +660,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 dadd6d267bcf6..476715ebb9ce1 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -19,6 +19,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" @@ -817,7 +818,7 @@ class Target : public std::enable_shared_from_this, 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, @@ -846,8 +847,8 @@ class Target : public std::enable_shared_from_this, 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, @@ -1438,6 +1439,10 @@ class Target : public std::enable_shared_from_this, const lldb_private::RegisterFlags &flags, uint32_t byte_size); + llvm::Expected + 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 d823126e3e2fd..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, ) @@ -173,6 +171,28 @@ def verify_breakpoint_hit(self, breakpoint_ids, timeout=DEFAULT_TIMEOUT): return self.assertTrue(False, f"breakpoint not hit, stopped_events={stopped_events}") + def verify_all_breakpoints_hit(self, breakpoint_ids, timeout=DEFAULT_TIMEOUT): + """Wait for the process we are debugging to stop, and verify we hit + all of the breakpoint locations in the "breakpoint_ids" array. + "breakpoint_ids" should be a list of int breakpoint IDs ([1, 2]).""" + stopped_events = self.dap_server.wait_for_stopped(timeout) + for stopped_event in stopped_events: + if "body" in stopped_event: + body = stopped_event["body"] + if "reason" not in body: + continue + if ( + body["reason"] != "breakpoint" + and body["reason"] != "instruction breakpoint" + ): + continue + if "hitBreakpointIds" not in body: + continue + hit_bps = body["hitBreakpointIds"] + if all(breakpoint_id in hit_bps for breakpoint_id in breakpoint_ids): + return + self.assertTrue(False, f"breakpoints not hit, stopped_events={stopped_events}") + def verify_stop_exception_info(self, expected_description, timeout=DEFAULT_TIMEOUT): """Wait for the process we are debugging to stop, and verify the stop reason is 'exception' and that the description matches diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp index 0cc25d0139f74..34f3c261719b2 100644 --- a/lldb/source/API/SBTarget.cpp +++ b/lldb/source/API/SBTarget.cpp @@ -771,16 +771,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); } } @@ -816,6 +819,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(); @@ -826,7 +840,8 @@ lldb::SBBreakpoint SBTarget::BreakpointCreateByName( std::lock_guard guard(target_sp->GetAPIMutex()); FunctionNameType mask = static_cast(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); } @@ -2013,29 +2028,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 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( 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 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 edde1c91b789c..33ebbab302ed5 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 resolver_sp = std::make_shared( 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/Expression/IRExecutionUnit.cpp b/lldb/source/Expression/IRExecutionUnit.cpp index 5746bf3eb7cbd..f1958ad947de0 100644 --- a/lldb/source/Expression/IRExecutionUnit.cpp +++ b/lldb/source/Expression/IRExecutionUnit.cpp @@ -721,8 +721,9 @@ class LoadAddressResolver { // If that didn't work, try the function. if (load_address == LLDB_INVALID_ADDRESS && candidate_sc.function) { Address addr = candidate_sc.function->GetAddress(); - load_address = m_target.GetProcessSP() ? addr.GetLoadAddress(&m_target) - : addr.GetFileAddress(); + load_address = m_target.GetProcessSP() + ? addr.GetCallableLoadAddress(&m_target) + : addr.GetFileAddress(); } // We found a load address. diff --git a/lldb/source/Host/common/NativeProcessProtocol.cpp b/lldb/source/Host/common/NativeProcessProtocol.cpp index 405acbb5662d6..196f54b93538d 100644 --- a/lldb/source/Host/common/NativeProcessProtocol.cpp +++ b/lldb/source/Host/common/NativeProcessProtocol.cpp @@ -366,12 +366,19 @@ Status NativeProcessProtocol::RemoveSoftwareBreakpoint(lldb::addr_t addr) { if (--it->second.ref_count > 0) return Status(); + // Remove the entry from m_software_breakpoints rightaway, so that we don't + // leave behind an entry with ref_count == 0 in case one of the following + // conditions returns an error. The breakpoint is moved so that it can be + // accessed below. + SoftwareBreakpoint bkpt = std::move(it->second); + m_software_breakpoints.erase(it); + // This is the last reference. Let's remove the breakpoint. Status error; // Clear a software breakpoint instruction - llvm::SmallVector curr_break_op( - it->second.breakpoint_opcodes.size(), 0); + llvm::SmallVector curr_break_op(bkpt.breakpoint_opcodes.size(), + 0); // Read the breakpoint opcode size_t bytes_read = 0; @@ -382,10 +389,10 @@ Status NativeProcessProtocol::RemoveSoftwareBreakpoint(lldb::addr_t addr) { "addr=0x%" PRIx64 ": tried to read %zu bytes but only read %zu", addr, curr_break_op.size(), bytes_read); } - const auto &saved = it->second.saved_opcodes; + const auto &saved = bkpt.saved_opcodes; // Make sure the breakpoint opcode exists at this address - if (llvm::ArrayRef(curr_break_op) != it->second.breakpoint_opcodes) { - if (curr_break_op != it->second.saved_opcodes) + if (llvm::ArrayRef(curr_break_op) != bkpt.breakpoint_opcodes) { + if (curr_break_op != bkpt.saved_opcodes) return Status::FromErrorString( "Original breakpoint trap is no longer in memory."); LLDB_LOG(log, @@ -418,7 +425,6 @@ Status NativeProcessProtocol::RemoveSoftwareBreakpoint(lldb::addr_t addr) { llvm::make_range(saved.begin(), saved.end())); } - m_software_breakpoints.erase(it); return Status(); } 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/ExpressionParser/Swift/SwiftREPL.cpp b/lldb/source/Plugins/ExpressionParser/Swift/SwiftREPL.cpp index 029c39444dd5a..7eb10de4ea3cb 100644 --- a/lldb/source/Plugins/ExpressionParser/Swift/SwiftREPL.cpp +++ b/lldb/source/Plugins/ExpressionParser/Swift/SwiftREPL.cpp @@ -173,6 +173,7 @@ lldb::REPLSP SwiftREPL::CreateInstanceFromDebugger(Status &err, eFunctionNameTypeAuto, // Name type eLanguageTypeUnknown, // Language 0, // offset + false, // offset_is_insn_count eLazyBoolYes, // skip_prologue, true, // internal false); // request_hardware 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( 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 d8c6da1037c3f..d2b9a1477a234 100644 --- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp +++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp @@ -1176,7 +1176,7 @@ AppleObjCRuntimeV2::CreateExceptionResolver(const BreakpointSP &bkpt, resolver_sp = std::make_shared( 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( 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 82f18c5fe37a2..9537c842f4419 100644 --- a/lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp +++ b/lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp @@ -1603,6 +1603,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; @@ -1610,7 +1611,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 a3414c89375aa..762b97868d6fb 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -581,10 +581,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( + shared_from_this()); + BreakpointResolverSP resolver_sp = + std::make_shared(nullptr, addr); return CreateBreakpoint(filter_sp, resolver_sp, internal, hardware, false); } @@ -592,10 +593,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( + shared_from_this()); + BreakpointResolverSP resolver_sp = + std::make_shared(nullptr, file_addr, + file_spec); return CreateBreakpoint(filter_sp, resolver_sp, internal, request_hardware, false); } @@ -604,7 +607,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( @@ -617,7 +621,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; @@ -3093,6 +3097,38 @@ lldb::addr_t Target::GetBreakableLoadAddress(lldb::addr_t addr) { return arch_plugin ? arch_plugin->GetBreakableLoadAddress(addr, *this) : addr; } +llvm::Expected +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(shared_from_this()); diff --git a/lldb/source/Target/ThreadPlanCallFunction.cpp b/lldb/source/Target/ThreadPlanCallFunction.cpp index a3ef537c4b194..445d25177a729 100644 --- a/lldb/source/Target/ThreadPlanCallFunction.cpp +++ b/lldb/source/Target/ThreadPlanCallFunction.cpp @@ -451,10 +451,11 @@ void ThreadPlanCallFunction::SetBreakpoints() { const LazyBool skip_prologue = eLazyBoolNo; const bool is_internal = true; const bool is_hardware = false; + const bool offset_is_insn_count = false; m_error_backstop_bp_sp = m_process.GetTarget().CreateBreakpoint( - &stdlib_module_list, NULL, backstop_name.str().c_str(), - eFunctionNameTypeFull, eLanguageTypeUnknown, 0, skip_prologue, - is_internal, is_hardware); + &stdlib_module_list, NULL, backstop_name.str().c_str(), + eFunctionNameTypeFull, eLanguageTypeUnknown, 0, + offset_is_insn_count, skip_prologue, is_internal, is_hardware); } } } diff --git a/lldb/test/API/arm/thumb-function-addr/Makefile b/lldb/test/API/arm/thumb-function-addr/Makefile new file mode 100644 index 0000000000000..10495940055b6 --- /dev/null +++ b/lldb/test/API/arm/thumb-function-addr/Makefile @@ -0,0 +1,3 @@ +C_SOURCES := main.c + +include Makefile.rules diff --git a/lldb/test/API/arm/thumb-function-addr/TestThumbFunctionAddr.py b/lldb/test/API/arm/thumb-function-addr/TestThumbFunctionAddr.py new file mode 100644 index 0000000000000..d08099f6331e5 --- /dev/null +++ b/lldb/test/API/arm/thumb-function-addr/TestThumbFunctionAddr.py @@ -0,0 +1,67 @@ +""" +Test that addresses of functions compiled for Arm Thumb include the Thumb mode +bit (bit 0 of the address) when resolved and used in expressions. +""" + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class TestThumbFunctionAddr(TestBase): + def do_thumb_function_test(self, language): + self.build(dictionary={"CFLAGS_EXTRAS": f"-x {language} -mthumb"}) + + exe = self.getBuildArtifact("a.out") + line = line_number("main.c", "// Set break point at this line.") + self.runCmd("target create %s" % exe) + bpid = lldbutil.run_break_set_by_file_and_line(self, "main.c", line) + + self.runCmd("run") + self.assertIsNotNone( + lldbutil.get_one_thread_stopped_at_breakpoint_id(self.process(), bpid), + "Process is not stopped at breakpoint", + ) + + # The compiler set this, so the mode bit will be included here. + a_function_addr_var = ( + self.thread().GetFrameAtIndex(0).FindVariable("a_function_addr") + ) + self.assertTrue(a_function_addr_var.IsValid()) + a_function_addr = a_function_addr_var.GetValueAsUnsigned() + self.assertTrue(a_function_addr & 1) + + self.expect("p/x a_function_addr", substrs=[f"0x{a_function_addr:08x}"]) + # If lldb did not pay attention to the mode bit this would SIGILL trying + # to execute Thumb encodings in Arm mode. + self.expect("expression -- a_function()", substrs=["= 123"]) + + # We cannot call GetCallableLoadAdress via. the API, so we expect this + # to not have the bit set as it's treating it as a non-function symbol. + found_function = self.target().FindFunctions("a_function")[0] + self.assertTrue(found_function.IsValid()) + found_function = found_function.GetFunction() + self.assertTrue(found_function.IsValid()) + found_function_addr = found_function.GetStartAddress() + a_function_load_addr = found_function_addr.GetLoadAddress(self.target()) + self.assertEqual(a_function_load_addr, a_function_addr & ~1) + + # image lookup should not include the mode bit. + a_function_file_addr = found_function_addr.GetFileAddress() + self.expect( + "image lookup -n a_function", substrs=[f"0x{a_function_file_addr:08x}"] + ) + + # This test is run for C and C++ because the two will take different paths + # trying to resolve the function's address. + + @skipIf(archs=no_match(["arm$"])) + @skipIf(archs=["arm64"]) + def test_function_addr_c(self): + self.do_thumb_function_test("c") + + @skipIf(archs=no_match(["arm$"])) + @skipIf(archs=["arm64"]) + def test_function_addr_cpp(self): + self.do_thumb_function_test("c++") diff --git a/lldb/test/API/arm/thumb-function-addr/main.c b/lldb/test/API/arm/thumb-function-addr/main.c new file mode 100644 index 0000000000000..f3e01b78f575b --- /dev/null +++ b/lldb/test/API/arm/thumb-function-addr/main.c @@ -0,0 +1,9 @@ +#include + +int a_function() { return 123; } + +int main() { + const uintptr_t a_function_addr = (uintptr_t)a_function; + // Set break point at this line. + return a_function(); +} 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/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py index 831edd6494c1e..2e860ff5d5e17 100644 --- a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py +++ b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py @@ -398,3 +398,26 @@ def test_column_breakpoints(self): self.stepIn() func_name = self.get_stackFrames()[0]["name"] self.assertEqual(func_name, "a::fourteen(int)") + + @skipIfWindows + def test_hit_multiple_breakpoints(self): + """Test that if we hit multiple breakpoints at the same address, they + all appear in the stop reason.""" + breakpoint_lines = [ + line_number("main.cpp", "// break non-breakpointable line"), + line_number("main.cpp", "// before loop"), + ] + + program = self.getBuildArtifact("a.out") + self.build_and_launch(program) + + # Set a pair of breakpoints that will both resolve to the same address. + breakpoint_ids = [ + int(bp_id) + for bp_id in self.set_source_breakpoints(self.main_path, breakpoint_lines) + ] + self.assertEqual(len(breakpoint_ids), 2, "expected two breakpoints") + self.dap_server.request_continue() + print(breakpoint_ids) + # Verify we hit both of the breakpoints we just set + self.verify_all_breakpoints_hit(breakpoint_ids) diff --git a/lldb/test/API/tools/lldb-dap/breakpoint/main.cpp b/lldb/test/API/tools/lldb-dap/breakpoint/main.cpp index a84546a95af15..2206b07f19494 100644 --- a/lldb/test/API/tools/lldb-dap/breakpoint/main.cpp +++ b/lldb/test/API/tools/lldb-dap/breakpoint/main.cpp @@ -33,7 +33,7 @@ int main(int argc, char const *argv[]) { if (foo == nullptr) { fprintf(stderr, "%s\n", dlerror()); exit(2); - } + } // break non-breakpointable line foo(12); // before loop for (int i = 0; i < 10; ++i) { diff --git a/lldb/test/API/tools/lldb-dap/coreFile/TestDAP_coreFile.py b/lldb/test/API/tools/lldb-dap/coreFile/TestDAP_coreFile.py index db43dbaf515cf..1143cd93a70b3 100644 --- a/lldb/test/API/tools/lldb-dap/coreFile/TestDAP_coreFile.py +++ b/lldb/test/API/tools/lldb-dap/coreFile/TestDAP_coreFile.py @@ -26,6 +26,7 @@ def test_core_file(self): "column": 0, "id": 524288, "line": 4, + "moduleId": "01DF54A6-045E-657D-3F8F-FB9CE1118789-14F8BD6D", "name": "bar", "source": {"name": "main.c", "path": "/home/labath/test/main.c"}, "instructionPointerReference": "0x40011C", @@ -34,6 +35,7 @@ def test_core_file(self): "column": 0, "id": 524289, "line": 10, + "moduleId": "01DF54A6-045E-657D-3F8F-FB9CE1118789-14F8BD6D", "name": "foo", "source": {"name": "main.c", "path": "/home/labath/test/main.c"}, "instructionPointerReference": "0x400142", @@ -42,6 +44,7 @@ def test_core_file(self): "column": 0, "id": 524290, "line": 16, + "moduleId": "01DF54A6-045E-657D-3F8F-FB9CE1118789-14F8BD6D", "name": "_start", "source": {"name": "main.c", "path": "/home/labath/test/main.c"}, "instructionPointerReference": "0x40015F", diff --git a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py index 94a0ac698b80c..2c0c5a583c58a 100644 --- a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py +++ b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py @@ -431,6 +431,8 @@ def test_commands(self): self.verify_commands("exitCommands", output, exitCommands) self.verify_commands("terminateCommands", output, terminateCommands) + # Flakey on 32-bit Arm Linux. + @skipIf(oslist=["linux"], archs=["arm$"]) def test_extra_launch_commands(self): """ Tests the "launchCommands" with extra launching settings diff --git a/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py b/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py index 2c3c1fe9cae6b..f51056d7020c6 100644 --- a/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py +++ b/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py @@ -126,6 +126,8 @@ def test_readMemory(self): self.continue_to_exit() + # Flakey on 32-bit Arm Linux. + @skipIf(oslist=["linux"], archs=["arm$"]) def test_writeMemory(self): """ Tests the 'writeMemory' request diff --git a/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py b/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py index abd469274ffd4..fd2037b5762d1 100644 --- a/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py +++ b/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py @@ -242,3 +242,36 @@ def test_StackFrameFormat(self): frame = self.get_stackFrames(format={"parameters": False, "module": True})[0] self.assertEqual(frame["name"], "a.out recurse") + + @skipIfWindows + def test_stack_frame_module_id(self): + program = self.getBuildArtifact("a.out") + self.build_and_launch(program) + source = "main.c" + lines = [line_number(source, "recurse end")] + breakpoint_ids = self.set_source_breakpoints(source, lines) + self.assertEqual( + len(breakpoint_ids), len(lines), "expect correct number of breakpoints" + ) + + self.continue_to_breakpoints(breakpoint_ids) + + modules = self.dap_server.get_modules() + name_to_id = { + name: info["id"] for name, info in modules.items() if "id" in info + } + + stack_frames = self.get_stackFrames() + for frame in stack_frames: + module_id = frame.get("moduleId") + source_name = frame.get("source", {}).get("name") + if module_id is None or source_name is None: + continue + + if source_name in name_to_id: + expected_id = name_to_id[source_name] + self.assertEqual( + module_id, + expected_id, + f"Expected moduleId '{expected_id}' for {source_name}, got: {module_id}", + ) 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 @@ -21,6 +25,22 @@ using namespace lldb_dap; +static std::optional +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 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 fd89f52595ec6..849712f724c69 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -983,7 +983,7 @@ llvm::Error DAP::Loop() { if (const protocol::Request *req = std::get_if(&*next); - req && req->arguments == "disconnect") + req && req->command == "disconnect") disconnecting = true; const std::optional cancel_args = @@ -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 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 namespace lldb_dap { diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp index 41ca29a405ac9..4f26599a49bac 100644 --- a/lldb/tools/lldb-dap/JSONUtils.cpp +++ b/lldb/tools/lldb-dap/JSONUtils.cpp @@ -550,6 +550,13 @@ llvm::json::Value CreateStackFrame(DAP &dap, lldb::SBFrame &frame, if (frame.IsArtificial() || frame.IsHidden()) object.try_emplace("presentationHint", "subtle"); + lldb::SBModule module = frame.GetModule(); + if (module.IsValid()) { + std::string uuid = module.GetUUIDString(); + if (!uuid.empty()) + object.try_emplace("moduleId", uuid); + } + return llvm::json::Value(std::move(object)); } @@ -654,12 +661,17 @@ llvm::json::Value CreateThreadStopped(DAP &dap, lldb::SBThread &thread, } else { body.try_emplace("reason", "breakpoint"); } - lldb::break_id_t bp_id = thread.GetStopReasonDataAtIndex(0); - lldb::break_id_t bp_loc_id = thread.GetStopReasonDataAtIndex(1); - std::string desc_str = - llvm::formatv("breakpoint {0}.{1}", bp_id, bp_loc_id); - body.try_emplace("hitBreakpointIds", - llvm::json::Array{llvm::json::Value(bp_id)}); + std::vector bp_ids; + std::ostringstream desc_sstream; + desc_sstream << "breakpoint"; + for (size_t idx = 0; idx < thread.GetStopReasonDataCount(); idx += 2) { + lldb::break_id_t bp_id = thread.GetStopReasonDataAtIndex(idx); + lldb::break_id_t bp_loc_id = thread.GetStopReasonDataAtIndex(idx + 1); + bp_ids.push_back(bp_id); + desc_sstream << " " << bp_id << "." << bp_loc_id; + } + std::string desc_str = desc_sstream.str(); + body.try_emplace("hitBreakpointIds", llvm::json::Array(bp_ids)); EmplaceSafeString(body, "description", desc_str); } } break; 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 +#include + +namespace lldb_dap::protocol { + +/// Data used to help lldb-dap resolve breakpoints persistently across different +/// 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 different +/// sessions. +struct SourceLLDBData { + /// Data that helps lldb resolve this source persistently across different + /// sessions. + std::optional 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; - // 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 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 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 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) diff --git a/lldb/tools/lldb-dap/package-lock.json b/lldb/tools/lldb-dap/package-lock.json index af90a9573aee6..1969b196accc6 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.14", + "version": "0.2.15", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "lldb-dap", - "version": "0.2.14", + "version": "0.2.15", "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 801abe73edd7d..d677a81cc7974 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.15", + "version": "0.2.16", "publisher": "llvm-vs-code-extensions", "homepage": "https://lldb.llvm.org", "description": "Debugging with LLDB in Visual Studio Code", @@ -45,7 +45,7 @@ "vscode:prepublish": "tsc -p ./", "watch": "tsc -watch -p ./", "format": "npx prettier './src-ts/' --write", - "package": "vsce package --out ./out/lldb-dap.vsix", + "package": "rm -rf ./out/lldb-dap.vsix && vsce package --out ./out/lldb-dap.vsix", "publish": "vsce publish", "vscode-uninstall": "code --uninstall-extension llvm-vs-code-extensions.lldb-dap", "vscode-install": "code --install-extension ./out/lldb-dap.vsix" @@ -81,10 +81,16 @@ "description": "The path to the lldb-dap binary, e.g. /usr/local/bin/lldb-dap" }, "lldb-dap.log-path": { + "scope": "machine-overridable", + "type": "string", + "description": "The log path for lldb-dap (if any)", + "markdownDeprecationMessage": "Use the `#lldb-dap.logFolder#` setting instead" + }, + "lldb-dap.logFolder": { "order": 0, "scope": "machine-overridable", "type": "string", - "description": "The log path for lldb-dap (if any)" + "markdownDescription": "The folder to persist lldb-dap logs. If no value is provided, logs will be persisted in the [Extension Logs Folder](command:workbench.action.openExtensionLogsFolder)." }, "lldb-dap.serverMode": { "order": 0, @@ -110,6 +116,11 @@ "additionalProperties": { "type": "string" } + }, + "lldb-dap.captureSessionLogs": { + "type": "boolean", + "description": "When enabled, LLDB-DAP session logs will be written to the Extension's log folder if the `lldb-dap.log-path` setting is not explicitly set.", + "default": false } } }, diff --git a/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts b/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts index b5db45b56d6a6..157aa2ac76a1f 100644 --- a/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts +++ b/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts @@ -5,6 +5,7 @@ import * as child_process from "child_process"; import * as fs from "node:fs/promises"; import { ConfigureButton, OpenSettingsButton } from "./ui/show-error-message"; import { ErrorWithNotification } from "./ui/error-with-notification"; +import { LogFilePathProvider, LogType } from "./logging"; const exec = util.promisify(child_process.execFile); @@ -160,20 +161,30 @@ async function getDAPArguments( * Creates a new {@link vscode.DebugAdapterExecutable} based on the provided workspace folder and * debug configuration. Assumes that the given debug configuration is for a local launch of lldb-dap. * + * @param logger The {@link vscode.LogOutputChannel} to log setup diagnostics + * @param logFilePath The {@link LogFilePathProvider} for determining where to put session logs * @param workspaceFolder The {@link vscode.WorkspaceFolder} that the debug session will be launched within * @param configuration The {@link vscode.DebugConfiguration} that will be launched * @throws An {@link ErrorWithNotification} if something went wrong * @returns The {@link vscode.DebugAdapterExecutable} that can be used to launch lldb-dap */ export async function createDebugAdapterExecutable( + logger: vscode.LogOutputChannel, + logFilePath: LogFilePathProvider, workspaceFolder: vscode.WorkspaceFolder | undefined, configuration: vscode.DebugConfiguration, ): Promise { - const config = vscode.workspace.workspaceFile ? vscode.workspace.getConfiguration("lldb-dap") : vscode.workspace.getConfiguration("lldb-dap", workspaceFolder); + const config = vscode.workspace.workspaceFile + ? vscode.workspace.getConfiguration("lldb-dap") + : vscode.workspace.getConfiguration("lldb-dap", workspaceFolder); const log_path = config.get("log-path"); let env: { [key: string]: string } = {}; if (log_path) { env["LLDBDAP_LOG"] = log_path; + } else if ( + vscode.workspace.getConfiguration("lldb-dap").get("captureSessionLogs", false) + ) { + env["LLDBDAP_LOG"] = logFilePath.get(LogType.DEBUG_SESSION); } const configEnvironment = config.get<{ [key: string]: string }>("environment") || {}; @@ -188,6 +199,11 @@ export async function createDebugAdapterExecutable( }; const dbgArgs = await getDAPArguments(workspaceFolder, configuration); + logger.info(`lldb-dap path: ${dapPath}`); + logger.info(`lldb-dap args: ${dbgArgs}`); + logger.info(`cwd: ${dbgOptions.cwd}`); + logger.info(`env: ${JSON.stringify(dbgOptions.env)}`); + return new vscode.DebugAdapterExecutable(dapPath, dbgArgs, dbgOptions); } @@ -198,18 +214,41 @@ export async function createDebugAdapterExecutable( export class LLDBDapDescriptorFactory implements vscode.DebugAdapterDescriptorFactory { + constructor( + private readonly logger: vscode.LogOutputChannel, + private logFilePath: LogFilePathProvider, + ) { + vscode.commands.registerCommand( + "lldb-dap.createDebugAdapterDescriptor", + ( + session: vscode.DebugSession, + executable: vscode.DebugAdapterExecutable | undefined, + ) => this.createDebugAdapterDescriptor(session, executable), + ); + } + async createDebugAdapterDescriptor( session: vscode.DebugSession, executable: vscode.DebugAdapterExecutable | undefined, ): Promise { + this.logger.info(`Creating debug adapter for session "${session.name}"`); + this.logger.info( + `Session "${session.name}" debug configuration:\n` + + JSON.stringify(session.configuration, undefined, 2), + ); if (executable) { - throw new Error( + const error = new Error( "Setting the debug adapter executable in the package.json is not supported.", ); + this.logger.error(error); + throw error; } // Use a server connection if the debugAdapterPort is provided if (session.configuration.debugAdapterPort) { + this.logger.info( + `Spawning debug adapter server on port ${session.configuration.debugAdapterPort}`, + ); return new vscode.DebugAdapterServer( session.configuration.debugAdapterPort, session.configuration.debugAdapterHostname, @@ -217,6 +256,8 @@ export class LLDBDapDescriptorFactory } return createDebugAdapterExecutable( + this.logger, + this.logFilePath, session.workspaceFolder, session.configuration, ); diff --git a/lldb/tools/lldb-dap/src-ts/debug-configuration-provider.ts b/lldb/tools/lldb-dap/src-ts/debug-configuration-provider.ts index 316ffaf47c3d2..1e16dac031125 100644 --- a/lldb/tools/lldb-dap/src-ts/debug-configuration-provider.ts +++ b/lldb/tools/lldb-dap/src-ts/debug-configuration-provider.ts @@ -5,6 +5,7 @@ import { LLDBDapServer } from "./lldb-dap-server"; import { createDebugAdapterExecutable } from "./debug-adapter-factory"; import { ConfigureButton, showErrorMessage } from "./ui/show-error-message"; import { ErrorWithNotification } from "./ui/error-with-notification"; +import { LogFilePathProvider } from "./logging"; const exec = util.promisify(child_process.execFile); @@ -71,13 +72,46 @@ const configurations: Record = { export class LLDBDapConfigurationProvider implements vscode.DebugConfigurationProvider { - constructor(private readonly server: LLDBDapServer) {} + constructor( + private readonly server: LLDBDapServer, + private readonly logger: vscode.LogOutputChannel, + private readonly logFilePath: LogFilePathProvider, + ) { + vscode.commands.registerCommand( + "lldb-dap.resolveDebugConfiguration", + ( + folder: vscode.WorkspaceFolder | undefined, + debugConfiguration: vscode.DebugConfiguration, + token?: vscode.CancellationToken, + ) => this.resolveDebugConfiguration(folder, debugConfiguration, token), + ); + vscode.commands.registerCommand( + "lldb-dap.resolveDebugConfigurationWithSubstitutedVariables", + ( + folder: vscode.WorkspaceFolder | undefined, + debugConfiguration: vscode.DebugConfiguration, + token?: vscode.CancellationToken, + ) => + this.resolveDebugConfigurationWithSubstitutedVariables( + folder, + debugConfiguration, + token, + ), + ); + } async resolveDebugConfiguration( folder: vscode.WorkspaceFolder | undefined, debugConfiguration: vscode.DebugConfiguration, token?: vscode.CancellationToken, ): Promise { + this.logger.info( + `Resolving debug configuration for "${debugConfiguration.name}"`, + ); + this.logger.debug( + "Initial debug configuration:\n" + + JSON.stringify(debugConfiguration, undefined, 2), + ); let config = vscode.workspace.getConfiguration("lldb-dap"); for (const [key, cfg] of Object.entries(configurations)) { if (Reflect.has(debugConfiguration, key)) { @@ -152,6 +186,8 @@ export class LLDBDapConfigurationProvider // Always try to create the debug adapter executable as this will show the user errors // if there are any. const executable = await createDebugAdapterExecutable( + this.logger, + this.logFilePath, folder, debugConfiguration, ); @@ -184,8 +220,14 @@ export class LLDBDapConfigurationProvider } } + this.logger.info( + "Resolved debug configuration:\n" + + JSON.stringify(debugConfiguration, undefined, 2), + ); + return debugConfiguration; } catch (error) { + this.logger.error(error as Error); // Show a better error message to the user if possible if (!(error instanceof ErrorWithNotification)) { throw error; diff --git a/lldb/tools/lldb-dap/src-ts/debug-session-tracker.ts b/lldb/tools/lldb-dap/src-ts/debug-session-tracker.ts index 50db1e1c3a7b0..7d7f73dbff92d 100644 --- a/lldb/tools/lldb-dap/src-ts/debug-session-tracker.ts +++ b/lldb/tools/lldb-dap/src-ts/debug-session-tracker.ts @@ -5,6 +5,7 @@ import * as vscode from "vscode"; // prettier-ignore interface EventMap { "module": DebugProtocol.ModuleEvent; + "exited": DebugProtocol.ExitedEvent; } /** A type assertion to check if a ProtocolMessage is an event or if it is a specific event. */ @@ -47,7 +48,7 @@ export class DebugSessionTracker onDidChangeModules: vscode.Event = this.modulesChanged.event; - constructor() { + constructor(private logger: vscode.LogOutputChannel) { this.onDidChangeModules(this.moduleChangedListener, this); vscode.debug.onDidChangeActiveDebugSession((session) => this.modulesChanged.fire(session), @@ -62,8 +63,12 @@ export class DebugSessionTracker createDebugAdapterTracker( session: vscode.DebugSession, ): vscode.ProviderResult { + this.logger.info(`Starting debug session "${session.name}"`); + let stopping = false; return { + onError: (error) => !stopping && this.logger.error(error), // Can throw benign read errors when shutting down. onDidSendMessage: (message) => this.onDidSendMessage(session, message), + onWillStopSession: () => (stopping = true), onExit: () => this.onExit(session), }; } @@ -134,6 +139,13 @@ export class DebugSessionTracker } this.modules.set(session, modules); this.modulesChanged.fire(session); + } else if (isEvent(message, "exited")) { + // The vscode.DebugAdapterTracker#onExit event is sometimes called with + // exitCode = undefined but the exit event from LLDB-DAP always has the "exitCode" + const { exitCode } = message.body; + this.logger.info( + `Session "${session.name}" exited with code ${exitCode}`, + ); } } } diff --git a/lldb/tools/lldb-dap/src-ts/extension.ts b/lldb/tools/lldb-dap/src-ts/extension.ts index c8e5146e29cea..4b7a35e6944c6 100644 --- a/lldb/tools/lldb-dap/src-ts/extension.ts +++ b/lldb/tools/lldb-dap/src-ts/extension.ts @@ -1,3 +1,4 @@ +import * as path from "path"; import * as vscode from "vscode"; import { LLDBDapDescriptorFactory } from "./debug-adapter-factory"; @@ -10,28 +11,35 @@ import { ModulesDataProvider, ModuleProperty, } from "./ui/modules-data-provider"; +import { LogFilePathProvider } from "./logging"; /** * This class represents the extension and manages its life cycle. Other extensions * using it as as library should use this class as the main entry point. */ export class LLDBDapExtension extends DisposableContext { - constructor() { + constructor( + logger: vscode.LogOutputChannel, + logFilePath: LogFilePathProvider, + outputChannel: vscode.OutputChannel, + ) { super(); const lldbDapServer = new LLDBDapServer(); - const sessionTracker = new DebugSessionTracker(); + const sessionTracker = new DebugSessionTracker(logger); this.pushSubscription( + logger, + outputChannel, lldbDapServer, sessionTracker, vscode.debug.registerDebugConfigurationProvider( "lldb-dap", - new LLDBDapConfigurationProvider(lldbDapServer), + new LLDBDapConfigurationProvider(lldbDapServer, logger, logFilePath), ), vscode.debug.registerDebugAdapterDescriptorFactory( "lldb-dap", - new LLDBDapDescriptorFactory(), + new LLDBDapDescriptorFactory(logger, logFilePath), ), vscode.debug.registerDebugAdapterTrackerFactory( "lldb-dap", @@ -54,6 +62,12 @@ export class LLDBDapExtension extends DisposableContext { /** * This is the entry point when initialized by VS Code. */ -export function activate(context: vscode.ExtensionContext) { - context.subscriptions.push(new LLDBDapExtension()); +export async function activate(context: vscode.ExtensionContext) { + const outputChannel = vscode.window.createOutputChannel("LLDB-DAP", { log: true }); + outputChannel.info("LLDB-DAP extension activating..."); + const logFilePath = new LogFilePathProvider(context, outputChannel); + context.subscriptions.push( + new LLDBDapExtension(outputChannel, logFilePath, outputChannel), + ); + outputChannel.info("LLDB-DAP extension activated"); } diff --git a/lldb/tools/lldb-dap/src-ts/lldb-dap-server.ts b/lldb/tools/lldb-dap/src-ts/lldb-dap-server.ts index 79573ec7342b1..5f9d8efdcb3a3 100644 --- a/lldb/tools/lldb-dap/src-ts/lldb-dap-server.ts +++ b/lldb/tools/lldb-dap/src-ts/lldb-dap-server.ts @@ -12,6 +12,13 @@ export class LLDBDapServer implements vscode.Disposable { private serverProcess?: child_process.ChildProcessWithoutNullStreams; private serverInfo?: Promise<{ host: string; port: number }>; + constructor() { + vscode.commands.registerCommand( + "lldb-dap.getServerProcess", + () => this.serverProcess, + ); + } + /** * Starts the server with the provided options. The server will be restarted or reused as * necessary. @@ -26,7 +33,7 @@ export class LLDBDapServer implements vscode.Disposable { args: string[], options?: child_process.SpawnOptionsWithoutStdio, ): Promise<{ host: string; port: number } | undefined> { - const dapArgs = [...args, "--connection", "listen://localhost:0" ]; + const dapArgs = [...args, "--connection", "listen://localhost:0"]; if (!(await this.shouldContinueStartup(dapPath, dapArgs))) { return undefined; } @@ -39,8 +46,7 @@ export class LLDBDapServer implements vscode.Disposable { const process = child_process.spawn(dapPath, dapArgs, options); process.on("error", (error) => { reject(error); - this.serverProcess = undefined; - this.serverInfo = undefined; + this.cleanUp(process); }); process.on("exit", (code, signal) => { let errorMessage = "Server process exited early"; @@ -50,8 +56,7 @@ export class LLDBDapServer implements vscode.Disposable { errorMessage += ` due to signal ${signal}`; } reject(new Error(errorMessage)); - this.serverProcess = undefined; - this.serverInfo = undefined; + this.cleanUp(process); }); process.stdout.setEncoding("utf8").on("data", (data) => { const connection = /connection:\/\/\[([^\]]+)\]:(\d+)/.exec( @@ -126,7 +131,16 @@ Restarting the server will interrupt any existing debug sessions and start a new return; } this.serverProcess.kill(); - this.serverProcess = undefined; - this.serverInfo = undefined; + this.cleanUp(this.serverProcess); + } + + cleanUp(process: child_process.ChildProcessWithoutNullStreams) { + // If the following don't equal, then the fields have already been updated + // (either a new process has started, or the fields were already cleaned + // up), and so the cleanup should be skipped. + if (this.serverProcess === process) { + this.serverProcess = undefined; + this.serverInfo = undefined; + } } } diff --git a/lldb/tools/lldb-dap/src-ts/logging.ts b/lldb/tools/lldb-dap/src-ts/logging.ts new file mode 100644 index 0000000000000..3b1c3c37ce1ce --- /dev/null +++ b/lldb/tools/lldb-dap/src-ts/logging.ts @@ -0,0 +1,67 @@ +import * as path from "path"; +import * as vscode from "vscode"; + +/** + * Formats the given date as a string in the form "YYYYMMddTHHMMSS". + * + * @param date The date to format as a string. + * @returns The formatted date. + */ +function formatDate(date: Date): string { + const year = date.getFullYear().toString().padStart(4, "0"); + const month = (date.getMonth() + 1).toString().padStart(2, "0"); + const day = date.getDate().toString().padStart(2, "0"); + const hour = date.getHours().toString().padStart(2, "0"); + const minute = date.getMinutes().toString().padStart(2, "0"); + const seconds = date.getSeconds().toString().padStart(2, "0"); + return `${year}${month}${day}T${hour}${minute}${seconds}`; +} + +export enum LogType { + DEBUG_SESSION, +} + +export class LogFilePathProvider { + private logFolder: string = ""; + + constructor( + private context: vscode.ExtensionContext, + private logger: vscode.LogOutputChannel, + ) { + this.updateLogFolder(); + context.subscriptions.push( + vscode.workspace.onDidChangeConfiguration(e => { + if ( + e.affectsConfiguration("lldb-dap.logFolder") + ) { + this.updateLogFolder(); + } + }) + ); + } + + get(type: LogType): string { + const logFolder = this.logFolder || this.context.logUri.fsPath; + switch(type) { + case LogType.DEBUG_SESSION: + return path.join(logFolder, `lldb-dap-session-${formatDate(new Date())}.log`); + break; + } + } + + private updateLogFolder() { + const config = vscode.workspace.getConfiguration("lldb-dap"); + let logFolder = + config.get("logFolder") || this.context.logUri.fsPath; + vscode.workspace.fs + .createDirectory(vscode.Uri.file(logFolder)) + .then(undefined, (error) => { + this.logger.error(`Failed to create log folder ${logFolder}: ${error}`); + logFolder = this.context.logUri.fsPath; + }) + .then(() => { + this.logFolder = logFolder; + this.logger.info(`Persisting lldb-dap logs to ${logFolder}`); + }); + } +} diff --git a/lldb/unittests/DAP/ProtocolTypesTest.cpp b/lldb/unittests/DAP/ProtocolTypesTest.cpp index 3a1b9402b40c9..4aab2dc223134 100644 --- a/lldb/unittests/DAP/ProtocolTypesTest.cpp +++ b/lldb/unittests/DAP/ProtocolTypesTest.cpp @@ -108,8 +108,9 @@ TEST(ProtocolTypesTest, Breakpoint) { breakpoint.id = 42; breakpoint.verified = true; breakpoint.message = "Breakpoint set successfully"; - breakpoint.source = Source{"test.cpp", "/path/to/test.cpp", 123, - Source::eSourcePresentationHintNormal}; + breakpoint.source = + Source{"test.cpp", "/path/to/test.cpp", 123, + Source::eSourcePresentationHintNormal, std::nullopt}; breakpoint.line = 10; breakpoint.column = 5; breakpoint.endLine = 15; @@ -567,8 +568,9 @@ TEST(ProtocolTypesTest, DisassembledInstruction) { instruction.instructionBytes = "0F 1F 00"; instruction.instruction = "mov eax, ebx"; instruction.symbol = "main"; - instruction.location = Source{"test.cpp", "/path/to/test.cpp", 123, - Source::eSourcePresentationHintNormal}; + instruction.location = + Source{"test.cpp", "/path/to/test.cpp", 123, + Source::eSourcePresentationHintNormal, std::nullopt}; instruction.line = 10; instruction.column = 5; instruction.endLine = 15; diff --git a/lldb/unittests/Host/NativeProcessProtocolTest.cpp b/lldb/unittests/Host/NativeProcessProtocolTest.cpp index a48e67c9213da..91c4fd69d6e54 100644 --- a/lldb/unittests/Host/NativeProcessProtocolTest.cpp +++ b/lldb/unittests/Host/NativeProcessProtocolTest.cpp @@ -73,6 +73,97 @@ TEST(NativeProcessProtocolTest, SetBreakpointFailVerify) { llvm::Failed()); } +TEST(NativeProcessProtocolTest, RemoveSoftwareBreakpoint) { + NiceMock DummyDelegate; + MockProcess Process(DummyDelegate, + ArchSpec("x86_64-pc-linux")); + auto Trap = cantFail(Process.GetSoftwareBreakpointTrapOpcode(1)); + auto Original = std::vector{0xbb}; + + // Set up a breakpoint. + { + InSequence S; + EXPECT_CALL(Process, ReadMemory(0x47, 1)) + .WillOnce(Return(ByMove(Original))); + EXPECT_CALL(Process, WriteMemory(0x47, Trap)).WillOnce(Return(ByMove(1))); + EXPECT_CALL(Process, ReadMemory(0x47, 1)).WillOnce(Return(ByMove(Trap))); + EXPECT_THAT_ERROR(Process.SetBreakpoint(0x47, 0, false).ToError(), + llvm::Succeeded()); + } + + // Remove the breakpoint for the first time. This should remove the breakpoint + // from m_software_breakpoints. + // + // Should succeed. + { + InSequence S; + EXPECT_CALL(Process, ReadMemory(0x47, 1)).WillOnce(Return(ByMove(Trap))); + EXPECT_CALL(Process, WriteMemory(0x47, llvm::ArrayRef(Original))) + .WillOnce(Return(ByMove(1))); + EXPECT_CALL(Process, ReadMemory(0x47, 1)) + .WillOnce(Return(ByMove(Original))); + EXPECT_THAT_ERROR(Process.RemoveBreakpoint(0x47, false).ToError(), + llvm::Succeeded()); + } + + // Remove the breakpoint for the second time. + // + // Should fail. None of the ReadMemory() or WriteMemory() should be called, + // because the function should early return when seeing that the breakpoint + // isn't in m_software_breakpoints. + { + EXPECT_CALL(Process, ReadMemory(_, _)).Times(0); + EXPECT_CALL(Process, WriteMemory(_, _)).Times(0); + EXPECT_THAT_ERROR(Process.RemoveBreakpoint(0x47, false).ToError(), + llvm::Failed()); + } +} + +TEST(NativeProcessProtocolTest, RemoveSoftwareBreakpointMemoryError) { + NiceMock DummyDelegate; + MockProcess Process(DummyDelegate, + ArchSpec("x86_64-pc-linux")); + auto Trap = cantFail(Process.GetSoftwareBreakpointTrapOpcode(1)); + auto Original = std::vector{0xbb}; + auto SomethingElse = std::vector{0xaa}; + + // Set up a breakpoint. + { + InSequence S; + EXPECT_CALL(Process, ReadMemory(0x47, 1)) + .WillOnce(Return(ByMove(Original))); + EXPECT_CALL(Process, WriteMemory(0x47, Trap)).WillOnce(Return(ByMove(1))); + EXPECT_CALL(Process, ReadMemory(0x47, 1)).WillOnce(Return(ByMove(Trap))); + EXPECT_THAT_ERROR(Process.SetBreakpoint(0x47, 0, false).ToError(), + llvm::Succeeded()); + } + + // Remove the breakpoint for the first time, with an unexpected value read by + // the first ReadMemory(). This should cause an early return, with the + // breakpoint removed from m_software_breakpoints. + // + // Should fail. + { + InSequence S; + EXPECT_CALL(Process, ReadMemory(0x47, 1)) + .WillOnce(Return(ByMove(SomethingElse))); + EXPECT_THAT_ERROR(Process.RemoveBreakpoint(0x47, false).ToError(), + llvm::Failed()); + } + + // Remove the breakpoint for the second time. + // + // Should fail. None of the ReadMemory() or WriteMemory() should be called, + // because the function should early return when seeing that the breakpoint + // isn't in m_software_breakpoints. + { + EXPECT_CALL(Process, ReadMemory(_, _)).Times(0); + EXPECT_CALL(Process, WriteMemory(_, _)).Times(0); + EXPECT_THAT_ERROR(Process.RemoveBreakpoint(0x47, false).ToError(), + llvm::Failed()); + } +} + TEST(NativeProcessProtocolTest, ReadMemoryWithoutTrap) { NiceMock DummyDelegate; MockProcess Process(DummyDelegate, @@ -146,4 +237,4 @@ TEST(NativeProcessProtocolTest, ReadCStringFromMemory_CrossPageBoundary) { bytes_read), llvm::HasValue(llvm::StringRef("hello"))); EXPECT_EQ(bytes_read, 6UL); -} \ No newline at end of file +}