All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4] perf-probe: Support escaped character in parser
@ 2017-12-12 15:05 Masami Hiramatsu
  2017-12-12 15:27 ` Arnaldo Carvalho de Melo
  2017-12-28 15:32 ` [tip:perf/core] perf probe: " tip-bot for Masami Hiramatsu
  0 siblings, 2 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2017-12-12 15:05 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.
 Changes in v4:
  - Fix a build error (Thanks for Arnaldo!)
    This time, I ensured that I ran "make build-test" and it passed.
---
 tools/perf/Documentation/perf-probe.txt |   16 +++++++++
 tools/perf/util/probe-event.c           |   58 +++++++++++++++++++------------
 2 files changed, 51 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..e1dbc9821617 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,31 @@ 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)

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v4] perf-probe: Support escaped character in parser
  2017-12-12 15:05 [PATCH v4] perf-probe: Support escaped character in parser Masami Hiramatsu
@ 2017-12-12 15:27 ` Arnaldo Carvalho de Melo
  2017-12-13 13:40   ` Masami Hiramatsu
  2017-12-28 15:32 ` [tip:perf/core] perf probe: " tip-bot for Masami Hiramatsu
  1 sibling, 1 reply; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-12-12 15:27 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: bhargavb, linux-kernel, Paul Clarke, Ravi Bangoria,
	Thomas Richter, linux-rt-users, linux-perf-users

Em Wed, Dec 13, 2017 at 12:05:12AM +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.

<SNIP>
 
>  Changes in v4:
>   - Fix a build error (Thanks for Arnaldo!)
>     This time, I ensured that I ran "make build-test" and it passed.

Thanks, testing it I still have that artifact:

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

[root@jouet perf]# 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

[root@jouet perf]# perf probe -l
Failed to find debug information for address dd38eca7
  probe_libc:malloc_get_state (on 0xdd38eca7 in /usr/lib64/libc-2.25.so)
[root@jouet perf]#

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

I mean the "on 0xdd38eca7" part:


That failed to find debug information part:

[root@jouet perf]# perf probe -vv -l
Add filter: *
Opening /sys/kernel/debug/tracing//kprobe_events write=0
Opening /sys/kernel/debug/tracing//uprobe_events write=0
Parsing probe_events: p:probe_libc/malloc_get_state /usr/lib64/libc-2.25.so:0x00000000dd38eca7
Group:probe_libc Event:malloc_get_state probe:p
try to find information at dd38eca7 in /usr/lib64/libc-2.25.so
Open Debuginfo file: /usr/lib/debug/usr/lib64/libc-2.25.so.debug
Failed to find debug information for address dd38eca7
Failed to find corresponding probes from debuginfo.
symsrc__init: build id mismatch for /usr/lib/debug/usr/lib64/libc-2.25.so.debug.
Failed to find probe point from both of dwarf and map.
  probe_libc:malloc_get_state (on 0xdd38eca7 in /usr/lib64/libc-2.25.so)
[root@jouet perf]#

Ok, so build-id mismatch, lets see:

[root@jouet perf]# rpm -q glibc glibc-debuginfo
glibc-2.25-10.fc26.x86_64
glibc-2.25-10.fc26.i686
glibc-debuginfo-2.25-12.fc26.x86_64
[root@jouet perf]#

Ok, the debuginfo file is newer than the glibc installed, updating glibc
now...

We could have a more informative message in this case, right? I.e.
something like:

[root@jouet perf]# perf probe -l
There was a build-id mismatch while trying to resolve 0xdd38eca7, please
check your system's debuginfo files for mismatches, e.g. check the
versions for glibc and glibc-debuginfo.
  probe_libc:malloc_get_state (on 0xdd38eca7 in /usr/lib64/libc-2.25.so)
[root@jouet perf]#

- Arnaldo

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4] perf-probe: Support escaped character in parser
  2017-12-12 15:27 ` Arnaldo Carvalho de Melo
