diff mbox

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

Message ID 1454403366-27055-3-git-send-email-wangnan0@huawei.com
State New
Headers show

Commit Message

Wang Nan Feb. 2, 2016, 8:56 a.m. UTC
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);
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@huawei.com>

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Cody P Schafer <dev@codyps.com>
Cc: He Kuang <hekuang@huawei.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kirill Smelkov <kirr@nexedi.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Li Zefan <lizefan@huawei.com>
Cc: pi3orama@163.com
---
 tools/perf/util/symbol-elf.c | 1 +
 1 file changed, 1 insertion(+)

-- 
1.8.3.4

Comments

Wang Nan Feb. 3, 2016, 2:12 a.m. UTC | #1
On 2016/2/2 23:18, Namhyung Kim wrote:
> 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/ ?


Thank you. Have you really tested this program?

>> 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@huawei.com>

>> Cc: Adrian Hunter <adrian.hunter@intel.com>

>> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>

>> Cc: Cody P Schafer <dev@codyps.com>

>> Cc: He Kuang <hekuang@huawei.com>

>> Cc: Jiri Olsa <jolsa@kernel.org>

>> Cc: Kirill Smelkov <kirr@nexedi.com>

>> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>

>> Cc: Namhyung Kim <namhyung@kernel.org>

>> Cc: Li Zefan <lizefan@huawei.com>

>> Cc: pi3orama@163.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..


OK. Let's make adjust_symbols the default choice.

Thank you.

> Thanks,

> Namhyung

>

>

>>   				dso__is_vdso(dso) ||

>>   				elf_section_by_name(elf, &ehdr, &shdr,

>>   						     ".gnu.prelink_undo",

>> -- 

>> 1.8.3.4

>>
diff mbox

Patch

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 ||
 				dso__is_vdso(dso) ||
 				elf_section_by_name(elf, &ehdr, &shdr,
 						     ".gnu.prelink_undo",