[PATCH v2 0/2] perf probe fixes for ppc64le

[PATCH v2 0/2] perf probe fixes for ppc64le

[Naveen N. Rao]

This patchset fixes three issues found with perf probe on ppc64le:
1. 'perf test kallsyms' failure on ppc64le (reported by Michael
Ellerman). This was due to the symbols being fixed up during symbol
table load. This is fixed in patch 2 by delaying symbol fixup until
later.
2. perf probe function offset was being calculated from the local entry
point (LEP), which does not match user expectation when trying to look
at function disassembly output (reported by Ananth N). This is fixed for
kallsyms in patch 1 and for symbol table in patch 2.
3. perf probe failure with kretprobe when using kallsyms. This was
failing as we were specifying an offset. This is fixed in patch 1.

A few examples demonstrating the issues and the fix:

Example for issue (2):
--------------------
    # objdump -d vmlinux | grep -A8 \<_do_fork\>:
    c0000000000b6a00 <_do_fork>:
    c0000000000b6a00:	f7 00 4c 3c 	addis   r2,r12,247
    c0000000000b6a04:	00 86 42 38 	addi    r2,r2,-31232
    c0000000000b6a08:	a6 02 08 7c 	mflr    r0
    c0000000000b6a0c:	d0 ff 41 fb 	std     r26,-48(r1)
    c0000000000b6a10:	26 80 90 7d 	mfocrf  r12,8
    c0000000000b6a14:	d8 ff 61 fb 	std     r27,-40(r1)
    c0000000000b6a18:	e0 ff 81 fb 	std     r28,-32(r1)
    c0000000000b6a1c:	e8 ff a1 fb 	std     r29,-24(r1)
    # perf probe -v _do_fork+4
    probe-definition(0): _do_fork+4 
    symbol:_do_fork file:(null) line:0 offset:4 return:0 lazy:(null)
    0 arguments
    Looking at the vmlinux_path (8 entries long)
    Using /proc/kcore for kernel object code
    Using /proc/kallsyms for symbols
    Opening /sys/kernel/debug/tracing//kprobe_events write=1
    Writing event: p:probe/_do_fork _text+748044
    Added new event:
      probe:_do_fork       (on _do_fork+4)

    You can now use it in all perf tools, such as:

	    perf record -e probe:_do_fork -aR sleep 1

    # printf "%x\n" 748044
    b6a0c
    ^^^^^
This is offset from the LEP. With this, there is also no way to ever
probe between the GEP and the LEP.

With this patchset:
    # perf probe -v _do_fork+4
    probe-definition(0): _do_fork+4 
    symbol:_do_fork file:(null) line:0 offset:4 return:0 lazy:(null)
    0 arguments
    Looking at the vmlinux_path (8 entries long)
    Using /proc/kcore for kernel object code
    Using /proc/kallsyms for symbols
    Opening /sys/kernel/debug/tracing//kprobe_events write=1
    Writing event: p:probe/_do_fork _text+748036
    Added new event:
      probe:_do_fork       (on _do_fork+4)

    You can now use it in all perf tools, such as:

	    perf record -e probe:_do_fork -aR sleep 1

    # perf probe -v _do_fork
    probe-definition(0): _do_fork 
    symbol:_do_fork file:(null) line:0 offset:0 return:0 lazy:(null)
    0 arguments
    Looking at the vmlinux_path (8 entries long)
    Using /proc/kcore for kernel object code
    Using /proc/kallsyms for symbols
    Opening /sys/kernel/debug/tracing//kprobe_events write=1
    Writing event: p:probe/_do_fork _text+748040
    Added new event:
      probe:_do_fork       (on _do_fork)

    You can now use it in all perf tools, such as:

	    perf record -e probe:_do_fork -aR sleep 1

We only offset to the LEP if function entry is specified, otherwise, we
offset from the GEP.

Example for issue (3):
---------------------
Before patch:
    # perf probe -v _do_fork:%return
    probe-definition(0): _do_fork:%return 
    symbol:_do_fork file:(null) line:0 offset:0 return:1 lazy:(null)
    0 arguments
    Looking at the vmlinux_path (8 entries long)
    Using /proc/kcore for kernel object code
    Using /proc/kallsyms for symbols
    Opening /sys/kernel/debug/tracing//kprobe_events write=1
    Writing event: r:probe/_do_fork _do_fork+8
    Failed to write event: Invalid argument
      Error: Failed to add events. Reason: Invalid argument (Code: -22)

