* [PATCH bpf-next v3 0/6] resolve_btfids: Support for BTF modifications
@ 2025-12-05 22:30 Ihor Solodrai
2025-12-05 22:30 ` [PATCH bpf-next v3 1/6] resolve_btfids: Rename object btf field to btf_path Ihor Solodrai
` (4 more replies)
0 siblings, 5 replies; 22+ messages in thread
From: Ihor Solodrai @ 2025-12-05 22:30 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Andrew Morton, Nathan Chancellor, Nicolas Schier, Tejun Heo,
David Vernet, Andrea Righi, Changwoo Min, Shuah Khan,
Nick Desaulniers, Bill Wendling, Justin Stitt, Alan Maguire,
Donglin Peng
Cc: bpf, dwarves, linux-kernel, linux-kbuild
This series changes resolve_btfids and kernel build scripts to enable
BTF transformations in resolve_btfids. Main motivation for enhancing
resolve_btfids is to reduce dependency of the kernel build on pahole
capabilities [1] and enable BTF features and optimizations [2][3]
particular to the kernel.
Patches #1-#3 in the series are non-functional refactoring in
resolve_btfids.
Patch #4 changes minimum version of pahole required for
CONFIG_DEBUG_INFO_BTF to v1.22
Patch #5 makes a small prep change in selftests/bpf build.
The last patch (#6) makes significant changes in resolve_btfids and
introduces scripts/gen-btf.sh. See implementation details in the patch
description.
Successful CI run: https://github.com/kernel-patches/bpf/actions/runs/19976024062?pr=10438
[1] https://lore.kernel.org/dwarves/ba1650aa-fafd-49a8-bea4-bdddee7c38c9@linux.dev/
[2] https://lore.kernel.org/bpf/20251029190113.3323406-1-ihor.solodrai@linux.dev/
[3] https://lore.kernel.org/bpf/20251119031531.1817099-1-dolinux.peng@gmail.com/
---
v2->v3:
- add patch #4 bumping minimum pahole version (Andrii, Alan)
- add patch #5 pre-fixing resolve_btfids test (Donglin)
- add GEN_BTF var and assemble RESOLVE_BTFIDS_FLAGS in Makefile.btf (Alan)
- implement --distill_base flag in resolve_btfids, set it depending
on KBUILD_EXTMOD in Makefile.btf (Eduard)
- various implementation nits, see the v2 thread for details (Andrii, Eduard)
v2: https://lore.kernel.org/bpf/20251127185242.3954132-1-ihor.solodrai@linux.dev/
v1->v2:
- gen-btf.sh and other shell script fixes (Donglin)
- update selftests build (Donglin)
- generate .BTF.base only when KBUILD_EXTMOD is set (Alan)
- proper endianness handling for cross-compilation
- change elf_begin mode from ELF_C_RDWR_MMAP to ELF_C_READ_MMAP_PRIVATE
- remove compressed_section_fix()
- nit NULL check in patch #3 (suggested by AI)
v1: https://lore.kernel.org/bpf/20251126012656.3546071-1-ihor.solodrai@linux.dev/
Ihor Solodrai (6):
resolve_btfids: Rename object btf field to btf_path
resolve_btfids: Factor out load_btf()
resolve_btfids: Introduce enum btf_id_kind
lib/Kconfig.debug: Set the minimum required pahole version to v1.22
selftests/bpf: Run resolve_btfids only for relevant .test.o objects
resolve_btfids: change in-place update with raw binary output
MAINTAINERS | 1 +
lib/Kconfig.debug | 13 +-
scripts/Makefile.btf | 26 +-
scripts/Makefile.modfinal | 5 +-
scripts/Makefile.vmlinux | 2 +-
scripts/gen-btf.sh | 157 ++++++++
scripts/link-vmlinux.sh | 46 +--
tools/bpf/resolve_btfids/main.c | 355 ++++++++++++------
tools/sched_ext/README.md | 1 -
tools/testing/selftests/bpf/.gitignore | 3 +
tools/testing/selftests/bpf/Makefile | 11 +-
.../selftests/bpf/prog_tests/resolve_btfids.c | 4 +-
12 files changed, 434 insertions(+), 190 deletions(-)
create mode 100755 scripts/gen-btf.sh
--
2.52.0
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH bpf-next v3 1/6] resolve_btfids: Rename object btf field to btf_path 2025-12-05 22:30 [PATCH bpf-next v3 0/6] resolve_btfids: Support for BTF modifications Ihor Solodrai @ 2025-12-05 22:30 ` Ihor Solodrai 2025-12-12 7:10 ` Eduard Zingerman 2025-12-05 22:30 ` [PATCH bpf-next v3 2/6] resolve_btfids: Factor out load_btf() Ihor Solodrai ` (3 subsequent siblings) 4 siblings, 1 reply; 22+ messages in thread From: Ihor Solodrai @ 2025-12-05 22:30 UTC (permalink / raw) To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Andrew Morton, Nathan Chancellor, Nicolas Schier, Tejun Heo, David Vernet, Andrea Righi, Changwoo Min, Shuah Khan, Nick Desaulniers, Bill Wendling, Justin Stitt, Alan Maguire, Donglin Peng Cc: bpf, dwarves, linux-kernel, linux-kbuild Rename the member of `struct object` holding the path to BTF data if provided via --btf arg. `btf_path` is less ambiguous. Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev> --- tools/bpf/resolve_btfids/main.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c index d47191c6e55e..164f0c941f04 100644 --- a/tools/bpf/resolve_btfids/main.c +++ b/tools/bpf/resolve_btfids/main.c @@ -113,7 +113,7 @@ struct btf_id { struct object { const char *path; - const char *btf; + const char *btf_path; const char *base_btf_path; struct { @@ -550,11 +550,11 @@ static int symbols_resolve(struct object *obj) } } - btf = btf__parse_split(obj->btf ?: obj->path, base_btf); + btf = btf__parse_split(obj->btf_path ?: obj->path, base_btf); err = libbpf_get_error(btf); if (err) { pr_err("FAILED: load BTF from %s: %s\n", - obj->btf ?: obj->path, strerror(-err)); + obj->btf_path ?: obj->path, strerror(-err)); goto out; } @@ -790,8 +790,8 @@ int main(int argc, const char **argv) struct option btfid_options[] = { OPT_INCR('v', "verbose", &verbose, "be more verbose (show errors, etc)"), - OPT_STRING(0, "btf", &obj.btf, "BTF data", - "BTF data"), + OPT_STRING(0, "btf", &obj.btf_path, "file", + "path to a file with input BTF data"), OPT_STRING('b', "btf_base", &obj.base_btf_path, "file", "path of file providing base BTF"), OPT_BOOLEAN(0, "fatal_warnings", &fatal_warnings, -- 2.52.0 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH bpf-next v3 1/6] resolve_btfids: Rename object btf field to btf_path 2025-12-05 22:30 ` [PATCH bpf-next v3 1/6] resolve_btfids: Rename object btf field to btf_path Ihor Solodrai @ 2025-12-12 7:10 ` Eduard Zingerman 0 siblings, 0 replies; 22+ messages in thread From: Eduard Zingerman @ 2025-12-12 7:10 UTC (permalink / raw) To: Ihor Solodrai, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Andrew Morton, Nathan Chancellor, Nicolas Schier, Tejun Heo, David Vernet, Andrea Righi, Changwoo Min, Shuah Khan, Nick Desaulniers, Bill Wendling, Justin Stitt, Alan Maguire, Donglin Peng Cc: bpf, dwarves, linux-kernel, linux-kbuild On Fri, 2025-12-05 at 14:30 -0800, Ihor Solodrai wrote: > Rename the member of `struct object` holding the path to BTF data if > provided via --btf arg. `btf_path` is less ambiguous. > > Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev> > --- Acked-by: Eduard Zingerman <eddyz87@gmail.com> ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH bpf-next v3 2/6] resolve_btfids: Factor out load_btf() 2025-12-05 22:30 [PATCH bpf-next v3 0/6] resolve_btfids: Support for BTF modifications Ihor Solodrai 2025-12-05 22:30 ` [PATCH bpf-next v3 1/6] resolve_btfids: Rename object btf field to btf_path Ihor Solodrai @ 2025-12-05 22:30 ` Ihor Solodrai 2025-12-05 22:57 ` bot+bpf-ci 2025-12-12 7:10 ` Eduard Zingerman 2025-12-05 22:30 ` [PATCH bpf-next v3 3/6] resolve_btfids: Introduce enum btf_id_kind Ihor Solodrai ` (2 subsequent siblings) 4 siblings, 2 replies; 22+ messages in thread From: Ihor Solodrai @ 2025-12-05 22:30 UTC (permalink / raw) To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Andrew Morton, Nathan Chancellor, Nicolas Schier, Tejun Heo, David Vernet, Andrea Righi, Changwoo Min, Shuah Khan, Nick Desaulniers, Bill Wendling, Justin Stitt, Alan Maguire, Donglin Peng Cc: bpf, dwarves, linux-kernel, linux-kbuild Increase the lifetime of parsed BTF in resolve_btfids by factoring load_btf() routine out of symbols_resolve() and storing the base_btf and btf pointers in the struct object. Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev> --- tools/bpf/resolve_btfids/main.c | 47 ++++++++++++++++++++++++--------- 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c index 164f0c941f04..b4caae1170dd 100644 --- a/tools/bpf/resolve_btfids/main.c +++ b/tools/bpf/resolve_btfids/main.c @@ -116,6 +116,9 @@ struct object { const char *btf_path; const char *base_btf_path; + struct btf *btf; + struct btf *base_btf; + struct { int fd; Elf *elf; @@ -529,16 +532,10 @@ static int symbols_collect(struct object *obj) return 0; } -static int symbols_resolve(struct object *obj) +static int load_btf(struct object *obj) { - int nr_typedefs = obj->nr_typedefs; - int nr_structs = obj->nr_structs; - int nr_unions = obj->nr_unions; - int nr_funcs = obj->nr_funcs; - struct btf *base_btf = NULL; - int err, type_id; - struct btf *btf; - __u32 nr_types; + struct btf *base_btf = NULL, *btf = NULL; + int err; if (obj->base_btf_path) { base_btf = btf__parse(obj->base_btf_path, NULL); @@ -546,7 +543,7 @@ static int symbols_resolve(struct object *obj) if (err) { pr_err("FAILED: load base BTF from %s: %s\n", obj->base_btf_path, strerror(-err)); - return -1; + goto out_err; } } @@ -555,9 +552,30 @@ static int symbols_resolve(struct object *obj) if (err) { pr_err("FAILED: load BTF from %s: %s\n", obj->btf_path ?: obj->path, strerror(-err)); - goto out; + goto out_err; } + obj->base_btf = base_btf; + obj->btf = btf; + + return 0; + +out_err: + btf__free(base_btf); + btf__free(btf); + return err; +} + +static int symbols_resolve(struct object *obj) +{ + int nr_typedefs = obj->nr_typedefs; + int nr_structs = obj->nr_structs; + int nr_unions = obj->nr_unions; + int nr_funcs = obj->nr_funcs; + struct btf *btf = obj->btf; + int err, type_id; + __u32 nr_types; + err = -1; nr_types = btf__type_cnt(btf); @@ -615,8 +633,6 @@ static int symbols_resolve(struct object *obj) err = 0; out: - btf__free(base_btf); - btf__free(btf); return err; } @@ -824,6 +840,9 @@ int main(int argc, const char **argv) if (symbols_collect(&obj)) goto out; + if (load_btf(&obj)) + goto out; + if (symbols_resolve(&obj)) goto out; @@ -833,6 +852,8 @@ int main(int argc, const char **argv) if (!(fatal_warnings && warnings)) err = 0; out: + btf__free(obj.base_btf); + btf__free(obj.btf); if (obj.efile.elf) { elf_end(obj.efile.elf); close(obj.efile.fd); -- 2.52.0 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH bpf-next v3 2/6] resolve_btfids: Factor out load_btf() 2025-12-05 22:30 ` [PATCH bpf-next v3 2/6] resolve_btfids: Factor out load_btf() Ihor Solodrai @ 2025-12-05 22:57 ` bot+bpf-ci 2025-12-05 23:12 ` Ihor Solodrai 2025-12-12 7:10 ` Eduard Zingerman 1 sibling, 1 reply; 22+ messages in thread From: bot+bpf-ci @ 2025-12-05 22:57 UTC (permalink / raw) To: ihor.solodrai, ast, daniel, andrii, martin.lau, eddyz87, song, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, akpm, nathan, nsc, tj, void, arighi, changwoo, shuah, nick.desaulniers+lkml, morbo, justinstitt, alan.maguire, dolinux.peng Cc: bpf, dwarves, linux-kernel, linux-kbuild, ast, andrii, daniel, martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai [-- Attachment #1: Type: text/plain, Size: 2660 bytes --] > diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c > index 164f0c941f04..b4caae1170dd 100644 > --- a/tools/bpf/resolve_btfids/main.c > +++ b/tools/bpf/resolve_btfids/main.c [ ... ] > @@ -529,16 +532,10 @@ static int symbols_collect(struct object *obj) > return 0; > } > > -static int symbols_resolve(struct object *obj) > +static int load_btf(struct object *obj) > { > - int nr_typedefs = obj->nr_typedefs; > - int nr_structs = obj->nr_structs; > - int nr_unions = obj->nr_unions; > - int nr_funcs = obj->nr_funcs; > - struct btf *base_btf = NULL; > - int err, type_id; > - struct btf *btf; > - __u32 nr_types; > + struct btf *base_btf = NULL, *btf = NULL; > + int err; > > if (obj->base_btf_path) { > base_btf = btf__parse(obj->base_btf_path, NULL); > err = libbpf_get_error(base_btf); > if (err) { > pr_err("FAILED: load base BTF from %s: %s\n", > obj->base_btf_path, strerror(-err)); > - return -1; > + goto out_err; > } > } > > btf = btf__parse_split(obj->btf_path ?: obj->path, base_btf); > err = libbpf_get_error(btf); > if (err) { > pr_err("FAILED: load BTF from %s: %s\n", > obj->btf_path ?: obj->path, strerror(-err)); > - goto out; > + goto out_err; > } > > + obj->base_btf = base_btf; > + obj->btf = btf; > + > + return 0; > + > +out_err: > + btf__free(base_btf); > + btf__free(btf); > + return err; Can btf__free() safely handle ERR_PTR values here? When btf__parse() fails on line 541, it returns ERR_PTR(-err), and libbpf_get_error() extracts the error code, but base_btf still holds the ERR_PTR value. The error path then calls btf__free(base_btf) where base_btf is ERR_PTR rather than NULL or a valid pointer. The same issue occurs if btf__parse_split() fails on line 550, where btf contains ERR_PTR when btf__free(btf) is called. In the old code at symbols_resolve(), if base_btf was ERR_PTR, the function would return -1 immediately without calling btf__free(), avoiding this issue. The new error handling with goto out_err changes this behavior. > +} > + > +static int symbols_resolve(struct object *obj) > +{ > + int nr_typedefs = obj->nr_typedefs; > + int nr_structs = obj->nr_structs; > + int nr_unions = obj->nr_unions; > + int nr_funcs = obj->nr_funcs; > + struct btf *btf = obj->btf; > + int err, type_id; > + __u32 nr_types; > + > err = -1; > nr_types = btf__type_cnt(btf); [ ... ] --- AI reviewed your patch. Please fix the bug or email reply why it's not a bug. See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md CI run summary: https://github.com/kernel-patches/bpf/actions/runs/19978081551 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH bpf-next v3 2/6] resolve_btfids: Factor out load_btf() 2025-12-05 22:57 ` bot+bpf-ci @ 2025-12-05 23:12 ` Ihor Solodrai 2025-12-05 23:18 ` Chris Mason 0 siblings, 1 reply; 22+ messages in thread From: Ihor Solodrai @ 2025-12-05 23:12 UTC (permalink / raw) To: bot+bpf-ci, ast, daniel, andrii, martin.lau, eddyz87, song, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, akpm, nathan, nsc, tj, void, arighi, changwoo, shuah, nick.desaulniers+lkml, morbo, justinstitt, alan.maguire, dolinux.peng Cc: bpf, dwarves, linux-kernel, linux-kbuild, martin.lau, clm On 12/5/25 2:57 PM, bot+bpf-ci@kernel.org wrote: >> diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c >> index 164f0c941f04..b4caae1170dd 100644 >> --- a/tools/bpf/resolve_btfids/main.c >> +++ b/tools/bpf/resolve_btfids/main.c > > [ ... ] > >> @@ -529,16 +532,10 @@ static int symbols_collect(struct object *obj) >> return 0; >> } >> >> -static int symbols_resolve(struct object *obj) >> +static int load_btf(struct object *obj) >> { >> - int nr_typedefs = obj->nr_typedefs; >> - int nr_structs = obj->nr_structs; >> - int nr_unions = obj->nr_unions; >> - int nr_funcs = obj->nr_funcs; >> - struct btf *base_btf = NULL; >> - int err, type_id; >> - struct btf *btf; >> - __u32 nr_types; >> + struct btf *base_btf = NULL, *btf = NULL; >> + int err; >> >> if (obj->base_btf_path) { >> base_btf = btf__parse(obj->base_btf_path, NULL); >> err = libbpf_get_error(base_btf); >> if (err) { >> pr_err("FAILED: load base BTF from %s: %s\n", >> obj->base_btf_path, strerror(-err)); >> - return -1; >> + goto out_err; >> } >> } >> >> btf = btf__parse_split(obj->btf_path ?: obj->path, base_btf); >> err = libbpf_get_error(btf); >> if (err) { >> pr_err("FAILED: load BTF from %s: %s\n", >> obj->btf_path ?: obj->path, strerror(-err)); >> - goto out; >> + goto out_err; >> } >> >> + obj->base_btf = base_btf; >> + obj->btf = btf; >> + >> + return 0; >> + >> +out_err: >> + btf__free(base_btf); >> + btf__free(btf); >> + return err; > > Can btf__free() safely handle ERR_PTR values here? > > When btf__parse() fails on line 541, it returns ERR_PTR(-err), and > libbpf_get_error() extracts the error code, but base_btf still holds > the ERR_PTR value. The error path then calls btf__free(base_btf) > where base_btf is ERR_PTR rather than NULL or a valid pointer. > > The same issue occurs if btf__parse_split() fails on line 550, where > btf contains ERR_PTR when btf__free(btf) is called. > > In the old code at symbols_resolve(), if base_btf was ERR_PTR, the > function would return -1 immediately without calling btf__free(), > avoiding this issue. The new error handling with goto out_err changes > this behavior. That's a good observation. Interesting that AI noticed this only on third try, probably due to changes in prompts. btf__free() checks for error values [1], so this is not an issue. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/lib/bpf/btf.c?h=v6.18#n968 > >> +} >> + >> +static int symbols_resolve(struct object *obj) >> +{ >> + int nr_typedefs = obj->nr_typedefs; >> + int nr_structs = obj->nr_structs; >> + int nr_unions = obj->nr_unions; >> + int nr_funcs = obj->nr_funcs; >> + struct btf *btf = obj->btf; >> + int err, type_id; >> + __u32 nr_types; >> + >> err = -1; >> nr_types = btf__type_cnt(btf); > > [ ... ] > > > --- > AI reviewed your patch. Please fix the bug or email reply why it's not a bug. > See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md > > CI run summary: https://github.com/kernel-patches/bpf/actions/runs/19978081551 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH bpf-next v3 2/6] resolve_btfids: Factor out load_btf() 2025-12-05 23:12 ` Ihor Solodrai @ 2025-12-05 23:18 ` Chris Mason 0 siblings, 0 replies; 22+ messages in thread From: Chris Mason @ 2025-12-05 23:18 UTC (permalink / raw) To: Ihor Solodrai, bot+bpf-ci, ast, daniel, andrii, martin.lau, eddyz87, song, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, akpm, nathan, nsc, tj, void, arighi, changwoo, shuah, nick.desaulniers+lkml, morbo, justinstitt, alan.maguire, dolinux.peng Cc: bpf, dwarves, linux-kernel, linux-kbuild, martin.lau On 12/5/25 6:12 PM, Ihor Solodrai wrote: > On 12/5/25 2:57 PM, bot+bpf-ci@kernel.org wrote: >>> diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c >>> index 164f0c941f04..b4caae1170dd 100644 >>> --- a/tools/bpf/resolve_btfids/main.c >>> +++ b/tools/bpf/resolve_btfids/main.c >> >> [ ... ] >> >>> @@ -529,16 +532,10 @@ static int symbols_collect(struct object *obj) >>> return 0; >>> } >>> >>> -static int symbols_resolve(struct object *obj) >>> +static int load_btf(struct object *obj) >>> { >>> - int nr_typedefs = obj->nr_typedefs; >>> - int nr_structs = obj->nr_structs; >>> - int nr_unions = obj->nr_unions; >>> - int nr_funcs = obj->nr_funcs; >>> - struct btf *base_btf = NULL; >>> - int err, type_id; >>> - struct btf *btf; >>> - __u32 nr_types; >>> + struct btf *base_btf = NULL, *btf = NULL; >>> + int err; >>> >>> if (obj->base_btf_path) { >>> base_btf = btf__parse(obj->base_btf_path, NULL); >>> err = libbpf_get_error(base_btf); >>> if (err) { >>> pr_err("FAILED: load base BTF from %s: %s\n", >>> obj->base_btf_path, strerror(-err)); >>> - return -1; >>> + goto out_err; >>> } >>> } >>> >>> btf = btf__parse_split(obj->btf_path ?: obj->path, base_btf); >>> err = libbpf_get_error(btf); >>> if (err) { >>> pr_err("FAILED: load BTF from %s: %s\n", >>> obj->btf_path ?: obj->path, strerror(-err)); >>> - goto out; >>> + goto out_err; >>> } >>> >>> + obj->base_btf = base_btf; >>> + obj->btf = btf; >>> + >>> + return 0; >>> + >>> +out_err: >>> + btf__free(base_btf); >>> + btf__free(btf); >>> + return err; >> >> Can btf__free() safely handle ERR_PTR values here? >> >> When btf__parse() fails on line 541, it returns ERR_PTR(-err), and >> libbpf_get_error() extracts the error code, but base_btf still holds >> the ERR_PTR value. The error path then calls btf__free(base_btf) >> where base_btf is ERR_PTR rather than NULL or a valid pointer. >> >> The same issue occurs if btf__parse_split() fails on line 550, where >> btf contains ERR_PTR when btf__free(btf) is called. >> >> In the old code at symbols_resolve(), if base_btf was ERR_PTR, the >> function would return -1 immediately without calling btf__free(), >> avoiding this issue. The new error handling with goto out_err changes >> this behavior. > > That's a good observation. Interesting that AI noticed this only on > third try, probably due to changes in prompts. > > btf__free() checks for error values [1], so this is not an issue. > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/lib/bpf/btf.c?h=v6.18#n968 > Hmm, it should have read btf__free() to answer this question on its own. I'll check a look. -chris ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH bpf-next v3 2/6] resolve_btfids: Factor out load_btf() 2025-12-05 22:30 ` [PATCH bpf-next v3 2/6] resolve_btfids: Factor out load_btf() Ihor Solodrai 2025-12-05 22:57 ` bot+bpf-ci @ 2025-12-12 7:10 ` Eduard Zingerman 1 sibling, 0 replies; 22+ messages in thread From: Eduard Zingerman @ 2025-12-12 7:10 UTC (permalink / raw) To: Ihor Solodrai, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Andrew Morton, Nathan Chancellor, Nicolas Schier, Tejun Heo, David Vernet, Andrea Righi, Changwoo Min, Shuah Khan, Nick Desaulniers, Bill Wendling, Justin Stitt, Alan Maguire, Donglin Peng Cc: bpf, dwarves, linux-kernel, linux-kbuild On Fri, 2025-12-05 at 14:30 -0800, Ihor Solodrai wrote: > Increase the lifetime of parsed BTF in resolve_btfids by factoring > load_btf() routine out of symbols_resolve() and storing the base_btf > and btf pointers in the struct object. > > Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev> > --- Acked-by: Eduard Zingerman <eddyz87@gmail.com> ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH bpf-next v3 3/6] resolve_btfids: Introduce enum btf_id_kind 2025-12-05 22:30 [PATCH bpf-next v3 0/6] resolve_btfids: Support for BTF modifications Ihor Solodrai 2025-12-05 22:30 ` [PATCH bpf-next v3 1/6] resolve_btfids: Rename object btf field to btf_path Ihor Solodrai 2025-12-05 22:30 ` [PATCH bpf-next v3 2/6] resolve_btfids: Factor out load_btf() Ihor Solodrai @ 2025-12-05 22:30 ` Ihor Solodrai 2025-12-12 7:09 ` Eduard Zingerman 2025-12-05 22:30 ` [PATCH bpf-next v3 4/6] lib/Kconfig.debug: Set the minimum required pahole version to v1.22 Ihor Solodrai 2025-12-05 22:30 ` [PATCH bpf-next v3 5/6] selftests/bpf: Run resolve_btfids only for relevant .test.o objects Ihor Solodrai 4 siblings, 1 reply; 22+ messages in thread From: Ihor Solodrai @ 2025-12-05 22:30 UTC (permalink / raw) To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Andrew Morton, Nathan Chancellor, Nicolas Schier, Tejun Heo, David Vernet, Andrea Righi, Changwoo Min, Shuah Khan, Nick Desaulniers, Bill Wendling, Justin Stitt, Alan Maguire, Donglin Peng Cc: bpf, dwarves, linux-kernel, linux-kbuild Instead of using multiple flags, make struct btf_id tagged with an enum value indicating its kind in the context of resolve_btfids. Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev> --- tools/bpf/resolve_btfids/main.c | 76 ++++++++++++++++++++++----------- 1 file changed, 52 insertions(+), 24 deletions(-) diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c index b4caae1170dd..cb1e69eb0bd7 100644 --- a/tools/bpf/resolve_btfids/main.c +++ b/tools/bpf/resolve_btfids/main.c @@ -98,6 +98,13 @@ # error "Unknown machine endianness!" #endif +enum btf_id_kind { + BTF_ID_KIND_NONE, + BTF_ID_KIND_SYM, + BTF_ID_KIND_SET, + BTF_ID_KIND_SET8 +}; + struct btf_id { struct rb_node rb_node; char *name; @@ -105,9 +112,8 @@ struct btf_id { int id; int cnt; }; + enum btf_id_kind kind; int addr_cnt; - bool is_set; - bool is_set8; Elf64_Addr addr[ADDR_CNT]; }; @@ -197,8 +203,7 @@ static struct btf_id *btf_id__find(struct rb_root *root, const char *name) return NULL; } -static struct btf_id * -btf_id__add(struct rb_root *root, char *name, bool unique) +static struct btf_id *btf_id__add(struct rb_root *root, char *name, enum btf_id_kind kind) { struct rb_node **p = &root->rb_node; struct rb_node *parent = NULL; @@ -213,14 +218,19 @@ btf_id__add(struct rb_root *root, char *name, bool unique) p = &(*p)->rb_left; else if (cmp > 0) p = &(*p)->rb_right; - else - return unique ? NULL : id; + else if (kind == BTF_ID_KIND_SYM && id->kind == BTF_ID_KIND_SYM) + return id; + else { + pr_err("Unexpected duplicate symbol %s of kind %d\n", name, id->kind); + return NULL; + } } id = zalloc(sizeof(*id)); if (id) { pr_debug("adding symbol %s\n", name); id->name = name; + id->kind = kind; rb_link_node(&id->rb_node, parent, p); rb_insert_color(&id->rb_node, root); } @@ -260,22 +270,36 @@ static char *get_id(const char *prefix_end) return id; } -static struct btf_id *add_set(struct object *obj, char *name, bool is_set8) +static struct btf_id *add_set(struct object *obj, char *name, enum btf_id_kind kind) { + int len = strlen(name); + int prefixlen; + char *id; + /* * __BTF_ID__set__name * name = ^ * id = ^ */ - char *id = name + (is_set8 ? sizeof(BTF_SET8 "__") : sizeof(BTF_SET "__")) - 1; - int len = strlen(name); + switch (kind) { + case BTF_ID_KIND_SET: + prefixlen = sizeof(BTF_SET "__") - 1; + break; + case BTF_ID_KIND_SET8: + prefixlen = sizeof(BTF_SET8 "__") - 1; + break; + default: + pr_err("Unexpected kind %d passed to %s() for symbol %s\n", kind, __func__, name); + return NULL; + } + id = name + prefixlen - 1; if (id >= name + len) { pr_err("FAILED to parse set name: %s\n", name); return NULL; } - return btf_id__add(&obj->sets, id, true); + return btf_id__add(&obj->sets, id, kind); } static struct btf_id *add_symbol(struct rb_root *root, char *name, size_t size) @@ -288,7 +312,7 @@ static struct btf_id *add_symbol(struct rb_root *root, char *name, size_t size) return NULL; } - return btf_id__add(root, id, false); + return btf_id__add(root, id, BTF_ID_KIND_SYM); } /* Older libelf.h and glibc elf.h might not yet define the ELF compression types. */ @@ -491,28 +515,24 @@ static int symbols_collect(struct object *obj) id = add_symbol(&obj->funcs, prefix, sizeof(BTF_FUNC) - 1); /* set8 */ } else if (!strncmp(prefix, BTF_SET8, sizeof(BTF_SET8) - 1)) { - id = add_set(obj, prefix, true); + id = add_set(obj, prefix, BTF_ID_KIND_SET8); /* * SET8 objects store list's count, which is encoded * in symbol's size, together with 'cnt' field hence * that - 1. */ - if (id) { + if (id) id->cnt = sym.st_size / sizeof(uint64_t) - 1; - id->is_set8 = true; - } /* set */ } else if (!strncmp(prefix, BTF_SET, sizeof(BTF_SET) - 1)) { - id = add_set(obj, prefix, false); + id = add_set(obj, prefix, BTF_ID_KIND_SET); /* * SET objects store list's count, which is encoded * in symbol's size, together with 'cnt' field hence * that - 1. */ - if (id) { + if (id) id->cnt = sym.st_size / sizeof(int) - 1; - id->is_set = true; - } } else { pr_err("FAILED unsupported prefix %s\n", prefix); return -1; @@ -643,7 +663,7 @@ static int id_patch(struct object *obj, struct btf_id *id) int i; /* For set, set8, id->id may be 0 */ - if (!id->id && !id->is_set && !id->is_set8) { + if (!id->id && id->kind != BTF_ID_KIND_SET && id->kind != BTF_ID_KIND_SET8) { pr_err("WARN: resolve_btfids: unresolved symbol %s\n", id->name); warnings++; } @@ -696,6 +716,7 @@ static int sets_patch(struct object *obj) { Elf_Data *data = obj->efile.idlist; struct rb_node *next; + int cnt; next = rb_first(&obj->sets); while (next) { @@ -715,11 +736,15 @@ static int sets_patch(struct object *obj) return -1; } - if (id->is_set) { + switch (id->kind) { + case BTF_ID_KIND_SET: set = data->d_buf + off; + cnt = set->cnt; qsort(set->ids, set->cnt, sizeof(set->ids[0]), cmp_id); - } else { + break; + case BTF_ID_KIND_SET8: set8 = data->d_buf + off; + cnt = set8->cnt; /* * Make sure id is at the beginning of the pairs * struct, otherwise the below qsort would not work. @@ -744,10 +769,13 @@ static int sets_patch(struct object *obj) bswap_32(set8->pairs[i].flags); } } + break; + default: + pr_err("Unexpected btf_id_kind %d for set '%s'\n", id->kind, id->name); + return -1; } - pr_debug("sorting addr %5lu: cnt %6d [%s]\n", - off, id->is_set ? set->cnt : set8->cnt, id->name); + pr_debug("sorting addr %5lu: cnt %6d [%s]\n", off, cnt, id->name); next = rb_next(next); } -- 2.52.0 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH bpf-next v3 3/6] resolve_btfids: Introduce enum btf_id_kind 2025-12-05 22:30 ` [PATCH bpf-next v3 3/6] resolve_btfids: Introduce enum btf_id_kind Ihor Solodrai @ 2025-12-12 7:09 ` Eduard Zingerman 0 siblings, 0 replies; 22+ messages in thread From: Eduard Zingerman @ 2025-12-12 7:09 UTC (permalink / raw) To: Ihor Solodrai, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Andrew Morton, Nathan Chancellor, Nicolas Schier, Tejun Heo, David Vernet, Andrea Righi, Changwoo Min, Shuah Khan, Nick Desaulniers, Bill Wendling, Justin Stitt, Alan Maguire, Donglin Peng Cc: bpf, dwarves, linux-kernel, linux-kbuild On Fri, 2025-12-05 at 14:30 -0800, Ihor Solodrai wrote: > Instead of using multiple flags, make struct btf_id tagged with an > enum value indicating its kind in the context of resolve_btfids. > > Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev> > --- Acked-by: Eduard Zingerman <eddyz87@gmail.com> (But see a question below). > @@ -213,14 +218,19 @@ btf_id__add(struct rb_root *root, char *name, bool unique) > p = &(*p)->rb_left; > else if (cmp > 0) > p = &(*p)->rb_right; > - else > - return unique ? NULL : id; > + else if (kind == BTF_ID_KIND_SYM && id->kind == BTF_ID_KIND_SYM) Nit: I'd keep the 'unique' parameter alongside 'kind' and resolve this condition on the function callsite. > + return id; > + else { > + pr_err("Unexpected duplicate symbol %s of kind %d\n", name, id->kind); > + return NULL; > + } [...] > @@ -491,28 +515,24 @@ static int symbols_collect(struct object *obj) > id = add_symbol(&obj->funcs, prefix, sizeof(BTF_FUNC) - 1); > /* set8 */ > } else if (!strncmp(prefix, BTF_SET8, sizeof(BTF_SET8) - 1)) { > - id = add_set(obj, prefix, true); > + id = add_set(obj, prefix, BTF_ID_KIND_SET8); > /* > * SET8 objects store list's count, which is encoded > * in symbol's size, together with 'cnt' field hence > * that - 1. > */ > - if (id) { > + if (id) > id->cnt = sym.st_size / sizeof(uint64_t) - 1; > - id->is_set8 = true; > - } > /* set */ > } else if (!strncmp(prefix, BTF_SET, sizeof(BTF_SET) - 1)) { > - id = add_set(obj, prefix, false); > + id = add_set(obj, prefix, BTF_ID_KIND_SET); > /* > * SET objects store list's count, which is encoded > * in symbol's size, together with 'cnt' field hence > * that - 1. > */ > - if (id) { > + if (id) Current patch is not a culprit, but shouldn't resolve_btfids fail if `id` cannot be added? (here and in a hunk above). > id->cnt = sym.st_size / sizeof(int) - 1; > - id->is_set = true; > - } > } else { > pr_err("FAILED unsupported prefix %s\n", prefix); > return -1; [...] ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH bpf-next v3 4/6] lib/Kconfig.debug: Set the minimum required pahole version to v1.22 2025-12-05 22:30 [PATCH bpf-next v3 0/6] resolve_btfids: Support for BTF modifications Ihor Solodrai ` (2 preceding siblings ...) 2025-12-05 22:30 ` [PATCH bpf-next v3 3/6] resolve_btfids: Introduce enum btf_id_kind Ihor Solodrai @ 2025-12-05 22:30 ` Ihor Solodrai 2025-12-05 22:49 ` bot+bpf-ci ` (3 more replies) 2025-12-05 22:30 ` [PATCH bpf-next v3 5/6] selftests/bpf: Run resolve_btfids only for relevant .test.o objects Ihor Solodrai 4 siblings, 4 replies; 22+ messages in thread From: Ihor Solodrai @ 2025-12-05 22:30 UTC (permalink / raw) To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Andrew Morton, Nathan Chancellor, Nicolas Schier, Tejun Heo, David Vernet, Andrea Righi, Changwoo Min, Shuah Khan, Nick Desaulniers, Bill Wendling, Justin Stitt, Alan Maguire, Donglin Peng Cc: bpf, dwarves, linux-kernel, linux-kbuild Subsequent patches in the series change vmlinux linking scripts to unconditionally pass --btf_encode_detached to pahole, which was introduced in v1.22 [1][2]. This change allows to remove PAHOLE_HAS_SPLIT_BTF Kconfig option and other checks of older pahole versions. [1] https://github.com/acmel/dwarves/releases/tag/v1.22 [2] https://lore.kernel.org/bpf/cbafbf4e-9073-4383-8ee6-1353f9e5869c@oracle.com/ Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev> --- lib/Kconfig.debug | 13 ++++--------- scripts/Makefile.btf | 9 +-------- tools/sched_ext/README.md | 1 - 3 files changed, 5 insertions(+), 18 deletions(-) diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 742b23ef0d8b..3abf3ae554b6 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -389,18 +389,13 @@ config DEBUG_INFO_BTF depends on !DEBUG_INFO_SPLIT && !DEBUG_INFO_REDUCED depends on !GCC_PLUGIN_RANDSTRUCT || COMPILE_TEST depends on BPF_SYSCALL - depends on PAHOLE_VERSION >= 116 - depends on DEBUG_INFO_DWARF4 || PAHOLE_VERSION >= 121 + depends on PAHOLE_VERSION >= 122 # pahole uses elfutils, which does not have support for Hexagon relocations depends on !HEXAGON help Generate deduplicated BTF type information from DWARF debug info. - Turning this on requires pahole v1.16 or later (v1.21 or later to - support DWARF 5), which will convert DWARF type info into equivalent - deduplicated BTF type info. - -config PAHOLE_HAS_SPLIT_BTF - def_bool PAHOLE_VERSION >= 119 + Turning this on requires pahole v1.22 or later, which will convert + DWARF type info into equivalent deduplicated BTF type info. config PAHOLE_HAS_BTF_TAG def_bool PAHOLE_VERSION >= 123 @@ -422,7 +417,7 @@ config PAHOLE_HAS_LANG_EXCLUDE config DEBUG_INFO_BTF_MODULES bool "Generate BTF type information for kernel modules" default y - depends on DEBUG_INFO_BTF && MODULES && PAHOLE_HAS_SPLIT_BTF + depends on DEBUG_INFO_BTF && MODULES help Generate compact split BTF type information for kernel modules. diff --git a/scripts/Makefile.btf b/scripts/Makefile.btf index db76335dd917..7c1cd6c2ff75 100644 --- a/scripts/Makefile.btf +++ b/scripts/Makefile.btf @@ -7,14 +7,7 @@ JOBS := $(patsubst -j%,%,$(filter -j%,$(MAKEFLAGS))) ifeq ($(call test-le, $(pahole-ver), 125),y) -# pahole 1.18 through 1.21 can't handle zero-sized per-CPU vars -ifeq ($(call test-le, $(pahole-ver), 121),y) -pahole-flags-$(call test-ge, $(pahole-ver), 118) += --skip_encoding_btf_vars -endif - -pahole-flags-$(call test-ge, $(pahole-ver), 121) += --btf_gen_floats - -pahole-flags-$(call test-ge, $(pahole-ver), 122) += -j$(JOBS) +pahole-flags-$(call test-ge, $(pahole-ver), 122) += --btf_gen_floats -j$(JOBS) pahole-flags-$(call test-ge, $(pahole-ver), 125) += --skip_encoding_btf_inconsistent_proto --btf_gen_optimized diff --git a/tools/sched_ext/README.md b/tools/sched_ext/README.md index 16a42e4060f6..56a9d1557ac4 100644 --- a/tools/sched_ext/README.md +++ b/tools/sched_ext/README.md @@ -65,7 +65,6 @@ It's also recommended that you also include the following Kconfig options: ``` CONFIG_BPF_JIT_ALWAYS_ON=y CONFIG_BPF_JIT_DEFAULT_ON=y -CONFIG_PAHOLE_HAS_SPLIT_BTF=y CONFIG_PAHOLE_HAS_BTF_TAG=y ``` -- 2.52.0 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH bpf-next v3 4/6] lib/Kconfig.debug: Set the minimum required pahole version to v1.22 2025-12-05 22:30 ` [PATCH bpf-next v3 4/6] lib/Kconfig.debug: Set the minimum required pahole version to v1.22 Ihor Solodrai @ 2025-12-05 22:49 ` bot+bpf-ci 2025-12-05 22:51 ` Ihor Solodrai 2025-12-06 0:32 ` Andrii Nakryiko ` (2 subsequent siblings) 3 siblings, 1 reply; 22+ messages in thread From: bot+bpf-ci @ 2025-12-05 22:49 UTC (permalink / raw) To: ihor.solodrai, ast, daniel, andrii, martin.lau, eddyz87, song, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, akpm, nathan, nsc, tj, void, arighi, changwoo, shuah, nick.desaulniers+lkml, morbo, justinstitt, alan.maguire, dolinux.peng Cc: bpf, dwarves, linux-kernel, linux-kbuild, ast, andrii, daniel, martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai [-- Attachment #1: Type: text/plain, Size: 1200 bytes --] > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > index 742b23ef0d8b..3abf3ae554b6 100644 > --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug [ ... ] > -config PAHOLE_HAS_SPLIT_BTF > - def_bool PAHOLE_VERSION >= 119 The PAHOLE_HAS_SPLIT_BTF config option was removed here. [ ... ] > diff --git a/tools/sched_ext/README.md b/tools/sched_ext/README.md > index 16a42e4060f6..56a9d1557ac4 100644 > --- a/tools/sched_ext/README.md > +++ b/tools/sched_ext/README.md > @@ -65,7 +65,6 @@ It's also recommended that you also include the following Kconfig options: > ``` > CONFIG_BPF_JIT_ALWAYS_ON=y > CONFIG_BPF_JIT_DEFAULT_ON=y > -CONFIG_PAHOLE_HAS_SPLIT_BTF=y > CONFIG_PAHOLE_HAS_BTF_TAG=y > ``` The PAHOLE_HAS_SPLIT_BTF reference was removed from tools/sched_ext/README.md, but Documentation/scheduler/sched-ext.rst still contains this config option at line 46. Should that documentation file also be updated to remove this obsolete config option? --- AI reviewed your patch. Please fix the bug or email reply why it's not a bug. See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md CI run summary: https://github.com/kernel-patches/bpf/actions/runs/19978081551 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH bpf-next v3 4/6] lib/Kconfig.debug: Set the minimum required pahole version to v1.22 2025-12-05 22:49 ` bot+bpf-ci @ 2025-12-05 22:51 ` Ihor Solodrai 0 siblings, 0 replies; 22+ messages in thread From: Ihor Solodrai @ 2025-12-05 22:51 UTC (permalink / raw) To: bot+bpf-ci, ast, daniel, andrii, martin.lau, eddyz87, song, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, akpm, nathan, nsc, tj, void, arighi, changwoo, shuah, nick.desaulniers+lkml, morbo, justinstitt, alan.maguire, dolinux.peng Cc: bpf, dwarves, linux-kernel, linux-kbuild, martin.lau, clm On 12/5/25 2:49 PM, bot+bpf-ci@kernel.org wrote: >> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug >> index 742b23ef0d8b..3abf3ae554b6 100644 >> --- a/lib/Kconfig.debug >> +++ b/lib/Kconfig.debug > > [ ... ] > >> -config PAHOLE_HAS_SPLIT_BTF >> - def_bool PAHOLE_VERSION >= 119 > > The PAHOLE_HAS_SPLIT_BTF config option was removed here. > > [ ... ] > >> diff --git a/tools/sched_ext/README.md b/tools/sched_ext/README.md >> index 16a42e4060f6..56a9d1557ac4 100644 >> --- a/tools/sched_ext/README.md >> +++ b/tools/sched_ext/README.md >> @@ -65,7 +65,6 @@ It's also recommended that you also include the following Kconfig options: >> ``` >> CONFIG_BPF_JIT_ALWAYS_ON=y >> CONFIG_BPF_JIT_DEFAULT_ON=y >> -CONFIG_PAHOLE_HAS_SPLIT_BTF=y >> CONFIG_PAHOLE_HAS_BTF_TAG=y >> ``` > > The PAHOLE_HAS_SPLIT_BTF reference was removed from > tools/sched_ext/README.md, but Documentation/scheduler/sched-ext.rst > still contains this config option at line 46. Should that documentation > file also be updated to remove this obsolete config option? Yes. I thought I removed it, but the change got lost in translation. > > > --- > AI reviewed your patch. Please fix the bug or email reply why it's not a bug. > See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md > > CI run summary: https://github.com/kernel-patches/bpf/actions/runs/19978081551 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH bpf-next v3 4/6] lib/Kconfig.debug: Set the minimum required pahole version to v1.22 2025-12-05 22:30 ` [PATCH bpf-next v3 4/6] lib/Kconfig.debug: Set the minimum required pahole version to v1.22 Ihor Solodrai 2025-12-05 22:49 ` bot+bpf-ci @ 2025-12-06 0:32 ` Andrii Nakryiko 2025-12-06 0:58 ` Ihor Solodrai 2025-12-12 7:09 ` Eduard Zingerman 2025-12-12 17:26 ` Alan Maguire 3 siblings, 1 reply; 22+ messages in thread From: Andrii Nakryiko @ 2025-12-06 0:32 UTC (permalink / raw) To: Ihor Solodrai Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Andrew Morton, Nathan Chancellor, Nicolas Schier, Tejun Heo, David Vernet, Andrea Righi, Changwoo Min, Shuah Khan, Nick Desaulniers, Bill Wendling, Justin Stitt, Alan Maguire, Donglin Peng, bpf, dwarves, linux-kernel, linux-kbuild On Fri, Dec 5, 2025 at 2:32 PM Ihor Solodrai <ihor.solodrai@linux.dev> wrote: > > Subsequent patches in the series change vmlinux linking scripts to > unconditionally pass --btf_encode_detached to pahole, which was > introduced in v1.22 [1][2]. > > This change allows to remove PAHOLE_HAS_SPLIT_BTF Kconfig option and > other checks of older pahole versions. > > [1] https://github.com/acmel/dwarves/releases/tag/v1.22 > [2] https://lore.kernel.org/bpf/cbafbf4e-9073-4383-8ee6-1353f9e5869c@oracle.com/ > > Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev> > --- > lib/Kconfig.debug | 13 ++++--------- > scripts/Makefile.btf | 9 +-------- > tools/sched_ext/README.md | 1 - > 3 files changed, 5 insertions(+), 18 deletions(-) > > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > index 742b23ef0d8b..3abf3ae554b6 100644 > --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug > @@ -389,18 +389,13 @@ config DEBUG_INFO_BTF > depends on !DEBUG_INFO_SPLIT && !DEBUG_INFO_REDUCED > depends on !GCC_PLUGIN_RANDSTRUCT || COMPILE_TEST > depends on BPF_SYSCALL > - depends on PAHOLE_VERSION >= 116 > - depends on DEBUG_INFO_DWARF4 || PAHOLE_VERSION >= 121 > + depends on PAHOLE_VERSION >= 122 > # pahole uses elfutils, which does not have support for Hexagon relocations > depends on !HEXAGON > help > Generate deduplicated BTF type information from DWARF debug info. > - Turning this on requires pahole v1.16 or later (v1.21 or later to > - support DWARF 5), which will convert DWARF type info into equivalent > - deduplicated BTF type info. > - > -config PAHOLE_HAS_SPLIT_BTF > - def_bool PAHOLE_VERSION >= 119 > + Turning this on requires pahole v1.22 or later, which will convert > + DWARF type info into equivalent deduplicated BTF type info. > > config PAHOLE_HAS_BTF_TAG > def_bool PAHOLE_VERSION >= 123 > @@ -422,7 +417,7 @@ config PAHOLE_HAS_LANG_EXCLUDE > config DEBUG_INFO_BTF_MODULES > bool "Generate BTF type information for kernel modules" > default y > - depends on DEBUG_INFO_BTF && MODULES && PAHOLE_HAS_SPLIT_BTF > + depends on DEBUG_INFO_BTF && MODULES > help > Generate compact split BTF type information for kernel modules. > > diff --git a/scripts/Makefile.btf b/scripts/Makefile.btf > index db76335dd917..7c1cd6c2ff75 100644 > --- a/scripts/Makefile.btf > +++ b/scripts/Makefile.btf > @@ -7,14 +7,7 @@ JOBS := $(patsubst -j%,%,$(filter -j%,$(MAKEFLAGS))) > > ifeq ($(call test-le, $(pahole-ver), 125),y) > > -# pahole 1.18 through 1.21 can't handle zero-sized per-CPU vars > -ifeq ($(call test-le, $(pahole-ver), 121),y) > -pahole-flags-$(call test-ge, $(pahole-ver), 118) += --skip_encoding_btf_vars > -endif > - > -pahole-flags-$(call test-ge, $(pahole-ver), 121) += --btf_gen_floats > - > -pahole-flags-$(call test-ge, $(pahole-ver), 122) += -j$(JOBS) > +pahole-flags-$(call test-ge, $(pahole-ver), 122) += --btf_gen_floats -j$(JOBS) this should be unconditional given we expect at least 1.22, no? > > pahole-flags-$(call test-ge, $(pahole-ver), 125) += --skip_encoding_btf_inconsistent_proto --btf_gen_optimized > > diff --git a/tools/sched_ext/README.md b/tools/sched_ext/README.md > index 16a42e4060f6..56a9d1557ac4 100644 > --- a/tools/sched_ext/README.md > +++ b/tools/sched_ext/README.md > @@ -65,7 +65,6 @@ It's also recommended that you also include the following Kconfig options: > ``` > CONFIG_BPF_JIT_ALWAYS_ON=y > CONFIG_BPF_JIT_DEFAULT_ON=y > -CONFIG_PAHOLE_HAS_SPLIT_BTF=y > CONFIG_PAHOLE_HAS_BTF_TAG=y > ``` > > -- > 2.52.0 > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH bpf-next v3 4/6] lib/Kconfig.debug: Set the minimum required pahole version to v1.22 2025-12-06 0:32 ` Andrii Nakryiko @ 2025-12-06 0:58 ` Ihor Solodrai 0 siblings, 0 replies; 22+ messages in thread From: Ihor Solodrai @ 2025-12-06 0:58 UTC (permalink / raw) To: Andrii Nakryiko Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Andrew Morton, Nathan Chancellor, Nicolas Schier, Tejun Heo, David Vernet, Andrea Righi, Changwoo Min, Shuah Khan, Nick Desaulniers, Bill Wendling, Justin Stitt, Alan Maguire, Donglin Peng, bpf, dwarves, linux-kernel, linux-kbuild On 12/5/25 4:32 PM, Andrii Nakryiko wrote: > On Fri, Dec 5, 2025 at 2:32 PM Ihor Solodrai <ihor.solodrai@linux.dev> wrote: >> >> Subsequent patches in the series change vmlinux linking scripts to >> unconditionally pass --btf_encode_detached to pahole, which was >> introduced in v1.22 [1][2]. >> >> This change allows to remove PAHOLE_HAS_SPLIT_BTF Kconfig option and >> other checks of older pahole versions. >> >> [1] https://github.com/acmel/dwarves/releases/tag/v1.22 >> [2] https://lore.kernel.org/bpf/cbafbf4e-9073-4383-8ee6-1353f9e5869c@oracle.com/ >> >> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev> >> --- >> lib/Kconfig.debug | 13 ++++--------- >> scripts/Makefile.btf | 9 +-------- >> tools/sched_ext/README.md | 1 - >> 3 files changed, 5 insertions(+), 18 deletions(-) >> >> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug >> index 742b23ef0d8b..3abf3ae554b6 100644 >> --- a/lib/Kconfig.debug >> +++ b/lib/Kconfig.debug >> @@ -389,18 +389,13 @@ config DEBUG_INFO_BTF >> depends on !DEBUG_INFO_SPLIT && !DEBUG_INFO_REDUCED >> depends on !GCC_PLUGIN_RANDSTRUCT || COMPILE_TEST >> depends on BPF_SYSCALL >> - depends on PAHOLE_VERSION >= 116 >> - depends on DEBUG_INFO_DWARF4 || PAHOLE_VERSION >= 121 >> + depends on PAHOLE_VERSION >= 122 >> # pahole uses elfutils, which does not have support for Hexagon relocations >> depends on !HEXAGON >> help >> Generate deduplicated BTF type information from DWARF debug info. >> - Turning this on requires pahole v1.16 or later (v1.21 or later to >> - support DWARF 5), which will convert DWARF type info into equivalent >> - deduplicated BTF type info. >> - >> -config PAHOLE_HAS_SPLIT_BTF >> - def_bool PAHOLE_VERSION >= 119 >> + Turning this on requires pahole v1.22 or later, which will convert >> + DWARF type info into equivalent deduplicated BTF type info. >> >> config PAHOLE_HAS_BTF_TAG >> def_bool PAHOLE_VERSION >= 123 >> @@ -422,7 +417,7 @@ config PAHOLE_HAS_LANG_EXCLUDE >> config DEBUG_INFO_BTF_MODULES >> bool "Generate BTF type information for kernel modules" >> default y >> - depends on DEBUG_INFO_BTF && MODULES && PAHOLE_HAS_SPLIT_BTF >> + depends on DEBUG_INFO_BTF && MODULES >> help >> Generate compact split BTF type information for kernel modules. >> >> diff --git a/scripts/Makefile.btf b/scripts/Makefile.btf >> index db76335dd917..7c1cd6c2ff75 100644 >> --- a/scripts/Makefile.btf >> +++ b/scripts/Makefile.btf >> @@ -7,14 +7,7 @@ JOBS := $(patsubst -j%,%,$(filter -j%,$(MAKEFLAGS))) >> >> ifeq ($(call test-le, $(pahole-ver), 125),y) >> >> -# pahole 1.18 through 1.21 can't handle zero-sized per-CPU vars >> -ifeq ($(call test-le, $(pahole-ver), 121),y) >> -pahole-flags-$(call test-ge, $(pahole-ver), 118) += --skip_encoding_btf_vars >> -endif >> - >> -pahole-flags-$(call test-ge, $(pahole-ver), 121) += --btf_gen_floats >> - >> -pahole-flags-$(call test-ge, $(pahole-ver), 122) += -j$(JOBS) >> +pahole-flags-$(call test-ge, $(pahole-ver), 122) += --btf_gen_floats -j$(JOBS) > > this should be unconditional given we expect at least 1.22, no? Yes, it can be unconditional, but still under if ver < 125. > >> >> pahole-flags-$(call test-ge, $(pahole-ver), 125) += --skip_encoding_btf_inconsistent_proto --btf_gen_optimized >> >> diff --git a/tools/sched_ext/README.md b/tools/sched_ext/README.md >> index 16a42e4060f6..56a9d1557ac4 100644 >> --- a/tools/sched_ext/README.md >> +++ b/tools/sched_ext/README.md >> @@ -65,7 +65,6 @@ It's also recommended that you also include the following Kconfig options: >> ``` >> CONFIG_BPF_JIT_ALWAYS_ON=y >> CONFIG_BPF_JIT_DEFAULT_ON=y >> -CONFIG_PAHOLE_HAS_SPLIT_BTF=y >> CONFIG_PAHOLE_HAS_BTF_TAG=y >> ``` >> >> -- >> 2.52.0 >> ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH bpf-next v3 4/6] lib/Kconfig.debug: Set the minimum required pahole version to v1.22 2025-12-05 22:30 ` [PATCH bpf-next v3 4/6] lib/Kconfig.debug: Set the minimum required pahole version to v1.22 Ihor Solodrai 2025-12-05 22:49 ` bot+bpf-ci 2025-12-06 0:32 ` Andrii Nakryiko @ 2025-12-12 7:09 ` Eduard Zingerman 2025-12-12 17:26 ` Alan Maguire 3 siblings, 0 replies; 22+ messages in thread From: Eduard Zingerman @ 2025-12-12 7:09 UTC (permalink / raw) To: Ihor Solodrai, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Andrew Morton, Nathan Chancellor, Nicolas Schier, Tejun Heo, David Vernet, Andrea Righi, Changwoo Min, Shuah Khan, Nick Desaulniers, Bill Wendling, Justin Stitt, Alan Maguire, Donglin Peng Cc: bpf, dwarves, linux-kernel, linux-kbuild On Fri, 2025-12-05 at 14:30 -0800, Ihor Solodrai wrote: > Subsequent patches in the series change vmlinux linking scripts to > unconditionally pass --btf_encode_detached to pahole, which was > introduced in v1.22 [1][2]. > > This change allows to remove PAHOLE_HAS_SPLIT_BTF Kconfig option and > other checks of older pahole versions. > > [1] https://github.com/acmel/dwarves/releases/tag/v1.22 > [2] https://lore.kernel.org/bpf/cbafbf4e-9073-4383-8ee6-1353f9e5869c@oracle.com/ > > Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev> > --- Acked-by: Eduard Zingerman <eddyz87@gmail.com> [...] ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH bpf-next v3 4/6] lib/Kconfig.debug: Set the minimum required pahole version to v1.22 2025-12-05 22:30 ` [PATCH bpf-next v3 4/6] lib/Kconfig.debug: Set the minimum required pahole version to v1.22 Ihor Solodrai ` (2 preceding siblings ...) 2025-12-12 7:09 ` Eduard Zingerman @ 2025-12-12 17:26 ` Alan Maguire 3 siblings, 0 replies; 22+ messages in thread From: Alan Maguire @ 2025-12-12 17:26 UTC (permalink / raw) To: Ihor Solodrai, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Andrew Morton, Nathan Chancellor, Nicolas Schier, Tejun Heo, David Vernet, Andrea Righi, Changwoo Min, Shuah Khan, Nick Desaulniers, Bill Wendling, Justin Stitt, Donglin Peng Cc: bpf, dwarves, linux-kernel, linux-kbuild On 05/12/2025 22:30, Ihor Solodrai wrote: > Subsequent patches in the series change vmlinux linking scripts to > unconditionally pass --btf_encode_detached to pahole, which was > introduced in v1.22 [1][2]. > > This change allows to remove PAHOLE_HAS_SPLIT_BTF Kconfig option and > other checks of older pahole versions. > > [1] https://github.com/acmel/dwarves/releases/tag/v1.22 > [2] https://lore.kernel.org/bpf/cbafbf4e-9073-4383-8ee6-1353f9e5869c@oracle.com/ > > Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev> > --- > lib/Kconfig.debug | 13 ++++--------- > scripts/Makefile.btf | 9 +-------- > tools/sched_ext/README.md | 1 - > 3 files changed, 5 insertions(+), 18 deletions(-) > > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > index 742b23ef0d8b..3abf3ae554b6 100644 > --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug > @@ -389,18 +389,13 @@ config DEBUG_INFO_BTF > depends on !DEBUG_INFO_SPLIT && !DEBUG_INFO_REDUCED > depends on !GCC_PLUGIN_RANDSTRUCT || COMPILE_TEST > depends on BPF_SYSCALL > - depends on PAHOLE_VERSION >= 116 > - depends on DEBUG_INFO_DWARF4 || PAHOLE_VERSION >= 121 > + depends on PAHOLE_VERSION >= 122 > # pahole uses elfutils, which does not have support for Hexagon relocations > depends on !HEXAGON > help > Generate deduplicated BTF type information from DWARF debug info. > - Turning this on requires pahole v1.16 or later (v1.21 or later to > - support DWARF 5), which will convert DWARF type info into equivalent > - deduplicated BTF type info. > - > -config PAHOLE_HAS_SPLIT_BTF > - def_bool PAHOLE_VERSION >= 119 > + Turning this on requires pahole v1.22 or later, which will convert > + DWARF type info into equivalent deduplicated BTF type info. > > config PAHOLE_HAS_BTF_TAG > def_bool PAHOLE_VERSION >= 123 > @@ -422,7 +417,7 @@ config PAHOLE_HAS_LANG_EXCLUDE > config DEBUG_INFO_BTF_MODULES > bool "Generate BTF type information for kernel modules" > default y > - depends on DEBUG_INFO_BTF && MODULES && PAHOLE_HAS_SPLIT_BTF > + depends on DEBUG_INFO_BTF && MODULES > help > Generate compact split BTF type information for kernel modules. > > diff --git a/scripts/Makefile.btf b/scripts/Makefile.btf > index db76335dd917..7c1cd6c2ff75 100644 > --- a/scripts/Makefile.btf > +++ b/scripts/Makefile.btf > @@ -7,14 +7,7 @@ JOBS := $(patsubst -j%,%,$(filter -j%,$(MAKEFLAGS))) > hi Ihor, a small suggestion here, and it is orthogonal to what you're doing here, so just for consideration if you're planning a v4 since you're touching this file. We've had problems in the past because we get pahole version from .config in Makefile.btf pahole-ver := $(CONFIG_PAHOLE_VERSION) and it can be outdated. Specifically the problem is that if "make oldconfig" is not run after updating pahole we don't get the actual pahole version during builds and options can be missing. See [1] for an example, but perhaps we should do pahole-ver := $(shell $(srctree)/scripts/pahole-version.sh) in Makefile.btf to ensure the value reflects latest pahole and that then determines which options we use? Andrii suggested an approach like CC_VERSION_TEXT might be worth pursuing; AFAICT that recomputes the CC_VERSION and warns the user if there is a version difference. Given that the CONFIG pahole version requirements are all pretty modest - it might simply be enough to recompute it in Makefile.btf and perhaps ensure it's not less than CONFIG_PAHOLE_VERSION. Just a thought anyway. Thanks! Alan [1] https://lore.kernel.org/bpf/CAEf4BzYi1xX3p_bY3j9dEuPvtCW3H7z=p2vdn-2GY0OOenxQAg@mail.gmail.com/ ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH bpf-next v3 5/6] selftests/bpf: Run resolve_btfids only for relevant .test.o objects 2025-12-05 22:30 [PATCH bpf-next v3 0/6] resolve_btfids: Support for BTF modifications Ihor Solodrai ` (3 preceding siblings ...) 2025-12-05 22:30 ` [PATCH bpf-next v3 4/6] lib/Kconfig.debug: Set the minimum required pahole version to v1.22 Ihor Solodrai @ 2025-12-05 22:30 ` Ihor Solodrai 2025-12-05 22:35 ` [PATCH bpf-next v3 6/6] resolve_btfids: change in-place update with raw binary output Ihor Solodrai 4 siblings, 1 reply; 22+ messages in thread From: Ihor Solodrai @ 2025-12-05 22:30 UTC (permalink / raw) To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Andrew Morton, Nathan Chancellor, Nicolas Schier, Tejun Heo, David Vernet, Andrea Righi, Changwoo Min, Shuah Khan, Nick Desaulniers, Bill Wendling, Justin Stitt, Alan Maguire, Donglin Peng Cc: bpf, dwarves, linux-kernel, linux-kbuild A selftest targeting resolve_btfids functionality relies on a resolved .BTF_ids section to be available in the TRUNNER_BINARY. The underlying BTF data is taken from a special BPF program (btf_data.c), and so resolve_btfids is executed as a part of a TRUNNER_BINARY build recipe on the final binary. Subsequent patches in this series allow resolve_btfids to modify BTF before resolving the symbols, which means that the test needs access to that modified BTF [1]. Currently the test simply reads in btf_data.bpf.o on the assumption that BTF hasn't changed. Implement resolve_btfids call only for particular test objects (just resolve_btfids.test.o for now). The test objects are linked into the TRUNNER_BINARY, and so .BTF_ids section will be available there. This will make it trivial for the resolve_btfids test to access BTF modified by resolve_btfids. [1] https://lore.kernel.org/bpf/CAErzpmvsgSDe-QcWH8SFFErL6y3p3zrqNri5-UHJ9iK2ChyiBw@mail.gmail.com/ Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev> --- tools/testing/selftests/bpf/Makefile | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index 4aa60e83ff19..ffd0a4c354c7 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -643,6 +643,9 @@ $(TRUNNER_TESTS_HDR): $(TRUNNER_TESTS_DIR)/*.c ) > $$@) endif +$(TRUNNER_OUTPUT)/resolve_btfids.test.o: $(RESOLVE_BTFIDS) $(TRUNNER_OUTPUT)/btf_data.bpf.o +$(TRUNNER_OUTPUT)/resolve_btfids.test.o: private TEST_NEEDS_BTFIDS = 1 + # compile individual test files # Note: we cd into output directory to ensure embedded BPF object is found $(TRUNNER_TEST_OBJS): $(TRUNNER_OUTPUT)/%.test.o: \ @@ -650,6 +653,9 @@ $(TRUNNER_TEST_OBJS): $(TRUNNER_OUTPUT)/%.test.o: \ | $(TRUNNER_OUTPUT)/%.test.d $$(call msg,TEST-OBJ,$(TRUNNER_BINARY),$$@) $(Q)cd $$(@D) && $$(CC) -I. $$(CFLAGS) -MMD -MT $$@ -c $(CURDIR)/$$< $$(LDLIBS) -o $$(@F) + $$(if $$(TEST_NEEDS_BTFIDS), \ + $$(call msg,BTFIDS,$(TRUNNER_BINARY),$$@) \ + $(RESOLVE_BTFIDS) --btf $(TRUNNER_OUTPUT)/btf_data.bpf.o $$@) $(TRUNNER_TEST_OBJS:.o=.d): $(TRUNNER_OUTPUT)/%.test.d: \ $(TRUNNER_TESTS_DIR)/%.c \ @@ -695,13 +701,11 @@ $(OUTPUT)/$(TRUNNER_BINARY): | $(TRUNNER_BPF_OBJS) $(OUTPUT)/$(TRUNNER_BINARY): $(TRUNNER_TEST_OBJS) \ $(TRUNNER_EXTRA_OBJS) $$(BPFOBJ) \ $(TRUNNER_LIB_OBJS) \ - $(RESOLVE_BTFIDS) \ $(TRUNNER_BPFTOOL) \ $(OUTPUT)/veristat \ | $(TRUNNER_BINARY)-extras $$(call msg,BINARY,,$$@) $(Q)$$(CC) $$(CFLAGS) $$(filter %.a %.o,$$^) $$(LDLIBS) $$(LLVM_LDLIBS) $$(LDFLAGS) $$(LLVM_LDFLAGS) -o $$@ - $(Q)$(RESOLVE_BTFIDS) --btf $(TRUNNER_OUTPUT)/btf_data.bpf.o $$@ $(Q)ln -sf $(if $2,..,.)/tools/build/bpftool/$(USE_BOOTSTRAP)bpftool \ $(OUTPUT)/$(if $2,$2/)bpftool -- 2.52.0 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH bpf-next v3 6/6] resolve_btfids: change in-place update with raw binary output 2025-12-05 22:30 ` [PATCH bpf-next v3 5/6] selftests/bpf: Run resolve_btfids only for relevant .test.o objects Ihor Solodrai @ 2025-12-05 22:35 ` Ihor Solodrai 2025-12-06 1:13 ` Andrii Nakryiko 2025-12-12 7:08 ` Eduard Zingerman 0 siblings, 2 replies; 22+ messages in thread From: Ihor Solodrai @ 2025-12-05 22:35 UTC (permalink / raw) To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Andrew Morton, Nathan Chancellor, Nicolas Schier, Tejun Heo, David Vernet, Andrea Righi, Changwoo Min, Shuah Khan, Nick Desaulniers, Bill Wendling, Justin Stitt, Alan Maguire, Donglin Peng Cc: bpf, dwarves, linux-kernel, linux-kbuild Currently resolve_btfids updates .BTF_ids section of an ELF file in-place, based on the contents of provided BTF, usually within the same input file, and optionally a BTF base. Change resolve_btfids behavior to enable BTF transformations as part of its main operation. To achieve this, in-place ELF write in resolve_btfids is replaced with generation of the following binaries: * ${1}.BTF with .BTF section data * ${1}.BTF_ids with .BTF_ids section data if it existed in ${1} * ${1}.BTF.base with .BTF.base section data for out-of-tree modules The execution of resolve_btfids and consumption of its output is orchestrated by scripts/gen-btf.sh introduced in this patch. The motivation for emitting binary data is that it allows simplifying resolve_btfids implementation by delegating ELF update to the $OBJCOPY tool [1], which is already widely used across the codebase. There are two distinct paths for BTF generation and resolve_btfids application in the kernel build: for vmlinux and for kernel modules. For the vmlinux binary a .BTF section is added in a roundabout way to ensure correct linking. The patch doesn't change this approach, only the implementation is a little different. Before this patch it worked as follows: * pahole consumed .tmp_vmlinux1 [2] and added .BTF section with llvm-objcopy [3] to it * then everything except the .BTF section was stripped from .tmp_vmlinux1 into a .tmp_vmlinux1.bpf.o object [2], later linked into vmlinux * resolve_btfids was executed later on vmlinux.unstripped [4], updating it in-place After this patch gen-btf.sh implements the following: * pahole consumes .tmp_vmlinux1 and produces a *detached* file with raw BTF data * resolve_btfids consumes .tmp_vmlinux1 and detached BTF to produce (potentially modified) .BTF, and .BTF_ids sections data * a .tmp_vmlinux1.bpf.o object is then produced with objcopy copying BTF output of resolve_btfids * .BTF_ids data gets embedded into vmlinux.unstripped in link-vmlinux.sh by objcopy --update-section For kernel modules, creating a special .bpf.o file is not necessary, and so embedding of sections data produced by resolve_btfids is straightforward with objcopy. With this patch an ELF file becomes effectively read-only within resolve_btfids, which allows deleting elf_update() call and satellite code (like compressed_section_fix [5]). Endianness handling of .BTF_ids data is also changed. Previously the "flags" part of the section was bswapped in sets_patch() [6], and then Elf_Type was modified before elf_update() to signal to libelf that bswap may be necessary. With this patch we explicitly bswap entire data buffer on load and on dump. [1] https://lore.kernel.org/bpf/131b4190-9c49-4f79-a99d-c00fac97fa44@linux.dev/ [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/link-vmlinux.sh?h=v6.18#n110 [3] https://git.kernel.org/pub/scm/devel/pahole/pahole.git/tree/btf_encoder.c?h=v1.31#n1803 [4] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/link-vmlinux.sh?h=v6.18#n284 [5] https://lore.kernel.org/bpf/20200819092342.259004-1-jolsa@kernel.org/ [6] https://lore.kernel.org/bpf/cover.1707223196.git.vmalik@redhat.com/ Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev> --- MAINTAINERS | 1 + scripts/Makefile.btf | 17 +- scripts/Makefile.modfinal | 5 +- scripts/Makefile.vmlinux | 2 +- scripts/gen-btf.sh | 157 ++++++++++++ scripts/link-vmlinux.sh | 46 +--- tools/bpf/resolve_btfids/main.c | 228 +++++++++++------- tools/testing/selftests/bpf/.gitignore | 3 + tools/testing/selftests/bpf/Makefile | 9 +- .../selftests/bpf/prog_tests/resolve_btfids.c | 4 +- 10 files changed, 338 insertions(+), 134 deletions(-) create mode 100755 scripts/gen-btf.sh diff --git a/MAINTAINERS b/MAINTAINERS index e36689cd7cc7..fe6141c69708 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -4673,6 +4673,7 @@ F: net/sched/act_bpf.c F: net/sched/cls_bpf.c F: samples/bpf/ F: scripts/bpf_doc.py +F: scripts/gen-btf.sh F: scripts/Makefile.btf F: scripts/pahole-version.sh F: tools/bpf/ diff --git a/scripts/Makefile.btf b/scripts/Makefile.btf index 7c1cd6c2ff75..d067e91049cb 100644 --- a/scripts/Makefile.btf +++ b/scripts/Makefile.btf @@ -1,5 +1,10 @@ # SPDX-License-Identifier: GPL-2.0 +gen-btf-y = +gen-btf-$(CONFIG_DEBUG_INFO_BTF) = $(srctree)/scripts/gen-btf.sh + +export GEN_BTF := $(gen-btf-y) + pahole-ver := $(CONFIG_PAHOLE_VERSION) pahole-flags-y := @@ -18,13 +23,15 @@ pahole-flags-$(call test-ge, $(pahole-ver), 126) = -j$(JOBS) --btf_features=enc pahole-flags-$(call test-ge, $(pahole-ver), 130) += --btf_features=attributes -ifneq ($(KBUILD_EXTMOD),) -module-pahole-flags-$(call test-ge, $(pahole-ver), 128) += --btf_features=distilled_base -endif - endif pahole-flags-$(CONFIG_PAHOLE_HAS_LANG_EXCLUDE) += --lang_exclude=rust export PAHOLE_FLAGS := $(pahole-flags-y) -export MODULE_PAHOLE_FLAGS := $(module-pahole-flags-y) + +resolve-btfids-flags-y := +resolve-btfids-flags-$(CONFIG_WERROR) += --fatal_warnings +resolve-btfids-flags-$(if $(KBUILD_EXTMOD),y) += --distill_base +resolve-btfids-flags-$(if $(KBUILD_VERBOSE),y) += --verbose + +export RESOLVE_BTFIDS_FLAGS := $(resolve-btfids-flags-y) diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal index 542ba462ed3e..9e4722e55efa 100644 --- a/scripts/Makefile.modfinal +++ b/scripts/Makefile.modfinal @@ -38,9 +38,8 @@ quiet_cmd_btf_ko = BTF [M] $@ cmd_btf_ko = \ if [ ! -f $(objtree)/vmlinux ]; then \ printf "Skipping BTF generation for %s due to unavailability of vmlinux\n" $@ 1>&2; \ - else \ - LLVM_OBJCOPY="$(OBJCOPY)" $(PAHOLE) -J $(PAHOLE_FLAGS) $(MODULE_PAHOLE_FLAGS) --btf_base $(objtree)/vmlinux $@; \ - $(RESOLVE_BTFIDS) -b $(objtree)/vmlinux $@; \ + else \ + $(GEN_BTF) --btf_base $(objtree)/vmlinux $@; \ fi; # Same as newer-prereqs, but allows to exclude specified extra dependencies diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux index cd788cac9d91..33e9999724cc 100644 --- a/scripts/Makefile.vmlinux +++ b/scripts/Makefile.vmlinux @@ -71,7 +71,7 @@ targets += vmlinux.unstripped .vmlinux.export.o vmlinux.unstripped: scripts/link-vmlinux.sh vmlinux.o .vmlinux.export.o $(KBUILD_LDS) FORCE +$(call if_changed_dep,link_vmlinux) ifdef CONFIG_DEBUG_INFO_BTF -vmlinux.unstripped: $(RESOLVE_BTFIDS) +vmlinux.unstripped: $(RESOLVE_BTFIDS) $(GEN_BTF) endif ifdef CONFIG_BUILDTIME_TABLE_SORT diff --git a/scripts/gen-btf.sh b/scripts/gen-btf.sh new file mode 100755 index 000000000000..4723c8a5953a --- /dev/null +++ b/scripts/gen-btf.sh @@ -0,0 +1,157 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2025 Meta Platforms, Inc. and affiliates. +# +# This script generates BTF data for the provided ELF file. +# +# Kernel BTF generation involves these conceptual steps: +# 1. pahole generates BTF from DWARF data +# 2. resolve_btfids applies kernel-specific btf2btf +# transformations and computes data for .BTF_ids section +# 3. the result gets linked/objcopied into the target binary +# +# How step (3) should be done differs between vmlinux, and +# kernel modules, which is the primary reason for the existence +# of this script. +# +# For modules the script expects vmlinux passed in as --btf_base. +# Generated .BTF, .BTF.base and .BTF_ids sections become embedded +# into the input ELF file with objcopy. +# +# For vmlinux the input file remains unchanged and two files are produced: +# - ${1}.btf.o ready for linking into vmlinux +# - ${1}.BTF_ids with .BTF_ids data blob +# This output is consumed by scripts/link-vmlinux.sh + +set -e + +usage() +{ + echo "Usage: $0 [--btf_base <file>] <target ELF file>" + exit 1 +} + +BTF_BASE="" + +while [ $# -gt 0 ]; do + case "$1" in + --btf_base) + BTF_BASE="$2" + shift 2 + ;; + -*) + echo "Unknown option: $1" >&2 + usage + ;; + *) + break + ;; + esac +done + +if [ $# -ne 1 ]; then + usage +fi + +ELF_FILE="$1" +shift + +is_enabled() { + grep -q "^$1=y" ${objtree}/include/config/auto.conf +} + +info() +{ + printf " %-7s %s\n" "${1}" "${2}" +} + +case "${KBUILD_VERBOSE}" in +*1*) + set -x + ;; +esac + + +gen_btf_data() +{ + info BTF "${ELF_FILE}" + btf1="${ELF_FILE}.BTF.1" + ${PAHOLE} -J ${PAHOLE_FLAGS} \ + ${BTF_BASE:+--btf_base ${BTF_BASE}} \ + --btf_encode_detached=${btf1} \ + "${ELF_FILE}" + + info BTFIDS "${ELF_FILE}" + ${RESOLVE_BTFIDS} ${RESOLVE_BTFIDS_FLAGS} \ + ${BTF_BASE:+--btf_base ${BTF_BASE}} \ + --btf ${btf1} "${ELF_FILE}" +} + +gen_btf_o() +{ + local btf_data=${ELF_FILE}.btf.o + + # Create ${btf_data} which contains just .BTF section but no symbols. Add + # SHF_ALLOC because .BTF will be part of the vmlinux image. --strip-all + # deletes all symbols including __start_BTF and __stop_BTF, which will + # be redefined in the linker script. + info OBJCOPY "${btf_data}" + echo "" | ${CC} -c -x c -o ${btf_data} - + ${OBJCOPY} --add-section .BTF=${ELF_FILE}.BTF \ + --set-section-flags .BTF=alloc,readonly ${btf_data} + ${OBJCOPY} --only-section=.BTF --strip-all ${btf_data} + + # Change e_type to ET_REL so that it can be used to link final vmlinux. + # GNU ld 2.35+ and lld do not allow an ET_EXEC input. + if is_enabled CONFIG_CPU_BIG_ENDIAN; then + et_rel='\0\1' + else + et_rel='\1\0' + fi + printf "${et_rel}" | dd of="${btf_data}" conv=notrunc bs=1 seek=16 status=none +} + +embed_btf_data() +{ + info OBJCOPY "${ELF_FILE}.BTF" + ${OBJCOPY} --add-section .BTF=${ELF_FILE}.BTF ${ELF_FILE} + + # a module might not have a .BTF_ids or .BTF.base section + local btf_base="${ELF_FILE}.BTF.base" + if [ -f "${btf_base}" ]; then + ${OBJCOPY} --add-section .BTF.base=${btf_base} ${ELF_FILE} + fi + local btf_ids="${ELF_FILE}.BTF_ids" + if [ -f "${btf_ids}" ]; then + ${OBJCOPY} --update-section .BTF_ids=${btf_ids} ${ELF_FILE} + fi +} + +cleanup() +{ + rm -f "${ELF_FILE}.BTF.1" + rm -f "${ELF_FILE}.BTF" + if [ "${BTFGEN_MODE}" = "module" ]; then + rm -f "${ELF_FILE}.BTF.base" + rm -f "${ELF_FILE}.BTF_ids" + fi +} +trap cleanup EXIT + +BTFGEN_MODE="vmlinux" +if [ -n "${BTF_BASE}" ]; then + BTFGEN_MODE="module" +fi + +gen_btf_data + +case "${BTFGEN_MODE}" in +vmlinux) + gen_btf_o + ;; +module) + embed_btf_data + ;; +esac + +exit 0 diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh index 2df714ba51a9..4d6c5e6eb61f 100755 --- a/scripts/link-vmlinux.sh +++ b/scripts/link-vmlinux.sh @@ -106,34 +106,6 @@ vmlinux_link() ${kallsymso} ${btf_vmlinux_bin_o} ${arch_vmlinux_o} ${ldlibs} } -# generate .BTF typeinfo from DWARF debuginfo -# ${1} - vmlinux image -gen_btf() -{ - local btf_data=${1}.btf.o - - info BTF "${btf_data}" - LLVM_OBJCOPY="${OBJCOPY}" ${PAHOLE} -J ${PAHOLE_FLAGS} ${1} - - # Create ${btf_data} which contains just .BTF section but no symbols. Add - # SHF_ALLOC because .BTF will be part of the vmlinux image. --strip-all - # deletes all symbols including __start_BTF and __stop_BTF, which will - # be redefined in the linker script. Add 2>/dev/null to suppress GNU - # objcopy warnings: "empty loadable segment detected at ..." - ${OBJCOPY} --only-section=.BTF --set-section-flags .BTF=alloc,readonly \ - --strip-all ${1} "${btf_data}" 2>/dev/null - # Change e_type to ET_REL so that it can be used to link final vmlinux. - # GNU ld 2.35+ and lld do not allow an ET_EXEC input. - if is_enabled CONFIG_CPU_BIG_ENDIAN; then - et_rel='\0\1' - else - et_rel='\1\0' - fi - printf "${et_rel}" | dd of="${btf_data}" conv=notrunc bs=1 seek=16 status=none - - btf_vmlinux_bin_o=${btf_data} -} - # Create ${2}.o file with all symbols from the ${1} object file kallsyms() { @@ -205,6 +177,7 @@ if is_enabled CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX; then fi btf_vmlinux_bin_o= +btfids_vmlinux= kallsymso= strip_debug= generate_map= @@ -224,12 +197,14 @@ if is_enabled CONFIG_KALLSYMS || is_enabled CONFIG_DEBUG_INFO_BTF; then vmlinux_link .tmp_vmlinux1 fi -if is_enabled CONFIG_DEBUG_INFO_BTF; then - if ! gen_btf .tmp_vmlinux1; then +if [ -n "${GEN_BTF}" ]; then + if ! ${GEN_BTF} .tmp_vmlinux1; then echo >&2 "Failed to generate BTF for vmlinux" echo >&2 "Try to disable CONFIG_DEBUG_INFO_BTF" exit 1 fi + btf_vmlinux_bin_o=.tmp_vmlinux1.btf.o + btfids_vmlinux=.tmp_vmlinux1.BTF_ids fi if is_enabled CONFIG_KALLSYMS; then @@ -282,14 +257,9 @@ fi vmlinux_link "${VMLINUX}" -# fill in BTF IDs -if is_enabled CONFIG_DEBUG_INFO_BTF; then - info BTFIDS "${VMLINUX}" - RESOLVE_BTFIDS_ARGS="" - if is_enabled CONFIG_WERROR; then - RESOLVE_BTFIDS_ARGS=" --fatal_warnings " - fi - ${RESOLVE_BTFIDS} ${RESOLVE_BTFIDS_ARGS} "${VMLINUX}" +if [ -n "${GEN_BTF}" ]; then + info OBJCOPY ${btfids_vmlinux} + ${OBJCOPY} --update-section .BTF_ids=${btfids_vmlinux} ${VMLINUX} fi mksysmap "${VMLINUX}" System.map diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c index cb1e69eb0bd7..e0e792017e77 100644 --- a/tools/bpf/resolve_btfids/main.c +++ b/tools/bpf/resolve_btfids/main.c @@ -71,9 +71,11 @@ #include <fcntl.h> #include <errno.h> #include <linux/btf_ids.h> +#include <linux/kallsyms.h> #include <linux/rbtree.h> #include <linux/zalloc.h> #include <linux/err.h> +#include <linux/limits.h> #include <bpf/btf.h> #include <bpf/libbpf.h> #include <subcmd/parse-options.h> @@ -124,6 +126,7 @@ struct object { struct btf *btf; struct btf *base_btf; + bool distill_base; struct { int fd; @@ -315,42 +318,16 @@ static struct btf_id *add_symbol(struct rb_root *root, char *name, size_t size) return btf_id__add(root, id, BTF_ID_KIND_SYM); } -/* Older libelf.h and glibc elf.h might not yet define the ELF compression types. */ -#ifndef SHF_COMPRESSED -#define SHF_COMPRESSED (1 << 11) /* Section with compressed data. */ -#endif - -/* - * The data of compressed section should be aligned to 4 - * (for 32bit) or 8 (for 64 bit) bytes. The binutils ld - * sets sh_addralign to 1, which makes libelf fail with - * misaligned section error during the update: - * FAILED elf_update(WRITE): invalid section alignment - * - * While waiting for ld fix, we fix the compressed sections - * sh_addralign value manualy. - */ -static int compressed_section_fix(Elf *elf, Elf_Scn *scn, GElf_Shdr *sh) +static void bswap_32_data(void *data, u32 nr_bytes) { - int expected = gelf_getclass(elf) == ELFCLASS32 ? 4 : 8; - - if (!(sh->sh_flags & SHF_COMPRESSED)) - return 0; - - if (sh->sh_addralign == expected) - return 0; + u32 cnt, i; + u32 *ptr; - pr_debug2(" - fixing wrong alignment sh_addralign %u, expected %u\n", - sh->sh_addralign, expected); + cnt = nr_bytes / sizeof(u32); + ptr = data; - sh->sh_addralign = expected; - - if (gelf_update_shdr(scn, sh) == 0) { - pr_err("FAILED cannot update section header: %s\n", - elf_errmsg(-1)); - return -1; - } - return 0; + for (i = 0; i < cnt; i++) + ptr[i] = bswap_32(ptr[i]); } static int elf_collect(struct object *obj) @@ -371,7 +348,7 @@ static int elf_collect(struct object *obj) elf_version(EV_CURRENT); - elf = elf_begin(fd, ELF_C_RDWR_MMAP, NULL); + elf = elf_begin(fd, ELF_C_READ_MMAP_PRIVATE, NULL); if (!elf) { close(fd); pr_err("FAILED cannot create ELF descriptor: %s\n", @@ -434,21 +411,20 @@ static int elf_collect(struct object *obj) obj->efile.symbols_shndx = idx; obj->efile.strtabidx = sh.sh_link; } else if (!strcmp(name, BTF_IDS_SECTION)) { + /* + * If target endianness differs from host, we need to bswap32 + * the .BTF_ids section data on load, because .BTF_ids has + * Elf_Type = ELF_T_BYTE, and so libelf returns data buffer in + * the target endiannes. We repeat this on dump. + */ + if (obj->efile.encoding != ELFDATANATIVE) { + pr_debug("bswap_32 .BTF_ids data from target to host endianness\n"); + bswap_32_data(data->d_buf, data->d_size); + } obj->efile.idlist = data; obj->efile.idlist_shndx = idx; obj->efile.idlist_addr = sh.sh_addr; - } else if (!strcmp(name, BTF_BASE_ELF_SEC)) { - /* If a .BTF.base section is found, do not resolve - * BTF ids relative to vmlinux; resolve relative - * to the .BTF.base section instead. btf__parse_split() - * will take care of this once the base BTF it is - * passed is NULL. - */ - obj->base_btf_path = NULL; } - - if (compressed_section_fix(elf, scn, &sh)) - return -1; } return 0; @@ -552,6 +528,13 @@ static int symbols_collect(struct object *obj) return 0; } +static inline bool is_envvar_set(const char *var_name) +{ + const char *value = getenv(var_name); + + return value && value[0] != '\0'; +} + static int load_btf(struct object *obj) { struct btf *base_btf = NULL, *btf = NULL; @@ -578,6 +561,19 @@ static int load_btf(struct object *obj) obj->base_btf = base_btf; obj->btf = btf; + if (obj->base_btf && obj->distill_base) { + err = btf__distill_base(obj->btf, &base_btf, &btf); + if (err) { + pr_err("FAILED to distill base BTF: %s\n", strerror(errno)); + goto out_err; + } + + btf__free(obj->btf); + btf__free(obj->base_btf); + obj->btf = btf; + obj->base_btf = base_btf; + } + return 0; out_err: @@ -751,24 +747,6 @@ static int sets_patch(struct object *obj) */ BUILD_BUG_ON((u32 *)set8->pairs != &set8->pairs[0].id); qsort(set8->pairs, set8->cnt, sizeof(set8->pairs[0]), cmp_id); - - /* - * When ELF endianness does not match endianness of the - * host, libelf will do the translation when updating - * the ELF. This, however, corrupts SET8 flags which are - * already in the target endianness. So, let's bswap - * them to the host endianness and libelf will then - * correctly translate everything. - */ - if (obj->efile.encoding != ELFDATANATIVE) { - int i; - - set8->flags = bswap_32(set8->flags); - for (i = 0; i < set8->cnt; i++) { - set8->pairs[i].flags = - bswap_32(set8->pairs[i].flags); - } - } break; default: pr_err("Unexpected btf_id_kind %d for set '%s'\n", id->kind, id->name); @@ -784,8 +762,6 @@ static int sets_patch(struct object *obj) static int symbols_patch(struct object *obj) { - off_t err; - if (__symbols_patch(obj, &obj->structs) || __symbols_patch(obj, &obj->unions) || __symbols_patch(obj, &obj->typedefs) || @@ -796,20 +772,90 @@ static int symbols_patch(struct object *obj) if (sets_patch(obj)) return -1; - /* Set type to ensure endian translation occurs. */ - obj->efile.idlist->d_type = ELF_T_WORD; + return 0; +} - elf_flagdata(obj->efile.idlist, ELF_C_SET, ELF_F_DIRTY); +static int dump_raw_data(const char *out_path, const void *data, u32 size) +{ + size_t written; + FILE *file; - err = elf_update(obj->efile.elf, ELF_C_WRITE); - if (err < 0) { - pr_err("FAILED elf_update(WRITE): %s\n", - elf_errmsg(-1)); + file = fopen(out_path, "wb"); + if (!file) { + pr_err("Couldn't open %s for writing\n", out_path); + return -1; + } + + written = fwrite(data, 1, size, file); + if (written != size) { + pr_err("Failed to write data to %s\n", out_path); + fclose(file); + unlink(out_path); + return -1; + } + + fclose(file); + pr_debug("Dumped %lu bytes of data to %s\n", size, out_path); + + return 0; +} + +static int dump_raw_btf_ids(struct object *obj, const char *out_path) +{ + Elf_Data *data = obj->efile.idlist; + int fd, err; + + if (!data || !data->d_buf) { + pr_debug("%s has no BTF_ids data to dump\n", obj->path); + return 0; + } + + /* + * If target endianness differs from host, we need to bswap32 the + * .BTF_ids section data before dumping so that the output is in + * target endianness. + */ + if (obj->efile.encoding != ELFDATANATIVE) { + pr_debug("bswap_32 .BTF_ids data from host to target endianness\n"); + bswap_32_data(data->d_buf, data->d_size); + } + + err = dump_raw_data(out_path, data->d_buf, data->d_size); + if (err) + return -1; + + return 0; +} + +static int dump_raw_btf(struct btf *btf, const char *out_path) +{ + const void *raw_btf_data; + u32 raw_btf_size; + int fd, err; + + raw_btf_data = btf__raw_data(btf, &raw_btf_size); + if (!raw_btf_data) { + pr_err("btf__raw_data() failed\n"); + return -1; + } + + err = dump_raw_data(out_path, raw_btf_data, raw_btf_size); + if (err) + return -1; + + return 0; +} + +static inline int make_out_path(char *buf, const char *in_path, const char *suffix) +{ + int len = snprintf(buf, PATH_MAX, "%s%s", in_path, suffix); + + if (len < 0 || len >= PATH_MAX) { + pr_err("Output path is too long: %s%s\n", in_path, suffix); + return -E2BIG; } - pr_debug("update %s for %s\n", - err >= 0 ? "ok" : "failed", obj->path); - return err < 0 ? -1 : 0; + return 0; } static const char * const resolve_btfids_usage[] = { @@ -831,6 +877,8 @@ int main(int argc, const char **argv) .sets = RB_ROOT, }; bool fatal_warnings = false; + char out_path[PATH_MAX]; + struct option btfid_options[] = { OPT_INCR('v', "verbose", &verbose, "be more verbose (show errors, etc)"), @@ -840,9 +888,11 @@ int main(int argc, const char **argv) "path of file providing base BTF"), OPT_BOOLEAN(0, "fatal_warnings", &fatal_warnings, "turn warnings into errors"), + OPT_BOOLEAN(0, "distill_base", &obj.distill_base, + "distill --btf_base and emit .BTF.base section data"), OPT_END() }; - int err = -1; + int err = -1, path_len; argc = parse_options(argc, argv, btfid_options, resolve_btfids_usage, PARSE_OPT_STOP_AT_NON_OPTION); @@ -851,6 +901,9 @@ int main(int argc, const char **argv) obj.path = argv[0]; + if (load_btf(&obj)) + goto out; + if (elf_collect(&obj)) goto out; @@ -860,23 +913,34 @@ int main(int argc, const char **argv) */ if (obj.efile.idlist_shndx == -1 || obj.efile.symbols_shndx == -1) { - pr_debug("Cannot find .BTF_ids or symbols sections, nothing to do\n"); - err = 0; - goto out; + pr_debug("Cannot find .BTF_ids or symbols sections, skip symbols resolution\n"); + goto dump_btf; } if (symbols_collect(&obj)) goto out; - if (load_btf(&obj)) - goto out; - if (symbols_resolve(&obj)) goto out; if (symbols_patch(&obj)) goto out; + err = make_out_path(out_path, obj.path, BTF_IDS_SECTION); + if (err || dump_raw_btf_ids(&obj, out_path)) + goto out; + +dump_btf: + err = make_out_path(out_path, obj.path, BTF_ELF_SEC); + if (err || dump_raw_btf(obj.btf, out_path)) + goto out; + + if (obj.base_btf && obj.distill_base) { + err = make_out_path(out_path, obj.path, BTF_BASE_ELF_SEC); + if (err || dump_raw_btf(obj.base_btf, out_path)) + goto out; + } + if (!(fatal_warnings && warnings)) err = 0; out: diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore index 19c1638e312a..b8bf51b7a0b0 100644 --- a/tools/testing/selftests/bpf/.gitignore +++ b/tools/testing/selftests/bpf/.gitignore @@ -45,3 +45,6 @@ xdp_synproxy xdp_hw_metadata xdp_features verification_cert.h +*.BTF +*.BTF_ids +*.BTF.base diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index ffd0a4c354c7..f28a32b16ff0 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -4,6 +4,7 @@ include ../../../scripts/Makefile.arch include ../../../scripts/Makefile.include CXX ?= $(CROSS_COMPILE)g++ +OBJCOPY ?= $(CROSS_COMPILE)objcopy CURDIR := $(abspath .) TOOLSDIR := $(abspath ../../..) @@ -653,9 +654,10 @@ $(TRUNNER_TEST_OBJS): $(TRUNNER_OUTPUT)/%.test.o: \ | $(TRUNNER_OUTPUT)/%.test.d $$(call msg,TEST-OBJ,$(TRUNNER_BINARY),$$@) $(Q)cd $$(@D) && $$(CC) -I. $$(CFLAGS) -MMD -MT $$@ -c $(CURDIR)/$$< $$(LDLIBS) -o $$(@F) - $$(if $$(TEST_NEEDS_BTFIDS), \ - $$(call msg,BTFIDS,$(TRUNNER_BINARY),$$@) \ - $(RESOLVE_BTFIDS) --btf $(TRUNNER_OUTPUT)/btf_data.bpf.o $$@) + $$(if $$(TEST_NEEDS_BTFIDS), \ + $$(call msg,BTFIDS,$(TRUNNER_BINARY),$$@) \ + $(RESOLVE_BTFIDS) --btf $(TRUNNER_OUTPUT)/btf_data.bpf.o $$@; \ + $(OBJCOPY) --update-section .BTF_ids=$$@.BTF_ids $$@) $(TRUNNER_TEST_OBJS:.o=.d): $(TRUNNER_OUTPUT)/%.test.d: \ $(TRUNNER_TESTS_DIR)/%.c \ @@ -894,6 +896,7 @@ EXTRA_CLEAN := $(SCRATCH_DIR) $(HOST_SCRATCH_DIR) \ prog_tests/tests.h map_tests/tests.h verifier/tests.h \ feature bpftool $(TEST_KMOD_TARGETS) \ $(addprefix $(OUTPUT)/,*.o *.d *.skel.h *.lskel.h *.subskel.h \ + *.BTF *.BTF_ids *.BTF.base \ no_alu32 cpuv4 bpf_gcc \ liburandom_read.so) \ $(OUTPUT)/FEATURE-DUMP.selftests \ diff --git a/tools/testing/selftests/bpf/prog_tests/resolve_btfids.c b/tools/testing/selftests/bpf/prog_tests/resolve_btfids.c index 51544372f52e..41dfaaabb73f 100644 --- a/tools/testing/selftests/bpf/prog_tests/resolve_btfids.c +++ b/tools/testing/selftests/bpf/prog_tests/resolve_btfids.c @@ -101,9 +101,9 @@ static int resolve_symbols(void) int type_id; __u32 nr; - btf = btf__parse_elf("btf_data.bpf.o", NULL); + btf = btf__parse_raw("resolve_btfids.test.o.BTF"); if (CHECK(libbpf_get_error(btf), "resolve", - "Failed to load BTF from btf_data.bpf.o\n")) + "Failed to load BTF from resolve_btfids.test.o.BTF\n")) return -1; nr = btf__type_cnt(btf); -- 2.52.0 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH bpf-next v3 6/6] resolve_btfids: change in-place update with raw binary output 2025-12-05 22:35 ` [PATCH bpf-next v3 6/6] resolve_btfids: change in-place update with raw binary output Ihor Solodrai @ 2025-12-06 1:13 ` Andrii Nakryiko 2025-12-06 1:37 ` Ihor Solodrai 2025-12-12 7:08 ` Eduard Zingerman 1 sibling, 1 reply; 22+ messages in thread From: Andrii Nakryiko @ 2025-12-06 1:13 UTC (permalink / raw) To: Ihor Solodrai Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Andrew Morton, Nathan Chancellor, Nicolas Schier, Tejun Heo, David Vernet, Andrea Righi, Changwoo Min, Shuah Khan, Nick Desaulniers, Bill Wendling, Justin Stitt, Alan Maguire, Donglin Peng, bpf, dwarves, linux-kernel, linux-kbuild On Fri, Dec 5, 2025 at 2:36 PM Ihor Solodrai <ihor.solodrai@linux.dev> wrote: > > Currently resolve_btfids updates .BTF_ids section of an ELF file > in-place, based on the contents of provided BTF, usually within the > same input file, and optionally a BTF base. > > Change resolve_btfids behavior to enable BTF transformations as part > of its main operation. To achieve this, in-place ELF write in > resolve_btfids is replaced with generation of the following binaries: > * ${1}.BTF with .BTF section data > * ${1}.BTF_ids with .BTF_ids section data if it existed in ${1} > * ${1}.BTF.base with .BTF.base section data for out-of-tree modules > > The execution of resolve_btfids and consumption of its output is > orchestrated by scripts/gen-btf.sh introduced in this patch. > > The motivation for emitting binary data is that it allows simplifying > resolve_btfids implementation by delegating ELF update to the $OBJCOPY > tool [1], which is already widely used across the codebase. > > There are two distinct paths for BTF generation and resolve_btfids > application in the kernel build: for vmlinux and for kernel modules. > > For the vmlinux binary a .BTF section is added in a roundabout way to > ensure correct linking. The patch doesn't change this approach, only > the implementation is a little different. > > Before this patch it worked as follows: > > * pahole consumed .tmp_vmlinux1 [2] and added .BTF section with > llvm-objcopy [3] to it > * then everything except the .BTF section was stripped from .tmp_vmlinux1 > into a .tmp_vmlinux1.bpf.o object [2], later linked into vmlinux > * resolve_btfids was executed later on vmlinux.unstripped [4], > updating it in-place > > After this patch gen-btf.sh implements the following: > > * pahole consumes .tmp_vmlinux1 and produces a *detached* file with > raw BTF data > * resolve_btfids consumes .tmp_vmlinux1 and detached BTF to produce > (potentially modified) .BTF, and .BTF_ids sections data > * a .tmp_vmlinux1.bpf.o object is then produced with objcopy copying > BTF output of resolve_btfids > * .BTF_ids data gets embedded into vmlinux.unstripped in > link-vmlinux.sh by objcopy --update-section > > For kernel modules, creating a special .bpf.o file is not necessary, > and so embedding of sections data produced by resolve_btfids is > straightforward with objcopy. > > With this patch an ELF file becomes effectively read-only within > resolve_btfids, which allows deleting elf_update() call and satellite > code (like compressed_section_fix [5]). > > Endianness handling of .BTF_ids data is also changed. Previously the > "flags" part of the section was bswapped in sets_patch() [6], and then > Elf_Type was modified before elf_update() to signal to libelf that > bswap may be necessary. With this patch we explicitly bswap entire > data buffer on load and on dump. > > [1] https://lore.kernel.org/bpf/131b4190-9c49-4f79-a99d-c00fac97fa44@linux.dev/ > [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/link-vmlinux.sh?h=v6.18#n110 > [3] https://git.kernel.org/pub/scm/devel/pahole/pahole.git/tree/btf_encoder.c?h=v1.31#n1803 > [4] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/link-vmlinux.sh?h=v6.18#n284 > [5] https://lore.kernel.org/bpf/20200819092342.259004-1-jolsa@kernel.org/ > [6] https://lore.kernel.org/bpf/cover.1707223196.git.vmalik@redhat.com/ > > Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev> > --- > MAINTAINERS | 1 + > scripts/Makefile.btf | 17 +- > scripts/Makefile.modfinal | 5 +- > scripts/Makefile.vmlinux | 2 +- > scripts/gen-btf.sh | 157 ++++++++++++ > scripts/link-vmlinux.sh | 46 +--- > tools/bpf/resolve_btfids/main.c | 228 +++++++++++------- > tools/testing/selftests/bpf/.gitignore | 3 + > tools/testing/selftests/bpf/Makefile | 9 +- > .../selftests/bpf/prog_tests/resolve_btfids.c | 4 +- > 10 files changed, 338 insertions(+), 134 deletions(-) > create mode 100755 scripts/gen-btf.sh > Overall it looks good, but I'd like another pair of eyes on this :) See some more minore nits below as well. pw-bot: cr > diff --git a/MAINTAINERS b/MAINTAINERS > index e36689cd7cc7..fe6141c69708 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -4673,6 +4673,7 @@ F: net/sched/act_bpf.c > F: net/sched/cls_bpf.c > F: samples/bpf/ > F: scripts/bpf_doc.py > +F: scripts/gen-btf.sh > F: scripts/Makefile.btf > F: scripts/pahole-version.sh > F: tools/bpf/ > diff --git a/scripts/Makefile.btf b/scripts/Makefile.btf > index 7c1cd6c2ff75..d067e91049cb 100644 > --- a/scripts/Makefile.btf > +++ b/scripts/Makefile.btf > @@ -1,5 +1,10 @@ > # SPDX-License-Identifier: GPL-2.0 > > +gen-btf-y = > +gen-btf-$(CONFIG_DEBUG_INFO_BTF) = $(srctree)/scripts/gen-btf.sh > + > +export GEN_BTF := $(gen-btf-y) > + What's the point of GEN_BTF? It's just so that you don't have to have $(srctree)/scripts/gen-btf.sh specified in three places? Between obscure $(GEN_BTF) (and having to understand where it is set and how it's exported) and explicit $(srctree)/scripts/gen-btf.sh in a few places, I'd prefer the latter, as it is way more greppable and it's not like we are going to rename or move this script frequently > pahole-ver := $(CONFIG_PAHOLE_VERSION) > pahole-flags-y := > [...] > @@ -371,7 +348,7 @@ static int elf_collect(struct object *obj) > > elf_version(EV_CURRENT); > > - elf = elf_begin(fd, ELF_C_RDWR_MMAP, NULL); > + elf = elf_begin(fd, ELF_C_READ_MMAP_PRIVATE, NULL); > if (!elf) { > close(fd); > pr_err("FAILED cannot create ELF descriptor: %s\n", > @@ -434,21 +411,20 @@ static int elf_collect(struct object *obj) > obj->efile.symbols_shndx = idx; > obj->efile.strtabidx = sh.sh_link; > } else if (!strcmp(name, BTF_IDS_SECTION)) { > + /* > + * If target endianness differs from host, we need to bswap32 > + * the .BTF_ids section data on load, because .BTF_ids has > + * Elf_Type = ELF_T_BYTE, and so libelf returns data buffer in > + * the target endiannes. We repeat this on dump. gmail screams at me for "endianness" > + */ > + if (obj->efile.encoding != ELFDATANATIVE) { > + pr_debug("bswap_32 .BTF_ids data from target to host endianness\n"); > + bswap_32_data(data->d_buf, data->d_size); this looks like a violation of ELF_C_READ_MMAP_PRIVATE promise, no?... would it be too create a copy here? for simplicity we can just always malloc() a copy, regardless of bswap(), it can never be a huge amount of data > + } > obj->efile.idlist = data; > obj->efile.idlist_shndx = idx; > obj->efile.idlist_addr = sh.sh_addr; > - } else if (!strcmp(name, BTF_BASE_ELF_SEC)) { > - /* If a .BTF.base section is found, do not resolve > - * BTF ids relative to vmlinux; resolve relative > - * to the .BTF.base section instead. btf__parse_split() > - * will take care of this once the base BTF it is > - * passed is NULL. > - */ > - obj->base_btf_path = NULL; > } > - > - if (compressed_section_fix(elf, scn, &sh)) > - return -1; > } > > return 0; > @@ -552,6 +528,13 @@ static int symbols_collect(struct object *obj) > return 0; > } > > +static inline bool is_envvar_set(const char *var_name) > +{ > + const char *value = getenv(var_name); > + > + return value && value[0] != '\0'; > +} > + leftovers? > static int load_btf(struct object *obj) > { > struct btf *base_btf = NULL, *btf = NULL; > @@ -578,6 +561,19 @@ static int load_btf(struct object *obj) > obj->base_btf = base_btf; > obj->btf = btf; > > + if (obj->base_btf && obj->distill_base) { > + err = btf__distill_base(obj->btf, &base_btf, &btf); > + if (err) { > + pr_err("FAILED to distill base BTF: %s\n", strerror(errno)); > + goto out_err; > + } > + > + btf__free(obj->btf); > + btf__free(obj->base_btf); > + obj->btf = btf; > + obj->base_btf = base_btf; > + } > + > return 0; > > out_err: [...] > +static int dump_raw_btf_ids(struct object *obj, const char *out_path) > +{ > + Elf_Data *data = obj->efile.idlist; > + int fd, err; > + > + if (!data || !data->d_buf) { > + pr_debug("%s has no BTF_ids data to dump\n", obj->path); > + return 0; > + } > + > + /* > + * If target endianness differs from host, we need to bswap32 the > + * .BTF_ids section data before dumping so that the output is in > + * target endianness. > + */ > + if (obj->efile.encoding != ELFDATANATIVE) { > + pr_debug("bswap_32 .BTF_ids data from host to target endianness\n"); > + bswap_32_data(data->d_buf, data->d_size); same about modifying ELF data in-place for what is supposed to be read-only use > + } > + > + err = dump_raw_data(out_path, data->d_buf, data->d_size); > + if (err) > + return -1; > + > + return 0; > +} > + > +static int dump_raw_btf(struct btf *btf, const char *out_path) > +{ > + const void *raw_btf_data; > + u32 raw_btf_size; > + int fd, err; > + > + raw_btf_data = btf__raw_data(btf, &raw_btf_size); > + if (!raw_btf_data) { > + pr_err("btf__raw_data() failed\n"); > + return -1; > + } did you check that libbpf does proper byte swap as well? > + > + err = dump_raw_data(out_path, raw_btf_data, raw_btf_size); > + if (err) > + return -1; > + > + return 0; > +} > + > +static inline int make_out_path(char *buf, const char *in_path, const char *suffix) > +{ > + int len = snprintf(buf, PATH_MAX, "%s%s", in_path, suffix); nit: normally you pass buffer and its size as input arguments instead of assuming and hard-coding common PATH_MAX constant in two separate places > + > + if (len < 0 || len >= PATH_MAX) { > + pr_err("Output path is too long: %s%s\n", in_path, suffix); > + return -E2BIG; > } > > - pr_debug("update %s for %s\n", > - err >= 0 ? "ok" : "failed", obj->path); > - return err < 0 ? -1 : 0; > + return 0; > } > > static const char * const resolve_btfids_usage[] = { [...] ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH bpf-next v3 6/6] resolve_btfids: change in-place update with raw binary output 2025-12-06 1:13 ` Andrii Nakryiko @ 2025-12-06 1:37 ` Ihor Solodrai 0 siblings, 0 replies; 22+ messages in thread From: Ihor Solodrai @ 2025-12-06 1:37 UTC (permalink / raw) To: Andrii Nakryiko Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Andrew Morton, Nathan Chancellor, Nicolas Schier, Tejun Heo, David Vernet, Andrea Righi, Changwoo Min, Shuah Khan, Nick Desaulniers, Alan Maguire, Bill Wendling, Justin Stitt, Donglin Peng, bpf, dwarves, linux-kernel, linux-kbuild On 12/5/25 5:13 PM, Andrii Nakryiko wrote: > On Fri, Dec 5, 2025 at 2:36 PM Ihor Solodrai <ihor.solodrai@linux.dev> wrote: >> >> [...] >> > > Overall it looks good, but I'd like another pair of eyes on this :) I don't think you need to worry about another pair of eyes :) > See some more minore nits below as well. > > pw-bot: cr > > >> diff --git a/MAINTAINERS b/MAINTAINERS >> index e36689cd7cc7..fe6141c69708 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -4673,6 +4673,7 @@ F: net/sched/act_bpf.c >> F: net/sched/cls_bpf.c >> F: samples/bpf/ >> F: scripts/bpf_doc.py >> +F: scripts/gen-btf.sh >> F: scripts/Makefile.btf >> F: scripts/pahole-version.sh >> F: tools/bpf/ >> diff --git a/scripts/Makefile.btf b/scripts/Makefile.btf >> index 7c1cd6c2ff75..d067e91049cb 100644 >> --- a/scripts/Makefile.btf >> +++ b/scripts/Makefile.btf >> @@ -1,5 +1,10 @@ >> # SPDX-License-Identifier: GPL-2.0 >> >> +gen-btf-y = >> +gen-btf-$(CONFIG_DEBUG_INFO_BTF) = $(srctree)/scripts/gen-btf.sh >> + >> +export GEN_BTF := $(gen-btf-y) >> + > > What's the point of GEN_BTF? It's just so that you don't have to have > $(srctree)/scripts/gen-btf.sh specified in three places? Between > obscure $(GEN_BTF) (and having to understand where it is set and how > it's exported) and explicit $(srctree)/scripts/gen-btf.sh in a few > places, I'd prefer the latter, as it is way more greppable and it's > not like we are going to rename or move this script frequently Yeah... It seems fine for me either way, tbh. I think a slight difference is that in link-vmlinux.sh checking for a config value is a grep on include/config/auto.conf, which is not necessary if we have a dedicated env variable. > > >> pahole-ver := $(CONFIG_PAHOLE_VERSION) >> pahole-flags-y := >> > > [...] > >> @@ -371,7 +348,7 @@ static int elf_collect(struct object *obj) >> >> elf_version(EV_CURRENT); >> >> - elf = elf_begin(fd, ELF_C_RDWR_MMAP, NULL); >> + elf = elf_begin(fd, ELF_C_READ_MMAP_PRIVATE, NULL); >> if (!elf) { >> close(fd); >> pr_err("FAILED cannot create ELF descriptor: %s\n", >> @@ -434,21 +411,20 @@ static int elf_collect(struct object *obj) >> obj->efile.symbols_shndx = idx; >> obj->efile.strtabidx = sh.sh_link; >> } else if (!strcmp(name, BTF_IDS_SECTION)) { >> + /* >> + * If target endianness differs from host, we need to bswap32 >> + * the .BTF_ids section data on load, because .BTF_ids has >> + * Elf_Type = ELF_T_BYTE, and so libelf returns data buffer in >> + * the target endiannes. We repeat this on dump. > > gmail screams at me for "endianness" That's how you know I am not AI. > >> + */ >> + if (obj->efile.encoding != ELFDATANATIVE) { >> + pr_debug("bswap_32 .BTF_ids data from target to host endianness\n"); >> + bswap_32_data(data->d_buf, data->d_size); > > this looks like a violation of ELF_C_READ_MMAP_PRIVATE promise, no?... > would it be too create a copy here? for simplicity we can just always > malloc() a copy, regardless of bswap(), it can never be a huge amount > of data No, it's not a violation. From libelf.h: ELF_C_READ_MMAP_PRIVATE, /* Read, but memory is writable, results are not written to the file. */ I haven't checked how it works under the hood, but from testing so far the comment seems to be telling the truth. > >> + } >> obj->efile.idlist = data; >> obj->efile.idlist_shndx = idx; >> obj->efile.idlist_addr = sh.sh_addr; >> - } else if (!strcmp(name, BTF_BASE_ELF_SEC)) { >> - /* If a .BTF.base section is found, do not resolve >> - * BTF ids relative to vmlinux; resolve relative >> - * to the .BTF.base section instead. btf__parse_split() >> - * will take care of this once the base BTF it is >> - * passed is NULL. >> - */ >> - obj->base_btf_path = NULL; >> } >> - >> - if (compressed_section_fix(elf, scn, &sh)) >> - return -1; >> } >> >> return 0; >> @@ -552,6 +528,13 @@ static int symbols_collect(struct object *obj) >> return 0; >> } >> >> +static inline bool is_envvar_set(const char *var_name) >> +{ >> + const char *value = getenv(var_name); >> + >> + return value && value[0] != '\0'; >> +} >> + > > leftovers? Yes. I think I should add "fail build on warnings" for resolve_btfids. > >> static int load_btf(struct object *obj) >> { >> struct btf *base_btf = NULL, *btf = NULL; >> @@ -578,6 +561,19 @@ static int load_btf(struct object *obj) >> obj->base_btf = base_btf; >> obj->btf = btf; >> >> + if (obj->base_btf && obj->distill_base) { >> + err = btf__distill_base(obj->btf, &base_btf, &btf); >> + if (err) { >> + pr_err("FAILED to distill base BTF: %s\n", strerror(errno)); >> + goto out_err; >> + } >> + >> + btf__free(obj->btf); >> + btf__free(obj->base_btf); >> + obj->btf = btf; >> + obj->base_btf = base_btf; >> + } >> + >> return 0; >> >> out_err: > > [...] > >> +static int dump_raw_btf_ids(struct object *obj, const char *out_path) >> +{ >> + Elf_Data *data = obj->efile.idlist; >> + int fd, err; >> + >> + if (!data || !data->d_buf) { >> + pr_debug("%s has no BTF_ids data to dump\n", obj->path); >> + return 0; >> + } >> + >> + /* >> + * If target endianness differs from host, we need to bswap32 the >> + * .BTF_ids section data before dumping so that the output is in >> + * target endianness. >> + */ >> + if (obj->efile.encoding != ELFDATANATIVE) { >> + pr_debug("bswap_32 .BTF_ids data from host to target endianness\n"); >> + bswap_32_data(data->d_buf, data->d_size); > > same about modifying ELF data in-place for what is supposed to be read-only use See above. > >> + } >> + >> + err = dump_raw_data(out_path, data->d_buf, data->d_size); >> + if (err) >> + return -1; >> + >> + return 0; >> +} >> + >> +static int dump_raw_btf(struct btf *btf, const char *out_path) >> +{ >> + const void *raw_btf_data; >> + u32 raw_btf_size; >> + int fd, err; >> + >> + raw_btf_data = btf__raw_data(btf, &raw_btf_size); >> + if (!raw_btf_data) { >> + pr_err("btf__raw_data() failed\n"); >> + return -1; >> + } > > did you check that libbpf does proper byte swap as well? libbpf is tracking BTF endianness internally, we don't have to worry about it here. If it wasn't, s390x selftests would have almost certainly failed (and they did when I messed up). https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/lib/bpf/btf.c?h=v6.18#n227 > >> + >> + err = dump_raw_data(out_path, raw_btf_data, raw_btf_size); >> + if (err) >> + return -1; >> + >> + return 0; >> +} >> + >> +static inline int make_out_path(char *buf, const char *in_path, const char *suffix) >> +{ >> + int len = snprintf(buf, PATH_MAX, "%s%s", in_path, suffix); > > nit: normally you pass buffer and its size as input arguments instead > of assuming and hard-coding common PATH_MAX constant in two separate > places acked > >> + >> + if (len < 0 || len >= PATH_MAX) { >> + pr_err("Output path is too long: %s%s\n", in_path, suffix); >> + return -E2BIG; >> } >> >> - pr_debug("update %s for %s\n", >> - err >= 0 ? "ok" : "failed", obj->path); >> - return err < 0 ? -1 : 0; >> + return 0; >> } >> >> static const char * const resolve_btfids_usage[] = { > > [...] ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH bpf-next v3 6/6] resolve_btfids: change in-place update with raw binary output 2025-12-05 22:35 ` [PATCH bpf-next v3 6/6] resolve_btfids: change in-place update with raw binary output Ihor Solodrai 2025-12-06 1:13 ` Andrii Nakryiko @ 2025-12-12 7:08 ` Eduard Zingerman 1 sibling, 0 replies; 22+ messages in thread From: Eduard Zingerman @ 2025-12-12 7:08 UTC (permalink / raw) To: Ihor Solodrai, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Andrew Morton, Nathan Chancellor, Nicolas Schier, Tejun Heo, David Vernet, Andrea Righi, Changwoo Min, Shuah Khan, Nick Desaulniers, Bill Wendling, Justin Stitt, Alan Maguire, Donglin Peng Cc: bpf, dwarves, linux-kernel, linux-kbuild On Fri, 2025-12-05 at 14:35 -0800, Ihor Solodrai wrote: > Currently resolve_btfids updates .BTF_ids section of an ELF file > in-place, based on the contents of provided BTF, usually within the > same input file, and optionally a BTF base. > > Change resolve_btfids behavior to enable BTF transformations as part > of its main operation. To achieve this, in-place ELF write in > resolve_btfids is replaced with generation of the following binaries: > * ${1}.BTF with .BTF section data > * ${1}.BTF_ids with .BTF_ids section data if it existed in ${1} > * ${1}.BTF.base with .BTF.base section data for out-of-tree modules > > The execution of resolve_btfids and consumption of its output is > orchestrated by scripts/gen-btf.sh introduced in this patch. > > The motivation for emitting binary data is that it allows simplifying > resolve_btfids implementation by delegating ELF update to the $OBJCOPY > tool [1], which is already widely used across the codebase. > > There are two distinct paths for BTF generation and resolve_btfids > application in the kernel build: for vmlinux and for kernel modules. > > For the vmlinux binary a .BTF section is added in a roundabout way to > ensure correct linking. The patch doesn't change this approach, only > the implementation is a little different. > > Before this patch it worked as follows: > > * pahole consumed .tmp_vmlinux1 [2] and added .BTF section with > llvm-objcopy [3] to it > * then everything except the .BTF section was stripped from .tmp_vmlinux1 > into a .tmp_vmlinux1.bpf.o object [2], later linked into vmlinux > * resolve_btfids was executed later on vmlinux.unstripped [4], > updating it in-place > > After this patch gen-btf.sh implements the following: > > * pahole consumes .tmp_vmlinux1 and produces a *detached* file with > raw BTF data > * resolve_btfids consumes .tmp_vmlinux1 and detached BTF to produce > (potentially modified) .BTF, and .BTF_ids sections data > * a .tmp_vmlinux1.bpf.o object is then produced with objcopy copying > BTF output of resolve_btfids > * .BTF_ids data gets embedded into vmlinux.unstripped in > link-vmlinux.sh by objcopy --update-section > > For kernel modules, creating a special .bpf.o file is not necessary, > and so embedding of sections data produced by resolve_btfids is > straightforward with objcopy. > > With this patch an ELF file becomes effectively read-only within > resolve_btfids, which allows deleting elf_update() call and satellite > code (like compressed_section_fix [5]). > > Endianness handling of .BTF_ids data is also changed. Previously the > "flags" part of the section was bswapped in sets_patch() [6], and then > Elf_Type was modified before elf_update() to signal to libelf that > bswap may be necessary. With this patch we explicitly bswap entire > data buffer on load and on dump. > > [1] https://lore.kernel.org/bpf/131b4190-9c49-4f79-a99d-c00fac97fa44@linux.dev/ > [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/link-vmlinux.sh?h=v6.18#n110 > [3] https://git.kernel.org/pub/scm/devel/pahole/pahole.git/tree/btf_encoder.c?h=v1.31#n1803 > [4] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/link-vmlinux.sh?h=v6.18#n284 > [5] https://lore.kernel.org/bpf/20200819092342.259004-1-jolsa@kernel.org/ > [6] https://lore.kernel.org/bpf/cover.1707223196.git.vmalik@redhat.com/ > > Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev> > --- Acked-by: Eduard Zingerman <eddyz87@gmail.com> > @@ -552,6 +528,13 @@ static int symbols_collect(struct object *obj) > return 0; > } > > +static inline bool is_envvar_set(const char *var_name) > +{ > + const char *value = getenv(var_name); > + > + return value && value[0] != '\0'; > +} > + This is a leftover, not used anywhere. [...] > @@ -860,23 +913,34 @@ int main(int argc, const char **argv) > */ > if (obj.efile.idlist_shndx == -1 || > obj.efile.symbols_shndx == -1) { > - pr_debug("Cannot find .BTF_ids or symbols sections, nothing to do\n"); > - err = 0; > - goto out; > + pr_debug("Cannot find .BTF_ids or symbols sections, skip symbols resolution\n"); > + goto dump_btf; > } > > if (symbols_collect(&obj)) > goto out; > > - if (load_btf(&obj)) > - goto out; > - > if (symbols_resolve(&obj)) > goto out; > > if (symbols_patch(&obj)) > goto out; > > + err = make_out_path(out_path, obj.path, BTF_IDS_SECTION); > + if (err || dump_raw_btf_ids(&obj, out_path)) > + goto out; > + > +dump_btf: > + err = make_out_path(out_path, obj.path, BTF_ELF_SEC); > + if (err || dump_raw_btf(obj.btf, out_path)) Nit: 'err' is not set if dump_raw_btf() errors out. Maybe use: err = make_out_path(out_path, obj.path, BTF_ELF_SEC); err = err ?: dump_raw_btf(obj.btf, out_path); if (err) goto out; ? > + goto out; > + > + if (obj.base_btf && obj.distill_base) { > + err = make_out_path(out_path, obj.path, BTF_BASE_ELF_SEC); > + if (err || dump_raw_btf(obj.base_btf, out_path)) > + goto out; > + } > + > if (!(fatal_warnings && warnings)) > err = 0; > out: [...] ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2025-12-12 17:27 UTC | newest] Thread overview: 22+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-12-05 22:30 [PATCH bpf-next v3 0/6] resolve_btfids: Support for BTF modifications Ihor Solodrai 2025-12-05 22:30 ` [PATCH bpf-next v3 1/6] resolve_btfids: Rename object btf field to btf_path Ihor Solodrai 2025-12-12 7:10 ` Eduard Zingerman 2025-12-05 22:30 ` [PATCH bpf-next v3 2/6] resolve_btfids: Factor out load_btf() Ihor Solodrai 2025-12-05 22:57 ` bot+bpf-ci 2025-12-05 23:12 ` Ihor Solodrai 2025-12-05 23:18 ` Chris Mason 2025-12-12 7:10 ` Eduard Zingerman 2025-12-05 22:30 ` [PATCH bpf-next v3 3/6] resolve_btfids: Introduce enum btf_id_kind Ihor Solodrai 2025-12-12 7:09 ` Eduard Zingerman 2025-12-05 22:30 ` [PATCH bpf-next v3 4/6] lib/Kconfig.debug: Set the minimum required pahole version to v1.22 Ihor Solodrai 2025-12-05 22:49 ` bot+bpf-ci 2025-12-05 22:51 ` Ihor Solodrai 2025-12-06 0:32 ` Andrii Nakryiko 2025-12-06 0:58 ` Ihor Solodrai 2025-12-12 7:09 ` Eduard Zingerman 2025-12-12 17:26 ` Alan Maguire 2025-12-05 22:30 ` [PATCH bpf-next v3 5/6] selftests/bpf: Run resolve_btfids only for relevant .test.o objects Ihor Solodrai 2025-12-05 22:35 ` [PATCH bpf-next v3 6/6] resolve_btfids: change in-place update with raw binary output Ihor Solodrai 2025-12-06 1:13 ` Andrii Nakryiko 2025-12-06 1:37 ` Ihor Solodrai 2025-12-12 7:08 ` Eduard Zingerman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).