Skip to content

Commit 09369f4

Browse files
author
Yonghong Song
committed
BPF: Use DebugLoc to find Filename for BTF line info
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. This is exactly what happened in the previous example. I found bpf selftests also have some cases where file names from DISubprogram and DebugLoc are different. It looks like that finding file_name from DebugLoc is more robust since all of file_name/line_num/col_num are from the same entity. This patch used this approach. With this patch, we have: $ 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 fefac5d commit 09369f4

File tree

2 files changed

+7
-9
lines changed

2 files changed

+7
-9
lines changed

llvm/lib/Target/BPF/BTFDebug.cpp

Lines changed: 5 additions & 7 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;
@@ -1377,7 +1376,7 @@ void BTFDebug::beginInstruction(const MachineInstr *MI) {
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.

0 commit comments

Comments
 (0)