* [PATCH v2 bpf-next 0/2] Multi-split BTF fixes and test
@ 2025-10-28 15:57 Alan Maguire
2025-10-28 15:57 ` [PATCH v2 bpf-next 1/2] libbpf: Fix parsing of multi-split BTF Alan Maguire
2025-10-28 15:57 ` [PATCH v2 bpf-next 2/2] selftests/bpf: Test parsing of (multi-)split BTF Alan Maguire
0 siblings, 2 replies; 8+ messages in thread
From: Alan Maguire @ 2025-10-28 15:57 UTC (permalink / raw)
To: andrii
Cc: eddyz87, ast, daniel, martin.lau, acme, ttreyer, yonghong.song,
song, john.fastabend, kpsingh, sdf, haoluo, jolsa, ihor.solodrai,
bpf, Alan Maguire
This small series consists of a fix to multi-split BTF parsing
(patch 1) and a test which exercises (multi-)split BTF parsing
(patch 2).
Changes since v1 [1]
- BPF code-review bot spotted another place that the string offset
needed to be adjusted based upon base start string offset + header
string offset.
- added selftests to extend split BTF testing to parsing
[1] https://lore.kernel.org/bpf/20251023142812.258870-1-alan.maguire@oracle.com/
Alan Maguire (2):
libbpf: Fix parsing of multi-split BTF
selftests/bpf: Test parsing of (multi-)split BTF
tools/lib/bpf/btf.c | 4 +-
.../selftests/bpf/prog_tests/btf_split.c | 71 ++++++++++++++++++-
2 files changed, 72 insertions(+), 3 deletions(-)
--
2.39.3
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 bpf-next 1/2] libbpf: Fix parsing of multi-split BTF
2025-10-28 15:57 [PATCH v2 bpf-next 0/2] Multi-split BTF fixes and test Alan Maguire
@ 2025-10-28 15:57 ` Alan Maguire
2025-10-28 20:01 ` Andrii Nakryiko
2025-10-28 15:57 ` [PATCH v2 bpf-next 2/2] selftests/bpf: Test parsing of (multi-)split BTF Alan Maguire
1 sibling, 1 reply; 8+ messages in thread
From: Alan Maguire @ 2025-10-28 15:57 UTC (permalink / raw)
To: andrii
Cc: eddyz87, ast, daniel, martin.lau, acme, ttreyer, yonghong.song,
song, john.fastabend, kpsingh, sdf, haoluo, jolsa, ihor.solodrai,
bpf, Alan Maguire
When creating multi-split BTF we correctly set the start string offset
to be the size of the base string section plus the base BTF start
string offset; the latter is needed for multi-split BTF since the
offset is non-zero there.
Unfortunately the BTF parsing case needed that logic and it was
missed.
Fixes: 4e29128a9ace ("libbpf/btf: Fix string handling to support
multi-split BTF")
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
tools/lib/bpf/btf.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index 18907f0fcf9f..9f141395c074 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -1061,7 +1061,7 @@ static struct btf *btf_new(const void *data, __u32 size, struct btf *base_btf, b
if (base_btf) {
btf->base_btf = base_btf;
btf->start_id = btf__type_cnt(base_btf);
- btf->start_str_off = base_btf->hdr->str_len;
+ btf->start_str_off = base_btf->hdr->str_len + base_btf->start_str_off;
}
if (is_mmap) {
@@ -5818,7 +5818,7 @@ void btf_set_base_btf(struct btf *btf, const struct btf *base_btf)
{
btf->base_btf = (struct btf *)base_btf;
btf->start_id = btf__type_cnt(base_btf);
- btf->start_str_off = base_btf->hdr->str_len;
+ btf->start_str_off = base_btf->hdr->str_len + base_btf->start_str_off;
}
int btf__relocate(struct btf *btf, const struct btf *base_btf)
--
2.39.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 bpf-next 2/2] selftests/bpf: Test parsing of (multi-)split BTF
2025-10-28 15:57 [PATCH v2 bpf-next 0/2] Multi-split BTF fixes and test Alan Maguire
2025-10-28 15:57 ` [PATCH v2 bpf-next 1/2] libbpf: Fix parsing of multi-split BTF Alan Maguire
@ 2025-10-28 15:57 ` Alan Maguire
2025-10-28 16:45 ` bot+bpf-ci
1 sibling, 1 reply; 8+ messages in thread
From: Alan Maguire @ 2025-10-28 15:57 UTC (permalink / raw)
To: andrii
Cc: eddyz87, ast, daniel, martin.lau, acme, ttreyer, yonghong.song,
song, john.fastabend, kpsingh, sdf, haoluo, jolsa, ihor.solodrai,
bpf, Alan Maguire
Write raw BTF to files, parse it and compare to original;
this allows us to test parsing of (multi-)split BTF code.
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
.../selftests/bpf/prog_tests/btf_split.c | 71 ++++++++++++++++++-
1 file changed, 70 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/btf_split.c b/tools/testing/selftests/bpf/prog_tests/btf_split.c
index 3696fb9a05ed..ee1481c5fe27 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf_split.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf_split.c
@@ -12,11 +12,43 @@ static void btf_dump_printf(void *ctx, const char *fmt, va_list args)
vfprintf(ctx, fmt, args);
}
+static int btf_raw_write(struct btf *btf, char *file)
+{
+ ssize_t written = 0;
+ const void *data;
+ __u32 size = 0;
+ int fd, ret;
+
+ fd = mkstemp(file);
+ if (!ASSERT_GE(fd, 0, "create_file"))
+ return -errno;
+
+ data = btf__raw_data(btf, &size);
+ if (!ASSERT_OK_PTR(data, "btf__raw_data")) {
+ close(fd);
+ return -EINVAL;
+ }
+ while (written < size) {
+ ret = write(fd, data + written, size - written);
+ if (!ASSERT_GE(ret, 0, "write succeeded")) {
+ close(fd);
+ return -errno;
+ }
+ written += ret;
+ }
+ close(fd);
+ return 0;
+}
+
static void __test_btf_split(bool multi)
{
+ char multisplit_btf_file[] = "/tmp/test_btf_multisplit.XXXXXX";
+ char split_btf_file[] = "/tmp/test_btf_split.XXXXXX";
+ char base_btf_file[] = "/tmp/test_btf_base.XXXXXX";
struct btf_dump *d = NULL;
- const struct btf_type *t;
+ const struct btf_type *t, *ot;
struct btf *btf1, *btf2, *btf3 = NULL;
+ struct btf *btf4, *btf5, *btf6 = NULL;
int str_off, i, err;
btf1 = btf__new_empty();
@@ -123,6 +155,35 @@ static void __test_btf_split(bool multi)
" int uf2;\n"
"};\n\n", "c_dump");
+ /* write base, split BTFs to files and ensure parsing succeeds */
+ if (btf_raw_write(btf1, base_btf_file) != 0)
+ goto cleanup;
+ if (btf_raw_write(btf2, split_btf_file) != 0)
+ goto cleanup;
+ btf4 = btf__parse(base_btf_file, NULL);
+ if (!ASSERT_OK_PTR(btf4, "parse_base"))
+ goto cleanup;
+ btf5 = btf__parse_split(split_btf_file, btf4);
+ if (!ASSERT_OK_PTR(btf5, "parse_split"))
+ goto cleanup;
+ if (multi) {
+ if (btf_raw_write(btf3, multisplit_btf_file) != 0)
+ goto cleanup;
+ btf6 = btf__parse_split(multisplit_btf_file, btf5);
+ if (!ASSERT_OK_PTR(btf5, "parse_multisplit"))
+ goto cleanup;
+ } else {
+ btf6 = btf5;
+ }
+
+ /* compare parsed to original BTF */
+ for (i = 1; i < btf__type_cnt(btf6); i++) {
+ t = btf__type_by_id(btf6, i);
+ ot = btf__type_by_id(btf3, i);
+ if (!ASSERT_EQ(memcmp(t, ot, sizeof(*ot)), 0, "cmp_parsed_orig_btf"))
+ goto cleanup;
+ }
+
cleanup:
if (dump_buf_file)
fclose(dump_buf_file);
@@ -132,6 +193,14 @@ static void __test_btf_split(bool multi)
btf__free(btf2);
if (btf2 != btf3)
btf__free(btf3);
+ btf__free(btf4);
+ btf__free(btf5);
+ if (btf5 != btf6)
+ btf__free(btf6);
+ unlink(base_btf_file);
+ unlink(split_btf_file);
+ if (multi)
+ unlink(multisplit_btf_file);
}
void test_btf_split(void)
--
2.39.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 bpf-next 2/2] selftests/bpf: Test parsing of (multi-)split BTF
2025-10-28 15:57 ` [PATCH v2 bpf-next 2/2] selftests/bpf: Test parsing of (multi-)split BTF Alan Maguire
@ 2025-10-28 16:45 ` bot+bpf-ci
2025-10-28 20:01 ` Andrii Nakryiko
0 siblings, 1 reply; 8+ messages in thread
From: bot+bpf-ci @ 2025-10-28 16:45 UTC (permalink / raw)
To: alan.maguire, andrii
Cc: eddyz87, ast, daniel, martin.lau, acme, ttreyer, yonghong.song,
song, john.fastabend, kpsingh, sdf, haoluo, jolsa, ihor.solodrai,
bpf, alan.maguire, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, clm, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 2082 bytes --]
```
commit 8509f4500fb2dfaec7a383a802dffaf84995c434
Author: Alan Maguire <alan.maguire@oracle.com>
selftests/bpf: Test parsing of (multi-)split BTF
This commit adds test coverage for parsing split BTF data that has been
written to files. It writes raw BTF data to temporary files, parses them
back, and compares the parsed results to the original in-memory BTF.
> diff --git a/tools/testing/selftests/bpf/prog_tests/btf_split.c b/tools/testing/selftests/bpf/prog_tests/btf_split.c
> index 3696fb9a0..ee1481c5f 100644
> --- a/tools/testing/selftests/bpf/prog_tests/btf_split.c
> +++ b/tools/testing/selftests/bpf/prog_tests/btf_split.c
[ ... ]
> @@ -123,6 +155,35 @@ static void __test_btf_split(bool multi)
> " int uf2;\n"
> "};\n\n", "c_dump");
>
> + /* write base, split BTFs to files and ensure parsing succeeds */
> + if (btf_raw_write(btf1, base_btf_file) != 0)
> + goto cleanup;
> + if (btf_raw_write(btf2, split_btf_file) != 0)
> + goto cleanup;
> + btf4 = btf__parse(base_btf_file, NULL);
> + if (!ASSERT_OK_PTR(btf4, "parse_base"))
> + goto cleanup;
> + btf5 = btf__parse_split(split_btf_file, btf4);
> + if (!ASSERT_OK_PTR(btf5, "parse_split"))
> + goto cleanup;
> + if (multi) {
> + if (btf_raw_write(btf3, multisplit_btf_file) != 0)
> + goto cleanup;
> + btf6 = btf__parse_split(multisplit_btf_file, btf5);
> + if (!ASSERT_OK_PTR(btf5, "parse_multisplit"))
^^^^^
Should this check btf6 instead of btf5? The line above assigns to btf6,
but this assertion checks btf5 which was already validated earlier.
Following the pattern from the btf4 and btf5 checks above, it looks like
this should be checking the newly assigned btf6 pointer.
> + goto cleanup;
> + } else {
> + btf6 = btf5;
> + }
[ ... ]
```
---
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
In-Reply-To-Subject: `selftests/bpf: Test parsing of (multi-)split BTF`
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/18881352510
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 bpf-next 2/2] selftests/bpf: Test parsing of (multi-)split BTF
2025-10-28 16:45 ` bot+bpf-ci
@ 2025-10-28 20:01 ` Andrii Nakryiko
2025-10-28 20:11 ` Andrii Nakryiko
0 siblings, 1 reply; 8+ messages in thread
From: Andrii Nakryiko @ 2025-10-28 20:01 UTC (permalink / raw)
To: bot+bpf-ci
Cc: alan.maguire, andrii, eddyz87, ast, daniel, martin.lau, acme,
ttreyer, yonghong.song, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa, ihor.solodrai, bpf, martin.lau, clm
On Tue, Oct 28, 2025 at 9:45 AM <bot+bpf-ci@kernel.org> wrote:
>
>
> ```
> commit 8509f4500fb2dfaec7a383a802dffaf84995c434
> Author: Alan Maguire <alan.maguire@oracle.com>
>
> selftests/bpf: Test parsing of (multi-)split BTF
>
> This commit adds test coverage for parsing split BTF data that has been
> written to files. It writes raw BTF data to temporary files, parses them
> back, and compares the parsed results to the original in-memory BTF.
>
> > diff --git a/tools/testing/selftests/bpf/prog_tests/btf_split.c b/tools/testing/selftests/bpf/prog_tests/btf_split.c
> > index 3696fb9a0..ee1481c5f 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/btf_split.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/btf_split.c
>
> [ ... ]
>
> > @@ -123,6 +155,35 @@ static void __test_btf_split(bool multi)
> > " int uf2;\n"
> > "};\n\n", "c_dump");
> >
> > + /* write base, split BTFs to files and ensure parsing succeeds */
> > + if (btf_raw_write(btf1, base_btf_file) != 0)
> > + goto cleanup;
> > + if (btf_raw_write(btf2, split_btf_file) != 0)
> > + goto cleanup;
> > + btf4 = btf__parse(base_btf_file, NULL);
> > + if (!ASSERT_OK_PTR(btf4, "parse_base"))
> > + goto cleanup;
> > + btf5 = btf__parse_split(split_btf_file, btf4);
> > + if (!ASSERT_OK_PTR(btf5, "parse_split"))
> > + goto cleanup;
> > + if (multi) {
> > + if (btf_raw_write(btf3, multisplit_btf_file) != 0)
> > + goto cleanup;
> > + btf6 = btf__parse_split(multisplit_btf_file, btf5);
> > + if (!ASSERT_OK_PTR(btf5, "parse_multisplit"))
> ^^^^^
>
> Should this check btf6 instead of btf5? The line above assigns to btf6,
> but this assertion checks btf5 which was already validated earlier.
> Following the pattern from the btf4 and btf5 checks above, it looks like
> this should be checking the newly assigned btf6 pointer.
>
AI has a good point, I fixed it up while applying.
Alan, while fixing this up, I noticed that __test_btf_split clean up
path can dereference a bunch of uninitialized btfs (btf4 and btf5).
Also, we can goto cleanup before we create
{base,split,multisplit}_btf_file, so it would be nice if you could
send a follow up fixing all that. Thanks!
> > + goto cleanup;
> > + } else {
> > + btf6 = btf5;
> > + }
>
> [ ... ]
>
>
> ```
>
> ---
> 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
>
> In-Reply-To-Subject: `selftests/bpf: Test parsing of (multi-)split BTF`
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/18881352510
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 bpf-next 1/2] libbpf: Fix parsing of multi-split BTF
2025-10-28 15:57 ` [PATCH v2 bpf-next 1/2] libbpf: Fix parsing of multi-split BTF Alan Maguire
@ 2025-10-28 20:01 ` Andrii Nakryiko
0 siblings, 0 replies; 8+ messages in thread
From: Andrii Nakryiko @ 2025-10-28 20:01 UTC (permalink / raw)
To: Alan Maguire
Cc: andrii, eddyz87, ast, daniel, martin.lau, acme, ttreyer,
yonghong.song, song, john.fastabend, kpsingh, sdf, haoluo, jolsa,
ihor.solodrai, bpf
On Tue, Oct 28, 2025 at 8:57 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> When creating multi-split BTF we correctly set the start string offset
> to be the size of the base string section plus the base BTF start
> string offset; the latter is needed for multi-split BTF since the
> offset is non-zero there.
>
> Unfortunately the BTF parsing case needed that logic and it was
> missed.
>
> Fixes: 4e29128a9ace ("libbpf/btf: Fix string handling to support
> multi-split BTF")
please make sure commit references are not wrapped when used as part
of Fixes: tag, I had to manually fix it
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---
> tools/lib/bpf/btf.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> index 18907f0fcf9f..9f141395c074 100644
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c
> @@ -1061,7 +1061,7 @@ static struct btf *btf_new(const void *data, __u32 size, struct btf *base_btf, b
> if (base_btf) {
> btf->base_btf = base_btf;
> btf->start_id = btf__type_cnt(base_btf);
> - btf->start_str_off = base_btf->hdr->str_len;
> + btf->start_str_off = base_btf->hdr->str_len + base_btf->start_str_off;
> }
>
> if (is_mmap) {
> @@ -5818,7 +5818,7 @@ void btf_set_base_btf(struct btf *btf, const struct btf *base_btf)
> {
> btf->base_btf = (struct btf *)base_btf;
> btf->start_id = btf__type_cnt(base_btf);
> - btf->start_str_off = base_btf->hdr->str_len;
> + btf->start_str_off = base_btf->hdr->str_len + base_btf->start_str_off;
> }
>
> int btf__relocate(struct btf *btf, const struct btf *base_btf)
> --
> 2.39.3
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 bpf-next 2/2] selftests/bpf: Test parsing of (multi-)split BTF
2025-10-28 20:01 ` Andrii Nakryiko
@ 2025-10-28 20:11 ` Andrii Nakryiko
2025-10-28 22:58 ` Alan Maguire
0 siblings, 1 reply; 8+ messages in thread
From: Andrii Nakryiko @ 2025-10-28 20:11 UTC (permalink / raw)
To: bot+bpf-ci
Cc: alan.maguire, andrii, eddyz87, ast, daniel, martin.lau, acme,
ttreyer, yonghong.song, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa, ihor.solodrai, bpf, martin.lau, clm
On Tue, Oct 28, 2025 at 1:01 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Tue, Oct 28, 2025 at 9:45 AM <bot+bpf-ci@kernel.org> wrote:
> >
> >
> > ```
> > commit 8509f4500fb2dfaec7a383a802dffaf84995c434
> > Author: Alan Maguire <alan.maguire@oracle.com>
> >
> > selftests/bpf: Test parsing of (multi-)split BTF
> >
> > This commit adds test coverage for parsing split BTF data that has been
> > written to files. It writes raw BTF data to temporary files, parses them
> > back, and compares the parsed results to the original in-memory BTF.
> >
> > > diff --git a/tools/testing/selftests/bpf/prog_tests/btf_split.c b/tools/testing/selftests/bpf/prog_tests/btf_split.c
> > > index 3696fb9a0..ee1481c5f 100644
> > > --- a/tools/testing/selftests/bpf/prog_tests/btf_split.c
> > > +++ b/tools/testing/selftests/bpf/prog_tests/btf_split.c
> >
> > [ ... ]
> >
> > > @@ -123,6 +155,35 @@ static void __test_btf_split(bool multi)
> > > " int uf2;\n"
> > > "};\n\n", "c_dump");
> > >
> > > + /* write base, split BTFs to files and ensure parsing succeeds */
> > > + if (btf_raw_write(btf1, base_btf_file) != 0)
> > > + goto cleanup;
> > > + if (btf_raw_write(btf2, split_btf_file) != 0)
> > > + goto cleanup;
> > > + btf4 = btf__parse(base_btf_file, NULL);
> > > + if (!ASSERT_OK_PTR(btf4, "parse_base"))
> > > + goto cleanup;
> > > + btf5 = btf__parse_split(split_btf_file, btf4);
> > > + if (!ASSERT_OK_PTR(btf5, "parse_split"))
> > > + goto cleanup;
> > > + if (multi) {
> > > + if (btf_raw_write(btf3, multisplit_btf_file) != 0)
> > > + goto cleanup;
> > > + btf6 = btf__parse_split(multisplit_btf_file, btf5);
> > > + if (!ASSERT_OK_PTR(btf5, "parse_multisplit"))
> > ^^^^^
> >
> > Should this check btf6 instead of btf5? The line above assigns to btf6,
> > but this assertion checks btf5 which was already validated earlier.
> > Following the pattern from the btf4 and btf5 checks above, it looks like
> > this should be checking the newly assigned btf6 pointer.
> >
>
> AI has a good point, I fixed it up while applying.
>
>
> Alan, while fixing this up, I noticed that __test_btf_split clean up
> path can dereference a bunch of uninitialized btfs (btf4 and btf5).
> Also, we can goto cleanup before we create
> {base,split,multisplit}_btf_file, so it would be nice if you could
> send a follow up fixing all that. Thanks!
Ok, so BPF CI noticed this as well. I ended up not pushing, please fix
the clean up path (unlink is pre-existing bug, but it doesn't cause
compiler to complain)
>
> > > + goto cleanup;
> > > + } else {
> > > + btf6 = btf5;
> > > + }
> >
> > [ ... ]
> >
> >
> > ```
> >
> > ---
> > 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
> >
> > In-Reply-To-Subject: `selftests/bpf: Test parsing of (multi-)split BTF`
> > CI run summary: https://github.com/kernel-patches/bpf/actions/runs/18881352510
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 bpf-next 2/2] selftests/bpf: Test parsing of (multi-)split BTF
2025-10-28 20:11 ` Andrii Nakryiko
@ 2025-10-28 22:58 ` Alan Maguire
0 siblings, 0 replies; 8+ messages in thread
From: Alan Maguire @ 2025-10-28 22:58 UTC (permalink / raw)
To: Andrii Nakryiko, bot+bpf-ci
Cc: andrii, eddyz87, ast, daniel, martin.lau, acme, ttreyer,
yonghong.song, song, john.fastabend, kpsingh, sdf, haoluo, jolsa,
ihor.solodrai, bpf, martin.lau, clm
On 28/10/2025 20:11, Andrii Nakryiko wrote:
> On Tue, Oct 28, 2025 at 1:01 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
>>
>> On Tue, Oct 28, 2025 at 9:45 AM <bot+bpf-ci@kernel.org> wrote:
>>>
>>>
>>> ```
>>> commit 8509f4500fb2dfaec7a383a802dffaf84995c434
>>> Author: Alan Maguire <alan.maguire@oracle.com>
>>>
>>> selftests/bpf: Test parsing of (multi-)split BTF
>>>
>>> This commit adds test coverage for parsing split BTF data that has been
>>> written to files. It writes raw BTF data to temporary files, parses them
>>> back, and compares the parsed results to the original in-memory BTF.
>>>
>>>> diff --git a/tools/testing/selftests/bpf/prog_tests/btf_split.c b/tools/testing/selftests/bpf/prog_tests/btf_split.c
>>>> index 3696fb9a0..ee1481c5f 100644
>>>> --- a/tools/testing/selftests/bpf/prog_tests/btf_split.c
>>>> +++ b/tools/testing/selftests/bpf/prog_tests/btf_split.c
>>>
>>> [ ... ]
>>>
>>>> @@ -123,6 +155,35 @@ static void __test_btf_split(bool multi)
>>>> " int uf2;\n"
>>>> "};\n\n", "c_dump");
>>>>
>>>> + /* write base, split BTFs to files and ensure parsing succeeds */
>>>> + if (btf_raw_write(btf1, base_btf_file) != 0)
>>>> + goto cleanup;
>>>> + if (btf_raw_write(btf2, split_btf_file) != 0)
>>>> + goto cleanup;
>>>> + btf4 = btf__parse(base_btf_file, NULL);
>>>> + if (!ASSERT_OK_PTR(btf4, "parse_base"))
>>>> + goto cleanup;
>>>> + btf5 = btf__parse_split(split_btf_file, btf4);
>>>> + if (!ASSERT_OK_PTR(btf5, "parse_split"))
>>>> + goto cleanup;
>>>> + if (multi) {
>>>> + if (btf_raw_write(btf3, multisplit_btf_file) != 0)
>>>> + goto cleanup;
>>>> + btf6 = btf__parse_split(multisplit_btf_file, btf5);
>>>> + if (!ASSERT_OK_PTR(btf5, "parse_multisplit"))
>>> ^^^^^
>>>
>>> Should this check btf6 instead of btf5? The line above assigns to btf6,
>>> but this assertion checks btf5 which was already validated earlier.
>>> Following the pattern from the btf4 and btf5 checks above, it looks like
>>> this should be checking the newly assigned btf6 pointer.
>>>
>>
>> AI has a good point, I fixed it up while applying.
>>
>>
>> Alan, while fixing this up, I noticed that __test_btf_split clean up
>> path can dereference a bunch of uninitialized btfs (btf4 and btf5).
>> Also, we can goto cleanup before we create
>> {base,split,multisplit}_btf_file, so it would be nice if you could
>> send a follow up fixing all that. Thanks!
>
> Ok, so BPF CI noticed this as well. I ended up not pushing, please fix
> the clean up path (unlink is pre-existing bug, but it doesn't cause
> compiler to complain)
>
sure, I've sent v3 which I think has all the cleanups needed now, along
with fixed Fixes: tag and ASSERT_OK_PTR() on right btf. Thanks!
Alan
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-10-28 22:59 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-28 15:57 [PATCH v2 bpf-next 0/2] Multi-split BTF fixes and test Alan Maguire
2025-10-28 15:57 ` [PATCH v2 bpf-next 1/2] libbpf: Fix parsing of multi-split BTF Alan Maguire
2025-10-28 20:01 ` Andrii Nakryiko
2025-10-28 15:57 ` [PATCH v2 bpf-next 2/2] selftests/bpf: Test parsing of (multi-)split BTF Alan Maguire
2025-10-28 16:45 ` bot+bpf-ci
2025-10-28 20:01 ` Andrii Nakryiko
2025-10-28 20:11 ` Andrii Nakryiko
2025-10-28 22:58 ` Alan Maguire
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox