[PATCH v2 0/5] perf-probe: Improve probing on versioned symbols

[PATCH v2 0/5] perf-probe: Improve probing on versioned symbols

[Masami Hiramatsu]

Hi,

Here is the 2nd version of the series for probing on
versioned symbols in libraries. This includes 5 patches
to fix the issues discussed on perf-users ML 
(https://www.spinics.net/lists/linux-perf-users/msg04637.html)

The first version is here; https://lkml.org/lkml/2017/12/5/1124

Here is the updates;

[3/5] Add a description in Documentation/perf-probe.txt
[5/5] Add a description and examples in Documentation/perf-probe.txt

Thank you,
---

Masami Hiramatsu (5):
      perf-probe: Add warning message if there is unexpected event name
      perf-probe: Cut off the version suffix from event name
      perf-probe: Add __return suffix for return events
      perf-probe: Find versioned symbols from map
      perf-probe: Support escaped character in parser


 tools/perf/Documentation/perf-probe.txt     |   18 ++++++
 tools/perf/arch/powerpc/util/sym-handling.c |    8 +++
 tools/perf/util/probe-event.c               |   81 +++++++++++++++++++--------
 tools/perf/util/string.c                    |   46 +++++++++++++++
 tools/perf/util/string2.h                   |    2 +
 tools/perf/util/symbol.c                    |    5 ++
 tools/perf/util/symbol.h                    |    1 
 7 files changed, 137 insertions(+), 24 deletions(-)

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

[PATCH v2 1/5] perf-probe: Add warning message if there is unexpected event name

[Masami Hiramatsu]

This improve the error message so that user can know
event-name error before writing new events to
kprobe-events interface.

E.g.
   ======
   #./perf probe -x /lib64/libc-2.25.so malloc_get_state*
   Internal error: "malloc_get_state@GLIBC_2" is wrong event name.
     Error: Failed to add events.
   ======

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 tools/perf/util/probe-event.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index b7aaf9b2294d..c0067950e56f 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2625,6 +2625,14 @@ static int get_new_event_name(char *buf, size_t len, const char *base,
 
 out:
 	free(nbase);
+
+	/* Final validation */
+	if (ret >= 0 && !is_c_func_name(buf)) {
+		pr_warning("Internal error: \"%s\" is wrong event name.\n",
+			   buf);
+		ret = -EINVAL;
+	}
+
 	return ret;
 }
 

Re: [PATCH v2 1/5] perf-probe: Add warning message if there is unexpected event name

[Arnaldo Carvalho de Melo]

Em Thu, Dec 07, 2017 at 04:19:45PM +0900, Masami Hiramatsu escreveu:
> This improve the error message so that user can know
> event-name error before writing new events to
> kprobe-events interface.
> 
> E.g.
>    ======
>    #./perf probe -x /lib64/libc-2.25.so malloc_get_state*
>    Internal error: "malloc_get_state@GLIBC_2" is wrong event name.

I'm changing this to:

Internal error: "malloc_get_state@GLIBC_2" is an invalid event name for the /sys/kernel/debug/tracing/kprobe_events interface.

Ok?

- Arnaldo

>      Error: Failed to add events.
>    ======
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  tools/perf/util/probe-event.c |    8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index b7aaf9b2294d..c0067950e56f 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -2625,6 +2625,14 @@ static int get_new_event_name(char *buf, size_t len, const char *base,
>  
>  out:
>  	free(nbase);
> +
> +	/* Final validation */
> +	if (ret >= 0 && !is_c_func_name(buf)) {
> +		pr_warning("Internal error: \"%s\" is wrong event name.\n",
> +			   buf);
> +		ret = -EINVAL;
> +	}
> +
>  	return ret;
>  }
>  

Re: [PATCH v2 1/5] perf-probe: Add warning message if there is unexpected event name

[Masami Hiramatsu]

On Thu, 7 Dec 2017 12:53:43 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Thu, Dec 07, 2017 at 04:19:45PM +0900, Masami Hiramatsu escreveu:
> > This improve the error message so that user can know
> > event-name error before writing new events to
> > kprobe-events interface.
> > 
> > E.g.
> >    ======
> >    #./perf probe -x /lib64/libc-2.25.so malloc_get_state*
> >    Internal error: "malloc_get_state@GLIBC_2" is wrong event name.
> 
> I'm changing this to:
> 
> Internal error: "malloc_get_state@GLIBC_2" is an invalid event name for the /sys/kernel/debug/tracing/kprobe_events interface.
> 
> Ok?

Hmm, it is a kernel limitation (or spec). Would you think we really need it?
(are there any chance to allow '@' for event name?)

Thank you,

> 
> - Arnaldo
> 
> >      Error: Failed to add events.
> >    ======
> > 
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > ---
> >  tools/perf/util/probe-event.c |    8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > index b7aaf9b2294d..c0067950e56f 100644
> > --- a/tools/perf/util/probe-event.c
> > +++ b/tools/perf/util/probe-event.c
> > @@ -2625,6 +2625,14 @@ static int get_new_event_name(char *buf, size_t len, const char *base,
> >  
> >  out:
> >  	free(nbase);
> > +
> > +	/* Final validation */
> > +	if (ret >= 0 && !is_c_func_name(buf)) {
> > +		pr_warning("Internal error: \"%s\" is wrong event name.\n",
> > +			   buf);
> > +		ret = -EINVAL;
> > +	}
> > +
> >  	return ret;
> >  }
> >  


-- 
Masami Hiramatsu <mhiramat@kernel.org>

[PATCH v2 2/5] perf-probe: Cut off the version suffix from event name

[Masami Hiramatsu]

Cut off the version suffix (e.g. @GLIBC_2.2.5 etc.) from
automatic generated event name. This fixes wildcard event
adding like below case;

  =====
  # perf probe -x /lib64/libc-2.25.so malloc*
  Internal error: "malloc_get_state@GLIBC_2" is wrong event name.
    Error: Failed to add events.
  =====

This failure was caused by a versioned suffix symbol.
With this fix, perf probe automatically cuts the
suffix after @ as below.

  =====
  # ./perf probe -x /lib64/libc-2.25.so malloc*
  Added new events:
    probe_libc:malloc_printerr (on malloc* in /usr/lib64/libc-2.25.so)
    probe_libc:malloc_consolidate (on malloc* in /usr/lib64/libc-2.25.so)
    probe_libc:malloc_check (on malloc* in /usr/lib64/libc-2.25.so)
    probe_libc:malloc_hook_ini (on malloc* in /usr/lib64/libc-2.25.so)
    probe_libc:malloc    (on malloc* in /usr/lib64/libc-2.25.so)
    probe_libc:malloc_trim (on malloc* in /usr/lib64/libc-2.25.so)
    probe_libc:malloc_usable_size (on malloc* in /usr/lib64/libc-2.25.so)
    probe_libc:malloc_stats (on malloc* in /usr/lib64/libc-2.25.so)
    probe_libc:malloc_info (on malloc* in /usr/lib64/libc-2.25.so)
    probe_libc:mallochook (on malloc* in /usr/lib64/libc-2.25.so)
    probe_libc:malloc_get_state (on malloc* in /usr/lib64/libc-2.25.so)
    probe_libc:malloc_set_state (on malloc* in /usr/lib64/libc-2.25.so)

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

	  perf record -e probe_libc:malloc_set_state -aR sleep 1

  =====

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
Reported-by: bhargavb <bhargavaramudu@gmail.com>
---
 tools/perf/util/probe-event.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index c0067950e56f..74435fb7ab7f 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2584,8 +2584,8 @@ static int get_new_event_name(char *buf, size_t len, const char *base,
 	if (!nbase)
 		return -ENOMEM;
 
-	/* Cut off the dot suffixes (e.g. .const, .isra)*/
-	p = strchr(nbase, '.');
+	/* Cut off the dot suffixes (e.g. .const, .isra) and version suffixes */
+	p = strpbrk(nbase, ".@");
 	if (p && p != nbase)
 		*p = '\0';
 

Re: [PATCH v2 2/5] perf-probe: Cut off the version suffix from event name

[Paul Clarke]

On 12/07/2017 01:20 AM, Masami Hiramatsu wrote:
> Cut off the version suffix (e.g. @GLIBC_2.2.5 etc.) from
> automatic generated event name. This fixes wildcard event
> adding like below case;
> 
>   =====
>   # perf probe -x /lib64/libc-2.25.so malloc*
>   Internal error: "malloc_get_state@GLIBC_2" is wrong event name.
>     Error: Failed to add events.
>   =====
> 
> This failure was caused by a versioned suffix symbol.
> With this fix, perf probe automatically cuts the
> suffix after @ as below.
> 
>   =====
>   # ./perf probe -x /lib64/libc-2.25.so malloc*
>   Added new events:
>     probe_libc:malloc_printerr (on malloc* in /usr/lib64/libc-2.25.so)
>     probe_libc:malloc_consolidate (on malloc* in /usr/lib64/libc-2.25.so)
>     probe_libc:malloc_check (on malloc* in /usr/lib64/libc-2.25.so)
>     probe_libc:malloc_hook_ini (on malloc* in /usr/lib64/libc-2.25.so)
>     probe_libc:malloc    (on malloc* in /usr/lib64/libc-2.25.so)
>     probe_libc:malloc_trim (on malloc* in /usr/lib64/libc-2.25.so)
>     probe_libc:malloc_usable_size (on malloc* in /usr/lib64/libc-2.25.so)
>     probe_libc:malloc_stats (on malloc* in /usr/lib64/libc-2.25.so)
>     probe_libc:malloc_info (on malloc* in /usr/lib64/libc-2.25.so)
>     probe_libc:mallochook (on malloc* in /usr/lib64/libc-2.25.so)
>     probe_libc:malloc_get_state (on malloc* in /usr/lib64/libc-2.25.so)
>     probe_libc:malloc_set_state (on malloc* in /usr/lib64/libc-2.25.so)
> 
>   You can now use it in all perf tools, such as:
> 
> 	  perf record -e probe_libc:malloc_set_state -aR sleep 1
> 
>   =====

I think this will still fall over for a case where there are multiple versions of the same base symbol, like:

$ nm /lib64/libc.so.6 | egrep ' sched_getaffinity'
0000000000134430 T sched_getaffinity@GLIBC_2.3.3
00000000000dcf00 T sched_getaffinity@@GLIBC_2.3.4

Should we retain the versioning string in some form?  "sched_getaffinity--GLIBC_2.3.4"?  Should we instead interpret the '@' symbol more flexibly, so maybe first assume it is a version string, and if not found, try to see if it's a valid "@SRC" specification?

PC

Re: [PATCH v2 2/5] perf-probe: Cut off the version suffix from event name

[Masami Hiramatsu]

On Thu, 7 Dec 2017 10:34:51 -0600
Paul Clarke <pc@us.ibm.com> wrote:

> 
> 
> On 12/07/2017 01:20 AM, Masami Hiramatsu wrote:
> > Cut off the version suffix (e.g. @GLIBC_2.2.5 etc.) from
> > automatic generated event name. This fixes wildcard event
> > adding like below case;
> > 
> >   =====
> >   # perf probe -x /lib64/libc-2.25.so malloc*
> >   Internal error: "malloc_get_state@GLIBC_2" is wrong event name.
> >     Error: Failed to add events.
> >   =====
> > 
> > This failure was caused by a versioned suffix symbol.
> > With this fix, perf probe automatically cuts the
> > suffix after @ as below.
> > 
> >   =====
> >   # ./perf probe -x /lib64/libc-2.25.so malloc*
> >   Added new events:
> >     probe_libc:malloc_printerr (on malloc* in /usr/lib64/libc-2.25.so)
> >     probe_libc:malloc_consolidate (on malloc* in /usr/lib64/libc-2.25.so)
> >     probe_libc:malloc_check (on malloc* in /usr/lib64/libc-2.25.so)
> >     probe_libc:malloc_hook_ini (on malloc* in /usr/lib64/libc-2.25.so)
> >     probe_libc:malloc    (on malloc* in /usr/lib64/libc-2.25.so)
> >     probe_libc:malloc_trim (on malloc* in /usr/lib64/libc-2.25.so)
> >     probe_libc:malloc_usable_size (on malloc* in /usr/lib64/libc-2.25.so)
> >     probe_libc:malloc_stats (on malloc* in /usr/lib64/libc-2.25.so)
> >     probe_libc:malloc_info (on malloc* in /usr/lib64/libc-2.25.so)
> >     probe_libc:mallochook (on malloc* in /usr/lib64/libc-2.25.so)
> >     probe_libc:malloc_get_state (on malloc* in /usr/lib64/libc-2.25.so)
> >     probe_libc:malloc_set_state (on malloc* in /usr/lib64/libc-2.25.so)
> > 
> >   You can now use it in all perf tools, such as:
> > 
> > 	  perf record -e probe_libc:malloc_set_state -aR sleep 1
> > 
> >   =====
> 
> I think this will still fall over for a case where there are multiple versions of the same base symbol, like:
> 
> $ nm /lib64/libc.so.6 | egrep ' sched_getaffinity'
> 0000000000134430 T sched_getaffinity@GLIBC_2.3.3
> 00000000000dcf00 T sched_getaffinity@@GLIBC_2.3.4

No, in that case perf probe adds number suffix (_1, _2...) for new events :)

This feature (number suffix) is anyway required for the case that inlined
function is spreaded in multiple places. I think this is natural for perf probe.

Thank you,

> 
> Should we retain the versioning string in some form?  "sched_getaffinity--GLIBC_2.3.4"?  Should we instead interpret the '@' symbol more flexibly, so maybe first assume it is a version string, and if not found, try to see if it's a valid "@SRC" specification?
> 
> PC
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH v2 2/5] perf-probe: Cut off the version suffix from event name

[Paul Clarke]

