Skip to contentSkip to conversation

perf-probe: Improve probing on versioned symbols

PATCH In review Masami Hiramatsu 3 participants 13 messages Dec 8, 2017 – Dec 28, 2017 perf events

lore
Share & tools
lore
#0 lore raw

v3 cover perf-probe: Improve probing on versioned symbols

Hi,

Here is the 3rd 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 (and detail note) is here;
https://lkml.org/lkml/2017/12/5/1124

Here is the updates;

[1/5] Update error message. Arnaldo, I dropped the latter
half from error message because it doesn't try to write the name to the kprobe_events interface.
[4/5] Fix 2 bugs (Thanks Thomas-Mich Richter!) [5/5] Update according to previous patch.

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               |   84 +++++++++++++++++++--------
 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, 139 insertions(+), 25 deletions(-)

--
Masami Hiramatsu (Linaro Ltd.) <[email protected]>

#1 lore raw

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

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.
   ======

---
Changes in v3:
- Update the error message.

---
 tools/perf/util/probe-event.c |    8 ++++++++
 1 file changed, 8 insertions(+)
tools/perf/util/probe-event.c +8 -0
index b7aaf9b2294d..262d5da86623 100644
@@ -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;
 }
 
#2 lore raw

[tip:perf/core] perf probe: Add warning message if there is unexpected event name

perf probe: Add warning message if there is unexpected event name
Commit-ID:  9f5c6d8777a2d962b0eeacb2a16f37da6bea545b
Gitweb:     https://git.kernel.org/tip/9f5c6d8777a2d962b0eeacb2a16f37da6bea545b
Author:     Masami Hiramatsu <[email protected]>
AuthorDate: Sat, 9 Dec 2017 01:26:46 +0900
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 27 Dec 2017 12:15:53 -0300

perf probe: Add warning message if there is unexpected event name

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.
   ======
---
 tools/perf/util/probe-event.c | 8 ++++++++
 1 file changed, 8 insertions(+)
tools/perf/util/probe-event.c +8 -0
index b7aaf9b..262d5da 100644
@@ -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;
 }
 
#3 lore raw

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

In reply to 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

=====

---
 tools/perf/util/probe-event.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
tools/perf/util/probe-event.c +2 -2
index 262d5da86623..7e582547ac07 100644
@@ -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';
 
#4 lore raw

PATCH v3 3/5 perf-probe: Add __return suffix for return events

In reply to 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

=====

---
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(-)
tools/perf/Documentation/perf-probe.txt +1 -1
index d7e4869905f1..f96382692f42 100644
@@ -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.
tools/perf/util/probe-event.c +5 -4
index 7e582547ac07..a68141d360b0 100644
@@ -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;
 
#5 lore raw

[tip:perf/core] perf probe: Add __return suffix for return events

perf probe: Add __return suffix for return events
Commit-ID:  e63c625a1e417edbe513b75b347a7238e9e7fea0
Gitweb:     https://git.kernel.org/tip/e63c625a1e417edbe513b75b347a7238e9e7fea0
Author:     Masami Hiramatsu <[email protected]>
AuthorDate: Sat, 9 Dec 2017 01:27:44 +0900
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 27 Dec 2017 12:15:54 -0300

perf probe: Add __return suffix for return events

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

=====

---
 tools/perf/Documentation/perf-probe.txt | 2 +-
 tools/perf/util/probe-event.c           | 9 +++++----
 2 files changed, 6 insertions(+), 5 deletions(-)
tools/perf/Documentation/perf-probe.txt +1 -1
index d7e4869..f963826 100644
@@ -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.
tools/perf/util/probe-event.c +5 -4
index 7e58254..a68141d 100644
@@ -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;
 
#6 lore raw

PATCH v3 4/5 perf-probe: Find versioned symbols from map

In reply to 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)
  =====

---
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(-)
tools/perf/arch/powerpc/util/sym-handling.c +8 -0
index 9c4e23d8c8ce..53d83d7e6a09 100644
@@ -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
tools/perf/util/probe-event.c +18 -2
index a68141d360b0..0d6c66d51939 100644
@@ -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;
tools/perf/util/symbol.c +5 -0
index 1b67a8639dfe..cc065d4bfafc 100644
@@ -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);
tools/perf/util/symbol.h +1 -0
index a4f0075b4e5c..0563f33c1eb3 100644
@@ -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
 
#7 lore raw

[tip:perf/core] perf probe: Find versioned symbols from map

perf probe: Find versioned symbols from map
Commit-ID:  4b3a2716dd785fabb9f6ac80c1d53cb29a88169d
Gitweb:     https://git.kernel.org/tip/4b3a2716dd785fabb9f6ac80c1d53cb29a88169d
Author:     Masami Hiramatsu <[email protected]>
AuthorDate: Sat, 9 Dec 2017 01:28:12 +0900
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 27 Dec 2017 12:15:54 -0300

perf probe: Find versioned symbols 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)
  =====
---
 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(-)
