[lldb-dap] persistent assembly breakpoints #148061
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
✅ With the latest revision this PR passed the Python code formatter. |
ashgti
reviewed
Jul 10, 2025
Comment on lines
+741
to
+744
| lldb::SBBreakpoint | ||
| BreakpointCreateByFileAddress(const SBFileSpec &file_spec, addr_t file_addr, | ||
| addr_t offset = 0, | ||
| addr_t instructions_offset = 0); |
There was a problem hiding this comment.
Do we need to create a new API for this?
If we're storing the file path + offset, we should be able to create the breakpoint by checking if the module is loaded with the same path and then looking up the address for the file offset.
Something along the lines of:
path = # persisted path
offset = # persisted offset
for m in lldb.target.modules:
if m.path == path:
addr = m.ResolveFileAddress(offset)
lldb.target.BreakpointCreateByAddress(addr.GetLoadAddress(lldb.target))There was a problem hiding this comment.
According the specification the only persistent data we can add is to the Source object in the adapterData field (https://microsoft.github.io/debug-adapter-protocol//specification.html#Types_Source).
The source object is common for all breakpoints in the same function, so the only way I found to differentiate between two breakpoints in the same Source is the line field in the Breakpoint object (where we can't add custom persistent data), which is equivalent to the instructions_offset from the start of the function.
There was a problem hiding this comment.
We can add new APIs for lldb-dap, but they should make sense on their own. Right now, what kind of idea what kind of breakpoint this would set. Is the FileSpec the name of the module you're setting the breakpoint in? And does file_addr identify a file in that module?
There was a problem hiding this comment.
@labath Yes, the intention is to set a breakpoint in a virtual address that relates to the file address in a given module.
A function like this already exists in main in Target.h (CreateAddressInModuleBreakpoint) although not exported in the public API.
There was a problem hiding this comment.
The breakpoint resolvers got a "user provided" offset field because it was useful for some breakpoint types to allow the user to specify symbol + offset or file & line address + offset. That was handy - especially symbol + offset - in the case where the user has no easy way to go from the initial specifier to an address when they are making the breakpoint, and so that's not math they can do up front.
But that wasn't used for Address breakpoints (and thus NOT passed in the original CreateAddressInModuleBreakpoint) because there's no point. You are starting with an lldb::addr_t, so you can immediately do whatever offsetting math you need to do before setting the breakpoint. There's no reason not to just add the offset to the file_addr you are passing in to create the breakpoint.
You are also adding to that another form of offset - the "instruction count offset" which is the size of a certain number of instructions after the initial addr (*). That's slightly more useful than the offset for address breakpoints, in that that computation is less trivial than adding two addresses.
But first off, if this IS useful as another way to specify "offset from some resolved location" it is useful for the other cases where we actually use offsets. So doing it only for breakpoints set by address doesn't make sense.
Secondly, It should be another variant of offsets. I don't think we anywhere need to offer an API that takes both an lldb::addr_t offset AND an "instruction count" offset. That seems unnecessary, and error prone (do you offset with the addr_t THEN count instructions or vice versa).
(*) You need to check that that addr disassembles, BTW. This is user input, so there's no guarantee that address lies on an instruction boundary...
lldb/tools/lldb-dap/DAP.cpp
Outdated
Comment on lines
1411
to
1416
| llvm::json::Value protocol_bp_value = toJSON(protocol_bp); | ||
|
|
||
| // "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_value.getAsObject()->erase("source"); |
There was a problem hiding this comment.
Instead of converting this to a Value, should we just set the protocol_bp.source = std::nullopt;?
|
@llvm/pr-subscribers-lldb Author: Ely Ronnen (eronnen) Changes
Patch is 42.14 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/148061.diff 23 Files Affected:
diff --git a/lldb/include/lldb/API/SBTarget.h b/lldb/include/lldb/API/SBTarget.h
index 2776a8f9010fe..a135e3ce4db58 100644
--- a/lldb/include/lldb/API/SBTarget.h
+++ b/lldb/include/lldb/API/SBTarget.h
@@ -23,6 +23,7 @@
#include "lldb/API/SBValue.h"
#include "lldb/API/SBWatchpoint.h"
#include "lldb/API/SBWatchpointOptions.h"
+#include "lldb/lldb-types.h"
namespace lldb_private {
namespace python {
@@ -737,6 +738,11 @@ class LLDB_API SBTarget {
lldb::SBBreakpoint BreakpointCreateBySBAddress(SBAddress &address);
+ lldb::SBBreakpoint
+ BreakpointCreateByFileAddress(const SBFileSpec &file_spec, addr_t file_addr,
+ addr_t offset = 0,
+ addr_t instructions_offset = 0);
+
/// Create a breakpoint using a scripted resolver.
///
/// \param[in] class_name
diff --git a/lldb/include/lldb/Breakpoint/BreakpointResolver.h b/lldb/include/lldb/Breakpoint/BreakpointResolver.h
index 52cd70e934e6d..adb577e2600b9 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointResolver.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointResolver.h
@@ -16,6 +16,7 @@
#include "lldb/Utility/FileSpec.h"
#include "lldb/Utility/RegularExpression.h"
#include "lldb/lldb-private.h"
+#include "lldb/lldb-types.h"
#include <optional>
namespace lldb_private {
@@ -45,9 +46,9 @@ class BreakpointResolver : public Searcher {
/// The breakpoint that owns this resolver.
/// \param[in] resolverType
/// The concrete breakpoint resolver type for this breakpoint.
- BreakpointResolver(const lldb::BreakpointSP &bkpt,
- unsigned char resolverType,
- lldb::addr_t offset = 0);
+ BreakpointResolver(const lldb::BreakpointSP &bkpt, unsigned char resolverType,
+ lldb::addr_t offset = 0,
+ lldb::addr_t instructions_offset = 0);
/// The Destructor is virtual, all significant breakpoint resolvers derive
/// from this class.
@@ -182,6 +183,7 @@ class BreakpointResolver : public Searcher {
SearchDepth,
SkipPrologue,
SymbolNameArray,
+ InstructionsOffset,
LastOptionName
};
static const char
@@ -220,6 +222,8 @@ class BreakpointResolver : public Searcher {
lldb::BreakpointWP m_breakpoint; // This is the breakpoint we add locations to.
lldb::addr_t m_offset; // A random offset the user asked us to add to any
// breakpoints we set.
+ lldb::addr_t m_instructions_offset; // Number of instructions to add to the
+ // resolved breakpoint address.
// Subclass identifier (for llvm isa/dyn_cast)
const unsigned char SubclassID;
diff --git a/lldb/include/lldb/Breakpoint/BreakpointResolverAddress.h b/lldb/include/lldb/Breakpoint/BreakpointResolverAddress.h
index 3a09892f3f194..01318bbfe0d4a 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointResolverAddress.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointResolverAddress.h
@@ -28,6 +28,10 @@ class BreakpointResolverAddress : public BreakpointResolver {
const Address &addr,
const FileSpec &module_spec);
+ BreakpointResolverAddress(const lldb::BreakpointSP &bkpt, const Address &addr,
+ const FileSpec &module_spec, lldb::addr_t offset,
+ lldb::addr_t instructions_offset);
+
~BreakpointResolverAddress() override = default;
static lldb::BreakpointResolverSP
diff --git a/lldb/include/lldb/Core/Disassembler.h b/lldb/include/lldb/Core/Disassembler.h
index 21bacb14f9b25..50a5d87835844 100644
--- a/lldb/include/lldb/Core/Disassembler.h
+++ b/lldb/include/lldb/Core/Disassembler.h
@@ -291,6 +291,8 @@ class InstructionList {
size_t GetSize() const;
+ size_t GetTotalByteSize() const;
+
uint32_t GetMaxOpcocdeByteSize() const;
lldb::InstructionSP GetInstructionAtIndex(size_t idx) const;
diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index 0d4e11b65339e..5730f7b369cd6 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -18,6 +18,7 @@
#include "lldb/Breakpoint/BreakpointList.h"
#include "lldb/Breakpoint/BreakpointName.h"
#include "lldb/Breakpoint/WatchpointList.h"
+#include "lldb/Core/Address.h"
#include "lldb/Core/Architecture.h"
#include "lldb/Core/Disassembler.h"
#include "lldb/Core/ModuleList.h"
@@ -723,11 +724,11 @@ class Target : public std::enable_shared_from_this<Target>,
lldb::BreakpointSP CreateBreakpoint(lldb::addr_t load_addr, bool internal,
bool request_hardware);
- // Use this to create a breakpoint from a load address and a module file spec
- lldb::BreakpointSP CreateAddressInModuleBreakpoint(lldb::addr_t file_addr,
- bool internal,
- const FileSpec &file_spec,
- bool request_hardware);
+ // Use this to create a breakpoint from a file address and a module file spec
+ lldb::BreakpointSP CreateAddressInModuleBreakpoint(
+ lldb::addr_t file_addr, bool internal, const FileSpec &file_spec,
+ bool request_hardware, lldb::addr_t offset = 0,
+ lldb::addr_t instructions_offset = 0);
// Use this to create Address breakpoints:
lldb::BreakpointSP CreateBreakpoint(const Address &addr, bool internal,
@@ -1328,6 +1329,10 @@ class Target : public std::enable_shared_from_this<Target>,
const lldb_private::RegisterFlags &flags,
uint32_t byte_size);
+ lldb::DisassemblerSP ReadInstructions(const Address &start_addr,
+ uint32_t count,
+ const char *flavor_string = nullptr);
+
// Target Stop Hooks
class StopHook : public UserID {
public:
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index 68f58bf1349a7..6c6a674f664ad 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -107,17 +107,23 @@ def dump_dap_log(log_file):
class Source(object):
def __init__(
- self, path: Optional[str] = None, source_reference: Optional[int] = None
+ self,
+ path: Optional[str] = None,
+ source_reference: Optional[int] = None,
+ raw_dict: Optional[dict[str, Any]] = None,
):
self._name = None
self._path = None
self._source_reference = None
+ self._raw_dict = None
if path is not None:
self._name = os.path.basename(path)
self._path = path
elif source_reference is not None:
self._source_reference = source_reference
+ elif raw_dict is not None:
+ self._raw_dict = raw_dict
else:
raise ValueError("Either path or source_reference must be provided")
@@ -125,6 +131,9 @@ def __str__(self):
return f"Source(name={self.name}, path={self.path}), source_reference={self.source_reference})"
def as_dict(self):
+ if self._raw_dict is not None:
+ return self._raw_dict
+
source_dict = {}
if self._name is not None:
source_dict["name"] = self._name
@@ -135,6 +144,19 @@ def as_dict(self):
return source_dict
+class Breakpoint(object):
+ def __init__(self, obj):
+ self._breakpoint = obj
+
+ def is_verified(self):
+ """Check if the breakpoint is verified."""
+ return self._breakpoint.get("verified", False)
+
+ def source(self):
+ """Get the source of the breakpoint."""
+ return self._breakpoint.get("source", {})
+
+
class NotSupportedError(KeyError):
"""Raised if a feature is not supported due to its capabilities."""
@@ -170,7 +192,7 @@ def __init__(
self.initialized = False
self.frame_scopes = {}
self.init_commands = init_commands
- self.resolved_breakpoints = {}
+ self.resolved_breakpoints: dict[str, Breakpoint] = {}
@classmethod
def encode_content(cls, s: str) -> bytes:
@@ -326,8 +348,8 @@ def _process_continued(self, all_threads_continued: bool):
def _update_verified_breakpoints(self, breakpoints: list[Event]):
for breakpoint in breakpoints:
if "id" in breakpoint:
- self.resolved_breakpoints[str(breakpoint["id"])] = breakpoint.get(
- "verified", False
+ self.resolved_breakpoints[str(breakpoint["id"])] = Breakpoint(
+ breakpoint
)
def send_packet(self, command_dict: Request, set_sequence=True):
@@ -484,7 +506,14 @@ def wait_for_breakpoints_to_be_verified(
if breakpoint_event is None:
break
- return [id for id in breakpoint_ids if id not in self.resolved_breakpoints]
+ return [
+ id
+ for id in breakpoint_ids
+ if (
+ id not in self.resolved_breakpoints
+ or not self.resolved_breakpoints[id].is_verified()
+ )
+ ]
def wait_for_exited(self, timeout: Optional[float] = None):
event_dict = self.wait_for_event("exited", timeout=timeout)
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
index 00b01d4bdb1c5..1cb885b149dfd 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
@@ -59,24 +59,22 @@ def set_source_breakpoints(
Each object in data is 1:1 mapping with the entry in lines.
It contains optional location/hitCondition/logMessage parameters.
"""
- response = self.dap_server.request_setBreakpoints(
- Source(source_path), lines, data
+ return self.set_source_breakpoints_from_source(
+ Source(path=source_path), lines, data, wait_for_resolve
)
- if response is None or not response["success"]:
- return []
- breakpoints = response["body"]["breakpoints"]
- breakpoint_ids = []
- for breakpoint in breakpoints:
- breakpoint_ids.append("%i" % (breakpoint["id"]))
- if wait_for_resolve:
- self.wait_for_breakpoints_to_resolve(breakpoint_ids)
- return breakpoint_ids
def set_source_breakpoints_assembly(
self, source_reference, lines, data=None, wait_for_resolve=True
+ ):
+ return self.set_source_breakpoints_from_source(
+ Source(source_reference=source_reference), lines, data, wait_for_resolve
+ )
+
+ def set_source_breakpoints_from_source(
+ self, source: Source, lines, data=None, wait_for_resolve=True
):
response = self.dap_server.request_setBreakpoints(
- Source(source_reference=source_reference),
+ source,
lines,
data,
)
diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index f26f7951edc6f..84478ffe22601 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -949,6 +949,24 @@ SBBreakpoint SBTarget::BreakpointCreateBySBAddress(SBAddress &sb_address) {
return sb_bp;
}
+SBBreakpoint
+SBTarget::BreakpointCreateByFileAddress(const SBFileSpec &file_spec,
+ addr_t file_addr, addr_t offset,
+ addr_t instructions_offset) {
+ LLDB_INSTRUMENT_VA(this, file_spec, file_addr);
+
+ SBBreakpoint sb_bp;
+ if (TargetSP target_sp = GetSP()) {
+ std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
+ const bool hardware = false;
+ sb_bp = target_sp->CreateAddressInModuleBreakpoint(
+ file_addr, false, *file_spec.get(), hardware, offset,
+ instructions_offset);
+ }
+
+ return sb_bp;
+}
+
lldb::SBBreakpoint
SBTarget::BreakpointCreateBySourceRegex(const char *source_regex,
const lldb::SBFileSpec &source_file,
@@ -1955,29 +1973,8 @@ lldb::SBInstructionList SBTarget::ReadInstructions(lldb::SBAddress base_addr,
if (TargetSP target_sp = GetSP()) {
if (Address *addr_ptr = base_addr.get()) {
- DataBufferHeap data(
- target_sp->GetArchitecture().GetMaximumOpcodeByteSize() * count, 0);
- bool force_live_memory = true;
- lldb_private::Status error;
- lldb::addr_t load_addr = LLDB_INVALID_ADDRESS;
- const size_t bytes_read =
- target_sp->ReadMemory(*addr_ptr, data.GetBytes(), data.GetByteSize(),
- error, force_live_memory, &load_addr);
-
- const bool data_from_file = load_addr == LLDB_INVALID_ADDRESS;
- if (!flavor_string || flavor_string[0] == '\0') {
- // FIXME - we don't have the mechanism in place to do per-architecture
- // settings. But since we know that for now we only support flavors on
- // x86 & x86_64,
- const llvm::Triple::ArchType arch =
- target_sp->GetArchitecture().GetTriple().getArch();
- if (arch == llvm::Triple::x86 || arch == llvm::Triple::x86_64)
- flavor_string = target_sp->GetDisassemblyFlavor();
- }
- sb_instructions.SetDisassembler(Disassembler::DisassembleBytes(
- target_sp->GetArchitecture(), nullptr, flavor_string,
- target_sp->GetDisassemblyCPU(), target_sp->GetDisassemblyFeatures(),
- *addr_ptr, data.GetBytes(), bytes_read, count, data_from_file));
+ sb_instructions.SetDisassembler(
+ target_sp->ReadInstructions(*addr_ptr, count, flavor_string));
}
}
diff --git a/lldb/source/Breakpoint/BreakpointResolver.cpp b/lldb/source/Breakpoint/BreakpointResolver.cpp
index 91fdff4a455da..0bca1337e3cff 100644
--- a/lldb/source/Breakpoint/BreakpointResolver.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolver.cpp
@@ -29,6 +29,7 @@
#include "lldb/Utility/Log.h"
#include "lldb/Utility/Stream.h"
#include "lldb/Utility/StreamString.h"
+#include "lldb/lldb-forward.h"
#include <optional>
using namespace lldb_private;
@@ -42,10 +43,12 @@ const char *BreakpointResolver::g_ty_to_name[] = {"FileAndLine", "Address",
const char *BreakpointResolver::g_option_names[static_cast<uint32_t>(
BreakpointResolver::OptionNames::LastOptionName)] = {
- "AddressOffset", "Exact", "FileName", "Inlines", "Language",
- "LineNumber", "Column", "ModuleName", "NameMask", "Offset",
- "PythonClass", "Regex", "ScriptArgs", "SectionName", "SearchDepth",
- "SkipPrologue", "SymbolNames"};
+ "AddressOffset", "Exact", "FileName",
+ "Inlines", "Language", "LineNumber",
+ "Column", "ModuleName", "NameMask",
+ "Offset", "PythonClass", "Regex",
+ "ScriptArgs", "SectionName", "SearchDepth",
+ "SkipPrologue", "SymbolNames", "InstructionsOffset"};
const char *BreakpointResolver::ResolverTyToName(enum ResolverTy type) {
if (type > LastKnownResolverType)
@@ -65,8 +68,10 @@ BreakpointResolver::NameToResolverTy(llvm::StringRef name) {
BreakpointResolver::BreakpointResolver(const BreakpointSP &bkpt,
const unsigned char resolverTy,
- lldb::addr_t offset)
- : m_breakpoint(bkpt), m_offset(offset), SubclassID(resolverTy) {}
+ lldb::addr_t offset,
+ lldb::addr_t instructions_offset)
+ : m_breakpoint(bkpt), m_offset(offset),
+ m_instructions_offset(instructions_offset), SubclassID(resolverTy) {}
BreakpointResolver::~BreakpointResolver() = default;
@@ -364,6 +369,13 @@ void BreakpointResolver::AddLocation(SearchFilter &filter,
BreakpointLocationSP BreakpointResolver::AddLocation(Address loc_addr,
bool *new_location) {
+ if (m_instructions_offset != 0) {
+ Target &target = GetBreakpoint()->GetTarget();
+ const DisassemblerSP instructions =
+ target.ReadInstructions(loc_addr, m_instructions_offset);
+ loc_addr.Slide(instructions->GetInstructionList().GetTotalByteSize());
+ }
+
loc_addr.Slide(m_offset);
return GetBreakpoint()->AddLocation(loc_addr, new_location);
}
diff --git a/lldb/source/Breakpoint/BreakpointResolverAddress.cpp b/lldb/source/Breakpoint/BreakpointResolverAddress.cpp
index 828647ceef637..fae2022220ba3 100644
--- a/lldb/source/Breakpoint/BreakpointResolverAddress.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolverAddress.cpp
@@ -30,6 +30,14 @@ BreakpointResolverAddress::BreakpointResolverAddress(const BreakpointSP &bkpt,
: BreakpointResolver(bkpt, BreakpointResolver::AddressResolver),
m_addr(addr), m_resolved_addr(LLDB_INVALID_ADDRESS) {}
+BreakpointResolverAddress::BreakpointResolverAddress(
+ const BreakpointSP &bkpt, const Address &addr, const FileSpec &module_spec,
+ lldb::addr_t offset, lldb::addr_t instructions_offset)
+ : BreakpointResolver(bkpt, BreakpointResolver::AddressResolver, offset,
+ instructions_offset),
+ m_addr(addr), m_resolved_addr(LLDB_INVALID_ADDRESS),
+ m_module_filespec(module_spec) {}
+
BreakpointResolverSP BreakpointResolverAddress::CreateFromStructuredData(
const StructuredData::Dictionary &options_dict, Status &error) {
llvm::StringRef module_name;
@@ -133,6 +141,11 @@ Searcher::CallbackReturn BreakpointResolverAddress::SearchCallback(
Address tmp_address;
if (module_sp->ResolveFileAddress(m_addr.GetOffset(), tmp_address))
m_addr = tmp_address;
+ else
+ return Searcher::eCallbackReturnStop;
+ } else {
+ // If we didn't find the module, then we can't resolve the address.
+ return Searcher::eCallbackReturnStop;
}
}
diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp
index 833e327579a29..90394cc9a5569 100644
--- a/lldb/source/Core/Disassembler.cpp
+++ b/lldb/source/Core/Disassembler.cpp
@@ -1014,6 +1014,16 @@ uint32_t InstructionList::GetMaxOpcocdeByteSize() const {
return max_inst_size;
}
+size_t InstructionList::GetTotalByteSize() const {
+ size_t total_byte_size = 0;
+ collection::const_iterator pos, end;
+ for (pos = m_instructions.begin(), end = m_instructions.end(); pos != end;
+ ++pos) {
+ total_byte_size += (*pos)->GetOpcode().GetByteSize();
+ }
+ return total_byte_size;
+}
+
InstructionSP InstructionList::GetInstructionAtIndex(size_t idx) const {
InstructionSP inst_sp;
if (idx < m_instructions.size())
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 7f569173eba20..b1d6fdf8f7537 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -65,6 +65,7 @@
#include "lldb/Utility/State.h"
#include "lldb/Utility/StreamString.h"
#include "lldb/Utility/Timer.h"
+#include "lldb/lldb-forward.h"
#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/SetVector.h"
@@ -564,14 +565,14 @@ BreakpointSP Target::CreateBreakpoint(const Address &addr, bool internal,
return CreateBreakpoint(filter_sp, resolver_sp, internal, hardware, false);
}
-lldb::BreakpointSP
-Target::CreateAddressInModuleBreakpoint(lldb::addr_t file_addr, bool internal,
- const FileSpec &file_spec,
- bool request_hardware) {
+lldb::BreakpointSP Target::CreateAddressInModuleBreakpoint(
+ lldb::addr_t file_addr, bool internal, const FileSpec &file_spec,
+ bool request_hardware, lldb::addr_t offset,
+ lldb::addr_t instructions_offset) {
SearchFilterSP filter_sp(
new SearchFilterForUnconstrainedSearches(shared_from_this()));
BreakpointResolverSP resolver_sp(new BreakpointResolverAddress(
- nullptr, file_addr, file_spec));
+ nullptr, file_addr, file_spec, offset, instructions_offset));
return CreateBreakpoint(filter_sp, resolver_sp, internal, request_hardware,
false);
}
@@ -2990,6 +2991,33 @@ lldb::addr_t Target::GetBr...
[truncated]
|
|
I don't understand what this address plus address offset breakpoint type is supposed to be? We have "address plus offset" already - where the initial address is a Section Offset - that's what Address-es are. Addresses are very handy for address breakpoint setting because they allow you to track the container (the section) if it unloads and reloads either because the library containing the section was unloaded & reloaded or because you reran. Internally, lldb will try to turn a "load address" breakpoint into an Address, and it will only try to reload on re-run address breakpoints if they were resolvable to an Address. That's because you have to be careful about setting raw address breakpoints, if you put them in the wrong place they can cause subtle and hard to diagnose debugger-caused bugs. Why could you not use Address-es to set this two-form breakpoint? Given a module and an lldb::addr_t that is in the module, you can always resolve the addr_t to it's section+offset form, so you can always make an Address. What do you gain with your new breakpoint setting form? Maybe you should write the documentation for your proposed new API so we can see what it is supposed to be for? |
jimingham
requested changes
Jul 14, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #141955
Sourceobject, in order for assembly breakpoints, which rely on a temporarysourceReferencevalue, to be able to resolve in future sessions like normal path+line breakpointsinstructions_offsetparameter toBreakpointResolver