linux-kernel - Re: [PATCH 2/2] perf tools: Adjust symbol for shared objects

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160202151814.GD4627@danjae.kornet>
Date:	Wed, 3 Feb 2016 00:18:14 +0900
From:	Namhyung Kim <namhyung@...nel.org>
To:	Wang Nan <wangnan0@...wei.com>
Cc:	acme@...nel.org, adrian.hunter@...el.com, dev@...yps.com,
	hekuang@...wei.com, kirr@...edi.com, linux-kernel@...r.kernel.org,
	Arnaldo Carvalho de Melo <acme@...hat.com>,
	Jiri Olsa <jolsa@...nel.org>,
	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
	Li Zefan <lizefan@...wei.com>, pi3orama@....com
Subject: Re: [PATCH 2/2] perf tools: Adjust symbol for shared objects

On Tue, Feb 02, 2016 at 08:56:06AM +0000, Wang Nan wrote:
> 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 it works correctly, but 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
> 
> This patch enables symbol adjustment for dynamic objects so the symbol
> address got from elfutils would be adjusted correctly.
> 
> Steps to reproduce the problem:
> 
>  $ cat << EOF > ./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
> EOF
> 
>  $ cat << EOF > ./buggy.c
> int fib(int x)
> {
> 	return (x == 0) ? 1 : (x == 1) ? 1 : fib(x - 1) + fib(x - 2);
> }
> EOF
> 
>  $ cat << EOF > ./main.c
>  #include <stdio.h>
> 
> extern int fix(int x);

s/fix/fib/ ?

> int main()
> {
> 	int i;
> 
> 	for (i = 0; i < 40; i++)
> 		printf("%d\n", fib(i));
> 	return 0;
> }
> EOF
> 
>  $ 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>
> 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: Namhyung Kim <namhyung@...nel.org>
> Cc: Li Zefan <lizefan@...wei.com>
> Cc: pi3orama@....com
> ---
>  tools/perf/util/symbol-elf.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index 5227186..d4e59dd 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -712,6 +712,7 @@ int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
>  		GElf_Shdr shdr;
>  		ss->adjust_symbols = (ehdr.e_type == ET_EXEC ||
>  				ehdr.e_type == ET_REL ||
> +				ehdr.e_type == ET_DYN ||

It seems there's no need to check the e_type anymore..

Thanks,
Namhyung


>  				dso__is_vdso(dso) ||
>  				elf_section_by_name(elf, &ehdr, &shdr,
>  						     ".gnu.prelink_undo",
> -- 
> 1.8.3.4
>