After patch:
    # perf probe _do_fork:%return
    Added new event:
      probe:_do_fork       (on _do_fork%return)

    You can now use it in all perf tools, such as:

	    perf record -e probe:_do_fork -aR sleep 1


Concept Acked-by: Michael Ellerman <mpe@ellerman.id.au>

Cc: Mark Wielaard <mjw@redhat.com>
Cc: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Balbir Singh <bsingharora@gmail.com>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>

Naveen N. Rao (2):
  perf tools: Fix kprobe and kretprobe handling with kallsyms on ppc64le
  perf tools: Fix kallsyms perf test on ppc64le

 tools/perf/arch/powerpc/util/sym-handling.c | 43 +++++++++++++++++++++--------
 tools/perf/util/probe-event.c               |  5 ++--
 tools/perf/util/probe-event.h               |  3 +-
 tools/perf/util/symbol-elf.c                |  7 +++--
 tools/perf/util/symbol.h                    |  3 +-
 5 files changed, 43 insertions(+), 18 deletions(-)

-- 
2.7.4

[PATCH v2 1/2] perf tools: Fix kprobe and kretprobe handling with kallsyms on ppc64le

[Naveen N. Rao]

So far, we used to treat probe point offsets as being offset from the
LEP. However, userspace applications (objdump/readelf) always show
disassembly and offsets from the function GEP. This is confusing to the
user as we will end up probing at an address different from what the
user expects when looking at the function disassembly with
readelf/objdump. Fix this by changing how we modify probe address with
perf.

If only the function name is provided, we assume the user needs the LEP.
Otherwise, if an offset is specified, we assume that the user knows the
exact address to probe based on function disassembly, and so we just
place the probe from the GEP offset.

Finally, kretprobe was also broken with kallsyms as we were trying to
specify an offset. This patch also fixes that issue.

Cc: Mark Wielaard <mjw@redhat.com>
Cc: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Balbir Singh <bsingharora@gmail.com>
Reported-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
v2: Removed un-necessary check for uprobes

 tools/perf/arch/powerpc/util/sym-handling.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/tools/perf/arch/powerpc/util/sym-handling.c b/tools/perf/arch/powerpc/util/sym-handling.c
index bbc1a50..6974ba0 100644
--- a/tools/perf/arch/powerpc/util/sym-handling.c
+++ b/tools/perf/arch/powerpc/util/sym-handling.c
@@ -71,12 +71,21 @@ void arch__fix_tev_from_maps(struct perf_probe_event *pev,
 			     struct probe_trace_event *tev, struct map *map)
 {
 	/*
-	 * ppc64 ABIv2 local entry point is currently always 2 instructions
-	 * (8 bytes) after the global entry point.
+	 * When probing at a function entry point, we normally always want the
+	 * LEP since that catches calls to the function through both the GEP and
+	 * the LEP. Hence, we would like to probe at an offset of 8 bytes if
+	 * the user only specified the function entry.
+	 *
+	 * However, if the user specifies an offset, we fall back to using the
+	 * GEP since all userspace applications (objdump/readelf) show function
+	 * disassembly with offsets from the GEP.
+	 *
+	 * In addition, we shouldn't specify an offset for kretprobes.
 	 */
-	if (!pev->uprobes && map->dso->symtab_type == DSO_BINARY_TYPE__KALLSYMS) {
-		tev->point.address += PPC64LE_LEP_OFFSET;
+	if (pev->point.offset || pev->point.retprobe || !map)
+		return;
+
+	if (map->dso->symtab_type == DSO_BINARY_TYPE__KALLSYMS)
 		tev->point.offset += PPC64LE_LEP_OFFSET;
-	}
 }
 #endif
-- 
2.7.4

[PATCH v2 2/2] perf tools: Fix kallsyms perf test on ppc64le

[Naveen N. Rao]

