* [PATCH v3 1/5] perf-probe: Add warning message if there is unexpected event name
2017-12-08 16:26 [PATCH v3 0/5] perf-probe: Improve probing on versioned symbols Masami Hiramatsu
@ 2017-12-08 16:26 ` Masami Hiramatsu
2017-12-08 16:27 ` [PATCH v3 2/5] perf-probe: Cut off the version suffix from " Masami Hiramatsu
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Masami Hiramatsu @ 2017-12-08 16:26 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Masami Hiramatsu, bhargavb, linux-kernel, Paul Clarke,
Ravi Bangoria, Thomas Richter, linux-rt-users, linux-perf-users
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 an invalid event name.
Error: Failed to add events.
======
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Acked-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
---
Changes in v3:
- Update the error message.
---
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..262d5da86623 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 an invalid event name.\n",
+ buf);
+ ret = -EINVAL;
+ }
+
return ret;
}
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v3 2/5] perf-probe: Cut off the version suffix from event name
2017-12-08 16:26 [PATCH v3 0/5] perf-probe: Improve probing on versioned symbols Masami Hiramatsu
2017-12-08 16:26 ` [PATCH v3 1/5] perf-probe: Add warning message if there is unexpected event name Masami Hiramatsu
@ 2017-12-08 16:27 ` Masami Hiramatsu
2017-12-08 16:27 ` [PATCH v3 3/5] perf-probe: Add __return suffix for return events Masami Hiramatsu
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Masami Hiramatsu @ 2017-12-08 16:27 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Masami Hiramatsu, bhargavb, linux-kernel, Paul Clarke,
Ravi Bangoria, Thomas Richter, linux-rt-users, linux-perf-users
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>
Acked-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.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 262d5da86623..7e582547ac07 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';
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v3 3/5] perf-probe: Add __return suffix for return events
2017-12-08 16:26 [PATCH v3 0/5] perf-probe: Improve probing on versioned symbols Masami Hiramatsu
2017-12-08 16:26 ` [PATCH v3 1/5] perf-probe: Add warning message if there is unexpected event name Masami Hiramatsu
2017-12-08 16:27 ` [PATCH v3 2/5] perf-probe: Cut off the version suffix from " Masami Hiramatsu
@ 2017-12-08 16:27 ` Masami Hiramatsu
2017-12-08 16:28 ` [PATCH v3 4/5] perf-probe: Find versioned symbols from map Masami Hiramatsu
2017-12-08 16:28 ` [PATCH v3 5/5] perf-probe: Support escaped character in parser Masami Hiramatsu
4 siblings, 0 replies; 9+ messages in thread
From: Masami Hiramatsu @ 2017-12-08 16:27 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Masami Hiramatsu, bhargavb, linux-kernel, Paul Clarke,
Ravi Bangoria, Thomas Richter, linux-rt-users, linux-perf-users
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>
Acked-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
---
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 7e582547ac07..a68141d360b0 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;
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v3 4/5] perf-probe: Find versioned symbols from map
2017-12-08 16:26 [PATCH v3 0/5] perf-probe: Improve probing on versioned symbols Masami Hiramatsu
` (2 preceding siblings ...)
2017-12-08 16:27 ` [PATCH v3 3/5] perf-probe: Add __return suffix for return events Masami Hiramatsu
@ 2017-12-08 16:28 ` Masami Hiramatsu
2017-12-08 16:28 ` [PATCH v3 5/5] perf-probe: Support escaped character in parser Masami Hiramatsu
4 siblings, 0 replies; 9+ messages in thread
From: Masami Hiramatsu @ 2017-12-08 16:28 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Masami Hiramatsu, bhargavb, linux-kernel, Paul Clarke,
Ravi Bangoria, Thomas Richter, linux-rt-users, linux-perf-users
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>
Reviewed-by: Thomas Richter <tmricht@linux.vnet.ibm.com>
Acked-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
---
Changes in v3:
- Fix to return -ENOMEM if strndup fails.
- Fix to check name != NULL before refering it in
arch__normalize_symbol_name() for ppc.
---
tools/perf/arch/powerpc/util/sym-handling.c | 8 ++++++++
tools/perf/util/probe-event.c | 20 ++++++++++++++++++--
tools/perf/util/symbol.c | 5 +++++
tools/perf/util/symbol.h | 1 +
4 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/tools/perf/arch/powerpc/util/sym-handling.c b/tools/perf/arch/powerpc/util/sym-handling.c
index 9c4e23d8c8ce..53d83d7e6a09 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 == '.')
+ 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 a68141d360b0..0d6c66d51939 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2801,16 +2801,32 @@ 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);
+ if (!buf)
+ return -ENOMEM;
+ norm = buf;
+ }
+ if (strglobmatch(norm, name)) {
found++;
if (syms && found < probe_conf.max_probes)
syms[found - 1] = sym;
}
+ if (buf)
+ zfree(&buf);
}
return found;
@@ -2856,7 +2872,7 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
* same name but different addresses, this lists all the symbols.
*/
num_matched_functions = find_probe_functions(map, pp->function, syms);
- if (num_matched_functions == 0) {
+ if (num_matched_functions <= 0) {
pr_err("Failed to find symbol %s in %s\n", pp->function,
pev->target ? : "kernel");
ret = -ENOENT;
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v3 5/5] perf-probe: Support escaped character in parser
2017-12-08 16:26 [PATCH v3 0/5] perf-probe: Improve probing on versioned symbols Masami Hiramatsu
` (3 preceding siblings ...)
2017-12-08 16:28 ` [PATCH v3 4/5] perf-probe: Find versioned symbols from map Masami Hiramatsu
@ 2017-12-08 16:28 ` Masami Hiramatsu
2017-12-11 20:03 ` Arnaldo Carvalho de Melo
4 siblings, 1 reply; 9+ messages in thread
From: Masami Hiramatsu @ 2017-12-08 16:28 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Masami Hiramatsu, bhargavb, linux-kernel, Paul Clarke,
Ravi Bangoria, Thomas Richter, linux-rt-users, linux-perf-users
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>
Reviewed-by: Thomas Richter <tmricht@linux.vnet.ibm.com>
Acked-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
---
Changes in v2:
- Add a description and samples in Documentation/perf-probe.txt
Changes in v3:
- Update for new series.
---
tools/perf/Documentation/perf-probe.txt | 16 +++++++++
tools/perf/util/probe-event.c | 57 ++++++++++++++++++-------------
tools/perf/util/string.c | 46 +++++++++++++++++++++++++
tools/perf/util/string2.h | 2 +
4 files changed, 98 insertions(+), 23 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 0d6c66d51939..735972e7bc6b 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,23 +2807,30 @@ 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);
- if (!buf)
- return -ENOMEM;
- norm = buf;
+ if (cut_version) {
+ /* We don't care about default symbol or not */
+ ver = strchr(norm, '@');
+ if (ver) {
+ buf = strndup(norm, ver - norm);
+ if (!buf)
+ return -ENOMEM;
+ 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 */
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v3 5/5] perf-probe: Support escaped character in parser
2017-12-08 16:28 ` [PATCH v3 5/5] perf-probe: Support escaped character in parser Masami Hiramatsu
@ 2017-12-11 20:03 ` Arnaldo Carvalho de Melo
2017-12-12 14:46 ` Masami Hiramatsu
0 siblings, 1 reply; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-12-11 20:03 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: bhargavb, linux-kernel, Paul Clarke, Ravi Bangoria,
Thomas Richter, linux-rt-users, linux-perf-users
Em Sat, Dec 09, 2017 at 01:28:41AM +0900, Masami Hiramatsu escreveu:
> 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>
> Reviewed-by: Thomas Richter <tmricht@linux.vnet.ibm.com>
> Acked-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> ---
> Changes in v2:
> - Add a description and samples in Documentation/perf-probe.txt
> Changes in v3:
> - Update for new series.
> ---
> tools/perf/Documentation/perf-probe.txt | 16 +++++++++
> tools/perf/util/probe-event.c | 57 ++++++++++++++++++-------------
> tools/perf/util/string.c | 46 +++++++++++++++++++++++++
> tools/perf/util/string2.h | 2 +
> 4 files changed, 98 insertions(+), 23 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 0d6c66d51939..735972e7bc6b 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,23 +2807,30 @@ 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);
> - if (!buf)
> - return -ENOMEM;
> - norm = buf;
> + if (cut_version) {
> + /* We don't care about default symbol or not */
> + ver = strchr(norm, '@');
> + if (ver) {
> + buf = strndup(norm, ver - norm);
> + if (!buf)
> + return -ENOMEM;
> + norm = buf;
You forgot a } here, please check this logic and resubmit just this last
patch, without the string.c and string2.h part, that I already split
from this one and applied.
gcc diagnostics:
LD /tmp/build/perf/perf-in.o
util/probe-event.c: In function ‘find_probe_functions’:
util/probe-event.c:2848:17: error: declaration of ‘map’ shadows a parameter [-Werror=shadow]
struct map *map __maybe_unused,
^~~
util/probe-event.c:2802:45: note: shadowed declaration is here
static int find_probe_functions(struct map *map, char *name,
^~~
util/probe-event.c:2849:20: error: declaration of ‘sym’ shadows a previous local [-Werror=shadow]
struct symbol *sym __maybe_unused) { }
------------------------
clang's:
CC /tmp/build/perf/util/probe-event.o
util/probe-event.c:2849:40: error: function definition is not allowed here
struct symbol *sym __maybe_unused) { }
^
util/probe-event.c:2857:1: error: function definition is not allowed here
{
^
util/probe-event.c:3009:1: error: function definition is not allowed here
{
^
util/probe-event.c:3098:1: error: function definition is not allowed here
{
^
util/probe-event.c:3112:1: error: function definition is not allowed here
------------------------
Neither are that informative... ;-\
- Arnaldo
> }
> +
> 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 */
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v3 5/5] perf-probe: Support escaped character in parser
2017-12-11 20:03 ` Arnaldo Carvalho de Melo
@ 2017-12-12 14:46 ` Masami Hiramatsu
2017-12-12 15:01 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 9+ messages in thread
From: Masami Hiramatsu @ 2017-12-12 14:46 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: bhargavb, linux-kernel, Paul Clarke, Ravi Bangoria,
Thomas Richter, linux-rt-users, linux-perf-users
On Mon, 11 Dec 2017 17:03:30 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> Em Sat, Dec 09, 2017 at 01:28:41AM +0900, Masami Hiramatsu escreveu:
> > 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>
> > Reviewed-by: Thomas Richter <tmricht@linux.vnet.ibm.com>
> > Acked-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> > ---
> > Changes in v2:
> > - Add a description and samples in Documentation/perf-probe.txt
> > Changes in v3:
> > - Update for new series.
> > ---
> > tools/perf/Documentation/perf-probe.txt | 16 +++++++++
> > tools/perf/util/probe-event.c | 57 ++++++++++++++++++-------------
> > tools/perf/util/string.c | 46 +++++++++++++++++++++++++
> > tools/perf/util/string2.h | 2 +
> > 4 files changed, 98 insertions(+), 23 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 0d6c66d51939..735972e7bc6b 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,23 +2807,30 @@ 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);
> > - if (!buf)
> > - return -ENOMEM;
> > - norm = buf;
> > + if (cut_version) {
> > + /* We don't care about default symbol or not */
> > + ver = strchr(norm, '@');
> > + if (ver) {
> > + buf = strndup(norm, ver - norm);
> > + if (!buf)
> > + return -ENOMEM;
> > + norm = buf;
>
> You forgot a } here, please check this logic and resubmit just this last
> patch, without the string.c and string2.h part, that I already split
> from this one and applied.
OOPS! thanks! I missed something around that.... maybe while updating patches.
Hmm, I might be so upset...
OK, anyway, I'll do that.
>
> gcc diagnostics:
>
> LD /tmp/build/perf/perf-in.o
> util/probe-event.c: In function ‘find_probe_functions’:
> util/probe-event.c:2848:17: error: declaration of ‘map’ shadows a parameter [-Werror=shadow]
> struct map *map __maybe_unused,
> ^~~
> util/probe-event.c:2802:45: note: shadowed declaration is here
> static int find_probe_functions(struct map *map, char *name,
> ^~~
> util/probe-event.c:2849:20: error: declaration of ‘sym’ shadows a previous local [-Werror=shadow]
> struct symbol *sym __maybe_unused) { }
>
> ------------------------
>
> clang's:
>
> CC /tmp/build/perf/util/probe-event.o
> util/probe-event.c:2849:40: error: function definition is not allowed here
> struct symbol *sym __maybe_unused) { }
> ^
> util/probe-event.c:2857:1: error: function definition is not allowed here
> {
> ^
> util/probe-event.c:3009:1: error: function definition is not allowed here
> {
> ^
> util/probe-event.c:3098:1: error: function definition is not allowed here
> {
> ^
> util/probe-event.c:3112:1: error: function definition is not allowed here
>
> ------------------------
>
> Neither are that informative... ;-\
Sorry, that's my mistake...
Thank you,
>
> - Arnaldo
>
> > }
> > +
> > 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 */
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v3 5/5] perf-probe: Support escaped character in parser
2017-12-12 14:46 ` Masami Hiramatsu
@ 2017-12-12 15:01 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-12-12 15:01 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: bhargavb, linux-kernel, Paul Clarke, Ravi Bangoria,
Thomas Richter, linux-rt-users, linux-perf-users
Em Tue, Dec 12, 2017 at 11:46:00PM +0900, Masami Hiramatsu escreveu:
> On Mon, 11 Dec 2017 17:03:30 -0300
> Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > > - /* We don't care about default symbol or not */
> > > - ver = strchr(norm, '@');
> > > - if (ver) {
> > > - buf = strndup(norm, ver - norm);
> > > - if (!buf)
> > > - return -ENOMEM;
> > > - norm = buf;
> > > + if (cut_version) {
> > > + /* We don't care about default symbol or not */
> > > + ver = strchr(norm, '@');
> > > + if (ver) {
> > > + buf = strndup(norm, ver - norm);
> > > + if (!buf)
> > > + return -ENOMEM;
> > > + norm = buf;
> > You forgot a } here, please check this logic and resubmit just this last
> > patch, without the string.c and string2.h part, that I already split
> > from this one and applied.
> OOPS! thanks! I missed something around that.... maybe while updating patches.
> Hmm, I might be so upset...
> OK, anyway, I'll do that.
Nah, happens sometimes, one last round of building before submitting
would catch that tho... 8-) :-)
- Arnaldo
^ permalink raw reply [flat|nested] 9+ messages in thread