[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160407200608.GC5327@redhat.com>
Date: Thu, 7 Apr 2016 17:06:08 -0300
From: Arnaldo Carvalho de Melo <acme@...hat.com>
To: Wang Nan <wangnan0@...wei.com>
Cc: lizefan@...wei.com, pi3orama@....com, linux-kernel@...r.kernel.org,
Adrian Hunter <adrian.hunter@...el.com>,
Cody P Schafer <dev@...yps.com>,
He Kuang <hekuang@...wei.com>, Jiri Olsa <jolsa@...nel.org>,
Kirill Smelkov <kirr@...edi.com>,
Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
Subject: Re: [RESEND PATCH 2/2] perf tools: Adjust symbol for shared objects
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@huawei.com
>
> Signed-off-by: Wang Nan <wangnan0@...wei.com>
> Acked-by: Namhyung Kim <namhyung@...nel.org>
> Cc: Adrian Hunter <adrian.hunter@...el.com>
> Cc: Arnaldo Carvalho de Melo <acme@...hat.com>
> Cc: Cody P Schafer <dev@...yps.com>
> Cc: He Kuang <hekuang@...wei.com>
> Cc: Jiri Olsa <jolsa@...nel.org>
> Cc: Kirill Smelkov <kirr@...edi.com>
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
> Cc: Li Zefan <lizefan@...wei.com>
> Cc: pi3orama@....com
> ---
> 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
Powered by blists - more mailing lists