@ 2017-12-13 13:40   ` Masami Hiramatsu
  2017-12-13 15:42     ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 6+ messages in thread
From: Masami Hiramatsu @ 2017-12-13 13:40 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 Tue, 12 Dec 2017 12:27:24 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Wed, Dec 13, 2017 at 12:05:12AM +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.
> 
> <SNIP>
>  
> >  Changes in v4:
> >   - Fix a build error (Thanks for Arnaldo!)
> >     This time, I ensured that I ran "make build-test" and it passed.
> 
> Thanks, testing it I still have that artifact:
> 
>  ---------------------------------------
> 
> [root@jouet perf]# 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
> 
> [root@jouet perf]# perf probe -l
> Failed to find debug information for address dd38eca7
>   probe_libc:malloc_get_state (on 0xdd38eca7 in /usr/lib64/libc-2.25.so)
> [root@jouet perf]#
> 
>  ---------------------------------------
> 
> I mean the "on 0xdd38eca7" part:
> 
> 
> That failed to find debug information part:
> 
> [root@jouet perf]# perf probe -vv -l
> Add filter: *
> Opening /sys/kernel/debug/tracing//kprobe_events write=0
> Opening /sys/kernel/debug/tracing//uprobe_events write=0
> Parsing probe_events: p:probe_libc/malloc_get_state /usr/lib64/libc-2.25.so:0x00000000dd38eca7
> Group:probe_libc Event:malloc_get_state probe:p
> try to find information at dd38eca7 in /usr/lib64/libc-2.25.so
> Open Debuginfo file: /usr/lib/debug/usr/lib64/libc-2.25.so.debug
> Failed to find debug information for address dd38eca7
> Failed to find corresponding probes from debuginfo.
> symsrc__init: build id mismatch for /usr/lib/debug/usr/lib64/libc-2.25.so.debug.
> Failed to find probe point from both of dwarf and map.
>   probe_libc:malloc_get_state (on 0xdd38eca7 in /usr/lib64/libc-2.25.so)
> [root@jouet perf]#

Ah, I got it. So symsrc__init checks debugfile to get symbols,
but failed.

> 
> Ok, so build-id mismatch, lets see:
> 
> [root@jouet perf]# rpm -q glibc glibc-debuginfo
> glibc-2.25-10.fc26.x86_64
> glibc-2.25-10.fc26.i686
> glibc-debuginfo-2.25-12.fc26.x86_64
> [root@jouet perf]#
> 
> Ok, the debuginfo file is newer than the glibc installed, updating glibc
> now...
> 
> We could have a more informative message in this case, right? I.e.
> something like:
> 
> [root@jouet perf]# perf probe -l
> There was a build-id mismatch while trying to resolve 0xdd38eca7, please
> check your system's debuginfo files for mismatches, e.g. check the
> versions for glibc and glibc-debuginfo.
>   probe_libc:malloc_get_state (on 0xdd38eca7 in /usr/lib64/libc-2.25.so)

OK, I'll try to check how debuginfo is searched in such environment.

Thank you,

> [root@jouet perf]#
> 
> - Arnaldo


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4] perf-probe: Support escaped character in parser
  2017-12-13 13:40   ` Masami Hiramatsu
@ 2017-12-13 15:42     ` Arnaldo Carvalho de Melo
  2017-12-16 14:52       ` Masami Hiramatsu
  0 siblings, 1 reply; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-12-13 15:42 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: bhargavb, linux-kernel, Paul Clarke, Ravi Bangoria,
	Thomas Richter, linux-rt-users, linux-perf-users