On 12/07/2017 09:01 PM, Masami Hiramatsu wrote:
> On Thu, 7 Dec 2017 10:34:51 -0600
> Paul Clarke <pc@us.ibm.com> wrote:
>> On 12/07/2017 01:20 AM, Masami Hiramatsu wrote:
>>> Cut off the version suffix (e.g. @GLIBC_2.2.5 etc.) from
>>> automatic generated event name. This fixes wildcard event
>>> adding like below case;
>>>
>>>   =====
>>>   # perf probe -x /lib64/libc-2.25.so malloc*
>>>   Internal error: "malloc_get_state@GLIBC_2" is wrong event name.
>>>     Error: Failed to add events.
>>>   =====
>>>
>>> This failure was caused by a versioned suffix symbol.
>>> With this fix, perf probe automatically cuts the
>>> suffix after @ as below.
>>>
>>>   =====
>>>   # ./perf probe -x /lib64/libc-2.25.so malloc*
>>>   Added new events:
>>>     probe_libc:malloc_printerr (on malloc* in /usr/lib64/libc-2.25.so)
>>>     probe_libc:malloc_consolidate (on malloc* in /usr/lib64/libc-2.25.so)
>>>     probe_libc:malloc_check (on malloc* in /usr/lib64/libc-2.25.so)
>>>     probe_libc:malloc_hook_ini (on malloc* in /usr/lib64/libc-2.25.so)
>>>     probe_libc:malloc    (on malloc* in /usr/lib64/libc-2.25.so)
>>>     probe_libc:malloc_trim (on malloc* in /usr/lib64/libc-2.25.so)
>>>     probe_libc:malloc_usable_size (on malloc* in /usr/lib64/libc-2.25.so)
>>>     probe_libc:malloc_stats (on malloc* in /usr/lib64/libc-2.25.so)
>>>     probe_libc:malloc_info (on malloc* in /usr/lib64/libc-2.25.so)
>>>     probe_libc:mallochook (on malloc* in /usr/lib64/libc-2.25.so)
>>>     probe_libc:malloc_get_state (on malloc* in /usr/lib64/libc-2.25.so)
>>>     probe_libc:malloc_set_state (on malloc* in /usr/lib64/libc-2.25.so)
>>>
>>>   You can now use it in all perf tools, such as:
>>>
>>> 	  perf record -e probe_libc:malloc_set_state -aR sleep 1
>>>
>>>   =====
>>
>> I think this will still fall over for a case where there are multiple versions of the same base symbol, like:
>>
>> $ nm /lib64/libc.so.6 | egrep ' sched_getaffinity'
>> 0000000000134430 T sched_getaffinity@GLIBC_2.3.3
>> 00000000000dcf00 T sched_getaffinity@@GLIBC_2.3.4
> 
> No, in that case perf probe adds number suffix (_1, _2...) for new events :)
> 
> This feature (number suffix) is anyway required for the case that inlined
> function is spreaded in multiple places. I think this is natural for perf probe.

I can live with that.

Is the versioning tag retained for display with "perf probe -l" display, so the probe points can be distinguished?

In other words, will one see:
# perf probe -l
  probe_libc:sched_getaffinity (on sched_getaffinity@GLIBC_2.3.3 in /usr/lib64/libc-2.17.so)
  probe_libc:sched_getaffinity_1 (on sched_getaffinity@@GLIBC_2.3.4 in /usr/lib64/libc-2.17.so)
or
# perf probe -l
  probe_libc:sched_getaffinity (on sched_getaffinity in /usr/lib64/libc-2.17.so)
  probe_libc:sched_getaffinity_1 (on sched_getaffinity in /usr/lib64/libc-2.17.so)

PC

Re: [PATCH v2 2/5] perf-probe: Cut off the version suffix from event name

[Masami Hiramatsu]

On Fri, 8 Dec 2017 08:49:46 -0600
Paul Clarke <pc@us.ibm.com> wrote:

> 
> 
> On 12/07/2017 09:01 PM, Masami Hiramatsu wrote:
> > On Thu, 7 Dec 2017 10:34:51 -0600
> > Paul Clarke <pc@us.ibm.com> wrote:
> >> On 12/07/2017 01:20 AM, Masami Hiramatsu wrote:
> >>> Cut off the version suffix (e.g. @GLIBC_2.2.5 etc.) from
> >>> automatic generated event name. This fixes wildcard event
> >>> adding like below case;
> >>>
> >>>   =====
> >>>   # perf probe -x /lib64/libc-2.25.so malloc*
> >>>   Internal error: "malloc_get_state@GLIBC_2" is wrong event name.
> >>>     Error: Failed to add events.
> >>>   =====
> >>>
> >>> This failure was caused by a versioned suffix symbol.
> >>> With this fix, perf probe automatically cuts the
> >>> suffix after @ as below.
> >>>
> >>>   =====
> >>>   # ./perf probe -x /lib64/libc-2.25.so malloc*
> >>>   Added new events:
> >>>     probe_libc:malloc_printerr (on malloc* in /usr/lib64/libc-2.25.so)
> >>>     probe_libc:malloc_consolidate (on malloc* in /usr/lib64/libc-2.25.so)
> >>>     probe_libc:malloc_check (on malloc* in /usr/lib64/libc-2.25.so)
> >>>     probe_libc:malloc_hook_ini (on malloc* in /usr/lib64/libc-2.25.so)
> >>>     probe_libc:malloc    (on malloc* in /usr/lib64/libc-2.25.so)
> >>>     probe_libc:malloc_trim (on malloc* in /usr/lib64/libc-2.25.so)
> >>>     probe_libc:malloc_usable_size (on malloc* in /usr/lib64/libc-2.25.so)
> >>>     probe_libc:malloc_stats (on malloc* in /usr/lib64/libc-2.25.so)
> >>>     probe_libc:malloc_info (on malloc* in /usr/lib64/libc-2.25.so)
> >>>     probe_libc:mallochook (on malloc* in /usr/lib64/libc-2.25.so)
> >>>     probe_libc:malloc_get_state (on malloc* in /usr/lib64/libc-2.25.so)
> >>>     probe_libc:malloc_set_state (on malloc* in /usr/lib64/libc-2.25.so)
> >>>
> >>>   You can now use it in all perf tools, such as:
> >>>
> >>> 	  perf record -e probe_libc:malloc_set_state -aR sleep 1
> >>>
> >>>   =====
> >>
> >> I think this will still fall over for a case where there are multiple versions of the same base symbol, like:
> >>
> >> $ nm /lib64/libc.so.6 | egrep ' sched_getaffinity'
> >> 0000000000134430 T sched_getaffinity@GLIBC_2.3.3
> >> 00000000000dcf00 T sched_getaffinity@@GLIBC_2.3.4
> > 
> > No, in that case perf probe adds number suffix (_1, _2...) for new events :)
> > 
> > This feature (number suffix) is anyway required for the case that inlined
> > function is spreaded in multiple places. I think this is natural for perf probe.
> 
> I can live with that.
> 
> Is the versioning tag retained for display with "perf probe -l" display, so the probe points can be distinguished?
> 
> In other words, will one see:
> # perf probe -l
>   probe_libc:sched_getaffinity (on sched_getaffinity@GLIBC_2.3.3 in /usr/lib64/libc-2.17.so)
>   probe_libc:sched_getaffinity_1 (on sched_getaffinity@@GLIBC_2.3.4 in /usr/lib64/libc-2.17.so)

Yes, this style, since the symbols are recorded with version suffixes.

Thank you,

> or
> # perf probe -l
>   probe_libc:sched_getaffinity (on sched_getaffinity in /usr/lib64/libc-2.17.so)
>   probe_libc:sched_getaffinity_1 (on sched_getaffinity in /usr/lib64/libc-2.17.so)
> 
> PC
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-perf-users" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH v2 2/5] perf-probe: Cut off the version suffix from event name

[Arnaldo Carvalho de Melo]

Em Fri, Dec 08, 2017 at 08:49:46AM -0600, Paul Clarke escreveu:
> 
> 
> On 12/07/2017 09:01 PM, Masami Hiramatsu wrote:
> > On Thu, 7 Dec 2017 10:34:51 -0600
> > Paul Clarke <pc@us.ibm.com> wrote:
> >> On 12/07/2017 01:20 AM, Masami Hiramatsu wrote:
> >>> Cut off the version suffix (e.g. @GLIBC_2.2.5 etc.) from
> >>> automatic generated event name. This fixes wildcard event
> >>> adding like below case;
> >>>
> >>>   =====
> >>>   # perf probe -x /lib64/libc-2.25.so malloc*
> >>>   Internal error: "malloc_get_state@GLIBC_2" is wrong event name.
> >>>     Error: Failed to add events.
> >>>   =====
> >>>
> >>> This failure was caused by a versioned suffix symbol.
> >>> With this fix, perf probe automatically cuts the
> >>> suffix after @ as below.
> >>>
> >>>   =====
> >>>   # ./perf probe -x /lib64/libc-2.25.so malloc*
> >>>   Added new events:
> >>>     probe_libc:malloc_printerr (on malloc* in /usr/lib64/libc-2.25.so)
> >>>     probe_libc:malloc_consolidate (on malloc* in /usr/lib64/libc-2.25.so)
> >>>     probe_libc:malloc_check (on malloc* in /usr/lib64/libc-2.25.so)
> >>>     probe_libc:malloc_hook_ini (on malloc* in /usr/lib64/libc-2.25.so)
> >>>     probe_libc:malloc    (on malloc* in /usr/lib64/libc-2.25.so)
> >>>     probe_libc:malloc_trim (on malloc* in /usr/lib64/libc-2.25.so)
> >>>     probe_libc:malloc_usable_size (on malloc* in /usr/lib64/libc-2.25.so)
> >>>     probe_libc:malloc_stats (on malloc* in /usr/lib64/libc-2.25.so)
> >>>     probe_libc:malloc_info (on malloc* in /usr/lib64/libc-2.25.so)
> >>>     probe_libc:mallochook (on malloc* in /usr/lib64/libc-2.25.so)
> >>>     probe_libc:malloc_get_state (on malloc* in /usr/lib64/libc-2.25.so)
> >>>     probe_libc:malloc_set_state (on malloc* in /usr/lib64/libc-2.25.so)
> >>>
> >>>   You can now use it in all perf tools, such as:
> >>>
> >>> 	  perf record -e probe_libc:malloc_set_state -aR sleep 1
> >>>
> >>>   =====
> >>
> >> I think this will still fall over for a case where there are multiple versions of the same base symbol, like:
> >>
> >> $ nm /lib64/libc.so.6 | egrep ' sched_getaffinity'
> >> 0000000000134430 T sched_getaffinity@GLIBC_2.3.3
> >> 00000000000dcf00 T sched_getaffinity@@GLIBC_2.3.4
> > 
> > No, in that case perf probe adds number suffix (_1, _2...) for new events :)
> > 
> > This feature (number suffix) is anyway required for the case that inlined
> > function is spreaded in multiple places. I think this is natural for perf probe.
> 
> I can live with that.
> 
> Is the versioning tag retained for display with "perf probe -l" display, so the probe points can be distinguished?
> 
> In other words, will one see:
> # perf probe -l
>   probe_libc:sched_getaffinity (on sched_getaffinity@GLIBC_2.3.3 in /usr/lib64/libc-2.17.so)
>   probe_libc:sched_getaffinity_1 (on sched_getaffinity@@GLIBC_2.3.4 in /usr/lib64/libc-2.17.so)
> or
> # perf probe -l
>   probe_libc:sched_getaffinity (on sched_getaffinity in /usr/lib64/libc-2.17.so)
>   probe_libc:sched_getaffinity_1 (on sched_getaffinity in /usr/lib64/libc-2.17.so)

[root@jouet ~]# perf probe -x /lib64/libc-2.25.so malloc*
Added new events:
  probe_libc:malloc_printerr (on malloc* in /usr/lib64/libc-2.25.so)
  probe_libc:malloc_consolidate (on malloc* in /usr/lib64/libc-2.25.so)
  probe_libc:malloc_check (on malloc* in /usr/lib64/libc-2.25.so)
  probe_libc:malloc_hook_ini (on malloc* in /usr/lib64/libc-2.25.so)
  probe_libc:malloc    (on malloc* in /usr/lib64/libc-2.25.so)
  probe_libc:malloc_trim (on malloc* in /usr/lib64/libc-2.25.so)
  probe_libc:malloc_usable_size (on malloc* in /usr/lib64/libc-2.25.so)
  probe_libc:malloc_stats (on malloc* in /usr/lib64/libc-2.25.so)
  probe_libc:malloc_info (on malloc* in /usr/lib64/libc-2.25.so)
  probe_libc:mallochook (on malloc* in /usr/lib64/libc-2.25.so)
  probe_libc:malloc_get_state (on malloc* in /usr/lib64/libc-2.25.so)
  probe_libc:malloc_set_state (on malloc* in /usr/lib64/libc-2.25.so)

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

	perf record -e probe_libc:malloc_set_state -aR sleep 1

[root@jouet ~]# perf probe -l
  probe_libc:malloc    (on 0xd22a6973 in /usr/lib64/libc-2.25.so)
  probe_libc:malloc_check (on 0x25db6f5a in /usr/lib64/libc-2.25.so)
  probe_libc:malloc_consolidate (on 0x88f7c44a in /usr/lib64/libc-2.25.so)
  probe_libc:malloc_get_state (on 0xdd38eca7 in /usr/lib64/libc-2.25.so)
  probe_libc:malloc_hook_ini (on 0x8de2b533 in /usr/lib64/libc-2.25.so)
  probe_libc:malloc_info (on 0x8b327bde in /usr/lib64/libc-2.25.so)
  probe_libc:malloc_printerr (on 0x8fff087c in /usr/lib64/libc-2.25.so)
  probe_libc:malloc_set_state (on 0x861940e6 in /usr/lib64/libc-2.25.so)
  probe_libc:malloc_stats (on 0x51fa3cdc in /usr/lib64/libc-2.25.so)
  probe_libc:malloc_trim (on 0xa9fa4e43 in /usr/lib64/libc-2.25.so)
  probe_libc:malloc_usable_size (on 0xceb5e090 in /usr/lib64/libc-2.25.so)
  probe_libc:mallochook (on 0x6ebc00f5 in /usr/lib64/libc-2.25.so)
[root@jouet ~]#

[acme@jouet perf]$ nm /lib64/libc-2.25.so | grep malloc_get_state
0000000000151ba0 T malloc_get_state@GLIBC_2.2.5
[acme@jouet perf]$

