diff mbox series

libbpf: Fix VERSIONED_SYM_COUNT number parsing

Message ID 20201118211350.1493421-1-jolsa@kernel.org
State Superseded
Headers show
Series libbpf: Fix VERSIONED_SYM_COUNT number parsing | expand

Commit Message

Jiri Olsa Nov. 18, 2020, 9:13 p.m. UTC
We remove "other info" from "readelf -s --wide" output when
parsing GLOBAL_SYM_COUNT variable, which was added in [1].
But we don't do that for VERSIONED_SYM_COUNT and it's failing
the check_abi target on powerpc Fedora 33.

The extra "other info" wasn't problem for VERSIONED_SYM_COUNT
parsing until commit [2] added awk in the pipe, which assumes
that the last column is symbol, but it can be "other info".

Adding "other info" removal for VERSIONED_SYM_COUNT the same
way as we did for GLOBAL_SYM_COUNT parsing.

[1] aa915931ac3e ("libbpf: Fix readelf output parsing for Fedora")
[2] 746f534a4809 ("tools/libbpf: Avoid counting local symbols in ABI check")

Cc: Tony Ambardar <tony.ambardar@gmail.com>
Cc: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Fixes: 746f534a4809 ("tools/libbpf: Avoid counting local symbols in ABI check")
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/bpf/Makefile | 2 ++
 1 file changed, 2 insertions(+)

Comments

Andrii Nakryiko Nov. 19, 2020, 1:57 a.m. UTC | #1
On Wed, Nov 18, 2020 at 1:15 PM Jiri Olsa <jolsa@kernel.org> wrote:
>
> We remove "other info" from "readelf -s --wide" output when
> parsing GLOBAL_SYM_COUNT variable, which was added in [1].
> But we don't do that for VERSIONED_SYM_COUNT and it's failing
> the check_abi target on powerpc Fedora 33.
>
> The extra "other info" wasn't problem for VERSIONED_SYM_COUNT
> parsing until commit [2] added awk in the pipe, which assumes
> that the last column is symbol, but it can be "other info".
>
> Adding "other info" removal for VERSIONED_SYM_COUNT the same
> way as we did for GLOBAL_SYM_COUNT parsing.
>
> [1] aa915931ac3e ("libbpf: Fix readelf output parsing for Fedora")
> [2] 746f534a4809 ("tools/libbpf: Avoid counting local symbols in ABI check")
>
> Cc: Tony Ambardar <tony.ambardar@gmail.com>
> Cc: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Fixes: 746f534a4809 ("tools/libbpf: Avoid counting local symbols in ABI check")
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---

LGTM. For the future, though, please specify the destination tree: [PATCH bpf].

Acked-by: Andrii Nakryiko <andrii@kernel.org>


>  tools/lib/bpf/Makefile | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
> index 5f9abed3e226..55bd78b3496f 100644
> --- a/tools/lib/bpf/Makefile
> +++ b/tools/lib/bpf/Makefile
> @@ -146,6 +146,7 @@ GLOBAL_SYM_COUNT = $(shell readelf -s --wide $(BPF_IN_SHARED) | \
>                            awk '/GLOBAL/ && /DEFAULT/ && !/UND/ {print $$NF}' | \
>                            sort -u | wc -l)
>  VERSIONED_SYM_COUNT = $(shell readelf --dyn-syms --wide $(OUTPUT)libbpf.so | \
> +                             sed 's/\[.*\]//' | \
>                               awk '/GLOBAL/ && /DEFAULT/ && !/UND/ {print $$NF}' | \
>                               grep -Eo '[^ ]+@LIBBPF_' | cut -d@ -f1 | sort -u | wc -l)
>
> @@ -214,6 +215,7 @@ check_abi: $(OUTPUT)libbpf.so
>                     awk '/GLOBAL/ && /DEFAULT/ && !/UND/ {print $$NF}'|  \
>                     sort -u > $(OUTPUT)libbpf_global_syms.tmp;           \
>                 readelf --dyn-syms --wide $(OUTPUT)libbpf.so |           \
> +                   sed 's/\[.*\]//' |                                   \
>                     awk '/GLOBAL/ && /DEFAULT/ && !/UND/ {print $$NF}'|  \
>                     grep -Eo '[^ ]+@LIBBPF_' | cut -d@ -f1 |             \
>                     sort -u > $(OUTPUT)libbpf_versioned_syms.tmp;        \
> --
> 2.26.2
>
Jiri Olsa Nov. 19, 2020, 9:48 a.m. UTC | #2
On Wed, Nov 18, 2020 at 05:57:25PM -0800, Andrii Nakryiko wrote:
> On Wed, Nov 18, 2020 at 1:15 PM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > We remove "other info" from "readelf -s --wide" output when
> > parsing GLOBAL_SYM_COUNT variable, which was added in [1].
> > But we don't do that for VERSIONED_SYM_COUNT and it's failing
> > the check_abi target on powerpc Fedora 33.
> >
> > The extra "other info" wasn't problem for VERSIONED_SYM_COUNT
> > parsing until commit [2] added awk in the pipe, which assumes
> > that the last column is symbol, but it can be "other info".
> >
> > Adding "other info" removal for VERSIONED_SYM_COUNT the same
> > way as we did for GLOBAL_SYM_COUNT parsing.
> >
> > [1] aa915931ac3e ("libbpf: Fix readelf output parsing for Fedora")
> > [2] 746f534a4809 ("tools/libbpf: Avoid counting local symbols in ABI check")
> >
> > Cc: Tony Ambardar <tony.ambardar@gmail.com>
> > Cc: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
> > Cc: Aurelien Jarno <aurelien@aurel32.net>
> > Fixes: 746f534a4809 ("tools/libbpf: Avoid counting local symbols in ABI check")
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> 
> LGTM. For the future, though, please specify the destination tree: [PATCH bpf].