ppc64le functions have a Global Entry Point (GEP) and a Local Entry
Point (LEP). While placing a probe, we always prefer the LEP since it
catches function calls through both the GEP and the LEP. In order to do
this, we fixup the function entry points during elf symbol table lookup
to point to the LEPs. This works, but breaks 'perf test kallsyms' since
the symbols loaded from the symbol table (pointing to the LEP) do not
match the symbols in kallsyms.

To fix this, we do not adjust all the symbols during symbol table load.
Instead, we note down st_other in a newly introduced arch-specific
member of perf symbol structure, and later use this to adjust the probe
trace point.

Cc: Mark Wielaard <mjw@redhat.com>
Cc: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Balbir Singh <bsingharora@gmail.com>
Acked-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Reported-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
v2: Added uprobes handling -- we modify the probe address, rather than
the offset.

 tools/perf/arch/powerpc/util/sym-handling.c | 28 ++++++++++++++++++++--------
 tools/perf/util/probe-event.c               |  5 +++--
 tools/perf/util/probe-event.h               |  3 ++-
 tools/perf/util/symbol-elf.c                |  7 ++++---
 tools/perf/util/symbol.h                    |  3 ++-
 5 files changed, 31 insertions(+), 15 deletions(-)

diff --git a/tools/perf/arch/powerpc/util/sym-handling.c b/tools/perf/arch/powerpc/util/sym-handling.c
index 6974ba0..c6d0f91 100644
--- a/tools/perf/arch/powerpc/util/sym-handling.c
+++ b/tools/perf/arch/powerpc/util/sym-handling.c
@@ -19,12 +19,6 @@ bool elf__needs_adjust_symbols(GElf_Ehdr ehdr)
 	       ehdr.e_type == ET_DYN;
 }
 
-#if defined(_CALL_ELF) && _CALL_ELF == 2
-void arch__elf_sym_adjust(GElf_Sym *sym)
-{
-	sym->st_value += PPC64_LOCAL_ENTRY_OFFSET(sym->st_other);
-}
-#endif
 #endif
 
 #if !defined(_CALL_ELF) || _CALL_ELF != 2
@@ -65,11 +59,21 @@ bool arch__prefers_symtab(void)
 	return true;
 }
 
+#ifdef HAVE_LIBELF_SUPPORT
+void arch__sym_update(struct symbol *s, GElf_Sym *sym)
+{
+	s->arch_sym = sym->st_other;
+}
+#endif
+
 #define PPC64LE_LEP_OFFSET	8
 
 void arch__fix_tev_from_maps(struct perf_probe_event *pev,
-			     struct probe_trace_event *tev, struct map *map)
+			     struct probe_trace_event *tev, struct map *map,
+			     struct symbol *sym)
 {
+	int lep_offset;
+
 	/*
 	 * When probing at a function entry point, we normally always want the
 	 * LEP since that catches calls to the function through both the GEP and
@@ -82,10 +86,18 @@ void arch__fix_tev_from_maps(struct perf_probe_event *pev,
 	 *
 	 * In addition, we shouldn't specify an offset for kretprobes.
 	 */
-	if (pev->point.offset || pev->point.retprobe || !map)
+	if (pev->point.offset || pev->point.retprobe || !map || !sym)
 		return;
 
+	lep_offset = PPC64_LOCAL_ENTRY_OFFSET(sym->arch_sym);
+
 	if (map->dso->symtab_type == DSO_BINARY_TYPE__KALLSYMS)
 		tev->point.offset += PPC64LE_LEP_OFFSET;
+	else if (lep_offset) {
+		if (pev->uprobes)
+			tev->point.address += lep_offset;
+		else
+			tev->point.offset += lep_offset;
+	}
 }
 #endif
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 8319fbb..d786a49 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2498,7 +2498,8 @@ static int find_probe_functions(struct map *map, char *name,
 
 void __weak arch__fix_tev_from_maps(struct perf_probe_event *pev __maybe_unused,
 				struct probe_trace_event *tev __maybe_unused,
-				struct map *map __maybe_unused) { }
+				struct map *map __maybe_unused,
+				struct symbol *sym __maybe_unused) { }
 
 /*
  * Find probe function addresses from map.
@@ -2624,7 +2625,7 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
 					strdup_or_goto(pev->args[i].type,
 							nomem_out);
 		}
-		arch__fix_tev_from_maps(pev, tev, map);
+		arch__fix_tev_from_maps(pev, tev, map, sym);
 	}
 	if (ret == skipped) {
 		ret = -ENOENT;
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index e54e7b0..9bbc0c1 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -154,7 +154,8 @@ int show_available_vars(struct perf_probe_event *pevs, int npevs,
 int show_available_funcs(const char *module, struct strfilter *filter, bool user);
 bool arch__prefers_symtab(void);
 void arch__fix_tev_from_maps(struct perf_probe_event *pev,
-			     struct probe_trace_event *tev, struct map *map);
+			     struct probe_trace_event *tev, struct map *map,
+			     struct symbol *sym);
 
 /* If there is no space to write, returns -E2BIG. */
 int e_snprintf(char *str, size_t size, const char *format, ...)
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index bc229a7..e6c032e 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -777,7 +777,8 @@ static bool want_demangle(bool is_kernel_sym)
 	return is_kernel_sym ? symbol_conf.demangle_kernel : symbol_conf.demangle;
 }
 
