[PATCH dwarves] btf_encoder: always initialize func_state to 0

[PATCH dwarves] btf_encoder: always initialize func_state to 0

[Ihor Solodrai]

BPF CI caught a segfault on aarch64 and s390x [1] after recent merges
into the master branch.

The segfault happened at free(func_state->annots) in
btf_encoder__delete_saved_funcs().

func_state->annots arrived there uninitialized because after patch [2]
in some cases func_state may be allocated with a realloc, but was not
zeroed out.

Fix this bug by always memset-ing a func_state to zero in
btf_encoder__alloc_func_state().

[1] https://github.com/kernel-patches/bpf/actions/runs/12700574327
[2] https://lore.kernel.org/dwarves/20250109185950.653110-11-ihor.solodrai@pm.me/
---
 btf_encoder.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/btf_encoder.c b/btf_encoder.c
index 78efd70..511c1ea 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -1083,7 +1083,7 @@ static bool funcs__match(struct btf_encoder_func_state *s1,
 
 static struct btf_encoder_func_state *btf_encoder__alloc_func_state(struct btf_encoder *encoder)
 {
-	struct btf_encoder_func_state *tmp;
+	struct btf_encoder_func_state *state, *tmp;
 
 	if (encoder->func_states.cnt >= encoder->func_states.cap) {
 
@@ -1100,7 +1100,10 @@ static struct btf_encoder_func_state *btf_encoder__alloc_func_state(struct btf_e
 		encoder->func_states.array = tmp;
 	}
 
-	return &encoder->func_states.array[encoder->func_states.cnt++];
+	state = &encoder->func_states.array[encoder->func_states.cnt++];
+	memset(state, 0, sizeof(*state));
+
+	return state;
 }
 
 static int32_t btf_encoder__save_func(struct btf_encoder *encoder, struct function *fn, struct elf_function *func)
-- 
2.47.1

Re: [PATCH dwarves] btf_encoder: always initialize func_state to 0

[Alan Maguire]

On 10/01/2025 02:31, Ihor Solodrai wrote:
> BPF CI caught a segfault on aarch64 and s390x [1] after recent merges
> into the master branch.
> 
> The segfault happened at free(func_state->annots) in
> btf_encoder__delete_saved_funcs().
> 
> func_state->annots arrived there uninitialized because after patch [2]
> in some cases func_state may be allocated with a realloc, but was not
> zeroed out.
> 
> Fix this bug by always memset-ing a func_state to zero in
> btf_encoder__alloc_func_state().
> 
> [1] https://github.com/kernel-patches/bpf/actions/runs/12700574327
> [2] https://lore.kernel.org/dwarves/20250109185950.653110-11-ihor.solodrai@pm.me/


Thanks for the quick fix! Reproduced this on an aarch64 system:

 BTF [M] kernel/resource_kunit.ko
/bin/sh: line 1: 630875 Segmentation fault      (core dumped)
LLVM_OBJCOPY="objcopy" pahole -J -j
--btf_features=encode_force,var,float,enum64,decl_tag,type_tag,optimized_func,consistent_func,decl_tag_kfuncs
--lang_exclude=rust --btf_base ./vmlinux kernel/kcsan/kcsan_test.ko
make[2]: *** [scripts/Makefile.modfinal:57: kernel/kcsan/kcsan_test.ko]
Error 139
make[2]: *** Deleting file 'kernel/kcsan/kcsan_test.ko'
make[2]: *** Waiting for unfinished jobs....
/bin/sh: line 1: 630907 Segmentation fault      (core dumped)
LLVM_OBJCOPY="objcopy" pahole -J -j
--btf_features=encode_force,var,float,enum64,decl_tag,type_tag,optimized_func,consistent_func,decl_tag_kfuncs
--lang_exclude=rust --btf_base ./vmlinux kernel/torture.ko
make[2]: *** [scripts/Makefile.modfinal:56: kernel/torture.ko] Error 139
make[2]: *** Deleting file 'kernel/torture.ko'

...and verified that with the fix all works well.

Nit: missing Signed-off-by

Tested-by: Alan Maguire <alan.maguire@oracle.com>

> ---
>  btf_encoder.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/btf_encoder.c b/btf_encoder.c
> index 78efd70..511c1ea 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -1083,7 +1083,7 @@ static bool funcs__match(struct btf_encoder_func_state *s1,
>  
>  static struct btf_encoder_func_state *btf_encoder__alloc_func_state(struct btf_encoder *encoder)
>  {
> -	struct btf_encoder_func_state *tmp;
> +	struct btf_encoder_func_state *state, *tmp;
>  
>  	if (encoder->func_states.cnt >= encoder->func_states.cap) {
>  
> @@ -1100,7 +1100,10 @@ static struct btf_encoder_func_state *btf_encoder__alloc_func_state(struct btf_e
>  		encoder->func_states.array = tmp;
>  	}
>  
> -	return &encoder->func_states.array[encoder->func_states.cnt++];
> +	state = &encoder->func_states.array[encoder->func_states.cnt++];
> +	memset(state, 0, sizeof(*state));
> +
> +	return state;
>  }
>  
>  static int32_t btf_encoder__save_func(struct btf_encoder *encoder, struct function *fn, struct elf_function *func)

Re: [PATCH dwarves] btf_encoder: always initialize func_state to 0

[Arnaldo Carvalho de Melo]

On Fri, Jan 10, 2025 at 10:39:50AM +0000, Alan Maguire wrote:
> On 10/01/2025 02:31, Ihor Solodrai wrote:
> > BPF CI caught a segfault on aarch64 and s390x [1] after recent merges
> > into the master branch.
> > 
> > The segfault happened at free(func_state->annots) in
> > btf_encoder__delete_saved_funcs().
> > 
> > func_state->annots arrived there uninitialized because after patch [2]
> > in some cases func_state may be allocated with a realloc, but was not
> > zeroed out.
> > 
> > Fix this bug by always memset-ing a func_state to zero in
> > btf_encoder__alloc_func_state().
> > 
> > [1] https://github.com/kernel-patches/bpf/actions/runs/12700574327
> > [2] https://lore.kernel.org/dwarves/20250109185950.653110-11-ihor.solodrai@pm.me/
> 
> 
> Thanks for the quick fix! Reproduced this on an aarch64 system:
> 
>  BTF [M] kernel/resource_kunit.ko
> /bin/sh: line 1: 630875 Segmentation fault      (core dumped)
> LLVM_OBJCOPY="objcopy" pahole -J -j
> --btf_features=encode_force,var,float,enum64,decl_tag,type_tag,optimized_func,consistent_func,decl_tag_kfuncs
> --lang_exclude=rust --btf_base ./vmlinux kernel/kcsan/kcsan_test.ko
> make[2]: *** [scripts/Makefile.modfinal:57: kernel/kcsan/kcsan_test.ko]
> Error 139
> make[2]: *** Deleting file 'kernel/kcsan/kcsan_test.ko'
> make[2]: *** Waiting for unfinished jobs....
> /bin/sh: line 1: 630907 Segmentation fault      (core dumped)
> LLVM_OBJCOPY="objcopy" pahole -J -j
> --btf_features=encode_force,var,float,enum64,decl_tag,type_tag,optimized_func,consistent_func,decl_tag_kfuncs
> --lang_exclude=rust --btf_base ./vmlinux kernel/torture.ko
> make[2]: *** [scripts/Makefile.modfinal:56: kernel/torture.ko] Error 139
> make[2]: *** Deleting file 'kernel/torture.ko'
> 
> ...and verified that with the fix all works well.
> 
> Nit: missing Signed-off-by

I'm adding it, ok?

- Arnaldo
 
> Tested-by: Alan Maguire <alan.maguire@oracle.com>

Thanks!
 
> > ---
> >  btf_encoder.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/btf_encoder.c b/btf_encoder.c
> > index 78efd70..511c1ea 100644
> > --- a/btf_encoder.c
> > +++ b/btf_encoder.c
> > @@ -1083,7 +1083,7 @@ static bool funcs__match(struct btf_encoder_func_state *s1,
> >  
> >  static struct btf_encoder_func_state *btf_encoder__alloc_func_state(struct btf_encoder *encoder)
> >  {
> > -	struct btf_encoder_func_state *tmp;
> > +	struct btf_encoder_func_state *state, *tmp;
> >  
> >  	if (encoder->func_states.cnt >= encoder->func_states.cap) {
> >  
> > @@ -1100,7 +1100,10 @@ static struct btf_encoder_func_state *btf_encoder__alloc_func_state(struct btf_e
> >  		encoder->func_states.array = tmp;
> >  	}
> >  
> > -	return &encoder->func_states.array[encoder->func_states.cnt++];
> > +	state = &encoder->func_states.array[encoder->func_states.cnt++];
> > +	memset(state, 0, sizeof(*state));
> > +
> > +	return state;
> >  }
> >  
> >  static int32_t btf_encoder__save_func(struct btf_encoder *encoder, struct function *fn, struct elf_function *func)

Re: [PATCH dwarves] btf_encoder: always initialize func_state to 0

[Ihor Solodrai]

On Friday, January 10th, 2025 at 5:48 AM, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> 
> 
> On Fri, Jan 10, 2025 at 10:39:50AM +0000, Alan Maguire wrote:
> 
> > On 10/01/2025 02:31, Ihor Solodrai wrote:
> > 
> > > BPF CI caught a segfault on aarch64 and s390x [1] after recent merges
> > > into the master branch.
> > > 
> > > The segfault happened at free(func_state->annots) in
> > > btf_encoder__delete_saved_funcs().
> > > 
> > > func_state->annots arrived there uninitialized because after patch [2]
> > > in some cases func_state may be allocated with a realloc, but was not
> > > zeroed out.
> > > 
> > > Fix this bug by always memset-ing a func_state to zero in
> > > btf_encoder__alloc_func_state().
> > > 
> > > [1] https://github.com/kernel-patches/bpf/actions/runs/12700574327
> > > [2] https://lore.kernel.org/dwarves/20250109185950.653110-11-ihor.solodrai@pm.me/
> > 
> > Thanks for the quick fix! Reproduced this on an aarch64 system:
> > 
> > BTF [M] kernel/resource_kunit.ko
> > /bin/sh: line 1: 630875 Segmentation fault (core dumped)
> > LLVM_OBJCOPY="objcopy" pahole -J -j
> > --btf_features=encode_force,var,float,enum64,decl_tag,type_tag,optimized_func,consistent_func,decl_tag_kfuncs
> > --lang_exclude=rust --btf_base ./vmlinux kernel/kcsan/kcsan_test.ko
> > make[2]: *** [scripts/Makefile.modfinal:57: kernel/kcsan/kcsan_test.ko]
> > Error 139
> > make[2]: *** Deleting file 'kernel/kcsan/kcsan_test.ko'
> > make[2]: *** Waiting for unfinished jobs....
> > /bin/sh: line 1: 630907 Segmentation fault (core dumped)
> > LLVM_OBJCOPY="objcopy" pahole -J -j
> > --btf_features=encode_force,var,float,enum64,decl_tag,type_tag,optimized_func,consistent_func,decl_tag_kfuncs
> > --lang_exclude=rust --btf_base ./vmlinux kernel/torture.ko
> > make[2]: *** [scripts/Makefile.modfinal:56: kernel/torture.ko] Error 139
> > make[2]: *** Deleting file 'kernel/torture.ko'
> > 
> > ...and verified that with the fix all works well.
> > 
> > Nit: missing Signed-off-by
> 
> 
> I'm adding it, ok?

Yes, thanks!

Signed-off-by: Ihor Solodrai <ihor.solodrai@pm.me>

> 
> - Arnaldo
> 
> > Tested-by: Alan Maguire alan.maguire@oracle.com
> 
> 
> Thanks!
> 
> > > ---
> > > btf_encoder.c | 7 +++++--
> > > 1 file changed, 5 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/btf_encoder.c b/btf_encoder.c
> > > index 78efd70..511c1ea 100644
> > > --- a/btf_encoder.c
> > > +++ b/btf_encoder.c
> > > @@ -1083,7 +1083,7 @@ static bool funcs__match(struct btf_encoder_func_state *s1,
> > > 
> > > static struct btf_encoder_func_state *btf_encoder__alloc_func_state(struct btf_encoder *encoder)
> > > {
> > > - struct btf_encoder_func_state *tmp;
> > > + struct btf_encoder_func_state *state, *tmp;
> > > 
> > > if (encoder->func_states.cnt >= encoder->func_states.cap) {
> > > 
> > > @@ -1100,7 +1100,10 @@ static struct btf_encoder_func_state *btf_encoder__alloc_func_state(struct btf_e
> > > encoder->func_states.array = tmp;
> > > }
> > > 
> > > - return &encoder->func_states.array[encoder->func_states.cnt++];
> > > + state = &encoder->func_states.array[encoder->func_states.cnt++];
> > > + memset(state, 0, sizeof(*state));
> > > +
> > > + return state;
> > > }
> > > 
> > > static int32_t btf_encoder__save_func(struct btf_encoder *encoder, struct function *fn, struct elf_function *func)

Re: [PATCH dwarves] btf_encoder: always initialize func_state to 0

[Arnaldo Carvalho de Melo]

On Fri, Jan 10, 2025 at 02:31:41AM +0000, Ihor Solodrai wrote:
> BPF CI caught a segfault on aarch64 and s390x [1] after recent merges
> into the master branch.

In the past the libbpf github actions was tracking the tmp.master (it would
be better to track "next") branch and I was looking at when it passed to
then move "next" to master, that would be great to have so that we
wouldn't be having these bugs in the git history, avoiding force pushes.

Anyhway, thanks for the fix, I'll add it and push it out.

- Arnaldo
 
> The segfault happened at free(func_state->annots) in
> btf_encoder__delete_saved_funcs().
> 
> func_state->annots arrived there uninitialized because after patch [2]
> in some cases func_state may be allocated with a realloc, but was not
> zeroed out.
> 
> Fix this bug by always memset-ing a func_state to zero in
> btf_encoder__alloc_func_state().
> 
> [1] https://github.com/kernel-patches/bpf/actions/runs/12700574327
> [2] https://lore.kernel.org/dwarves/20250109185950.653110-11-ihor.solodrai@pm.me/
> ---
>  btf_encoder.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/btf_encoder.c b/btf_encoder.c
> index 78efd70..511c1ea 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -1083,7 +1083,7 @@ static bool funcs__match(struct btf_encoder_func_state *s1,
>  
>  static struct btf_encoder_func_state *btf_encoder__alloc_func_state(struct btf_encoder *encoder)
>  {
> -	struct btf_encoder_func_state *tmp;
> +	struct btf_encoder_func_state *state, *tmp;
>  
>  	if (encoder->func_states.cnt >= encoder->func_states.cap) {
>  
> @@ -1100,7 +1100,10 @@ static struct btf_encoder_func_state *btf_encoder__alloc_func_state(struct btf_e
>  		encoder->func_states.array = tmp;
>  	}
>  
> -	return &encoder->func_states.array[encoder->func_states.cnt++];
> +	state = &encoder->func_states.array[encoder->func_states.cnt++];
> +	memset(state, 0, sizeof(*state));
> +
> +	return state;
>  }
>  
>  static int32_t btf_encoder__save_func(struct btf_encoder *encoder, struct function *fn, struct elf_function *func)
> -- 
> 2.47.1
> 

Re: [PATCH dwarves] btf_encoder: always initialize func_state to 0

[Ihor Solodrai]

On Friday, January 10th, 2025 at 5:51 AM, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> 
> 
> On Fri, Jan 10, 2025 at 02:31:41AM +0000, Ihor Solodrai wrote:
> 
> > BPF CI caught a segfault on aarch64 and s390x [1] after recent merges
> > into the master branch.
> 
> 
> In the past the libbpf github actions was tracking the tmp.master (it would
> be better to track "next") branch and I was looking at when it passed to
> then move "next" to master, that would be great to have so that we
> wouldn't be having these bugs in the git history, avoiding force pushes.

libbpf CI is still tracking tmp.master:

https://github.com/libbpf/libbpf/actions/runs/12696027660/job/35389206487

However it only runs once a day. BPF CI runs more frequently due to the
volume of incoming patches. As of recently, BPF CI has been using "master".
Yesterday, when I saw the failures, I switched BPF CI to v1.28.

I think the right way to approach this is for libbpf/libbpf to track "next",
and BPF CI use "master". Then, most importantly, only merge next into master
after libbpf CI has passed.

This can potentially be automated, but would require push access to the
pahole repo. Until then, a maintainer would need to manually check the
libbpf CI status here:

https://github.com/libbpf/libbpf/actions/workflows/test.yml

Another thing is that libbpf CI only tests x86_64 currently.
We could add aarch64 to libbpf, or migrate pahole staging job to
kernel-patches/vmtest (which is almost identical to BPF CI).

Andrii, what do you think?

> 
> Anyhway, thanks for the fix, I'll add it and push it out.
> 
> - Arnaldo
> 
> > The segfault happened at free(func_state->annots) in
> > btf_encoder__delete_saved_funcs().
> > 
> > func_state->annots arrived there uninitialized because after patch [2]
> > in some cases func_state may be allocated with a realloc, but was not
> > zeroed out.
> > 
> > Fix this bug by always memset-ing a func_state to zero in
> > btf_encoder__alloc_func_state().
> > 
> > [1] https://github.com/kernel-patches/bpf/actions/runs/12700574327
> > [2] https://lore.kernel.org/dwarves/20250109185950.653110-11-ihor.solodrai@pm.me/
> > ---
> > btf_encoder.c | 7 +++++--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/btf_encoder.c b/btf_encoder.c
> > index 78efd70..511c1ea 100644
> > --- a/btf_encoder.c
> > +++ b/btf_encoder.c
> > @@ -1083,7 +1083,7 @@ static bool funcs__match(struct btf_encoder_func_state *s1,
> > 
> > static struct btf_encoder_func_state *btf_encoder__alloc_func_state(struct btf_encoder *encoder)
> > {
> > - struct btf_encoder_func_state *tmp;
> > + struct btf_encoder_func_state *state, *tmp;
> > 
> > if (encoder->func_states.cnt >= encoder->func_states.cap) {
> > 
> > @@ -1100,7 +1100,10 @@ static struct btf_encoder_func_state *btf_encoder__alloc_func_state(struct btf_e
> > encoder->func_states.array = tmp;
> > }
> > 
> > - return &encoder->func_states.array[encoder->func_states.cnt++];
> > + state = &encoder->func_states.array[encoder->func_states.cnt++];
> > + memset(state, 0, sizeof(*state));
> > +
> > + return state;
> > }
> > 
> > static int32_t btf_encoder__save_func(struct btf_encoder *encoder, struct function *fn, struct elf_function *func)
> > --
> > 2.47.1

Re: [PATCH dwarves] btf_encoder: always initialize func_state to 0

[Andrii Nakryiko]

On Fri, Jan 10, 2025 at 7:58 AM Ihor Solodrai <ihor.solodrai@pm.me> wrote:
>
> On Friday, January 10th, 2025 at 5:51 AM, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> >
> >
> > On Fri, Jan 10, 2025 at 02:31:41AM +0000, Ihor Solodrai wrote:
> >
> > > BPF CI caught a segfault on aarch64 and s390x [1] after recent merges
> > > into the master branch.
> >
> >
> > In the past the libbpf github actions was tracking the tmp.master (it would
> > be better to track "next") branch and I was looking at when it passed to
> > then move "next" to master, that would be great to have so that we
> > wouldn't be having these bugs in the git history, avoiding force pushes.
>
> libbpf CI is still tracking tmp.master:
>
> https://github.com/libbpf/libbpf/actions/runs/12696027660/job/35389206487
>
> However it only runs once a day. BPF CI runs more frequently due to the
> volume of incoming patches. As of recently, BPF CI has been using "master".
> Yesterday, when I saw the failures, I switched BPF CI to v1.28.
>
> I think the right way to approach this is for libbpf/libbpf to track "next",
> and BPF CI use "master". Then, most importantly, only merge next into master
> after libbpf CI has passed.
>
> This can potentially be automated, but would require push access to the
> pahole repo. Until then, a maintainer would need to manually check the
> libbpf CI status here:
>
> https://github.com/libbpf/libbpf/actions/workflows/test.yml
>
> Another thing is that libbpf CI only tests x86_64 currently.
> We could add aarch64 to libbpf, or migrate pahole staging job to
> kernel-patches/vmtest (which is almost identical to BPF CI).
>
> Andrii, what do you think?
>

It would be nice to test pahole as soon as any relevant libbpf patch
is sent upstream for review. We should make sure this doesn't happen
for all combinations of compiler/arch/etc, just pick one configuration
for x86-64 and aarch64 and run that?

[...]

Re: [PATCH dwarves] btf_encoder: always initialize func_state to 0

[Ihor Solodrai]

On Friday, January 10th, 2025 at 7:58 AM, Ihor Solodrai <ihor.solodrai@pm.me> wrote:

> 
> On Friday, January 10th, 2025 at 5:51 AM, Arnaldo Carvalho de Melo acme@kernel.org wrote:
> 
> > On Fri, Jan 10, 2025 at 02:31:41AM +0000, Ihor Solodrai wrote:
> > 
> > > BPF CI caught a segfault on aarch64 and s390x [1] after recent merges
> > > into the master branch.
> > 
> > In the past the libbpf github actions was tracking the tmp.master (it would
> > be better to track "next") branch and I was looking at when it passed to
> > then move "next" to master, that would be great to have so that we
> > wouldn't be having these bugs in the git history, avoiding force pushes.
> 
> 
> libbpf CI is still tracking tmp.master:
> 
> https://github.com/libbpf/libbpf/actions/runs/12696027660/job/35389206487
> 
> However it only runs once a day. BPF CI runs more frequently due to the
> volume of incoming patches. As of recently, BPF CI has been using "master".
> Yesterday, when I saw the failures, I switched BPF CI to v1.28.
> 
> I think the right way to approach this is for libbpf/libbpf to track "next",
> and BPF CI use "master". Then, most importantly, only merge next into master
> after libbpf CI has passed.
> 
> This can potentially be automated, but would require push access to the
> pahole repo. Until then, a maintainer would need to manually check the
> libbpf CI status here:
> 
> https://github.com/libbpf/libbpf/actions/workflows/test.yml
> 
> Another thing is that libbpf CI only tests x86_64 currently.
> We could add aarch64 to libbpf, or migrate pahole staging job to
> kernel-patches/vmtest (which is almost identical to BPF CI).

Hi everyone.

I looked into adding aarch64 to libbpf/libbpf CI: we can't do that
because Github does not provide hosted Linux aarch64 runners, only
macs [1].

Since BPF CI already has all the infrastructure in place, I figured
it's going to be relatively easy to set up an additional workflow
specifically for pahole.

Here is how it looks like:
https://github.com/kernel-patches/vmtest/actions/runs/12796621827

This is basically a simplified BPF CI run: build pahole, build kernel
with gcc, build selftests/bpf with clang-18 and run test_progs (only
one runner, but we can run more if appropriate).

I am thinking to set it up to run once a week, testing pahole/next.
If we do that, tmp.master tracking on libbpf/libbpf can be removed.

I also thought about email notifications to dwarves list, like we have
for BPF CI. Unfortunately, there is no nice-and-easy way to set this
up: we either have to maintain a dummy email account and use a
third-party action (like [2]), or we could add a code path to Kernel
Patches Daemon that currently handles BPF CI notifications. I don't
like either way.

Alternatively, maintainers could subscribe to github notifications on
kernel-patches/bpf (aka "watch"), but that might be too noisy as there
is a lot of activity there. Of course, you could check the CI runs
without any notifications too.

Alan, Arnaldo, Andrii, please let me know if you have any suggestions
or objections.

[1] https://docs.github.com/en/actions/using-github-hosted-runners/using-github-hosted-runners/about-github-hosted-runners#standard-github-hosted-runners-for-public-repositories
[2] https://github.com/marketplace/actions/send-email

> 
> [...]

Re: [PATCH dwarves] btf_encoder: always initialize func_state to 0

[Andrii Nakryiko]

On Wed, Jan 15, 2025 at 1:06 PM Ihor Solodrai <ihor.solodrai@pm.me> wrote:
>
> On Friday, January 10th, 2025 at 7:58 AM, Ihor Solodrai <ihor.solodrai@pm.me> wrote:
>
> >
> > On Friday, January 10th, 2025 at 5:51 AM, Arnaldo Carvalho de Melo acme@kernel.org wrote:
> >
> > > On Fri, Jan 10, 2025 at 02:31:41AM +0000, Ihor Solodrai wrote:
> > >
> > > > BPF CI caught a segfault on aarch64 and s390x [1] after recent merges
> > > > into the master branch.
> > >
> > > In the past the libbpf github actions was tracking the tmp.master (it would
> > > be better to track "next") branch and I was looking at when it passed to
> > > then move "next" to master, that would be great to have so that we
> > > wouldn't be having these bugs in the git history, avoiding force pushes.
> >
> >
> > libbpf CI is still tracking tmp.master:
> >
> > https://github.com/libbpf/libbpf/actions/runs/12696027660/job/35389206487
> >
> > However it only runs once a day. BPF CI runs more frequently due to the
> > volume of incoming patches. As of recently, BPF CI has been using "master".
> > Yesterday, when I saw the failures, I switched BPF CI to v1.28.
> >
> > I think the right way to approach this is for libbpf/libbpf to track "next",
> > and BPF CI use "master". Then, most importantly, only merge next into master
> > after libbpf CI has passed.
> >
> > This can potentially be automated, but would require push access to the
> > pahole repo. Until then, a maintainer would need to manually check the
> > libbpf CI status here:
> >
> > https://github.com/libbpf/libbpf/actions/workflows/test.yml
> >
> > Another thing is that libbpf CI only tests x86_64 currently.
> > We could add aarch64 to libbpf, or migrate pahole staging job to
> > kernel-patches/vmtest (which is almost identical to BPF CI).
>
> Hi everyone.
>
> I looked into adding aarch64 to libbpf/libbpf CI: we can't do that
> because Github does not provide hosted Linux aarch64 runners, only
> macs [1].
>
> Since BPF CI already has all the infrastructure in place, I figured
> it's going to be relatively easy to set up an additional workflow
> specifically for pahole.
>
> Here is how it looks like:
> https://github.com/kernel-patches/vmtest/actions/runs/12796621827
>
> This is basically a simplified BPF CI run: build pahole, build kernel
> with gcc, build selftests/bpf with clang-18 and run test_progs (only
> one runner, but we can run more if appropriate).
>
> I am thinking to set it up to run once a week, testing pahole/next.
> If we do that, tmp.master tracking on libbpf/libbpf can be removed.

Why not daily? I was even thinking of running it as part of patch
testing to catch regressions as early as possible, but then we'd have
red CI for any small pahole regression, which would be cumbersome. So
perhaps a daily separate run that wouldn't affect upstream patches?

>
> I also thought about email notifications to dwarves list, like we have
> for BPF CI. Unfortunately, there is no nice-and-easy way to set this
> up: we either have to maintain a dummy email account and use a
> third-party action (like [2]), or we could add a code path to Kernel
> Patches Daemon that currently handles BPF CI notifications. I don't
> like either way.
>
> Alternatively, maintainers could subscribe to github notifications on
> kernel-patches/bpf (aka "watch"), but that might be too noisy as there
> is a lot of activity there. Of course, you could check the CI runs
> without any notifications too.

Can you send email from inside the CI itself? E.g., if pahole build
fails or selftests fail, explicitly send email before failing CI run?
Just one idea, don't have anything better.

>
> Alan, Arnaldo, Andrii, please let me know if you have any suggestions
> or objections.
>
> [1] https://docs.github.com/en/actions/using-github-hosted-runners/using-github-hosted-runners/about-github-hosted-runners#standard-github-hosted-runners-for-public-repositories
> [2] https://github.com/marketplace/actions/send-email
>
> >
> > [...]
>
>

Re: [PATCH dwarves] btf_encoder: always initialize func_state to 0

[Ihor Solodrai]

On Thursday, January 16th, 2025 at 3:41 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:

> 
> 
> On Wed, Jan 15, 2025 at 1:06 PM Ihor Solodrai ihor.solodrai@pm.me wrote:
> 
> > On Friday, January 10th, 2025 at 7:58 AM, Ihor Solodrai ihor.solodrai@pm.me wrote:
> > 
> > > On Friday, January 10th, 2025 at 5:51 AM, Arnaldo Carvalho de Melo acme@kernel.org wrote:
> > > 
> > > > On Fri, Jan 10, 2025 at 02:31:41AM +0000, Ihor Solodrai wrote:
> > > > 
> > > > > BPF CI caught a segfault on aarch64 and s390x [1] after recent merges
> > > > > into the master branch.
> > > > 
> > > > In the past the libbpf github actions was tracking the tmp.master (it would
> > > > be better to track "next") branch and I was looking at when it passed to
> > > > then move "next" to master, that would be great to have so that we
> > > > wouldn't be having these bugs in the git history, avoiding force pushes.
> > > 
> > > libbpf CI is still tracking tmp.master:
> > > 
> > > https://github.com/libbpf/libbpf/actions/runs/12696027660/job/35389206487
> > > 
> > > However it only runs once a day. BPF CI runs more frequently due to the
> > > volume of incoming patches. As of recently, BPF CI has been using "master".
> > > Yesterday, when I saw the failures, I switched BPF CI to v1.28.
> > > 
> > > I think the right way to approach this is for libbpf/libbpf to track "next",
> > > and BPF CI use "master". Then, most importantly, only merge next into master
> > > after libbpf CI has passed.
> > > 
> > > This can potentially be automated, but would require push access to the
> > > pahole repo. Until then, a maintainer would need to manually check the
> > > libbpf CI status here:
> > > 
> > > https://github.com/libbpf/libbpf/actions/workflows/test.yml
> > > 
> > > Another thing is that libbpf CI only tests x86_64 currently.
> > > We could add aarch64 to libbpf, or migrate pahole staging job to
> > > kernel-patches/vmtest (which is almost identical to BPF CI).
> > 
> > Hi everyone.
> > 
> > I looked into adding aarch64 to libbpf/libbpf CI: we can't do that
> > because Github does not provide hosted Linux aarch64 runners, only
> > macs [1].
> > 
> > Since BPF CI already has all the infrastructure in place, I figured
> > it's going to be relatively easy to set up an additional workflow
> > specifically for pahole.
> > 
> > Here is how it looks like:
> > https://github.com/kernel-patches/vmtest/actions/runs/12796621827
> > 
> > This is basically a simplified BPF CI run: build pahole, build kernel
> > with gcc, build selftests/bpf with clang-18 and run test_progs (only
> > one runner, but we can run more if appropriate).
> > 
> > I am thinking to set it up to run once a week, testing pahole/next.
> > If we do that, tmp.master tracking on libbpf/libbpf can be removed.
> 
> 
> Why not daily? I was even thinking of running it as part of patch
> testing to catch regressions as early as possible, but then we'd have
> red CI for any small pahole regression, which would be cumbersome. So
> perhaps a daily separate run that wouldn't affect upstream patches?

We can run daily, that's not an issue. I was thinking weekly, because
there isn't much activity on the pahole side usually.

> 
> > I also thought about email notifications to dwarves list, like we have
> > for BPF CI. Unfortunately, there is no nice-and-easy way to set this
> > up: we either have to maintain a dummy email account and use a
> > third-party action (like [2]), or we could add a code path to Kernel
> > Patches Daemon that currently handles BPF CI notifications. I don't
> > like either way.
> > 
> > Alternatively, maintainers could subscribe to github notifications on
> > kernel-patches/bpf (aka "watch"), but that might be too noisy as there
> > is a lot of activity there. Of course, you could check the CI runs
> > without any notifications too.
> 
> 
> Can you send email from inside the CI itself? E.g., if pahole build
> fails or selftests fail, explicitly send email before failing CI run?
> Just one idea, don't have anything better.

Yeah, this is what I meant by using third-party action.
The issue with it is that you need to provide mail server credentials,
which are passed to a third-party code. So, to do that a bot-ty email
account needs to be created and maintained somewhere.

> 
> > Alan, Arnaldo, Andrii, please let me know if you have any suggestions
> > or objections.
> > 
> > [1] https://docs.github.com/en/actions/using-github-hosted-runners/using-github-hosted-runners/about-github-hosted-runners#standard-github-hosted-runners-for-public-repositories
> > [2] https://github.com/marketplace/actions/send-email
> > 
> > > [...]

Re: [PATCH dwarves] btf_encoder: always initialize func_state to 0

[Arnaldo Carvalho de Melo]

On Fri, Jan 10, 2025 at 02:31:41AM +0000, Ihor Solodrai wrote:
> @@ -1100,7 +1100,10 @@ static struct btf_encoder_func_state *btf_encoder__alloc_func_state(struct btf_e
>  		encoder->func_states.array = tmp;
>  	}
  
> -	return &encoder->func_states.array[encoder->func_states.cnt++];
> +	state = &encoder->func_states.array[encoder->func_states.cnt++];
> +	memset(state, 0, sizeof(*state));
> +
> +	return state;

Just a super nit, the following is equivalent and shorter:

	state = &encoder->func_states.array[encoder->func_states.cnt++];
	return memset(state, 0, sizeof(*state));

:-)

But nah, I'm appling your patch as-is.

Thanks!

- Arnaldo