BPF Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] btf_encoder: Add extra checks for symbol names
@ 2021-01-12 18:40 Jiri Olsa
  2021-01-12 19:20 ` Andrii Nakryiko
  2021-01-13  0:27 ` Tom Stellard
  0 siblings, 2 replies; 15+ messages in thread
From: Jiri Olsa @ 2021-01-12 18:40 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: dwarves, bpf, Alexei Starovoitov, Andrii Nakryiko, Yonghong Song,
	Hao Luo, Sedat Dilek, Tom Stellard

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;
 
 	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;
-- 
2.26.2


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] btf_encoder: Add extra checks for symbol names
  2021-01-12 18:40 [PATCH] btf_encoder: Add extra checks for symbol names Jiri Olsa
@ 2021-01-12 19:20 ` Andrii Nakryiko
  2021-01-12 19:47   ` Jiri Olsa
  2021-01-13  0:27 ` Tom Stellard
  1 sibling, 1 reply; 15+ messages in thread
From: Andrii Nakryiko @ 2021-01-12 19:20 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, dwarves, bpf, Alexei Starovoitov,
	Andrii Nakryiko, Yonghong Song, Hao Luo, Sedat Dilek,
	Tom Stellard

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
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] btf_encoder: Add extra checks for symbol names
  2021-01-12 19:20 ` Andrii Nakryiko
@ 2021-01-12 19:47   ` Jiri Olsa
  2021-01-12 20:17     ` Sedat Dilek
  0 siblings, 1 reply; 15+ messages in thread
From: Jiri Olsa @ 2021-01-12 19:47 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, dwarves, bpf,
	Alexei Starovoitov, Andrii Nakryiko, Yonghong Song, Hao Luo,
	Sedat Dilek, Tom Stellard

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
> >
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] btf_encoder: Add extra checks for symbol names
  2021-01-12 19:47   ` Jiri Olsa
@ 2021-01-12 20:17     ` Sedat Dilek
  0 siblings, 0 replies; 15+ messages in thread
From: Sedat Dilek @ 2021-01-12 20:17 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Andrii Nakryiko, Jiri Olsa, Arnaldo Carvalho de Melo, dwarves,
	bpf, Alexei Starovoitov, Andrii Nakryiko, Yonghong Song, Hao Luo,
	Tom Stellard

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
> > >
> >
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] btf_encoder: Add extra checks for symbol names
  2021-01-12 18:40 [PATCH] btf_encoder: Add extra checks for symbol names Jiri Olsa
  2021-01-12 19:20 ` Andrii Nakryiko
@ 2021-01-13  0:27 ` Tom Stellard
  2021-01-14  7:50   ` Sedat Dilek
  2021-01-21 13:38   ` Arnaldo Carvalho de Melo
  1 sibling, 2 replies; 15+ messages in thread
From: Tom Stellard @ 2021-01-13  0:27 UTC (permalink / raw)
  To: Jiri Olsa, Arnaldo Carvalho de Melo
  Cc: dwarves, bpf, Alexei Starovoitov, Andrii Nakryiko, Yonghong Song,
	Hao Luo, Sedat Dilek

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;
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] btf_encoder: Add extra checks for symbol names
  2021-01-13  0:27 ` Tom Stellard
@ 2021-01-14  7:50   ` Sedat Dilek
  2021-01-14 14:33     ` Tom Stellard
  2021-01-21 13:38   ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 15+ messages in thread
From: Sedat Dilek @ 2021-01-14  7:50 UTC (permalink / raw)
  To: tstellar
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, dwarves, bpf,
	Alexei Starovoitov, Andrii Nakryiko, Yonghong Song, Hao Luo

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;
> >
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] btf_encoder: Add extra checks for symbol names
  2021-01-14  7:50   ` Sedat Dilek
@ 2021-01-14 14:33     ` Tom Stellard
  2021-01-14 14:39       ` Sedat Dilek
  0 siblings, 1 reply; 15+ messages in thread
From: Tom Stellard @ 2021-01-14 14:33 UTC (permalink / raw)
  To: sedat.dilek
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, dwarves, bpf,
	Alexei Starovoitov, Andrii Nakryiko, Yonghong Song, Hao Luo

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;
>>>
>>
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] btf_encoder: Add extra checks for symbol names
  2021-01-14 14:33     ` Tom Stellard
@ 2021-01-14 14:39       ` Sedat Dilek
  0 siblings, 0 replies; 15+ messages in thread
From: Sedat Dilek @ 2021-01-14 14:39 UTC (permalink / raw)
  To: tstellar
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, dwarves, bpf,
	Alexei Starovoitov, Andrii Nakryiko, Yonghong Song, Hao Luo

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;
> >>>
> >>
> >
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] btf_encoder: Add extra checks for symbol names
  2021-01-13  0:27 ` Tom Stellard
  2021-01-14  7:50   ` Sedat Dilek
