[PATCH v2 0/4] perf-probe: Fix GNU IFUNC probe issue etc.

[PATCH v2 0/4] perf-probe: Fix GNU IFUNC probe issue etc.

[Masami Hiramatsu]

Hi,

Here are the v2 patches to fix some issues of probing on GNU IFUNC,
duplicated symbols, and memory leak, which were reported by Andi.
V1 is here:

 https://lkml.kernel.org/r/159428201109.56570.3802208017109058146.stgit@devnote2

In this version, I've added Srikar's reviewed-by and Andi's Tested-by (Thanks
Srikar and Andi!) and fix some messages and code according to the comment.

Thank you,

---

Masami Hiramatsu (4):
      perf-probe: Avoid setting probes on same address on same event
      perf-probe: Fix wrong variable warning when the probe point is not found
      perf-probe: Fix memory leakage when the probe point is not found
      perf-probe: Warn if the target function is GNU Indirect function


 tools/perf/util/probe-event.c  |   18 ++++++++++++++++--
 tools/perf/util/probe-finder.c |    5 ++++-
 2 files changed, 20 insertions(+), 3 deletions(-)

--
Masami Hiramatsu (Linaro) <mhiramat@kernel.org>

[PATCH v2 1/4] perf-probe: Avoid setting probes on same address on same event

[Masami Hiramatsu]

There is a case that the several same-name symbols points
same address. In that case, perf probe returns an error.

E.g.

  perf probe -x /lib64/libc-2.30.so -v -a "memcpy arg1=%di"
  probe-definition(0): memcpy arg1=%di
  symbol:memcpy file:(null) line:0 offset:0 return:0 lazy:(null)
  parsing arg: arg1=%di into name:arg1 %di
  1 arguments
  symbol:setjmp file:(null) line:0 offset:0 return:0 lazy:(null)
  symbol:longjmp file:(null) line:0 offset:0 return:0 lazy:(null)
  symbol:longjmp_target file:(null) line:0 offset:0 return:0 lazy:(null)
  symbol:lll_lock_wait_private file:(null) line:0 offset:0 return:0 lazy:(null)
  symbol:memory_mallopt_arena_max file:(null) line:0 offset:0 return:0 lazy:(null)
  symbol:memory_mallopt_arena_test file:(null) line:0 offset:0 return:0 lazy:(null)
  symbol:memory_tunable_tcache_max_bytes file:(null) line:0 offset:0 return:0 lazy:(null)
  symbol:memory_tunable_tcache_count file:(null) line:0 offset:0 return:0 lazy:(null)
  symbol:memory_tunable_tcache_unsorted_limit file:(null) line:0 offset:0 return:0 lazy:(null)
  symbol:memory_mallopt_trim_threshold file:(null) line:0 offset:0 return:0 lazy:(null)
  symbol:memory_mallopt_top_pad file:(null) line:0 offset:0 return:0 lazy:(null)
  symbol:memory_mallopt_mmap_threshold file:(null) line:0 offset:0 return:0 lazy:(null)
  symbol:memory_mallopt_mmap_max file:(null) line:0 offset:0 return:0 lazy:(null)
  symbol:memory_mallopt_perturb file:(null) line:0 offset:0 return:0 lazy:(null)
  symbol:memory_mallopt_mxfast file:(null) line:0 offset:0 return:0 lazy:(null)
  symbol:memory_heap_new file:(null) line:0 offset:0 return:0 lazy:(null)
  symbol:memory_arena_reuse_free_list file:(null) line:0 offset:0 return:0 lazy:(null)
  symbol:memory_arena_reuse file:(null) line:0 offset:0 return:0 lazy:(null)
  symbol:memory_arena_reuse_wait file:(null) line:0 offset:0 return:0 lazy:(null)
  symbol:memory_arena_new file:(null) line:0 offset:0 return:0 lazy:(null)
  symbol:memory_arena_retry file:(null) line:0 offset:0 return:0 lazy:(null)
  symbol:memory_sbrk_less file:(null) line:0 offset:0 return:0 lazy:(null)
  symbol:memory_heap_free file:(null) line:0 offset:0 return:0 lazy:(null)
  symbol:memory_heap_less file:(null) line:0 offset:0 return:0 lazy:(null)
  symbol:memory_tcache_double_free file:(null) line:0 offset:0 return:0 lazy:(null)
  symbol:memory_heap_more file:(null) line:0 offset:0 return:0 lazy:(null)
  symbol:memory_sbrk_more file:(null) line:0 offset:0 return:0 lazy:(null)
  symbol:memory_malloc_retry file:(null) line:0 offset:0 return:0 lazy:(null)
  symbol:memory_memalign_retry file:(null) line:0 offset:0 return:0 lazy:(null)
  symbol:memory_mallopt_free_dyn_thresholds file:(null) line:0 offset:0 return:0 lazy:(null)
  symbol:memory_realloc_retry file:(null) line:0 offset:0 return:0 lazy:(null)
  symbol:memory_calloc_retry file:(null) line:0 offset:0 return:0 lazy:(null)
  symbol:memory_mallopt file:(null) line:0 offset:0 return:0 lazy:(null)
  Open Debuginfo file: /usr/lib/debug/usr/lib64/libc-2.30.so.debug
  Try to find probe point from debuginfo.
  Opening /sys/kernel/debug/tracing//README write=0
  Failed to find the location of the '%di' variable at this address.
   Perhaps it has been optimized out.
   Use -V with the --range option to show '%di' location range.
  An error occurred in debuginfo analysis (-2).
  Trying to use symbols.
  Opening /sys/kernel/debug/tracing//uprobe_events write=1
  Writing event: p:probe_libc/memcpy /usr/lib64/libc-2.30.so:0x914c0 arg1=%di
  Writing event: p:probe_libc/memcpy /usr/lib64/libc-2.30.so:0x914c0 arg1=%di
  Failed to write event: File exists
    Error: Failed to add events. Reason: File exists (Code: -17)

