[lldb] Add SymbolContext::GetFunctionOrSymbolAddress #123340
Conversation
|
@llvm/pr-subscribers-backend-hexagon Author: Pavel Labath (labath) ChangesMany (most?) uses of SC::GetAddressRange were not interested in the range, but in the address of the function/symbol contained inside the symbol context. They were getting that by calling the GetBaseAddress on the returned range, which worked well enough so far, but isn't compatible with discontinuous functions, whose address (entry point) may not be the lowest address in the range. To resolve this problem, this PR creates a new function whose purpose is return the address of the object inside the symbol context ("address of a block" is a somewhat fuzzy concept, but I've made it so that mirrors what the GetAddressRange function does). It also changes all of the callers of GetAddressRange which do not actually care about the range to call this function instead. I've also changed all of the callers which are interested in the entry point of a function/symbol to pass eSymbolContextFunction | eSymbolContextSymbol instead of eSymbolContextEverything, as a block or a line will not have a callable address. Full diff: https://github.com/llvm/llvm-project/pull/123340.diff 9 Files Affected:
diff --git a/lldb/include/lldb/Symbol/SymbolContext.h b/lldb/include/lldb/Symbol/SymbolContext.h
index 07769cd8dffae7..fde11f275dc60a 100644
--- a/lldb/include/lldb/Symbol/SymbolContext.h
+++ b/lldb/include/lldb/Symbol/SymbolContext.h
@@ -192,6 +192,33 @@ class SymbolContext {
bool GetAddressRange(uint32_t scope, uint32_t range_idx,
bool use_inline_block_range, AddressRange &range) const;
+ /// Get the address represented by this symbol context.
+ ///
+ /// The exact meaning of the address depends on object being queried and the
+ /// flags. Priority is as follows:
+ /// - The beginning (lowest address) of the line_entry if line_entry is
+ /// valid and eSymbolContextLineEntry is set in \a scope
+ /// - The beginning of the block if block is not nullptr and
+ /// eSymbolContextBlock is set in \a scope
+ /// - function address (entry point) if function is not nullptr and
+ /// eSymbolContextFunction is set in \a scope
+ /// - symbol address if symbol is not nullptr and eSymbolContextSymbol is
+ /// set in \a scope
+ ///
+ /// \param[in] scope
+ /// A mask of symbol context bits telling this function which
+ /// address ranges it can use when trying to extract one from
+ /// the valid (non-nullptr) symbol context classes.
+ ///
+ /// \param[in] use_inline_block_range
+ /// If \a scope has the eSymbolContextBlock bit set, and there
+ /// is a valid block in the symbol context, return the block
+ /// address range for the containing inline function block, not
+ /// the deepest most block. This allows us to extract information
+ /// for the address range of the inlined function block, not
+ /// the deepest lexical block.
+ Address GetAddress(uint32_t scope, bool use_inline_block_range) const;
+
llvm::Error GetAddressRangeFromHereToEndLine(uint32_t end_line,
AddressRange &range);
diff --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp
index d8265e41a7384e..705c69167badd3 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -1621,12 +1621,8 @@ static void DumpSymbolContextList(
if (!first_module)
strm.EOL();
- AddressRange range;
-
- sc.GetAddressRange(eSymbolContextEverything, 0, true, range);
-
- DumpAddress(exe_scope, range.GetBaseAddress(), verbose, all_ranges, strm,
- settings);
+ Address addr = sc.GetAddress(eSymbolContextEverything, true);
+ DumpAddress(exe_scope, addr, verbose, all_ranges, strm, settings);
first_module = false;
}
strm.IndentLess();
@@ -3570,16 +3566,13 @@ class CommandObjectTargetModulesShowUnwind : public CommandObjectParsed {
continue;
if (!sc.module_sp || sc.module_sp->GetObjectFile() == nullptr)
continue;
- AddressRange range;
- if (!sc.GetAddressRange(eSymbolContextFunction | eSymbolContextSymbol, 0,
- false, range))
- continue;
- if (!range.GetBaseAddress().IsValid())
+ Address addr = sc.GetAddress(eSymbolContextFunction | eSymbolContextSymbol, false);
+ if (!addr.IsValid())
continue;
ConstString funcname(sc.GetFunctionName());
if (funcname.IsEmpty())
continue;
- addr_t start_addr = range.GetBaseAddress().GetLoadAddress(target);
+ addr_t start_addr = addr.GetLoadAddress(target);
if (abi)
start_addr = abi->FixCodeAddress(start_addr);
diff --git a/lldb/source/Plugins/DynamicLoader/Hexagon-DYLD/DynamicLoaderHexagonDYLD.cpp b/lldb/source/Plugins/DynamicLoader/Hexagon-DYLD/DynamicLoaderHexagonDYLD.cpp
index 96c94535c623c7..a724af973bf80a 100644
--- a/lldb/source/Plugins/DynamicLoader/Hexagon-DYLD/DynamicLoaderHexagonDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/Hexagon-DYLD/DynamicLoaderHexagonDYLD.cpp
@@ -426,9 +426,9 @@ DynamicLoaderHexagonDYLD::GetStepThroughTrampolinePlan(Thread &thread,
typedef std::vector<lldb::addr_t> AddressVector;
AddressVector addrs;
for (const SymbolContext &context : target_symbols) {
- AddressRange range;
- context.GetAddressRange(eSymbolContextEverything, 0, false, range);
- lldb::addr_t addr = range.GetBaseAddress().GetLoadAddress(&target);
+ addr_t addr =
+ context.GetAddress(eSymbolContextFunction | eSymbolContextSymbol, false)
+ .GetLoadAddress(&target);
if (addr != LLDB_INVALID_ADDRESS)
addrs.push_back(addr);
}
diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
index 8949b14c571893..7aa449ba359d44 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
@@ -939,13 +939,11 @@ DynamicLoaderDarwin::GetStepThroughTrampolinePlan(Thread &thread,
images.FindSymbolsWithNameAndType(trampoline_name, eSymbolTypeCode,
code_symbols);
for (const SymbolContext &context : code_symbols) {
- AddressRange addr_range;
- context.GetAddressRange(eSymbolContextEverything, 0, false,
- addr_range);
- addresses.push_back(addr_range.GetBaseAddress());
+ Address addr = context.GetAddress(
+ eSymbolContextFunction | eSymbolContextSymbol, false);
+ addresses.push_back(addr);
if (log) {
- addr_t load_addr =
- addr_range.GetBaseAddress().GetLoadAddress(target_sp.get());
+ addr_t load_addr = addr.GetLoadAddress(target_sp.get());
LLDB_LOGF(log, "Found a trampoline target symbol at 0x%" PRIx64 ".",
load_addr);
@@ -980,13 +978,11 @@ DynamicLoaderDarwin::GetStepThroughTrampolinePlan(Thread &thread,
indirect_symbols);
for (const SymbolContext &context : indirect_symbols) {
- AddressRange addr_range;
- context.GetAddressRange(eSymbolContextEverything, 0, false,
- addr_range);
- addresses.push_back(addr_range.GetBaseAddress());
+ Address addr = context.GetAddress(
+ eSymbolContextSymbol | eSymbolContextFunction, false);
+ addresses.push_back(addr);
if (log) {
- addr_t load_addr =
- addr_range.GetBaseAddress().GetLoadAddress(target_sp.get());
+ addr_t load_addr = addr.GetLoadAddress(target_sp.get());
LLDB_LOGF(log, "Found an indirect target symbol at 0x%" PRIx64 ".",
load_addr);
diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 5614bc32468d5c..364e50e3bc5243 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -512,9 +512,9 @@ DynamicLoaderPOSIXDYLD::GetStepThroughTrampolinePlan(Thread &thread,
typedef std::vector<lldb::addr_t> AddressVector;
AddressVector addrs;
for (const SymbolContext &context : target_symbols) {
- AddressRange range;
- context.GetAddressRange(eSymbolContextEverything, 0, false, range);
- lldb::addr_t addr = range.GetBaseAddress().GetLoadAddress(&target);
+ addr_t addr =
+ context.GetAddress(eSymbolContextSymbol | eSymbolContextFunction, false)
+ .GetLoadAddress(&target);
if (addr != LLDB_INVALID_ADDRESS)
addrs.push_back(addr);
}
diff --git a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
index fb706544ea560d..613733de0b2e6d 100644
--- a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
@@ -142,10 +142,8 @@ line_entry_helper(Target &target, const SymbolContext &sc, Symbol *symbol,
CPPLanguageRuntime::LibCppStdFunctionCallableInfo optional_info;
- AddressRange range;
- sc.GetAddressRange(eSymbolContextEverything, 0, false, range);
-
- Address address = range.GetBaseAddress();
+ Address address =
+ sc.GetAddress(eSymbolContextFunction | eSymbolContextSymbol, false);
Address addr;
if (target.ResolveLoadAddress(address.GetCallableLoadAddress(&target),
diff --git a/lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp b/lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp
index 99426c7d1f261d..ae1d371c080376 100644
--- a/lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp
+++ b/lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp
@@ -52,9 +52,6 @@ bool lldb_private::InferiorCallMmap(Process *process, addr_t &allocated_addr,
if (count > 0) {
SymbolContext sc;
if (sc_list.GetContextAtIndex(0, sc)) {
- const uint32_t range_scope =
- eSymbolContextFunction | eSymbolContextSymbol;
- const bool use_inline_block_range = false;
EvaluateExpressionOptions options;
options.SetStopOthers(true);
options.SetUnwindOnError(true);
@@ -77,9 +74,11 @@ bool lldb_private::InferiorCallMmap(Process *process, addr_t &allocated_addr,
prot_arg |= PROT_WRITE;
}
- AddressRange mmap_range;
- if (sc.GetAddressRange(range_scope, 0, use_inline_block_range,
- mmap_range)) {
+ const uint32_t range_scope =
+ eSymbolContextFunction | eSymbolContextSymbol;
+ const bool use_inline_block_range = false;
+ Address mmap_addr = sc.GetAddress(range_scope, use_inline_block_range);
+ if (mmap_addr.IsValid()) {
auto type_system_or_err =
process->GetTarget().GetScratchTypeSystemForLanguage(
eLanguageTypeC);
@@ -96,9 +95,8 @@ bool lldb_private::InferiorCallMmap(Process *process, addr_t &allocated_addr,
MmapArgList args =
process->GetTarget().GetPlatform()->GetMmapArgumentList(
arch, addr, length, prot_arg, flags, fd, offset);
- lldb::ThreadPlanSP call_plan_sp(
- new ThreadPlanCallFunction(*thread, mmap_range.GetBaseAddress(),
- void_ptr_type, args, options));
+ lldb::ThreadPlanSP call_plan_sp(new ThreadPlanCallFunction(
+ *thread, mmap_addr, void_ptr_type, args, options));
if (call_plan_sp) {
DiagnosticManager diagnostics;
@@ -149,9 +147,6 @@ bool lldb_private::InferiorCallMunmap(Process *process, addr_t addr,
if (count > 0) {
SymbolContext sc;
if (sc_list.GetContextAtIndex(0, sc)) {
- const uint32_t range_scope =
- eSymbolContextFunction | eSymbolContextSymbol;
- const bool use_inline_block_range = false;
EvaluateExpressionOptions options;
options.SetStopOthers(true);
options.SetUnwindOnError(true);
@@ -161,13 +156,14 @@ bool lldb_private::InferiorCallMunmap(Process *process, addr_t addr,
options.SetTimeout(process->GetUtilityExpressionTimeout());
options.SetTrapExceptions(false);
- AddressRange munmap_range;
- if (sc.GetAddressRange(range_scope, 0, use_inline_block_range,
- munmap_range)) {
+ const uint32_t range_scope =
+ eSymbolContextFunction | eSymbolContextSymbol;
+ const bool use_inline_block_range = false;
+ Address munmap_addr = sc.GetAddress(range_scope, use_inline_block_range);
+ if (munmap_addr.IsValid()) {
lldb::addr_t args[] = {addr, length};
- lldb::ThreadPlanSP call_plan_sp(
- new ThreadPlanCallFunction(*thread, munmap_range.GetBaseAddress(),
- CompilerType(), args, options));
+ lldb::ThreadPlanSP call_plan_sp(new ThreadPlanCallFunction(
+ *thread, munmap_addr, CompilerType(), args, options));
if (call_plan_sp) {
DiagnosticManager diagnostics;
diff --git a/lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp b/lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
index 0556b877778847..0893e8beaf4fe0 100644
--- a/lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
+++ b/lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
@@ -632,10 +632,8 @@ bool SystemRuntimeMacOSX::BacktraceRecordingHeadersInitialized() {
if (!sc_list.IsEmpty()) {
SymbolContext sc;
sc_list.GetContextAtIndex(0, sc);
- AddressRange addr_range;
- sc.GetAddressRange(eSymbolContextSymbol, 0, false, addr_range);
- queue_info_version_address =
- addr_range.GetBaseAddress().GetLoadAddress(&target);
+ Address addr = sc.GetAddress(eSymbolContextSymbol, false);
+ queue_info_version_address = addr.GetLoadAddress(&target);
}
sc_list.Clear();
@@ -646,10 +644,8 @@ bool SystemRuntimeMacOSX::BacktraceRecordingHeadersInitialized() {
if (!sc_list.IsEmpty()) {
SymbolContext sc;
sc_list.GetContextAtIndex(0, sc);
- AddressRange addr_range;
- sc.GetAddressRange(eSymbolContextSymbol, 0, false, addr_range);
- queue_info_data_offset_address =
- addr_range.GetBaseAddress().GetLoadAddress(&target);
+ Address addr = sc.GetAddress(eSymbolContextSymbol, false);
+ queue_info_data_offset_address = addr.GetLoadAddress(&target);
}
sc_list.Clear();
@@ -660,10 +656,8 @@ bool SystemRuntimeMacOSX::BacktraceRecordingHeadersInitialized() {
if (!sc_list.IsEmpty()) {
SymbolContext sc;
sc_list.GetContextAtIndex(0, sc);
- AddressRange addr_range;
- sc.GetAddressRange(eSymbolContextSymbol, 0, false, addr_range);
- item_info_version_address =
- addr_range.GetBaseAddress().GetLoadAddress(&target);
+ Address addr = sc.GetAddress(eSymbolContextSymbol, false);
+ item_info_version_address = addr.GetLoadAddress(&target);
}
sc_list.Clear();
@@ -674,10 +668,8 @@ bool SystemRuntimeMacOSX::BacktraceRecordingHeadersInitialized() {
if (!sc_list.IsEmpty()) {
SymbolContext sc;
sc_list.GetContextAtIndex(0, sc);
- AddressRange addr_range;
- sc.GetAddressRange(eSymbolContextSymbol, 0, false, addr_range);
- item_info_data_offset_address =
- addr_range.GetBaseAddress().GetLoadAddress(&target);
+ Address addr = sc.GetAddress(eSymbolContextSymbol, false);
+ item_info_data_offset_address = addr.GetLoadAddress(&target);
}
if (queue_info_version_address != LLDB_INVALID_ADDRESS &&
diff --git a/lldb/source/Symbol/SymbolContext.cpp b/lldb/source/Symbol/SymbolContext.cpp
index f4270ee8396760..0732dc8da8deed 100644
--- a/lldb/source/Symbol/SymbolContext.cpp
+++ b/lldb/source/Symbol/SymbolContext.cpp
@@ -370,6 +370,31 @@ bool SymbolContext::GetAddressRange(uint32_t scope, uint32_t range_idx,
return false;
}
+Address SymbolContext::GetAddress(uint32_t scope,
+ bool use_inline_block_range) const {
+ if ((scope & eSymbolContextLineEntry) && line_entry.IsValid())
+ return line_entry.range.GetBaseAddress();
+
+ if (scope & eSymbolContextBlock) {
+ Block *block_to_use = (block && use_inline_block_range)
+ ? block->GetContainingInlinedBlock()
+ : block;
+ if (block_to_use) {
+ Address addr;
+ block_to_use->GetStartAddress(addr);
+ return addr;
+ }
+ }
+
+ if ((scope & eSymbolContextFunction) && (function != nullptr))
+ return function->GetAddress();
+
+ if ((scope & eSymbolContextSymbol) && (symbol != nullptr))
+ return symbol->GetAddress();
+
+ return Address();
+}
+
LanguageType SymbolContext::GetLanguage() const {
LanguageType lang;
if (function && (lang = function->GetLanguage()) != eLanguageTypeUnknown) {
|
|
@llvm/pr-subscribers-lldb Author: Pavel Labath (labath) ChangesMany (most?) uses of SC::GetAddressRange were not interested in the range, but in the address of the function/symbol contained inside the symbol context. They were getting that by calling the GetBaseAddress on the returned range, which worked well enough so far, but isn't compatible with discontinuous functions, whose address (entry point) may not be the lowest address in the range. To resolve this problem, this PR creates a new function whose purpose is return the address of the object inside the symbol context ("address of a block" is a somewhat fuzzy concept, but I've made it so that mirrors what the GetAddressRange function does). It also changes all of the callers of GetAddressRange which do not actually care about the range to call this function instead. I've also changed all of the callers which are interested in the entry point of a function/symbol to pass eSymbolContextFunction | eSymbolContextSymbol instead of eSymbolContextEverything, as a block or a line will not have a callable address. Full diff: https://github.com/llvm/llvm-project/pull/123340.diff 9 Files Affected:
diff --git a/lldb/include/lldb/Symbol/SymbolContext.h b/lldb/include/lldb/Symbol/SymbolContext.h
index 07769cd8dffae7..fde11f275dc60a 100644
--- a/lldb/include/lldb/Symbol/SymbolContext.h
+++ b/lldb/include/lldb/Symbol/SymbolContext.h
@@ -192,6 +192,33 @@ class SymbolContext {
bool GetAddressRange(uint32_t scope, uint32_t range_idx,
bool use_inline_block_range, AddressRange &range) const;
+ /// Get the address represented by this symbol context.
+ ///
+ /// The exact meaning of the address depends on object being queried and the
+ /// flags. Priority is as follows:
+ /// - The beginning (lowest address) of the line_entry if line_entry is
+ /// valid and eSymbolContextLineEntry is set in \a scope
+ /// - The beginning of the block if block is not nullptr and
+ /// eSymbolContextBlock is set in \a scope
+ /// - function address (entry point) if function is not nullptr and
+ /// eSymbolContextFunction is set in \a scope
+ /// - symbol address if symbol is not nullptr and eSymbolContextSymbol is
+ /// set in \a scope
+ ///
+ /// \param[in] scope
+ /// A mask of symbol context bits telling this function which
+ /// address ranges it can use when trying to extract one from
+ /// the valid (non-nullptr) symbol context classes.
+ ///
+ /// \param[in] use_inline_block_range
+ /// If \a scope has the eSymbolContextBlock bit set, and there
+ /// is a valid block in the symbol context, return the block
+ /// address range for the containing inline function block, not
+ /// the deepest most block. This allows us to extract information
+ /// for the address range of the inlined function block, not
+ /// the deepest lexical block.
+ Address GetAddress(uint32_t scope, bool use_inline_block_range) const;
+
llvm::Error GetAddressRangeFromHereToEndLine(uint32_t end_line,
AddressRange &range);
diff --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp
index d8265e41a7384e..705c69167badd3 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -1621,12 +1621,8 @@ static void DumpSymbolContextList(
if (!first_module)
strm.EOL();
- AddressRange range;
-
- sc.GetAddressRange(eSymbolContextEverything, 0, true, range);
-
- DumpAddress(exe_scope, range.GetBaseAddress(), verbose, all_ranges, strm,
- settings);
+ Address addr = sc.GetAddress(eSymbolContextEverything, true);
+ DumpAddress(exe_scope, addr, verbose, all_ranges, strm, settings);
first_module = false;
}
strm.IndentLess();
@@ -3570,16 +3566,13 @@ class CommandObjectTargetModulesShowUnwind : public CommandObjectParsed {
continue;
if (!sc.module_sp || sc.module_sp->GetObjectFile() == nullptr)
continue;
- AddressRange range;
- if (!sc.GetAddressRange(eSymbolContextFunction | eSymbolContextSymbol, 0,
- false, range))
- continue;
- if (!range.GetBaseAddress().IsValid())
+ Address addr = sc.GetAddress(eSymbolContextFunction | eSymbolContextSymbol, false);
+ if (!addr.IsValid())
continue;
ConstString funcname(sc.GetFunctionName());
if (funcname.IsEmpty())
continue;
- addr_t start_addr = range.GetBaseAddress().GetLoadAddress(target);
+ addr_t start_addr = addr.GetLoadAddress(target);
if (abi)
start_addr = abi->FixCodeAddress(start_addr);
diff --git a/lldb/source/Plugins/DynamicLoader/Hexagon-DYLD/DynamicLoaderHexagonDYLD.cpp b/lldb/source/Plugins/DynamicLoader/Hexagon-DYLD/DynamicLoaderHexagonDYLD.cpp
index 96c94535c623c7..a724af973bf80a 100644
--- a/lldb/source/Plugins/DynamicLoader/Hexagon-DYLD/DynamicLoaderHexagonDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/Hexagon-DYLD/DynamicLoaderHexagonDYLD.cpp
@@ -426,9 +426,9 @@ DynamicLoaderHexagonDYLD::GetStepThroughTrampolinePlan(Thread &thread,
typedef std::vector<lldb::addr_t> AddressVector;
AddressVector addrs;
for (const SymbolContext &context : target_symbols) {
- AddressRange range;
- context.GetAddressRange(eSymbolContextEverything, 0, false, range);
- lldb::addr_t addr = range.GetBaseAddress().GetLoadAddress(&target);
+ addr_t addr =
+ context.GetAddress(eSymbolContextFunction | eSymbolContextSymbol, false)
+ .GetLoadAddress(&target);
if (addr != LLDB_INVALID_ADDRESS)
addrs.push_back(addr);
}
diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
index 8949b14c571893..7aa449ba359d44 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
@@ -939,13 +939,11 @@ DynamicLoaderDarwin::GetStepThroughTrampolinePlan(Thread &thread,
images.FindSymbolsWithNameAndType(trampoline_name, eSymbolTypeCode,
code_symbols);
for (const SymbolContext &context : code_symbols) {
- AddressRange addr_range;
- context.GetAddressRange(eSymbolContextEverything, 0, false,
- addr_range);
- addresses.push_back(addr_range.GetBaseAddress());
+ Address addr = context.GetAddress(
+ eSymbolContextFunction | eSymbolContextSymbol, false);
+ addresses.push_back(addr);
if (log) {
- addr_t load_addr =
- addr_range.GetBaseAddress().GetLoadAddress(target_sp.get());
+ addr_t load_addr = addr.GetLoadAddress(target_sp.get());
LLDB_LOGF(log, "Found a trampoline target symbol at 0x%" PRIx64 ".",
load_addr);
@@ -980,13 +978,11 @@ DynamicLoaderDarwin::GetStepThroughTrampolinePlan(Thread &thread,
indirect_symbols);
for (const SymbolContext &context : indirect_symbols) {
- AddressRange addr_range;
- context.GetAddressRange(eSymbolContextEverything, 0, false,
- addr_range);
- addresses.push_back(addr_range.GetBaseAddress());
+ Address addr = context.GetAddress(
+ eSymbolContextSymbol | eSymbolContextFunction, false);
+ addresses.push_back(addr);
if (log) {
- addr_t load_addr =
- addr_range.GetBaseAddress().GetLoadAddress(target_sp.get());
+ addr_t load_addr = addr.GetLoadAddress(target_sp.get());
LLDB_LOGF(log, "Found an indirect target symbol at 0x%" PRIx64 ".",
load_addr);
diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 5614bc32468d5c..364e50e3bc5243 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -512,9 +512,9 @@ DynamicLoaderPOSIXDYLD::GetStepThroughTrampolinePlan(Thread &thread,
typedef std::vector<lldb::addr_t> AddressVector;
AddressVector addrs;
for (const SymbolContext &context : target_symbols) {
- AddressRange range;
- context.GetAddressRange(eSymbolContextEverything, 0, false, range);
- lldb::addr_t addr = range.GetBaseAddress().GetLoadAddress(&target);
+ addr_t addr =
+ context.GetAddress(eSymbolContextSymbol | eSymbolContextFunction, false)
+ .GetLoadAddress(&target);
if (addr != LLDB_INVALID_ADDRESS)
addrs.push_back(addr);
}
diff --git a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
index fb706544ea560d..613733de0b2e6d 100644
--- a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
@@ -142,10 +142,8 @@ line_entry_helper(Target &target, const SymbolContext &sc, Symbol *symbol,
CPPLanguageRuntime::LibCppStdFunctionCallableInfo optional_info;
- AddressRange range;
- sc.GetAddressRange(eSymbolContextEverything, 0, false, range);
-
- Address address = range.GetBaseAddress();
+ Address address =
+ sc.GetAddress(eSymbolContextFunction | eSymbolContextSymbol, false);
Address addr;
if (target.ResolveLoadAddress(address.GetCallableLoadAddress(&target),
diff --git a/lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp b/lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp
index 99426c7d1f261d..ae1d371c080376 100644
--- a/lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp
+++ b/lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp
@@ -52,9 +52,6 @@ bool lldb_private::InferiorCallMmap(Process *process, addr_t &allocated_addr,
if (count > 0) {
SymbolContext sc;
if (sc_list.GetContextAtIndex(0, sc)) {
- const uint32_t range_scope =
- eSymbolContextFunction | eSymbolContextSymbol;
- const bool use_inline_block_range = false;
EvaluateExpressionOptions options;
options.SetStopOthers(true);
options.SetUnwindOnError(true);
@@ -77,9 +74,11 @@ bool lldb_private::InferiorCallMmap(Process *process, addr_t &allocated_addr,
prot_arg |= PROT_WRITE;
}
- AddressRange mmap_range;
- if (sc.GetAddressRange(range_scope, 0, use_inline_block_range,
- mmap_range)) {
+ const uint32_t range_scope =
+ eSymbolContextFunction | eSymbolContextSymbol;
+ const bool use_inline_block_range = false;
+ Address mmap_addr = sc.GetAddress(range_scope, use_inline_block_range);
+ if (mmap_addr.IsValid()) {
auto type_system_or_err =
process->GetTarget().GetScratchTypeSystemForLanguage(
eLanguageTypeC);
@@ -96,9 +95,8 @@ bool lldb_private::InferiorCallMmap(Process *process, addr_t &allocated_addr,
MmapArgList args =
process->GetTarget().GetPlatform()->GetMmapArgumentList(
arch, addr, length, prot_arg, flags, fd, offset);
- lldb::ThreadPlanSP call_plan_sp(
- new ThreadPlanCallFunction(*thread, mmap_range.GetBaseAddress(),
- void_ptr_type, args, options));
+ lldb::ThreadPlanSP call_plan_sp(new ThreadPlanCallFunction(
+ *thread, mmap_addr, void_ptr_type, args, options));
if (call_plan_sp) {
DiagnosticManager diagnostics;
@@ -149,9 +147,6 @@ bool lldb_private::InferiorCallMunmap(Process *process, addr_t addr,
if (count > 0) {
SymbolContext sc;
if (sc_list.GetContextAtIndex(0, sc)) {
- const uint32_t range_scope =
- eSymbolContextFunction | eSymbolContextSymbol;
- const bool use_inline_block_range = false;
EvaluateExpressionOptions options;
options.SetStopOthers(true);
options.SetUnwindOnError(true);
@@ -161,13 +156,14 @@ bool lldb_private::InferiorCallMunmap(Process *process, addr_t addr,
options.SetTimeout(process->GetUtilityExpressionTimeout());
options.SetTrapExceptions(false);
- AddressRange munmap_range;
- if (sc.GetAddressRange(range_scope, 0, use_inline_block_range,
- munmap_range)) {
+ const uint32_t range_scope =
+ eSymbolContextFunction | eSymbolContextSymbol;
+ const bool use_inline_block_range = false;
+ Address munmap_addr = sc.GetAddress(range_scope, use_inline_block_range);
+ if (munmap_addr.IsValid()) {
lldb::addr_t args[] = {addr, length};
- lldb::ThreadPlanSP call_plan_sp(
- new ThreadPlanCallFunction(*thread, munmap_range.GetBaseAddress(),
- CompilerType(), args, options));
+ lldb::ThreadPlanSP call_plan_sp(new ThreadPlanCallFunction(
+ *thread, munmap_addr, CompilerType(), args, options));
if (call_plan_sp) {
DiagnosticManager diagnostics;
diff --git a/lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp b/lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
index 0556b877778847..0893e8beaf4fe0 100644
--- a/lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
+++ b/lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
@@ -632,10 +632,8 @@ bool SystemRuntimeMacOSX::BacktraceRecordingHeadersInitialized() {
if (!sc_list.IsEmpty()) {
SymbolContext sc;
sc_list.GetContextAtIndex(0, sc);
- AddressRange addr_range;
- sc.GetAddressRange(eSymbolContextSymbol, 0, false, addr_range);
- queue_info_version_address =
- addr_range.GetBaseAddress().GetLoadAddress(&target);
+ Address addr = sc.GetAddress(eSymbolContextSymbol, false);
+ queue_info_version_address = addr.GetLoadAddress(&target);
}
sc_list.Clear();
@@ -646,10 +644,8 @@ bool SystemRuntimeMacOSX::BacktraceRecordingHeadersInitialized() {
if (!sc_list.IsEmpty()) {
SymbolContext sc;
sc_list.GetContextAtIndex(0, sc);
- AddressRange addr_range;
- sc.GetAddressRange(eSymbolContextSymbol, 0, false, addr_range);
- queue_info_data_offset_address =
- addr_range.GetBaseAddress().GetLoadAddress(&target);
+ Address addr = sc.GetAddress(eSymbolContextSymbol, false);
+ queue_info_data_offset_address = addr.GetLoadAddress(&target);
}
sc_list.Clear();
@@ -660,10 +656,8 @@ bool SystemRuntimeMacOSX::BacktraceRecordingHeadersInitialized() {
if (!sc_list.IsEmpty()) {
SymbolContext sc;
sc_list.GetContextAtIndex(0, sc);
- AddressRange addr_range;
- sc.GetAddressRange(eSymbolContextSymbol, 0, false, addr_range);
- item_info_version_address =
- addr_range.GetBaseAddress().GetLoadAddress(&target);
+ Address addr = sc.GetAddress(eSymbolContextSymbol, false);
+ item_info_version_address = addr.GetLoadAddress(&target);
}
sc_list.Clear();
@@ -674,10 +668,8 @@ bool SystemRuntimeMacOSX::BacktraceRecordingHeadersInitialized() {
if (!sc_list.IsEmpty()) {
SymbolContext sc;
sc_list.GetContextAtIndex(0, sc);
- AddressRange addr_range;
- sc.GetAddressRange(eSymbolContextSymbol, 0, false, addr_range);
- item_info_data_offset_address =
- addr_range.GetBaseAddress().GetLoadAddress(&target);
+ Address addr = sc.GetAddress(eSymbolContextSymbol, false);
+ item_info_data_offset_address = addr.GetLoadAddress(&target);
}
if (queue_info_version_address != LLDB_INVALID_ADDRESS &&
diff --git a/lldb/source/Symbol/SymbolContext.cpp b/lldb/source/Symbol/SymbolContext.cpp
index f4270ee8396760..0732dc8da8deed 100644
--- a/lldb/source/Symbol/SymbolContext.cpp
+++ b/lldb/source/Symbol/SymbolContext.cpp
@@ -370,6 +370,31 @@ bool SymbolContext::GetAddressRange(uint32_t scope, uint32_t range_idx,
return false;
}
+Address SymbolContext::GetAddress(uint32_t scope,
+ bool use_inline_block_range) const {
+ if ((scope & eSymbolContextLineEntry) && line_entry.IsValid())
+ return line_entry.range.GetBaseAddress();
+
+ if (scope & eSymbolContextBlock) {
+ Block *block_to_use = (block && use_inline_block_range)
+ ? block->GetContainingInlinedBlock()
+ : block;
+ if (block_to_use) {
+ Address addr;
+ block_to_use->GetStartAddress(addr);
+ return addr;
+ }
+ }
+
+ if ((scope & eSymbolContextFunction) && (function != nullptr))
+ return function->GetAddress();
+
+ if ((scope & eSymbolContextSymbol) && (symbol != nullptr))
+ return symbol->GetAddress();
+
+ return Address();
+}
+
LanguageType SymbolContext::GetLanguage() const {
LanguageType lang;
if (function && (lang = function->GetLanguage()) != eLanguageTypeUnknown) {
|
lldb/source/Symbol/SymbolContext.cpp
Outdated
| if ((scope & eSymbolContextLineEntry) && line_entry.IsValid()) | ||
| return line_entry.range.GetBaseAddress(); | ||
|
|
||
| if (scope & eSymbolContextBlock) { | ||
| Block *block_to_use = (block && use_inline_block_range) | ||
| ? block->GetContainingInlinedBlock() | ||
| : block; | ||
| if (block_to_use) { | ||
| Address addr; | ||
| block_to_use->GetStartAddress(addr); | ||
| return addr; | ||
| } | ||
| } |
There was a problem hiding this comment.
I have very mixed opinions about this part. The symmetry with GetAddressRange is very nice, but OTOH I'm having trouble imagining a use of these flags which wouldn't be a bug.
There was a problem hiding this comment.
Can you say a little bit more why this concerns you? I can't see either of these queries as being terribly useful, but you are answering the question that was asked correctly.
There was a problem hiding this comment.
The part that bothers me is that the "address of a block" isn't a very well defined concept. For functions/symbols, I think it's clear -- is the address you jump to when you want to "call" the functions. Line entries are contiguous so it sort of makes sense to say that its address is the beginning of that range, though I don't think that guarantees that address will ever be executed (*). For blocks, it's even messier, because even "regular" compiler optimizations (no propellers or bolts) can make the block discontiguous. Block::GetStartAddress returns the lowest address in the block, so the result maybe be not just be not executed, it may be in a completely different part of the function. It's not wrong, if that's what you expect. I think it's just.. confusing. But not supporting the flag would be confusing as well, so... 🤷
(*) I don't know if you can get a compiler to emit something like this, but I don't think it would be wrong to describe insn1 and insn2 in the following snippet with the same line entry:
jcc 2f
insn1
2: insn2
(insn1 is executed only if some condition is met and the two happen to be on the same line. Let's assume we're not using column numbers here)
There was a problem hiding this comment.
Giving a single address to a block considered as a logical entity is problematic, but the address of the start of the block referenced by a SymbolContext doesn't have that problem. I think the confusing part is the fact that we call what is in the symbol context "Block" rather than "BlockElement" or something. We don't make a distinction between logical blocks and the address ranges that constitute them the way a Function is the logical entity that organizes the block(s) that constitute it.
But for instance, when the "step through range targeting function" wants to limit the stepping range to the extent of the block we're currently sitting in, getting the SymbolContext from the current address and using that block element is what you need to do. Note, in that case we use the end address rather than the start but the idea is still the same.
There was a problem hiding this comment.
Giving a single address to a block considered as a logical entity is problematic, but the address of the start of the block referenced by a SymbolContext doesn't have that problem.
I guess, if you define the "start of a block" as "the lowest address in the block". This definition is sort of what I have a problem with because we have also defined the "start of a function" as the "lowest address in the function", but then also used it as "the entry point of the function", and this was fine until it wasn't. I don't think we're going to have the same problem with blocks that we did with functions, but I don't think this is completely right either. I also don't really have an alternative proposal, and we already do have Block::GetStartAddress, which I have no intention of touching, so if you say this is okay, I'm happy to go along with it.
There was a problem hiding this comment.
I needed to think about this a little more.
For functions, there's an actually useful notion of "Start" - functions get entered from one address, and the question "where would I put a breakpoint to capture the first instruction of this function's execution" is one we want the answer to. Even if you have alternate entry points, those usually are named separately, and I think you'd be surprised if you asked to stop on the function name and got one of the alternate entry points instead. So calling it GetStartAddress was reasonable, and when asking for the "address" of a SymbolContext that's just got a Function, the entry point address is pretty clearly what you want to know.
But there's no similar useful notion for blocks. After all, C++ goto's don't make new scopes, so you can't say "this is the first bit of code that will get executed when this block is executed." We probably should have called Block::GetStartAddress: Block::GetLowestAddress.
I confused myself because I'm more often using LineEntry's and not Blocks. Those entities seem similar but they are not. Blocks actually represent logical entities and contain all the ranges for that logical entity. But LineEntry's do not represent the logical entity "the contributions to the code from this source line". They are "one out of many possible contributions from this source line." So when you ask a LineEntry for it's address, you're just getting "the lowest address of the particular range that the address you made the SymbolContext from happened to lie within".
However, some kind of consistency would be useful. For instance, if I get the outermost Function Block from a Function, make a SymbolContext from that Block, and then ask for it's Address, it really should return the same answer as if I had made the SymbolContext from the Function directly. If I'm reading the code correctly, that won't necessarily be true, will it? For the Function, you promise to return the entry point, but for the Function's outermost Block, we return the start of the first address range.
That does seem potentially confusing to me.
There was a problem hiding this comment.
Okay, everything you've said now makes sense to me, including the part about the confusion between the function block and the function itself (it is as you described, for the block we would return the lowest address).
I think we could fix that by changing Block::GetStartAddress for function blocks. Basically to change this:
if (function) {
addr = function->GetAddress();
addr.Slide(m_ranges.GetEntryRef(0).GetRangeBase());
return true;
}
into something like this:
if (function) {
addr = function->GetAddress();
if (bool is_function_block = GetParent() == nullptr; !is_function_block)
addr.Slide(m_ranges.GetEntryRef(0).GetRangeBase());
return true;
}
Would that be acceptable?
The other option I see is to ditch the SymbolContextItem flags and rename the function into something that makes it clear that it only considers functions and symbols. The implementation of that should be easy, we only need to figure out the name. I don't want it to be too obscure, as I think this is the thing that a lot of callers will actually want to use, but it should also be clear about what its doing. GetFunctionOrSymbolAddress is the best I can come up with right now.
There was a problem hiding this comment.
I'm kind of leaning towards GetFunctionOrSymbolAddress, only doing what clearly makes sense seems best. I also can't think of a better name.
There was a problem hiding this comment.
Okay, please take a look at the new version. I had to inline the old implementation into one place (CommandObjectTarget, for image lookup), but generally, I think this looks better than what I expected.
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
| /// set in \a scope | ||
| /// | ||
| /// \param[in] scope | ||
| /// A mask of symbol context bits telling this function which |
There was a problem hiding this comment.
Might explicitly name the enum these bits come from to help people new to the codebase?
There was a problem hiding this comment.
I'm not sure what you mean. The individual bits and their meaning is listed in the paragraph above this. I don't think it makes sense to repeat them here. I could move the whole paragraph here, but I'm just following the pattern of GetAddressRange
There was a problem hiding this comment.
If you know lldb a bit at the public layer, you would know that eSymbolContextWhatever is an enum, and if you wanted to find the whole enum and where it is documented you go to include/lldb/include/, etc. But if you said "these flag is composed of bits from the enum SymbolContextItem", that would lead directly there.
There was a problem hiding this comment.
Okay, I can do that.
Many uses of SC::GetAddressRange were not interested in the range, but in the address of the function/symbol contained inside the symbol context. They were getting that by calling the GetBaseAddress on the returned range, which worked well enough so far, but isn't compatible with discontinuous functions, whose address (entry point) may not be the lowest address in the range. To resolve this problem, this PR creates a new function whose purpose is return the address of the function or symbol inside the symbol context. It also changes all of the callers of GetAddressRange which do not actually care about the range to call this function instead.
labath
changed the title
|
@jimingham, is the new version of the patch (new function name) what you had in mind? |
Many uses of SC::GetAddressRange were not interested in the range, but in the address of the function/symbol contained inside the symbol context. They were getting that by calling the GetBaseAddress on the returned range, which worked well enough so far, but isn't compatible with discontinuous functions, whose address (entry point) may not be the lowest address in the range. To resolve this problem, this PR creates a new function whose purpose is return the address of the function or symbol inside the symbol context. It also changes all of the callers of GetAddressRange which do not actually care about the range to call this function instead.
Many uses of SC::GetAddressRange were not interested in the range, but in the address of the function/symbol contained inside the symbol context. They were getting that by calling the GetBaseAddress on the returned range, which worked well enough so far, but isn't compatible with discontinuous functions, whose address (entry point) may not be the lowest address in the range.
To resolve this problem, this PR creates a new function whose purpose is return the address of the function or symbol inside the symbol context. It also changes all of the callers of GetAddressRange which do not actually care about the range to call this function instead.