Em Wed, Dec 13, 2017 at 10:40:24PM +0900, Masami Hiramatsu escreveu:
> On Tue, 12 Dec 2017 12:27:24 -0300
> Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> 
> > Em Wed, Dec 13, 2017 at 12:05:12AM +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.
> > 
> > <SNIP>
> >  
> > >  Changes in v4:
> > >   - Fix a build error (Thanks for Arnaldo!)
> > >     This time, I ensured that I ran "make build-test" and it passed.
> > 
> > Thanks, testing it I still have that artifact:
> > 
> >  ---------------------------------------
> > 
> > [root@jouet perf]# 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
> > 
> > [root@jouet perf]# perf probe -l
> > Failed to find debug information for address dd38eca7
> >   probe_libc:malloc_get_state (on 0xdd38eca7 in /usr/lib64/libc-2.25.so)
> > [root@jouet perf]#
> > 
> >  ---------------------------------------
> > 
> > I mean the "on 0xdd38eca7" part:
> > 
> > 
> > That failed to find debug information part:
> > 
> > [root@jouet perf]# perf probe -vv -l
> > Add filter: *
> > Opening /sys/kernel/debug/tracing//kprobe_events write=0
> > Opening /sys/kernel/debug/tracing//uprobe_events write=0
> > Parsing probe_events: p:probe_libc/malloc_get_state /usr/lib64/libc-2.25.so:0x00000000dd38eca7
> > Group:probe_libc Event:malloc_get_state probe:p
> > try to find information at dd38eca7 in /usr/lib64/libc-2.25.so
> > Open Debuginfo file: /usr/lib/debug/usr/lib64/libc-2.25.so.debug
> > Failed to find debug information for address dd38eca7
> > Failed to find corresponding probes from debuginfo.
> > symsrc__init: build id mismatch for /usr/lib/debug/usr/lib64/libc-2.25.so.debug.
> > Failed to find probe point from both of dwarf and map.
> >   probe_libc:malloc_get_state (on 0xdd38eca7 in /usr/lib64/libc-2.25.so)
> > [root@jouet perf]#
> 
> Ah, I got it. So symsrc__init checks debugfile to get symbols,
> but failed.
> 
> > 
> > Ok, so build-id mismatch, lets see:
> > 
> > [root@jouet perf]# rpm -q glibc glibc-debuginfo
> > glibc-2.25-10.fc26.x86_64
> > glibc-2.25-10.fc26.i686
> > glibc-debuginfo-2.25-12.fc26.x86_64
> > [root@jouet perf]#
> > 
> > Ok, the debuginfo file is newer than the glibc installed, updating glibc
> > now...
> > 
> > We could have a more informative message in this case, right? I.e.
> > something like:
> > 
> > [root@jouet perf]# perf probe -l
> > There was a build-id mismatch while trying to resolve 0xdd38eca7, please
> > check your system's debuginfo files for mismatches, e.g. check the
> > versions for glibc and glibc-debuginfo.
> >   probe_libc:malloc_get_state (on 0xdd38eca7 in /usr/lib64/libc-2.25.so)
> 
> OK, I'll try to check how debuginfo is searched in such environment.

So, tools/perf/util/symbol.c, function dso__load(), it will have this
loop:


        /*
         * Read the build id if possible. This is required for
         * DSO_BINARY_TYPE__BUILDID_DEBUGINFO to work
         */
        if (!dso->has_build_id && is_regular_file(dso->long_name)) {
            __symbol__join_symfs(name, PATH_MAX, dso->long_name);
            if (filename__read_build_id(name, build_id, BUILD_ID_SIZE) > 0)
                dso__set_build_id(dso, build_id);
        }

        /*
         * Iterate over candidate debug images.
         * Keep track of "interesting" ones (those which have a symtab, dynsym,
         * and/or opd section) for processing.
         */
        for (i = 0; i < DSO_BINARY_TYPE__SYMTAB_CNT; i++) {

                if (dso__read_binary_type_filename(dso, symtab_type, root_dir, name, PATH_MAX))


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

So the glibc file _has_ a build id, it reads it and then will try to
find a file that has that build id, that 'name' variable will have
the file being tried, which may be, for instance, at:

[acme@jouet ~]$ perf buildid-list -i /lib64/libc-2.25.so 
7a4787e7c1263dc25461a7b2f2abd2acaa186102
[acme@jouet ~]$ ls -la /usr/lib/debug/.build-id/7a/4787e7c1263dc25461a7b2f2abd2acaa186102
lrwxrwxrwx. 1 root root 33 Oct 11 09:49 /usr/lib/debug/.build-id/7a/4787e7c1263dc25461a7b2f2abd2acaa186102 -> ../../../../../lib64/libc-2.25.so
[acme@jouet ~]$ 

etc.

So perhaps we can have a routine that does just that loop and prints the
files it finds with the debuginfo they have to help tell which files
where considered?

But perhaps the message I suggested may be good enough?

- Arnaldo

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4] perf-probe: Support escaped character in parser
  2017-12-13 15:42     ` Arnaldo Carvalho de Melo
@ 2017-12-16 14:52       ` Masami Hiramatsu
  0 siblings, 0 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2017-12-16 14:52 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 Wed, 13 Dec 2017 12:42:27 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Wed, Dec 13, 2017 at 10:40:24PM +0900, Masami Hiramatsu escreveu:
