Skip to content

Commit d966272

Browse files
labathlabath
committed
[lldb] Fix offset computation in RegisterContextUnwind
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.
1 parent bae70db commit d966272

File tree

5 files changed

+65
-86
lines changed

5 files changed

+65
-86
lines changed

‎lldb/include/lldb/Core/Address.h‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -371,22 +371,15 @@ class Address {
371371
bool ResolveAddressUsingFileSections(lldb::addr_t addr,
372372
const SectionList *sections);
373373

374-
/// Resolve this address to its containing function and optionally get
375-
/// that function's address range.
374+
/// Resolve this address to its containing function.
376375
///
377376
/// \param[out] sym_ctx
378377
/// The symbol context describing the function in which this address lies
379378
///
380-
/// \parm[out] addr_range_ptr
381-
/// Pointer to the AddressRange to fill in with the function's address
382-
/// range. Caller may pass null if they don't need the address range.
383-
///
384379
/// \return
385-
/// Returns \b false if the function/symbol could not be resolved
386-
/// or if the address range was requested and could not be resolved;
380+
/// Returns \b false if the function/symbol could not be resolved;
387381
/// returns \b true otherwise.
388-
bool ResolveFunctionScope(lldb_private::SymbolContext &sym_ctx,
389-
lldb_private::AddressRange *addr_range_ptr = nullptr);
382+
bool ResolveFunctionScope(lldb_private::SymbolContext &sym_ctx);
390383

391384
/// Set the address to represent \a load_addr.
392385
///

‎lldb/source/Core/Address.cpp‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -263,22 +263,11 @@ bool Address::ResolveAddressUsingFileSections(addr_t file_addr,
263263
return false; // Failed to resolve this address to a section offset value
264264
}
265265

266-
/// if "addr_range_ptr" is not NULL, then fill in with the address range of the function.
267-
bool Address::ResolveFunctionScope(SymbolContext &sym_ctx,
268-
AddressRange *addr_range_ptr) {
266+
bool Address::ResolveFunctionScope(SymbolContext &sym_ctx) {
269267
constexpr SymbolContextItem resolve_scope =
270268
eSymbolContextFunction | eSymbolContextSymbol;
271269

272-
if (!(CalculateSymbolContext(&sym_ctx, resolve_scope) & resolve_scope)) {
273-
if (addr_range_ptr)
274-
addr_range_ptr->Clear();
275-
return false;
276-
}
277-
278-
if (!addr_range_ptr)
279-
return true;
280-
281-
return sym_ctx.GetAddressRange(resolve_scope, 0, false, *addr_range_ptr);
270+
return CalculateSymbolContext(&sym_ctx, resolve_scope) & resolve_scope;
282271
}
283272

284273
ModuleSP Address::GetModule() const {

‎lldb/source/Target/RegisterContextUnwind.cpp‎

Lines changed: 16 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -160,8 +160,7 @@ void RegisterContextUnwind::InitializeZerothFrame() {
160160
UnwindLogMsg("using architectural default unwind method");
161161
}
162162

163-
AddressRange addr_range;
164-
m_sym_ctx_valid = m_current_pc.ResolveFunctionScope(m_sym_ctx, &addr_range);
163+
m_sym_ctx_valid = m_current_pc.ResolveFunctionScope(m_sym_ctx);
165164

166165
if (m_sym_ctx.symbol) {
167166
UnwindLogMsg("with pc value of 0x%" PRIx64 ", symbol name is '%s'",
@@ -185,15 +184,9 @@ void RegisterContextUnwind::InitializeZerothFrame() {
185184
// If we were able to find a symbol/function, set addr_range to the bounds of
186185
// that symbol/function. else treat the current pc value as the start_pc and
187186
// record no offset.
188-
if (addr_range.GetBaseAddress().IsValid()) {
189-
m_start_pc = addr_range.GetBaseAddress();
190-
if (m_current_pc.GetSection() == m_start_pc.GetSection()) {
191-
m_current_offset = m_current_pc.GetOffset() - m_start_pc.GetOffset();
192-
} else if (m_current_pc.GetModule() == m_start_pc.GetModule()) {
193-
// This means that whatever symbol we kicked up isn't really correct ---
194-
// we should not cross section boundaries ... We really should NULL out
195-
// the function/symbol in this case unless there is a bad assumption here
196-
// due to inlined functions?
187+
if (m_sym_ctx_valid) {
188+
m_start_pc = m_sym_ctx.GetFunctionOrSymbolAddress();
189+
if (m_current_pc.GetModule() == m_start_pc.GetModule()) {
197190
m_current_offset =
198191
m_current_pc.GetFileAddress() - m_start_pc.GetFileAddress();
199192
}
@@ -498,8 +491,7 @@ void RegisterContextUnwind::InitializeNonZerothFrame() {
498491
return;
499492
}
500493

501-
AddressRange addr_range;
502-
m_sym_ctx_valid = m_current_pc.ResolveFunctionScope(m_sym_ctx, &addr_range);
494+
m_sym_ctx_valid = m_current_pc.ResolveFunctionScope(m_sym_ctx);
503495

504496
if (m_sym_ctx.symbol) {
505497
UnwindLogMsg("with pc value of 0x%" PRIx64 ", symbol name is '%s'", pc,
@@ -523,9 +515,8 @@ void RegisterContextUnwind::InitializeNonZerothFrame() {
523515
// Don't decrement if we're "above" an asynchronous event like
524516
// sigtramp.
525517
decr_pc_and_recompute_addr_range = false;
526-
} else if (!addr_range.GetBaseAddress().IsValid() ||
527-
addr_range.GetBaseAddress().GetSection() != m_current_pc.GetSection() ||
528-
addr_range.GetBaseAddress().GetOffset() != m_current_pc.GetOffset()) {
518+
} else if (Address addr = m_sym_ctx.GetFunctionOrSymbolAddress();
519+
addr != m_current_pc) {
529520
// If our "current" pc isn't the start of a function, decrement the pc
530521
// if we're up the stack.
531522
if (m_behaves_like_zeroth_frame)
@@ -558,7 +549,7 @@ void RegisterContextUnwind::InitializeNonZerothFrame() {
558549
Address temporary_pc;
559550
temporary_pc.SetLoadAddress(pc - 1, &process->GetTarget());
560551
m_sym_ctx.Clear(false);
561-
m_sym_ctx_valid = temporary_pc.ResolveFunctionScope(m_sym_ctx, &addr_range);
552+
m_sym_ctx_valid = temporary_pc.ResolveFunctionScope(m_sym_ctx);
562553

563554
UnwindLogMsg("Symbol is now %s",
564555
GetSymbolOrFunctionName(m_sym_ctx).AsCString(""));
@@ -567,8 +558,8 @@ void RegisterContextUnwind::InitializeNonZerothFrame() {
567558
// If we were able to find a symbol/function, set addr_range_ptr to the
568559
// bounds of that symbol/function. else treat the current pc value as the
569560
// start_pc and record no offset.
570-
if (addr_range.GetBaseAddress().IsValid()) {
571-
m_start_pc = addr_range.GetBaseAddress();
561+
if (m_sym_ctx_valid) {
562+
m_start_pc = m_sym_ctx.GetFunctionOrSymbolAddress();
572563
m_current_offset = pc - m_start_pc.GetLoadAddress(&process->GetTarget());
573564
m_current_offset_backed_up_one = m_current_offset;
574565
if (decr_pc_and_recompute_addr_range &&
@@ -1939,8 +1930,7 @@ void RegisterContextUnwind::PropagateTrapHandlerFlagFromUnwindPlan(
19391930
GetSymbolOrFunctionName(m_sym_ctx).AsCString(""));
19401931
m_current_offset_backed_up_one = m_current_offset;
19411932

1942-
AddressRange addr_range;
1943-
m_sym_ctx_valid = m_current_pc.ResolveFunctionScope(m_sym_ctx, &addr_range);
1933+
m_sym_ctx_valid = m_current_pc.ResolveFunctionScope(m_sym_ctx);
19441934

19451935
UnwindLogMsg("Symbol is now %s",
19461936
GetSymbolOrFunctionName(m_sym_ctx).AsCString(""));
@@ -1949,9 +1939,11 @@ void RegisterContextUnwind::PropagateTrapHandlerFlagFromUnwindPlan(
19491939
Process *process = exe_ctx.GetProcessPtr();
19501940
Target *target = &process->GetTarget();
19511941

1952-
m_start_pc = addr_range.GetBaseAddress();
1953-
m_current_offset =
1954-
m_current_pc.GetLoadAddress(target) - m_start_pc.GetLoadAddress(target);
1942+
if (m_sym_ctx_valid) {
1943+
m_start_pc = m_sym_ctx.GetFunctionOrSymbolAddress();
1944+
m_current_offset = m_current_pc.GetLoadAddress(target) -
1945+
m_start_pc.GetLoadAddress(target);
1946+
}
19551947
}
19561948
}
19571949

‎lldb/test/Shell/Unwind/Inputs/basic-block-sections-with-dwarf.s‎

Lines changed: 31 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -19,19 +19,16 @@ baz:
1919
.Lbaz_end:
2020
.size baz, .Lbaz_end-baz
2121

22-
.type foo,@function
23-
foo:
22+
foo.__part.3:
2423
.cfi_startproc
25-
pushq %rbx
26-
.cfi_def_cfa_offset 16
24+
.cfi_def_cfa_offset 32
2725
.cfi_offset %rbx, -16
28-
movl %edi, %ebx
29-
cmpl $0, %ebx
30-
je foo.__part.2
31-
jmp foo.__part.1
26+
addq $24, %rsp
27+
.cfi_def_cfa %rsp, 8
28+
retq
29+
.Lfoo.__part.3_end:
30+
.size foo.__part.3, .Lfoo.__part.3_end-foo.__part.3
3231
.cfi_endproc
33-
.Lfoo_end:
34-
.size foo, .Lfoo_end-foo
3532

3633
# NB: Deliberately inserting padding to separate the two parts of the function
3734
# as we're currently only parsing a single FDE entry from a (coalesced) address
@@ -40,58 +37,64 @@ foo:
4037

4138
foo.__part.1:
4239
.cfi_startproc
43-
.cfi_def_cfa_offset 16
40+
.cfi_def_cfa_offset 32
4441
.cfi_offset %rbx, -16
4542
subq $16, %rsp
46-
.cfi_def_cfa_offset 32
43+
.cfi_def_cfa_offset 48
4744
callq bar
45+
addq $16, %rsp
46+
.cfi_def_cfa_offset 32
4847
jmp foo.__part.3
4948
.Lfoo.__part.1_end:
5049
.size foo.__part.1, .Lfoo.__part.1_end-foo.__part.1
5150
.cfi_endproc
5251

5352
bar:
5453
.cfi_startproc
55-
subq $24, %rsp
56-
.cfi_def_cfa_offset 32
54+
subq $88, %rsp
55+
.cfi_def_cfa_offset 96
5756
xorl %edi, %edi
5857
callq foo
59-
addq $24, %rsp
58+
addq $88, %rsp
6059
.cfi_def_cfa %rsp, 8
6160
retq
6261
.cfi_endproc
6362
.Lbar_end:
6463
.size bar, .Lbar_end-bar
6564

66-
foo.__part.2:
65+
.type foo,@function
66+
foo:
6767
.cfi_startproc
68+
pushq %rbx
6869
.cfi_def_cfa_offset 16
6970
.cfi_offset %rbx, -16
71+
movl %edi, %ebx
72+
cmpl $0, %ebx
73+
je foo.__part.2
7074
subq $16, %rsp
7175
.cfi_def_cfa_offset 32
72-
callq baz
73-
jmp foo.__part.3
74-
.Lfoo.__part.2_end:
75-
.size foo.__part.2, .Lfoo.__part.2_end-foo.__part.2
76+
jmp foo.__part.1
7677
.cfi_endproc
78+
.Lfoo_end:
79+
.size foo, .Lfoo_end-foo
7780

7881
# NB: Deliberately inserting padding to separate the two parts of the function
7982
# as we're currently only parsing a single FDE entry from a (coalesced) address
8083
# range.
8184
nop
8285

83-
foo.__part.3:
86+
foo.__part.2:
8487
.cfi_startproc
85-
.cfi_def_cfa_offset 32
88+
.cfi_def_cfa_offset 16
8689
.cfi_offset %rbx, -16
87-
addq $24, %rsp
88-
.cfi_def_cfa %rsp, 8
89-
retq
90-
.Lfoo.__part.3_end:
91-
.size foo.__part.3, .Lfoo.__part.3_end-foo.__part.3
90+
subq $16, %rsp
91+
.cfi_def_cfa_offset 32
92+
callq baz
93+
jmp foo.__part.3
94+
.Lfoo.__part.2_end:
95+
.size foo.__part.2, .Lfoo.__part.2_end-foo.__part.2
9296
.cfi_endproc
9397

94-
9598
.globl main
9699
.type main,@function
97100
main:

‎lldb/test/Shell/Unwind/basic-block-sections-with-dwarf-static.test‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,17 @@ image show-unwind --cached true -n foo
2222
# CHECK-NEXT: This UnwindPlan is sourced from the compiler: yes.
2323
# CHECK-NEXT: This UnwindPlan is valid at all instruction locations: no.
2424
# CHECK-NEXT: This UnwindPlan is for a trap handler function: no.
25-
# CHECK-NEXT: Address range of this UnwindPlan: [{{.*}}.text + 6-0x0000000000000010)[{{.*}}.text + 17-0x000000000000001c)[{{.*}}.text + 44-0x0000000000000037)[{{.*}}.text + 56-0x000000000000003d)
26-
# CHECK-NEXT: row[0]: 0: CFA=rsp +8 => rip=[CFA-8]
27-
# CHECK-NEXT: row[1]: 1: CFA=rsp+16 => rbx=[CFA-16] rip=[CFA-8]
28-
# CHECK-NEXT: row[2]: 11: CFA=rsp+16 => rbx=[CFA-16] rip=[CFA-8]
29-
# CHECK-NEXT: row[3]: 15: CFA=rsp+32 => rbx=[CFA-16] rip=[CFA-8]
30-
# CHECK-NEXT: row[4]: 38: CFA=rsp+16 => rbx=[CFA-16] rip=[CFA-8]
31-
# CHECK-NEXT: row[5]: 42: CFA=rsp+32 => rbx=[CFA-16] rip=[CFA-8]
32-
# CHECK-NEXT: row[6]: 50: CFA=rsp+32 => rbx=[CFA-16] rip=[CFA-8]
33-
# CHECK-NEXT: row[7]: 54: CFA=rsp +8 => rbx=[CFA-16] rip=[CFA-8]
25+
# CHECK-NEXT: Address range of this UnwindPlan: [{{.*}}.text + 6-0x000000000000000b)[{{.*}}.text + 12-0x000000000000001b)[{{.*}}.text + 43-0x0000000000000039)[{{.*}}.text + 58-0x0000000000000045)
26+
# CHECK-NEXT: row[0]: -37: CFA=rsp+32 => rbx=[CFA-16] rip=[CFA-8]
27+
# CHECK-NEXT: row[1]: -33: CFA=rsp +8 => rbx=[CFA-16] rip=[CFA-8]
28+
# CHECK-NEXT: row[2]: -31: CFA=rsp+32 => rbx=[CFA-16] rip=[CFA-8]
29+
# CHECK-NEXT: row[3]: -27: CFA=rsp+48 => rbx=[CFA-16] rip=[CFA-8]
30+
# CHECK-NEXT: row[4]: -18: CFA=rsp+32 => rbx=[CFA-16] rip=[CFA-8]
31+
# CHECK-NEXT: row[5]: 0: CFA=rsp +8 => rip=[CFA-8]
32+
# CHECK-NEXT: row[6]: 1: CFA=rsp+16 => rbx=[CFA-16] rip=[CFA-8]
33+
# CHECK-NEXT: row[7]: 12: CFA=rsp+32 => rbx=[CFA-16] rip=[CFA-8]
34+
# CHECK-NEXT: row[8]: 15: CFA=rsp+16 => rbx=[CFA-16] rip=[CFA-8]
35+
# CHECK-NEXT: row[9]: 19: CFA=rsp+32 => rbx=[CFA-16] rip=[CFA-8]
3436
# CHECK-EMPTY:
3537

3638
image show-unwind --cached true -n bar
@@ -41,8 +43,8 @@ image show-unwind --cached true -n bar
4143
# CHECK-NEXT: This UnwindPlan is sourced from the compiler: yes.
4244
# CHECK-NEXT: This UnwindPlan is valid at all instruction locations: no.
4345
# CHECK-NEXT: This UnwindPlan is for a trap handler function: no.
44-
# CHECK-NEXT: Address range of this UnwindPlan: [{{.*}}.text + 28-0x000000000000002c)
46+
# CHECK-NEXT: Address range of this UnwindPlan: [{{.*}}.text + 27-0x000000000000002b)
4547
# CHECK-NEXT: row[0]: 0: CFA=rsp +8 => rip=[CFA-8]
46-
# CHECK-NEXT: row[1]: 4: CFA=rsp+32 => rip=[CFA-8]
48+
# CHECK-NEXT: row[1]: 4: CFA=rsp+96 => rip=[CFA-8]
4749
# CHECK-NEXT: row[2]: 15: CFA=rsp +8 => rip=[CFA-8]
4850
# CHECK-EMPTY:

0 commit comments

Comments
 (0)