[lldb] Fix offset computation in RegisterContextUnwind (#137155)

AddressFunctionScope was always returning the first address range of the
function (assuming it was the only one). This doesn't work for
RegisterContextUnwind (it's only caller), when the function doesn't
start at the lowest address because it throws off the 'how many bytes
"into" a function I am' computation. This patch replaces the result with
a call to (recently introduced)
SymbolContext::GetFunctionOrSymbolAddress.
This commit is contained in:
Pavel Labath 2025-05-15 09:39:11 +02:00 committed by GitHub
parent 647db1b02d
commit 85c3c98630
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 79 additions and 100 deletions

View File

@ -371,22 +371,15 @@ public:
bool ResolveAddressUsingFileSections(lldb::addr_t addr,
const SectionList *sections);
/// Resolve this address to its containing function and optionally get
/// that function's address range.
/// Resolve this address to its containing function.
///
/// \param[out] sym_ctx
/// The symbol context describing the function in which this address lies
///
/// \parm[out] addr_range_ptr
/// Pointer to the AddressRange to fill in with the function's address
/// range. Caller may pass null if they don't need the address range.
///
/// \return
/// Returns \b false if the function/symbol could not be resolved
/// or if the address range was requested and could not be resolved;
/// Returns \b false if the function/symbol could not be resolved;
/// returns \b true otherwise.
bool ResolveFunctionScope(lldb_private::SymbolContext &sym_ctx,
lldb_private::AddressRange *addr_range_ptr = nullptr);
bool ResolveFunctionScope(lldb_private::SymbolContext &sym_ctx);
/// Set the address to represent \a load_addr.
///

View File

@ -263,22 +263,11 @@ bool Address::ResolveAddressUsingFileSections(addr_t file_addr,
return false; // Failed to resolve this address to a section offset value
}
/// if "addr_range_ptr" is not NULL, then fill in with the address range of the function.
bool Address::ResolveFunctionScope(SymbolContext &sym_ctx,
AddressRange *addr_range_ptr) {
bool Address::ResolveFunctionScope(SymbolContext &sym_ctx) {
constexpr SymbolContextItem resolve_scope =
eSymbolContextFunction | eSymbolContextSymbol;
if (!(CalculateSymbolContext(&sym_ctx, resolve_scope) & resolve_scope)) {
if (addr_range_ptr)
addr_range_ptr->Clear();
return false;
}
if (!addr_range_ptr)
return true;
return sym_ctx.GetAddressRange(resolve_scope, 0, false, *addr_range_ptr);
return CalculateSymbolContext(&sym_ctx, resolve_scope) & resolve_scope;
}
ModuleSP Address::GetModule() const {

View File

@ -160,8 +160,7 @@ void RegisterContextUnwind::InitializeZerothFrame() {
UnwindLogMsg("using architectural default unwind method");
}
AddressRange addr_range;
m_sym_ctx_valid = m_current_pc.ResolveFunctionScope(m_sym_ctx, &addr_range);
m_sym_ctx_valid = m_current_pc.ResolveFunctionScope(m_sym_ctx);
if (m_sym_ctx.symbol) {
UnwindLogMsg("with pc value of 0x%" PRIx64 ", symbol name is '%s'",
@ -185,15 +184,9 @@ void RegisterContextUnwind::InitializeZerothFrame() {
// If we were able to find a symbol/function, set addr_range to the bounds of
// that symbol/function. else treat the current pc value as the start_pc and
// record no offset.
if (addr_range.GetBaseAddress().IsValid()) {
m_start_pc = addr_range.GetBaseAddress();
if (m_current_pc.GetSection() == m_start_pc.GetSection()) {
m_current_offset = m_current_pc.GetOffset() - m_start_pc.GetOffset();
} else if (m_current_pc.GetModule() == m_start_pc.GetModule()) {
// This means that whatever symbol we kicked up isn't really correct ---
// we should not cross section boundaries ... We really should NULL out
// the function/symbol in this case unless there is a bad assumption here
// due to inlined functions?
if (m_sym_ctx_valid) {
m_start_pc = m_sym_ctx.GetFunctionOrSymbolAddress();
if (m_current_pc.GetModule() == m_start_pc.GetModule()) {
m_current_offset =
m_current_pc.GetFileAddress() - m_start_pc.GetFileAddress();
}
@ -499,8 +492,7 @@ void RegisterContextUnwind::InitializeNonZerothFrame() {
return;
}
AddressRange addr_range;
m_sym_ctx_valid = m_current_pc.ResolveFunctionScope(m_sym_ctx, &addr_range);
m_sym_ctx_valid = m_current_pc.ResolveFunctionScope(m_sym_ctx);
if (m_sym_ctx.symbol) {
UnwindLogMsg("with pc value of 0x%" PRIx64 ", symbol name is '%s'", pc,
@ -524,9 +516,8 @@ void RegisterContextUnwind::InitializeNonZerothFrame() {
// Don't decrement if we're "above" an asynchronous event like
// sigtramp.
decr_pc_and_recompute_addr_range = false;
} else if (!addr_range.GetBaseAddress().IsValid() ||
addr_range.GetBaseAddress().GetSection() != m_current_pc.GetSection() ||
addr_range.GetBaseAddress().GetOffset() != m_current_pc.GetOffset()) {
} else if (Address addr = m_sym_ctx.GetFunctionOrSymbolAddress();
addr != m_current_pc) {
// If our "current" pc isn't the start of a function, decrement the pc
// if we're up the stack.
if (m_behaves_like_zeroth_frame)
@ -559,7 +550,7 @@ void RegisterContextUnwind::InitializeNonZerothFrame() {
Address temporary_pc;
temporary_pc.SetLoadAddress(pc - 1, &process->GetTarget());
m_sym_ctx.Clear(false);
m_sym_ctx_valid = temporary_pc.ResolveFunctionScope(m_sym_ctx, &addr_range);
m_sym_ctx_valid = temporary_pc.ResolveFunctionScope(m_sym_ctx);
UnwindLogMsg("Symbol is now %s",
GetSymbolOrFunctionName(m_sym_ctx).AsCString(""));
@ -568,8 +559,8 @@ void RegisterContextUnwind::InitializeNonZerothFrame() {
// If we were able to find a symbol/function, set addr_range_ptr to the
// bounds of that symbol/function. else treat the current pc value as the
// start_pc and record no offset.
if (addr_range.GetBaseAddress().IsValid()) {
m_start_pc = addr_range.GetBaseAddress();
if (m_sym_ctx_valid) {
m_start_pc = m_sym_ctx.GetFunctionOrSymbolAddress();
m_current_offset = pc - m_start_pc.GetLoadAddress(&process->GetTarget());
m_current_offset_backed_up_one = m_current_offset;
if (decr_pc_and_recompute_addr_range &&
@ -1952,8 +1943,7 @@ void RegisterContextUnwind::PropagateTrapHandlerFlagFromUnwindPlan(
GetSymbolOrFunctionName(m_sym_ctx).AsCString(""));
m_current_offset_backed_up_one = m_current_offset;
AddressRange addr_range;
m_sym_ctx_valid = m_current_pc.ResolveFunctionScope(m_sym_ctx, &addr_range);
m_sym_ctx_valid = m_current_pc.ResolveFunctionScope(m_sym_ctx);
UnwindLogMsg("Symbol is now %s",
GetSymbolOrFunctionName(m_sym_ctx).AsCString(""));
@ -1962,9 +1952,11 @@ void RegisterContextUnwind::PropagateTrapHandlerFlagFromUnwindPlan(
Process *process = exe_ctx.GetProcessPtr();
Target *target = &process->GetTarget();
m_start_pc = addr_range.GetBaseAddress();
m_current_offset =
m_current_pc.GetLoadAddress(target) - m_start_pc.GetLoadAddress(target);
if (m_sym_ctx_valid) {
m_start_pc = m_sym_ctx.GetFunctionOrSymbolAddress();
m_current_offset = m_current_pc.GetLoadAddress(target) -
m_start_pc.GetLoadAddress(target);
}
}
}

View File

@ -19,6 +19,49 @@ baz:
.Lbaz_end:
.size baz, .Lbaz_end-baz
foo.__part.3:
.cfi_startproc
.cfi_def_cfa_offset 32
.cfi_offset %rbx, -16
addq $24, %rsp
.cfi_def_cfa %rsp, 8
retq
.Lfoo.__part.3_end:
.size foo.__part.3, .Lfoo.__part.3_end-foo.__part.3
.cfi_endproc
# NB: Deliberately inserting padding to separate the two parts of the function
# as we're currently only parsing a single FDE entry from a (coalesced) address
# range.
nop
foo.__part.1:
.cfi_startproc
.cfi_def_cfa_offset 32
.cfi_offset %rbx, -16
subq $16, %rsp
.cfi_def_cfa_offset 48
callq bar
addq $16, %rsp
.cfi_def_cfa_offset 32
jmp foo.__part.3
.Lfoo.__part.1_end:
.size foo.__part.1, .Lfoo.__part.1_end-foo.__part.1
.cfi_endproc
bar:
.cfi_startproc
subq $88, %rsp
.cfi_def_cfa_offset 96
xorl %edi, %edi
callq foo
addq $88, %rsp
.cfi_def_cfa %rsp, 8
retq
.cfi_endproc
.Lbar_end:
.size bar, .Lbar_end-bar
.type foo,@function
foo:
.cfi_startproc
@ -28,6 +71,8 @@ foo:
movl %edi, %ebx
cmpl $0, %ebx
je foo.__part.2
subq $16, %rsp
.cfi_def_cfa_offset 32
jmp foo.__part.1
.cfi_endproc
.Lfoo_end:
@ -38,31 +83,6 @@ foo:
# range.
nop
foo.__part.1:
.cfi_startproc
.cfi_def_cfa_offset 16
.cfi_offset %rbx, -16
subq $16, %rsp
.cfi_def_cfa_offset 32
callq bar
jmp foo.__part.3
.Lfoo.__part.1_end:
.size foo.__part.1, .Lfoo.__part.1_end-foo.__part.1
.cfi_endproc
bar:
.cfi_startproc
subq $24, %rsp
.cfi_def_cfa_offset 32
xorl %edi, %edi
callq foo
addq $24, %rsp
.cfi_def_cfa %rsp, 8
retq
.cfi_endproc
.Lbar_end:
.size bar, .Lbar_end-bar
foo.__part.2:
.cfi_startproc
.cfi_def_cfa_offset 16
@ -75,23 +95,6 @@ foo.__part.2:
.size foo.__part.2, .Lfoo.__part.2_end-foo.__part.2
.cfi_endproc
# NB: Deliberately inserting padding to separate the two parts of the function
# as we're currently only parsing a single FDE entry from a (coalesced) address
# range.
nop
foo.__part.3:
.cfi_startproc
.cfi_def_cfa_offset 32
.cfi_offset %rbx, -16
addq $24, %rsp
.cfi_def_cfa %rsp, 8
retq
.Lfoo.__part.3_end:
.size foo.__part.3, .Lfoo.__part.3_end-foo.__part.3
.cfi_endproc
.globl main
.type main,@function
main:

View File

@ -22,15 +22,17 @@ image show-unwind --cached true -n foo
# CHECK-NEXT: This UnwindPlan is sourced from the compiler: yes.
# CHECK-NEXT: This UnwindPlan is valid at all instruction locations: no.
# CHECK-NEXT: This UnwindPlan is for a trap handler function: no.
# CHECK-NEXT: Address range of this UnwindPlan: [{{.*}}.text + 6-0x0000000000000010)[{{.*}}.text + 17-0x000000000000001c)[{{.*}}.text + 44-0x0000000000000037)[{{.*}}.text + 56-0x000000000000003d)
# CHECK-NEXT: row[0]: 0: CFA=rsp +8 => rip=[CFA-8]
# CHECK-NEXT: row[1]: 1: CFA=rsp+16 => rbx=[CFA-16] rip=[CFA-8]
# CHECK-NEXT: row[2]: 11: CFA=rsp+16 => rbx=[CFA-16] rip=[CFA-8]
# CHECK-NEXT: row[3]: 15: CFA=rsp+32 => rbx=[CFA-16] rip=[CFA-8]
# CHECK-NEXT: row[4]: 38: CFA=rsp+16 => rbx=[CFA-16] rip=[CFA-8]
# CHECK-NEXT: row[5]: 42: CFA=rsp+32 => rbx=[CFA-16] rip=[CFA-8]
# CHECK-NEXT: row[6]: 50: CFA=rsp+32 => rbx=[CFA-16] rip=[CFA-8]
# CHECK-NEXT: row[7]: 54: CFA=rsp +8 => rbx=[CFA-16] rip=[CFA-8]
# CHECK-NEXT: Address range of this UnwindPlan: [{{.*}}.text + 6-0x000000000000000b)[{{.*}}.text + 12-0x000000000000001b)[{{.*}}.text + 43-0x0000000000000039)[{{.*}}.text + 58-0x0000000000000045)
# CHECK-NEXT: row[0]: -37: CFA=rsp+32 => rbx=[CFA-16] rip=[CFA-8]
# CHECK-NEXT: row[1]: -33: CFA=rsp +8 => rbx=[CFA-16] rip=[CFA-8]
# CHECK-NEXT: row[2]: -31: CFA=rsp+32 => rbx=[CFA-16] rip=[CFA-8]
# CHECK-NEXT: row[3]: -27: CFA=rsp+48 => rbx=[CFA-16] rip=[CFA-8]
# CHECK-NEXT: row[4]: -18: CFA=rsp+32 => rbx=[CFA-16] rip=[CFA-8]
# CHECK-NEXT: row[5]: 0: CFA=rsp +8 => rip=[CFA-8]
# CHECK-NEXT: row[6]: 1: CFA=rsp+16 => rbx=[CFA-16] rip=[CFA-8]
# CHECK-NEXT: row[7]: 12: CFA=rsp+32 => rbx=[CFA-16] rip=[CFA-8]
# CHECK-NEXT: row[8]: 15: CFA=rsp+16 => rbx=[CFA-16] rip=[CFA-8]
# CHECK-NEXT: row[9]: 19: CFA=rsp+32 => rbx=[CFA-16] rip=[CFA-8]
# CHECK-EMPTY:
image show-unwind --cached true -n bar
@ -41,8 +43,8 @@ image show-unwind --cached true -n bar
# CHECK-NEXT: This UnwindPlan is sourced from the compiler: yes.
# CHECK-NEXT: This UnwindPlan is valid at all instruction locations: no.
# CHECK-NEXT: This UnwindPlan is for a trap handler function: no.
# CHECK-NEXT: Address range of this UnwindPlan: [{{.*}}.text + 28-0x000000000000002c)
# CHECK-NEXT: Address range of this UnwindPlan: [{{.*}}.text + 27-0x000000000000002b)
# CHECK-NEXT: row[0]: 0: CFA=rsp +8 => rip=[CFA-8]
# CHECK-NEXT: row[1]: 4: CFA=rsp+32 => rip=[CFA-8]
# CHECK-NEXT: row[1]: 4: CFA=rsp+96 => rip=[CFA-8]
# CHECK-NEXT: row[2]: 15: CFA=rsp +8 => rip=[CFA-8]
# CHECK-EMPTY: