Skip to content

Commit 4c70157

Browse files
BPF: Use DebugLoc to find Filename for BTF line info (#90302)
Andrii found an issue where the BTF line info may have empty source which seems wrong. The program is a Meta internal bpf program. I can reproduce with latest upstream compiler as well. Let the bpf program built without this patch and then with the following veristat check where veristat is a bpf verifier tool to do kernel verification for bpf programs: $ veristat -vl2 yhs.bpf.o --log-size=150000000 >& log $ rg '^;' log | sort | uniq -c | sort -nr | head -n10 4206 ; } else if (action->dry_run) { @ src_mitigations.h:57 3907 ; if (now < start_allow_time) { @ ban.h:17 3674 ; @ src_mitigations.h:0 3223 ; if (action->vip_id != ALL_VIPS_ID && action->vip_id != vip_id) { @ src_mitigations.h:85 1737 ; pkt_info->is_dry_run_drop = action->dry_run; @ src_mitigations.h:26 1737 ; if (mitigation == ALLOW) { @ src_mitigations.h:28 1737 ; enum match_action mitigation = action->action; @ src_mitigations.h:25 1727 ; void* res = bpf_map_lookup_elem(bpf_map, key); @ filter_helpers.h:498 1691 ; bpf_map_lookup_elem(&rate_limit_config_map, rule_id); @ rate_limit.h:76 1688 ; if (throttle_cfg) { @ rate_limit.h:85 You can see 3674 ; @ src_mitigations.h:0 where we do not have proper line information and line number. In LLVM Machine IR, some instructions may carry DebugLoc information to specify where the corresponding source is for this instruction. The information includes file_name, line_num and col_num. Each instruction may also attribute to a function in debuginfo. So there are two ways to find file_name for a particular insn: (1) find the corresponding function in debuginfo (MI->getMF()->getFunction().getSubprogram()) and then find the file_name from DISubprogram. (2) find the corresponding file_name from DebugLoc. The option (1) is used in current implementation. This mostly works. But if one instruction is somehow generated from multiple functions, the compiler has to pick just one. This may cause a mismatch between file_name and line_num/col_num. Besides potential incorrect mismatch of file_name vs. line_num/col_num, There is another issue where some DebugLoc has line number 0. For example, I dumped the dwarf line table for the above bpf program: Address Line Column File ISA Discriminator OpIndex Flags ------------------ ------ ------ ------ --- ------------- ------- ------------- 0x0000000000000000 96 0 17 0 0 0 is_stmt 0x0000000000000010 100 12 17 0 0 0 is_stmt prologue_end 0x0000000000000020 0 12 17 0 0 0 0x0000000000000058 37 7 17 0 0 0 is_stmt 0x0000000000000060 0 0 17 0 0 0 0x0000000000000088 37 7 17 0 0 0 0x0000000000000090 42 75 17 0 0 0 is_stmt 0x00000000000000a8 42 52 17 0 0 0 0x00000000000000c0 120 9 17 0 0 0 is_stmt 0x00000000000000c8 0 9 17 0 0 0 0x00000000000000d0 106 21 17 0 0 0 is_stmt 0x00000000000000d8 106 3 17 0 0 0 0x00000000000000e0 110 25 17 0 0 0 is_stmt 0x00000000000000f8 110 36 17 0 0 0 0x0000000000000100 0 36 17 0 0 0 ... These DebugLoc with line number 0 needs to be skipped since we cannot map them to the correct source code. Note that selftest offset-reloc-basic.ll has this issue as well which is adjusted by this patch. With the above two fixes, empty lines for source annotation are removed. $ veristat -vl2 yhs.bpf.o --log-size=150000000 >& log $ rg '^;' log.latest | sort | uniq -c | sort -nr | head -n10 4206 ; } else if (action->dry_run) { @ src_mitigations.h:57 3907 ; if (now < start_allow_time) { @ ban.h:17 3223 ; if (action->vip_id != ALL_VIPS_ID && action->vip_id != vip_id) { @ src_mitigations.h:85 1737 ; pkt_info->is_dry_run_drop = action->dry_run; @ src_mitigations.h:26 1737 ; if (mitigation == ALLOW) { @ src_mitigations.h:28 1737 ; enum match_action mitigation = action->action; @ src_mitigations.h:25 1727 ; void* res = bpf_map_lookup_elem(bpf_map, key); @ filter_helpers.h:498 1691 ; bpf_map_lookup_elem(&rate_limit_config_map, rule_id); @ rate_limit.h:76 1688 ; if (throttle_cfg) { @ rate_limit.h:85 1670 ; if (rl_cfg) { @ rate_limit.h:77 You can see that we do not have empty line any more. 3223 ; if (action->vip_id != ALL_VIPS_ID && action->vip_id != vip_id) { @ src_mitigations.h:85 Signed-off-by: Yonghong Song <[email protected]>
1 parent b07177f commit 4c70157

File tree

3 files changed

+12
-14
lines changed

3 files changed

+12
-14
lines changed

llvm/lib/Target/BPF/BTFDebug.cpp

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -973,8 +973,7 @@ void BTFDebug::visitMapDefType(const DIType *Ty, uint32_t &TypeId) {
973973
}
974974

975975
/// Read file contents from the actual file or from the source
976-
std::string BTFDebug::populateFileContent(const DISubprogram *SP) {
977-
auto File = SP->getFile();
976+
std::string BTFDebug::populateFileContent(const DIFile *File) {
978977
std::string FileName;
979978

980979
if (!File->getFilename().starts_with("/") && File->getDirectory().size())
@@ -1005,9 +1004,9 @@ std::string BTFDebug::populateFileContent(const DISubprogram *SP) {
10051004
return FileName;
10061005
}
10071006

1008-
void BTFDebug::constructLineInfo(const DISubprogram *SP, MCSymbol *Label,
1007+
void BTFDebug::constructLineInfo(MCSymbol *Label, const DIFile *File,
10091008
uint32_t Line, uint32_t Column) {
1010-
std::string FileName = populateFileContent(SP);
1009+
std::string FileName = populateFileContent(File);
10111010
BTFLineInfo LineInfo;
10121011

10131012
LineInfo.Label = Label;
@@ -1366,18 +1365,18 @@ void BTFDebug::beginInstruction(const MachineInstr *MI) {
13661365
if (!CurMI) // no debug info
13671366
return;
13681367

1369-
// Skip this instruction if no DebugLoc or the DebugLoc
1370-
// is the same as the previous instruction.
1368+
// Skip this instruction if no DebugLoc, the DebugLoc
1369+
// is the same as the previous instruction or Line is 0.
13711370
const DebugLoc &DL = MI->getDebugLoc();
1372-
if (!DL || PrevInstLoc == DL) {
1371+
if (!DL || PrevInstLoc == DL || DL.getLine() == 0) {
13731372
// This instruction will be skipped, no LineInfo has
13741373
// been generated, construct one based on function signature.
13751374
if (LineInfoGenerated == false) {
13761375
auto *S = MI->getMF()->getFunction().getSubprogram();
13771376
if (!S)
13781377
return;
13791378
MCSymbol *FuncLabel = Asm->getFunctionBegin();
1380-
constructLineInfo(S, FuncLabel, S->getLine(), 0);
1379+
constructLineInfo(FuncLabel, S->getFile(), S->getLine(), 0);
13811380
LineInfoGenerated = true;
13821381
}
13831382

@@ -1389,8 +1388,7 @@ void BTFDebug::beginInstruction(const MachineInstr *MI) {
13891388
OS.emitLabel(LineSym);
13901389

13911390
// Construct the lineinfo.
1392-
auto SP = DL->getScope()->getSubprogram();
1393-
constructLineInfo(SP, LineSym, DL.getLine(), DL.getCol());
1391+
constructLineInfo(LineSym, DL->getFile(), DL.getLine(), DL.getCol());
13941392

13951393
LineInfoGenerated = true;
13961394
PrevInstLoc = DL;

llvm/lib/Target/BPF/BTFDebug.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -343,10 +343,10 @@ class BTFDebug : public DebugHandlerBase {
343343

344344
/// Get the file content for the subprogram. Certain lines of the file
345345
/// later may be put into string table and referenced by line info.
346-
std::string populateFileContent(const DISubprogram *SP);
346+
std::string populateFileContent(const DIFile *File);
347347

348348
/// Construct a line info.
349-
void constructLineInfo(const DISubprogram *SP, MCSymbol *Label, uint32_t Line,
349+
void constructLineInfo(MCSymbol *Label, const DIFile *File, uint32_t Line,
350350
uint32_t Column);
351351

352352
/// Generate types and variables for globals.

llvm/test/CodeGen/BPF/CORE/offset-reloc-basic.ll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,8 @@ define dso_local i32 @bpf_prog(ptr) local_unnamed_addr #0 !dbg !15 {
108108
; CHECK-NEXT: .long 0
109109
; CHECK-NEXT: .long 20
110110
; CHECK-NEXT: .long 20
111-
; CHECK-NEXT: .long 124
112-
; CHECK-NEXT: .long 144
111+
; CHECK-NEXT: .long 108
112+
; CHECK-NEXT: .long 128
113113
; CHECK-NEXT: .long 28
114114
; CHECK-NEXT: .long 8 # FuncInfo
115115

0 commit comments

Comments
 (0)