Re: [RESEND PATCH 2/2] perf tools: Adjust symbol for shared objects

[Arnaldo Carvalho de Melo]

Em Thu, Apr 07, 2016 at 10:24:31AM +0000, Wang Nan escreveu:
> He Kuang reported a problem that perf fails to get correct symbol on
> Android platform in [1]. The problem can be reproduced on normal x86_64
> platform. I will describe the reproducing steps in detail at the end of
> commit message.
> 
> The reason of this problem is the missing of symbol adjustment for normal
> shared objects. In most of the cases skipping adjustment is okay. However,
> when '.text' section have different 'address' and 'offset' the result is wrong.
> I checked all shared objects in my working platform, only wine dll objects and
> debug objects (in .debug) have this problem. However, it is common on Android.
> For example:
> 
>  $ readelf -S ./libsurfaceflinger.so | grep \.text
>    [10] .text             PROGBITS         0000000000029030  00012030

I saved the output of 'perf report' --tui and --stdio before this patch
and after it, it seems to have fixed a few issues with a 'fixdep' binary
generated by by the perf build, things like:

--- perf.hist.before	2016-04-07 16:44:18.220217602 -0300
+++ perf.hist.3	2016-04-07 16:51:18.320693627 -0300
@@ -3538,6 +3538,7 @@
      0.01%     0.01%  cc1              cc1                                 [.] cselib_hash_rtx
+     0.01%     0.00%  fixdep           fixdep                              [.] main
      0.01%     0.00%  CompositorTileW  chrome                              [.] 0xffffaaa40bca467f
@@ -3549,6 +3550,7 @@
      0.01%     0.00%  ld               [unknown]                           [.] 0x0000559d84243798
+     0.01%     0.00%  fixdep           fixdep                              [.] print_deps
      0.01%     0.01%  cc1              [kernel.vmlinux]                    [k] memset_erms
@@ -5916,8 +5918,6 @@
      0.01%     0.00%  grep             [kernel.vmlinux]                    [k] handle_mm_fault
-     0.01%     0.00%  fixdep           fixdep                              [.] 0xffffffffffc00c09
-     0.01%     0.00%  fixdep           fixdep                              [.] 0xffffffffffc00c7c
      0.01%     0.01%  CompositorTileW  chrome                              [.] 0x00000000013ba770

 
> This patch enables symbol adjustment for dynamic objects so the symbol
> address got from elfutils would be adjusted correctly.
> 
> Now nearly all types of ELF files should adjust symbols. Makes
> ss->adjust_symbols default to true.
> 
> Steps to reproduce the problem:

Followed them and found the same results as you describe, thanks a lot for doing this!

- Arnaldo
 
>  $ cat ./Makefile
> PWD := $(shell pwd)
> LDFLAGS += "-Wl,-rpath=$(PWD)"
> CFLAGS += -g
> main: main.c libbuggy.so
> libbuggy.so: buggy.c
> 	gcc -g -shared -fPIC -Wl,-Ttext-segment=0x200000 $< -o $@
> clean:
> 	rm -rf main libbuggy.so *.o
> 
>  $ cat ./buggy.c
>  int fib(int x)
>  {
>      return (x == 0) ? 1 : (x == 1) ? 1 : fib(x - 1) + fib(x - 2);
>  }
> 
>  $ cat ./main.c
>  #include <stdio.h>
> 
>  extern int fib(int x);
>  int main()
>  {
>      int i;
> 
>      for (i = 0; i < 40; i++)
>          printf("%d\n", fib(i));
>      return 0;
>  }
> 
>  $ make
>  $ perf record ./main
>  ...
>  $ perf report --stdio
>  # Overhead  Command  Shared Object      Symbol
>  # ........  .......  .................  ...............................
>  #
>      14.97%  main     libbuggy.so        [.] 0x000000000000066c
>       8.68%  main     libbuggy.so        [.] 0x00000000000006aa
>       8.52%  main     libbuggy.so        [.] fib@plt
>       7.95%  main     libbuggy.so        [.] 0x0000000000000664
>       5.94%  main     libbuggy.so        [.] 0x00000000000006a9
>       5.35%  main     libbuggy.so        [.] 0x0000000000000678
>  ...
> 
> The correct result should be (after this patch):
> 
>  # Overhead  Command  Shared Object      Symbol
>  # ........  .......  .................  ...............................
>  #
>      91.47%  main     libbuggy.so        [.] fib
>       8.52%  main     libbuggy.so        [.] fib@plt
>       0.00%  main     [kernel.kallsyms]  [k] kmem_cache_free
> 
> [1] http://lkml.kernel.org/g/1452567507-54013-1-git-send-email-hekuang@xxxxxxxxxx
> 
> Signed-off-by: Wang Nan <wangnan0@xxxxxxxxxx>
> Acked-by: Namhyung Kim <namhyung@xxxxxxxxxx>
> Cc: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> Cc: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> Cc: Cody P Schafer <dev@xxxxxxxxxx>
> Cc: He Kuang <hekuang@xxxxxxxxxx>
> Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
> Cc: Kirill Smelkov <kirr@xxxxxxxxxx>
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@xxxxxxxxxxx>
> Cc: Li Zefan <lizefan@xxxxxxxxxx>
> Cc: pi3orama@xxxxxxx
> ---
>  tools/perf/util/symbol-elf.c | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index bc229a7..3f9d679 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -709,17 +709,10 @@ int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
>  	if (ss->opdshdr.sh_type != SHT_PROGBITS)
>  		ss->opdsec = NULL;
>  
> -	if (dso->kernel == DSO_TYPE_USER) {
> -		GElf_Shdr shdr;
> -		ss->adjust_symbols = (ehdr.e_type == ET_EXEC ||
> -				ehdr.e_type == ET_REL ||
> -				dso__is_vdso(dso) ||
> -				elf_section_by_name(elf, &ehdr, &shdr,
> -						     ".gnu.prelink_undo",
> -						     NULL) != NULL);
> -	} else {
> +	if (dso->kernel == DSO_TYPE_USER)
> +		ss->adjust_symbols = true;
> +	else
>  		ss->adjust_symbols = elf__needs_adjust_symbols(ehdr);
> -	}
>  
>  	ss->name   = strdup(name);
>  	if (!ss->name) {
> -- 
> 1.8.3.4