For your example it finds two symbols, adds two but I couldn't find a
simple way of mapping one to the other, would be nice if 'perf probe -l'
showed them like 'nm' does.

[root@jouet ~]# nm /lib64/libc-2.25.so | grep ' sched_getaffinity'
0000000000151cd0 T sched_getaffinity@GLIBC_2.3.3
00000000000f3c10 T sched_getaffinity@@GLIBC_2.3.4
[root@jouet ~]#

[root@jouet ~]# perf probe -x /lib64/libc-2.25.so sched_getaffinity*
Added new events:
  probe_libc:sched_getaffinity (on sched_getaffinity* in /usr/lib64/libc-2.25.so)
  probe_libc:sched_getaffinity_1 (on sched_getaffinity* in /usr/lib64/libc-2.25.so)

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

	perf record -e probe_libc:sched_getaffinity_1 -aR sleep 1

[root@jouet ~]# perf probe -l
  probe_libc:sched_getaffinity (on 0x5fd95e7c in /usr/lib64/libc-2.25.so)
  probe_libc:sched_getaffinity_1 (on 0xd8d51601 in /usr/lib64/libc-2.25.so)
[root@jouet ~]#

Re: [PATCH v2 2/5] perf-probe: Cut off the version suffix from event name

[Masami Hiramatsu]

On Mon, 11 Dec 2017 15:25:47 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Fri, Dec 08, 2017 at 08:49:46AM -0600, Paul Clarke escreveu:
> > 
> > 
> > On 12/07/2017 09:01 PM, Masami Hiramatsu wrote:
> > > On Thu, 7 Dec 2017 10:34:51 -0600
> > > Paul Clarke <pc@us.ibm.com> wrote:
> > >> On 12/07/2017 01:20 AM, Masami Hiramatsu wrote:
> > >>> Cut off the version suffix (e.g. @GLIBC_2.2.5 etc.) from
> > >>> automatic generated event name. This fixes wildcard event
> > >>> adding like below case;
> > >>>
> > >>>   =====
> > >>>   # perf probe -x /lib64/libc-2.25.so malloc*
> > >>>   Internal error: "malloc_get_state@GLIBC_2" is wrong event name.
> > >>>     Error: Failed to add events.
> > >>>   =====
> > >>>
> > >>> This failure was caused by a versioned suffix symbol.
> > >>> With this fix, perf probe automatically cuts the
> > >>> suffix after @ as below.
> > >>>
> > >>>   =====
> > >>>   # ./perf probe -x /lib64/libc-2.25.so malloc*
> > >>>   Added new events:
> > >>>     probe_libc:malloc_printerr (on malloc* in /usr/lib64/libc-2.25.so)
> > >>>     probe_libc:malloc_consolidate (on malloc* in /usr/lib64/libc-2.25.so)
> > >>>     probe_libc:malloc_check (on malloc* in /usr/lib64/libc-2.25.so)
> > >>>     probe_libc:malloc_hook_ini (on malloc* in /usr/lib64/libc-2.25.so)
> > >>>     probe_libc:malloc    (on malloc* in /usr/lib64/libc-2.25.so)
> > >>>     probe_libc:malloc_trim (on malloc* in /usr/lib64/libc-2.25.so)
> > >>>     probe_libc:malloc_usable_size (on malloc* in /usr/lib64/libc-2.25.so)
> > >>>     probe_libc:malloc_stats (on malloc* in /usr/lib64/libc-2.25.so)
> > >>>     probe_libc:malloc_info (on malloc* in /usr/lib64/libc-2.25.so)
> > >>>     probe_libc:mallochook (on malloc* in /usr/lib64/libc-2.25.so)
> > >>>     probe_libc:malloc_get_state (on malloc* in /usr/lib64/libc-2.25.so)
> > >>>     probe_libc:malloc_set_state (on malloc* in /usr/lib64/libc-2.25.so)
> > >>>
> > >>>   You can now use it in all perf tools, such as:
> > >>>
> > >>> 	  perf record -e probe_libc:malloc_set_state -aR sleep 1
> > >>>
> > >>>   =====
> > >>
> > >> I think this will still fall over for a case where there are multiple versions of the same base symbol, like:
> > >>
> > >> $ nm /lib64/libc.so.6 | egrep ' sched_getaffinity'
> > >> 0000000000134430 T sched_getaffinity@GLIBC_2.3.3
> > >> 00000000000dcf00 T sched_getaffinity@@GLIBC_2.3.4
> > > 
> > > No, in that case perf probe adds number suffix (_1, _2...) for new events :)
> > > 
> > > This feature (number suffix) is anyway required for the case that inlined
> > > function is spreaded in multiple places. I think this is natural for perf probe.
> > 
> > I can live with that.
> > 
> > Is the versioning tag retained for display with "perf probe -l" display, so the probe points can be distinguished?
> > 
> > In other words, will one see:
> > # perf probe -l
> >   probe_libc:sched_getaffinity (on sched_getaffinity@GLIBC_2.3.3 in /usr/lib64/libc-2.17.so)
> >   probe_libc:sched_getaffinity_1 (on sched_getaffinity@@GLIBC_2.3.4 in /usr/lib64/libc-2.17.so)
> > or
> > # perf probe -l
> >   probe_libc:sched_getaffinity (on sched_getaffinity in /usr/lib64/libc-2.17.so)
> >   probe_libc:sched_getaffinity_1 (on sched_getaffinity in /usr/lib64/libc-2.17.so)
> 
> [root@jouet ~]# perf probe -x /lib64/libc-2.25.so malloc*
> Added new events:
>   probe_libc:malloc_printerr (on malloc* in /usr/lib64/libc-2.25.so)
>   probe_libc:malloc_consolidate (on malloc* in /usr/lib64/libc-2.25.so)
>   probe_libc:malloc_check (on malloc* in /usr/lib64/libc-2.25.so)
>   probe_libc:malloc_hook_ini (on malloc* in /usr/lib64/libc-2.25.so)
>   probe_libc:malloc    (on malloc* in /usr/lib64/libc-2.25.so)
>   probe_libc:malloc_trim (on malloc* in /usr/lib64/libc-2.25.so)
>   probe_libc:malloc_usable_size (on malloc* in /usr/lib64/libc-2.25.so)
>   probe_libc:malloc_stats (on malloc* in /usr/lib64/libc-2.25.so)
>   probe_libc:malloc_info (on malloc* in /usr/lib64/libc-2.25.so)
>   probe_libc:mallochook (on malloc* in /usr/lib64/libc-2.25.so)
>   probe_libc:malloc_get_state (on malloc* in /usr/lib64/libc-2.25.so)
>   probe_libc:malloc_set_state (on malloc* in /usr/lib64/libc-2.25.so)
> 
> You can now use it in all perf tools, such as:
> 
> 	perf record -e probe_libc:malloc_set_state -aR sleep 1
> 
> [root@jouet ~]# perf probe -l
>   probe_libc:malloc    (on 0xd22a6973 in /usr/lib64/libc-2.25.so)
>   probe_libc:malloc_check (on 0x25db6f5a in /usr/lib64/libc-2.25.so)
>   probe_libc:malloc_consolidate (on 0x88f7c44a in /usr/lib64/libc-2.25.so)
>   probe_libc:malloc_get_state (on 0xdd38eca7 in /usr/lib64/libc-2.25.so)
>   probe_libc:malloc_hook_ini (on 0x8de2b533 in /usr/lib64/libc-2.25.so)
>   probe_libc:malloc_info (on 0x8b327bde in /usr/lib64/libc-2.25.so)
>   probe_libc:malloc_printerr (on 0x8fff087c in /usr/lib64/libc-2.25.so)
>   probe_libc:malloc_set_state (on 0x861940e6 in /usr/lib64/libc-2.25.so)
>   probe_libc:malloc_stats (on 0x51fa3cdc in /usr/lib64/libc-2.25.so)
>   probe_libc:malloc_trim (on 0xa9fa4e43 in /usr/lib64/libc-2.25.so)
>   probe_libc:malloc_usable_size (on 0xceb5e090 in /usr/lib64/libc-2.25.so)
>   probe_libc:mallochook (on 0x6ebc00f5 in /usr/lib64/libc-2.25.so)

This looks something wrong. perf-probe should try to get symbol
at least. What happened on that ? Could you run perf probe -l with -vv ?

On my Fedora26, 

$ sudo ./perf probe -l
  probe_libc:malloc    (on __malloc in /usr/lib64/libc-2.25.so)
  probe_libc:malloc_check (on malloc_check in /usr/lib64/libc-2.25.so)
  probe_libc:malloc_consolidate (on malloc_consolidate.part.1 in /usr/lib64/libc-2.25.so)
  probe_libc:malloc_get_state (on malloc_get_state@GLIBC_2.2.5 in /usr/lib64/libc-2.25.so)
  probe_libc:malloc_hook_ini (on malloc_hook_ini in /usr/lib64/libc-2.25.so)
  probe_libc:malloc_info (on __malloc_info in /usr/lib64/libc-2.25.so)
  probe_libc:malloc_printerr (on malloc_printerr in /usr/lib64/libc-2.25.so)
  probe_libc:malloc_set_state (on malloc_set_state@GLIBC_2.2.5 in /usr/lib64/libc-2.25.so)
  probe_libc:malloc_stats (on __malloc_stats in /usr/lib64/libc-2.25.so)
  probe_libc:malloc_trim (on __malloc_trim in /usr/lib64/libc-2.25.so)
  probe_libc:malloc_usable_size (on __malloc_usable_size in /usr/lib64/libc-2.25.so)
  probe_libc:mallochook (on mallochook in /usr/lib64/libc-2.25.so)

I can see the symbols with version suffixes.


> [root@jouet ~]#
> 
> [acme@jouet perf]$ nm /lib64/libc-2.25.so | grep malloc_get_state
> 0000000000151ba0 T malloc_get_state@GLIBC_2.2.5
> [acme@jouet perf]$
> 
> For your example it finds two symbols, adds two but I couldn't find a
> simple way of mapping one to the other, would be nice if 'perf probe -l'
> showed them like 'nm' does.
> 
> [root@jouet ~]# nm /lib64/libc-2.25.so | grep ' sched_getaffinity'
> 0000000000151cd0 T sched_getaffinity@GLIBC_2.3.3
> 00000000000f3c10 T sched_getaffinity@@GLIBC_2.3.4
> [root@jouet ~]#
> 
> [root@jouet ~]# perf probe -x /lib64/libc-2.25.so sched_getaffinity*
> Added new events:
>   probe_libc:sched_getaffinity (on sched_getaffinity* in /usr/lib64/libc-2.25.so)
>   probe_libc:sched_getaffinity_1 (on sched_getaffinity* in /usr/lib64/libc-2.25.so)
> 
> You can now use it in all perf tools, such as:
> 
> 	perf record -e probe_libc:sched_getaffinity_1 -aR sleep 1
> 
> [root@jouet ~]# perf probe -l
>   probe_libc:sched_getaffinity (on 0x5fd95e7c in /usr/lib64/libc-2.25.so)
>   probe_libc:sched_getaffinity_1 (on 0xd8d51601 in /usr/lib64/libc-2.25.so)
> [root@jouet ~]#

This case also shows

$ sudo ./perf probe -l
  probe_libc:sched_getaffinity (on __sched_getaffinity_new in /usr/lib64/libc-2.25.so)
  probe_libc:sched_getaffinity_1 (on sched_getaffinity@GLIBC_2.3.3 in /usr/lib64/libc-2.25.so)

(but the first one seems a bit different, maybe it is an alias of sched_getaffinity@<some new version>.)

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH v2 2/5] perf-probe: Cut off the version suffix from event name

[Arnaldo Carvalho de Melo]

Em Thu, Dec 07, 2017 at 04:20:28PM +0900, Masami Hiramatsu escreveu:
> Cut off the version suffix (e.g. @GLIBC_2.2.5 etc.) from
> automatic generated event name. This fixes wildcard event
> adding like below case;
> 
>   =====
>   # perf probe -x /lib64/libc-2.25.so malloc*
>   Internal error: "malloc_get_state@GLIBC_2" is wrong event name.
>     Error: Failed to add events.
>   =====
> 
> This failure was caused by a versioned suffix symbol.
> With this fix, perf probe automatically cuts the
> suffix after @ as below.
> 
>   =====
>   # ./perf probe -x /lib64/libc-2.25.so malloc*
>   Added new events:
>     probe_libc:malloc_printerr (on malloc* in /usr/lib64/libc-2.25.so)
>     probe_libc:malloc_consolidate (on malloc* in /usr/lib64/libc-2.25.so)
>     probe_libc:malloc_check (on malloc* in /usr/lib64/libc-2.25.so)
>     probe_libc:malloc_hook_ini (on malloc* in /usr/lib64/libc-2.25.so)
>     probe_libc:malloc    (on malloc* in /usr/lib64/libc-2.25.so)
>     probe_libc:malloc_trim (on malloc* in /usr/lib64/libc-2.25.so)
>     probe_libc:malloc_usable_size (on malloc* in /usr/lib64/libc-2.25.so)
>     probe_libc:malloc_stats (on malloc* in /usr/lib64/libc-2.25.so)
>     probe_libc:malloc_info (on malloc* in /usr/lib64/libc-2.25.so)
>     probe_libc:mallochook (on malloc* in /usr/lib64/libc-2.25.so)
>     probe_libc:malloc_get_state (on malloc* in /usr/lib64/libc-2.25.so)
>     probe_libc:malloc_set_state (on malloc* in /usr/lib64/libc-2.25.so)

Thanks for working on this! I'm now going over it, and one thing I
noticed was that the (on malloc*) on all the entries matched by that
wildcard, can I suggest that you expand it there as well? I.e.

     probe_libc:malloc_set_state (on malloc_set_state in /usr/lib64/libc-2.25.so)

This way we'll now when aliases are used, and also for the versioning
case, i.e. to which version is a probe pointing?

See also Paul Clarke's question and suggestion, which I agree, i.e.
instead of chopping off the version, just replace the chars with valid
ones or better, do what Paul suggests, be more flexible in interpreting
@, i.e. if it is a number and/or fails to point to any file, interpret
it as versioning.