tools/perf/arch/powerpc/util/sym-handling.c +8 -0
index 9c4e23d..53d83d7 100644
@@ -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
tools/perf/util/probe-event.c +18 -2
index a68141d..0d6c66d 100644
@@ -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;
tools/perf/util/symbol.c +5 -0
index 1b67a86..cc065d4 100644
@@ -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);
tools/perf/util/symbol.h +1 -0
index a4f0075..0563f33 100644
@@ -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
 
#8 lore raw

PATCH v3 5/5 perf-probe: Support escaped character in parser

In reply to 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.

---
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(-)
tools/perf/Documentation/perf-probe.txt +16 -0
index f96382692f42..b6866a05edd2 100644
@@ -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
 --------
tools/perf/util/probe-event.c +34 -23

137 lines not on this page · raw message

tools/perf/util/string.c +46 -0

49 lines not on this page · raw message

tools/perf/util/string2.h +2 -0
index ee14ca5451ab..4c68a09b97e8 100644
@@ -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 */
#9 lore raw

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

Em Sat, Dec 09, 2017 at 01:28:41AM +0900, Masami Hiramatsu escreveu:

Quoted 219 lines

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.

---
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(-)
tools/perf/Documentation/perf-probe.txt +16 -0
index f96382692f42..b6866a05edd2 100644
@@ -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
 --------
tools/perf/util/probe-event.c +25 -21
index 0d6c66d51939..735972e7bc6b 100644
@@ -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)

tools/perf/util/probe-event.c static int find_probe_functions(struct map *map, char *name, · line 2831

Quoted 10 lines
tools/perf/util/probe-event.c +8 -2
-				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

Quoted 71 lines
tools/perf/util/probe-event.c +1 -0
 		}
+
 		if (strglobmatch(norm, name)) {
 			found++;
 			if (syms && found < probe_conf.max_probes)
tools/perf/util/string.c +46 -0
index aaa08ee8c717..d8bfd0c4d2cb 100644
@@ -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;
+}
tools/perf/util/string2.h +2 -0
index ee14ca5451ab..4c68a09b97e8 100644
@@ -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 */
#10 lore raw

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

Quoted 226 lines · On Mon, 11 Dec 2017 17:03:30 -0300 Arnaldo Carvalho de Melo <[email protected]>…

Em Sat, Dec 09, 2017 at 01:28:41AM +0900, Masami Hiramatsu escreveu:

Quoted 225 lines

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.

---
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(-)
tools/perf/Documentation/perf-probe.txt +16 -0
index f96382692f42..b6866a05edd2 100644
@@ -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
 --------
tools/perf/util/probe-event.c +29 -23
index 0d6c66d51939..735972e7bc6b 100644
@@ -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) {
Quoted 8 lines

tools/perf/util/probe-event.c static int find_probe_functions(struct map *map, char *name, · line 2831

Quoted 4 lines
tools/perf/util/probe-event.c +4 -0
+				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.

Quoted 33 lines

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

Quoted 3 lines

------------------------

Neither are that informative... ;-\

Sorry, that's my mistake...

Thank you,

Quoted 74 lines

- Arnaldo

Quoted 71 lines
tools/perf/util/probe-event.c +1 -0
 		}
+
 		if (strglobmatch(norm, name)) {
 			found++;
 			if (syms && found < probe_conf.max_probes)
tools/perf/util/string.c +46 -0
index aaa08ee8c717..d8bfd0c4d2cb 100644
@@ -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;
+}
tools/perf/util/string2.h +2 -0
index ee14ca5451ab..4c68a09b97e8 100644
@@ -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 */
Signature · 1 line
Masami Hiramatsu <[email protected]>
#11 lore raw

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

Em Tue, Dec 12, 2017 at 11:46:00PM +0900, Masami Hiramatsu escreveu:

Quoted 22 lines
Quoted 11 lines · On Mon, 11 Dec 2017 17:03:30 -0300 Arnaldo Carvalho de Melo <[email protected]>…
Quoted 11 lines
tools/perf/util/probe-event.c +4 -7
-		/* 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) {
Quoted 8 lines

tools/perf/util/probe-event.c static int find_probe_functions(struct map *map, char *name, · line 2831

Quoted 4 lines
tools/perf/util/probe-event.c +4 -0
+				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.

Quoted 4 lines

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

#12 lore raw

[tip:perf/core] perf string: Add {strdup,strpbrk}_esc()

In reply to Masami Hiramatsu
perf string: Add {strdup,strpbrk}_esc()
Commit-ID:  1e9f9e8af0de80e8f6a47d991df66090934be0c6
Gitweb:     https://git.kernel.org/tip/1e9f9e8af0de80e8f6a47d991df66090934be0c6
Author:     Masami Hiramatsu <[email protected]>
AuthorDate: Sat, 9 Dec 2017 01:28:41 +0900
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 27 Dec 2017 12:15:55 -0300

perf string: Add {strdup,strpbrk}_esc()

To support the special characters escaped by '\' in 'perf probe' event parser.

---
 tools/perf/util/string.c  | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/string2.h |  2 ++
 2 files changed, 48 insertions(+)
tools/perf/util/string.c +46 -0
index aaa08ee..d8bfd0c 100644
@@ -396,3 +396,49 @@ out_err_overflow:
 	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;
+}
tools/perf/util/string2.h +2 -0
index ee14ca5..4c68a09 100644
@@ -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 */