LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] perf: unwind: fix segbase for libunwind.
@ 2015-04-01 14:08 Wang Nan
  2015-04-01 14:48 ` Ingo Molnar
  0 siblings, 1 reply; 5+ messages in thread
From: Wang Nan @ 2015-04-01 14:08 UTC (permalink / raw)
  To: acme, jolsa, namhyung; +Cc: mingo, lizefan, pi3orama, linux-kernel

Perf passes incorrect segbase and table_data to libunwind when
map->pgoff != 0, causes unwind failure. This patch fixes this problem.

segbase is an absolute offset from the head of object file, directly
read from ELF file. Original code computes corresponding virtual address
using map->start + segbase, doesn't consider map->pgoff. Which causes
libunwind read from incorrect offset.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
---
 tools/perf/util/unwind-libunwind.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/unwind-libunwind.c b/tools/perf/util/unwind-libunwind.c
index a78c280..c2a44fe 100644
--- a/tools/perf/util/unwind-libunwind.c
+++ b/tools/perf/util/unwind-libunwind.c
@@ -342,8 +342,8 @@ find_proc_info(unw_addr_space_t as, unw_word_t ip, unw_proc_info_t *pi,
 		di.format   = UNW_INFO_FORMAT_REMOTE_TABLE;
 		di.start_ip = map->start;
 		di.end_ip   = map->end;
-		di.u.rti.segbase    = map->start + segbase;
-		di.u.rti.table_data = map->start + table_data;
+		di.u.rti.segbase    = map->start - map->pgoff + segbase;
+		di.u.rti.table_data = map->start - map->pgoff + table_data;
 		di.u.rti.table_len  = fde_count * sizeof(struct table_entry)
 				      / sizeof(unw_word_t);
 		return dwarf_search_unwind_table(as, ip, &di, pi,
-- 
1.8.3.4


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] perf: unwind: fix segbase for libunwind.
  2015-04-01 14:08 [PATCH] perf: unwind: fix segbase for libunwind Wang Nan
@ 2015-04-01 14:48 ` Ingo Molnar
  2015-04-02  5:07   ` Wang Nan
  0 siblings, 1 reply; 5+ messages in thread
From: Ingo Molnar @ 2015-04-01 14:48 UTC (permalink / raw)
  To: Wang Nan; +Cc: acme, jolsa, namhyung, mingo, lizefan, pi3orama, linux-kernel


* Wang Nan <wangnan0@huawei.com> wrote:

> Perf passes incorrect segbase and table_data to libunwind when 
> map->pgoff != 0, causes unwind failure. This patch fixes this 
> problem.
> 
> segbase is an absolute offset from the head of object file, directly 
> read from ELF file. Original code computes corresponding virtual 
> address using map->start + segbase, doesn't consider map->pgoff. 
> Which causes libunwind read from incorrect offset.

What's the effect of this bug in practice?

Is there any before/after output you can show that demonstrates the 
fix?

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] perf: unwind: fix segbase for libunwind.
  2015-04-01 14:48 ` Ingo Molnar
@ 2015-04-02  5:07   ` Wang Nan
  2015-04-02  5:19     ` Wang Nan
  0 siblings, 1 reply; 5+ messages in thread
From: Wang Nan @ 2015-04-02  5:07 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: acme, jolsa, namhyung, mingo, lizefan, pi3orama, linux-kernel

On 2015/4/1 22:48, Ingo Molnar wrote:
> 
> * Wang Nan <wangnan0@huawei.com> wrote:
> 
>> Perf passes incorrect segbase and table_data to libunwind when 
>> map->pgoff != 0, causes unwind failure. This patch fixes this 
>> problem.
>>
>> segbase is an absolute offset from the head of object file, directly 
>> read from ELF file. Original code computes corresponding virtual 
>> address using map->start + segbase, doesn't consider map->pgoff. 
>> Which causes libunwind read from incorrect offset.
> 
> What's the effect of this bug in practice?
> 
> Is there any before/after output you can show that demonstrates the 
> fix?
> 

I found the problem when testing my '--map-adjustment' argument. I tried to
construct a test case for normal case, but it seems triggers futher bugs.

Following is the reproducing steps. The first two

Step 1: create a C file like this (libtest-load.c):

--------test-load.c-----------
#define DATA_SZ 65535
int data[DATA_SZ] = {1,1,2,3,5,8,13,};

int fib(int x)
{
        if (x >= DATA_SZ)
                return -1;
        if ((x == 0) || (x == 1))
                return 1;
        data[x] = fib(x - 1) + fib(x - 2);
        return data[x];
}
----------------------

Step 2: create a shared object with folowing ld-script (test-load.lds) using:

$ gcc -shared -fPIC ./test-load.c -Wl,-T./test-load.lds -O0 -o libtest-load.so

--------------test-load.lds---------------
SECTIONS
{
        .note.gnu.build-id : { *(.node.*) } :text
        .gnu.hash : { *(.gnu.hash) } :text
        .dynsym : { *(.dynsym) } :text
        .dynstr : { *(.dynstr) } :text
        .gnu.version : { *(.gnu.version) } :text
        .gnu.version_r : { *(.gnu.version_r) } :text
        .rela.dyn : { *(.rela.dyn) } :text
        .rela.plt : { *(.rela.plt) } :text
        .init : { *(.init) } :text
        .plt : { *(.plt) } :text
        .text : { *(.text) } :text
        .fini : { *(.fini) } :text
        .eh_frame_hdr : { *(.eh_frame_hdr) } :text
        .eh_frame : { *(.eh_frame) } :text

        .ctors : { *(.ctors) } :data
        .dtors : { *(.dtors) } :data
        .jcr : { *(.jcr) } :data
        .got : { *(.got) } :data
        .got.plt : { *(.got.plt) } :data
        .data : { *(.data) } :data
        .bss : { *(.bss) } :data
        .dynamic : { *(.dynamic) } :dynamic

}

PHDRS
{
        text PT_LOAD FLAGS(7);
        data PT_LOAD FLAGS(7);
        dynamic PT_DYNAMIC FLAGS(7);
        note PT_NOTE FLAGS(4);
}
-----------------------------

In my environment, I get a shared object with:

$ readelf -l ./libtest-load.so

Elf file type is DYN (Shared object file)
Entry point 0x390
There are 4 program headers, starting at offset 64

Program Headers:
  Type           Offset             VirtAddr           PhysAddr
                 FileSiz            MemSiz              Flags  Align
  LOAD           0x0000000000200000 0x0000000000000000 0x0000000000000000
                 0x00000000000005c4 0x00000000000005c4  RWE    200000
  LOAD           0x00000000002005c8 0x00000000000005c8 0x00000000000005c8
                 0x00000000000400a0 0x00000000000400b0  RWE    200000
  DYNAMIC        0x0000000000240678 0x0000000000040678 0x0000000000040678
                 0x0000000000000180 0x0000000000000180  RWE    8
  NOTE           0x0000000000000000 0x0000000000000000 0x0000000000000000
                 0x0000000000000000 0x0000000000000000  R      8

 Section to Segment mapping:
  Segment Sections...
   00     .gnu.hash .dynsym .dynstr .gnu.version .gnu.version_r .rela.plt .init .plt .text .fini .eh_frame_hdr .eh_frame .rela.dyn .note.gnu.build-id
   01     .ctors .dtors .jcr .got .got.plt .data .data.rel .bss
   02     .dynamic
   03

The goal of the first 2 steps is to create a shared object which will be mmap-ed with
pgoff != 0 (offset of a PT_LOAD segment != 0). I'm not sure which part of the ld scripts
leads to this, so I paste all of them.

Step 3: Use that shared object: compile following C file using:

$ gcc ./test.c -O0 -ltest-load -L.

--------- test.c ---------
#include <stdio.h>
extern int fib(int x);
int main()
{
	printf("%d\n", fib(30));
	return 0;
}
-----------------

Step 4: perf record:

$ perf record -g --call-graph=dwarf   ./a.out

Step 5: perf report:

$ perf report --stdio --no-children

# To display the perf.data header info, please use --header/--header-only options.
#
# Samples: 40  of event 'cycles'
# Event count (approx.): 28432005
#
# Overhead  Command  Shared Object     Symbol
# ........  .......  ................  ......................
#
    18.40%  a.out    libtest-load.so   [.] 0x00000000002004ee
              |
              ---0x7f17127964ee

    16.85%  a.out    libtest-load.so   [.] 0x00000000002004bf
              |
              ---0x7f17127964bf

    15.20%  a.out    libtest-load.so   [.] 0x00000000002004c2
              |
              ---0x7f17127964c2

     9.33%  a.out    libtest-load.so   [.] 0x000000000020049c
              |
              ---0x7f171279649c


Perf failed to extract symbol name and also failed to unwind.


With my patch, perf unwind works:

#
# Overhead  Command  Shared Object     Symbol
# ........  .......  ................  ......................
#
    18.40%  a.out    libtest-load.so   [.] 0x00000000002004ee
              |
              ---0x7f17127964ee
                 |
                 |--50.52%-- 0x7f17127964cc
                 |          |
                 |          |--64.45%-- 0x7f17127964cc
                 |          |          0x7f17127964db
                 |          |          |
                 |          |          |--98.69%-- 0x7f17127964db
                 |          |          |          |
                 |          |          |          |--93.86%-- 0x7f17127964db
                 |          |          |          |          0x7f17127964cc
                 |          |          |          |          0x7f17127964cc
                 |          |          |          |          0x7f17127964cc
                 |          |          |          |          0x7f17127964db
                 |          |          |          |          0x7f17127964cc
                 |          |          |          |          0x7f17127964cc
                 |          |          |          |          0x7f17127964cc
                 |          |          |          |          0x7f17127964db
                 |          |          |          |          0x7f17127964cc
                 |          |          |          |          0x7f17127964db
                 |          |          |          |          0x7f17127964cc
                 |          |          |          |          0x7f17127964cc
                 |          |          |          |          0x7f17127964db
                 |          |          |          |          0x7f17127964db
                 |          |          |          |          0x7f17127964cc
                 |          |          |          |          main
                 |          |          |          |          __libc_start_main
                 |          |          |          |          _start
                 |          |          |          |
                 |          |          |           --6.14%-- 0x7f17127964cc

However, still unable to find symbol names.

