bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv2] btf_encoder: Add extra checks for symbol names
@ 2021-01-13 10:25 Jiri Olsa
  2021-01-13 17:20 ` Yonghong Song
  2021-01-14  7:45 ` Sedat Dilek
  0 siblings, 2 replies; 4+ messages in thread
From: Jiri Olsa @ 2021-01-13 10:25 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>
---
  v2 changes:
    - reorg the code based on Andrii's suggestion

 btf_encoder.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/btf_encoder.c b/btf_encoder.c
index 333973054b61..5557c9efd365 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -68,10 +68,14 @@ static int collect_function(struct btf_elf *btfe, GElf_Sym *sym)
 	struct elf_function *new;
 	static GElf_Shdr sh;
 	static int last_idx;
+	const char *name;
 	int idx;
 
 	if (elf_sym__type(sym) != STT_FUNC)
 		return 0;
+	name = elf_sym__name(sym, btfe->symtab);
+	if (!name)
+		return 0;
 
 	if (functions_cnt == functions_alloc) {
 		functions_alloc = max(1000, functions_alloc * 3 / 2);
@@ -94,7 +98,7 @@ static int collect_function(struct btf_elf *btfe, GElf_Sym *sym)
 		last_idx = idx;
 	}
 
-	functions[functions_cnt].name = elf_sym__name(sym, btfe->symtab);
+	functions[functions_cnt].name = name;
 	functions[functions_cnt].addr = elf_sym__value(sym);
 	functions[functions_cnt].sh_addr = sh.sh_addr;
 	functions[functions_cnt].generated = false;
@@ -731,8 +735,13 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
 			continue;
 		if (functions_cnt) {
 			struct elf_function *func;
+			const char *name;
+
+			name = function__name(fn, cu);
+			if (!name)
+				continue;
 
-			func = find_function(btfe, function__name(fn, cu));
+			func = find_function(btfe, name);
 			if (!func || func->generated)
 				continue;
 			func->generated = true;
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCHv2] btf_encoder: Add extra checks for symbol names
  2021-01-13 10:25 [PATCHv2] btf_encoder: Add extra checks for symbol names Jiri Olsa
@ 2021-01-13 17:20 ` Yonghong Song
  2021-01-13 18:27   ` Jiri Olsa
  2021-01-14  7:45 ` Sedat Dilek
  1 sibling, 1 reply; 4+ messages in thread
From: Yonghong Song @ 2021-01-13 17:20 UTC (permalink / raw)
  To: Jiri Olsa, Arnaldo Carvalho de Melo
  Cc: dwarves, bpf, Alexei Starovoitov, Andrii Nakryiko, Hao Luo,
	Sedat Dilek, Tom Stellard



On 1/13/21 2:25 AM, Jiri Olsa wrote:
> When processing kernel image build by clang we can
> find some functions without the name, which causes
> pahole to segfault.

Just curious. What kinds of functions gcc generates
names and clang doesn't?

> 
> Adding extra checks to make sure we always have
> function's name defined before using it.
> 
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>    v2 changes:
>      - reorg the code based on Andrii's suggestion
> 
>   btf_encoder.c | 13 +++++++++++--
>   1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/btf_encoder.c b/btf_encoder.c
> index 333973054b61..5557c9efd365 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -68,10 +68,14 @@ static int collect_function(struct btf_elf *btfe, GElf_Sym *sym)
>   	struct elf_function *new;
>   	static GElf_Shdr sh;
>   	static int last_idx;
> +	const char *name;
>   	int idx;
>   
>   	if (elf_sym__type(sym) != STT_FUNC)
>   		return 0;
> +	name = elf_sym__name(sym, btfe->symtab);
> +	if (!name)
> +		return 0;
>   
>   	if (functions_cnt == functions_alloc) {
>   		functions_alloc = max(1000, functions_alloc * 3 / 2);
> @@ -94,7 +98,7 @@ static int collect_function(struct btf_elf *btfe, GElf_Sym *sym)
>   		last_idx = idx;
>   	}
>   
> -	functions[functions_cnt].name = elf_sym__name(sym, btfe->symtab);
> +	functions[functions_cnt].name = name;
>   	functions[functions_cnt].addr = elf_sym__value(sym);
>   	functions[functions_cnt].sh_addr = sh.sh_addr;
>   	functions[functions_cnt].generated = false;
> @@ -731,8 +735,13 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
>   			continue;
>   		if (functions_cnt) {
>   			struct elf_function *func;
> +			const char *name;
> +
> +			name = function__name(fn, cu);
> +			if (!name)
> +				continue;
>   
> -			func = find_function(btfe, function__name(fn, cu));
> +			func = find_function(btfe, name);
>   			if (!func || func->generated)
>   				continue;
>   			func->generated = true;
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCHv2] btf_encoder: Add extra checks for symbol names
  2021-01-13 17:20 ` Yonghong Song