You can see the perf tried to write completely same probe definition
twice, which caused an error.

To fix this issue, check the symbol list and drop duplicated
symbols (which has same symbol name and address) from it.

With this patch;

  # perf probe -x /lib64/libc-2.30.so -a "memcpy arg1=%di"
  Failed to find the location of the '%di' variable at this address.
   Perhaps it has been optimized out.
   Use -V with the --range option to show '%di' location range.
  Added new events:
    probe_libc:memcpy    (on memcpy in /usr/lib64/libc-2.30.so with arg1=%di)
    probe_libc:memcpy    (on memcpy in /usr/lib64/libc-2.30.so with arg1=%di)

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

  	perf record -e probe_libc:memcpy -aR sleep 1


Reported-by: Andi Kleen <andi@firstfloor.org>
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
 Changes in V2
  - Change "find" word to "Found".
---
 tools/perf/util/probe-event.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index df713a5d1e26..8cd1224e5f4c 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2968,6 +2968,16 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
 	for (j = 0; j < num_matched_functions; j++) {
 		sym = syms[j];
 
+		/* There can be duplicated symbols in the map */
+		for (i = 0; i < j; i++)
+			if (sym->start == syms[i]->start) {
+				pr_debug("Found duplicated symbol %s @ %lx\n",
+					 sym->name, sym->start);
+				break;
+			}
+		if (i != j)
+			continue;
+
 		tev = (*tevs) + ret;
 		tp = &tev->point;
 		if (ret == num_matched_functions) {

Re: [PATCH v2 1/4] perf-probe: Avoid setting probes on same address on same event

[Arnaldo Carvalho de Melo]

Em Fri, Jul 10, 2020 at 10:11:04PM +0900, Masami Hiramatsu escreveu:
> There is a case that the several same-name symbols points
> same address. In that case, perf probe returns an error.
> 
> E.g.
> 
>   perf probe -x /lib64/libc-2.30.so -v -a "memcpy arg1=%di"
>   probe-definition(0): memcpy arg1=%di
>   symbol:memcpy file:(null) line:0 offset:0 return:0 lazy:(null)
>   parsing arg: arg1=%di into name:arg1 %di
>   1 arguments
>   symbol:setjmp file:(null) line:0 offset:0 return:0 lazy:(null)
>   symbol:longjmp file:(null) line:0 offset:0 return:0 lazy:(null)
>   symbol:longjmp_target file:(null) line:0 offset:0 return:0 lazy:(null)
>   symbol:lll_lock_wait_private file:(null) line:0 offset:0 return:0 lazy:(null)
>   symbol:memory_mallopt_arena_max file:(null) line:0 offset:0 return:0 lazy:(null)
>   symbol:memory_mallopt_arena_test file:(null) line:0 offset:0 return:0 lazy:(null)
>   symbol:memory_tunable_tcache_max_bytes file:(null) line:0 offset:0 return:0 lazy:(null)
>   symbol:memory_tunable_tcache_count file:(null) line:0 offset:0 return:0 lazy:(null)
>   symbol:memory_tunable_tcache_unsorted_limit file:(null) line:0 offset:0 return:0 lazy:(null)
>   symbol:memory_mallopt_trim_threshold file:(null) line:0 offset:0 return:0 lazy:(null)
>   symbol:memory_mallopt_top_pad file:(null) line:0 offset:0 return:0 lazy:(null)
>   symbol:memory_mallopt_mmap_threshold file:(null) line:0 offset:0 return:0 lazy:(null)
>   symbol:memory_mallopt_mmap_max file:(null) line:0 offset:0 return:0 lazy:(null)
>   symbol:memory_mallopt_perturb file:(null) line:0 offset:0 return:0 lazy:(null)
>   symbol:memory_mallopt_mxfast file:(null) line:0 offset:0 return:0 lazy:(null)
>   symbol:memory_heap_new file:(null) line:0 offset:0 return:0 lazy:(null)
>   symbol:memory_arena_reuse_free_list file:(null) line:0 offset:0 return:0 lazy:(null)
>   symbol:memory_arena_reuse file:(null) line:0 offset:0 return:0 lazy:(null)
>   symbol:memory_arena_reuse_wait file:(null) line:0 offset:0 return:0 lazy:(null)
>   symbol:memory_arena_new file:(null) line:0 offset:0 return:0 lazy:(null)
>   symbol:memory_arena_retry file:(null) line:0 offset:0 return:0 lazy:(null)
>   symbol:memory_sbrk_less file:(null) line:0 offset:0 return:0 lazy:(null)
>   symbol:memory_heap_free file:(null) line:0 offset:0 return:0 lazy:(null)
>   symbol:memory_heap_less file:(null) line:0 offset:0 return:0 lazy:(null)
>   symbol:memory_tcache_double_free file:(null) line:0 offset:0 return:0 lazy:(null)
>   symbol:memory_heap_more file:(null) line:0 offset:0 return:0 lazy:(null)
>   symbol:memory_sbrk_more file:(null) line:0 offset:0 return:0 lazy:(null)
>   symbol:memory_malloc_retry file:(null) line:0 offset:0 return:0 lazy:(null)
>   symbol:memory_memalign_retry file:(null) line:0 offset:0 return:0 lazy:(null)
>   symbol:memory_mallopt_free_dyn_thresholds file:(null) line:0 offset:0 return:0 lazy:(null)
>   symbol:memory_realloc_retry file:(null) line:0 offset:0 return:0 lazy:(null)
>   symbol:memory_calloc_retry file:(null) line:0 offset:0 return:0 lazy:(null)
>   symbol:memory_mallopt file:(null) line:0 offset:0 return:0 lazy:(null)
>   Open Debuginfo file: /usr/lib/debug/usr/lib64/libc-2.30.so.debug
>   Try to find probe point from debuginfo.
>   Opening /sys/kernel/debug/tracing//README write=0
>   Failed to find the location of the '%di' variable at this address.
>    Perhaps it has been optimized out.
>    Use -V with the --range option to show '%di' location range.
>   An error occurred in debuginfo analysis (-2).
>   Trying to use symbols.
>   Opening /sys/kernel/debug/tracing//uprobe_events write=1
>   Writing event: p:probe_libc/memcpy /usr/lib64/libc-2.30.so:0x914c0 arg1=%di
>   Writing event: p:probe_libc/memcpy /usr/lib64/libc-2.30.so:0x914c0 arg1=%di
>   Failed to write event: File exists
>     Error: Failed to add events. Reason: File exists (Code: -17)
> 
> You can see the perf tried to write completely same probe definition
> twice, which caused an error.
> 
> To fix this issue, check the symbol list and drop duplicated
> symbols (which has same symbol name and address) from it.
> 
> With this patch;
> 
>   # perf probe -x /lib64/libc-2.30.so -a "memcpy arg1=%di"
>   Failed to find the location of the '%di' variable at this address.
>    Perhaps it has been optimized out.
>    Use -V with the --range option to show '%di' location range.
>   Added new events:
>     probe_libc:memcpy    (on memcpy in /usr/lib64/libc-2.30.so with arg1=%di)
>     probe_libc:memcpy    (on memcpy in /usr/lib64/libc-2.30.so with arg1=%di)
> 
>   You can now use it in all perf tools, such as:
> 
>   	perf record -e probe_libc:memcpy -aR sleep 1
> 
> 
> Reported-by: Andi Kleen <andi@firstfloor.org>
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> ---
>  Changes in V2
>   - Change "find" word to "Found".
> ---
>  tools/perf/util/probe-event.c |   10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index df713a5d1e26..8cd1224e5f4c 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -2968,6 +2968,16 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
>  	for (j = 0; j < num_matched_functions; j++) {
>  		sym = syms[j];
>  
> +		/* There can be duplicated symbols in the map */
> +		for (i = 0; i < j; i++)
> +			if (sym->start == syms[i]->start) {
> +				pr_debug("Found duplicated symbol %s @ %lx\n",
> +					 sym->name, sym->start);
> +				break;
> +			}

Breaks 32-bit builds with:

  CC       /tmp/build/perf/util/demangle-java.o
In file included from util/probe-event.c:27:
util/probe-event.c: In function 'find_probe_trace_events_from_map':
util/probe-event.c:2978:14: error: format '%lx' expects argument of type 'long unsigned int', but argument 5 has type 'u64' {aka 'long long unsigned int'} [-Werror=format=]
     pr_debug("Found duplicated symbol %s @ %lx\n",
              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
util/debug.h:17:21: note: in definition of macro 'pr_fmt'
 #define pr_fmt(fmt) fmt
                     ^~~
util/probe-event.c:2978:5: note: in expansion of macro 'pr_debug'
     pr_debug("Found duplicated symbol %s @ %lx\n",
     ^~~~~~~~
  CC       /tmp/build/perf/util/demangle-rust.o


I'll change this to use PRIx64.

- Arnaldo

> +		if (i != j)
> +			continue;
> +
>  		tev = (*tevs) + ret;
>  		tp = &tev->point;
>  		if (ret == num_matched_functions) {
> 

-- 

- Arnaldo

Re: [PATCH v2 1/4] perf-probe: Avoid setting probes on same address on same event

[Masami Hiramatsu]

On Thu, 16 Jul 2020 18:47:14 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Fri, Jul 10, 2020 at 10:11:04PM +0900, Masami Hiramatsu escreveu:
> > There is a case that the several same-name symbols points
> > same address. In that case, perf probe returns an error.
> > 
> > E.g.
> > 
> >   perf probe -x /lib64/libc-2.30.so -v -a "memcpy arg1=%di"
> >   probe-definition(0): memcpy arg1=%di
> >   symbol:memcpy file:(null) line:0 offset:0 return:0 lazy:(null)
> >   parsing arg: arg1=%di into name:arg1 %di
> >   1 arguments
> >   symbol:setjmp file:(null) line:0 offset:0 return:0 lazy:(null)
> >   symbol:longjmp file:(null) line:0 offset:0 return:0 lazy:(null)
> >   symbol:longjmp_target file:(null) line:0 offset:0 return:0 lazy:(null)
> >   symbol:lll_lock_wait_private file:(null) line:0 offset:0 return:0 lazy:(null)
> >   symbol:memory_mallopt_arena_max file:(null) line:0 offset:0 return:0 lazy:(null)
> >   symbol:memory_mallopt_arena_test file:(null) line:0 offset:0 return:0 lazy:(null)
> >   symbol:memory_tunable_tcache_max_bytes file:(null) line:0 offset:0 return:0 lazy:(null)
> >   symbol:memory_tunable_tcache_count file:(null) line:0 offset:0 return:0 lazy:(null)
> >   symbol:memory_tunable_tcache_unsorted_limit file:(null) line:0 offset:0 return:0 lazy:(null)
> >   symbol:memory_mallopt_trim_threshold file:(null) line:0 offset:0 return:0 lazy:(null)
> >   symbol:memory_mallopt_top_pad file:(null) line:0 offset:0 return:0 lazy:(null)
> >   symbol:memory_mallopt_mmap_threshold file:(null) line:0 offset:0 return:0 lazy:(null)
> >   symbol:memory_mallopt_mmap_max file:(null) line:0 offset:0 return:0 lazy:(null)
> >   symbol:memory_mallopt_perturb file:(null) line:0 offset:0 return:0 lazy:(null)
> >   symbol:memory_mallopt_mxfast file:(null) line:0 offset:0 return:0 lazy:(null)
> >   symbol:memory_heap_new file:(null) line:0 offset:0 return:0 lazy:(null)
> >   symbol:memory_arena_reuse_free_list file:(null) line:0 offset:0 return:0 lazy:(null)
> >   symbol:memory_arena_reuse file:(null) line:0 offset:0 return:0 lazy:(null)
> >   symbol:memory_arena_reuse_wait file:(null) line:0 offset:0 return:0 lazy:(null)
> >   symbol:memory_arena_new file:(null) line:0 offset:0 return:0 lazy:(null)
> >   symbol:memory_arena_retry file:(null) line:0 offset:0 return:0 lazy:(null)
> >   symbol:memory_sbrk_less file:(null) line:0 offset:0 return:0 lazy:(null)
> >   symbol:memory_heap_free file:(null) line:0 offset:0 return:0 lazy:(null)
> >   symbol:memory_heap_less file:(null) line:0 offset:0 return:0 lazy:(null)
> >   symbol:memory_tcache_double_free file:(null) line:0 offset:0 return:0 lazy:(null)
> >   symbol:memory_heap_more file:(null) line:0 offset:0 return:0 lazy:(null)
> >   symbol:memory_sbrk_more file:(null) line:0 offset:0 return:0 lazy:(null)
> >   symbol:memory_malloc_retry file:(null) line:0 offset:0 return:0 lazy:(null)
> >   symbol:memory_memalign_retry file:(null) line:0 offset:0 return:0 lazy:(null)
> >   symbol:memory_mallopt_free_dyn_thresholds file:(null) line:0 offset:0 return:0 lazy:(null)
> >   symbol:memory_realloc_retry file:(null) line:0 offset:0 return:0 lazy:(null)
> >   symbol:memory_calloc_retry file:(null) line:0 offset:0 return:0 lazy:(null)
> >   symbol:memory_mallopt file:(null) line:0 offset:0 return:0 lazy:(null)
> >   Open Debuginfo file: /usr/lib/debug/usr/lib64/libc-2.30.so.debug
> >   Try to find probe point from debuginfo.
> >   Opening /sys/kernel/debug/tracing//README write=0
> >   Failed to find the location of the '%di' variable at this address.
> >    Perhaps it has been optimized out.
> >    Use -V with the --range option to show '%di' location range.
> >   An error occurred in debuginfo analysis (-2).
> >   Trying to use symbols.
> >   Opening /sys/kernel/debug/tracing//uprobe_events write=1
> >   Writing event: p:probe_libc/memcpy /usr/lib64/libc-2.30.so:0x914c0 arg1=%di
> >   Writing event: p:probe_libc/memcpy /usr/lib64/libc-2.30.so:0x914c0 arg1=%di
> >   Failed to write event: File exists
> >     Error: Failed to add events. Reason: File exists (Code: -17)
> > 
> > You can see the perf tried to write completely same probe definition
> > twice, which caused an error.
> > 
> > To fix this issue, check the symbol list and drop duplicated
> > symbols (which has same symbol name and address) from it.
> > 
> > With this patch;
> > 
> >   # perf probe -x /lib64/libc-2.30.so -a "memcpy arg1=%di"
> >   Failed to find the location of the '%di' variable at this address.
> >    Perhaps it has been optimized out.
> >    Use -V with the --range option to show '%di' location range.
> >   Added new events:
> >     probe_libc:memcpy    (on memcpy in /usr/lib64/libc-2.30.so with arg1=%di)
> >     probe_libc:memcpy    (on memcpy in /usr/lib64/libc-2.30.so with arg1=%di)
> > 
> >   You can now use it in all perf tools, such as:
> > 
> >   	perf record -e probe_libc:memcpy -aR sleep 1
> > 
> > 
> > Reported-by: Andi Kleen <andi@firstfloor.org>
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> > ---
> >  Changes in V2
> >   - Change "find" word to "Found".
> > ---
> >  tools/perf/util/probe-event.c |   10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > index df713a5d1e26..8cd1224e5f4c 100644
> > --- a/tools/perf/util/probe-event.c
> > +++ b/tools/perf/util/probe-event.c
> > @@ -2968,6 +2968,16 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
> >  	for (j = 0; j < num_matched_functions; j++) {
> >  		sym = syms[j];
> >  
> > +		/* There can be duplicated symbols in the map */
> > +		for (i = 0; i < j; i++)
> > +			if (sym->start == syms[i]->start) {
> > +				pr_debug("Found duplicated symbol %s @ %lx\n",
> > +					 sym->name, sym->start);
> > +				break;
> > +			}
> 
> Breaks 32-bit builds with:
> 
>   CC       /tmp/build/perf/util/demangle-java.o
> In file included from util/probe-event.c:27:
> util/probe-event.c: In function 'find_probe_trace_events_from_map':
> util/probe-event.c:2978:14: error: format '%lx' expects argument of type 'long unsigned int', but argument 5 has type 'u64' {aka 'long long unsigned int'} [-Werror=format=]
>      pr_debug("Found duplicated symbol %s @ %lx\n",
>               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> util/debug.h:17:21: note: in definition of macro 'pr_fmt'
>  #define pr_fmt(fmt) fmt
>                      ^~~
> util/probe-event.c:2978:5: note: in expansion of macro 'pr_debug'
>      pr_debug("Found duplicated symbol %s @ %lx\n",
>      ^~~~~~~~
>   CC       /tmp/build/perf/util/demangle-rust.o

Oops.

> I'll change this to use PRIx64.

Yeah, I should have used it.

Thank you very much!

> 
> - Arnaldo
> 
> > +		if (i != j)
> > +			continue;
> > +
> >  		tev = (*tevs) + ret;
> >  		tp = &tev->point;
> >  		if (ret == num_matched_functions) {
> > 
> 
> -- 
> 
> - Arnaldo


-- 
Masami Hiramatsu <mhiramat@kernel.org>

[PATCH v2 2/4] perf-probe: Fix wrong variable warning when the probe point is not found

[Masami Hiramatsu]

Fix a wrong "variable not found" warning when the probe point is
not found in the debuginfo.
Since the debuginfo__find_probes() can return 0 even if it does not
find given probe point in the debuginfo, fill_empty_trace_arg() can
be called with tf.ntevs == 0 and it can warn a wrong warning.
To fix this, reject ntevs == 0 in fill_empty_trace_arg().

E.g. without this patch;

  # perf probe -x /lib64/libc-2.30.so -a "memcpy arg1=%di"
  Failed to find the location of the '%di' variable at this address.
   Perhaps it has been optimized out.
   Use -V with the --range option to show '%di' location range.
  Added new events:
    probe_libc:memcpy    (on memcpy in /usr/lib64/libc-2.30.so with arg1=%di)
    probe_libc:memcpy    (on memcpy in /usr/lib64/libc-2.30.so with arg1=%di)

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

  	perf record -e probe_libc:memcpy -aR sleep 1

With this;

  # perf probe -x /lib64/libc-2.30.so -a "memcpy arg1=%di"
  Added new events:
    probe_libc:memcpy    (on memcpy in /usr/lib64/libc-2.30.so with arg1=%di)
    probe_libc:memcpy    (on memcpy in /usr/lib64/libc-2.30.so with arg1=%di)

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

  	perf record -e probe_libc:memcpy -aR sleep 1


Reported-by: Andi Kleen <andi@firstfloor.org>
Fixes: cb4027308570 ("perf probe: Trace a magic number if variable is not found")
Cc: stable@vger.kernel.org
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Tested-by: Andi Kleen <ak@linux.intel.com>
Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
 tools/perf/util/probe-finder.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 55924255c535..9963e4e8ea20 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1408,6 +1408,9 @@ static int fill_empty_trace_arg(struct perf_probe_event *pev,
 	char *type;
 	int i, j, ret;
 
+	if (!ntevs)
+		return -ENOENT;
+
 	for (i = 0; i < pev->nargs; i++) {
 		type = NULL;
 		for (j = 0; j < ntevs; j++) {

[PATCH v2 3/4] perf-probe: Fix memory leakage when the probe point is not found

[Masami Hiramatsu]

Fix the memory leakage in debuginfo__find_trace_events() when the probe
point is not found in the debuginfo. If there is no probe point found in
the debuginfo, debuginfo__find_probes() will NOT return -ENOENT, but 0.
Thus the caller of debuginfo__find_probes() must check the tf.ntevs and
release the allocated memory for the array of struct probe_trace_event.

The current code releases the memory only if the debuginfo__find_probes()
hits an error but not checks tf.ntevs. In the result, the memory allocated
on *tevs are not released if tf.ntevs == 0.

This fixes the memory leakage by checking tf.ntevs == 0 in addition to
ret < 0.

Fixes: ff741783506c ("perf probe: Introduce debuginfo to encapsulate dwarf information")
Cc: stable@vger.kernel.org
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
 tools/perf/util/probe-finder.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 9963e4e8ea20..659024342e9a 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1467,7 +1467,7 @@ int debuginfo__find_trace_events(struct debuginfo *dbg,
 	if (ret >= 0 && tf.pf.skip_empty_arg)
 		ret = fill_empty_trace_arg(pev, tf.tevs, tf.ntevs);
 
-	if (ret < 0) {
+	if (ret < 0 || tf.ntevs == 0) {
 		for (i = 0; i < tf.ntevs; i++)
 			clear_probe_trace_event(&tf.tevs[i]);
 		zfree(tevs);

[PATCH v2 4/4] perf-probe: Warn if the target function is GNU Indirect function

[Masami Hiramatsu]

Warn if the probe target function is GNU indirect function (GNU_IFUNC)
because it may not what the user want to probe.

The GNU indirect function ( https://sourceware.org/glibc/wiki/GNU_IFUNC )
is the dynamic solved symbol at runtime. IFUNC function is a selector
which is invoked from the elf loader, but the symbol address of the
function which will be modified by the IFUNC is same as the IFUNC in
the symbol table. This can confuse users who is trying to probe on
such functions.

For example, the memcpy is one of IFUNC.

# perf probe -x /lib64/libc-2.30.so -a memcpy
# perf probe -l
  probe_libc:memcpy    (on __new_memcpy_ifunc@x86_64/multiarch/memcpy.c in /usr/lib64/libc-2.30.so)

the probe is put on a IFUNC.

# perf record -e probe_libc:memcpy --call-graph dwarf -aR ./perf
# perf script
perf  1742 [000] 26201.715632: probe_libc:memcpy: (7fdaa53824c0)
            7fdaa53824c0 __new_memcpy_ifunc+0x0 (inlined)
            7fdaa5d4a980 elf_machine_rela+0x6c0 (inlined)
            7fdaa5d4a980 elf_dynamic_do_Rela+0x6c0 (inlined)
            7fdaa5d4a980 _dl_relocate_object+0x6c0 (/usr/lib64/ld-2.30.so)
            7fdaa5d42155 dl_main+0x1cc5 (/usr/lib64/ld-2.30.so)
            7fdaa5d5831a _dl_sysdep_start+0x54a (/usr/lib64/ld-2.30.so)
            7fdaa5d3ffeb _dl_start_final+0x25b (inlined)
            7fdaa5d3ffeb _dl_start+0x25b (/usr/lib64/ld-2.30.so)
            7fdaa5d3f117 .annobin_rtld.c+0x7 (inlined)
...

And the event is invoked from the elf loader instead of the target
program's main code.


Moreover, at this moment, we can not probe on the function which will
be selected by the IFUNC, because it is determined at runtime. But
uprobe will be prepared before running the target binary.

Thus, I decided to warn user when the perf probe detects the probe point
is on the GNU IFUNC symbol. Someone who wants to probe an IFUNC symbol to
debug the IFUNC function, they can ignore this warning.

Reported-by: Andi Kleen <andi@firstfloor.org>
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
  Changes in v2:
   - Check GNU_IFUNC only for uprobe
   - Show function name instead of the address.
   - Update the warning message according to Andi's comment.
---
 tools/perf/util/probe-event.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 8cd1224e5f4c..679447f13c20 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -375,9 +375,13 @@ static int find_alternative_probe_point(struct debuginfo *dinfo,
 
 	/* Find the address of given function */
 	map__for_each_symbol_by_name(map, pp->function, sym) {
-		if (uprobes)
+		if (uprobes) {
 			address = sym->start;
-		else
+			if (sym->type == STT_GNU_IFUNC)
+				pr_warning("Warning: The probe function (%s) is a GNU indirect function.\n"
+					   "Consider identifying the final function used at run time and set the probe directly on that.\n",
+					   pp->function);
+		} else
 			address = map->unmap_ip(map, sym->start) - map->reloc;
 		break;
 	}

Re: [PATCH v2 4/4] perf-probe: Warn if the target function is GNU Indirect function

[Srikar Dronamraju]

* Masami Hiramatsu <mhiramat@kernel.org> [2020-07-10 22:11:33]:

> Warn if the probe target function is GNU indirect function (GNU_IFUNC)
> because it may not what the user want to probe.
> 
> The GNU indirect function ( https://sourceware.org/glibc/wiki/GNU_IFUNC )
> is the dynamic solved symbol at runtime. IFUNC function is a selector
> which is invoked from the elf loader, but the symbol address of the
> function which will be modified by the IFUNC is same as the IFUNC in
> the symbol table. This can confuse users who is trying to probe on
> such functions.
> 
> For example, the memcpy is one of IFUNC.
> 
> # perf probe -x /lib64/libc-2.30.so -a memcpy
> # perf probe -l
>   probe_libc:memcpy    (on __new_memcpy_ifunc@x86_64/multiarch/memcpy.c in /usr/lib64/libc-2.30.so)
> 
> the probe is put on a IFUNC.
> 
> # perf record -e probe_libc:memcpy --call-graph dwarf -aR ./perf
> 
> Thus, I decided to warn user when the perf probe detects the probe point
> is on the GNU IFUNC symbol. Someone who wants to probe an IFUNC symbol to
> debug the IFUNC function, they can ignore this warning.
> 
> Reported-by: Andi Kleen <andi@firstfloor.org>
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>

Looks good to me.

Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>

> ---
>   Changes in v2:
>    - Check GNU_IFUNC only for uprobe
>    - Show function name instead of the address.
>    - Update the warning message according to Andi's comment.
> ---
>  tools/perf/util/probe-event.c |    8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 8cd1224e5f4c..679447f13c20 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -375,9 +375,13 @@ static int find_alternative_probe_point(struct debuginfo *dinfo,
> 
>  	/* Find the address of given function */
>  	map__for_each_symbol_by_name(map, pp->function, sym) {
> -		if (uprobes)
> +		if (uprobes) {
>  			address = sym->start;
> -		else
> +			if (sym->type == STT_GNU_IFUNC)
> +				pr_warning("Warning: The probe function (%s) is a GNU indirect function.\n"
> +					   "Consider identifying the final function used at run time and set the probe directly on that.\n",
> +					   pp->function);
> +		} else
>  			address = map->unmap_ip(map, sym->start) - map->reloc;
>  		break;
>  	}
> 

-- 
Thanks and Regards
Srikar Dronamraju

Re: [PATCH v2 4/4] perf-probe: Warn if the target function is GNU Indirect function

[Arnaldo Carvalho de Melo]

Em Fri, Jul 10, 2020 at 07:27:12PM +0530, Srikar Dronamraju escreveu:
> * Masami Hiramatsu <mhiramat@kernel.org> [2020-07-10 22:11:33]:
> 
> > Warn if the probe target function is GNU indirect function (GNU_IFUNC)
> > because it may not what the user want to probe.
> > 
> > The GNU indirect function ( https://sourceware.org/glibc/wiki/GNU_IFUNC )
> > is the dynamic solved symbol at runtime. IFUNC function is a selector
> > which is invoked from the elf loader, but the symbol address of the
> > function which will be modified by the IFUNC is same as the IFUNC in
> > the symbol table. This can confuse users who is trying to probe on
> > such functions.
> > 
> > For example, the memcpy is one of IFUNC.
> > 
> > # perf probe -x /lib64/libc-2.30.so -a memcpy
> > # perf probe -l
> >   probe_libc:memcpy    (on __new_memcpy_ifunc@x86_64/multiarch/memcpy.c in /usr/lib64/libc-2.30.so)
> > 
> > the probe is put on a IFUNC.
> > 
> > # perf record -e probe_libc:memcpy --call-graph dwarf -aR ./perf
> > 
> > Thus, I decided to warn user when the perf probe detects the probe point
> > is on the GNU IFUNC symbol. Someone who wants to probe an IFUNC symbol to
> > debug the IFUNC function, they can ignore this warning.
> > 
> > Reported-by: Andi Kleen <andi@firstfloor.org>
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> 
> Looks good to me.
> 
> Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>

Thanks, applied.

- Arnaldo