> Thanks,
> 
> 	Ingo
> 



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] perf: unwind: fix segbase for libunwind.
  2015-04-02  5:07   ` Wang Nan
@ 2015-04-02  5:19     ` Wang Nan
  2015-04-02  8:11       ` Namhyung Kim
  0 siblings, 1 reply; 5+ messages in thread
From: Wang Nan @ 2015-04-02  5:19 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: acme, jolsa, namhyung, mingo, lizefan, pi3orama, linux-kernel

On 2015/4/2 13:07, Wang Nan wrote:
> On 2015/4/1 22:48, Ingo Molnar wrote:
>>
>> * Wang Nan <wangnan0@huawei.com> wrote:
>>
>>> Perf passes incorrect segbase and table_data to libunwind when 
>>> map->pgoff != 0, causes unwind failure. This patch fixes this 
>>> problem.
>>>
>>> segbase is an absolute offset from the head of object file, directly 
>>> read from ELF file. Original code computes corresponding virtual 
>>> address using map->start + segbase, doesn't consider map->pgoff. 
>>> Which causes libunwind read from incorrect offset.
>>
>> What's the effect of this bug in practice?
>>
>> Is there any before/after output you can show that demonstrates the 
>> fix?
>>
> 
> I found the problem when testing my '--map-adjustment' argument. I tried to
> construct a test case for normal case, but it seems triggers futher bugs.
> 
> Following is the reproducing steps. The first two
> 
> Step 1: create a C file like this (libtest-load.c):
> 
> --------test-load.c-----------
> #define DATA_SZ 65535
> int data[DATA_SZ] = {1,1,2,3,5,8,13,};
> 
> int fib(int x)
> {
>         if (x >= DATA_SZ)
>                 return -1;
>         if ((x == 0) || (x == 1))
>                 return 1;
>         data[x] = fib(x - 1) + fib(x - 2);
>         return data[x];
> }
> ----------------------
> 
> Step 2: create a shared object with folowing ld-script (test-load.lds) using:
> 
> $ gcc -shared -fPIC ./test-load.c -Wl,-T./test-load.lds -O0 -o libtest-load.so
> 
> --------------test-load.lds---------------
> SECTIONS
> {
>         .note.gnu.build-id : { *(.node.*) } :text
>         .gnu.hash : { *(.gnu.hash) } :text
>         .dynsym : { *(.dynsym) } :text
>         .dynstr : { *(.dynstr) } :text
>         .gnu.version : { *(.gnu.version) } :text
>         .gnu.version_r : { *(.gnu.version_r) } :text
>         .rela.dyn : { *(.rela.dyn) } :text
>         .rela.plt : { *(.rela.plt) } :text
>         .init : { *(.init) } :text
>         .plt : { *(.plt) } :text
>         .text : { *(.text) } :text
>         .fini : { *(.fini) } :text
>         .eh_frame_hdr : { *(.eh_frame_hdr) } :text
>         .eh_frame : { *(.eh_frame) } :text
> 
>         .ctors : { *(.ctors) } :data
>         .dtors : { *(.dtors) } :data
>         .jcr : { *(.jcr) } :data
>         .got : { *(.got) } :data
>         .got.plt : { *(.got.plt) } :data
>         .data : { *(.data) } :data
>         .bss : { *(.bss) } :data
>         .dynamic : { *(.dynamic) } :dynamic
> 
> }
> 
> PHDRS
> {
>         text PT_LOAD FLAGS(7);
>         data PT_LOAD FLAGS(7);
>         dynamic PT_DYNAMIC FLAGS(7);
>         note PT_NOTE FLAGS(4);
> }
> -----------------------------
> 
> In my environment, I get a shared object with:
> 
> $ readelf -l ./libtest-load.so
> 
> Elf file type is DYN (Shared object file)
> Entry point 0x390
> There are 4 program headers, starting at offset 64
> 
> Program Headers:
>   Type           Offset             VirtAddr           PhysAddr
>                  FileSiz            MemSiz              Flags  Align
>   LOAD           0x0000000000200000 0x0000000000000000 0x0000000000000000
>                  0x00000000000005c4 0x00000000000005c4  RWE    200000
>   LOAD           0x00000000002005c8 0x00000000000005c8 0x00000000000005c8
>                  0x00000000000400a0 0x00000000000400b0  RWE    200000
>   DYNAMIC        0x0000000000240678 0x0000000000040678 0x0000000000040678
>                  0x0000000000000180 0x0000000000000180  RWE    8
>   NOTE           0x0000000000000000 0x0000000000000000 0x0000000000000000
>                  0x0000000000000000 0x0000000000000000  R      8
> 
>  Section to Segment mapping:
>   Segment Sections...
>    00     .gnu.hash .dynsym .dynstr .gnu.version .gnu.version_r .rela.plt .init .plt .text .fini .eh_frame_hdr .eh_frame .rela.dyn .note.gnu.build-id
>    01     .ctors .dtors .jcr .got .got.plt .data .data.rel .bss
>    02     .dynamic
>    03
> 
> The goal of the first 2 steps is to create a shared object which will be mmap-ed with
> pgoff != 0 (offset of a PT_LOAD segment != 0). I'm not sure which part of the ld scripts
> leads to this, so I paste all of them.
> 
> Step 3: Use that shared object: compile following C file using:
> 
> $ gcc ./test.c -O0 -ltest-load -L.
> 
> --------- test.c ---------
> #include <stdio.h>
> extern int fib(int x);
> int main()
> {
> 	printf("%d\n", fib(30));
> 	return 0;
> }
> -----------------
> 
> Step 4: perf record:
> 
> $ perf record -g --call-graph=dwarf   ./a.out
> 
> Step 5: perf report:
> 
> $ perf report --stdio --no-children
> 
> # To display the perf.data header info, please use --header/--header-only options.
> #
> # Samples: 40  of event 'cycles'
> # Event count (approx.): 28432005
> #
> # Overhead  Command  Shared Object     Symbol
> # ........  .......  ................  ......................
> #
>     18.40%  a.out    libtest-load.so   [.] 0x00000000002004ee
>               |
>               ---0x7f17127964ee
> 
>     16.85%  a.out    libtest-load.so   [.] 0x00000000002004bf
>               |
>               ---0x7f17127964bf
> 
>     15.20%  a.out    libtest-load.so   [.] 0x00000000002004c2
>               |
>               ---0x7f17127964c2
> 
>      9.33%  a.out    libtest-load.so   [.] 0x000000000020049c
>               |
>               ---0x7f171279649c
> 
> 
> Perf failed to extract symbol name and also failed to unwind.
> 
> 
> With my patch, perf unwind works:
> 
> #
> # Overhead  Command  Shared Object     Symbol
> # ........  .......  ................  ......................
> #
>     18.40%  a.out    libtest-load.so   [.] 0x00000000002004ee
>               |
>               ---0x7f17127964ee
>                  |
>                  |--50.52%-- 0x7f17127964cc
>                  |          |
>                  |          |--64.45%-- 0x7f17127964cc
>                  |          |          0x7f17127964db
>                  |          |          |
>                  |          |          |--98.69%-- 0x7f17127964db
>                  |          |          |          |
>                  |          |          |          |--93.86%-- 0x7f17127964db
>                  |          |          |          |          0x7f17127964cc
>                  |          |          |          |          0x7f17127964cc
>                  |          |          |          |          0x7f17127964cc
>                  |          |          |          |          0x7f17127964db
>                  |          |          |          |          0x7f17127964cc
>                  |          |          |          |          0x7f17127964cc
>                  |          |          |          |          0x7f17127964cc
>                  |          |          |          |          0x7f17127964db
>                  |          |          |          |          0x7f17127964cc
>                  |          |          |          |          0x7f17127964db
>                  |          |          |          |          0x7f17127964cc
>                  |          |          |          |          0x7f17127964cc
>                  |          |          |          |          0x7f17127964db
>                  |          |          |          |          0x7f17127964db
>                  |          |          |          |          0x7f17127964cc
>                  |          |          |          |          main
>                  |          |          |          |          __libc_start_main
>                  |          |          |          |          _start
>                  |          |          |          |
>                  |          |          |           --6.14%-- 0x7f17127964cc
> 
> However, still unable to find symbol names.
> 
>> Thanks,
>>
>> 	Ingo
>>
> 


Additional information:

With following patch it seems to work:

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 476268c..1177f02 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1049,8 +1049,10 @@ new_symbol:
                        if (demangled != NULL)
                                elf_name = demangled;
                }
-               f = symbol__new(sym.st_value, sym.st_size,
+               f = symbol__new(sym.st_value + map->pgoff, sym.st_size,
                                GELF_ST_BIND(sym.st_info), elf_name);
                free(demangled);
                if (!f)
                        goto out_elf_end;


result:

 Overhead  Command  Shared Object     Symbol
# ........  .......  ................  ......................
#
    86.37%  a.out    libtest-load.so   [.] fib
              |
              ---fib
                 fib
                 fib
                 fib
                 fib
                 fib
                 fib
                 fib
                 fib
                 fib
                 fib
                 fib
                 fib
                 fib
                 |
                 |--94.35%-- fib
                 |          fib
                 |          |
                 |          |--93.42%-- fib
                 |          |          |
                 |          |          |--93.12%-- fib
                 |          |          |          fib
                 |          |          |          |
                 |          |          |          |--84.90%-- fib
                 |          |          |          |          |
                 |          |          |          |          |--74.34%-- fib
                 |          |          |          |          |          |
                 |          |          |          |          |          |--90.56%-- fib
                 |          |          |          |          |          |          |
                 |          |          |          |          |          |          |--57.37%-- fib
                 |          |          |          |          |          |          |          |
                 |          |          |          |          |          |          |          |--83.51%-- main
                 |          |          |          |          |          |          |          |          __libc_start_main
                 |          |          |          |          |          |          |          |          _start
                 |          |          |          |          |          |          |          |
                 |          |          |          |          |          |          |           --16.49%-- fib
                 |          |          |          |          |          |          |                     main
                 |          |          |          |          |          |          |                     __libc_start_main
                 |          |          |          |          |          |          |                     _start
...

However, I think this is only a temporary solutsion. What we need is to obay both
Offset and VirtAddr in PHDR.

Thank you.



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] perf: unwind: fix segbase for libunwind.
  2015-04-02  5:19     ` Wang Nan