@ 2021-01-13 18:27   ` Jiri Olsa
  0 siblings, 0 replies; 4+ messages in thread
From: Jiri Olsa @ 2021-01-13 18:27 UTC (permalink / raw)
  To: Yonghong Song
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, dwarves, bpf,
	Alexei Starovoitov, Andrii Nakryiko, Hao Luo, Sedat Dilek,
	Tom Stellard

On Wed, Jan 13, 2021 at 09:20:17AM -0800, Yonghong Song wrote:
> 
> 
> On 1/13/21 2:25 AM, Jiri Olsa wrote:
> > When processing kernel image build by clang we can
> > find some functions without the name, which causes
> > pahole to segfault.
> 
> Just curious. What kinds of functions gcc generates
> names and clang doesn't?

can't say.. I guess we could compare nm output
of both gcc and clang vmlinux for same .config

jirka

> 
> > 
> > Adding extra checks to make sure we always have
> > function's name defined before using it.
> > 
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >    v2 changes:
> >      - reorg the code based on Andrii's suggestion
> > 
> >   btf_encoder.c | 13 +++++++++++--
> >   1 file changed, 11 insertions(+), 2 deletions(-)
> > 
> > diff --git a/btf_encoder.c b/btf_encoder.c
> > index 333973054b61..5557c9efd365 100644
> > --- a/btf_encoder.c
> > +++ b/btf_encoder.c
> > @@ -68,10 +68,14 @@ static int collect_function(struct btf_elf *btfe, GElf_Sym *sym)
> >   	struct elf_function *new;
> >   	static GElf_Shdr sh;
> >   	static int last_idx;
> > +	const char *name;
> >   	int idx;
> >   	if (elf_sym__type(sym) != STT_FUNC)
> >   		return 0;
> > +	name = elf_sym__name(sym, btfe->symtab);
> > +	if (!name)
> > +		return 0;
> >   	if (functions_cnt == functions_alloc) {
> >   		functions_alloc = max(1000, functions_alloc * 3 / 2);
> > @@ -94,7 +98,7 @@ static int collect_function(struct btf_elf *btfe, GElf_Sym *sym)
> >   		last_idx = idx;
> >   	}
> > -	functions[functions_cnt].name = elf_sym__name(sym, btfe->symtab);
> > +	functions[functions_cnt].name = name;
> >   	functions[functions_cnt].addr = elf_sym__value(sym);
> >   	functions[functions_cnt].sh_addr = sh.sh_addr;
> >   	functions[functions_cnt].generated = false;
> > @@ -731,8 +735,13 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
> >   			continue;
> >   		if (functions_cnt) {
> >   			struct elf_function *func;
> > +			const char *name;
> > +
> > +			name = function__name(fn, cu);
> > +			if (!name)
> > +				continue;
> > -			func = find_function(btfe, function__name(fn, cu));
> > +			func = find_function(btfe, name);
> >   			if (!func || func->generated)
> >   				continue;
> >   			func->generated = true;
> > 
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCHv2] btf_encoder: Add extra checks for symbol names
  2021-01-13 10:25 [PATCHv2] btf_encoder: Add extra checks for symbol names Jiri Olsa
  2021-01-13 17:20 ` Yonghong Song
@ 2021-01-14  7:45 ` Sedat Dilek
  1 sibling, 0 replies; 4+ messages in thread
From: Sedat Dilek @ 2021-01-14  7:45 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, dwarves, bpf, Alexei Starovoitov,
	Andrii Nakryiko, Yonghong Song, Hao Luo, Tom Stellard

On Wed, Jan 13, 2021 at 11:25 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>

Applied on top of latest pahole Git.

Tested-by: Sedat Dilek <sedat.dilek@gmail.com>

- Sedat -

> ---
>   v2 changes:
>     - reorg the code based on Andrii's suggestion
>
>  btf_encoder.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/btf_encoder.c b/btf_encoder.c
> index 333973054b61..5557c9efd365 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -68,10 +68,14 @@ static int collect_function(struct btf_elf *btfe, GElf_Sym *sym)
>         struct elf_function *new;
>         static GElf_Shdr sh;
>         static int last_idx;
> +       const char *name;
>         int idx;
>
>         if (elf_sym__type(sym) != STT_FUNC)
>                 return 0;
> +       name = elf_sym__name(sym, btfe->symtab);
> +       if (!name)
> +               return 0;
>
>         if (functions_cnt == functions_alloc) {
>                 functions_alloc = max(1000, functions_alloc * 3 / 2);
> @@ -94,7 +98,7 @@ static int collect_function(struct btf_elf *btfe, GElf_Sym *sym)
>                 last_idx = idx;
>         }
>
> -       functions[functions_cnt].name = elf_sym__name(sym, btfe->symtab);
> +       functions[functions_cnt].name = name;
>         functions[functions_cnt].addr = elf_sym__value(sym);
>         functions[functions_cnt].sh_addr = sh.sh_addr;
>         functions[functions_cnt].generated = false;
> @@ -731,8 +735,13 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
>                         continue;
>                 if (functions_cnt) {
>                         struct elf_function *func;
> +                       const char *name;
> +
> +                       name = function__name(fn, cu);
> +                       if (!name)
> +                               continue;
>
> -                       func = find_function(btfe, function__name(fn, cu));
> +                       func = find_function(btfe, name);
>                         if (!func || func->generated)
>                                 continue;
>                         func->generated = true;
> --
> 2.26.2
>

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-01-14  7:46 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-13 10:25 [PATCHv2] btf_encoder: Add extra checks for symbol names Jiri Olsa
2021-01-13 17:20 ` Yonghong Song
2021-01-13 18:27   ` Jiri Olsa
2021-01-14  7:45 ` Sedat Dilek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).