[bug] wrong result of android callchain

He Kuang

author

2016-01-05 09:16 UTC #0 lore raw

I found a wrong result of aarch64 callchain when using perf script on a android phone.

Here's the callchain record fragment from the output of perf script:

init 369 [002] 339.970607: raw_syscalls:sys_enter: NR 22 (b, 7fd9e360a0, 10, ffffffff, 0, 8)
...
230ac [unknown] (/system/lib64/libsurfaceflinger.so)
11a0 main (/system/bin/surfaceflinger)
1c3fc __libc_init (/system/lib64/libc.so)
fd0 _start (/system/bin/surfaceflinger)
29ec __dl__start (/system/bin/linker64)

The fault occured in the '[unknown]' line, from objdump result of /system/bin/surfaceflinger, we can see the branch instruction before 0x11a0:

 # objdump /system/bin/surfaceflinger
    1198:       f9400fe0        ldr     x0, [sp,#24]
    119c:       97ffff05        bl      db0 <_ZN7android14SurfaceFlinger3runEv@plt>
    11a0:       f9400be8        ldr     x8, [sp,#16]
    11a4:       b40000c8        cbz     x8, 11bc <main+0x150>

The function '_ZN7android14SurfaceFlinger3runEv' is located at 0x3a094 ~ 0x3a0ac in libsurfaceflinger.so, but perf misparsed that value to 0x230ac:

 # objdump libsurfaceflinger.so
  000000000003a094 <_ZN7android14SurfaceFlinger3runEv>:
    3a094:       a9be4ff4        stp     x20, x19, [sp,#-32]!
    3a098:       a9017bfd        stp     x29, x30, [sp,#16]
    3a09c:       910043fd        add     x29, sp, #0x10
    3a0a0:       910c0013        add     x19, x0, #0x300
    3a0a4:       aa1303e0        mov     x0, x19
    3a0a8:       97fff12f        bl      36564 <_ZN7android12MessageQueue11waitMessageEv>
    3a0ac:       17fffffe        b       3a0a4 <_ZN7android14SurfaceFlinger3runEv+0x10>

There's a difference of 0x17000 between those two offsets, it seems that this value is the VirtAddr of this dynamic library.

 # readelf -a libsurfaceflinger.so
  Program Headers:
    Type           Offset             VirtAddr           PhysAddr
                   FileSiz            MemSiz              Flags  Align
    LOAD           0x0000000000000000 0x0000000000017000 0x0000000000017000
                   0x0000000000057258 0x0000000000057258  R E    1000

Peter Zijlstra

maintainer

2016-01-05 10:21 UTC #1 lore raw

Quoted 2 lines · He Kuang

I found a wrong result of aarch64 callchain when using perf script on a android phone.

Might help to include the AARGH64 people then.. seeing I have no clue about all that. Cc's added, email preserved etc..

Quoted 47 lines

Here's the callchain record fragment from the output of perf script:
init 369 [002] 339.970607: raw_syscalls:sys_enter: NR 22 (b, 7fd9e360a0, 10, ffffffff, 0, 8)
...
230ac [unknown] (/system/lib64/libsurfaceflinger.so)
11a0 main (/system/bin/surfaceflinger)
1c3fc __libc_init (/system/lib64/libc.so)
fd0 _start (/system/bin/surfaceflinger)
29ec __dl__start (/system/bin/linker64)
The fault occured in the '[unknown]' line, from objdump result of /system/bin/surfaceflinger, we can see the branch instruction before 0x11a0:
 # objdump /system/bin/surfaceflinger
    1198:       f9400fe0        ldr     x0, [sp,#24]
    119c:       97ffff05        bl      db0 <_ZN7android14SurfaceFlinger3runEv@plt>
    11a0:       f9400be8        ldr     x8, [sp,#16]
    11a4:       b40000c8        cbz     x8, 11bc <main+0x150>
The function '_ZN7android14SurfaceFlinger3runEv' is located at 0x3a094 ~ 0x3a0ac in libsurfaceflinger.so, but perf misparsed that value to 0x230ac:
 # objdump libsurfaceflinger.so
  000000000003a094 <_ZN7android14SurfaceFlinger3runEv>:
    3a094:       a9be4ff4        stp     x20, x19, [sp,#-32]!
    3a098:       a9017bfd        stp     x29, x30, [sp,#16]
    3a09c:       910043fd        add     x29, sp, #0x10
    3a0a0:       910c0013        add     x19, x0, #0x300
    3a0a4:       aa1303e0        mov     x0, x19
    3a0a8:       97fff12f        bl      36564 <_ZN7android12MessageQueue11waitMessageEv>
    3a0ac:       17fffffe        b       3a0a4 <_ZN7android14SurfaceFlinger3runEv+0x10>
There's a difference of 0x17000 between those two offsets, it seems that this value is the VirtAddr of this dynamic library.
 # readelf -a libsurfaceflinger.so
  Program Headers:
    Type           Offset             VirtAddr           PhysAddr
                   FileSiz            MemSiz              Flags  Align
    LOAD           0x0000000000000000 0x0000000000017000 0x0000000000017000
                   0x0000000000057258 0x0000000000057258  R E    1000

Will Deacon

2016-01-05 11:14 UTC #2 lore raw

Quoted 6 lines · Peter Zijlstra

Quoted 2 lines · He Kuang
I found a wrong result of aarch64 callchain when using perf script on a android phone.
Might help to include the AARGH64 people then.. seeing I have no clue about all that. Cc's added, email preserved etc..

It looks like the issue is in the perf tool, where it fails to resolve the address to a symbol correctly.

Quoted 38 lines

Quoted 38 lines
Here's the callchain record fragment from the output of perf script:
init 369 [002] 339.970607: raw_syscalls:sys_enter: NR 22 (b, 7fd9e360a0, 10, ffffffff, 0, 8)
...
230ac [unknown] (/system/lib64/libsurfaceflinger.so)
11a0 main (/system/bin/surfaceflinger)
1c3fc __libc_init (/system/lib64/libc.so)
fd0 _start (/system/bin/surfaceflinger)
29ec __dl__start (/system/bin/linker64)
The fault occured in the '[unknown]' line, from objdump result of /system/bin/surfaceflinger, we can see the branch instruction before 0x11a0:
 # objdump /system/bin/surfaceflinger
    1198:       f9400fe0        ldr     x0, [sp,#24]
    119c:       97ffff05        bl      db0 <_ZN7android14SurfaceFlinger3runEv@plt>
    11a0:       f9400be8        ldr     x8, [sp,#16]
    11a4:       b40000c8        cbz     x8, 11bc <main+0x150>
The function '_ZN7android14SurfaceFlinger3runEv' is located at 0x3a094 ~ 0x3a0ac in libsurfaceflinger.so, but perf misparsed that value to 0x230ac:
 # objdump libsurfaceflinger.so
  000000000003a094 <_ZN7android14SurfaceFlinger3runEv>:
    3a094:       a9be4ff4        stp     x20, x19, [sp,#-32]!
    3a098:       a9017bfd        stp     x29, x30, [sp,#16]
    3a09c:       910043fd        add     x29, sp, #0x10
    3a0a0:       910c0013        add     x19, x0, #0x300
    3a0a4:       aa1303e0        mov     x0, x19
    3a0a8:       97fff12f        bl      36564 <_ZN7android12MessageQueue11waitMessageEv>
    3a0ac:       17fffffe        b       3a0a4 <_ZN7android14SurfaceFlinger3runEv+0x10>
There's a difference of 0x17000 between those two offsets, it seems that this value is the VirtAddr of this dynamic library.

Quoted 6 lines

 # readelf -a libsurfaceflinger.so
  Program Headers:
    Type           Offset             VirtAddr           PhysAddr
                   FileSiz            MemSiz              Flags  Align
    LOAD           0x0000000000000000 0x0000000000017000 0x0000000000017000
                   0x0000000000057258 0x0000000000057258  R E    1000

This reminds me of:

http://lkml.kernel.org/r/[email protected]

but that was all vdso-specific. Can you look at what gets passed to objdump?

Will

He Kuang

author

2016-01-06 08:26 UTC #3 lore raw

PATCH 1/1 perf symbols: Fix wrong symbol offset according to p_addr

Perf missed the 'VirtAddr' value in elf program_headers when adjusting symbol address for dynamic libraries. This patch considers the p_addr value and gets the right symbol offset.

Before this patch, some symbols can not be right parsed on android phone:

init 369 [002] 339.970607: raw_syscalls:sys_enter: NR 22 (b, 7fd9e360a0, 10, ffffffff, 0, 8)
...
230ac [unknown] (/system/lib64/libsurfaceflinger.so)
11a0 main (/system/bin/surfaceflinger)
1c3fc __libc_init (/system/lib64/libc.so)
fd0 _start (/system/bin/surfaceflinger)
29ec __dl__start (/system/bin/linker64)

After this patch:

init 369 [002] 339.970607: raw_syscalls:sys_enter: NR 22 (b, 7fd9e360a0, 10, ffffffff, 0, 8)
...
3a0ac _ZN7android14SurfaceFlinger3runEv (/system/lib64/libsurfaceflinger.so)
11a0 main (/system/bin/surfaceflinger)
1c3fc __libc_init (/system/lib64/libc.so)
fd0 _start (/system/bin/surfaceflinger)
29ec __dl__start (/system/bin/linker64)

Signed-off-by He Kuang <[email protected]>

---
 tools/perf/util/dso.c        |  6 ++++++
 tools/perf/util/dso.h        |  1 +
 tools/perf/util/event.c      |  6 +++++-
 tools/perf/util/map.h        |  7 +++++--
 tools/perf/util/symbol-elf.c | 20 ++++++++++++++++++++
 5 files changed, 37 insertions(+), 3 deletions(-)

tools/perf/util/dso.c +6 -0

index e8e9a9d..93bf13a 100644
@@ -1063,6 +1063,7 @@ struct dso *dso__new(const char *name)
 		dso->needs_swap = DSO_SWAP__UNSET;
 		RB_CLEAR_NODE(&dso->rb_node);
 		dso->root = NULL;
+		dso->vaddr = 0;
 		INIT_LIST_HEAD(&dso->node);
 		INIT_LIST_HEAD(&dso->data.open_entry);
 		pthread_mutex_init(&dso->lock, NULL);

@@ -1367,3 +1368,8 @@ int dso__strerror_load(struct dso *dso, char *buf, size_t buflen)
 	scnprintf(buf, buflen, "%s", dso_load__error_str[idx]);
 	return 0;
 }
+
+u64 dso__get_vaddr(struct dso *dso)
+{
+	return dso->vaddr;
+}

tools/perf/util/dso.h +1 -0

index 45ec4d0..b6c894f 100644
@@ -166,6 +166,7 @@ struct dso {
 	const char	 *long_name;
 	u16		 long_name_len;
 	u16		 short_name_len;
+	u64		vaddr;
 	void		*dwfl;			/* DWARF debug info */
 	struct auxtrace_cache *auxtrace_cache;

tools/perf/util/event.c +5 -1

index cd61bb1..ef964c0 100644
@@ -1273,8 +1273,12 @@ try_again:
 		 * Kernel maps might be changed when loading symbols so loading
 		 * must be done prior to using kernel maps.
 		 */
+		map__load(al->map, machine->symbol_filter);
+
+		/* exclude kernel kallsyms */
 		if (load_map)
-			map__load(al->map, machine->symbol_filter);
+			al->map->dso->vaddr = 0;
+
 		al->addr = al->map->map_ip(al->map, al->addr);
 	}
 }

tools/perf/util/map.h +5 -2

index 7309d64..68ab703 100644
@@ -86,14 +86,17 @@ void map_groups__put(struct map_groups *mg);
 struct kmap *map__kmap(struct map *map);
 struct map_groups *map__kmaps(struct map *map);
 
+u64 dso__get_vaddr(struct dso *dso);
 static inline u64 map__map_ip(struct map *map, u64 ip)
 {
-	return ip - map->start + map->pgoff;
+	return ip - map->start + map->pgoff +
+		dso__get_vaddr(map->dso);
 }
 
 static inline u64 map__unmap_ip(struct map *map, u64 ip)
 {
-	return ip + map->start - map->pgoff;
+	return ip + map->start - map->pgoff -
+		dso__get_vaddr(map->dso);
 }
 
 static inline u64 identity__map_ip(struct map *map __maybe_unused, u64 ip)

tools/perf/util/symbol-elf.c +20 -0

index 562b8eb..de0129f 100644
@@ -15,6 +15,8 @@
 #define EM_AARCH64	183  /* ARM 64 bit */
 #endif
 
+static int elf_read_maps_ex(Elf * elf, bool exe, mapfn_t mapfn, void *data,
+			    struct map *map);
 
 #ifdef HAVE_CPLUS_DEMANGLE_SUPPORT
 extern char *cplus_demangle(const char *, int);

@@ -831,6 +833,10 @@ int dso__load_sym(struct dso *dso, struct map *map,
 	sec = syms_ss->symtab;
 	shdr = syms_ss->symshdr;
 
+	err = elf_read_maps_ex(elf, ehdr.e_type == ET_EXEC ||
+			       ehdr.e_type == ET_REL,
+			       NULL, NULL, map);
+
 	if (runtime_ss->opdsec)
 		opddata = elf_rawdata(runtime_ss->opdsec, NULL);

@@ -1116,6 +1122,13 @@ out_elf_end:
 
 static int elf_read_maps(Elf *elf, bool exe, mapfn_t mapfn, void *data)
 {
+
+	return elf_read_maps_ex(elf, exe, mapfn, data, NULL);
+}
+
+static int elf_read_maps_ex(Elf *elf, bool exe, mapfn_t mapfn, void *data,
+			    struct map *map)
+{
 	GElf_Phdr phdr;
 	size_t i, phdrnum;
 	int err;

@@ -1135,10 +1148,17 @@ static int elf_read_maps(Elf *elf, bool exe, mapfn_t mapfn, void *data)
 		} else {
 			if (!(phdr.p_flags & PF_R))
 				continue;
+			if (map && (phdr.p_flags & PF_X))
+				map->dso->vaddr = phdr.p_vaddr;
 		}
+
 		sz = min(phdr.p_memsz, phdr.p_filesz);
 		if (!sz)
 			continue;
+
+		if (!mapfn)
+			continue;
+
 		err = mapfn(phdr.p_vaddr, sz, phdr.p_offset, data);
 		if (err)
 			return err;

Signature · 1 line

1.8.5.2

Ingo Molnar

2016-01-11 11:25 UTC #4 lore raw

Re: [PATCH] perf symbols: Fix wrong symbol offset according to p_addr

Quoted 3 lines · He Kuang

Perf missed the 'VirtAddr' value in elf program_headers when adjusting symbol address for dynamic libraries. This patch considers the p_addr value and gets the right symbol offset.

s/gets/calculates

Quoted 2 lines

Before this patch, some symbols can not be right parsed on android phone:

s/Before this patch, some symbols are not parsed correctly on android phones:

Quoted 22 lines

init 369 [002] 339.970607: raw_syscalls:sys_enter: NR 22 (b, 7fd9e360a0, 10, ffffffff, 0, 8)
...
230ac [unknown] (/system/lib64/libsurfaceflinger.so)
11a0 main (/system/bin/surfaceflinger)
1c3fc __libc_init (/system/lib64/libc.so)
fd0 _start (/system/bin/surfaceflinger)
29ec __dl__start (/system/bin/linker64)
After this patch:
init 369 [002] 339.970607: raw_syscalls:sys_enter: NR 22 (b, 7fd9e360a0, 10, ffffffff, 0, 8)
...
3a0ac _ZN7android14SurfaceFlinger3runEv (/system/lib64/libsurfaceflinger.so)
11a0 main (/system/bin/surfaceflinger)
1c3fc __libc_init (/system/lib64/libc.so)
fd0 _start (/system/bin/surfaceflinger)
29ec __dl__start (/system/bin/linker64)

tools/perf/util/dso.h struct dso { · line 170

Quoted 5 lines

tools/perf/util/dso.h +1 -0

@@ -166,6 +166,7 @@ struct dso {
 	const char	 *long_name;
 	u16		 long_name_len;
 	u16		 short_name_len;
+	u64		vaddr;
 	void		*dwfl;			/* DWARF debug info */

btw., looks like there's some extra spaces in front of those other field definitions.

Also, might make sense to describe what 'vaddr' stands for precisely, the naming is pretty ambiguous.

Thanks,

Ingo

He Kuang

author

2016-01-12 03:05 UTC #5 lore raw

PATCH v2 1/1 perf symbols: Fix wrong symbol offset according to p_addr

Perf missed the 'VirtAddr' value in elf program_headers when adjusting symbol address for dynamic libraries. This patch considers the p_addr value and calculates the right symbol offset.

Before this patch, some symbols are not parsed correctly on android phone:

init 369 [002] 339.970607: raw_syscalls:sys_enter: NR 22 (b, 7fd9e360a0, 10, ffffffff, 0, 8)
...
230ac [unknown] (/system/lib64/libsurfaceflinger.so)
11a0 main (/system/bin/surfaceflinger)
1c3fc __libc_init (/system/lib64/libc.so)
fd0 _start (/system/bin/surfaceflinger)
29ec __dl__start (/system/bin/linker64)

After this patch:

init 369 [002] 339.970607: raw_syscalls:sys_enter: NR 22 (b, 7fd9e360a0, 10, ffffffff, 0, 8)
...
3a0ac _ZN7android14SurfaceFlinger3runEv (/system/lib64/libsurfaceflinger.so)
11a0 main (/system/bin/surfaceflinger)
1c3fc __libc_init (/system/lib64/libc.so)
fd0 _start (/system/bin/surfaceflinger)
29ec __dl__start (/system/bin/linker64)

Signed-off-by He Kuang <[email protected]>

---

Patch v1 to v2:
- Modify commit message by following Ingo's suggestion.
- Change ambiguous virable name 'vaddr' to 'load_virtaddr'.

---
 tools/perf/util/dso.c        |  6 ++++++
 tools/perf/util/dso.h        |  3 ++-
 tools/perf/util/event.c      |  6 +++++-
 tools/perf/util/map.h        |  7 +++++--
 tools/perf/util/symbol-elf.c | 20 ++++++++++++++++++++
 5 files changed, 38 insertions(+), 4 deletions(-)

tools/perf/util/dso.c +6 -0

index e8e9a9d..afc51ce 100644
@@ -1063,6 +1063,7 @@ struct dso *dso__new(const char *name)
 		dso->needs_swap = DSO_SWAP__UNSET;
 		RB_CLEAR_NODE(&dso->rb_node);
 		dso->root = NULL;
+		dso->load_virtaddr = 0;
 		INIT_LIST_HEAD(&dso->node);
 		INIT_LIST_HEAD(&dso->data.open_entry);
 		pthread_mutex_init(&dso->lock, NULL);

@@ -1367,3 +1368,8 @@ int dso__strerror_load(struct dso *dso, char *buf, size_t buflen)
 	scnprintf(buf, buflen, "%s", dso_load__error_str[idx]);
 	return 0;
 }
+
+u64 dso__get_load_virtaddr(struct dso *dso)
+{
+	return dso->load_virtaddr;
+}

tools/perf/util/dso.h +2 -1

index 45ec4d0..2a03b87 100644
@@ -166,7 +166,8 @@ struct dso {
 	const char	 *long_name;
 	u16		 long_name_len;
 	u16		 short_name_len;
-	void		*dwfl;			/* DWARF debug info */
+	u64		 load_virtaddr;		/* Load virtual address */
+	void		 *dwfl;			/* DWARF debug info */
 	struct auxtrace_cache *auxtrace_cache;
 
 	/* dso data file */

tools/perf/util/event.c +5 -1

index cd61bb1..7f1dbdf 100644
@@ -1273,8 +1273,12 @@ try_again:
 		 * Kernel maps might be changed when loading symbols so loading
 		 * must be done prior to using kernel maps.
 		 */
+		map__load(al->map, machine->symbol_filter);
+
+		/* exclude kernel kallsyms */
 		if (load_map)
-			map__load(al->map, machine->symbol_filter);
+			al->map->dso->load_virtaddr = 0;
+
 		al->addr = al->map->map_ip(al->map, al->addr);
 	}
 }

tools/perf/util/map.h +5 -2

index 7309d64..db284a4 100644
@@ -86,14 +86,17 @@ void map_groups__put(struct map_groups *mg);
 struct kmap *map__kmap(struct map *map);
 struct map_groups *map__kmaps(struct map *map);
 
+u64 dso__get_load_virtaddr(struct dso *dso);
 static inline u64 map__map_ip(struct map *map, u64 ip)
 {
-	return ip - map->start + map->pgoff;
+	return ip - map->start + map->pgoff +
+		dso__get_load_virtaddr(map->dso);
 }
 
 static inline u64 map__unmap_ip(struct map *map, u64 ip)
 {
-	return ip + map->start - map->pgoff;
+	return ip + map->start - map->pgoff -
+		dso__get_load_virtaddr(map->dso);
 }
 
 static inline u64 identity__map_ip(struct map *map __maybe_unused, u64 ip)

tools/perf/util/symbol-elf.c +20 -0

index 562b8eb..9a02b3e 100644
@@ -15,6 +15,8 @@
 #define EM_AARCH64	183  /* ARM 64 bit */
 #endif
 
+static int elf_read_maps_ex(Elf * elf, bool exe, mapfn_t mapfn, void *data,
+			    struct map *map);
 
 #ifdef HAVE_CPLUS_DEMANGLE_SUPPORT
 extern char *cplus_demangle(const char *, int);

@@ -831,6 +833,10 @@ int dso__load_sym(struct dso *dso, struct map *map,
 	sec = syms_ss->symtab;
 	shdr = syms_ss->symshdr;
 
+	err = elf_read_maps_ex(elf, ehdr.e_type == ET_EXEC ||
+			       ehdr.e_type == ET_REL,
+			       NULL, NULL, map);
+
 	if (runtime_ss->opdsec)
 		opddata = elf_rawdata(runtime_ss->opdsec, NULL);

@@ -1116,6 +1122,13 @@ out_elf_end:
 
 static int elf_read_maps(Elf *elf, bool exe, mapfn_t mapfn, void *data)
 {
+
+	return elf_read_maps_ex(elf, exe, mapfn, data, NULL);
+}
+
+static int elf_read_maps_ex(Elf *elf, bool exe, mapfn_t mapfn, void *data,
+			    struct map *map)
+{
 	GElf_Phdr phdr;
 	size_t i, phdrnum;
 	int err;

@@ -1135,10 +1148,17 @@ static int elf_read_maps(Elf *elf, bool exe, mapfn_t mapfn, void *data)
 		} else {
 			if (!(phdr.p_flags & PF_R))
 				continue;
+			if (map && (phdr.p_flags & PF_X))
+				map->dso->load_virtaddr = phdr.p_vaddr;
 		}
+
 		sz = min(phdr.p_memsz, phdr.p_filesz);
 		if (!sz)
 			continue;
+
+		if (!mapfn)
+			continue;
+
 		err = mapfn(phdr.p_vaddr, sz, phdr.p_offset, data);
 		if (err)
 			return err;

Signature · 1 line

1.8.5.2