Attach BTF to generated BPF programs #2804

viktormalik · 2023-10-17T07:56:27Z

Modern BPF programs loaded through libbpf should include BTF information describing their structure and data. This is required for some BPF features such as subprograms or kernel functions (kfuncs) to work. It is also a necessary prerequisite for #2334.

This PR is the first step for doing this in bpftrace. We produce standard (DWARF) debug info for our probes which LLVM then converts into proper BTF sections. These are then passed to libbpf upon bpf_prog_load. At the moment, this doesn't add any value but we at least check that libbpf doesn't reject our produced BTF info.

This is the second part of #2710 posted separately to simplify reviewing.

Checklist

Language changes are updated in man/adoc/bpftrace.adoc and if needed in docs/reference_guide.md
User-visible and non-trivial changes updated in CHANGELOG.md
The new behaviour is covered by tests

danobi

seems reasonable. I'm not very familiar with llvm debuginfo interface though

src/utils.cpp

src/ast/dibuilderbpf.h

viktormalik · 2023-11-06T11:50:05Z

@ajor do you want to review this, too?

ajor

Looks good, mainly just some nits. I'll do some testing myself to check out the createFile thing.

ajor · 2023-11-13T14:15:35Z

src/ast/dibuilderbpf.cpp

+
+DIBuilderBPF::DIBuilderBPF(Module &module) : DIBuilder(module)
+{
+  file = createFile("bpftrace", ".");


I'm a bit unsure about this line but I'm not familiar enough with createFile to be certain.

Does this actually create a file? Does "." mean current working directory? Can this result in permission errors if bpftrace is run from a non-writable directory?

Will do a bit of testing myself.

I don't think it creates an actual file. I quickly skimmed the code of createFile and IIUC, it should just create a debug info entry which is required to produce correct debug info for a function. We just need to provide a directory and a file name strings to create it, so we give the current directory and ”bpftrace” for the file name. I think that the strings themselves don't mean anything, we could provide anything.

I ran bpftrace through strace and can confirm it doesn't create a file at this location. If we print debug output ("-d") then it does attempt to read this file though:

openat(AT_FDCWD, "./bpftrace", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)

I can make bpftrace read arbitrary data by manually creating a file at the expected location. It's not necessarily a problem but feels a bit dodgy to me.

How about we do createFile("", "")? This seems to work and ensures that there can be no real file associated with our DIFile object.

I ran bpftrace through strace and can confirm it doesn't create a file at this location. If we print debug output ("-d") then it does attempt to read this file though:

openat(AT_FDCWD, "./bpftrace", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)

Interesting, it's probably trying to open the file when dumping LLVM IR, I've no idea why.

How about we do createFile("", "")? This seems to work and ensures that there can be no real file associated with our DIFile object.

Sure, makes sense to me.

So, unfortunately, we must pass a valid filename, otherwise LLVM IR verification (which we run in codegen tests) complains. I reverted it to the original version.

I ran a few more tests and discovered that LLVM dump tries to open the file and either read it (if it's a text file) or mmap it to memory (if it's an ELF). While this shouldn't be harmful, it's certainly an unnecessary operation.

Using ./bpftrace is therefore not ideal as it will exist when running ./bpftrace -e ... and will do an unnecessary mmap of the bpftrace binary. So, I changed it to bpftrace.bpf.o which is less likely to exist.

src/utils.h

src/utils.cpp

ajor · 2023-11-13T15:37:14Z

src/ast/passes/codegen_llvm.cpp

+                           0,
+                           StringRef(),
+                           DICompileUnit::DebugEmissionKind::LineTablesOnly);
+  module_->addModuleFlag(llvm::Module::Warning,


Any idea what this is for?

This is required by DWARF.

src/ast/dibuilderbpf.cpp

src/attached_probe.cpp

In order for bpftrace to adapt to libbpf style loading of programs and to support subprograms, it first needs to produce BTF information for BPF programs. In order for it to produce BTF, it first needs to produce any debugging info, so that LLVM can convert it into BTF/BTF.ext sections. This commit emits subprogram entries for probe definitions. This does not use any line info but should be sufficient to create func infos down the road. Since we're going to generate more and more debug info in future, introduce a dedicated class DIBuilderBPF for encapsulating related logic and methods. DIBuilderBPF inherits from llvm::DIBuilder, the relationship is analogous to the one between IRBuilderBPF and llvm::IRBuilder.

BTF is not used for any purpose at the moment but it will be used in future and this way we can at least check that libbpf accepts our produced BTF.

viktormalik · 2023-11-20T11:14:58Z

I rebased to the current master to run tests with the CI libbpf. I'll wait 24 hours and then merge this, unless there are objections.

viktormalik requested review from ajor, danobi and fbs as code owners October 17, 2023 07:56

viktormalik force-pushed the emit-btf branch from 63e98a4 to cb63af4 Compare October 17, 2023 07:57

danobi reviewed Oct 27, 2023

View reviewed changes

src/utils.cpp Outdated Show resolved Hide resolved

src/utils.cpp Show resolved Hide resolved

src/ast/dibuilderbpf.h Show resolved Hide resolved

viktormalik force-pushed the emit-btf branch 2 times, most recently from 45c290f to 000ff11 Compare October 30, 2023 10:16

danobi approved these changes Oct 30, 2023

View reviewed changes

ajor reviewed Nov 13, 2023

View reviewed changes

viktormalik force-pushed the emit-btf branch 2 times, most recently from 01ee3be to daab7f9 Compare November 14, 2023 12:50

ajor approved these changes Nov 15, 2023

View reviewed changes

viktormalik force-pushed the emit-btf branch 4 times, most recently from 82fff69 to 9aa23a6 Compare November 20, 2023 10:46

Delyan Kratunov and others added 2 commits November 20, 2023 12:09

Pass BTF info on program load

1dc0c60

BTF is not used for any purpose at the moment but it will be used in future and this way we can at least check that libbpf accepts our produced BTF.

viktormalik force-pushed the emit-btf branch from 9aa23a6 to 1dc0c60 Compare November 20, 2023 11:09

danobi approved these changes Nov 20, 2023

View reviewed changes

viktormalik merged commit 280042e into bpftrace:master Nov 21, 2023
18 checks passed

viktormalik mentioned this pull request Dec 11, 2023

Prepare to use subprograms (emit BTF) #2710

Closed

3 tasks

viktormalik deleted the emit-btf branch January 10, 2024 09:21

giorio94 mentioned this pull request Feb 20, 2024

bpftrace v0.20.1, Linux 4.19 - ERROR: Error loading program #3011

Closed

bernhardkaindl mentioned this pull request May 4, 2024

Fix bpftrace for older kernels: Fallback to no func_infos, no BTF #3149

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Attach BTF to generated BPF programs #2804

Attach BTF to generated BPF programs #2804

viktormalik commented Oct 17, 2023 •

edited

Loading

danobi left a comment

viktormalik commented Nov 6, 2023

ajor left a comment

ajor Nov 13, 2023

viktormalik Nov 13, 2023

ajor Nov 14, 2023 •

edited

Loading

viktormalik Nov 14, 2023

viktormalik Nov 16, 2023

viktormalik Nov 16, 2023

viktormalik Nov 20, 2023

ajor Nov 13, 2023

viktormalik Nov 13, 2023

viktormalik commented Nov 20, 2023

Attach BTF to generated BPF programs #2804

Attach BTF to generated BPF programs #2804

Conversation

viktormalik commented Oct 17, 2023 • edited Loading

Checklist

danobi left a comment

Choose a reason for hiding this comment

viktormalik commented Nov 6, 2023

ajor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ajor Nov 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

viktormalik commented Nov 20, 2023

viktormalik commented Oct 17, 2023 •

edited

Loading

ajor Nov 14, 2023 •

edited

Loading