On Monday, October 14th, 2024 at 8:06 AM, Alan Maguire <alan.maguire@xxxxxxxxxx> wrote:
>
>
> On 11/10/2024 23:55, Ihor Solodrai wrote:
>
> > On Friday, October 11th, 2024 at 9:52 AM, Ihor Solodrai ihor.solodrai@xxxxx wrote:
> >
> > [...]
> >
> > Eduard discovered a bug when building kernel using pahole with this
> > patch. I already debugged it and made a fix.
> >
> > TL;DR there might be fewer encoders than threads, which I didn't take
> > into account here:
> >
> > diff --git a/pahole.c b/pahole.c
> > index a45aa7a..90f712d 100644
> > --- a/pahole.c
> > +++ b/pahole.c
> > @@ -3257,29 +3257,34 @@ static int pahole_threads_collect(struct conf_load *conf, int nr_threads, void *
> > {
> > struct thread_data **threads = (struct thread_data **)thr_data;
> > struct btf_encoder *encoders[nr_threads];
> > - int i;
> > + int nr_encoders = 0;
> > int err = 0;
> > + int i;
> >
> > if (error)
> > goto out;
> >
> > - for (i = 0; i < nr_threads; i++)
> > - encoders[i] = threads[i]->encoder;
> > + for (i = 0; i < nr_threads; i++) {
> > + if (threads[i]->encoder) {
> > + encoders[nr_encoders] = threads[i]->encoder;
> > + nr_encoders++;
> > + }
> > + }
>
>
> I tried the above, but still saw segmentation faults; the following was
> needed at my end to get module BTF working:
>
> iff --git a/pahole.c b/pahole.c
> index a45aa7a..bb17aec 100644
> --- a/pahole.c
> +++ b/pahole.c
> @@ -3257,20 +3257,25 @@ static int pahole_threads_collect(struct
> conf_load *conf, int nr_threads, void *
> {
> struct thread_data **threads = (struct thread_data **)thr_data;
> struct btf_encoder *encoders[nr_threads];
> - int i;
> + int nr_encoders = 0;
> int err = 0;
> + int i;
>
> if (error)
> goto out;
>
> - for (i = 0; i < nr_threads; i++)
> - encoders[i] = threads[i]->encoder;
>
> + for (i = 0; i < nr_threads; i++) {
> + if (threads[i]->encoder && threads[i]->encoder !=
>
> btf_encoder) {
> + encoders[nr_encoders] = threads[i]->encoder;
>
> + nr_encoders++;
> + }
> + }
>
> /*
> * Merge content of the btf instances of worker threads to the btf
> * instance of the primary btf_encoder.
> */
> - err = btf_encoder__merge_encoders(btf_encoder, encoders,
> nr_threads);
> + err = btf_encoder__merge_encoders(btf_encoder, encoders,
> nr_encoders);
> if (err < 0)
> goto out;
> err = 0;
>
> In particular, nr_encoders must be used at btf_encoder__merge_encoders().
>
> With this in place, I hit another segmentation fault, due this time to
> the fact that we need to handle the case where there are no ELF
> functions (this occurs for one module when building the kernel). Fix is
> below:
>
> diff --git a/btf_encoder.c b/btf_encoder.c
> index 07d2031..b61a89c 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -2274,6 +2274,8 @@ static int elf_functions__collect(struct
> elf_functions *functions)
> functions->cnt,
>
> sizeof(functions->entries[0]),
>
> functions_cmp);
> + else
> + return 0;
>
> // Reallocate to the exact size
> functions->entries = realloc(functions->entries, functions->cnt
>
> * sizeof(struct elf_function));
>
Alan, thank you again for testing. I missed some lines in the diff I
pasted... Anyway this will be incorporated in v3, thanks.
> [...]
