Skip to content

[lldb] Fix offset computation in RegisterContextUnwind #137155

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 15, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 3 additions & 10 deletions lldb/include/lldb/Core/Address.h
Original file line number Diff line number Diff line change
Expand Up @@ -371,22 +371,15 @@ class Address {
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.
///
Expand Down
15 changes: 2 additions & 13 deletions lldb/source/Core/Address.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
40 changes: 16 additions & 24 deletions lldb/source/Target/RegisterContextUnwind.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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'",
Expand All @@ -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();
}
Expand Down Expand Up @@ -498,8 +491,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,
Expand All @@ -523,9 +515,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)
Expand Down Expand Up @@ -558,7 +549,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(""));
Expand All @@ -567,8 +558,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 &&
Expand Down Expand Up @@ -1939,8 +1930,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(""));
Expand All @@ -1949,9 +1939,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);
}
}
}

Expand Down
59 changes: 31 additions & 28 deletions lldb/test/Shell/Unwind/Inputs/basic-block-sections-with-dwarf.s
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,16 @@ baz:
.Lbaz_end:
.size baz, .Lbaz_end-baz

.type foo,@function
foo:
foo.__part.3:
.cfi_startproc
pushq %rbx
.cfi_def_cfa_offset 16
.cfi_def_cfa_offset 32
.cfi_offset %rbx, -16
movl %edi, %ebx
cmpl $0, %ebx
je foo.__part.2
jmp foo.__part.1
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
.Lfoo_end:
.size foo, .Lfoo_end-foo

# 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
Expand All @@ -40,58 +37,64 @@ foo:

foo.__part.1:
.cfi_startproc
.cfi_def_cfa_offset 16
.cfi_def_cfa_offset 32
.cfi_offset %rbx, -16
subq $16, %rsp
.cfi_def_cfa_offset 32
.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 $24, %rsp
.cfi_def_cfa_offset 32
subq $88, %rsp
.cfi_def_cfa_offset 96
xorl %edi, %edi
callq foo
addq $24, %rsp
addq $88, %rsp
.cfi_def_cfa %rsp, 8
retq
.cfi_endproc
.Lbar_end:
.size bar, .Lbar_end-bar

foo.__part.2:
.type foo,@function
foo:
.cfi_startproc
pushq %rbx
.cfi_def_cfa_offset 16
.cfi_offset %rbx, -16
movl %edi, %ebx
cmpl $0, %ebx
je foo.__part.2
subq $16, %rsp
.cfi_def_cfa_offset 32
callq baz
jmp foo.__part.3
.Lfoo.__part.2_end:
.size foo.__part.2, .Lfoo.__part.2_end-foo.__part.2
jmp foo.__part.1
.cfi_endproc
.Lfoo_end:
.size foo, .Lfoo_end-foo

# 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:
foo.__part.2:
.cfi_startproc
.cfi_def_cfa_offset 32
.cfi_def_cfa_offset 16
.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
subq $16, %rsp
.cfi_def_cfa_offset 32
callq baz
jmp foo.__part.3
.Lfoo.__part.2_end:
.size foo.__part.2, .Lfoo.__part.2_end-foo.__part.2
.cfi_endproc


.globl main
.type main,@function
main:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Loading