| Message ID | 20210112184004.1302879-1-jolsa@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | btf_encoder: Add extra checks for symbol names | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Tue, Jan 12, 2021 at 10:43 AM Jiri Olsa <jolsa@kernel.org> wrote: > > When processing kernel image build by clang we can > find some functions without the name, which causes > pahole to segfault. > > Adding extra checks to make sure we always have > function's name defined before using it. > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > --- > btf_encoder.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/btf_encoder.c b/btf_encoder.c > index 333973054b61..17f7a14f2ef0 100644 > --- a/btf_encoder.c > +++ b/btf_encoder.c > @@ -72,6 +72,8 @@ static int collect_function(struct btf_elf *btfe, GElf_Sym *sym) > > if (elf_sym__type(sym) != STT_FUNC) > return 0; > + if (!elf_sym__name(sym, btfe->symtab)) > + return 0; elf_sym__name() is called below again, so might be better to just use local variable to store result? > > if (functions_cnt == functions_alloc) { > functions_alloc = max(1000, functions_alloc * 3 / 2); > @@ -730,9 +732,11 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force, > if (!has_arg_names(cu, &fn->proto)) > continue; > if (functions_cnt) { > - struct elf_function *func; > + const char *name = function__name(fn, cu); > + struct elf_function *func = NULL; > > - func = find_function(btfe, function__name(fn, cu)); > + if (name) > + func = find_function(btfe, name); isn't this a more convoluted way of writing: name = function__name(fn, cu); if (!name) continue; func = find_function(btfe, name); if (!func || func->generated) continue ? > if (!func || func->generated) > continue; > func->generated = true; > -- > 2.26.2 >
On Tue, Jan 12, 2021 at 11:20:44AM -0800, Andrii Nakryiko wrote: > On Tue, Jan 12, 2021 at 10:43 AM Jiri Olsa <jolsa@kernel.org> wrote: > > > > When processing kernel image build by clang we can > > find some functions without the name, which causes > > pahole to segfault. > > > > Adding extra checks to make sure we always have > > function's name defined before using it. > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > --- > > btf_encoder.c | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/btf_encoder.c b/btf_encoder.c > > index 333973054b61..17f7a14f2ef0 100644 > > --- a/btf_encoder.c > > +++ b/btf_encoder.c > > @@ -72,6 +72,8 @@ static int collect_function(struct btf_elf *btfe, GElf_Sym *sym) > > > > if (elf_sym__type(sym) != STT_FUNC) > > return 0; > > + if (!elf_sym__name(sym, btfe->symtab)) > > + return 0; > > elf_sym__name() is called below again, so might be better to just use > local variable to store result? right, will add > > > > > if (functions_cnt == functions_alloc) { > > functions_alloc = max(1000, functions_alloc * 3 / 2); > > @@ -730,9 +732,11 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force, > > if (!has_arg_names(cu, &fn->proto)) > > continue; > > if (functions_cnt) { > > - struct elf_function *func; > > + const char *name = function__name(fn, cu); > > + struct elf_function *func = NULL; > > > > - func = find_function(btfe, function__name(fn, cu)); > > + if (name) > > + func = find_function(btfe, name); > > isn't this a more convoluted way of writing: > > name = function__name(fn, cu); > if (!name) > continue; > > func = find_function(btfe, name); > if (!func || func->generated) > continue > > ? convoluted is my middle name ;-) I'll change it thanks, jirka > > > if (!func || func->generated) > > continue; > > func->generated = true; > > -- > > 2.26.2 > > >
On Tue, Jan 12, 2021 at 8:47 PM Jiri Olsa <jolsa@redhat.com> wrote: > > On Tue, Jan 12, 2021 at 11:20:44AM -0800, Andrii Nakryiko wrote: > > On Tue, Jan 12, 2021 at 10:43 AM Jiri Olsa <jolsa@kernel.org> wrote: > > > > > > When processing kernel image build by clang we can > > > find some functions without the name, which causes > > > pahole to segfault. > > > > > > Adding extra checks to make sure we always have > > > function's name defined before using it. > > > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > > --- > > > btf_encoder.c | 8 ++++++-- > > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > > > diff --git a/btf_encoder.c b/btf_encoder.c > > > index 333973054b61..17f7a14f2ef0 100644 > > > --- a/btf_encoder.c > > > +++ b/btf_encoder.c > > > @@ -72,6 +72,8 @@ static int collect_function(struct btf_elf *btfe, GElf_Sym *sym) > > > > > > if (elf_sym__type(sym) != STT_FUNC) > > > return 0; > > > + if (!elf_sym__name(sym, btfe->symtab)) > > > + return 0; > > > > elf_sym__name() is called below again, so might be better to just use > > local variable to store result? > > right, will add > > > > > > > > > if (functions_cnt == functions_alloc) { > > > functions_alloc = max(1000, functions_alloc * 3 / 2); > > > @@ -730,9 +732,11 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force, > > > if (!has_arg_names(cu, &fn->proto)) > > > continue; > > > if (functions_cnt) { > > > - struct elf_function *func; > > > + const char *name = function__name(fn, cu); > > > + struct elf_function *func = NULL; > > > > > > - func = find_function(btfe, function__name(fn, cu)); > > > + if (name) > > > + func = find_function(btfe, name); > > > > isn't this a more convoluted way of writing: > > > > name = function__name(fn, cu); > > if (!name) > > continue; > > > > func = find_function(btfe, name); > > if (!func || func->generated) > > continue > > > > ? > > convoluted is my middle name ;-) I'll change it > OK, a v2 will follow. Thanks JCO. - sed@ - > thanks, > jirka > > > > > > if (!func || func->generated) > > > continue; > > > func->generated = true; > > > -- > > > 2.26.2 > > > > > >
On 1/12/21 10:40 AM, Jiri Olsa wrote: > When processing kernel image build by clang we can > find some functions without the name, which causes > pahole to segfault. > > Adding extra checks to make sure we always have > function's name defined before using it. > I backported this patch to pahole 1.19, and I can confirm it fixes the segfault for me. -Tom > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > --- > btf_encoder.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/btf_encoder.c b/btf_encoder.c > index 333973054b61..17f7a14f2ef0 100644 > --- a/btf_encoder.c > +++ b/btf_encoder.c > @@ -72,6 +72,8 @@ static int collect_function(struct btf_elf *btfe, GElf_Sym *sym) > > if (elf_sym__type(sym) != STT_FUNC) > return 0; > + if (!elf_sym__name(sym, btfe->symtab)) > + return 0; > > if (functions_cnt == functions_alloc) { > functions_alloc = max(1000, functions_alloc * 3 / 2); > @@ -730,9 +732,11 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force, > if (!has_arg_names(cu, &fn->proto)) > continue; > if (functions_cnt) { > - struct elf_function *func; > + const char *name = function__name(fn, cu); > + struct elf_function *func = NULL; > > - func = find_function(btfe, function__name(fn, cu)); > + if (name) > + func = find_function(btfe, name); > if (!func || func->generated) > continue; > func->generated = true; >
On Wed, Jan 13, 2021 at 1:28 AM Tom Stellard <tstellar@redhat.com> wrote: > > On 1/12/21 10:40 AM, Jiri Olsa wrote: > > When processing kernel image build by clang we can > > find some functions without the name, which causes > > pahole to segfault. > > > > Adding extra checks to make sure we always have > > function's name defined before using it. > > > > I backported this patch to pahole 1.19, and I can confirm it fixes the > segfault for me. > Thanks for testing. Can you give me Git commit-id of LLVM-12 you tried? - Sedat - > -Tom > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > --- > > btf_encoder.c | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/btf_encoder.c b/btf_encoder.c > > index 333973054b61..17f7a14f2ef0 100644 > > --- a/btf_encoder.c > > +++ b/btf_encoder.c > > @@ -72,6 +72,8 @@ static int collect_function(struct btf_elf *btfe, GElf_Sym *sym) > > > > if (elf_sym__type(sym) != STT_FUNC) > > return 0; > > + if (!elf_sym__name(sym, btfe->symtab)) > > + return 0; > > > > if (functions_cnt == functions_alloc) { > > functions_alloc = max(1000, functions_alloc * 3 / 2); > > @@ -730,9 +732,11 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force, > > if (!has_arg_names(cu, &fn->proto)) > > continue; > > if (functions_cnt) { > > - struct elf_function *func; > > + const char *name = function__name(fn, cu); > > + struct elf_function *func = NULL; > > > > - func = find_function(btfe, function__name(fn, cu)); > > + if (name) > > + func = find_function(btfe, name); > > if (!func || func->generated) > > continue; > > func->generated = true; > > >
On 1/13/21 11:50 PM, Sedat Dilek wrote: > On Wed, Jan 13, 2021 at 1:28 AM Tom Stellard <tstellar@redhat.com> wrote: >> >> On 1/12/21 10:40 AM, Jiri Olsa wrote: >>> When processing kernel image build by clang we can >>> find some functions without the name, which causes >>> pahole to segfault. >>> >>> Adding extra checks to make sure we always have >>> function's name defined before using it. >>> >> >> I backported this patch to pahole 1.19, and I can confirm it fixes the >> segfault for me. >> > > Thanks for testing. > > Can you give me Git commit-id of LLVM-12 you tried? > I was building with LLVM 11.0.1 -Tom > - Sedat - > >> -Tom >> >>> Signed-off-by: Jiri Olsa <jolsa@kernel.org> >>> --- >>> btf_encoder.c | 8 ++++++-- >>> 1 file changed, 6 insertions(+), 2 deletions(-) >>> >>> diff --git a/btf_encoder.c b/btf_encoder.c >>> index 333973054b61..17f7a14f2ef0 100644 >>> --- a/btf_encoder.c >>> +++ b/btf_encoder.c >>> @@ -72,6 +72,8 @@ static int collect_function(struct btf_elf *btfe, GElf_Sym *sym) >>> >>> if (elf_sym__type(sym) != STT_FUNC) >>> return 0; >>> + if (!elf_sym__name(sym, btfe->symtab)) >>> + return 0; >>> >>> if (functions_cnt == functions_alloc) { >>> functions_alloc = max(1000, functions_alloc * 3 / 2); >>> @@ -730,9 +732,11 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force, >>> if (!has_arg_names(cu, &fn->proto)) >>> continue; >>> if (functions_cnt) { >>> - struct elf_function *func; >>> + const char *name = function__name(fn, cu); >>> + struct elf_function *func = NULL; >>> >>> - func = find_function(btfe, function__name(fn, cu)); >>> + if (name) >>> + func = find_function(btfe, name); >>> if (!func || func->generated) >>> continue; >>> func->generated = true; >>> >> >
On Thu, Jan 14, 2021 at 3:33 PM Tom Stellard <tstellar@redhat.com> wrote: > > On 1/13/21 11:50 PM, Sedat Dilek wrote: > > On Wed, Jan 13, 2021 at 1:28 AM Tom Stellard <tstellar@redhat.com> wrote: > >> > >> On 1/12/21 10:40 AM, Jiri Olsa wrote: > >>> When processing kernel image build by clang we can > >>> find some functions without the name, which causes > >>> pahole to segfault. > >>> > >>> Adding extra checks to make sure we always have > >>> function's name defined before using it. > >>> > >> > >> I backported this patch to pahole 1.19, and I can confirm it fixes the > >> segfault for me. > >> > > > > Thanks for testing. > > > > Can you give me Git commit-id of LLVM-12 you tried? > > > > I was building with LLVM 11.0.1 > Thanks Tom. - Sedat - > -Tom > > > - Sedat - > > > >> -Tom > >> > >>> Signed-off-by: Jiri Olsa <jolsa@kernel.org> > >>> --- > >>> btf_encoder.c | 8 ++++++-- > >>> 1 file changed, 6 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/btf_encoder.c b/btf_encoder.c > >>> index 333973054b61..17f7a14f2ef0 100644 > >>> --- a/btf_encoder.c > >>> +++ b/btf_encoder.c > >>> @@ -72,6 +72,8 @@ static int collect_function(struct btf_elf *btfe, GElf_Sym *sym) > >>> > >>> if (elf_sym__type(sym) != STT_FUNC) > >>> return 0; > >>> + if (!elf_sym__name(sym, btfe->symtab)) > >>> + return 0; > >>> > >>> if (functions_cnt == functions_alloc) { > >>> functions_alloc = max(1000, functions_alloc * 3 / 2); > >>> @@ -730,9 +732,11 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force, > >>> if (!has_arg_names(cu, &fn->proto)) > >>> continue; > >>> if (functions_cnt) { > >>> - struct elf_function *func; > >>> + const char *name = function__name(fn, cu); > >>> + struct elf_function *func = NULL; > >>> > >>> - func = find_function(btfe, function__name(fn, cu)); > >>> + if (name) > >>> + func = find_function(btfe, name); > >>> if (!func || func->generated) > >>> continue; > >>> func->generated = true; > >>> > >> > > >
Em Tue, Jan 12, 2021 at 04:27:59PM -0800, Tom Stellard escreveu: > On 1/12/21 10:40 AM, Jiri Olsa wrote: > > When processing kernel image build by clang we can > > find some functions without the name, which causes > > pahole to segfault. > > > > Adding extra checks to make sure we always have > > function's name defined before using it. > > > > I backported this patch to pahole 1.19, and I can confirm it fixes the > segfault for me. I'm applying v2 for this patch and based on your above statement I'm adding a: Tested-by: Tom Stellard <tstellar@redhat.com> Ok? Who originally reported this? - Arnaldo > -Tom > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > --- > > btf_encoder.c | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/btf_encoder.c b/btf_encoder.c > > index 333973054b61..17f7a14f2ef0 100644 > > --- a/btf_encoder.c > > +++ b/btf_encoder.c > > @@ -72,6 +72,8 @@ static int collect_function(struct btf_elf *btfe, GElf_Sym *sym) > > if (elf_sym__type(sym) != STT_FUNC) > > return 0; > > + if (!elf_sym__name(sym, btfe->symtab)) > > + return 0; > > if (functions_cnt == functions_alloc) { > > functions_alloc = max(1000, functions_alloc * 3 / 2); > > @@ -730,9 +732,11 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force, > > if (!has_arg_names(cu, &fn->proto)) > > continue; > > if (functions_cnt) { > > - struct elf_function *func; > > + const char *name = function__name(fn, cu); > > + struct elf_function *func = NULL; > > - func = find_function(btfe, function__name(fn, cu)); > > + if (name) > > + func = find_function(btfe, name); > > if (!func || func->generated) > > continue; > > func->generated = true; > > >
On Thu, Jan 21, 2021 at 2:38 PM Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote: > > Em Tue, Jan 12, 2021 at 04:27:59PM -0800, Tom Stellard escreveu: > > On 1/12/21 10:40 AM, Jiri Olsa wrote: > > > When processing kernel image build by clang we can > > > find some functions without the name, which causes > > > pahole to segfault. > > > > > > Adding extra checks to make sure we always have > > > function's name defined before using it. > > > > > > > I backported this patch to pahole 1.19, and I can confirm it fixes the > > segfault for me. > > I'm applying v2 for this patch and based on your above statement I'm > adding a: > > Tested-by: Tom Stellard <tstellar@redhat.com> > > Ok? > > Who originally reported this? > The origin was AFAICS the thread where I asked initially [1]. Tom reported in the same thread in [2] that pahole segfaults. Later in the thread Jiri offered a draft of this patch after doing some tests. I have tested all diffs and v1 and v2 of Jiri's patch. ( Anyway, latest pahole ToT plus Jiri's patch did not solve my origin problem. ) So up to you Arnaldo for the credits. - Sedat - [1] https://marc.info/?t=161036949500004&r=1&w=2 [2] https://marc.info/?t=161036949500004&r=1&w=2 > - Arnaldo > > > -Tom > > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > > --- > > > btf_encoder.c | 8 ++++++-- > > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > > > diff --git a/btf_encoder.c b/btf_encoder.c > > > index 333973054b61..17f7a14f2ef0 100644 > > > --- a/btf_encoder.c > > > +++ b/btf_encoder.c > > > @@ -72,6 +72,8 @@ static int collect_function(struct btf_elf *btfe, GElf_Sym *sym) > > > if (elf_sym__type(sym) != STT_FUNC) > > > return 0; > > > + if (!elf_sym__name(sym, btfe->symtab)) > > > + return 0; > > > if (functions_cnt == functions_alloc) { > > > functions_alloc = max(1000, functions_alloc * 3 / 2); > > > @@ -730,9 +732,11 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force, > > > if (!has_arg_names(cu, &fn->proto)) > > > continue; > > > if (functions_cnt) { > > > - struct elf_function *func; > > > + const char *name = function__name(fn, cu); > > > + struct elf_function *func = NULL; > > > - func = find_function(btfe, function__name(fn, cu)); > > > + if (name) > > > + func = find_function(btfe, name); > > > if (!func || func->generated) > > > continue; > > > func->generated = true; > > > > > > > -- > > - Arnaldo
On 1/21/21 5:38 AM, Arnaldo Carvalho de Melo wrote: > Em Tue, Jan 12, 2021 at 04:27:59PM -0800, Tom Stellard escreveu: >> On 1/12/21 10:40 AM, Jiri Olsa wrote: >>> When processing kernel image build by clang we can >>> find some functions without the name, which causes >>> pahole to segfault. >>> >>> Adding extra checks to make sure we always have >>> function's name defined before using it. >>> >> >> I backported this patch to pahole 1.19, and I can confirm it fixes the >> segfault for me. > > I'm applying v2 for this patch and based on your above statement I'm > adding a: > > Tested-by: Tom Stellard <tstellar@redhat.com> > > Ok? > Yes, this is fine. I also backported the v2 patch and tested it and it fixed the issue. -Tom > Who originally reported this? > > - Arnaldo > >> -Tom >> >>> Signed-off-by: Jiri Olsa <jolsa@kernel.org> >>> --- >>> btf_encoder.c | 8 ++++++-- >>> 1 file changed, 6 insertions(+), 2 deletions(-) >>> >>> diff --git a/btf_encoder.c b/btf_encoder.c >>> index 333973054b61..17f7a14f2ef0 100644 >>> --- a/btf_encoder.c >>> +++ b/btf_encoder.c >>> @@ -72,6 +72,8 @@ static int collect_function(struct btf_elf *btfe, GElf_Sym *sym) >>> if (elf_sym__type(sym) != STT_FUNC) >>> return 0; >>> + if (!elf_sym__name(sym, btfe->symtab)) >>> + return 0; >>> if (functions_cnt == functions_alloc) { >>> functions_alloc = max(1000, functions_alloc * 3 / 2); >>> @@ -730,9 +732,11 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force, >>> if (!has_arg_names(cu, &fn->proto)) >>> continue; >>> if (functions_cnt) { >>> - struct elf_function *func; >>> + const char *name = function__name(fn, cu); >>> + struct elf_function *func = NULL; >>> - func = find_function(btfe, function__name(fn, cu)); >>> + if (name) >>> + func = find_function(btfe, name); >>> if (!func || func->generated) >>> continue; >>> func->generated = true; >>> >> >
Em Thu, Jan 21, 2021 at 05:06:25PM +0100, Sedat Dilek escreveu: > On Thu, Jan 21, 2021 at 2:38 PM Arnaldo Carvalho de Melo > <arnaldo.melo@gmail.com> wrote: > > > > Em Tue, Jan 12, 2021 at 04:27:59PM -0800, Tom Stellard escreveu: > > > On 1/12/21 10:40 AM, Jiri Olsa wrote: > > > > When processing kernel image build by clang we can > > > > find some functions without the name, which causes > > > > pahole to segfault. > > > > > > > > Adding extra checks to make sure we always have > > > > function's name defined before using it. > > > > > > > > > > I backported this patch to pahole 1.19, and I can confirm it fixes the > > > segfault for me. > > > > I'm applying v2 for this patch and based on your above statement I'm > > adding a: > > > > Tested-by: Tom Stellard <tstellar@redhat.com> > > > > Ok? > > > > Who originally reported this? > > > > The origin was AFAICS the thread where I asked initially [1]. > > Tom reported in the same thread in [2] that pahole segfaults. > > Later in the thread Jiri offered a draft of this patch after doing some tests. > > I have tested all diffs and v1 and v2 of Jiri's patch. > ( Anyway, latest pahole ToT plus Jiri's patch did not solve my origin problem. ) > > So up to you Arnaldo for the credits. Reported-by: Sedat Dilek <sedat.dilek@gmail.com> Tested-by: Tom Stellard <tstellar@redhat.com> is how I'm going about it. Thanks for clarifying, - Arnaldo > - Sedat - > > [1] https://marc.info/?t=161036949500004&r=1&w=2 > [2] https://marc.info/?t=161036949500004&r=1&w=2 > > > - Arnaldo > > > > > -Tom > > > > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > > > --- > > > > btf_encoder.c | 8 ++++++-- > > > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/btf_encoder.c b/btf_encoder.c > > > > index 333973054b61..17f7a14f2ef0 100644 > > > > --- a/btf_encoder.c > > > > +++ b/btf_encoder.c > > > > @@ -72,6 +72,8 @@ static int collect_function(struct btf_elf *btfe, GElf_Sym *sym) > > > > if (elf_sym__type(sym) != STT_FUNC) > > > > return 0; > > > > + if (!elf_sym__name(sym, btfe->symtab)) > > > > + return 0; > > > > if (functions_cnt == functions_alloc) { > > > > functions_alloc = max(1000, functions_alloc * 3 / 2); > > > > @@ -730,9 +732,11 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force, > > > > if (!has_arg_names(cu, &fn->proto)) > > > > continue; > > > > if (functions_cnt) { > > > > - struct elf_function *func; > > > > + const char *name = function__name(fn, cu); > > > > + struct elf_function *func = NULL; > > > > - func = find_function(btfe, function__name(fn, cu)); > > > > + if (name) > > > > + func = find_function(btfe, name); > > > > if (!func || func->generated) > > > > continue; > > > > func->generated = true; > > > > > > > > > > > -- > > > > - Arnaldo
On Thu, Jan 21, 2021 at 8:09 AM Sedat Dilek <sedat.dilek@gmail.com> wrote: > > On Thu, Jan 21, 2021 at 2:38 PM Arnaldo Carvalho de Melo > <arnaldo.melo@gmail.com> wrote: > > > > Em Tue, Jan 12, 2021 at 04:27:59PM -0800, Tom Stellard escreveu: > > > On 1/12/21 10:40 AM, Jiri Olsa wrote: > > > > When processing kernel image build by clang we can > > > > find some functions without the name, which causes > > > > pahole to segfault. > > > > > > > > Adding extra checks to make sure we always have > > > > function's name defined before using it. > > > > > > > > > > I backported this patch to pahole 1.19, and I can confirm it fixes the > > > segfault for me. > > > > I'm applying v2 for this patch and based on your above statement I'm > > adding a: > > > > Tested-by: Tom Stellard <tstellar@redhat.com> > > > > Ok? > > > > Who originally reported this? > > > > The origin was AFAICS the thread where I asked initially [1]. > > Tom reported in the same thread in [2] that pahole segfaults. > > Later in the thread Jiri offered a draft of this patch after doing some tests. > > I have tested all diffs and v1 and v2 of Jiri's patch. > ( Anyway, latest pahole ToT plus Jiri's patch did not solve my origin problem. ) Your original problem was with DWARF5 or DWARF4? I think you mentioned both at some point, but I remember I couldn't repro DWARF4 problems. If you still have problems, can you start a new thread with steps to repro (including Kconfig, tooling versions, etc). And one for each problem, no all at the same time, please. I honestly lost track of what's still not working among those multiple intertwined email threads, sorry about that. > > So up to you Arnaldo for the credits. > > - Sedat - > > [1] https://marc.info/?t=161036949500004&r=1&w=2 > [2] https://marc.info/?t=161036949500004&r=1&w=2 > > > - Arnaldo > > > > > -Tom > > > > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > > > --- > > > > btf_encoder.c | 8 ++++++-- > > > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/btf_encoder.c b/btf_encoder.c > > > > index 333973054b61..17f7a14f2ef0 100644 > > > > --- a/btf_encoder.c > > > > +++ b/btf_encoder.c > > > > @@ -72,6 +72,8 @@ static int collect_function(struct btf_elf *btfe, GElf_Sym *sym) > > > > if (elf_sym__type(sym) != STT_FUNC) > > > > return 0; > > > > + if (!elf_sym__name(sym, btfe->symtab)) > > > > + return 0; > > > > if (functions_cnt == functions_alloc) { > > > > functions_alloc = max(1000, functions_alloc * 3 / 2); > > > > @@ -730,9 +732,11 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force, > > > > if (!has_arg_names(cu, &fn->proto)) > > > > continue; > > > > if (functions_cnt) { > > > > - struct elf_function *func; > > > > + const char *name = function__name(fn, cu); > > > > + struct elf_function *func = NULL; > > > > - func = find_function(btfe, function__name(fn, cu)); > > > > + if (name) > > > > + func = find_function(btfe, name); > > > > if (!func || func->generated) > > > > continue; > > > > func->generated = true; > > > > > > > > > > > -- > > > > - Arnaldo
On Thu, Jan 21, 2021 at 9:53 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Thu, Jan 21, 2021 at 8:09 AM Sedat Dilek <sedat.dilek@gmail.com> wrote: > > > > On Thu, Jan 21, 2021 at 2:38 PM Arnaldo Carvalho de Melo > > <arnaldo.melo@gmail.com> wrote: > > > > > > Em Tue, Jan 12, 2021 at 04:27:59PM -0800, Tom Stellard escreveu: > > > > On 1/12/21 10:40 AM, Jiri Olsa wrote: > > > > > When processing kernel image build by clang we can > > > > > find some functions without the name, which causes > > > > > pahole to segfault. > > > > > > > > > > Adding extra checks to make sure we always have > > > > > function's name defined before using it. > > > > > > > > > > > > > I backported this patch to pahole 1.19, and I can confirm it fixes the > > > > segfault for me. > > > > > > I'm applying v2 for this patch and based on your above statement I'm > > > adding a: > > > > > > Tested-by: Tom Stellard <tstellar@redhat.com> > > > > > > Ok? > > > > > > Who originally reported this? > > > > > > > The origin was AFAICS the thread where I asked initially [1]. > > > > Tom reported in the same thread in [2] that pahole segfaults. > > > > Later in the thread Jiri offered a draft of this patch after doing some tests. > > > > I have tested all diffs and v1 and v2 of Jiri's patch. > > ( Anyway, latest pahole ToT plus Jiri's patch did not solve my origin problem. ) > > Your original problem was with DWARF5 or DWARF4? I think you mentioned > both at some point, but I remember I couldn't repro DWARF4 problems. > If you still have problems, can you start a new thread with steps to > repro (including Kconfig, tooling versions, etc). And one for each > problem, no all at the same time, please. I honestly lost track of > what's still not working among those multiple intertwined email > threads, sorry about that. > I love people saying "I have a (one) problem." :-). The origin was Debian kernel-team enabled BTF-debuginfo Kconfig. My main focus is to be as close to Debian's kernel-config and if this works well with (experimental) Linux DWARF v5 support I am a happy guy. Do you want Nick's DWARF v5 patch-series as a base? Thinking of DWARF-v4? Use Nick's patchset or DWARF-v4 what is in Linux upstream means Linux v5.11-rc4+? What Git tree to use - Linus or one of your BPF/BTF folks? What version of pahole (latest Git) etc.? - Sedat - > > > > So up to you Arnaldo for the credits. > > > > - Sedat - > > > > [1] https://marc.info/?t=161036949500004&r=1&w=2 > > [2] https://marc.info/?t=161036949500004&r=1&w=2 > > > > > - Arnaldo > > > > > > > -Tom > > > > > > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > > > > --- > > > > > btf_encoder.c | 8 ++++++-- > > > > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/btf_encoder.c b/btf_encoder.c > > > > > index 333973054b61..17f7a14f2ef0 100644 > > > > > --- a/btf_encoder.c > > > > > +++ b/btf_encoder.c > > > > > @@ -72,6 +72,8 @@ static int collect_function(struct btf_elf *btfe, GElf_Sym *sym) > > > > > if (elf_sym__type(sym) != STT_FUNC) > > > > > return 0; > > > > > + if (!elf_sym__name(sym, btfe->symtab)) > > > > > + return 0; > > > > > if (functions_cnt == functions_alloc) { > > > > > functions_alloc = max(1000, functions_alloc * 3 / 2); > > > > > @@ -730,9 +732,11 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force, > > > > > if (!has_arg_names(cu, &fn->proto)) > > > > > continue; > > > > > if (functions_cnt) { > > > > > - struct elf_function *func; > > > > > + const char *name = function__name(fn, cu); > > > > > + struct elf_function *func = NULL; > > > > > - func = find_function(btfe, function__name(fn, cu)); > > > > > + if (name) > > > > > + func = find_function(btfe, name); > > > > > if (!func || func->generated) > > > > > continue; > > > > > func->generated = true; > > > > > > > > > > > > > > > -- > > > > > > - Arnaldo
On Thu, Jan 21, 2021 at 6:07 PM Sedat Dilek <sedat.dilek@gmail.com> wrote: > > On Thu, Jan 21, 2021 at 9:53 PM Andrii Nakryiko > <andrii.nakryiko@gmail.com> wrote: > > > > On Thu, Jan 21, 2021 at 8:09 AM Sedat Dilek <sedat.dilek@gmail.com> wrote: > > > > > > On Thu, Jan 21, 2021 at 2:38 PM Arnaldo Carvalho de Melo > > > <arnaldo.melo@gmail.com> wrote: > > > > > > > > Em Tue, Jan 12, 2021 at 04:27:59PM -0800, Tom Stellard escreveu: > > > > > On 1/12/21 10:40 AM, Jiri Olsa wrote: > > > > > > When processing kernel image build by clang we can > > > > > > find some functions without the name, which causes > > > > > > pahole to segfault. > > > > > > > > > > > > Adding extra checks to make sure we always have > > > > > > function's name defined before using it. > > > > > > > > > > > > > > > > I backported this patch to pahole 1.19, and I can confirm it fixes the > > > > > segfault for me. > > > > > > > > I'm applying v2 for this patch and based on your above statement I'm > > > > adding a: > > > > > > > > Tested-by: Tom Stellard <tstellar@redhat.com> > > > > > > > > Ok? > > > > > > > > Who originally reported this? > > > > > > > > > > The origin was AFAICS the thread where I asked initially [1]. > > > > > > Tom reported in the same thread in [2] that pahole segfaults. > > > > > > Later in the thread Jiri offered a draft of this patch after doing some tests. > > > > > > I have tested all diffs and v1 and v2 of Jiri's patch. > > > ( Anyway, latest pahole ToT plus Jiri's patch did not solve my origin problem. ) > > > > Your original problem was with DWARF5 or DWARF4? I think you mentioned > > both at some point, but I remember I couldn't repro DWARF4 problems. > > If you still have problems, can you start a new thread with steps to > > repro (including Kconfig, tooling versions, etc). And one for each > > problem, no all at the same time, please. I honestly lost track of > > what's still not working among those multiple intertwined email > > threads, sorry about that. > > > > I love people saying "I have a (one) problem." :-). > > The origin was Debian kernel-team enabled BTF-debuginfo Kconfig. > > My main focus is to be as close to Debian's kernel-config and if this > works well with (experimental) Linux DWARF v5 support I am a happy > guy. I don't know what kernel config Debian is using, that's why I'm asking for kernel config that does cause the problem. Because the one I'm using doesn't. But this problem can be a result of a lot of things, specific compiler and its version, specific kernel config, who knows what else. > > Do you want Nick's DWARF v5 patch-series as a base? Arnaldo was going to figure out the DWARF v5 problem, so I'm leaving it up to him. I'm curious about DWARF v4 problems because no one yet reported that previously. > Thinking of DWARF-v4? > Use Nick's patchset or DWARF-v4 what is in Linux upstream means Linux > v5.11-rc4+? > What Git tree to use - Linus or one of your BPF/BTF folks? I checked both v5.11-rc4 and the latest bpf-next with CONFIG_DEBUG_INFO_DWARF4=y and CONFIG_DEBUG_INFO_BTF=y. I get no warnings, everything works. > > What version of pahole (latest Git) etc.? Latest pahole built from Git, yes. But let's not do it in a backwards manner with me telling you what works (my environment, my config), rather you telling us what *doesn't* work (your config, your environment), so that we can try to reproduce. > > - Sedat - > > > > > > > So up to you Arnaldo for the credits. > > > > > > - Sedat - > > > > > > [1] https://marc.info/?t=161036949500004&r=1&w=2 > > > [2] https://marc.info/?t=161036949500004&r=1&w=2 > > > > > > > - Arnaldo > > > > > > > > > -Tom > > > > > > > > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > > > > > --- > > > > > > btf_encoder.c | 8 ++++++-- > > > > > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > > > > > > > > > diff --git a/btf_encoder.c b/btf_encoder.c > > > > > > index 333973054b61..17f7a14f2ef0 100644 > > > > > > --- a/btf_encoder.c > > > > > > +++ b/btf_encoder.c > > > > > > @@ -72,6 +72,8 @@ static int collect_function(struct btf_elf *btfe, GElf_Sym *sym) > > > > > > if (elf_sym__type(sym) != STT_FUNC) > > > > > > return 0; > > > > > > + if (!elf_sym__name(sym, btfe->symtab)) > > > > > > + return 0; > > > > > > if (functions_cnt == functions_alloc) { > > > > > > functions_alloc = max(1000, functions_alloc * 3 / 2); > > > > > > @@ -730,9 +732,11 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force, > > > > > > if (!has_arg_names(cu, &fn->proto)) > > > > > > continue; > > > > > > if (functions_cnt) { > > > > > > - struct elf_function *func; > > > > > > + const char *name = function__name(fn, cu); > > > > > > + struct elf_function *func = NULL; > > > > > > - func = find_function(btfe, function__name(fn, cu)); > > > > > > + if (name) > > > > > > + func = find_function(btfe, name); > > > > > > if (!func || func->generated) > > > > > > continue; > > > > > > func->generated = true; > > > > > > > > > > > > > > > > > > > -- > > > > > > > > - Arnaldo
Em Thu, Jan 21, 2021 at 08:11:17PM -0800, Andrii Nakryiko escreveu: > On Thu, Jan 21, 2021 at 6:07 PM Sedat Dilek <sedat.dilek@gmail.com> wrote: > > Do you want Nick's DWARF v5 patch-series as a base? > Arnaldo was going to figure out the DWARF v5 problem, so I'm leaving > it up to him. I'm curious about DWARF v4 problems because no one yet > reported that previously. I think I have the reported one fixed, Andrii, can you please do whatever pre-release tests you can in your environment with what is in: https://git.kernel.org/pub/scm/devel/pahole/pahole.git/log/?h=DW_AT_data_bit_offset ? The cset has the tests I performed and the references to the bugzilla ticket and Daniel has tested as well for his XDR + gcc 11 problem. Thanks, - Arnaldo
On Thu, Jan 28, 2021 at 9:00 PM Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote: > > Em Thu, Jan 21, 2021 at 08:11:17PM -0800, Andrii Nakryiko escreveu: > > On Thu, Jan 21, 2021 at 6:07 PM Sedat Dilek <sedat.dilek@gmail.com> wrote: > > > Do you want Nick's DWARF v5 patch-series as a base? > > > Arnaldo was going to figure out the DWARF v5 problem, so I'm leaving > > it up to him. I'm curious about DWARF v4 problems because no one yet > > reported that previously. > > I think I have the reported one fixed, Andrii, can you please do > whatever pre-release tests you can in your environment with what is in: > > https://git.kernel.org/pub/scm/devel/pahole/pahole.git/log/?h=DW_AT_data_bit_offset > > ? > > The cset has the tests I performed and the references to the bugzilla > ticket and Daniel has tested as well for his XDR + gcc 11 problem. > > Thanks, > What Git tree should someone use to test this? Linus Git? bpf / bpf-next? - Sedat -
Em Thu, Jan 28, 2021 at 09:57:14PM +0100, Sedat Dilek escreveu: > On Thu, Jan 28, 2021 at 9:00 PM Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote: > > Em Thu, Jan 21, 2021 at 08:11:17PM -0800, Andrii Nakryiko escreveu: > > > On Thu, Jan 21, 2021 at 6:07 PM Sedat Dilek <sedat.dilek@gmail.com> wrote: > > > > Do you want Nick's DWARF v5 patch-series as a base? > > > Arnaldo was going to figure out the DWARF v5 problem, so I'm leaving > > > it up to him. I'm curious about DWARF v4 problems because no one yet > > > reported that previously. > > I think I have the reported one fixed, Andrii, can you please do > > whatever pre-release tests you can in your environment with what is in: > > https://git.kernel.org/pub/scm/devel/pahole/pahole.git/log/?h=DW_AT_data_bit_offset > > ? > > The cset has the tests I performed and the references to the bugzilla > > ticket and Daniel has tested as well for his XDR + gcc 11 problem. > > What Git tree should someone use to test this? > Linus Git? > bpf / bpf-next? The one you were having problems with :) This pahole branch should be handling multiple problems, this is the list of changes since v1.19: [acme@five pahole]$ git log --oneline v1.19.. b91b19840b0062b8 (HEAD -> master, quaco/master, origin/DW_AT_data_bit_offset) dwarf_loader: Support DW_AT_data_bit_offset c692e8ac5ccbab99 dwarf_loader: Optimize a bit the reading of DW_AT_data_member_location 65917b24942ce620 dwarf_loader: Fix typo 77205a119c85e396 dwarf_loader: Introduce __attr_offset() to reuse call to dwarf_attr() 8ec231f6b0c8aaef dwarf_loader: Support DW_FORM_implicit_const in attr_numeric() 7453895e01edb535 (origin/master, origin/HEAD) btf_encoder: Improve ELF error reporting 1bb49897dd2b65b0 bpf_encoder: Translate SHN_XINDEX in symbol's st_shndx values 3f8aad340bf1a188 elf_symtab: Handle SHN_XINDEX index in elf_section_by_name() e32b9800e650a6eb btf_encoder: Add extra checks for symbol names 82749180b23d3c9c libbpf: allow to use packaged version 452dbcf35f1a7bf9 btf_encoder: Improve error-handling around objcopy cf381f9a3822d68b btf_encoder: Fix handling of restrict qualifier b688e35970600c15 btf_encoder: fix skipping per-CPU variables at offset 0 8c009d6ce762dfc9 btf_encoder: fix BTF variable generation for kernel modules b94e97e015a94e6b dwarves: Fix compilation on 32-bit architectures 17df51c700248f02 btf_encoder: Detect kernel module ftrace addresses 06ca639505fc56c6 btf_encoder: Use address size based on ELF's class aff60970d16b909e btf_encoder: Factor filter_functions function 1e6a3fed6e52d365 rpm: Fix changelog date [acme@five pahole]$ Now I just need to do the boilerplate update of the version number, Changes and .spec file, to release v1.20. - Arnaldo
On Thu, Jan 28, 2021 at 10:11 PM Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote: > > Em Thu, Jan 28, 2021 at 09:57:14PM +0100, Sedat Dilek escreveu: > > On Thu, Jan 28, 2021 at 9:00 PM Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote: > > > > Em Thu, Jan 21, 2021 at 08:11:17PM -0800, Andrii Nakryiko escreveu: > > > > On Thu, Jan 21, 2021 at 6:07 PM Sedat Dilek <sedat.dilek@gmail.com> wrote: > > > > > Do you want Nick's DWARF v5 patch-series as a base? > > > > > Arnaldo was going to figure out the DWARF v5 problem, so I'm leaving > > > > it up to him. I'm curious about DWARF v4 problems because no one yet > > > > reported that previously. > > > > I think I have the reported one fixed, Andrii, can you please do > > > whatever pre-release tests you can in your environment with what is in: > > > > https://git.kernel.org/pub/scm/devel/pahole/pahole.git/log/?h=DW_AT_data_bit_offset > > > > ? > > > > The cset has the tests I performed and the references to the bugzilla > > > ticket and Daniel has tested as well for his XDR + gcc 11 problem. > > > > What Git tree should someone use to test this? > > Linus Git? > > bpf / bpf-next? > > The one you were having problems with :) > > This pahole branch should be handling multiple problems, this is the > list of changes since v1.19: > > [acme@five pahole]$ git log --oneline v1.19.. > b91b19840b0062b8 (HEAD -> master, quaco/master, origin/DW_AT_data_bit_offset) dwarf_loader: Support DW_AT_data_bit_offset > c692e8ac5ccbab99 dwarf_loader: Optimize a bit the reading of DW_AT_data_member_location > 65917b24942ce620 dwarf_loader: Fix typo > 77205a119c85e396 dwarf_loader: Introduce __attr_offset() to reuse call to dwarf_attr() > 8ec231f6b0c8aaef dwarf_loader: Support DW_FORM_implicit_const in attr_numeric() > 7453895e01edb535 (origin/master, origin/HEAD) btf_encoder: Improve ELF error reporting > 1bb49897dd2b65b0 bpf_encoder: Translate SHN_XINDEX in symbol's st_shndx values > 3f8aad340bf1a188 elf_symtab: Handle SHN_XINDEX index in elf_section_by_name() > e32b9800e650a6eb btf_encoder: Add extra checks for symbol names > 82749180b23d3c9c libbpf: allow to use packaged version > 452dbcf35f1a7bf9 btf_encoder: Improve error-handling around objcopy > cf381f9a3822d68b btf_encoder: Fix handling of restrict qualifier > b688e35970600c15 btf_encoder: fix skipping per-CPU variables at offset 0 > 8c009d6ce762dfc9 btf_encoder: fix BTF variable generation for kernel modules > b94e97e015a94e6b dwarves: Fix compilation on 32-bit architectures > 17df51c700248f02 btf_encoder: Detect kernel module ftrace addresses > 06ca639505fc56c6 btf_encoder: Use address size based on ELF's class > aff60970d16b909e btf_encoder: Factor filter_functions function > 1e6a3fed6e52d365 rpm: Fix changelog date > [acme@five pahole]$ > > Now I just need to do the boilerplate update of the version number, > Changes and .spec file, to release v1.20. > When do you plan to release v1.20? I can wait until this is done. - Sedat -
On Thu, Jan 28, 2021 at 12:00 PM Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote: > > Em Thu, Jan 21, 2021 at 08:11:17PM -0800, Andrii Nakryiko escreveu: > > On Thu, Jan 21, 2021 at 6:07 PM Sedat Dilek <sedat.dilek@gmail.com> wrote: > > > Do you want Nick's DWARF v5 patch-series as a base? > > > Arnaldo was going to figure out the DWARF v5 problem, so I'm leaving > > it up to him. I'm curious about DWARF v4 problems because no one yet > > reported that previously. > > I think I have the reported one fixed, Andrii, can you please do > whatever pre-release tests you can in your environment with what is in: > Hi Arnaldo, Sorry for the delay, just back from a short PTO. It all looks to be working fine on my side. There is a compilation error in our libbpf CI when building the latest pahole from sources due to DW_FORM_implicit_const being undefined. I'm updating our VMs to use Ubuntu Focal 20.04, up from Bionic 18.04, and that should hopefully solve the issue due to newer versions of libdw. If you worry about breaking others, though, we might want to add #ifndef guards and re-define DW_FORM_implicit_const as 0x21 explicitly in pahole source code. But otherwise, all good from what I can see in my environment. Looking forward to 1.20 release! I'll let you know if, after updating to Ubuntu Focal, any new pahole issues crop up. > https://git.kernel.org/pub/scm/devel/pahole/pahole.git/log/?h=DW_AT_data_bit_offset > > ? > > The cset has the tests I performed and the references to the bugzilla > ticket and Daniel has tested as well for his XDR + gcc 11 problem. > > Thanks, > > - Arnaldo
On Tue, Feb 2, 2021 at 8:48 AM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Thu, Jan 28, 2021 at 12:00 PM Arnaldo Carvalho de Melo > <arnaldo.melo@gmail.com> wrote: > > > > Em Thu, Jan 21, 2021 at 08:11:17PM -0800, Andrii Nakryiko escreveu: > > > On Thu, Jan 21, 2021 at 6:07 PM Sedat Dilek <sedat.dilek@gmail.com> wrote: > > > > Do you want Nick's DWARF v5 patch-series as a base? > > > > > Arnaldo was going to figure out the DWARF v5 problem, so I'm leaving > > > it up to him. I'm curious about DWARF v4 problems because no one yet > > > reported that previously. > > > > I think I have the reported one fixed, Andrii, can you please do > > whatever pre-release tests you can in your environment with what is in: > > > > Hi Arnaldo, > > Sorry for the delay, just back from a short PTO. > > It all looks to be working fine on my side. There is a compilation > error in our libbpf CI when building the latest pahole from sources > due to DW_FORM_implicit_const being undefined. I'm updating our VMs to > use Ubuntu Focal 20.04, up from Bionic 18.04, and that should > hopefully solve the issue due to newer versions of libdw. If you worry > about breaking others, though, we might want to add #ifndef guards and > re-define DW_FORM_implicit_const as 0x21 explicitly in pahole source > code. > > But otherwise, all good from what I can see in my environment. Looking > forward to 1.20 release! I'll let you know if, after updating to > Ubuntu Focal, any new pahole issues crop up. > Last weekend I did some testing with <pahole.git#DW_AT_data_bit_offset> and DWARF-v5 support for the Linux-kernel. The good: I was able to compile :-). The bad: My build-log grew up to 1.2GiB and I could not boot in QEMU. The ugly: I killed the archive which had all relevant material. Yesterday, I compiled latest pahole.git: $ git describe v1.19-25-g8d6f06f053a0 $ git log -1 --oneline 8d6f06f053a0 (HEAD -> master, origin/master, origin/HEAD) dwarf_loader: Add conditional DW_FORM_implicit_const definition for older system I cannot promise to test it with Nick Desaulniers' DWARF-v5 patchset but the recent DWARF changes within pahole.git look promising. - Sedat - > > > https://git.kernel.org/pub/scm/devel/pahole/pahole.git/log/?h=DW_AT_data_bit_offset > > > > ? > > > > The cset has the tests I performed and the references to the bugzilla > > ticket and Daniel has tested as well for his XDR + gcc 11 problem. > > > > Thanks, > > > > - Arnaldo
Hi, On Wed, 2021-02-03 at 10:03 +0100, Sedat Dilek wrote: > > It all looks to be working fine on my side. There is a compilation > > error in our libbpf CI when building the latest pahole from sources > > due to DW_FORM_implicit_const being undefined. I'm updating our VMs to > > use Ubuntu Focal 20.04, up from Bionic 18.04, and that should > > hopefully solve the issue due to newer versions of libdw. If you worry > > about breaking others, though, we might want to add #ifndef guards and > > re-define DW_FORM_implicit_const as 0x21 explicitly in pahole source > > code. I think that might be a good idea for older setups. But that also means that the underlying elfutils libdw doesn't support DWARF5, so pahole itself also wouldn't work (the define would only fix the compile time issue, not the runtime issue of not being able to parse DW_FORM_implicit_const). That might not be a problem because such systems also wouldn't have GCC11 defaulting to DWARF5. > > But otherwise, all good from what I can see in my environment. > > Looking > > forward to 1.20 release! I'll let you know if, after updating to > > Ubuntu Focal, any new pahole issues crop up. > > > > Last weekend I did some testing with > <pahole.git#DW_AT_data_bit_offset> and DWARF-v5 support for the > Linux-kernel. > > The good: I was able to compile :-). > The bad: My build-log grew up to 1.2GiB and I could not boot in QEMU. > The ugly: I killed the archive which had all relevant material. I think the build-log grew so much because of warnings about unknown tags. At least when using GCC11 you'll get a couple of standardized DWARF5 tags instead of the GNU extensions to DWARF4. That should be solved by: commit d783117162c0212d4f75f6cea185f493d2f244e1 Author: Mark Wielaard <mark@klomp.org> Date: Sun Jan 31 01:27:31 2021 +0100 dwarf_loader: Handle DWARF5 DW_TAG_call_site like DW_TAG_GNU_call_site Cheers, Mark
On Wed, Feb 3, 2021 at 11:23 AM Mark Wielaard <mark@klomp.org> wrote: > > Hi, > > On Wed, 2021-02-03 at 10:03 +0100, Sedat Dilek wrote: > > > It all looks to be working fine on my side. There is a compilation > > > error in our libbpf CI when building the latest pahole from sources > > > due to DW_FORM_implicit_const being undefined. I'm updating our VMs to > > > use Ubuntu Focal 20.04, up from Bionic 18.04, and that should > > > hopefully solve the issue due to newer versions of libdw. If you worry > > > about breaking others, though, we might want to add #ifndef guards and > > > re-define DW_FORM_implicit_const as 0x21 explicitly in pahole source > > > code. > > I think that might be a good idea for older setups. But that also means > that the underlying elfutils libdw doesn't support DWARF5, so pahole > itself also wouldn't work (the define would only fix the compile time > issue, not the runtime issue of not being able to parse > DW_FORM_implicit_const). That might not be a problem because such > systems also wouldn't have GCC11 defaulting to DWARF5. > > > > But otherwise, all good from what I can see in my environment. > > > Looking > > > forward to 1.20 release! I'll let you know if, after updating to > > > Ubuntu Focal, any new pahole issues crop up. > > > > > > > Last weekend I did some testing with > > <pahole.git#DW_AT_data_bit_offset> and DWARF-v5 support for the > > Linux-kernel. > > > > The good: I was able to compile :-). > > The bad: My build-log grew up to 1.2GiB and I could not boot in QEMU. > > The ugly: I killed the archive which had all relevant material. > > I think the build-log grew so much because of warnings about unknown > tags. At least when using GCC11 you'll get a couple of standardized > DWARF5 tags instead of the GNU extensions to DWARF4. That should be > solved by: > > commit d783117162c0212d4f75f6cea185f493d2f244e1 > Author: Mark Wielaard <mark@klomp.org> > Date: Sun Jan 31 01:27:31 2021 +0100 > > dwarf_loader: Handle DWARF5 DW_TAG_call_site like DW_TAG_GNU_call_site > I had some conversation with Mark directly as I dropped by accident the CC list. With latest pahole from Git and CONFIG_DEBUG_INFO_BTF=y I was not able to build with DWARF-v4 and DWARF-v5. Hope it is OK for you Mark when I quote you: > Here I use LLVM/Clang v12.0.0-rc1 with Clang's Integrated Assembler > (make LLVM_IAS=1). Note I haven't personally tested llvm with DWARF5. I know some other tools cannot (yet) handle the DWARF5 produced by llvm (for example valgrind, rpm debugedit and dwz don't handle all the forms llvm emits when it produces DWARF5, which aren't emitted by GCC unless requesting split-dwarf). In theory dwarves/pahole should be able to handle it because elfutils libdw (at least versions > 0.172) does handle it. But I don't know if anybody ever tested that. But I believe llvm will by default emit DWARF4, not 5. More quotes from Mark: I would try to avoid using clang producing DWARF5. It clearly has some incompatibilities with dwarves/pahole. It should work if you don't set DEBUG_INFO_DWARF5. Try GCC 11 (which defaults to -gdwarf-5) or an earlier version (probably at least GCC 8 or higher) using -gdwarf-5 explicitly. What makes me nerves are reports from Red Hat's CKI reporting: 'failed to validate module [something] BTF: -22 ' This is was from ClangBuiltLinux mailing-list. Looks like CONFIG_DEBUG_INFO_BTF=y makes troubles with LLVM/Clang. Can we have a fix for Linux v5.11-rc6+ to avoid a selection of it when CC_IS_CLANG=y? - Sedat - - Sedat -
On Wed, Feb 3, 2021 at 1:48 PM Sedat Dilek <sedat.dilek@gmail.com> wrote: > > On Wed, Feb 3, 2021 at 11:23 AM Mark Wielaard <mark@klomp.org> wrote: > > > > Hi, > > > > On Wed, 2021-02-03 at 10:03 +0100, Sedat Dilek wrote: > > > > It all looks to be working fine on my side. There is a compilation > > > > error in our libbpf CI when building the latest pahole from sources > > > > due to DW_FORM_implicit_const being undefined. I'm updating our VMs to > > > > use Ubuntu Focal 20.04, up from Bionic 18.04, and that should > > > > hopefully solve the issue due to newer versions of libdw. If you worry > > > > about breaking others, though, we might want to add #ifndef guards and > > > > re-define DW_FORM_implicit_const as 0x21 explicitly in pahole source > > > > code. > > > > I think that might be a good idea for older setups. But that also means > > that the underlying elfutils libdw doesn't support DWARF5, so pahole > > itself also wouldn't work (the define would only fix the compile time > > issue, not the runtime issue of not being able to parse > > DW_FORM_implicit_const). That might not be a problem because such > > systems also wouldn't have GCC11 defaulting to DWARF5. > > > > > > But otherwise, all good from what I can see in my environment. > > > > Looking > > > > forward to 1.20 release! I'll let you know if, after updating to > > > > Ubuntu Focal, any new pahole issues crop up. > > > > > > > > > > Last weekend I did some testing with > > > <pahole.git#DW_AT_data_bit_offset> and DWARF-v5 support for the > > > Linux-kernel. > > > > > > The good: I was able to compile :-). > > > The bad: My build-log grew up to 1.2GiB and I could not boot in QEMU. > > > The ugly: I killed the archive which had all relevant material. > > > > I think the build-log grew so much because of warnings about unknown > > tags. At least when using GCC11 you'll get a couple of standardized > > DWARF5 tags instead of the GNU extensions to DWARF4. That should be > > solved by: > > > > commit d783117162c0212d4f75f6cea185f493d2f244e1 > > Author: Mark Wielaard <mark@klomp.org> > > Date: Sun Jan 31 01:27:31 2021 +0100 > > > > dwarf_loader: Handle DWARF5 DW_TAG_call_site like DW_TAG_GNU_call_site > > > > I had some conversation with Mark directly as I dropped by accident the CC list. > > With latest pahole from Git and CONFIG_DEBUG_INFO_BTF=y I was not able > to build with DWARF-v4 and DWARF-v5. There is hardly anything actionable without all the extra info I've asked you before. What's the issue? What's the kernel config? Tool versions? > > Hope it is OK for you Mark when I quote you: > > > Here I use LLVM/Clang v12.0.0-rc1 with Clang's Integrated Assembler > > (make LLVM_IAS=1). > > Note I haven't personally tested llvm with DWARF5. I know some other > tools cannot (yet) handle the DWARF5 produced by llvm (for example > valgrind, rpm debugedit and dwz don't handle all the forms llvm emits > when it produces DWARF5, which aren't emitted by GCC unless requesting > split-dwarf). In theory dwarves/pahole should be able to handle it > because elfutils libdw (at least versions > 0.172) does handle it. But > I don't know if anybody ever tested that. But I believe llvm will by > default emit DWARF4, not 5. > > More quotes from Mark: > > I would try to avoid using clang producing DWARF5. It clearly has some > incompatibilities with dwarves/pahole. It should work if you don't set > DEBUG_INFO_DWARF5. Try GCC 11 (which defaults to -gdwarf-5) or an > earlier version (probably at least GCC 8 or higher) using -gdwarf-5 > explicitly. > > What makes me nerves are reports from Red Hat's CKI reporting: > > 'failed to validate module [something] BTF: -22 ' > > This is was from ClangBuiltLinux mailing-list. And no link to the issue, of course. If you are hoping for someone to try to help and fix issues, please provide extra info. If this is what I think it is, that was the problem with kernel rejecting empty BTF and it was fixed already in v5.11-fbk6. But who knows, I can only guess. > > Looks like CONFIG_DEBUG_INFO_BTF=y makes troubles with LLVM/Clang. > Can we have a fix for Linux v5.11-rc6+ to avoid a selection of it when > CC_IS_CLANG=y? Let's first understand problems and try to fix them, please. > > - Sedat - > > > - Sedat -
Em Wed, Feb 03, 2021 at 03:21:58PM -0800, Andrii Nakryiko escreveu: > On Wed, Feb 3, 2021 at 1:48 PM Sedat Dilek <sedat.dilek@gmail.com> wrote: > > Looks like CONFIG_DEBUG_INFO_BTF=y makes troubles with LLVM/Clang. > > Can we have a fix for Linux v5.11-rc6+ to avoid a selection of it when > > CC_IS_CLANG=y? > Let's first understand problems and try to fix them, please. Yeah, from what I've read this is something only related to clang building the kernel with DWARF5, right? I just delayed to tomorrow releasing pahole 1.20 as its late here and there is this discussion going on, what I have is at: https://git.kernel.org/pub/scm/devel/pahole/pahole.git/log/?h=tmp.1.20 There is one patch more than what I think Sedat tried, that makes a Mark's patch for DWARF_TAG_call_site{_parameter} to be applied as well when processing inline expansions, which for what is being discussed here seems unrelated, just avoids tons of warnings when processing vmlinux with gcc 11 in fedora rawhide. - Arnaldo
On Thu, Feb 4, 2021 at 12:22 AM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Wed, Feb 3, 2021 at 1:48 PM Sedat Dilek <sedat.dilek@gmail.com> wrote: > > > > On Wed, Feb 3, 2021 at 11:23 AM Mark Wielaard <mark@klomp.org> wrote: > > > > > > Hi, > > > > > > On Wed, 2021-02-03 at 10:03 +0100, Sedat Dilek wrote: > > > > > It all looks to be working fine on my side. There is a compilation > > > > > error in our libbpf CI when building the latest pahole from sources > > > > > due to DW_FORM_implicit_const being undefined. I'm updating our VMs to > > > > > use Ubuntu Focal 20.04, up from Bionic 18.04, and that should > > > > > hopefully solve the issue due to newer versions of libdw. If you worry > > > > > about breaking others, though, we might want to add #ifndef guards and > > > > > re-define DW_FORM_implicit_const as 0x21 explicitly in pahole source > > > > > code. > > > > > > I think that might be a good idea for older setups. But that also means > > > that the underlying elfutils libdw doesn't support DWARF5, so pahole > > > itself also wouldn't work (the define would only fix the compile time > > > issue, not the runtime issue of not being able to parse > > > DW_FORM_implicit_const). That might not be a problem because such > > > systems also wouldn't have GCC11 defaulting to DWARF5. > > > > > > > > But otherwise, all good from what I can see in my environment. > > > > > Looking > > > > > forward to 1.20 release! I'll let you know if, after updating to > > > > > Ubuntu Focal, any new pahole issues crop up. > > > > > > > > > > > > > Last weekend I did some testing with > > > > <pahole.git#DW_AT_data_bit_offset> and DWARF-v5 support for the > > > > Linux-kernel. > > > > > > > > The good: I was able to compile :-). > > > > The bad: My build-log grew up to 1.2GiB and I could not boot in QEMU. > > > > The ugly: I killed the archive which had all relevant material. > > > > > > I think the build-log grew so much because of warnings about unknown > > > tags. At least when using GCC11 you'll get a couple of standardized > > > DWARF5 tags instead of the GNU extensions to DWARF4. That should be > > > solved by: > > > > > > commit d783117162c0212d4f75f6cea185f493d2f244e1 > > > Author: Mark Wielaard <mark@klomp.org> > > > Date: Sun Jan 31 01:27:31 2021 +0100 > > > > > > dwarf_loader: Handle DWARF5 DW_TAG_call_site like DW_TAG_GNU_call_site > > > > > > > I had some conversation with Mark directly as I dropped by accident the CC list. > > > > With latest pahole from Git and CONFIG_DEBUG_INFO_BTF=y I was not able > > to build with DWARF-v4 and DWARF-v5. > > There is hardly anything actionable without all the extra info I've > asked you before. What's the issue? What's the kernel config? Tool > versions? > > > > > Hope it is OK for you Mark when I quote you: > > > > > Here I use LLVM/Clang v12.0.0-rc1 with Clang's Integrated Assembler > > > (make LLVM_IAS=1). > > > > Note I haven't personally tested llvm with DWARF5. I know some other > > tools cannot (yet) handle the DWARF5 produced by llvm (for example > > valgrind, rpm debugedit and dwz don't handle all the forms llvm emits > > when it produces DWARF5, which aren't emitted by GCC unless requesting > > split-dwarf). In theory dwarves/pahole should be able to handle it > > because elfutils libdw (at least versions > 0.172) does handle it. But > > I don't know if anybody ever tested that. But I believe llvm will by > > default emit DWARF4, not 5. > > > > More quotes from Mark: > > > > I would try to avoid using clang producing DWARF5. It clearly has some > > incompatibilities with dwarves/pahole. It should work if you don't set > > DEBUG_INFO_DWARF5. Try GCC 11 (which defaults to -gdwarf-5) or an > > earlier version (probably at least GCC 8 or higher) using -gdwarf-5 > > explicitly. > > > > What makes me nerves are reports from Red Hat's CKI reporting: > > > > 'failed to validate module [something] BTF: -22 ' > > > > This is was from ClangBuiltLinux mailing-list. > > And no link to the issue, of course. If you are hoping for someone to > try to help and fix issues, please provide extra info. If this is what > I think it is, that was the problem with kernel rejecting empty BTF > and it was fixed already in v5.11-fbk6. But who knows, I can only > guess. > "Do one thing and do it well." ( Unix philosophy ) https://en.wikipedia.org/wiki/Unix_philosophy#Do_One_Thing_and_Do_It_Well Yesterday, I did two things in parallel. ( I had some other stuff to dig into which is fixed for me. ) As for fun I will do a new test with <pahole.git#tmp.1.20>. I will attach kernel-config and BTF/pahole warnings. For the ClangBuiltLinux mailing-list link: I have asked on the list to have the links so that people from outlands can read them. "Just for fun" (IMHO that is the title of Linus Torvalds' biographie :-)?) - Sedat - > > > > Looks like CONFIG_DEBUG_INFO_BTF=y makes troubles with LLVM/Clang. > > Can we have a fix for Linux v5.11-rc6+ to avoid a selection of it when > > CC_IS_CLANG=y? > > Let's first understand problems and try to fix them, please. > > > > > - Sedat - > > > > > > - Sedat -
diff --git a/btf_encoder.c b/btf_encoder.c index 333973054b61..17f7a14f2ef0 100644 --- a/btf_encoder.c +++ b/btf_encoder.c @@ -72,6 +72,8 @@ static int collect_function(struct btf_elf *btfe, GElf_Sym *sym) if (elf_sym__type(sym) != STT_FUNC) return 0; + if (!elf_sym__name(sym, btfe->symtab)) + return 0; if (functions_cnt == functions_alloc) { functions_alloc = max(1000, functions_alloc * 3 / 2); @@ -730,9 +732,11 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force, if (!has_arg_names(cu, &fn->proto)) continue; if (functions_cnt) { - struct elf_function *func; + const char *name = function__name(fn, cu); + struct elf_function *func = NULL; - func = find_function(btfe, function__name(fn, cu)); + if (name) + func = find_function(btfe, name); if (!func || func->generated) continue; func->generated = true;
When processing kernel image build by clang we can find some functions without the name, which causes pahole to segfault. Adding extra checks to make sure we always have function's name defined before using it. Signed-off-by: Jiri Olsa <jolsa@kernel.org> --- btf_encoder.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)