Re: [PATCH] perf tools: Filter out hidden symbols from labels

Arnaldo Carvalho de Melo
2019-01-29T09:07:55Z

Em Mon, Jan 28, 2019 at 02:35:26PM +0100, Jiri Olsa escreveu:
> On Tue, Jan 15, 2019 at 06:38:38PM +0100, Jiri Olsa wrote:
> > On Tue, Jan 15, 2019 at 04:13:16PM +0000, Nick Clifton wrote:
> > > Just to be awkward, if you are going to ignore STV_HIDDEN
> > > symbols then you should probably also ignore STV_INTERNAL ones
> > > as well... Annobin does not generate them, but you never know,
> > > one day some other tool might create some.

> > sounds good, thanks

> there were no objections for rfc, sending patch

I don't see a problem, Nick, can you provide an Acked-by, or better yet,
a Reviewed-by so that Jiri can collect in this patch and I can push it
to perf/urgent?

Thanks,

- Arnaldo

> thanks,
> jirka
>
>
> ---
> When perf is built with annobin plugin (RHEL8 build) extra symbols
> are added to its binary:
>
> # nm perf | grep annobin | head -10
> 0000000000241100 t .annobin_annotate.c
> 0000000000326490 t .annobin_annotate.c
> 0000000000249255 t .annobin_annotate.c_end
> 00000000003283a8 t .annobin_annotate.c_end
> 00000000001bce18 t .annobin_annotate.c_end.hot
> 00000000001bce18 t .annobin_annotate.c_end.hot
> 00000000001bc3e2 t .annobin_annotate.c_end.unlikely
> 00000000001bc400 t .annobin_annotate.c_end.unlikely
> 00000000001bce18 t .annobin_annotate.c.hot
> 00000000001bce18 t .annobin_annotate.c.hot
> ...
>
> those symbols have no use for report or annotation and should be skipped.
> Moreover they interfere with dwarf unwind test on ppc arch, where they
> are mixed with checked symbols and test fails:
>
> # perf test dwarf -v
> 59: Test dwarf unwind :
> --- start ---
> test child forked, pid 8515
> unwind: .annobin_dwarf_unwind.c:ip = 0x10dba40dc (0x2740dc)
> ...
> got: .annobin_dwarf_unwind.c 0x10dba40dc, expecting test__arch_unwind_sample
> unwind: failed with 'no error'
>
> The annobin symbols are defined as NOTYPE/LOCAL/HIDDEN:
>
> # readelf -s ./perf | grep annobin | head -1
> 40: 00000000001bce4f 0 NOTYPE LOCAL HIDDEN 13 .annobin_init.c
>
> They can still pass the check for the label symbol. Adding
> check for HIDDEN and INTERNAL (as suggested by Nick below)
> visibility and filter out such symbols.
>
> > Just to be awkward, if you are going to ignore STV_HIDDEN
> > symbols then you should probably also ignore STV_INTERNAL ones
> > as well... Annobin does not generate them, but you never know,
> > one day some other tool might create some.
>
> Link: http://lkml.kernel.org/n/tip-4yuna6qhhg0df3q147cjdyuu@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> tools/perf/util/symbol-elf.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index 66a84d5846c8..03cb8c6d620a 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -87,6 +87,11 @@ static inline uint8_t elf_sym__type(const GElf_Sym *sym)
> return GELF_ST_TYPE(sym->st_info);
> }
>
> +static inline uint8_t elf_sym__visibility(const GElf_Sym *sym)
> +{
> + return GELF_ST_VISIBILITY(sym->st_other);
> +}
> +
> #ifndef STT_GNU_IFUNC
> #define STT_GNU_IFUNC 10
> #endif
> @@ -111,7 +116,9 @@ static inline int elf_sym__is_label(const GElf_Sym *sym)
> return elf_sym__type(sym) == STT_NOTYPE &&
> sym->st_name != 0 &&
> sym->st_shndx != SHN_UNDEF &&
> - sym->st_shndx != SHN_ABS;
> + sym->st_shndx != SHN_ABS &&
> + elf_sym__visibility(sym) != STV_HIDDEN &&
> + elf_sym__visibility(sym) != STV_INTERNAL;
> }
>
> static bool elf_sym__filter(GElf_Sym *sym)
> --
> 2.17.2

--

- Arnaldo