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

["Wangnan (F)" <wangnan0@huawei.com>]

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
>>