diff mbox series

[PATCHv2] btf_encoder: Add extra checks for symbol names

Message ID 20210113102509.1338601-1-jolsa@kernel.org (mailing list archive)
State Not Applicable
Delegated to: BPF
Headers show
Series [PATCHv2] btf_encoder: Add extra checks for symbol names | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Jiri Olsa Jan. 13, 2021, 10:25 a.m. UTC
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(-)

Comments

Yonghong Song Jan. 13, 2021, 5:20 p.m. UTC | #1
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;
>
Jiri Olsa Jan. 13, 2021, 6:27 p.m. UTC | #2
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;
> > 
>
Sedat Dilek Jan. 14, 2021, 7:45 a.m. UTC | #3
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
>
diff mbox series

Patch

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;