-void __weak arch__elf_sym_adjust(GElf_Sym *sym __maybe_unused) { }
+void __weak arch__sym_update(struct symbol *s __maybe_unused,
+		GElf_Sym *sym __maybe_unused) { }
 
 int dso__load_sym(struct dso *dso, struct map *map,
 		  struct symsrc *syms_ss, struct symsrc *runtime_ss,
@@ -954,8 +955,6 @@ int dso__load_sym(struct dso *dso, struct map *map,
 		    (sym.st_value & 1))
 			--sym.st_value;
 
-		arch__elf_sym_adjust(&sym);
-
 		if (dso->kernel || kmodule) {
 			char dso_name[PATH_MAX];
 
@@ -1089,6 +1088,8 @@ new_symbol:
 		if (!f)
 			goto out_elf_end;
 
+		arch__sym_update(f, &sym);
+
 		if (filter && filter(curr_map, f))
 			symbol__delete(f);
 		else {
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index c8b7544..f0e62e8 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -55,6 +55,7 @@ struct symbol {
 	u16		namelen;
 	u8		binding;
 	bool		ignore;
+	u8		arch_sym;
 	char		name[0];
 };
 
@@ -310,7 +311,7 @@ int setup_intlist(struct intlist **list, const char *list_str,
 
 #ifdef HAVE_LIBELF_SUPPORT
 bool elf__needs_adjust_symbols(GElf_Ehdr ehdr);
-void arch__elf_sym_adjust(GElf_Sym *sym);
+void arch__sym_update(struct symbol *s, GElf_Sym *sym);
 #endif
 
 #define SYMBOL_A 0
-- 
2.7.4

Re: [PATCH v2 0/2] perf probe fixes for ppc64le

[Balbir Singh]

On 12/04/16 19:10, Naveen N. Rao wrote:
> This patchset fixes three issues found with perf probe on ppc64le:
> 1. 'perf test kallsyms' failure on ppc64le (reported by Michael
> Ellerman). This was due to the symbols being fixed up during symbol
> table load. This is fixed in patch 2 by delaying symbol fixup until
> later.
> 2. perf probe function offset was being calculated from the local entry
> point (LEP), which does not match user expectation when trying to look
> at function disassembly output (reported by Ananth N). This is fixed for
> kallsyms in patch 1 and for symbol table in patch 2.
> 3. perf probe failure with kretprobe when using kallsyms. This was
> failing as we were specifying an offset. This is fixed in patch 1.
> 
> A few examples demonstrating the issues and the fix:
> 

Given the choices, I think this makes sense

Acked-by: Balbir Singh <bsingharora@gmail.com>

Re: [PATCH v2 0/2] perf probe fixes for ppc64le

[Naveen N. Rao]

On 2016/04/12 02:40PM, Naveen N Rao wrote:
> This patchset fixes three issues found with perf probe on ppc64le:
> 1. 'perf test kallsyms' failure on ppc64le (reported by Michael
> Ellerman). This was due to the symbols being fixed up during symbol
> table load. This is fixed in patch 2 by delaying symbol fixup until
> later.
> 2. perf probe function offset was being calculated from the local entry
> point (LEP), which does not match user expectation when trying to look
> at function disassembly output (reported by Ananth N). This is fixed for
> kallsyms in patch 1 and for symbol table in patch 2.
> 3. perf probe failure with kretprobe when using kallsyms. This was
> failing as we were specifying an offset. This is fixed in patch 1.
> 
> A few examples demonstrating the issues and the fix:
> 
> Example for issue (2):
> --------------------
>     # objdump -d vmlinux | grep -A8 \<_do_fork\>:
>     c0000000000b6a00 <_do_fork>:
>     c0000000000b6a00:	f7 00 4c 3c 	addis   r2,r12,247
>     c0000000000b6a04:	00 86 42 38 	addi    r2,r2,-31232
>     c0000000000b6a08:	a6 02 08 7c 	mflr    r0
>     c0000000000b6a0c:	d0 ff 41 fb 	std     r26,-48(r1)
>     c0000000000b6a10:	26 80 90 7d 	mfocrf  r12,8
>     c0000000000b6a14:	d8 ff 61 fb 	std     r27,-40(r1)
>     c0000000000b6a18:	e0 ff 81 fb 	std     r28,-32(r1)
>     c0000000000b6a1c:	e8 ff a1 fb 	std     r29,-24(r1)
>     # perf probe -v _do_fork+4
>     probe-definition(0): _do_fork+4 
>     symbol:_do_fork file:(null) line:0 offset:4 return:0 lazy:(null)
>     0 arguments
>     Looking at the vmlinux_path (8 entries long)
>     Using /proc/kcore for kernel object code
>     Using /proc/kallsyms for symbols
>     Opening /sys/kernel/debug/tracing//kprobe_events write=1
>     Writing event: p:probe/_do_fork _text+748044
>     Added new event:
>       probe:_do_fork       (on _do_fork+4)
> 
>     You can now use it in all perf tools, such as:
> 
> 	    perf record -e probe:_do_fork -aR sleep 1
> 
>     # printf "%x\n" 748044
>     b6a0c
>     ^^^^^
> This is offset from the LEP. With this, there is also no way to ever
> probe between the GEP and the LEP.
> 
> With this patchset:
>     # perf probe -v _do_fork+4
>     probe-definition(0): _do_fork+4 
>     symbol:_do_fork file:(null) line:0 offset:4 return:0 lazy:(null)
>     0 arguments
>     Looking at the vmlinux_path (8 entries long)
>     Using /proc/kcore for kernel object code
>     Using /proc/kallsyms for symbols
>     Opening /sys/kernel/debug/tracing//kprobe_events write=1
>     Writing event: p:probe/_do_fork _text+748036
>     Added new event:
>       probe:_do_fork       (on _do_fork+4)
> 
>     You can now use it in all perf tools, such as:
> 
> 	    perf record -e probe:_do_fork -aR sleep 1
> 
>     # perf probe -v _do_fork
>     probe-definition(0): _do_fork 
>     symbol:_do_fork file:(null) line:0 offset:0 return:0 lazy:(null)
>     0 arguments
>     Looking at the vmlinux_path (8 entries long)
>     Using /proc/kcore for kernel object code
>     Using /proc/kallsyms for symbols
>     Opening /sys/kernel/debug/tracing//kprobe_events write=1
>     Writing event: p:probe/_do_fork _text+748040
>     Added new event:
>       probe:_do_fork       (on _do_fork)
> 
>     You can now use it in all perf tools, such as:
> 
> 	    perf record -e probe:_do_fork -aR sleep 1
> 
> We only offset to the LEP if function entry is specified, otherwise, we
> offset from the GEP.
> 
> Example for issue (3):
> ---------------------
> Before patch:
>     # perf probe -v _do_fork:%return
>     probe-definition(0): _do_fork:%return 
>     symbol:_do_fork file:(null) line:0 offset:0 return:1 lazy:(null)
>     0 arguments
>     Looking at the vmlinux_path (8 entries long)
>     Using /proc/kcore for kernel object code
>     Using /proc/kallsyms for symbols
>     Opening /sys/kernel/debug/tracing//kprobe_events write=1
>     Writing event: r:probe/_do_fork _do_fork+8
>     Failed to write event: Invalid argument
>       Error: Failed to add events. Reason: Invalid argument (Code: -22)
> 
> After patch:
>     # perf probe _do_fork:%return
>     Added new event:
>       probe:_do_fork       (on _do_fork%return)
> 
>     You can now use it in all perf tools, such as:
> 
> 	    perf record -e probe:_do_fork -aR sleep 1
> 
> 
> Concept Acked-by: Michael Ellerman <mpe@ellerman.id.au>
> 
> Cc: Mark Wielaard <mjw@redhat.com>
> Cc: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Balbir Singh <bsingharora@gmail.com>
> Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>

Arnaldo,
Can you please pick this up if you're ok with the changes?

- Naveen

> 
> Naveen N. Rao (2):
>   perf tools: Fix kprobe and kretprobe handling with kallsyms on ppc64le
>   perf tools: Fix kallsyms perf test on ppc64le
> 
>  tools/perf/arch/powerpc/util/sym-handling.c | 43 +++++++++++++++++++++--------
>  tools/perf/util/probe-event.c               |  5 ++--
>  tools/perf/util/probe-event.h               |  3 +-
>  tools/perf/util/symbol-elf.c                |  7 +++--
>  tools/perf/util/symbol.h                    |  3 +-
>  5 files changed, 43 insertions(+), 18 deletions(-)
> 
> -- 
> 2.7.4
> 

[tip:perf/core] perf powerpc: Fix kprobe and kretprobe handling with kallsyms on ppc64le

[tip-bot for Naveen N. Rao]

Commit-ID:  239aeba764092b29dd7cab177cd47f472390622e
Gitweb:     http://git.kernel.org/tip/239aeba764092b29dd7cab177cd47f472390622e
Author:     Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
AuthorDate: Tue, 12 Apr 2016 14:40:49 +0530
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 5 May 2016 21:04:02 -0300

perf powerpc: Fix kprobe and kretprobe handling with kallsyms on ppc64le

So far, we used to treat probe point offsets as being offset from the
LEP. However, userspace applications (objdump/readelf) always show
disassembly and offsets from the function GEP. This is confusing to the
user as we will end up probing at an address different from what the
user expects when looking at the function disassembly with
readelf/objdump. Fix this by changing how we modify probe address with
perf.

If only the function name is provided, we assume the user needs the LEP.
Otherwise, if an offset is specified, we assume that the user knows the
exact address to probe based on function disassembly, and so we just
place the probe from the GEP offset.

Finally, kretprobe was also broken with kallsyms as we were trying to
specify an offset. This patch also fixes that issue.

Reported-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Acked-by: Balbir Singh <bsingharora@gmail.com>
Cc: Mark Wielaard <mjw@redhat.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
Cc: linuxppc-dev@lists.ozlabs.org
Link: http://lkml.kernel.org/r/75df860aad8216bf4b9bcd10c6351ecc0e3dee54.1460451721.git.naveen.n.rao@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/arch/powerpc/util/sym-handling.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/tools/perf/arch/powerpc/util/sym-handling.c b/tools/perf/arch/powerpc/util/sym-handling.c
index bbc1a50..6974ba0 100644
--- a/tools/perf/arch/powerpc/util/sym-handling.c
+++ b/tools/perf/arch/powerpc/util/sym-handling.c
@@ -71,12 +71,21 @@ void arch__fix_tev_from_maps(struct perf_probe_event *pev,
 			     struct probe_trace_event *tev, struct map *map)
 {
 	/*
-	 * ppc64 ABIv2 local entry point is currently always 2 instructions
-	 * (8 bytes) after the global entry point.
+	 * When probing at a function entry point, we normally always want the
+	 * LEP since that catches calls to the function through both the GEP and
+	 * the LEP. Hence, we would like to probe at an offset of 8 bytes if
+	 * the user only specified the function entry.
+	 *
+	 * However, if the user specifies an offset, we fall back to using the
+	 * GEP since all userspace applications (objdump/readelf) show function
+	 * disassembly with offsets from the GEP.
+	 *
+	 * In addition, we shouldn't specify an offset for kretprobes.
 	 */
-	if (!pev->uprobes && map->dso->symtab_type == DSO_BINARY_TYPE__KALLSYMS) {
-		tev->point.address += PPC64LE_LEP_OFFSET;
+	if (pev->point.offset || pev->point.retprobe || !map)
+		return;
+
+	if (map->dso->symtab_type == DSO_BINARY_TYPE__KALLSYMS)
 		tev->point.offset += PPC64LE_LEP_OFFSET;
-	}
 }
 #endif

[tip:perf/core] perf symbols: Fix kallsyms perf test on ppc64le

[tip-bot for Naveen N. Rao]

Commit-ID:  0b3c2264ae30ed692fd1ffd2b84c5fbdf737cb0d
Gitweb:     http://git.kernel.org/tip/0b3c2264ae30ed692fd1ffd2b84c5fbdf737cb0d
Author:     Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
AuthorDate: Tue, 12 Apr 2016 14:40:50 +0530
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 5 May 2016 21:04:03 -0300

perf symbols: Fix kallsyms perf test on ppc64le

ppc64le functions have a Global Entry Point (GEP) and a Local Entry
Point (LEP). While placing a probe, we always prefer the LEP since it
catches function calls through both the GEP and the LEP. In order to do
this, we fixup the function entry points during elf symbol table lookup
to point to the LEPs. This works, but breaks 'perf test kallsyms' since
the symbols loaded from the symbol table (pointing to the LEP) do not
match the symbols in kallsyms.

To fix this, we do not adjust all the symbols during symbol table load.
Instead, we note down st_other in a newly introduced arch-specific
member of perf symbol structure, and later use this to adjust the probe
trace point.

Reported-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Acked-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Acked-by: Balbir Singh <bsingharora@gmail.com>
Cc: Mark Wielaard <mjw@redhat.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
Cc: linuxppc-dev@lists.ozlabs.org
Link: http://lkml.kernel.org/r/6be7c2b17e370100c2f79dd444509df7929bdd3e.1460451721.git.naveen.n.rao@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/arch/powerpc/util/sym-handling.c | 28 ++++++++++++++++++++--------
 tools/perf/util/probe-event.c               |  5 +++--
 tools/perf/util/probe-event.h               |  3 ++-
 tools/perf/util/symbol-elf.c                |  7 ++++---
 tools/perf/util/symbol.h                    |  3 ++-
 5 files changed, 31 insertions(+), 15 deletions(-)

diff --git a/tools/perf/arch/powerpc/util/sym-handling.c b/tools/perf/arch/powerpc/util/sym-handling.c
index 6974ba0..c6d0f91 100644
--- a/tools/perf/arch/powerpc/util/sym-handling.c
+++ b/tools/perf/arch/powerpc/util/sym-handling.c
@@ -19,12 +19,6 @@ bool elf__needs_adjust_symbols(GElf_Ehdr ehdr)
 	       ehdr.e_type == ET_DYN;
 }
 
-#if defined(_CALL_ELF) && _CALL_ELF == 2
-void arch__elf_sym_adjust(GElf_Sym *sym)
-{
-	sym->st_value += PPC64_LOCAL_ENTRY_OFFSET(sym->st_other);
-}
-#endif
 #endif
 
 #if !defined(_CALL_ELF) || _CALL_ELF != 2
@@ -65,11 +59,21 @@ bool arch__prefers_symtab(void)
 	return true;
 }
 
+#ifdef HAVE_LIBELF_SUPPORT
+void arch__sym_update(struct symbol *s, GElf_Sym *sym)
+{
+	s->arch_sym = sym->st_other;
+}
+#endif
+
 #define PPC64LE_LEP_OFFSET	8
 
 void arch__fix_tev_from_maps(struct perf_probe_event *pev,
-			     struct probe_trace_event *tev, struct map *map)
+			     struct probe_trace_event *tev, struct map *map,
+			     struct symbol *sym)
 {
+	int lep_offset;
+
 	/*
 	 * When probing at a function entry point, we normally always want the
 	 * LEP since that catches calls to the function through both the GEP and
@@ -82,10 +86,18 @@ void arch__fix_tev_from_maps(struct perf_probe_event *pev,
 	 *
 	 * In addition, we shouldn't specify an offset for kretprobes.
 	 */
-	if (pev->point.offset || pev->point.retprobe || !map)
+	if (pev->point.offset || pev->point.retprobe || !map || !sym)
 		return;
 
+	lep_offset = PPC64_LOCAL_ENTRY_OFFSET(sym->arch_sym);
+
 	if (map->dso->symtab_type == DSO_BINARY_TYPE__KALLSYMS)
 		tev->point.offset += PPC64LE_LEP_OFFSET;
+	else if (lep_offset) {
+		if (pev->uprobes)
+			tev->point.address += lep_offset;
+		else
+			tev->point.offset += lep_offset;
+	}
 }
 #endif
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 85d82f4..c82c625 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2477,7 +2477,8 @@ static int find_probe_functions(struct map *map, char *name,
 
 void __weak arch__fix_tev_from_maps(struct perf_probe_event *pev __maybe_unused,
 				struct probe_trace_event *tev __maybe_unused,
-				struct map *map __maybe_unused) { }
+				struct map *map __maybe_unused,
+				struct symbol *sym __maybe_unused) { }
 
 /*
  * Find probe function addresses from map.
@@ -2614,7 +2615,7 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
 					strdup_or_goto(pev->args[i].type,
 							nomem_out);
 		}
-		arch__fix_tev_from_maps(pev, tev, map);
+		arch__fix_tev_from_maps(pev, tev, map, sym);
 	}
 	if (ret == skipped) {
 		ret = -ENOENT;
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index e220962..5a27eb4 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -154,7 +154,8 @@ int show_available_vars(struct perf_probe_event *pevs, int npevs,
 int show_available_funcs(const char *module, struct strfilter *filter, bool user);
 bool arch__prefers_symtab(void);
 void arch__fix_tev_from_maps(struct perf_probe_event *pev,
-			     struct probe_trace_event *tev, struct map *map);
+			     struct probe_trace_event *tev, struct map *map,
+			     struct symbol *sym);
 
 /* If there is no space to write, returns -E2BIG. */
 int e_snprintf(char *str, size_t size, const char *format, ...)
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 3f9d679..87a297d 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -770,7 +770,8 @@ static bool want_demangle(bool is_kernel_sym)
 	return is_kernel_sym ? symbol_conf.demangle_kernel : symbol_conf.demangle;
 }
 