> > On Tue, 12 Dec 2017 12:27:24 -0300
> > Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > 
> > > Em Wed, Dec 13, 2017 at 12:05:12AM +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.
> > > 
> > > <SNIP>
> > >  
> > > >  Changes in v4:
> > > >   - Fix a build error (Thanks for Arnaldo!)
> > > >     This time, I ensured that I ran "make build-test" and it passed.
> > > 
> > > Thanks, testing it I still have that artifact:
> > > 
> > >  ---------------------------------------
> > > 
> > > [root@jouet perf]# 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
> > > 
> > > [root@jouet perf]# perf probe -l
> > > Failed to find debug information for address dd38eca7
> > >   probe_libc:malloc_get_state (on 0xdd38eca7 in /usr/lib64/libc-2.25.so)
> > > [root@jouet perf]#
> > > 
> > >  ---------------------------------------
> > > 
> > > I mean the "on 0xdd38eca7" part:
> > > 
> > > 
> > > That failed to find debug information part:
> > > 
> > > [root@jouet perf]# perf probe -vv -l
> > > Add filter: *
> > > Opening /sys/kernel/debug/tracing//kprobe_events write=0
> > > Opening /sys/kernel/debug/tracing//uprobe_events write=0
> > > Parsing probe_events: p:probe_libc/malloc_get_state /usr/lib64/libc-2.25.so:0x00000000dd38eca7
> > > Group:probe_libc Event:malloc_get_state probe:p
> > > try to find information at dd38eca7 in /usr/lib64/libc-2.25.so
> > > Open Debuginfo file: /usr/lib/debug/usr/lib64/libc-2.25.so.debug
> > > Failed to find debug information for address dd38eca7
> > > Failed to find corresponding probes from debuginfo.
> > > symsrc__init: build id mismatch for /usr/lib/debug/usr/lib64/libc-2.25.so.debug.
> > > Failed to find probe point from both of dwarf and map.
> > >   probe_libc:malloc_get_state (on 0xdd38eca7 in /usr/lib64/libc-2.25.so)
> > > [root@jouet perf]#
> > 
> > Ah, I got it. So symsrc__init checks debugfile to get symbols,
> > but failed.
> > 
> > > 
> > > Ok, so build-id mismatch, lets see:
> > > 
> > > [root@jouet perf]# rpm -q glibc glibc-debuginfo
> > > glibc-2.25-10.fc26.x86_64
> > > glibc-2.25-10.fc26.i686
> > > glibc-debuginfo-2.25-12.fc26.x86_64
> > > [root@jouet perf]#
> > > 
> > > Ok, the debuginfo file is newer than the glibc installed, updating glibc
> > > now...
> > > 
> > > We could have a more informative message in this case, right? I.e.
> > > something like:
> > > 
> > > [root@jouet perf]# perf probe -l
> > > There was a build-id mismatch while trying to resolve 0xdd38eca7, please
> > > check your system's debuginfo files for mismatches, e.g. check the
> > > versions for glibc and glibc-debuginfo.
> > >   probe_libc:malloc_get_state (on 0xdd38eca7 in /usr/lib64/libc-2.25.so)
> > 
> > OK, I'll try to check how debuginfo is searched in such environment.
> 
> So, tools/perf/util/symbol.c, function dso__load(), it will have this
> loop:
> 
> 
>         /*
>          * Read the build id if possible. This is required for
>          * DSO_BINARY_TYPE__BUILDID_DEBUGINFO to work
>          */
>         if (!dso->has_build_id && is_regular_file(dso->long_name)) {
>             __symbol__join_symfs(name, PATH_MAX, dso->long_name);
>             if (filename__read_build_id(name, build_id, BUILD_ID_SIZE) > 0)
>                 dso__set_build_id(dso, build_id);
>         }
> 
>         /*
>          * Iterate over candidate debug images.
>          * Keep track of "interesting" ones (those which have a symtab, dynsym,
>          * and/or opd section) for processing.
>          */
>         for (i = 0; i < DSO_BINARY_TYPE__SYMTAB_CNT; i++) {
> 
>                 if (dso__read_binary_type_filename(dso, symtab_type, root_dir, name, PATH_MAX))
> 
> 
> -------------------
> 
> So the glibc file _has_ a build id, it reads it and then will try to
> find a file that has that build id, that 'name' variable will have
> the file being tried, which may be, for instance, at:
> 
> [acme@jouet ~]$ perf buildid-list -i /lib64/libc-2.25.so 
> 7a4787e7c1263dc25461a7b2f2abd2acaa186102
> [acme@jouet ~]$ ls -la /usr/lib/debug/.build-id/7a/4787e7c1263dc25461a7b2f2abd2acaa186102
> lrwxrwxrwx. 1 root root 33 Oct 11 09:49 /usr/lib/debug/.build-id/7a/4787e7c1263dc25461a7b2f2abd2acaa186102 -> ../../../../../lib64/libc-2.25.so
> [acme@jouet ~]$ 
> 
> etc.
> 
> So perhaps we can have a routine that does just that loop and prints the
> files it finds with the debuginfo they have to help tell which files
> where considered?

Hmm, as far as I can see, this occured in debuginfo__new(), 

struct debuginfo *debuginfo__new(const char *path)
{
        enum dso_binary_type *type;
        char buf[PATH_MAX], nil = '\0';
        struct dso *dso;
        struct debuginfo *dinfo = NULL;

        /* Try to open distro debuginfo files */
        dso = dso__new(path);
        if (!dso)
                goto out;

        for (type = distro_dwarf_types;
             !dinfo && *type != DSO_BINARY_TYPE__NOT_FOUND;
             type++) {
                if (dso__read_binary_type_filename(dso, *type, &nil,
                                                   buf, PATH_MAX) < 0)
                        continue;
                dinfo = __debuginfo__new(buf);
        }
        dso__put(dso);

out:
        /* if failed to open all distro debuginfo, open given binary */
        return dinfo ? : __debuginfo__new(path);
}

And dso__read_binary_type_filename() makes a debuginfo path
from target path but doesn't check build-id. So,
(I still can not reproduce your situation but) I guess,
dso__read_binary_type_filename() returns some path, but
__debuginfo__new() just failed to open it.
Or, __debuginfo_new() opened it but since its buildid is
different, it really failed to find any lines on given address.

> 
> But perhaps the message I suggested may be good enough?

Yeah, but this means we need to check the buildid of both
binaries.

Thank you,

> 
> - Arnaldo


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [tip:perf/core] perf probe: Support escaped character in parser
  2017-12-12 15:05 [PATCH v4] perf-probe: Support escaped character in parser Masami Hiramatsu
  2017-12-12 15:27 ` Arnaldo Carvalho de Melo
@ 2017-12-28 15:32 ` tip-bot for Masami Hiramatsu
  1 sibling, 0 replies; 6+ messages in thread
From: tip-bot for Masami Hiramatsu @ 2017-12-28 15:32 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: bhargavaramudu, linux-kernel, mingo, acme, pc, tglx, mhiramat,
	ravi.bangoria, hpa, tmricht

Commit-ID:  c588d158124d5b60184fc612e551a19720720d68
Gitweb:     https://git.kernel.org/tip/c588d158124d5b60184fc612e551a19720720d68
Author:     Masami Hiramatsu <mhiramat@kernel.org>
AuthorDate: Wed, 13 Dec 2017 00:05:12 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 27 Dec 2017 12:15:55 -0300

perf probe: Support escaped character in parser

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>
Cc: Paul Clarke <pc@us.ibm.com>
Cc: bhargavb <bhargavaramudu@gmail.com>
Cc: linux-rt-users@vger.kernel.org
Link: http://lkml.kernel.org/r/151309111236.18107.5634753157435343410.stgit@devbox
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Documentation/perf-probe.txt | 16 +++++++++
 tools/perf/util/probe-event.c           | 58 ++++++++++++++++++++-------------
 2 files changed, 51 insertions(+), 23 deletions(-)

diff --git a/tools/perf/Documentation/perf-probe.txt b/tools/perf/Documentation/perf-probe.txt
index f963826..b6866a0 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 0d6c66d..e1dbc98 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,31 @@ 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)

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-12-28 15:35 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-12 15:05 [PATCH v4] perf-probe: Support escaped character in parser Masami Hiramatsu
2017-12-12 15:27 ` Arnaldo Carvalho de Melo
2017-12-13 13:40   ` Masami Hiramatsu
2017-12-13 15:42     ` Arnaldo Carvalho de Melo
2017-12-16 14:52       ` Masami Hiramatsu
2017-12-28 15:32 ` [tip:perf/core] perf probe: " tip-bot for Masami Hiramatsu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.