- Arnaldo

>   You can now use it in all perf tools, such as:
> 
> 	  perf record -e probe_libc:malloc_set_state -aR sleep 1
> 
>   =====
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
> Reported-by: bhargavb <bhargavaramudu@gmail.com>
> ---
>  tools/perf/util/probe-event.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index c0067950e56f..74435fb7ab7f 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -2584,8 +2584,8 @@ static int get_new_event_name(char *buf, size_t len, const char *base,
>  	if (!nbase)
>  		return -ENOMEM;
>  
> -	/* Cut off the dot suffixes (e.g. .const, .isra)*/
> -	p = strchr(nbase, '.');
> +	/* Cut off the dot suffixes (e.g. .const, .isra) and version suffixes */
> +	p = strpbrk(nbase, ".@");
>  	if (p && p != nbase)
>  		*p = '\0';
>  

Re: [PATCH v2 2/5] perf-probe: Cut off the version suffix from event name

[Paul Clarke]

On 12/07/2017 10:56 AM, Arnaldo Carvalho de Melo wrote:
> Em Thu, Dec 07, 2017 at 04:20:28PM +0900, Masami Hiramatsu escreveu:
>> Cut off the version suffix (e.g. @GLIBC_2.2.5 etc.) from
>> automatic generated event name. This fixes wildcard event
>> adding like below case;
>>
>>   =====
>>   # perf probe -x /lib64/libc-2.25.so malloc*
>>   Internal error: "malloc_get_state@GLIBC_2" is wrong event name.
>>     Error: Failed to add events.
>>   =====
>>
>> This failure was caused by a versioned suffix symbol.
>> With this fix, perf probe automatically cuts the
>> suffix after @ as below.
>>
>>   =====
>>   # ./perf probe -x /lib64/libc-2.25.so malloc*
>>   Added new events:
>>     probe_libc:malloc_printerr (on malloc* in /usr/lib64/libc-2.25.so)
>>     probe_libc:malloc_consolidate (on malloc* in /usr/lib64/libc-2.25.so)
>>     probe_libc:malloc_check (on malloc* in /usr/lib64/libc-2.25.so)
>>     probe_libc:malloc_hook_ini (on malloc* in /usr/lib64/libc-2.25.so)
>>     probe_libc:malloc    (on malloc* in /usr/lib64/libc-2.25.so)
>>     probe_libc:malloc_trim (on malloc* in /usr/lib64/libc-2.25.so)
>>     probe_libc:malloc_usable_size (on malloc* in /usr/lib64/libc-2.25.so)
>>     probe_libc:malloc_stats (on malloc* in /usr/lib64/libc-2.25.so)
>>     probe_libc:malloc_info (on malloc* in /usr/lib64/libc-2.25.so)
>>     probe_libc:mallochook (on malloc* in /usr/lib64/libc-2.25.so)
>>     probe_libc:malloc_get_state (on malloc* in /usr/lib64/libc-2.25.so)
>>     probe_libc:malloc_set_state (on malloc* in /usr/lib64/libc-2.25.so)
> 
> Thanks for working on this! I'm now going over it, and one thing I
> noticed was that the (on malloc*) on all the entries matched by that
> wildcard, can I suggest that you expand it there as well? I.e.
> 
>      probe_libc:malloc_set_state (on malloc_set_state in /usr/lib64/libc-2.25.so)
> 
> This way we'll now when aliases are used, and also for the versioning
> case, i.e. to which version is a probe pointing?
> 
> See also Paul Clarke's question and suggestion, which I agree, i.e.
> instead of chopping off the version, just replace the chars with valid
> ones or better, do what Paul suggests, be more flexible in interpreting
> @, i.e. if it is a number and/or fails to point to any file, interpret
> it as versioning.

It's a nit, and subjective, but I'd favor checking for versioning first, then file.  The namespaces are very unlikely to intersect, but I could foresee symbols like "sym@implA.c" and "sym@implB.c" more likely than a symbol in a file "GLIBC_2.2.5".

Perhaps straying toward bikeshedding...

PC

Re: [PATCH v2 2/5] perf-probe: Cut off the version suffix from event name

[Arnaldo Carvalho de Melo]

Em Thu, Dec 07, 2017 at 11:24:47AM -0600, Paul Clarke escreveu:
> On 12/07/2017 10:56 AM, Arnaldo Carvalho de Melo wrote:
> > Em Thu, Dec 07, 2017 at 04:20:28PM +0900, Masami Hiramatsu escreveu:
> > See also Paul Clarke's question and suggestion, which I agree, i.e.
> > instead of chopping off the version, just replace the chars with valid
> > ones or better, do what Paul suggests, be more flexible in interpreting
> > @, i.e. if it is a number and/or fails to point to any file, interpret
> > it as versioning.
 
> It's a nit, and subjective, but I'd favor checking for versioning
> first, then file.  The namespaces are very unlikely to intersect, but
> I could foresee symbols like "sym@implA.c" and "sym@implB.c" more
> likely than a symbol in a file "GLIBC_2.2.5".
 
> Perhaps straying toward bikeshedding...

Nah, those are valid concerns, and I share them.

- Arnaldo

Re: [PATCH v2 2/5] perf-probe: Cut off the version suffix from event name

[Masami Hiramatsu]

On Thu, 7 Dec 2017 13:56:59 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Thu, Dec 07, 2017 at 04:20:28PM +0900, Masami Hiramatsu escreveu:
> > Cut off the version suffix (e.g. @GLIBC_2.2.5 etc.) from
> > automatic generated event name. This fixes wildcard event
> > adding like below case;
> > 
> >   =====
> >   # perf probe -x /lib64/libc-2.25.so malloc*
> >   Internal error: "malloc_get_state@GLIBC_2" is wrong event name.
> >     Error: Failed to add events.
> >   =====
> > 
> > This failure was caused by a versioned suffix symbol.
> > With this fix, perf probe automatically cuts the
> > suffix after @ as below.
> > 
> >   =====
> >   # ./perf probe -x /lib64/libc-2.25.so malloc*
> >   Added new events:
> >     probe_libc:malloc_printerr (on malloc* in /usr/lib64/libc-2.25.so)
> >     probe_libc:malloc_consolidate (on malloc* in /usr/lib64/libc-2.25.so)
> >     probe_libc:malloc_check (on malloc* in /usr/lib64/libc-2.25.so)
> >     probe_libc:malloc_hook_ini (on malloc* in /usr/lib64/libc-2.25.so)
> >     probe_libc:malloc    (on malloc* in /usr/lib64/libc-2.25.so)
> >     probe_libc:malloc_trim (on malloc* in /usr/lib64/libc-2.25.so)
> >     probe_libc:malloc_usable_size (on malloc* in /usr/lib64/libc-2.25.so)
> >     probe_libc:malloc_stats (on malloc* in /usr/lib64/libc-2.25.so)
> >     probe_libc:malloc_info (on malloc* in /usr/lib64/libc-2.25.so)
> >     probe_libc:mallochook (on malloc* in /usr/lib64/libc-2.25.so)
> >     probe_libc:malloc_get_state (on malloc* in /usr/lib64/libc-2.25.so)
> >     probe_libc:malloc_set_state (on malloc* in /usr/lib64/libc-2.25.so)
> 
> Thanks for working on this! I'm now going over it, and one thing I
> noticed was that the (on malloc*) on all the entries matched by that
> wildcard, can I suggest that you expand it there as well? I.e.
> 
>      probe_libc:malloc_set_state (on malloc_set_state in /usr/lib64/libc-2.25.so)
> 

Hmm, the reason why I chose it was perf probe accepted multiple
probe points at once. e.g.

./perf probe -x /lib64/libc-2.25.so -a malloc* -a free* 

So, if we specify several probe points, I guess user will like to
see which events come from which one. 


> This way we'll now when aliases are used, and also for the versioning
> case, i.e. to which version is a probe pointing?

Currently we can know that by perf probe -l. I would recommend this way,
since -l can show you more precise information (and if it take a time
to read/analyze debuginfo, user can understand the reason why).

Thank you,

> 
> See also Paul Clarke's question and suggestion, which I agree, i.e.
> instead of chopping off the version, just replace the chars with valid
> ones or better, do what Paul suggests, be more flexible in interpreting
> @, i.e. if it is a number and/or fails to point to any file, interpret
> it as versioning.
> 
> - Arnaldo
> 
> >   You can now use it in all perf tools, such as:
> > 
> > 	  perf record -e probe_libc:malloc_set_state -aR sleep 1
> > 
> >   =====
> > 
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
> > Reported-by: bhargavb <bhargavaramudu@gmail.com>
> > ---
> >  tools/perf/util/probe-event.c |    4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > index c0067950e56f..74435fb7ab7f 100644
> > --- a/tools/perf/util/probe-event.c
> > +++ b/tools/perf/util/probe-event.c
> > @@ -2584,8 +2584,8 @@ static int get_new_event_name(char *buf, size_t len, const char *base,
> >  	if (!nbase)
> >  		return -ENOMEM;
> >  
> > -	/* Cut off the dot suffixes (e.g. .const, .isra)*/
> > -	p = strchr(nbase, '.');
> > +	/* Cut off the dot suffixes (e.g. .const, .isra) and version suffixes */
> > +	p = strpbrk(nbase, ".@");
> >  	if (p && p != nbase)
> >  		*p = '\0';
> >  


-- 
Masami Hiramatsu <mhiramat@kernel.org>

[PATCH v2 3/5] perf-probe: Add __return suffix for return events

[Masami Hiramatsu]

Add __return suffix for function return events
automatically. Without this, user have to give --force
option and will see the number suffix for each event
like "function_1", which is not easy to recognize.
Instead, this adds __return suffix to it automatically.
E.g.

  =====
  # ./perf probe -x /lib64/libc-2.25.so 'malloc*%return'
  Added new events:
    probe_libc:malloc_printerr__return (on malloc*%return in /usr/lib64/libc-2.25.so)
    probe_libc:malloc_consolidate__return (on malloc*%return in /usr/lib64/libc-2.25.so)
    probe_libc:malloc_check__return (on malloc*%return in /usr/lib64/libc-2.25.so)
    probe_libc:malloc_hook_ini__return (on malloc*%return in /usr/lib64/libc-2.25.so)
    probe_libc:malloc__return (on malloc*%return in /usr/lib64/libc-2.25.so)
    probe_libc:malloc_trim__return (on malloc*%return in /usr/lib64/libc-2.25.so)
    probe_libc:malloc_usable_size__return (on malloc*%return in /usr/lib64/libc-2.25.so)
    probe_libc:malloc_stats__return (on malloc*%return in /usr/lib64/libc-2.25.so)
    probe_libc:malloc_info__return (on malloc*%return in /usr/lib64/libc-2.25.so)
    probe_libc:mallochook__return (on malloc*%return in /usr/lib64/libc-2.25.so)
    probe_libc:malloc_get_state__return (on malloc*%return in /usr/lib64/libc-2.25.so)
    probe_libc:malloc_set_state__return (on malloc*%return in /usr/lib64/libc-2.25.so)

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

	  perf record -e probe_libc:malloc_set_state__return -aR sleep 1

  =====

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
---
  Changes in v2:
   - Add a description in Documentation/perf-probe.txt.
---
 tools/perf/Documentation/perf-probe.txt |    2 +-
 tools/perf/util/probe-event.c           |    9 +++++----
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/tools/perf/Documentation/perf-probe.txt b/tools/perf/Documentation/perf-probe.txt
index d7e4869905f1..f96382692f42 100644
--- a/tools/perf/Documentation/perf-probe.txt
+++ b/tools/perf/Documentation/perf-probe.txt
@@ -170,7 +170,7 @@ Probe points are defined by following syntax.
      or,
      sdt_PROVIDER:SDTEVENT
 
-'EVENT' specifies the name of new event, if omitted, it will be set the name of the probed function. You can also specify a group name by 'GROUP', if omitted, set 'probe' is used for kprobe and 'probe_<bin>' is used for uprobe.
+'EVENT' specifies the name of new event, if omitted, it will be set the name of the probed function, and for return probes, a "\_\_return" suffix is automatically added to the function name. You can also specify a group name by 'GROUP', if omitted, set 'probe' is used for kprobe and 'probe_<bin>' is used for uprobe.
 Note that using existing group name can conflict with other events. Especially, using the group name reserved for kernel modules can hide embedded events in the
 modules.
 'FUNC' specifies a probed function name, and it may have one of the following options; '+OFFS' is the offset from function entry address in bytes, ':RLN' is the relative-line number from function entry line, and '%return' means that it probes function return. And ';PTN' means lazy matching pattern (see LAZY MATCHING). Note that ';PTN' must be the end of the probe point definition.  In addition, '@SRC' specifies a source file which has that function.
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 74435fb7ab7f..959c4d2ef455 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2573,7 +2573,8 @@ int show_perf_probe_events(struct strfilter *filter)
 }
 
 static int get_new_event_name(char *buf, size_t len, const char *base,
-			      struct strlist *namelist, bool allow_suffix)
+			      struct strlist *namelist, bool ret_event,
+			      bool allow_suffix)
 {
 	int i, ret;
 	char *p, *nbase;
@@ -2590,7 +2591,7 @@ static int get_new_event_name(char *buf, size_t len, const char *base,
 		*p = '\0';
 
 	/* Try no suffix number */
-	ret = e_snprintf(buf, len, "%s", nbase);
+	ret = e_snprintf(buf, len, "%s%s", nbase, ret_event ? "__return" : "");
 	if (ret < 0) {
 		pr_debug("snprintf() failed: %d\n", ret);
 		goto out;
@@ -2689,8 +2690,8 @@ static int probe_trace_event__set_name(struct probe_trace_event *tev,
 		group = PERFPROBE_GROUP;
 
 	/* Get an unused new event name */
-	ret = get_new_event_name(buf, 64, event,
-				 namelist, allow_suffix);
+	ret = get_new_event_name(buf, 64, event, namelist,
+				 tev->point.retprobe, allow_suffix);
 	if (ret < 0)
 		return ret;
 

[PATCH v2 4/5] perf-probe: Find versioned symbols from map

[Masami Hiramatsu]

Find versioned symbols correctly from map.
Commit d80406453ad4 ("perf symbols: Allow user probes on
versioned symbols") allows user to find default versioned
symbols (with "@@") in map. However, it did not enable
normal versioned symbol (with "@") for perf-probe.
E.g.

  =====
  # ./perf probe -x /lib64/libc-2.25.so malloc_get_state
  Failed to find symbol malloc_get_state in /usr/lib64/libc-2.25.so
    Error: Failed to add events.
  =====

This solves above issue by improving perf-probe symbol
search function, as below.

  =====
  # ./perf probe -x /lib64/libc-2.25.so malloc_get_state
  Added new event:
    probe_libc:malloc_get_state (on malloc_get_state in /usr/lib64/libc-2.25.so)

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

	  perf record -e probe_libc:malloc_get_state -aR sleep 1

  # ./perf probe -l
    probe_libc:malloc_get_state (on malloc_get_state@GLIBC_2.2.5 in /usr/lib64/libc-2.25.so)
  =====

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 tools/perf/arch/powerpc/util/sym-handling.c |    8 ++++++++
 tools/perf/util/probe-event.c               |   16 +++++++++++++++-
 tools/perf/util/symbol.c                    |    5 +++++
 tools/perf/util/symbol.h                    |    1 +
 4 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/tools/perf/arch/powerpc/util/sym-handling.c b/tools/perf/arch/powerpc/util/sym-handling.c
index 9c4e23d8c8ce..a3613c8d97b6 100644
--- a/tools/perf/arch/powerpc/util/sym-handling.c
+++ b/tools/perf/arch/powerpc/util/sym-handling.c
@@ -64,6 +64,14 @@ int arch__compare_symbol_names_n(const char *namea, const char *nameb,
 
 	return strncmp(namea, nameb, n);
 }
+
+const char *arch__normalize_symbol_name(const char *name)
+{
+	/* Skip over initial dot */
+	if (*name == '.')
+		name++;
+	return name;
+}
 #endif
 
 #if defined(_CALL_ELF) && _CALL_ELF == 2
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 959c4d2ef455..94acc5846e2a 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2801,16 +2801,30 @@ static int find_probe_functions(struct map *map, char *name,
 	int found = 0;
 	struct symbol *sym;
 	struct rb_node *tmp;
+	const char *norm, *ver;
+	char *buf = NULL;
 
 	if (map__load(map) < 0)
 		return 0;
 
 	map__for_each_symbol(map, sym, tmp) {
-		if (strglobmatch(sym->name, name)) {
+		norm = arch__normalize_symbol_name(sym->name);
+		if (!norm)
+			continue;
+
+		/* We don't care about default symbol or not */
+		ver = strchr(norm, '@');
+		if (ver) {
+			buf = strndup(norm, ver - norm);
+			norm = buf;
+		}
+		if (strglobmatch(norm, name)) {
 			found++;
 			if (syms && found < probe_conf.max_probes)
 				syms[found - 1] = sym;
 		}
+		if (buf)
+			zfree(&buf);
 	}
 
 	return found;
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 1b67a8639dfe..cc065d4bfafc 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -94,6 +94,11 @@ static int prefix_underscores_count(const char *str)
 	return tail - str;
 }
 
+const char * __weak arch__normalize_symbol_name(const char *name)
+{
+	return name;
+}
+
 int __weak arch__compare_symbol_names(const char *namea, const char *nameb)
 {
 	return strcmp(namea, nameb);
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index a4f0075b4e5c..0563f33c1eb3 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -349,6 +349,7 @@ bool elf__needs_adjust_symbols(GElf_Ehdr ehdr);
 void arch__sym_update(struct symbol *s, GElf_Sym *sym);
 #endif
 
+const char *arch__normalize_symbol_name(const char *name);
 #define SYMBOL_A 0
 #define SYMBOL_B 1
 

Re: [PATCH v2 4/5] perf-probe: Find versioned symbols from map

[Thomas-Mich Richter]

On 12/07/2017 08:21 AM, Masami Hiramatsu wrote:
> Find versioned symbols correctly from map.
> Commit d80406453ad4 ("perf symbols: Allow user probes on
> versioned symbols") allows user to find default versioned
> symbols (with "@@") in map. However, it did not enable
> normal versioned symbol (with "@") for perf-probe.
> E.g.
> 
>   =====
>   # ./perf probe -x /lib64/libc-2.25.so malloc_get_state
>   Failed to find symbol malloc_get_state in /usr/lib64/libc-2.25.so
>     Error: Failed to add events.
>   =====
> 
> This solves above issue by improving perf-probe symbol
> search function, as below.
> 
>   =====
>   # ./perf probe -x /lib64/libc-2.25.so malloc_get_state
>   Added new event:
>     probe_libc:malloc_get_state (on malloc_get_state in /usr/lib64/libc-2.25.so)
> 
>   You can now use it in all perf tools, such as:
> 
> 	  perf record -e probe_libc:malloc_get_state -aR sleep 1
> 
>   # ./perf probe -l
>     probe_libc:malloc_get_state (on malloc_get_state@GLIBC_2.2.5 in /usr/lib64/libc-2.25.so)
>   =====
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  tools/perf/arch/powerpc/util/sym-handling.c |    8 ++++++++
>  tools/perf/util/probe-event.c               |   16 +++++++++++++++-
>  tools/perf/util/symbol.c                    |    5 +++++
>  tools/perf/util/symbol.h                    |    1 +
>  4 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/arch/powerpc/util/sym-handling.c b/tools/perf/arch/powerpc/util/sym-handling.c
> index 9c4e23d8c8ce..a3613c8d97b6 100644
> --- a/tools/perf/arch/powerpc/util/sym-handling.c
> +++ b/tools/perf/arch/powerpc/util/sym-handling.c
> @@ -64,6 +64,14 @@ int arch__compare_symbol_names_n(const char *namea, const char *nameb,
> 
>  	return strncmp(namea, nameb, n);
>  }
> +
> +const char *arch__normalize_symbol_name(const char *name)
> +{
> +	/* Skip over initial dot */
> +	if (*name == '.')
> +		name++;
> +	return name;
> +}
>  #endif
> 
>  #if defined(_CALL_ELF) && _CALL_ELF == 2
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 959c4d2ef455..94acc5846e2a 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -2801,16 +2801,30 @@ static int find_probe_functions(struct map *map, char *name,
>  	int found = 0;
>  	struct symbol *sym;
>  	struct rb_node *tmp;
> +	const char *norm, *ver;
> +	char *buf = NULL;
> 
>  	if (map__load(map) < 0)
>  		return 0;
> 
>  	map__for_each_symbol(map, sym, tmp) {
> -		if (strglobmatch(sym->name, name)) {
> +		norm = arch__normalize_symbol_name(sym->name);

The weak default function arch__normalize_symbol_name() simply returns
its parameter, so norm can be a NULL ptr when parameter sym->name is a 
NULL pointer.
However when sym->name is a NULL pointer (or can be one) then the
Powerpc version of arch__normalize_symbol_name() dereferences a NULL
ptr (by checking the symbol's first character is a '.').

> +		if (!norm)
> +			continue;
> +
> +		/* We don't care about default symbol or not */
> +		ver = strchr(norm, '@');
> +		if (ver) {
> +			buf = strndup(norm, ver - norm);
> +			norm = buf;

if strndup() returns a NULL pointer (due to lack of memory)
then variable norm is a NULL ptr and strglobmatch() --> __match_glob()
derefenences that NULL pointer (1. parameter).

> +		}
> +		if (strglobmatch(norm, name)) {
>  			found++;
>  			if (syms && found < probe_conf.max_probes)
>  				syms[found - 1] = sym;
>  		}
> +		if (buf)
> +			zfree(&buf);
>  	}
> 
>  	return found;
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 1b67a8639dfe..cc065d4bfafc 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -94,6 +94,11 @@ static int prefix_underscores_count(const char *str)
>  	return tail - str;
>  }
> 
> +const char * __weak arch__normalize_symbol_name(const char *name)
> +{
> +	return name;
> +}
> +
>  int __weak arch__compare_symbol_names(const char *namea, const char *nameb)
>  {
>  	return strcmp(namea, nameb);
> diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> index a4f0075b4e5c..0563f33c1eb3 100644
> --- a/tools/perf/util/symbol.h
> +++ b/tools/perf/util/symbol.h
> @@ -349,6 +349,7 @@ bool elf__needs_adjust_symbols(GElf_Ehdr ehdr);
>  void arch__sym_update(struct symbol *s, GElf_Sym *sym);
>  #endif
> 
> +const char *arch__normalize_symbol_name(const char *name);
>  #define SYMBOL_A 0
>  #define SYMBOL_B 1
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-perf-users" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


-- 
Thomas Richter, Dept 3303, IBM LTC Boeblingen Germany
--
Vorsitzende des Aufsichtsrats: Martina Koederitz 
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294

Re: [PATCH v2 4/5] perf-probe: Find versioned symbols from map

[Masami Hiramatsu]

On Fri, 8 Dec 2017 12:08:41 +0100
Thomas-Mich Richter <tmricht@linux.vnet.ibm.com> wrote:

> On 12/07/2017 08:21 AM, Masami Hiramatsu wrote:
> > Find versioned symbols correctly from map.
> > Commit d80406453ad4 ("perf symbols: Allow user probes on
> > versioned symbols") allows user to find default versioned
> > symbols (with "@@") in map. However, it did not enable
> > normal versioned symbol (with "@") for perf-probe.
> > E.g.
> > 
> >   =====
> >   # ./perf probe -x /lib64/libc-2.25.so malloc_get_state
> >   Failed to find symbol malloc_get_state in /usr/lib64/libc-2.25.so
> >     Error: Failed to add events.
> >   =====
> > 
> > This solves above issue by improving perf-probe symbol
> > search function, as below.
> > 
> >   =====
> >   # ./perf probe -x /lib64/libc-2.25.so malloc_get_state
> >   Added new event:
> >     probe_libc:malloc_get_state (on malloc_get_state in /usr/lib64/libc-2.25.so)
> > 
> >   You can now use it in all perf tools, such as:
> > 
> > 	  perf record -e probe_libc:malloc_get_state -aR sleep 1
> > 
> >   # ./perf probe -l
> >     probe_libc:malloc_get_state (on malloc_get_state@GLIBC_2.2.5 in /usr/lib64/libc-2.25.so)
> >   =====
> > 
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > ---
> >  tools/perf/arch/powerpc/util/sym-handling.c |    8 ++++++++
> >  tools/perf/util/probe-event.c               |   16 +++++++++++++++-
> >  tools/perf/util/symbol.c                    |    5 +++++
> >  tools/perf/util/symbol.h                    |    1 +
> >  4 files changed, 29 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/perf/arch/powerpc/util/sym-handling.c b/tools/perf/arch/powerpc/util/sym-handling.c
> > index 9c4e23d8c8ce..a3613c8d97b6 100644
> > --- a/tools/perf/arch/powerpc/util/sym-handling.c
> > +++ b/tools/perf/arch/powerpc/util/sym-handling.c
> > @@ -64,6 +64,14 @@ int arch__compare_symbol_names_n(const char *namea, const char *nameb,
> > 
> >  	return strncmp(namea, nameb, n);
> >  }
> > +
> > +const char *arch__normalize_symbol_name(const char *name)
> > +{
> > +	/* Skip over initial dot */
> > +	if (*name == '.')
> > +		name++;
> > +	return name;
> > +}
> >  #endif
> > 
> >  #if defined(_CALL_ELF) && _CALL_ELF == 2
> > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > index 959c4d2ef455..94acc5846e2a 100644
> > --- a/tools/perf/util/probe-event.c
> > +++ b/tools/perf/util/probe-event.c
> > @@ -2801,16 +2801,30 @@ static int find_probe_functions(struct map *map, char *name,
> >  	int found = 0;
> >  	struct symbol *sym;
> >  	struct rb_node *tmp;
> > +	const char *norm, *ver;
> > +	char *buf = NULL;
> > 
> >  	if (map__load(map) < 0)
> >  		return 0;
> > 
> >  	map__for_each_symbol(map, sym, tmp) {
> > -		if (strglobmatch(sym->name, name)) {
> > +		norm = arch__normalize_symbol_name(sym->name);
> 
> The weak default function arch__normalize_symbol_name() simply returns
> its parameter, so norm can be a NULL ptr when parameter sym->name is a 
> NULL pointer.
> However when sym->name is a NULL pointer (or can be one) then the
> Powerpc version of arch__normalize_symbol_name() dereferences a NULL
> ptr (by checking the symbol's first character is a '.').

Oops, right!
Thank you for pointing out. :)

> 
> > +		if (!norm)
> > +			continue;
> > +
> > +		/* We don't care about default symbol or not */
> > +		ver = strchr(norm, '@');
> > +		if (ver) {
> > +			buf = strndup(norm, ver - norm);
> > +			norm = buf;
> 
> if strndup() returns a NULL pointer (due to lack of memory)
> then variable norm is a NULL ptr and strglobmatch() --> __match_glob()
> derefenences that NULL pointer (1. parameter).

Ah, right. easy miss...

OK, I'll update.

Thank you,

> 
> > +		}
> > +		if (strglobmatch(norm, name)) {
> >  			found++;
> >  			if (syms && found < probe_conf.max_probes)
> >  				syms[found - 1] = sym;
> >  		}
> > +		if (buf)
> > +			zfree(&buf);
> >  	}
> > 
> >  	return found;
> > diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> > index 1b67a8639dfe..cc065d4bfafc 100644
> > --- a/tools/perf/util/symbol.c
> > +++ b/tools/perf/util/symbol.c
> > @@ -94,6 +94,11 @@ static int prefix_underscores_count(const char *str)
> >  	return tail - str;
> >  }
> > 
> > +const char * __weak arch__normalize_symbol_name(const char *name)
> > +{
> > +	return name;
> > +}
> > +
> >  int __weak arch__compare_symbol_names(const char *namea, const char *nameb)
> >  {
> >  	return strcmp(namea, nameb);
> > diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> > index a4f0075b4e5c..0563f33c1eb3 100644
> > --- a/tools/perf/util/symbol.h
> > +++ b/tools/perf/util/symbol.h
> > @@ -349,6 +349,7 @@ bool elf__needs_adjust_symbols(GElf_Ehdr ehdr);
> >  void arch__sym_update(struct symbol *s, GElf_Sym *sym);
> >  #endif
> > 
> > +const char *arch__normalize_symbol_name(const char *name);
> >  #define SYMBOL_A 0
> >  #define SYMBOL_B 1
> > 
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-perf-users" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 
> 
> -- 
> Thomas Richter, Dept 3303, IBM LTC Boeblingen Germany
> --
> Vorsitzende des Aufsichtsrats: Martina Koederitz 
> Geschäftsführung: Dirk Wittkopp
> Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

[PATCH v2 5/5] perf-probe: Support escaped character in parser

[Masami Hiramatsu]

Support the special characters escaped by '\' in parser.
This allows user to specify versions directly like below.

  =====
  # ./perf probe -x /lib64/libc-2.25.so malloc_get_state\\@GLIBC_2.2.5
  Added new event:
    probe_libc:malloc_get_state (on malloc_get_state@GLIBC_2.2.5 in /usr/lib64/libc-2.25.so)

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

	  perf record -e probe_libc:malloc_get_state -aR sleep 1

  =====

Or, you can use separators in source filename, e.g.

  =====
  # ./perf probe -x /opt/test/a.out foo+bar.c:3
  Semantic error :There is non-digit character in offset.
    Error: Command Parse Error.
  =====

Usually "+" in source file cause parser error, but

  =====
  # ./perf probe -x /opt/test/a.out foo\\+bar.c:4
  Added new event:
    probe_a:main         (on @foo+bar.c:4 in /opt/test/a.out)

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

	  perf record -e probe_a:main -aR sleep 1
  =====

escaped "\+" allows you to specify that.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 Changes in v2:
  - Add a description and samples in Documentation/perf-probe.txt
---
 tools/perf/Documentation/perf-probe.txt |   16 +++++++++
 tools/perf/util/probe-event.c           |   54 +++++++++++++++++++------------
 tools/perf/util/string.c                |   46 ++++++++++++++++++++++++++
 tools/perf/util/string2.h               |    2 +
 4 files changed, 97 insertions(+), 21 deletions(-)

diff --git a/tools/perf/Documentation/perf-probe.txt b/tools/perf/Documentation/perf-probe.txt
index f96382692f42..b6866a05edd2 100644
--- a/tools/perf/Documentation/perf-probe.txt
+++ b/tools/perf/Documentation/perf-probe.txt
@@ -182,6 +182,14 @@ Note that before using the SDT event, the target binary (on which SDT events are
 For details of the SDT, see below.
 https://sourceware.org/gdb/onlinedocs/gdb/Static-Probe-Points.html
 
+ESCAPED CHARACTER
+-----------------
+
+In the probe syntax, '=', '@', '+', ':' and ';' are treated as a special character. You can use a backslash ('\') to escape the special characters.
+This is useful if you need to probe on a specific versioned symbols, like @GLIBC_... suffixes, or also you need to specify a source file which includes the special characters.
+Note that usually single backslash is consumed by shell, so you might need to pass double backslash (\\) or wrapping with single quotes (\'AAA\@BBB').
+See EXAMPLES how it is used.
+
 PROBE ARGUMENT
 --------------
 Each probe argument follows below syntax.
@@ -277,6 +285,14 @@ Add a USDT probe to a target process running in a different mount namespace
 
  ./perf probe --target-ns <target pid> -x /usr/lib/jvm/java-1.8.0-openjdk-1.8.0.121-0.b13.el7_3.x86_64/jre/lib/amd64/server/libjvm.so %sdt_hotspot:thread__sleep__end
 
+Add a probe on specific versioned symbol by backslash escape
+
+ ./perf probe -x /lib64/libc-2.25.so 'malloc_get_state\@GLIBC_2.2.5'
+
+Add a probe in a source file using special characters by backslash escape
+
+ ./perf probe -x /opt/test/a.out 'foo\+bar.c:4'
+
 
 SEE ALSO
 --------
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 94acc5846e2a..c082a27982e5 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1325,27 +1325,30 @@ static int parse_perf_probe_event_name(char **arg, struct perf_probe_event *pev)
 {
 	char *ptr;
 
-	ptr = strchr(*arg, ':');
+	ptr = strpbrk_esc(*arg, ":");
 	if (ptr) {
 		*ptr = '\0';
 		if (!pev->sdt && !is_c_func_name(*arg))
 			goto ng_name;
-		pev->group = strdup(*arg);
+		pev->group = strdup_esc(*arg);
 		if (!pev->group)
 			return -ENOMEM;
 		*arg = ptr + 1;
 	} else
 		pev->group = NULL;
-	if (!pev->sdt && !is_c_func_name(*arg)) {
+
+	pev->event = strdup_esc(*arg);
+	if (pev->event == NULL)
+		return -ENOMEM;
+
+	if (!pev->sdt && !is_c_func_name(pev->event)) {
+		zfree(&pev->event);
 ng_name:
+		zfree(&pev->group);
 		semantic_error("%s is bad for event name -it must "
 			       "follow C symbol-naming rule.\n", *arg);
 		return -EINVAL;
 	}
-	pev->event = strdup(*arg);
-	if (pev->event == NULL)
-		return -ENOMEM;
-
 	return 0;
 }
 
@@ -1373,7 +1376,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
 			arg++;
 	}
 
-	ptr = strpbrk(arg, ";=@+%");
+	ptr = strpbrk_esc(arg, ";=@+%");
 	if (pev->sdt) {
 		if (ptr) {
 			if (*ptr != '@') {
@@ -1387,7 +1390,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
 				pev->target = build_id_cache__origname(tmp);
 				free(tmp);
 			} else
-				pev->target = strdup(ptr + 1);
+				pev->target = strdup_esc(ptr + 1);
 			if (!pev->target)
 				return -ENOMEM;
 			*ptr = '\0';
@@ -1421,13 +1424,14 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
 	 *
 	 * Otherwise, we consider arg to be a function specification.
 	 */
-	if (!strpbrk(arg, "+@%") && (ptr = strpbrk(arg, ";:")) != NULL) {
+	if (!strpbrk_esc(arg, "+@%")) {
+		ptr = strpbrk_esc(arg, ";:");
 		/* This is a file spec if it includes a '.' before ; or : */
-		if (memchr(arg, '.', ptr - arg))
+		if (ptr && memchr(arg, '.', ptr - arg))
 			file_spec = true;
 	}
 
-	ptr = strpbrk(arg, ";:+@%");
+	ptr = strpbrk_esc(arg, ";:+@%");
 	if (ptr) {
 		nc = *ptr;
 		*ptr++ = '\0';
@@ -1436,7 +1440,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
 	if (arg[0] == '\0')
 		tmp = NULL;
 	else {
-		tmp = strdup(arg);
+		tmp = strdup_esc(arg);
 		if (tmp == NULL)
 			return -ENOMEM;
 	}
@@ -1469,12 +1473,12 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
 		arg = ptr;
 		c = nc;
 		if (c == ';') {	/* Lazy pattern must be the last part */
-			pp->lazy_line = strdup(arg);
+			pp->lazy_line = strdup(arg); /* let leave escapes */
 			if (pp->lazy_line == NULL)
 				return -ENOMEM;
 			break;
 		}
-		ptr = strpbrk(arg, ";:+@%");
+		ptr = strpbrk_esc(arg, ";:+@%");
 		if (ptr) {
 			nc = *ptr;
 			*ptr++ = '\0';
@@ -1501,7 +1505,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
 				semantic_error("SRC@SRC is not allowed.\n");
 				return -EINVAL;
 			}
-			pp->file = strdup(arg);
+			pp->file = strdup_esc(arg);
 			if (pp->file == NULL)
 				return -ENOMEM;
 			break;
@@ -2803,21 +2807,29 @@ static int find_probe_functions(struct map *map, char *name,
 	struct rb_node *tmp;
 	const char *norm, *ver;
 	char *buf = NULL;
+	bool cut_version = true;
 
 	if (map__load(map) < 0)
 		return 0;
 
+	/* If user gives a version, don't cut off the version from symbols */
+	if (strchr(name, '@'))
+		cut_version = false;
+
 	map__for_each_symbol(map, sym, tmp) {
 		norm = arch__normalize_symbol_name(sym->name);
 		if (!norm)
 			continue;
 
-		/* We don't care about default symbol or not */
-		ver = strchr(norm, '@');
-		if (ver) {
-			buf = strndup(norm, ver - norm);
-			norm = buf;
+		if (cut_version) {
+			/* We don't care about default symbol or not */
+			ver = strchr(norm, '@');
+			if (ver) {
+				buf = strndup(norm, ver - norm);
+				norm = buf;
+			}
 		}
+
 		if (strglobmatch(norm, name)) {
 			found++;
 			if (syms && found < probe_conf.max_probes)
diff --git a/tools/perf/util/string.c b/tools/perf/util/string.c
index aaa08ee8c717..d8bfd0c4d2cb 100644
--- a/tools/perf/util/string.c
+++ b/tools/perf/util/string.c
@@ -396,3 +396,49 @@ char *asprintf_expr_inout_ints(const char *var, bool in, size_t nints, int *ints
 	free(expr);
 	return NULL;
 }
+
+/* Like strpbrk(), but not break if it is right after a backslash (escaped) */
+char *strpbrk_esc(char *str, const char *stopset)
+{
+	char *ptr;
+
+	do {
+		ptr = strpbrk(str, stopset);
+		if (ptr == str ||
+		    (ptr == str + 1 && *(ptr - 1) != '\\'))
+			break;
+		str = ptr + 1;
+	} while (ptr && *(ptr - 1) == '\\' && *(ptr - 2) != '\\');
+
+	return ptr;
+}
+
+/* Like strdup, but do not copy a single backslash */
+char *strdup_esc(const char *str)
+{
+	char *s, *d, *p, *ret = strdup(str);
+
+	if (!ret)
+		return NULL;
+
+	d = strchr(ret, '\\');
+	if (!d)
+		return ret;
+
+	s = d + 1;
+	do {
+		if (*s == '\0') {
+			*d = '\0';
+			break;
+		}
+		p = strchr(s + 1, '\\');
+		if (p) {
+			memmove(d, s, p - s);
+			d += p - s;
+			s = p + 1;
+		} else
+			memmove(d, s, strlen(s) + 1);
+	} while (p);
+
+	return ret;
+}
diff --git a/tools/perf/util/string2.h b/tools/perf/util/string2.h
index ee14ca5451ab..4c68a09b97e8 100644
--- a/tools/perf/util/string2.h
+++ b/tools/perf/util/string2.h
@@ -39,5 +39,7 @@ static inline char *asprintf_expr_not_in_ints(const char *var, size_t nints, int
 	return asprintf_expr_inout_ints(var, false, nints, ints);
 }
 
+char *strpbrk_esc(char *str, const char *stopset);
+char *strdup_esc(const char *str);
 
 #endif /* PERF_STRING_H */

Re: [PATCH v2 5/5] perf-probe: Support escaped character in parser

[Thomas-Mich Richter]

On 12/07/2017 08:21 AM, Masami Hiramatsu wrote:
> Support the special characters escaped by '\' in parser.
> This allows user to specify versions directly like below.
> 
>   =====
>   # ./perf probe -x /lib64/libc-2.25.so malloc_get_state\\@GLIBC_2.2.5
>   Added new event:
>     probe_libc:malloc_get_state (on malloc_get_state@GLIBC_2.2.5 in /usr/lib64/libc-2.25.so)
> 
>   You can now use it in all perf tools, such as:
> 
> 	  perf record -e probe_libc:malloc_get_state -aR sleep 1
> 
>   =====
> 
> Or, you can use separators in source filename, e.g.
> 
>   =====
>   # ./perf probe -x /opt/test/a.out foo+bar.c:3
>   Semantic error :There is non-digit character in offset.
>     Error: Command Parse Error.
>   =====
> 
> Usually "+" in source file cause parser error, but
> 
>   =====
>   # ./perf probe -x /opt/test/a.out foo\\+bar.c:4
>   Added new event:
>     probe_a:main         (on @foo+bar.c:4 in /opt/test/a.out)
> 
>   You can now use it in all perf tools, such as:
> 
> 	  perf record -e probe_a:main -aR sleep 1
>   =====
> 
> escaped "\+" allows you to specify that.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  Changes in v2:
>   - Add a description and samples in Documentation/perf-probe.txt
> ---
>  tools/perf/Documentation/perf-probe.txt |   16 +++++++++
>  tools/perf/util/probe-event.c           |   54 +++++++++++++++++++------------
>  tools/perf/util/string.c                |   46 ++++++++++++++++++++++++++
>  tools/perf/util/string2.h               |    2 +
>  4 files changed, 97 insertions(+), 21 deletions(-)
> 
> diff --git a/tools/perf/Documentation/perf-probe.txt b/tools/perf/Documentation/perf-probe.txt
> index f96382692f42..b6866a05edd2 100644
> --- a/tools/perf/Documentation/perf-probe.txt
> +++ b/tools/perf/Documentation/perf-probe.txt
> @@ -182,6 +182,14 @@ Note that before using the SDT event, the target binary (on which SDT events are
>  For details of the SDT, see below.
>  https://sourceware.org/gdb/onlinedocs/gdb/Static-Probe-Points.html
> 
> +ESCAPED CHARACTER
> +-----------------
> +
> +In the probe syntax, '=', '@', '+', ':' and ';' are treated as a special character. You can use a backslash ('\') to escape the special characters.
> +This is useful if you need to probe on a specific versioned symbols, like @GLIBC_... suffixes, or also you need to specify a source file which includes the special characters.
> +Note that usually single backslash is consumed by shell, so you might need to pass double backslash (\\) or wrapping with single quotes (\'AAA\@BBB').
> +See EXAMPLES how it is used.
> +
>  PROBE ARGUMENT
>  --------------
>  Each probe argument follows below syntax.
> @@ -277,6 +285,14 @@ Add a USDT probe to a target process running in a different mount namespace
> 
>   ./perf probe --target-ns <target pid> -x /usr/lib/jvm/java-1.8.0-openjdk-1.8.0.121-0.b13.el7_3.x86_64/jre/lib/amd64/server/libjvm.so %sdt_hotspot:thread__sleep__end
> 
> +Add a probe on specific versioned symbol by backslash escape
> +
> + ./perf probe -x /lib64/libc-2.25.so 'malloc_get_state\@GLIBC_2.2.5'
> +
> +Add a probe in a source file using special characters by backslash escape
> +
> + ./perf probe -x /opt/test/a.out 'foo\+bar.c:4'
> +
> 
>  SEE ALSO
>  --------
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 94acc5846e2a..c082a27982e5 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -1325,27 +1325,30 @@ static int parse_perf_probe_event_name(char **arg, struct perf_probe_event *pev)
>  {
>  	char *ptr;
> 
> -	ptr = strchr(*arg, ':');
> +	ptr = strpbrk_esc(*arg, ":");
>  	if (ptr) {
>  		*ptr = '\0';
>  		if (!pev->sdt && !is_c_func_name(*arg))
>  			goto ng_name;
> -		pev->group = strdup(*arg);
> +		pev->group = strdup_esc(*arg);
>  		if (!pev->group)
>  			return -ENOMEM;
>  		*arg = ptr + 1;
>  	} else
>  		pev->group = NULL;
> -	if (!pev->sdt && !is_c_func_name(*arg)) {
> +
> +	pev->event = strdup_esc(*arg);
> +	if (pev->event == NULL)
> +		return -ENOMEM;
> +
> +	if (!pev->sdt && !is_c_func_name(pev->event)) {
> +		zfree(&pev->event);
>  ng_name:
> +		zfree(&pev->group);
>  		semantic_error("%s is bad for event name -it must "
>  			       "follow C symbol-naming rule.\n", *arg);
>  		return -EINVAL;
>  	}
> -	pev->event = strdup(*arg);
> -	if (pev->event == NULL)
> -		return -ENOMEM;
> -
>  	return 0;
>  }
> 
> @@ -1373,7 +1376,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
>  			arg++;
>  	}
> 
> -	ptr = strpbrk(arg, ";=@+%");
> +	ptr = strpbrk_esc(arg, ";=@+%");
>  	if (pev->sdt) {
>  		if (ptr) {
>  			if (*ptr != '@') {
> @@ -1387,7 +1390,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
>  				pev->target = build_id_cache__origname(tmp);
>  				free(tmp);
>  			} else
> -				pev->target = strdup(ptr + 1);
> +				pev->target = strdup_esc(ptr + 1);
>  			if (!pev->target)
>  				return -ENOMEM;
>  			*ptr = '\0';
> @@ -1421,13 +1424,14 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
>  	 *
>  	 * Otherwise, we consider arg to be a function specification.
>  	 */
> -	if (!strpbrk(arg, "+@%") && (ptr = strpbrk(arg, ";:")) != NULL) {
> +	if (!strpbrk_esc(arg, "+@%")) {
> +		ptr = strpbrk_esc(arg, ";:");
>  		/* This is a file spec if it includes a '.' before ; or : */
> -		if (memchr(arg, '.', ptr - arg))
> +		if (ptr && memchr(arg, '.', ptr - arg))
>  			file_spec = true;
>  	}
> 
> -	ptr = strpbrk(arg, ";:+@%");
> +	ptr = strpbrk_esc(arg, ";:+@%");
>  	if (ptr) {
>  		nc = *ptr;
>  		*ptr++ = '\0';
> @@ -1436,7 +1440,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
>  	if (arg[0] == '\0')
>  		tmp = NULL;
>  	else {
> -		tmp = strdup(arg);
> +		tmp = strdup_esc(arg);
>  		if (tmp == NULL)
>  			return -ENOMEM;
>  	}
> @@ -1469,12 +1473,12 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
>  		arg = ptr;
>  		c = nc;
>  		if (c == ';') {	/* Lazy pattern must be the last part */
> -			pp->lazy_line = strdup(arg);
> +			pp->lazy_line = strdup(arg); /* let leave escapes */
>  			if (pp->lazy_line == NULL)
>  				return -ENOMEM;
>  			break;
>  		}
> -		ptr = strpbrk(arg, ";:+@%");
> +		ptr = strpbrk_esc(arg, ";:+@%");
>  		if (ptr) {
>  			nc = *ptr;
>  			*ptr++ = '\0';
> @@ -1501,7 +1505,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
>  				semantic_error("SRC@SRC is not allowed.\n");
>  				return -EINVAL;
>  			}
> -			pp->file = strdup(arg);
> +			pp->file = strdup_esc(arg);
>  			if (pp->file == NULL)
>  				return -ENOMEM;
>  			break;
> @@ -2803,21 +2807,29 @@ static int find_probe_functions(struct map *map, char *name,
>  	struct rb_node *tmp;
>  	const char *norm, *ver;
>  	char *buf = NULL;
> +	bool cut_version = true;
> 
>  	if (map__load(map) < 0)
>  		return 0;
> 
> +	/* If user gives a version, don't cut off the version from symbols */
> +	if (strchr(name, '@'))
> +		cut_version = false;
> +
>  	map__for_each_symbol(map, sym, tmp) {
>  		norm = arch__normalize_symbol_name(sym->name);

Same as before (patch 4)

>  		if (!norm)
>  			continue;
> 
> -		/* We don't care about default symbol or not */
> -		ver = strchr(norm, '@');
> -		if (ver) {
> -			buf = strndup(norm, ver - norm);
> -			norm = buf;

Again strndup() may return a NULL ptr and then strglobmatch() / strchr() below
dereferences it.

> +		if (cut_version) {
> +			/* We don't care about default symbol or not */
> +			ver = strchr(norm, '@');
> +			if (ver) {
> +				buf = strndup(norm, ver - norm);
> +				norm = buf;
> +			}
>  		}
> +
>  		if (strglobmatch(norm, name)) {
>  			found++;
>  			if (syms && found < probe_conf.max_probes)
> diff --git a/tools/perf/util/string.c b/tools/perf/util/string.c
> index aaa08ee8c717..d8bfd0c4d2cb 100644
> --- a/tools/perf/util/string.c
> +++ b/tools/perf/util/string.c
> @@ -396,3 +396,49 @@ char *asprintf_expr_inout_ints(const char *var, bool in, size_t nints, int *ints
>  	free(expr);
>  	return NULL;
>  }
> +
> +/* Like strpbrk(), but not break if it is right after a backslash (escaped) */
> +char *strpbrk_esc(char *str, const char *stopset)
> +{
> +	char *ptr;
> +
> +	do {
> +		ptr = strpbrk(str, stopset);
> +		if (ptr == str ||
> +		    (ptr == str + 1 && *(ptr - 1) != '\\'))
> +			break;
> +		str = ptr + 1;
> +	} while (ptr && *(ptr - 1) == '\\' && *(ptr - 2) != '\\');
> +
> +	return ptr;
> +}
> +
> +/* Like strdup, but do not copy a single backslash */
> +char *strdup_esc(const char *str)
> +{
> +	char *s, *d, *p, *ret = strdup(str);
> +
> +	if (!ret)
> +		return NULL;
> +
> +	d = strchr(ret, '\\');
> +	if (!d)
> +		return ret;
> +
> +	s = d + 1;
> +	do {
> +		if (*s == '\0') {
> +			*d = '\0';
> +			break;
> +		}
> +		p = strchr(s + 1, '\\');
> +		if (p) {
> +			memmove(d, s, p - s);
> +			d += p - s;
> +			s = p + 1;
> +		} else
> +			memmove(d, s, strlen(s) + 1);
> +	} while (p);
> +
> +	return ret;
> +}
> diff --git a/tools/perf/util/string2.h b/tools/perf/util/string2.h
> index ee14ca5451ab..4c68a09b97e8 100644
> --- a/tools/perf/util/string2.h
> +++ b/tools/perf/util/string2.h
> @@ -39,5 +39,7 @@ static inline char *asprintf_expr_not_in_ints(const char *var, size_t nints, int
>  	return asprintf_expr_inout_ints(var, false, nints, ints);
>  }
> 
> +char *strpbrk_esc(char *str, const char *stopset);
> +char *strdup_esc(const char *str);
> 
>  #endif /* PERF_STRING_H */
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-perf-users" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


-- 
Thomas Richter, Dept 3303, IBM LTC Boeblingen Germany
--
Vorsitzende des Aufsichtsrats: Martina Koederitz 
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294

Re: [PATCH v2 5/5] perf-probe: Support escaped character in parser

[Masami Hiramatsu]

On Fri, 8 Dec 2017 12:45:45 +0100
Thomas-Mich Richter <tmricht@linux.vnet.ibm.com> wrote:

> On 12/07/2017 08:21 AM, Masami Hiramatsu wrote:
> > Support the special characters escaped by '\' in parser.
> > This allows user to specify versions directly like below.
> > 
> >   =====
> >   # ./perf probe -x /lib64/libc-2.25.so malloc_get_state\\@GLIBC_2.2.5
> >   Added new event:
> >     probe_libc:malloc_get_state (on malloc_get_state@GLIBC_2.2.5 in /usr/lib64/libc-2.25.so)
> > 
> >   You can now use it in all perf tools, such as:
> > 
> > 	  perf record -e probe_libc:malloc_get_state -aR sleep 1
> > 
> >   =====
> > 
> > Or, you can use separators in source filename, e.g.
> > 
> >   =====
> >   # ./perf probe -x /opt/test/a.out foo+bar.c:3
> >   Semantic error :There is non-digit character in offset.
> >     Error: Command Parse Error.
> >   =====
> > 
> > Usually "+" in source file cause parser error, but
> > 
> >   =====
> >   # ./perf probe -x /opt/test/a.out foo\\+bar.c:4
> >   Added new event:
> >     probe_a:main         (on @foo+bar.c:4 in /opt/test/a.out)
> > 
> >   You can now use it in all perf tools, such as:
> > 
> > 	  perf record -e probe_a:main -aR sleep 1
> >   =====
> > 
> > escaped "\+" allows you to specify that.
> > 
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > ---
> >  Changes in v2:
> >   - Add a description and samples in Documentation/perf-probe.txt
> > ---
> >  tools/perf/Documentation/perf-probe.txt |   16 +++++++++
> >  tools/perf/util/probe-event.c           |   54 +++++++++++++++++++------------
> >  tools/perf/util/string.c                |   46 ++++++++++++++++++++++++++
> >  tools/perf/util/string2.h               |    2 +
> >  4 files changed, 97 insertions(+), 21 deletions(-)
> > 
> > diff --git a/tools/perf/Documentation/perf-probe.txt b/tools/perf/Documentation/perf-probe.txt
> > index f96382692f42..b6866a05edd2 100644
> > --- a/tools/perf/Documentation/perf-probe.txt
> > +++ b/tools/perf/Documentation/perf-probe.txt
> > @@ -182,6 +182,14 @@ Note that before using the SDT event, the target binary (on which SDT events are
> >  For details of the SDT, see below.
> >  https://sourceware.org/gdb/onlinedocs/gdb/Static-Probe-Points.html
> > 
> > +ESCAPED CHARACTER
> > +-----------------
> > +
> > +In the probe syntax, '=', '@', '+', ':' and ';' are treated as a special character. You can use a backslash ('\') to escape the special characters.
> > +This is useful if you need to probe on a specific versioned symbols, like @GLIBC_... suffixes, or also you need to specify a source file which includes the special characters.
> > +Note that usually single backslash is consumed by shell, so you might need to pass double backslash (\\) or wrapping with single quotes (\'AAA\@BBB').
> > +See EXAMPLES how it is used.
> > +
> >  PROBE ARGUMENT
> >  --------------
> >  Each probe argument follows below syntax.
> > @@ -277,6 +285,14 @@ Add a USDT probe to a target process running in a different mount namespace
> > 
> >   ./perf probe --target-ns <target pid> -x /usr/lib/jvm/java-1.8.0-openjdk-1.8.0.121-0.b13.el7_3.x86_64/jre/lib/amd64/server/libjvm.so %sdt_hotspot:thread__sleep__end
> > 
> > +Add a probe on specific versioned symbol by backslash escape
> > +
> > + ./perf probe -x /lib64/libc-2.25.so 'malloc_get_state\@GLIBC_2.2.5'
> > +
> > +Add a probe in a source file using special characters by backslash escape
> > +
> > + ./perf probe -x /opt/test/a.out 'foo\+bar.c:4'
> > +
> > 
> >  SEE ALSO
> >  --------
> > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > index 94acc5846e2a..c082a27982e5 100644
> > --- a/tools/perf/util/probe-event.c
> > +++ b/tools/perf/util/probe-event.c
> > @@ -1325,27 +1325,30 @@ static int parse_perf_probe_event_name(char **arg, struct perf_probe_event *pev)
> >  {
> >  	char *ptr;
> > 
> > -	ptr = strchr(*arg, ':');
> > +	ptr = strpbrk_esc(*arg, ":");
> >  	if (ptr) {
> >  		*ptr = '\0';
> >  		if (!pev->sdt && !is_c_func_name(*arg))
> >  			goto ng_name;
> > -		pev->group = strdup(*arg);
> > +		pev->group = strdup_esc(*arg);
> >  		if (!pev->group)
> >  			return -ENOMEM;
> >  		*arg = ptr + 1;
> >  	} else
> >  		pev->group = NULL;
> > -	if (!pev->sdt && !is_c_func_name(*arg)) {
> > +
> > +	pev->event = strdup_esc(*arg);
> > +	if (pev->event == NULL)
> > +		return -ENOMEM;
> > +
> > +	if (!pev->sdt && !is_c_func_name(pev->event)) {
> > +		zfree(&pev->event);
> >  ng_name:
> > +		zfree(&pev->group);
> >  		semantic_error("%s is bad for event name -it must "
> >  			       "follow C symbol-naming rule.\n", *arg);
> >  		return -EINVAL;
> >  	}
> > -	pev->event = strdup(*arg);
> > -	if (pev->event == NULL)
> > -		return -ENOMEM;
> > -
> >  	return 0;
> >  }
> > 
> > @@ -1373,7 +1376,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> >  			arg++;
> >  	}
> > 
> > -	ptr = strpbrk(arg, ";=@+%");
> > +	ptr = strpbrk_esc(arg, ";=@+%");
> >  	if (pev->sdt) {
> >  		if (ptr) {
> >  			if (*ptr != '@') {
> > @@ -1387,7 +1390,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> >  				pev->target = build_id_cache__origname(tmp);
> >  				free(tmp);
> >  			} else
> > -				pev->target = strdup(ptr + 1);
> > +				pev->target = strdup_esc(ptr + 1);
> >  			if (!pev->target)
> >  				return -ENOMEM;
> >  			*ptr = '\0';
> > @@ -1421,13 +1424,14 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> >  	 *
> >  	 * Otherwise, we consider arg to be a function specification.
> >  	 */
> > -	if (!strpbrk(arg, "+@%") && (ptr = strpbrk(arg, ";:")) != NULL) {
> > +	if (!strpbrk_esc(arg, "+@%")) {
> > +		ptr = strpbrk_esc(arg, ";:");
> >  		/* This is a file spec if it includes a '.' before ; or : */
> > -		if (memchr(arg, '.', ptr - arg))
> > +		if (ptr && memchr(arg, '.', ptr - arg))
> >  			file_spec = true;
> >  	}
> > 
> > -	ptr = strpbrk(arg, ";:+@%");
> > +	ptr = strpbrk_esc(arg, ";:+@%");
> >  	if (ptr) {
> >  		nc = *ptr;
> >  		*ptr++ = '\0';
> > @@ -1436,7 +1440,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> >  	if (arg[0] == '\0')
> >  		tmp = NULL;
> >  	else {
> > -		tmp = strdup(arg);
> > +		tmp = strdup_esc(arg);
> >  		if (tmp == NULL)
> >  			return -ENOMEM;
> >  	}
> > @@ -1469,12 +1473,12 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> >  		arg = ptr;
> >  		c = nc;
> >  		if (c == ';') {	/* Lazy pattern must be the last part */
> > -			pp->lazy_line = strdup(arg);
> > +			pp->lazy_line = strdup(arg); /* let leave escapes */
> >  			if (pp->lazy_line == NULL)
> >  				return -ENOMEM;
> >  			break;
> >  		}
> > -		ptr = strpbrk(arg, ";:+@%");
> > +		ptr = strpbrk_esc(arg, ";:+@%");
> >  		if (ptr) {
> >  			nc = *ptr;
> >  			*ptr++ = '\0';
> > @@ -1501,7 +1505,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> >  				semantic_error("SRC@SRC is not allowed.\n");
> >  				return -EINVAL;
> >  			}
> > -			pp->file = strdup(arg);
> > +			pp->file = strdup_esc(arg);
> >  			if (pp->file == NULL)
> >  				return -ENOMEM;
> >  			break;
> > @@ -2803,21 +2807,29 @@ static int find_probe_functions(struct map *map, char *name,
> >  	struct rb_node *tmp;
> >  	const char *norm, *ver;
> >  	char *buf = NULL;
> > +	bool cut_version = true;
> > 
> >  	if (map__load(map) < 0)
> >  		return 0;
> > 
> > +	/* If user gives a version, don't cut off the version from symbols */
> > +	if (strchr(name, '@'))
> > +		cut_version = false;
> > +
> >  	map__for_each_symbol(map, sym, tmp) {
> >  		norm = arch__normalize_symbol_name(sym->name);
> 
> Same as before (patch 4)
> 
> >  		if (!norm)
> >  			continue;
> > 
> > -		/* We don't care about default symbol or not */
> > -		ver = strchr(norm, '@');
> > -		if (ver) {
> > -			buf = strndup(norm, ver - norm);
> > -			norm = buf;
> 
> Again strndup() may return a NULL ptr and then strglobmatch() / strchr() below
> dereferences it.

OK, but those will be fixed in patch 4/5.
(Of course below block should be updated)

Anyway, Thank you!

> 
> > +		if (cut_version) {
> > +			/* We don't care about default symbol or not */
> > +			ver = strchr(norm, '@');
> > +			if (ver) {
> > +				buf = strndup(norm, ver - norm);
> > +				norm = buf;
> > +			}
> >  		}
> > +
> >  		if (strglobmatch(norm, name)) {
> >  			found++;
> >  			if (syms && found < probe_conf.max_probes)
> > diff --git a/tools/perf/util/string.c b/tools/perf/util/string.c
> > index aaa08ee8c717..d8bfd0c4d2cb 100644
> > --- a/tools/perf/util/string.c
> > +++ b/tools/perf/util/string.c
> > @@ -396,3 +396,49 @@ char *asprintf_expr_inout_ints(const char *var, bool in, size_t nints, int *ints
> >  	free(expr);
> >  	return NULL;
> >  }
> > +
> > +/* Like strpbrk(), but not break if it is right after a backslash (escaped) */
> > +char *strpbrk_esc(char *str, const char *stopset)
> > +{
> > +	char *ptr;
> > +
> > +	do {
> > +		ptr = strpbrk(str, stopset);
> > +		if (ptr == str ||
> > +		    (ptr == str + 1 && *(ptr - 1) != '\\'))
> > +			break;
> > +		str = ptr + 1;
> > +	} while (ptr && *(ptr - 1) == '\\' && *(ptr - 2) != '\\');
> > +
> > +	return ptr;
> > +}
> > +
> > +/* Like strdup, but do not copy a single backslash */
> > +char *strdup_esc(const char *str)
> > +{
> > +	char *s, *d, *p, *ret = strdup(str);
> > +
> > +	if (!ret)
> > +		return NULL;
> > +
> > +	d = strchr(ret, '\\');
> > +	if (!d)
> > +		return ret;
> > +
> > +	s = d + 1;
> > +	do {
> > +		if (*s == '\0') {
> > +			*d = '\0';
> > +			break;
> > +		}
> > +		p = strchr(s + 1, '\\');
> > +		if (p) {
> > +			memmove(d, s, p - s);
> > +			d += p - s;
> > +			s = p + 1;
> > +		} else
> > +			memmove(d, s, strlen(s) + 1);
> > +	} while (p);
> > +
> > +	return ret;
> > +}
> > diff --git a/tools/perf/util/string2.h b/tools/perf/util/string2.h
> > index ee14ca5451ab..4c68a09b97e8 100644
> > --- a/tools/perf/util/string2.h
> > +++ b/tools/perf/util/string2.h
> > @@ -39,5 +39,7 @@ static inline char *asprintf_expr_not_in_ints(const char *var, size_t nints, int
> >  	return asprintf_expr_inout_ints(var, false, nints, ints);
> >  }
> > 
> > +char *strpbrk_esc(char *str, const char *stopset);
> > +char *strdup_esc(const char *str);
> > 
> >  #endif /* PERF_STRING_H */
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-perf-users" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 
> 
> -- 
> Thomas Richter, Dept 3303, IBM LTC Boeblingen Germany
> --
> Vorsitzende des Aufsichtsrats: Martina Koederitz 
> Geschäftsführung: Dirk Wittkopp
> Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH v2 0/5] perf-probe: Improve probing on versioned symbols

[Ravi Bangoria]

On 12/07/2017 12:49 PM, Masami Hiramatsu wrote:
> Hi,
>
> Here is the 2nd version of the series for probing on
> versioned symbols in libraries. This includes 5 patches
> to fix the issues discussed on perf-users ML 
> (https://www.spinics.net/lists/linux-perf-users/msg04637.html)
>
> The first version is here; https://lkml.org/lkml/2017/12/5/1124
>
> Here is the updates;
>
> [3/5] Add a description in Documentation/perf-probe.txt
> [5/5] Add a description and examples in Documentation/perf-probe.txt
>
> Thank you,

Ack-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>

Re: [PATCH v2 0/5] perf-probe: Improve probing on versioned symbols

[Masami Hiramatsu]

On Thu, 7 Dec 2017 18:17:51 +0530
Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote:

> 
> 
> On 12/07/2017 12:49 PM, Masami Hiramatsu wrote:
> > Hi,
> >
> > Here is the 2nd version of the series for probing on
> > versioned symbols in libraries. This includes 5 patches
> > to fix the issues discussed on perf-users ML 
> > (https://www.spinics.net/lists/linux-perf-users/msg04637.html)
> >
> > The first version is here; https://lkml.org/lkml/2017/12/5/1124
> >
> > Here is the updates;
> >
> > [3/5] Add a description in Documentation/perf-probe.txt
> > [5/5] Add a description and examples in Documentation/perf-probe.txt
> >
> > Thank you,
> 
> Ack-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>

Thanks!, is that OK for series? and Acked-by ? B-)


> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH v2 0/5] perf-probe: Improve probing on versioned symbols

[Ravi Bangoria]

On 12/08/2017 09:54 PM, Masami Hiramatsu wrote:
> On Thu, 7 Dec 2017 18:17:51 +0530
> Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote:
>
>>
>> On 12/07/2017 12:49 PM, Masami Hiramatsu wrote:
>>> Hi,
>>>
>>> Here is the 2nd version of the series for probing on
>>> versioned symbols in libraries. This includes 5 patches
>>> to fix the issues discussed on perf-users ML 
>>> (https://www.spinics.net/lists/linux-perf-users/msg04637.html)
>>>
>>> The first version is here; https://lkml.org/lkml/2017/12/5/1124
>>>
>>> Here is the updates;
>>>
>>> [3/5] Add a description in Documentation/perf-probe.txt
>>> [5/5] Add a description and examples in Documentation/perf-probe.txt
>>>
>>> Thank you,
>> Ack-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> Thanks!, is that OK for series? and Acked-by ? B-)

Yes Masami, Acked for the series.

( Sorry, I just forgot to add 'ed' in Acked-by :) )

Thanks,
Ravi

Re: [PATCH v2 0/5] perf-probe: Improve probing on versioned symbols

[Thomas-Mich Richter]

On 12/07/2017 08:19 AM, Masami Hiramatsu wrote:
> Hi,
> 
> Here is the 2nd version of the series for probing on
> versioned symbols in libraries. This includes 5 patches
> to fix the issues discussed on perf-users ML 
> (https://www.spinics.net/lists/linux-perf-users/msg04637.html)
> 
> The first version is here; https://lkml.org/lkml/2017/12/5/1124
> 
> Here is the updates;
> 
> [3/5] Add a description in Documentation/perf-probe.txt
> [5/5] Add a description and examples in Documentation/perf-probe.txt
> 
> Thank you,
> ---
> 
> Masami Hiramatsu (5):
>       perf-probe: Add warning message if there is unexpected event name
>       perf-probe: Cut off the version suffix from event name
>       perf-probe: Add __return suffix for return events
>       perf-probe: Find versioned symbols from map
>       perf-probe: Support escaped character in parser
> 
> 
>  tools/perf/Documentation/perf-probe.txt     |   18 ++++++
>  tools/perf/arch/powerpc/util/sym-handling.c |    8 +++
>  tools/perf/util/probe-event.c               |   81 +++++++++++++++++++--------
>  tools/perf/util/string.c                    |   46 +++++++++++++++
>  tools/perf/util/string2.h                   |    2 +
>  tools/perf/util/symbol.c                    |    5 ++
>  tools/perf/util/symbol.h                    |    1 
>  7 files changed, 137 insertions(+), 24 deletions(-)
> 
> --
> Masami Hiramatsu (Linaro Ltd.) <mhiramat@kernel.org>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-perf-users" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

If you fix the findings for patch 4 and 5 you have my 

Reviewed-by: Thomas Richter <tmricht@linux.vnet.ibm.com>
-- 
Thomas Richter, Dept 3303, IBM LTC Boeblingen Germany
--
Vorsitzende des Aufsichtsrats: Martina Koederitz 
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294

Re: [PATCH v2 0/5] perf-probe: Improve probing on versioned symbols

[Masami Hiramatsu]

On Fri, 8 Dec 2017 12:56:06 +0100
Thomas-Mich Richter <tmricht@linux.vnet.ibm.com> wrote:

> On 12/07/2017 08:19 AM, Masami Hiramatsu wrote:
> > Hi,
> > 
> > Here is the 2nd version of the series for probing on
> > versioned symbols in libraries. This includes 5 patches
> > to fix the issues discussed on perf-users ML 
> > (https://www.spinics.net/lists/linux-perf-users/msg04637.html)
> > 
> > The first version is here; https://lkml.org/lkml/2017/12/5/1124
> > 
> > Here is the updates;
> > 
> > [3/5] Add a description in Documentation/perf-probe.txt
> > [5/5] Add a description and examples in Documentation/perf-probe.txt
> > 
> > Thank you,
> > ---
> > 
> > Masami Hiramatsu (5):
> >       perf-probe: Add warning message if there is unexpected event name
> >       perf-probe: Cut off the version suffix from event name
> >       perf-probe: Add __return suffix for return events
> >       perf-probe: Find versioned symbols from map
> >       perf-probe: Support escaped character in parser
> > 
> > 
> >  tools/perf/Documentation/perf-probe.txt     |   18 ++++++
> >  tools/perf/arch/powerpc/util/sym-handling.c |    8 +++
> >  tools/perf/util/probe-event.c               |   81 +++++++++++++++++++--------
> >  tools/perf/util/string.c                    |   46 +++++++++++++++
> >  tools/perf/util/string2.h                   |    2 +
> >  tools/perf/util/symbol.c                    |    5 ++
> >  tools/perf/util/symbol.h                    |    1 
> >  7 files changed, 137 insertions(+), 24 deletions(-)
> > 
> > --
> > Masami Hiramatsu (Linaro Ltd.) <mhiramat@kernel.org>
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-perf-users" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 
> If you fix the findings for patch 4 and 5 you have my 
> 
> Reviewed-by: Thomas Richter <tmricht@linux.vnet.ibm.com>

OK, Thanks!


-- 
Masami Hiramatsu <mhiramat@kernel.org>