-void __weak arch__elf_sym_adjust(GElf_Sym *sym __maybe_unused) { }
+void __weak arch__sym_update(struct symbol *s __maybe_unused,
+		GElf_Sym *sym __maybe_unused) { }
 
 int dso__load_sym(struct dso *dso, struct map *map,
 		  struct symsrc *syms_ss, struct symsrc *runtime_ss,
@@ -947,8 +948,6 @@ int dso__load_sym(struct dso *dso, struct map *map,
 		    (sym.st_value & 1))
 			--sym.st_value;
 
-		arch__elf_sym_adjust(&sym);
-
 		if (dso->kernel || kmodule) {
 			char dso_name[PATH_MAX];
 
@@ -1082,6 +1081,8 @@ new_symbol:
 		if (!f)
 			goto out_elf_end;
 
+		arch__sym_update(f, &sym);
+
 		if (filter && filter(curr_map, f))
 			symbol__delete(f);
 		else {
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index c8e4397..07211c2 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -55,6 +55,7 @@ struct symbol {
 	u16		namelen;
 	u8		binding;
 	bool		ignore;
+	u8		arch_sym;
 	char		name[0];
 };
 
@@ -323,7 +324,7 @@ int setup_intlist(struct intlist **list, const char *list_str,
 
 #ifdef HAVE_LIBELF_SUPPORT
 bool elf__needs_adjust_symbols(GElf_Ehdr ehdr);
-void arch__elf_sym_adjust(GElf_Sym *sym);
+void arch__sym_update(struct symbol *s, GElf_Sym *sym);
 #endif
 
 #define SYMBOL_A 0