[lldb] Add Function::GetAddress and redirect some uses #115836
Merged
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
|
@llvm/pr-subscribers-lldb Author: Pavel Labath (labath) ChangesMany calls to Function::GetAddressRange() were not interested in the range itself. Instead they wanted to find the address of the function (its entry point) or the base address for relocation of function-scoped entities (technically, the two don't need to be the same, but there's isn't good reason for them not to be). This PR creates a separate function for retrieving this, and changes the existing (non-controversial) uses to call that instead. Patch is 23.51 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/115836.diff 22 Files Affected:
diff --git a/lldb/include/lldb/Symbol/Function.h b/lldb/include/lldb/Symbol/Function.h
index bbfc25fe74ea06..dd4226f300c377 100644
--- a/lldb/include/lldb/Symbol/Function.h
+++ b/lldb/include/lldb/Symbol/Function.h
@@ -446,6 +446,11 @@ class Function : public UserID, public SymbolContextScope {
const AddressRange &GetAddressRange() { return m_range; }
+ /// Return the address of the function (its entry point). This address is also
+ /// used as a base address for relocation of function-scope entities (blocks
+ /// and variables).
+ Address GetAddress() const;
+
lldb::LanguageType GetLanguage() const;
/// Find the file and line number of the source location of the start of the
/// function. This will use the declaration if present and fall back on the
diff --git a/lldb/source/API/SBBlock.cpp b/lldb/source/API/SBBlock.cpp
index b921ccd9802454..2ef4cc7227cf95 100644
--- a/lldb/source/API/SBBlock.cpp
+++ b/lldb/source/API/SBBlock.cpp
@@ -176,8 +176,7 @@ bool SBBlock::GetDescription(SBStream &description) {
m_opaque_ptr->CalculateSymbolContext(&sc);
if (sc.function) {
m_opaque_ptr->DumpAddressRanges(
- &strm,
- sc.function->GetAddressRange().GetBaseAddress().GetFileAddress());
+ &strm, sc.function->GetAddress().GetFileAddress());
}
} else
strm.PutCString("No value");
diff --git a/lldb/source/API/SBFunction.cpp b/lldb/source/API/SBFunction.cpp
index ac61220ec8736a..91f5e243af4144 100644
--- a/lldb/source/API/SBFunction.cpp
+++ b/lldb/source/API/SBFunction.cpp
@@ -119,8 +119,7 @@ SBInstructionList SBFunction::GetInstructions(SBTarget target,
if (m_opaque_ptr) {
TargetSP target_sp(target.GetSP());
std::unique_lock<std::recursive_mutex> lock;
- ModuleSP module_sp(
- m_opaque_ptr->GetAddressRange().GetBaseAddress().GetModule());
+ ModuleSP module_sp(m_opaque_ptr->GetAddress().GetModule());
if (target_sp && module_sp) {
lock = std::unique_lock<std::recursive_mutex>(target_sp->GetAPIMutex());
const bool force_live_memory = true;
diff --git a/lldb/source/Breakpoint/BreakpointResolver.cpp b/lldb/source/Breakpoint/BreakpointResolver.cpp
index 9643602d78c751..5fe544908c39e2 100644
--- a/lldb/source/Breakpoint/BreakpointResolver.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolver.cpp
@@ -325,7 +325,7 @@ void BreakpointResolver::AddLocation(SearchFilter &filter,
// If the line number is before the prologue end, move it there...
bool skipped_prologue = false;
if (skip_prologue && sc.function) {
- Address prologue_addr(sc.function->GetAddressRange().GetBaseAddress());
+ Address prologue_addr = sc.function->GetAddress();
if (prologue_addr.IsValid() && (line_start == prologue_addr)) {
const uint32_t prologue_byte_size = sc.function->GetPrologueByteSize();
if (prologue_byte_size) {
diff --git a/lldb/source/Breakpoint/BreakpointResolverName.cpp b/lldb/source/Breakpoint/BreakpointResolverName.cpp
index 264638eb836dc6..b9c8a1635d8440 100644
--- a/lldb/source/Breakpoint/BreakpointResolverName.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolverName.cpp
@@ -339,7 +339,7 @@ BreakpointResolverName::SearchCallback(SearchFilter &filter,
if (!sc.block->GetStartAddress(break_addr))
break_addr.Clear();
} else if (sc.function) {
- break_addr = sc.function->GetAddressRange().GetBaseAddress();
+ break_addr = sc.function->GetAddress();
if (m_skip_prologue && break_addr.IsValid()) {
const uint32_t prologue_byte_size = sc.function->GetPrologueByteSize();
if (prologue_byte_size)
diff --git a/lldb/source/Core/SearchFilter.cpp b/lldb/source/Core/SearchFilter.cpp
index 8027a62414bac0..6cd00af64870ae 100644
--- a/lldb/source/Core/SearchFilter.cpp
+++ b/lldb/source/Core/SearchFilter.cpp
@@ -152,7 +152,7 @@ bool SearchFilter::FunctionPasses(Function &function) {
// This is a slightly cheesy job, but since we don't have finer grained
// filters yet, just checking that the start address passes is probably
// good enough for the base class behavior.
- Address addr = function.GetAddressRange().GetBaseAddress();
+ Address addr = function.GetAddress();
return AddressPasses(addr);
}
diff --git a/lldb/source/Expression/DWARFExpressionList.cpp b/lldb/source/Expression/DWARFExpressionList.cpp
index 7a5cf9f9a0be46..be6c0a151159dc 100644
--- a/lldb/source/Expression/DWARFExpressionList.cpp
+++ b/lldb/source/Expression/DWARFExpressionList.cpp
@@ -126,8 +126,7 @@ bool DWARFExpressionList::MatchesOperand(
if (!sc.function)
return false;
- addr_t load_function_start =
- sc.function->GetAddressRange().GetBaseAddress().GetFileAddress();
+ addr_t load_function_start = sc.function->GetAddress().GetFileAddress();
if (load_function_start == LLDB_INVALID_ADDRESS)
return false;
diff --git a/lldb/source/Expression/IRExecutionUnit.cpp b/lldb/source/Expression/IRExecutionUnit.cpp
index 9158b43b1c5c0d..c8b4ddf705ec48 100644
--- a/lldb/source/Expression/IRExecutionUnit.cpp
+++ b/lldb/source/Expression/IRExecutionUnit.cpp
@@ -731,8 +731,7 @@ class LoadAddressResolver {
// If that didn't work, try the function.
if (load_address == LLDB_INVALID_ADDRESS && candidate_sc.function) {
- Address addr =
- candidate_sc.function->GetAddressRange().GetBaseAddress();
+ Address addr = candidate_sc.function->GetAddress();
load_address = m_target->GetProcessSP() ? addr.GetLoadAddress(m_target)
: addr.GetFileAddress();
}
diff --git a/lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp b/lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp
index 27ba10524141ed..5aa903443c760c 100644
--- a/lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp
+++ b/lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp
@@ -97,7 +97,7 @@ lldb::addr_t ArchitectureMips::GetBreakableLoadAddress(lldb::addr_t addr,
resolve_scope, sc);
Address sym_addr;
if (sc.function)
- sym_addr = sc.function->GetAddressRange().GetBaseAddress();
+ sym_addr = sc.function->GetAddress();
else if (sc.symbol)
sym_addr = sc.symbol->GetAddress();
diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
index 3659dfcd3c4ca3..8949b14c571893 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
@@ -796,10 +796,8 @@ bool DynamicLoaderDarwin::AlwaysRelyOnEHUnwindInfo(SymbolContext &sym_ctx) {
if (sym_ctx.symbol) {
module_sp = sym_ctx.symbol->GetAddressRef().GetModule();
}
- if (module_sp.get() == nullptr && sym_ctx.function) {
- module_sp =
- sym_ctx.function->GetAddressRange().GetBaseAddress().GetModule();
- }
+ if (module_sp.get() == nullptr && sym_ctx.function)
+ module_sp = sym_ctx.function->GetAddress().GetModule();
if (module_sp.get() == nullptr)
return false;
diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 34aca50df0ac4b..5614bc32468d5c 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -856,8 +856,7 @@ bool DynamicLoaderPOSIXDYLD::AlwaysRelyOnEHUnwindInfo(
if (sym_ctx.symbol)
module_sp = sym_ctx.symbol->GetAddressRef().GetModule();
if (!module_sp && sym_ctx.function)
- module_sp =
- sym_ctx.function->GetAddressRange().GetBaseAddress().GetModule();
+ module_sp = sym_ctx.function->GetAddress().GetModule();
if (!module_sp)
return false;
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
index bbe401b827f17d..9e96f6557c7baf 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -1883,7 +1883,7 @@ void ClangExpressionDeclMap::AddOneFunction(NameSearchContext &context,
return;
}
- fun_address = function->GetAddressRange().GetBaseAddress();
+ fun_address = function->GetAddress();
CompilerType copied_function_type = GuardedCopyType(function_clang_type);
if (copied_function_type) {
diff --git a/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp b/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
index fadc19676609bf..7e14e0477ee21c 100644
--- a/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
+++ b/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
@@ -304,7 +304,7 @@ size_t SymbolFileBreakpad::ParseBlocksRecursive(Function &func) {
blocks.push_back(&block);
size_t blocks_added = 0;
- addr_t func_base = func.GetAddressRange().GetBaseAddress().GetOffset();
+ addr_t func_base = func.GetAddress().GetOffset();
CompUnitData &data = m_cu_data->GetEntryRef(comp_unit->GetID()).data;
LineIterator It(*m_objfile_sp, Record::Func, data.bookmark),
End(*m_objfile_sp);
@@ -398,7 +398,7 @@ SymbolFileBreakpad::ResolveSymbolContext(const Address &so_addr,
Block &block = func_sp->GetBlock(true);
sc.block = block.FindInnermostBlockByOffset(
so_addr.GetFileAddress() -
- sc.function->GetAddressRange().GetBaseAddress().GetFileAddress());
+ sc.function->GetAddress().GetFileAddress());
if (sc.block)
result |= eSymbolContextBlock;
}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
index 08ea4c6d1645ad..71501881d4d86d 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
@@ -1062,8 +1062,7 @@ static void RemoveFunctionsWithModuleNotEqualTo(const ModuleSP &module_sp,
SymbolContext sc;
sc_list.GetContextAtIndex(i, sc);
if (sc.function) {
- const SectionSP section_sp(
- sc.function->GetAddressRange().GetBaseAddress().GetSection());
+ const SectionSP section_sp = sc.function->GetAddress().GetSection();
if (section_sp->GetModule() != module_sp) {
sc_list.RemoveContextAtIndex(i);
continue;
diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
index c0416b4d06815d..9f1470418660e0 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -421,8 +421,7 @@ Block &SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) {
lldbassert(func);
lldb::addr_t block_base =
m_index->MakeVirtualAddress(block.Segment, block.CodeOffset);
- lldb::addr_t func_base =
- func->GetAddressRange().GetBaseAddress().GetFileAddress();
+ lldb::addr_t func_base = func->GetAddress().GetFileAddress();
if (block_base >= func_base)
child_block->AddRange(Block::Range(block_base - func_base, block.CodeSize));
else {
@@ -1116,8 +1115,7 @@ uint32_t SymbolFileNativePDB::ResolveSymbolContext(
sc.function = GetOrCreateFunction(csid, *sc.comp_unit).get();
if (sc.function) {
Block &block = sc.function->GetBlock(true);
- addr_t func_base =
- sc.function->GetAddressRange().GetBaseAddress().GetFileAddress();
+ addr_t func_base = sc.function->GetAddress().GetFileAddress();
addr_t offset = file_addr - func_base;
sc.block = block.FindInnermostBlockByOffset(offset);
}
@@ -1128,8 +1126,7 @@ uint32_t SymbolFileNativePDB::ResolveSymbolContext(
sc.function = block.CalculateSymbolContextFunction();
if (sc.function) {
sc.function->GetBlock(true);
- addr_t func_base =
- sc.function->GetAddressRange().GetBaseAddress().GetFileAddress();
+ addr_t func_base = sc.function->GetAddress().GetFileAddress();
addr_t offset = file_addr - func_base;
sc.block = block.FindInnermostBlockByOffset(offset);
}
@@ -1854,8 +1851,7 @@ VariableSP SymbolFileNativePDB::CreateLocalVariable(PdbCompilandSymId scope_id,
// when lookuping local variables in this scope.
if (!var_info.location.IsValid())
var_info.location = DWARFExpressionList(module, DWARFExpression(), nullptr);
- var_info.location.SetFuncFileAddress(
- func->GetAddressRange().GetBaseAddress().GetFileAddress());
+ var_info.location.SetFuncFileAddress(func->GetAddress().GetFileAddress());
CompilandIndexItem *cii = m_index->compilands().GetCompiland(var_id.modi);
CompUnitSP comp_unit_sp = GetOrCreateCompileUnit(*cii);
TypeSP type_sp = GetOrCreateType(var_info.type);
diff --git a/lldb/source/Symbol/Block.cpp b/lldb/source/Symbol/Block.cpp
index 5c7772a6db780d..85d0aeca968e7b 100644
--- a/lldb/source/Symbol/Block.cpp
+++ b/lldb/source/Symbol/Block.cpp
@@ -37,10 +37,9 @@ void Block::GetDescription(Stream *s, Function *function,
addr_t base_addr = LLDB_INVALID_ADDRESS;
if (target)
- base_addr =
- function->GetAddressRange().GetBaseAddress().GetLoadAddress(target);
+ base_addr = function->GetAddress().GetLoadAddress(target);
if (base_addr == LLDB_INVALID_ADDRESS)
- base_addr = function->GetAddressRange().GetBaseAddress().GetFileAddress();
+ base_addr = function->GetAddress().GetFileAddress();
s->Printf(", range%s = ", num_ranges > 1 ? "s" : "");
for (size_t i = 0; i < num_ranges; ++i) {
@@ -305,7 +304,7 @@ bool Block::GetRangeAtIndex(uint32_t range_idx, AddressRange &range) {
Function *function = CalculateSymbolContextFunction();
if (function) {
const Range &vm_range = m_ranges.GetEntryRef(range_idx);
- range.GetBaseAddress() = function->GetAddressRange().GetBaseAddress();
+ range.GetBaseAddress() = function->GetAddress();
range.GetBaseAddress().Slide(vm_range.GetRangeBase());
range.SetByteSize(vm_range.GetByteSize());
return true;
@@ -323,7 +322,7 @@ AddressRanges Block::GetRanges() {
ranges.emplace_back();
auto &range = ranges.back();
const Range &vm_range = m_ranges.GetEntryRef(i);
- range.GetBaseAddress() = function->GetAddressRange().GetBaseAddress();
+ range.GetBaseAddress() = function->GetAddress();
range.GetBaseAddress().Slide(vm_range.GetRangeBase());
range.SetByteSize(vm_range.GetByteSize());
}
@@ -336,7 +335,7 @@ bool Block::GetStartAddress(Address &addr) {
Function *function = CalculateSymbolContextFunction();
if (function) {
- addr = function->GetAddressRange().GetBaseAddress();
+ addr = function->GetAddress();
addr.Slide(m_ranges.GetEntryRef(0).GetRangeBase());
return true;
}
@@ -355,8 +354,7 @@ void Block::AddRange(const Range &range) {
if (log) {
ModuleSP module_sp(m_parent_scope->CalculateSymbolContextModule());
Function *function = m_parent_scope->CalculateSymbolContextFunction();
- const addr_t function_file_addr =
- function->GetAddressRange().GetBaseAddress().GetFileAddress();
+ const addr_t function_file_addr = function->GetAddress().GetFileAddress();
const addr_t block_start_addr = function_file_addr + range.GetRangeBase();
const addr_t block_end_addr = function_file_addr + range.GetRangeEnd();
Type *func_type = function->GetType();
diff --git a/lldb/source/Symbol/Function.cpp b/lldb/source/Symbol/Function.cpp
index f590cf4632bce0..cd5728b8180f23 100644
--- a/lldb/source/Symbol/Function.cpp
+++ b/lldb/source/Symbol/Function.cpp
@@ -133,7 +133,7 @@ lldb::addr_t CallEdge::GetLoadAddress(lldb::addr_t unresolved_pc,
Function &caller, Target &target) {
Log *log = GetLog(LLDBLog::Step);
- const Address &caller_start_addr = caller.GetAddressRange().GetBaseAddress();
+ const Address &caller_start_addr = caller.GetAddress();
ModuleSP caller_module_sp = caller_start_addr.GetModule();
if (!caller_module_sp) {
@@ -307,8 +307,7 @@ void Function::GetStartLineSourceInfo(FileSpec &source_file,
return;
LineEntry line_entry;
- if (line_table->FindLineEntryByAddress(GetAddressRange().GetBaseAddress(),
- line_entry, nullptr)) {
+ if (line_table->FindLineEntryByAddress(GetAddress(), line_entry, nullptr)) {
line_no = line_entry.line;
source_file = line_entry.GetFile();
}
@@ -407,6 +406,15 @@ CompileUnit *Function::GetCompileUnit() { return m_comp_unit; }
const CompileUnit *Function::GetCompileUnit() const { return m_comp_unit; }
+Address Function::GetAddress() const {
+ if (m_ranges.empty())
+ return Address();
+ // We're using a (DWARF-like) convention where the base address of the first
+ // interval denotes the entry point of the function. If that turns out to be
+ // insufficient, we can introduce a separate "entry point address" field.
+ return m_ranges[0].GetBaseAddress();
+}
+
void Function::GetDescription(Stream *s, lldb::DescriptionLevel level,
Target *target) {
ConstString name = GetName();
@@ -476,7 +484,7 @@ Function *Function::CalculateSymbolContextFunction() { return this; }
lldb::DisassemblerSP Function::GetInstructions(const ExecutionContext &exe_ctx,
const char *flavor,
bool prefer_file_cache) {
- ModuleSP module_sp(GetAddressRange().GetBaseAddress().GetModule());
+ ModuleSP module_sp = GetAddress().GetModule();
if (module_sp && exe_ctx.HasTargetScope()) {
return Disassembler::DisassembleRange(
module_sp->GetArchitecture(), nullptr, nullptr, nullptr, flavor,
@@ -593,8 +601,7 @@ uint32_t Function::GetPrologueByteSize() {
if (line_table) {
LineEntry first_line_entry;
uint32_t first_line_entry_idx = UINT32_MAX;
- if (line_table->FindLineEntryByAddress(GetAddressRange().GetBaseAddress(),
- first_line_entry,
+ if (line_table->FindLineEntryByAddress(GetAddress(), first_line_entry,
&first_line_entry_idx)) {
// Make sure the first line entry isn't already the end of the prologue
addr_t prologue_end_file_addr = LLDB_INVALID_ADDRESS;
diff --git a/lldb/source/Symbol/SymbolContext.cpp b/lldb/source/Symbol/SymbolContext.cpp
index de083e81206e2a..76a4c3d6b04e97 100644
--- a/lldb/source/Symbol/SymbolContext.cpp
+++ b/lldb/source/Symbol/SymbolContext.cpp
@@ -104,8 +104,7 @@ bool SymbolContext::DumpStopContext(
if (addr.IsValid()) {
const addr_t function_offset =
- addr.GetOffset() -
- function->GetAddressRange().GetBaseAddress().GetOffset();
+ addr.GetOffset() - function->GetAddress().GetOffset();
if (!show_function_name) {
// Print +offset even if offset is 0
dumped_something = true;
@@ -698,9 +697,7 @@ LineEntry SymbolContext::GetFunctionStartLineEntry() const {
}
if (function) {
- if (function->GetAddressRange()
- .GetBaseAddress()
- .CalculateSymbolContextLineEntry(line_entry))
+ if (function->GetAddress().CalculateSymbolContextLineEntry(line_entry))
return line_entry;
}
return LineEntry();
@@ -1226,8 +1223,7 @@ bool SymbolContextList::AppendIfUnique(const SymbolContext &sc,
continue;
if (pos->function) {
- if (pos->function->GetAddressRange().GetBaseAddress() ==
- sc.symbol->GetAddressRef()) {
+ if (pos->function->GetAddress() == sc.symbol->GetAddressRef()) {
// Do we already have a function with this symbol?
if (pos->symbol == sc.symbol)
return false;
diff --git a/lldb/source/Symbol/Variable.cpp b/lldb/source/Symbol/Variable.cpp
index a63e4f973537fa..8244725aba545a 100644
--- a/lldb/source/Symbol/Variable.cpp
+++ b/lldb/source/Symbol/Variable.cpp
@@ -221,8 +221,7 @@ bool Variable::LocationIsValidForFrame(StackFrame *frame) {
...
[truncated]
|
jasonmolenda
approved these changes
Nov 13, 2024
lldb/source/Symbol/Function.cpp
Outdated
| // We're using a (DWARF-like) convention where the base address of the first | ||
| // interval denotes the entry point of the function. If that turns out to be | ||
| // insufficient, we can introduce a separate "entry point address" field. | ||
| return m_ranges[0].GetBaseAddress(); |
There was a problem hiding this comment.
Function::CollapseRanges seems to imply that the first Range may not have the lowest address of all ranges. Should this return m_range.GetBaseAddress() using CollapseRanges's calculation intsead?
There was a problem hiding this comment.
There's no guarantee that the function's entry point will be its lowest numbered address. DWARF even allows you (via DW_AT_entry_pc) to have the entry pc point to the middle of a range. I don't think any compiler emits that, and lldb doesn't support that right now, but if we wanted to do it, then we'd need to have a separate m_address field here independent of any range (that's what I'm alluding to in that comment).
The "base address for relocation of blocks and variables" use case is totally under our control, so we could choose that to be based on the lowest address of the function, but that would mean we'd have to have two accessors (and then be careful about which one is used where). I think it's better to use the same address for both -- and accept the fact that some of the block/variable offsets may be negative. (Having written that, I realize this also means that we'd need to change the block/variable offsets to signed representations. Right now they're unsigned, which I might sort of make this patch a regression for those kinds of functions, but given that discontinuous functions are barely supported anyway, I'd like to do that separately.) This also fits in nicely with DWARF which says that "The base address of the scope for any of the debugging information entries listed
above is given by either the DW_AT_low_pc attribute or the first address in the first range entry in the list of ranges given by the DW_AT_ranges attribute." -- it doesn't care whether the base address is numerically lowest.
Let me know what you think.
There was a problem hiding this comment.
Also having written this, I realized that this patch does make a functional change (for the better) -- lldb will now correctly return the entry point of the function in case its entry point is not its lowest address. I'm going to add a test case for that.
There was a problem hiding this comment.
And that discovers another problem -- we sort the ranges long before they make their way here. I'll need to rethink this patch (the result will probably involve adding an explicit "entry point" member).
There was a problem hiding this comment.
Finally got back to this. The main change is that the Function class now has a separate member for the function address. Right now it's still the lowest function address, but in the next patch I'll change it to pass the function address explicitly (and independently of the sorted ranges of the function).
Many calls to Function::GetAddressRange() were not interested in the range itself. Instead they wanted to find the address of the function (its entry point) or the base address for relocation of function-scoped entities. This PR creates a separate function for retrieving this, and changes the existing (non-controversial) uses to call that instead.
DavidSpickett
approved these changes
Jan 9, 2025
|
The reason I picked this name is to match Symbol::GetAddress. |
BaiXilin
pushed a commit
to BaiXilin/llvm-fix-vnni-instr-types
that referenced
this pull request
Jan 12, 2025
Many calls to Function::GetAddressRange() were not interested in the range itself. Instead they wanted to find the address of the function (its entry point) or the base address for relocation of function-scoped entities (technically, the two don't need to be the same, but there's isn't good reason for them not to be). This PR creates a separate function for retrieving this, and changes the existing (non-controversial) uses to call that instead.
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.
Many calls to Function::GetAddressRange() were not interested in the range itself. Instead they wanted to find the address of the function (its entry point) or the base address for relocation of function-scoped entities (technically, the two don't need to be the same, but there's isn't good reason for them not to be). This PR creates a separate function for retrieving this, and changes the existing (non-controversial) uses to call that instead.