Skip to content

Commit a89e016

Browse files
labathlabath
authored
[lldb] Improve unwinding for discontinuous functions (llvm#111409)
Currently, our unwinder assumes that the functions are continuous (or at least, that there are no functions which are "in the middle" of other functions). Neither of these assumptions is true for functions optimized by tools like propeller and (probably) bolt. While there are many things that go wrong for these functions, the biggest damage is caused by the unwind plan caching code, which currently takes the maximalist extent of the function and assumes that the unwind plan we get for that is going to be valid for all code inside that range. If a part of the function has been moved into a "cold" section, then the range of the function can be many megabytes, meaning that any function within that range will probably fail to unwind. We end up with this maximalist range because the unwinder asks for the Function object for its range. This is only one of the strategies for determining the range, but it is the first one -- and also the most incorrect one. The second choice would is asking the eh_frame section for the range of the function, and this one returns something reasonable here (the address range of the current function fragment) -- which it does because each fragment gets its own eh_frame entry (it has to, because they have to be continuous). With this in mind, this patch moves the eh_frame (and debug_frame) to the front of the queue. I think that preferring this range makes sense because eh_frame is one of the unwind plans that we return, and some others (augmented eh_frame) are based on it. In theory this could break some functions, where the debug info and eh_frame disagree on the extent of the function (and eh_frame is the one who's wrong), but I don't know of any such scenarios.
1 parent b5600c6 commit a89e016

File tree

6 files changed

+379
-7
lines changed

6 files changed

+379
-7
lines changed

‎lldb/source/Commands/CommandObjectTarget.cpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3583,10 +3583,12 @@ class CommandObjectTargetModulesShowUnwind : public CommandObjectParsed {
35833583
addr_t start_addr = range.GetBaseAddress().GetLoadAddress(target);
35843584
if (abi)
35853585
start_addr = abi->FixCodeAddress(start_addr);
3586+
range.GetBaseAddress().SetLoadAddress(start_addr, target);
35863587

35873588
FuncUnwindersSP func_unwinders_sp(
35883589
sc.module_sp->GetUnwindTable()
3589-
.GetUncachedFuncUnwindersContainingAddress(start_addr, sc));
3590+
.GetUncachedFuncUnwindersContainingAddress(range.GetBaseAddress(),
3591+
sc));
35903592
if (!func_unwinders_sp)
35913593
continue;
35923594

‎lldb/source/Symbol/UnwindTable.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -99,12 +99,6 @@ UnwindTable::GetAddressRange(const Address &addr, const SymbolContext &sc) {
9999
m_object_file_unwind_up->GetAddressRange(addr, range))
100100
return range;
101101

102-
// Check the symbol context
103-
if (sc.GetAddressRange(eSymbolContextFunction | eSymbolContextSymbol, 0,
104-
false, range) &&
105-
range.GetBaseAddress().IsValid())
106-
return range;
107-
108102
// Does the eh_frame unwind info has a function bounds for this addr?
109103
if (m_eh_frame_up && m_eh_frame_up->GetAddressRange(addr, range))
110104
return range;
@@ -113,6 +107,12 @@ UnwindTable::GetAddressRange(const Address &addr, const SymbolContext &sc) {
113107
if (m_debug_frame_up && m_debug_frame_up->GetAddressRange(addr, range))
114108
return range;
115109

110+
// Check the symbol context
111+
if (sc.GetAddressRange(eSymbolContextFunction | eSymbolContextSymbol, 0,
112+
false, range) &&
113+
range.GetBaseAddress().IsValid())
114+
return range;
115+
116116
return std::nullopt;
117117
}
118118

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

Lines changed: 256 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,256 @@
1+
# An example of a function which has been split into two parts. Roughly
2+
# corresponds to this C code.
3+
# int baz() { return 47; }
4+
# int bar() { return foo(0); }
5+
# int foo(int flag) { return flag ? bar() : baz(); }
6+
# int main() { return foo(1); }
7+
# The function bar has been placed "in the middle" of foo.
8+
9+
.text
10+
11+
.type baz,@function
12+
baz:
13+
.cfi_startproc
14+
movl $47, %eax
15+
retq
16+
.cfi_endproc
17+
.Lbaz_end:
18+
.size baz, .Lbaz_end-baz
19+
20+
.type foo,@function
21+
foo:
22+
.cfi_startproc
23+
pushq %rbp
24+
.cfi_def_cfa_offset 16
25+
.cfi_offset %rbp, -16
26+
movq %rsp, %rbp
27+
.cfi_def_cfa_register %rbp
28+
subq $16, %rsp
29+
movl %edi, -8(%rbp)
30+
cmpl $0, -8(%rbp)
31+
je foo.__part.2
32+
jmp foo.__part.1
33+
.cfi_endproc
34+
.Lfoo_end:
35+
.size foo, .Lfoo_end-foo
36+
37+
foo.__part.1:
38+
.cfi_startproc
39+
.cfi_def_cfa %rbp, 16
40+
.cfi_offset %rbp, -16
41+
callq bar
42+
movl %eax, -4(%rbp)
43+
jmp foo.__part.3
44+
.Lfoo.__part.1_end:
45+
.size foo.__part.1, .Lfoo.__part.1_end-foo.__part.1
46+
.cfi_endproc
47+
48+
bar:
49+
.cfi_startproc
50+
# NB: Decrease the stack pointer to make the unwind info for this function
51+
# different from the surrounding foo function.
52+
subq $24, %rsp
53+
.cfi_def_cfa_offset 32
54+
xorl %edi, %edi
55+
callq foo
56+
addq $24, %rsp
57+
.cfi_def_cfa %rsp, 8
58+
retq
59+
.cfi_endproc
60+
.Lbar_end:
61+
.size bar, .Lbar_end-bar
62+
63+
foo.__part.2:
64+
.cfi_startproc
65+
.cfi_def_cfa %rbp, 16
66+
.cfi_offset %rbp, -16
67+
callq baz
68+
movl %eax, -4(%rbp)
69+
jmp foo.__part.3
70+
.Lfoo.__part.2_end:
71+
.size foo.__part.2, .Lfoo.__part.2_end-foo.__part.2
72+
.cfi_endproc
73+
74+
foo.__part.3:
75+
.cfi_startproc
76+
.cfi_def_cfa %rbp, 16
77+
.cfi_offset %rbp, -16
78+
movl -4(%rbp), %eax
79+
addq $16, %rsp
80+
popq %rbp
81+
.cfi_def_cfa %rsp, 8
82+
retq
83+
.Lfoo.__part.3_end:
84+
.size foo.__part.3, .Lfoo.__part.3_end-foo.__part.3
85+
.cfi_endproc
86+
87+
88+
.globl main
89+
.type main,@function
90+
main:
91+
.cfi_startproc
92+
movl $1, %edi
93+
callq foo
94+
retq
95+
.cfi_endproc
96+
.Lmain_end:
97+
.size main, .Lmain_end-main
98+
99+
.section .debug_abbrev,"",@progbits
100+
.byte 1 # Abbreviation Code
101+
.byte 17 # DW_TAG_compile_unit
102+
.byte 1 # DW_CHILDREN_yes
103+
.byte 37 # DW_AT_producer
104+
.byte 8 # DW_FORM_string
105+
.byte 19 # DW_AT_language
106+
.byte 5 # DW_FORM_data2
107+
.byte 17 # DW_AT_low_pc
108+
.byte 1 # DW_FORM_addr
109+
.byte 85 # DW_AT_ranges
110+
.byte 35 # DW_FORM_rnglistx
111+
.byte 116 # DW_AT_rnglists_base
112+
.byte 23 # DW_FORM_sec_offset
113+
.byte 0 # EOM(1)
114+
.byte 0 # EOM(2)
115+
.byte 2 # Abbreviation Code
116+
.byte 46 # DW_TAG_subprogram
117+
.byte 0 # DW_CHILDREN_no
118+
.byte 17 # DW_AT_low_pc
119+
.byte 1 # DW_FORM_addr
120+
.byte 18 # DW_AT_high_pc
121+
.byte 1 # DW_FORM_addr
122+
.byte 3 # DW_AT_name
123+
.byte 8 # DW_FORM_string
124+
.byte 0 # EOM(1)
125+
.byte 0 # EOM(2)
126+
.byte 3 # Abbreviation Code
127+
.byte 46 # DW_TAG_subprogram
128+
.byte 1 # DW_CHILDREN_yes
129+
.byte 85 # DW_AT_ranges
130+
.byte 35 # DW_FORM_rnglistx
131+
.byte 64 # DW_AT_frame_base
132+
.byte 24 # DW_FORM_exprloc
133+
.byte 3 # DW_AT_name
134+
.byte 8 # DW_FORM_string
135+
.byte 0 # EOM(1)
136+
.byte 0 # EOM(2)
137+
.byte 4 # Abbreviation Code
138+
.byte 5 # DW_TAG_formal_parameter
139+
.byte 0 # DW_CHILDREN_no
140+
.byte 2 # DW_AT_location
141+
.byte 24 # DW_FORM_exprloc
142+
.byte 3 # DW_AT_name
143+
.byte 8 # DW_FORM_string
144+
.byte 73 # DW_AT_type
145+
.byte 19 # DW_FORM_ref4
146+
.byte 0 # EOM(1)
147+
.byte 0 # EOM(2)
148+
.byte 5 # Abbreviation Code
149+
.byte 36 # DW_TAG_base_type
150+
.byte 0 # DW_CHILDREN_no
151+
.byte 3 # DW_AT_name
152+
.byte 8 # DW_FORM_string
153+
.byte 62 # DW_AT_encoding
154+
.byte 11 # DW_FORM_data1
155+
.byte 11 # DW_AT_byte_size
156+
.byte 11 # DW_FORM_data1
157+
.byte 0 # EOM(1)
158+
.byte 0 # EOM(2)
159+
.byte 0 # EOM(3)
160+
161+
.section .debug_info,"",@progbits
162+
.Lcu_begin0:
163+
.long .Ldebug_info_end0-.Ldebug_info_start0 # Length of Unit
164+
.Ldebug_info_start0:
165+
.short 5 # DWARF version number
166+
.byte 1 # DWARF Unit Type
167+
.byte 8 # Address Size (in bytes)
168+
.long .debug_abbrev # Offset Into Abbrev. Section
169+
.byte 1 # Abbrev [1] DW_TAG_compile_unit
170+
.asciz "Hand-written DWARF" # DW_AT_producer
171+
.short 29 # DW_AT_language
172+
.quad 0 # DW_AT_low_pc
173+
.byte 1 # DW_AT_ranges
174+
.long .Lrnglists_table_base0 # DW_AT_rnglists_base
175+
.byte 2 # Abbrev [2] DW_TAG_subprogram
176+
.quad baz # DW_AT_low_pc
177+
.quad .Lbaz_end # DW_AT_high_pc
178+
.asciz "baz" # DW_AT_name
179+
.byte 2 # Abbrev [2] DW_TAG_subprogram
180+
.quad bar # DW_AT_low_pc
181+
.quad .Lbar_end # DW_AT_high_pc
182+
.asciz "bar" # DW_AT_name
183+
.byte 3 # Abbrev [3] DW_TAG_subprogram
184+
.byte 0 # DW_AT_ranges
185+
.byte 1 # DW_AT_frame_base
186+
.byte 86
187+
.asciz "foo" # DW_AT_name
188+
.byte 4 # Abbrev [4] DW_TAG_formal_parameter
189+
.byte 2 # DW_AT_location
190+
.byte 145
191+
.byte 120
192+
.asciz "flag" # DW_AT_name
193+
.long .Lint-.Lcu_begin0 # DW_AT_type
194+
.byte 0 # End Of Children Mark
195+
.byte 2 # Abbrev [2] DW_TAG_subprogram
196+
.quad main # DW_AT_low_pc
197+
.quad .Lmain_end # DW_AT_high_pc
198+
.asciz "main" # DW_AT_name
199+
.Lint:
200+
.byte 5 # Abbrev [5] DW_TAG_base_type
201+
.asciz "int" # DW_AT_name
202+
.byte 5 # DW_AT_encoding
203+
.byte 4 # DW_AT_byte_size
204+
.byte 0 # End Of Children Mark
205+
.Ldebug_info_end0:
206+
207+
.section .debug_rnglists,"",@progbits
208+
.long .Ldebug_list_header_end0-.Ldebug_list_header_start0 # Length
209+
.Ldebug_list_header_start0:
210+
.short 5 # Version
211+
.byte 8 # Address size
212+
.byte 0 # Segment selector size
213+
.long 2 # Offset entry count
214+
.Lrnglists_table_base0:
215+
.long .Ldebug_ranges0-.Lrnglists_table_base0
216+
.long .Ldebug_ranges1-.Lrnglists_table_base0
217+
.Ldebug_ranges0:
218+
.byte 6 # DW_RLE_start_end
219+
.quad foo
220+
.quad .Lfoo_end
221+
.byte 6 # DW_RLE_start_end
222+
.quad foo.__part.1
223+
.quad .Lfoo.__part.1_end
224+
.byte 6 # DW_RLE_start_end
225+
.quad foo.__part.2
226+
.quad .Lfoo.__part.2_end
227+
.byte 6 # DW_RLE_start_end
228+
.quad foo.__part.3
229+
.quad .Lfoo.__part.3_end
230+
.byte 0 # DW_RLE_end_of_list
231+
.Ldebug_ranges1:
232+
.byte 6 # DW_RLE_start_end
233+
.quad baz
234+
.quad .Lbaz_end
235+
.byte 6 # DW_RLE_start_end
236+
.quad bar
237+
.quad .Lbar_end
238+
.byte 6 # DW_RLE_start_end
239+
.quad foo.__part.1
240+
.quad .Lfoo.__part.1_end
241+
.byte 6 # DW_RLE_start_end
242+
.quad foo.__part.2
243+
.quad .Lfoo.__part.2_end
244+
.byte 6 # DW_RLE_start_end
245+
.quad foo.__part.3
246+
.quad .Lfoo.__part.3_end
247+
.byte 6 # DW_RLE_start_end
248+
.quad foo
249+
.quad .Lfoo_end
250+
.byte 6 # DW_RLE_start_end
251+
.quad main
252+
.quad .Lmain_end
253+
.byte 0 # DW_RLE_end_of_list
254+
.Ldebug_list_header_end0:
255+
256+
.section ".note.GNU-stack","",@progbits

‎lldb/test/Shell/Unwind/Inputs/linux-x86_64.yaml‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
--- !minidump
2+
Streams:
3+
- Type: ThreadList
4+
Threads:
5+
- Thread Id: 0x000074DD
6+
Context: 0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000B0010000000000033000000000000000000000002020100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000040109600000000000100000000000000000000000000000068E7D0C8FF7F000068E7D0C8FF7F000097E6D0C8FF7F000010109600000000000000000000000000020000000000000088E4D0C8FF7F0000603FFF85C77F0000F00340000000000080E7D0C8FF7F000000000000000000000000000000000000E0034000000000007F0300000000000000000000000000000000000000000000801F0000FFFF00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF252525252525252525252525252525250000000000000000000000000000000000000000000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000FF00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
7+
Stack:
8+
Start of Memory Range: 0x00007FFFC8D0E000
9+
Content: DEADBEEFBAADF00D
10+
- Type: Exception
11+
Thread ID: 0x000074DD
12+
Exception Record:
13+
Exception Code: 0x0000000B
14+
Thread Context: 00000000
15+
- Type: SystemInfo
16+
Processor Arch: AMD64
17+
Processor Level: 6
18+
Processor Revision: 15876
19+
Number of Processors: 40
20+
Platform ID: Linux
21+
CSD Version: 'Linux 3.13.0-91-generic'
22+
CPU:
23+
Vendor ID: GenuineIntel
24+
Version Info: 0x00000000
25+
Feature Info: 0x00000000
26+
...

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

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
# Test unwind info for functions which have been split into two or more parts.
2+
# In particular, check that the address ranges of these plans are correct, as
3+
# overly large ranges have caused a bug in the past.
4+
5+
# REQUIRES: lld, target-x86_64
6+
7+
# RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj \
8+
# RUN: %S/Inputs/basic-block-sections-with-dwarf.s > %t.o
9+
# RUN: ld.lld %t.o -o %t
10+
## NB: This minidump exists only as a receptacle for the object file built
11+
## above. This is a workaround for the fact that "image show-unwind" does not
12+
## work without a Process object.
13+
# RUN: yaml2obj %S/Inputs/linux-x86_64.yaml > %t.core
14+
# RUN: %lldb -c %t.core %t -o "image load --file %t --slide 0" -s %s -o exit | \
15+
# RUN: FileCheck --implicit-check-not="UNWIND PLANS" %s
16+
17+
image show-unwind -n foo
18+
# CHECK: UNWIND PLANS for {{.*}}`foo
19+
#
20+
# CHECK: Asynchronous (not restricted to call-sites) UnwindPlan is 'eh_frame CFI'
21+
# CHECK-NEXT: Synchronous (restricted to call-sites) UnwindPlan is 'eh_frame CFI'
22+
23+
# CHECK: Assembly language inspection UnwindPlan:
24+
# CHECK-NEXT: This UnwindPlan originally sourced from assembly insn profiling
25+
# CHECK-NEXT: This UnwindPlan is sourced from the compiler: no.
26+
# CHECK-NEXT: This UnwindPlan is valid at all instruction locations: yes.
27+
# CHECK-NEXT: This UnwindPlan is for a trap handler function: no.
28+
# CHECK-NEXT: Address range of this UnwindPlan: [{{.*}}.text + 6-0x0000000000000019)
29+
30+
# CHECK: eh_frame UnwindPlan:
31+
# CHECK-NEXT: This UnwindPlan originally sourced from eh_frame CFI
32+
# CHECK-NEXT: This UnwindPlan is sourced from the compiler: yes.
33+
# CHECK-NEXT: This UnwindPlan is valid at all instruction locations: no.
34+
# CHECK-NEXT: This UnwindPlan is for a trap handler function: no.
35+
# CHECK-NEXT: Address range of this UnwindPlan: [{{.*}}.text + 6-0x0000000000000019)
36+
# CHECK-NEXT: row[0]: 0: CFA=rsp +8 => rip=[CFA-8]
37+
# CHECK-NEXT: row[1]: 1: CFA=rsp+16 => rbp=[CFA-16] rip=[CFA-8]
38+
# CHECK-NEXT: row[2]: 4: CFA=rbp+16 => rbp=[CFA-16] rip=[CFA-8]
39+
# CHECK-EMPTY:
40+
41+
image show-unwind -n foo.__part.1
42+
# CHECK: UNWIND PLANS for {{.*}}`foo.__part.1
43+
44+
## As of this writing (Oct 2024), the async unwind plan is "assembly insn
45+
## profiling", which isn't ideal, because this "function" does not follow the
46+
## standard ABI. We end up choosing this plan because the eh_frame unwind plan
47+
## looks like the unwind plan for a regular function without the prologue
48+
## information.
49+
# CHECK: Synchronous (restricted to call-sites) UnwindPlan is 'eh_frame CFI'
50+
51+
# CHECK: Assembly language inspection UnwindPlan:
52+
# CHECK-NEXT: This UnwindPlan originally sourced from assembly insn profiling
53+
# CHECK-NEXT: This UnwindPlan is sourced from the compiler: no.
54+
# CHECK-NEXT: This UnwindPlan is valid at all instruction locations: yes.
55+
# CHECK-NEXT: This UnwindPlan is for a trap handler function: no.
56+
# CHECK-NEXT: Address range of this UnwindPlan: [{{.*}}.text + 25-0x0000000000000023)
57+
58+
# CHECK: eh_frame UnwindPlan:
59+
# CHECK-NEXT: This UnwindPlan originally sourced from eh_frame CFI
60+
# CHECK-NEXT: This UnwindPlan is sourced from the compiler: yes.
61+
# CHECK-NEXT: This UnwindPlan is valid at all instruction locations: no.
62+
# CHECK-NEXT: This UnwindPlan is for a trap handler function: no.
63+
# CHECK-NEXT: Address range of this UnwindPlan: [{{.*}}.text + 25-0x0000000000000023)
64+
# CHECK-NEXT: row[0]: 0: CFA=rbp+16 => rbp=[CFA-16] rip=[CFA-8]
65+
# CHECK-EMPTY:

0 commit comments

Comments
 (0)