Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 29 Jan 2019 10:07:50 +0100
From:   Arnaldo Carvalho de Melo <acme@kernel.org>
To:     Nick Clifton <nickc@redhat.com>, Jiri Olsa <jolsa@redhat.com>
Cc:     Jiri Olsa <jolsa@kernel.org>, lkml <linux-kernel@vger.kernel.org>,
        Ingo Molnar <mingo@kernel.org>,
        Namhyung Kim <namhyung@kernel.org>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Peter Zijlstra <a.p.zijlstra@chello.nl>,
        Michael Petlan <mpetlan@redhat.com>,
        Masami Hiramatsu <mhiramat@kernel.org>
Subject: Re: [PATCH] perf tools: Filter out hidden symbols from labels
Message-ID: <20190129090750.GE4344@kernel.org>
References: <20190115135354.11572-1-jolsa@kernel.org>
 <960f0b83-a623-67a6-ab55-d56139d10ba2@redhat.com>
 <20190115173838.GB31411@krava>
 <20190128133526.GD15461@krava>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20190128133526.GD15461@krava>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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