@ 2021-01-21 13:38   ` Arnaldo Carvalho de Melo
  2021-01-21 16:06     ` Sedat Dilek
  2021-01-21 17:37     ` Tom Stellard
  1 sibling, 2 replies; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-01-21 13:38 UTC (permalink / raw)
  To: Tom Stellard
  Cc: Jiri Olsa, dwarves, bpf, Alexei Starovoitov, Andrii Nakryiko,
	Yonghong Song, Hao Luo, Sedat Dilek

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;
> > 
> 

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] btf_encoder: Add extra checks for symbol names
  2021-01-21 13:38   ` Arnaldo Carvalho de Melo
@ 2021-01-21 16:06     ` Sedat Dilek
  2021-01-21 19:37       ` Arnaldo Carvalho de Melo
  2021-01-21 20:53       ` Andrii Nakryiko
  2021-01-21 17:37     ` Tom Stellard
  1 sibling, 2 replies; 15+ messages in thread
From: Sedat Dilek @ 2021-01-21 16:06 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Tom Stellard, Jiri Olsa, dwarves, bpf, Alexei Starovoitov,
	Andrii Nakryiko, Yonghong Song, Hao Luo

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] btf_encoder: Add extra checks for symbol names
  2021-01-21 13:38   ` Arnaldo Carvalho de Melo
  2021-01-21 16:06     ` Sedat Dilek
@ 2021-01-21 17:37     ` Tom Stellard
  1 sibling, 0 replies; 15+ messages in thread
From: Tom Stellard @ 2021-01-21 17:37 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, dwarves, bpf, Alexei Starovoitov, Andrii Nakryiko,
	Yonghong Song, Hao Luo, Sedat Dilek

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;
>>>
>>
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] btf_encoder: Add extra checks for symbol names
  2021-01-21 16:06     ` Sedat Dilek
@ 2021-01-21 19:37       ` Arnaldo Carvalho de Melo
  2021-01-21 20:53       ` Andrii Nakryiko
  1 sibling, 0 replies; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-01-21 19:37 UTC (permalink / raw)
  To: Sedat Dilek
  Cc: Arnaldo Carvalho de Melo, Tom Stellard, Jiri Olsa, dwarves, bpf,
	Alexei Starovoitov, Andrii Nakryiko, Yonghong Song, Hao Luo

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

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] btf_encoder: Add extra checks for symbol names
  2021-01-21 16:06     ` Sedat Dilek
  2021-01-21 19:37       ` Arnaldo Carvalho de Melo
@ 2021-01-21 20:53       ` Andrii Nakryiko
  2021-01-22  2:07         ` Sedat Dilek
  1 sibling, 1 reply; 15+ messages in thread
From: Andrii Nakryiko @ 2021-01-21 20:53 UTC (permalink / raw)
  To: Sedat Dilek
  Cc: Arnaldo Carvalho de Melo, Tom Stellard, Jiri Olsa, dwarves, bpf,
	Alexei Starovoitov, Andrii Nakryiko, Yonghong Song, Hao Luo

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] btf_encoder: Add extra checks for symbol names
  2021-01-21 20:53       ` Andrii Nakryiko
@ 2021-01-22  2:07         ` Sedat Dilek
  2021-01-22  4:11           ` Andrii Nakryiko
  0 siblings, 1 reply; 15+ messages in thread
From: Sedat Dilek @ 2021-01-22  2:07 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Arnaldo Carvalho de Melo, Tom Stellard, Jiri Olsa, dwarves, bpf,
	Alexei Starovoitov, Andrii Nakryiko, Yonghong Song, Hao Luo

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] btf_encoder: Add extra checks for symbol names
  2021-01-22  2:07         ` Sedat Dilek
@ 2021-01-22  4:11           ` Andrii Nakryiko
  0 siblings, 0 replies; 15+ messages in thread
From: Andrii Nakryiko @ 2021-01-22  4:11 UTC (permalink / raw)
  To: Sedat Dilek
  Cc: Arnaldo Carvalho de Melo, Tom Stellard, Jiri Olsa, dwarves, bpf,
	Alexei Starovoitov, Andrii Nakryiko, Yonghong Song, Hao Luo

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, back to index

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-12 18:40 [PATCH] btf_encoder: Add extra checks for symbol names Jiri Olsa
2021-01-12 19:20 ` Andrii Nakryiko
2021-01-12 19:47   ` Jiri Olsa
2021-01-12 20:17     ` Sedat Dilek
2021-01-13  0:27 ` Tom Stellard
2021-01-14  7:50   ` Sedat Dilek
2021-01-14 14:33     ` Tom Stellard
2021-01-14 14:39       ` Sedat Dilek
2021-01-21 13:38   ` Arnaldo Carvalho de Melo
2021-01-21 16:06     ` Sedat Dilek
2021-01-21 19:37       ` Arnaldo Carvalho de Melo
2021-01-21 20:53       ` Andrii Nakryiko
2021-01-22  2:07         ` Sedat Dilek
2021-01-22  4:11           ` Andrii Nakryiko
2021-01-21 17:37     ` Tom Stellard

BPF Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/bpf/0 bpf/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 bpf bpf/ https://lore.kernel.org/bpf \
		bpf@vger.kernel.org
	public-inbox-index bpf

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.bpf


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git