Skip to content

Commit 71af48f

Browse files
labathlabath
authored
[lldb] Fixing edge cases in "source list" (#126526)
While looking at how to make Function::GetEndLineSourceInfo (which is only used in "command source") work with discontinuous functions, I realized there are other corner cases that this function doesn't handle. The code assumed that the last line entry in the function will also correspond to the last source line. This is probably true for unoptimized code, but I don't think we can rely on the optimizer to preserve this property. What's worse, the code didn't check that the last line entry belonged to the same file as the first one, so if this line entry was the result of inlining, we could end up using a line from a completely different file. To fix this, I change the algorithm to iterate over all line entries in the function (which belong to the same file) and find the max line number out of those. This way we can naturally handle the discontinuous case as well. This implementation is going to be slower than the previous one, but I don't think that matters, because: - this command is only used rarely, and interactively - we have plenty of other code which iterates through the line table I added some basic tests for the function operation. I don't claim the tests to be comprehensive, or that the function handles all edge cases, but test framework created here could be used for testing other fixes/edge cases as well.
1 parent 8616c87 commit 71af48f

File tree

4 files changed

+310
-32
lines changed

4 files changed

+310
-32
lines changed

‎lldb/include/lldb/Symbol/Function.h

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "lldb/Expression/DWARFExpressionList.h"
1616
#include "lldb/Symbol/Block.h"
1717
#include "lldb/Utility/UserID.h"
18+
#include "lldb/lldb-forward.h"
1819
#include "llvm/ADT/ArrayRef.h"
1920

2021
#include <mutex>
@@ -460,6 +461,7 @@ class Function : public UserID, public SymbolContextScope {
460461
}
461462

462463
lldb::LanguageType GetLanguage() const;
464+
463465
/// Find the file and line number of the source location of the start of the
464466
/// function. This will use the declaration if present and fall back on the
465467
/// line table if that fails. So there may NOT be a line table entry for
@@ -473,16 +475,9 @@ class Function : public UserID, public SymbolContextScope {
473475
void GetStartLineSourceInfo(lldb::SupportFileSP &source_file_sp,
474476
uint32_t &line_no);
475477

476-
/// Find the file and line number of the source location of the end of the
477-
/// function.
478-
///
479-
///
480-
/// \param[out] source_file
481-
/// The source file.
482-
///
483-
/// \param[out] line_no
484-
/// The line number.
485-
void GetEndLineSourceInfo(FileSpec &source_file, uint32_t &line_no);
478+
using SourceRange = Range<uint32_t, uint32_t>;
479+
/// Find the file and line number range of the function.
480+
llvm::Expected<std::pair<lldb::SupportFileSP, SourceRange>> GetSourceInfo();
486481

487482
/// Get the outgoing call edges from this function, sorted by their return
488483
/// PC addresses (in increasing order).

‎lldb/source/Commands/CommandObjectSource.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -784,14 +784,14 @@ class CommandObjectSourceList : public CommandObjectParsed {
784784

785785
if (sc.block == nullptr) {
786786
// Not an inlined function
787-
sc.function->GetStartLineSourceInfo(start_file, start_line);
788-
if (start_line == 0) {
789-
result.AppendErrorWithFormat("Could not find line information for "
790-
"start of function: \"%s\".\n",
791-
source_info.function.GetCString());
787+
auto expected_info = sc.function->GetSourceInfo();
788+
if (!expected_info) {
789+
result.AppendError(llvm::toString(expected_info.takeError()));
792790
return 0;
793791
}
794-
sc.function->GetEndLineSourceInfo(end_file, end_line);
792+
start_file = expected_info->first;
793+
start_line = expected_info->second.GetRangeBase();
794+
end_line = expected_info->second.GetRangeEnd();
795795
} else {
796796
// We have an inlined function
797797
start_file = source_info.line_entry.file_sp;

‎lldb/source/Symbol/Function.cpp

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -320,25 +320,31 @@ void Function::GetStartLineSourceInfo(SupportFileSP &source_file_sp,
320320
}
321321
}
322322

323-
void Function::GetEndLineSourceInfo(FileSpec &source_file, uint32_t &line_no) {
324-
line_no = 0;
325-
source_file.Clear();
326-
327-
// The -1 is kind of cheesy, but I want to get the last line entry for the
328-
// given function, not the first entry of the next.
329-
Address scratch_addr(GetAddressRange().GetBaseAddress());
330-
scratch_addr.SetOffset(scratch_addr.GetOffset() +
331-
GetAddressRange().GetByteSize() - 1);
332-
323+
llvm::Expected<std::pair<SupportFileSP, Function::SourceRange>>
324+
Function::GetSourceInfo() {
325+
SupportFileSP source_file_sp;
326+
uint32_t start_line;
327+
GetStartLineSourceInfo(source_file_sp, start_line);
333328
LineTable *line_table = m_comp_unit->GetLineTable();
334-
if (line_table == nullptr)
335-
return;
329+
if (start_line == 0 || !line_table) {
330+
return llvm::createStringError(llvm::formatv(
331+
"Could not find line information for function \"{0}\".", GetName()));
332+
}
336333

337-
LineEntry line_entry;
338-
if (line_table->FindLineEntryByAddress(scratch_addr, line_entry, nullptr)) {
339-
line_no = line_entry.line;
340-
source_file = line_entry.GetFile();
334+
uint32_t end_line = start_line;
335+
for (const AddressRange &range : GetAddressRanges()) {
336+
for (auto [idx, end] = line_table->GetLineEntryIndexRange(range); idx < end;
337+
++idx) {
338+
LineEntry entry;
339+
// Ignore entries belonging to inlined functions or #included files.
340+
if (line_table->GetLineEntryAtIndex(idx, entry) &&
341+
source_file_sp->Equal(*entry.file_sp,
342+
SupportFile::eEqualFileSpecAndChecksumIfSet))
343+
end_line = std::max(end_line, entry.line);
344+
}
341345
}
346+
return std::make_pair(std::move(source_file_sp),
347+
SourceRange(start_line, end_line - start_line));
342348
}
343349

344350
llvm::ArrayRef<std::unique_ptr<CallEdge>> Function::GetCallEdges() {

‎lldb/test/Shell/Commands/command-source-list.s

Lines changed: 277 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,277 @@
1+
# REQUIRES: x86
2+
3+
# RUN: split-file %s %t
4+
# RUN: llvm-mc --triple=x86_64-pc-linux -filetype=obj %t/a.s -o %t/a.o
5+
# RUN: %lldb %t/a.o -o "settings set target.source-map . %t" \
6+
# RUN: -o "settings set interpreter.stop-command-source-on-error false" \
7+
# RUN: -s %t/commands -o exit 2>&1 | FileCheck %s
8+
9+
#--- commands
10+
# CASE 0: function at the start of the file
11+
source list -n func0
12+
# CHECK-LABEL: source list -n func0
13+
# CHECK-NEXT: File: file0.c
14+
# CHECK-NEXT: 1 content of file0.c:1
15+
# CHECK-NEXT: 2 content of file0.c:2
16+
# CHECK-NEXT: 3 content of file0.c:3
17+
# CHECK-NEXT: 4 content of file0.c:4
18+
# CHECK-NEXT: 5 content of file0.c:5
19+
# CHECK-NEXT: 6 content of file0.c:6
20+
# CHECK-NEXT: 7 content of file0.c:7
21+
# CHECK-NEXT: 8 content of file0.c:8
22+
# CHECK-NEXT: 9 content of file0.c:9
23+
# CHECK-NEXT: 10 content of file0.c:10
24+
25+
# CASE 1: function in the middle of the file
26+
source list -n func1
27+
# CHECK-NEXT: source list -n func1
28+
# CHECK-NEXT: File: file0.c
29+
# CHECK-NEXT: 5 content of file0.c:5
30+
# CHECK-NEXT: 6 content of file0.c:6
31+
# CHECK-NEXT: 7 content of file0.c:7
32+
# CHECK-NEXT: 8 content of file0.c:8
33+
# CHECK-NEXT: 9 content of file0.c:9
34+
# CHECK-NEXT: 10 content of file0.c:10
35+
# CHECK-NEXT: 11 content of file0.c:11
36+
# CHECK-NEXT: 12 content of file0.c:12
37+
# CHECK-NEXT: 13 content of file0.c:13
38+
# CHECK-NEXT: 14 content of file0.c:14
39+
# CHECK-NEXT: 15 content of file0.c:15
40+
# CHECK-NEXT: 16 content of file0.c:16
41+
# CHECK-NEXT: 17 content of file0.c:17
42+
43+
# CASE 2: function at the end of the file
44+
source list -n func2
45+
# CHECK-NEXT: source list -n func2
46+
# CHECK-NEXT: File: file0.c
47+
# CHECK-NEXT: 20 content of file0.c:20
48+
# CHECK-NEXT: 21 content of file0.c:21
49+
# CHECK-NEXT: 22 content of file0.c:22
50+
# CHECK-NEXT: 23 content of file0.c:23
51+
# CHECK-NEXT: 24 content of file0.c:24
52+
# CHECK-NEXT: 25 content of file0.c:25
53+
# CHECK-NEXT: 26 content of file0.c:26
54+
# CHECK-NEXT: 27 content of file0.c:27
55+
# CHECK-NEXT: 28 content of file0.c:28
56+
# CHECK-NEXT: 29 content of file0.c:29
57+
# CHECK-NEXT: 30 content of file0.c:30
58+
59+
# CASE 3: function ends in a different file
60+
source list -n func3
61+
# CHECK-NEXT: source list -n func3
62+
# CHECK-NEXT: File: file0.c
63+
# CHECK-NEXT: 1 content of file0.c:1
64+
# CHECK-NEXT: 2 content of file0.c:2
65+
# CHECK-NEXT: 3 content of file0.c:3
66+
# CHECK-NEXT: 4 content of file0.c:4
67+
# CHECK-NEXT: 5 content of file0.c:5
68+
# CHECK-NEXT: 6 content of file0.c:6
69+
# CHECK-NEXT: 7 content of file0.c:7
70+
# CHECK-NEXT: 8 content of file0.c:8
71+
# CHECK-NEXT: 9 content of file0.c:9
72+
# CHECK-NEXT: 10 content of file0.c:10
73+
74+
# CASE 4: function has no line entry with line!=0
75+
source list -n func4
76+
# CHECK-LABEL: source list -n func4
77+
# CHECK: error: Could not find line information for function "func4".
78+
79+
# CASE 5: discontinuous function
80+
source list -n func5
81+
# CHECK-LABEL: source list -n func5
82+
# CHECK-NEXT: File: file0.c
83+
# CHECK-NEXT: 1 content of file0.c:1
84+
# CHECK-NEXT: 2 content of file0.c:2
85+
# CHECK-NEXT: 3 content of file0.c:3
86+
# CHECK-NEXT: 4 content of file0.c:4
87+
# CHECK-NEXT: 5 content of file0.c:5
88+
# CHECK-NEXT: 6 content of file0.c:6
89+
# CHECK-NEXT: 7 content of file0.c:7
90+
# CHECK-NEXT: 8 content of file0.c:8
91+
# CHECK-NEXT: 9 content of file0.c:9
92+
# CHECK-NEXT: 10 content of file0.c:10
93+
94+
95+
#--- a.s
96+
.file 0 "." "file0.c"
97+
.file 1 "." "file1.c"
98+
.text
99+
func0:
100+
.loc 0 1
101+
nop
102+
.loc 0 5
103+
nop
104+
.Lfunc0_end:
105+
106+
func1:
107+
.loc 0 10
108+
nop
109+
.loc 0 12
110+
nop
111+
.Lfunc1_end:
112+
113+
func2:
114+
.loc 0 25
115+
nop
116+
.loc 0 30
117+
nop
118+
.Lfunc2_end:
119+
120+
func3:
121+
.loc 0 1
122+
nop
123+
.loc 0 5
124+
nop
125+
.loc 1 5
126+
nop
127+
.Lfunc3_end:
128+
129+
func4:
130+
.loc 0 0
131+
nop
132+
.Lfunc4_end:
133+
134+
func5.__part.1:
135+
.loc 0 1
136+
nop
137+
.Lfunc5.__part.1_end:
138+
139+
.Lpadding:
140+
nop
141+
142+
func5:
143+
.loc 0 5
144+
nop
145+
.Lfunc5_end:
146+
147+
.Ltext_end:
148+
149+
.section .debug_abbrev,"",@progbits
150+
.byte 1 # Abbreviation Code
151+
.byte 17 # DW_TAG_compile_unit
152+
.byte 1 # DW_CHILDREN_yes
153+
.byte 3 # DW_AT_name
154+
.byte 8 # DW_FORM_string
155+
.byte 37 # DW_AT_producer
156+
.byte 8 # DW_FORM_string
157+
.byte 19 # DW_AT_language
158+
.byte 5 # DW_FORM_data2
159+
.byte 17 # DW_AT_low_pc
160+
.byte 1 # DW_FORM_addr
161+
.byte 18 # DW_AT_high_pc
162+
.byte 1 # DW_FORM_addr
163+
.byte 16 # DW_AT_stmt_list
164+
.byte 23 # DW_FORM_sec_offset
165+
.byte 0 # EOM(1)
166+
.byte 0 # EOM(2)
167+
.byte 2 # Abbreviation Code
168+
.byte 46 # DW_TAG_subprogram
169+
.byte 0 # DW_CHILDREN_no
170+
.byte 17 # DW_AT_low_pc
171+
.byte 1 # DW_FORM_addr
172+
.byte 18 # DW_AT_high_pc
173+
.byte 1 # DW_FORM_addr
174+
.byte 3 # DW_AT_name
175+
.byte 8 # DW_FORM_string
176+
.byte 0 # EOM(1)
177+
.byte 0 # EOM(2)
178+
.byte 3 # Abbreviation Code
179+
.byte 46 # DW_TAG_subprogram
180+
.byte 0 # DW_CHILDREN_no
181+
.byte 85 # DW_AT_ranges
182+
.byte 23 # DW_FORM_sec_offset
183+
.byte 3 # DW_AT_name
184+
.byte 8 # DW_FORM_string
185+
.byte 0 # EOM(1)
186+
.byte 0 # EOM(2)
187+
.byte 0 # EOM(3)
188+
.section .debug_info,"",@progbits
189+
.Lcu_begin0:
190+
.long .Ldebug_info_end0-.Ldebug_info_start0 # Length of Unit
191+
.Ldebug_info_start0:
192+
.short 5 # DWARF version number
193+
.byte 1 # DWARF Unit Type
194+
.byte 8 # Address Size (in bytes)
195+
.long .debug_abbrev # Offset Into Abbrev. Section
196+
.byte 1 # Abbrev DW_TAG_compile_unit
197+
.asciz "file0.c" # DW_AT_producer
198+
.asciz "Hand-written DWARF" # DW_AT_producer
199+
.short 29 # DW_AT_language
200+
.quad .text # DW_AT_low_pc
201+
.quad .Ltext_end # DW_AT_high_pc
202+
.long .Lline_table_start0 # DW_AT_stmt_list
203+
.rept 5
204+
.byte 2 # Abbrev DW_TAG_subprogram
205+
.quad func\+ # DW_AT_low_pc
206+
.quad .Lfunc\+_end # DW_AT_high_pc
207+
.asciz "func\+" # DW_AT_name
208+
.endr
209+
.byte 3 # Abbrev DW_TAG_subprogram
210+
.long .Ldebug_ranges0
211+
.asciz "func5" # DW_AT_name
212+
.byte 0 # End Of Children Mark
213+
.Ldebug_info_end0:
214+
215+
.section .debug_rnglists,"",@progbits
216+
.long .Ldebug_list_header_end0-.Ldebug_list_header_start0 # Length
217+
.Ldebug_list_header_start0:
218+
.short 5 # Version
219+
.byte 8 # Address size
220+
.byte 0 # Segment selector size
221+
.long 2 # Offset entry count
222+
.Lrnglists_table_base0:
223+
.long .Ldebug_ranges0-.Lrnglists_table_base0
224+
.Ldebug_ranges0:
225+
.byte 6 # DW_RLE_start_end
226+
.quad func5
227+
.quad .Lfunc5_end
228+
.byte 6 # DW_RLE_start_end
229+
.quad func5.__part.1
230+
.quad .Lfunc5.__part.1_end
231+
.byte 0 # DW_RLE_end_of_list
232+
.Ldebug_list_header_end0:
233+
.section .debug_line,"",@progbits
234+
.Lline_table_start0:
235+
236+
#--- file0.c
237+
content of file0.c:1
238+
content of file0.c:2
239+
content of file0.c:3
240+
content of file0.c:4
241+
content of file0.c:5
242+
content of file0.c:6
243+
content of file0.c:7
244+
content of file0.c:8
245+
content of file0.c:9
246+
content of file0.c:10
247+
content of file0.c:11
248+
content of file0.c:12
249+
content of file0.c:13
250+
content of file0.c:14
251+
content of file0.c:15
252+
content of file0.c:16
253+
content of file0.c:17
254+
content of file0.c:18
255+
content of file0.c:19
256+
content of file0.c:20
257+
content of file0.c:21
258+
content of file0.c:22
259+
content of file0.c:23
260+
content of file0.c:24
261+
content of file0.c:25
262+
content of file0.c:26
263+
content of file0.c:27
264+
content of file0.c:28
265+
content of file0.c:29
266+
content of file0.c:30
267+
#--- file1.c
268+
content of file1.c:1
269+
content of file1.c:2
270+
content of file1.c:3
271+
content of file1.c:4
272+
content of file1.c:5
273+
content of file1.c:6
274+
content of file1.c:7
275+
content of file1.c:8
276+
content of file1.c:9
277+
content of file1.c:10

0 commit comments

Comments
 (0)