@ 2015-04-02  8:11       ` Namhyung Kim
  0 siblings, 0 replies; 5+ messages in thread
From: Namhyung Kim @ 2015-04-02  8:11 UTC (permalink / raw)
  To: Wang Nan; +Cc: Ingo Molnar, acme, jolsa, mingo, lizefan, pi3orama, linux-kernel

On Thu, Apr 02, 2015 at 01:19:24PM +0800, Wang Nan wrote:
> Additional information:
> 
> With following patch it seems to work:
> 
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index 476268c..1177f02 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -1049,8 +1049,10 @@ new_symbol:
>                         if (demangled != NULL)
>                                 elf_name = demangled;
>                 }
> -               f = symbol__new(sym.st_value, sym.st_size,
> +               f = symbol__new(sym.st_value + map->pgoff, sym.st_size,
>                                 GELF_ST_BIND(sym.st_info), elf_name);
>                 free(demangled);
>                 if (!f)
>                         goto out_elf_end;
> 
> 
> result:
[SNIP]
> 
> However, I think this is only a temporary solutsion. What we need is to obay both
> Offset and VirtAddr in PHDR.

Agreed.

Thanks,
Namhyung

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-04-02  8:17 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-01 14:08 [PATCH] perf: unwind: fix segbase for libunwind Wang Nan
2015-04-01 14:48 ` Ingo Molnar
2015-04-02  5:07   ` Wang Nan
2015-04-02  5:19     ` Wang Nan
2015-04-02  8:11       ` Namhyung Kim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).