ugh, sry will do

jirka

> 
> Acked-by: Andrii Nakryiko <andrii@kernel.org>
> 
> 
> >  tools/lib/bpf/Makefile | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
> > index 5f9abed3e226..55bd78b3496f 100644
> > --- a/tools/lib/bpf/Makefile
> > +++ b/tools/lib/bpf/Makefile
> > @@ -146,6 +146,7 @@ GLOBAL_SYM_COUNT = $(shell readelf -s --wide $(BPF_IN_SHARED) | \
> >                            awk '/GLOBAL/ && /DEFAULT/ && !/UND/ {print $$NF}' | \
> >                            sort -u | wc -l)
> >  VERSIONED_SYM_COUNT = $(shell readelf --dyn-syms --wide $(OUTPUT)libbpf.so | \
> > +                             sed 's/\[.*\]//' | \
> >                               awk '/GLOBAL/ && /DEFAULT/ && !/UND/ {print $$NF}' | \
> >                               grep -Eo '[^ ]+@LIBBPF_' | cut -d@ -f1 | sort -u | wc -l)
> >
> > @@ -214,6 +215,7 @@ check_abi: $(OUTPUT)libbpf.so
> >                     awk '/GLOBAL/ && /DEFAULT/ && !/UND/ {print $$NF}'|  \
> >                     sort -u > $(OUTPUT)libbpf_global_syms.tmp;           \
> >                 readelf --dyn-syms --wide $(OUTPUT)libbpf.so |           \
> > +                   sed 's/\[.*\]//' |                                   \
> >                     awk '/GLOBAL/ && /DEFAULT/ && !/UND/ {print $$NF}'|  \
> >                     grep -Eo '[^ ]+@LIBBPF_' | cut -d@ -f1 |             \
> >                     sort -u > $(OUTPUT)libbpf_versioned_syms.tmp;        \
> > --
> > 2.26.2
> >
>
patchwork-bot+netdevbpf@kernel.org Nov. 19, 2020, 5 p.m. UTC | #3
Hello:

This patch was applied to bpf/bpf.git (refs/heads/master):

On Wed, 18 Nov 2020 22:13:50 +0100 you wrote:
> We remove "other info" from "readelf -s --wide" output when
> parsing GLOBAL_SYM_COUNT variable, which was added in [1].
> But we don't do that for VERSIONED_SYM_COUNT and it's failing
> the check_abi target on powerpc Fedora 33.
> 
> The extra "other info" wasn't problem for VERSIONED_SYM_COUNT
> parsing until commit [2] added awk in the pipe, which assumes
> that the last column is symbol, but it can be "other info".
> 
> [...]

Here is the summary with links:
  - libbpf: Fix VERSIONED_SYM_COUNT number parsing
    https://git.kernel.org/bpf/bpf/c/1fd6cee127e2

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
diff mbox series

Patch

diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
index 5f9abed3e226..55bd78b3496f 100644
--- a/tools/lib/bpf/Makefile
+++ b/tools/lib/bpf/Makefile
@@ -146,6 +146,7 @@  GLOBAL_SYM_COUNT = $(shell readelf -s --wide $(BPF_IN_SHARED) | \
 			   awk '/GLOBAL/ && /DEFAULT/ && !/UND/ {print $$NF}' | \
 			   sort -u | wc -l)
 VERSIONED_SYM_COUNT = $(shell readelf --dyn-syms --wide $(OUTPUT)libbpf.so | \
+			      sed 's/\[.*\]//' | \
 			      awk '/GLOBAL/ && /DEFAULT/ && !/UND/ {print $$NF}' | \
 			      grep -Eo '[^ ]+@LIBBPF_' | cut -d@ -f1 | sort -u | wc -l)
 
@@ -214,6 +215,7 @@  check_abi: $(OUTPUT)libbpf.so
 		    awk '/GLOBAL/ && /DEFAULT/ && !/UND/ {print $$NF}'|  \
 		    sort -u > $(OUTPUT)libbpf_global_syms.tmp;		 \
 		readelf --dyn-syms --wide $(OUTPUT)libbpf.so |		 \
+		    sed 's/\[.*\]//' |					 \
 		    awk '/GLOBAL/ && /DEFAULT/ && !/UND/ {print $$NF}'|  \
 		    grep -Eo '[^ ]+@LIBBPF_' | cut -d@ -f1 |		 \
 		    sort -u > $(OUTPUT)libbpf_versioned_syms.tmp; 	 \