Hi,
Here is 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)
[1/5] Warn if the event name is invalid. This notices
user that there is internal error which caused by
unexpected input.
[2/5] Cut off the version suffix when making event name
from symbol name. This fixes a issue when a wildcard
hits versioned symbol.
[3/5] Add "__return" suffix to the event name
automatically for the return probe, so that user
can easily identify which event is on function
return.
[4/5] Find versioned symbol from map without wildcard.
This allows user to specify a symbol without version
suffix nor wildcard. Anyway, at this point we have
no way to specify version suffix, because parser
recognizes "@" is a separator of "function@file"
syntax. This is fixed in next patch.
[5/5] Allow user to use backslash-escaped characters
on command line. This has 2 effects, user can specify
version suffix directly, and we can use special
characters in source file.
Thanks,
---
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/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
6 files changed, 120 insertions(+), 23 deletions(-)
--
Masami Hiramatsu (Linaro Ltd.) <[email protected]>
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 <[email protected]>
---
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;
}
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 <[email protected]>
Reported-by: Arnaldo Carvalho de Melo <[email protected]>
Reported-by: bhargavb <[email protected]>
---
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';
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 <[email protected]>
Reported-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/probe-event.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
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;
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 <[email protected]>
---
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
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 <[email protected]>
---
tools/perf/util/probe-event.c | 54 +++++++++++++++++++++++++----------------
tools/perf/util/string.c | 46 +++++++++++++++++++++++++++++++++++
tools/perf/util/string2.h | 2 ++
3 files changed, 81 insertions(+), 21 deletions(-)
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 */
Hi Masami,
On 12/06/2017 08:36 AM, Masami Hiramatsu wrote:
> [4/5] Find versioned symbol from map without wildcard.
> This allows user to specify a symbol without version
> suffix nor wildcard. Anyway, at this point we have
> no way to specify version suffix, because parser
> recognizes "@" is a separator of "function@file"
> syntax. This is fixed in next patch.
> [5/5] Allow user to use backslash-escaped characters
> on command line. This has 2 effects, user can specify
> version suffix directly, and we can use special
> characters in source file.
Just a thought. Will it be good to describe / illustrate this
in man page as well?
Thanks,
Ravi
On Thu, 7 Dec 2017 10:13:28 +0530
Ravi Bangoria <[email protected]> wrote:
> Hi Masami,
>
>
> On 12/06/2017 08:36 AM, Masami Hiramatsu wrote:
> > [4/5] Find versioned symbol from map without wildcard.
> > This allows user to specify a symbol without version
> > suffix nor wildcard. Anyway, at this point we have
> > no way to specify version suffix, because parser
> > recognizes "@" is a separator of "function@file"
> > syntax. This is fixed in next patch.
> > [5/5] Allow user to use backslash-escaped characters
> > on command line. This has 2 effects, user can specify
> > version suffix directly, and we can use special
> > characters in source file.
>
> Just a thought. Will it be good to describe / illustrate this
> in man page as well?
Good catch! I forgot that...
Thanks!
>
> Thanks,
> Ravi
>
--
Masami Hiramatsu <[email protected]>