linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] perf probe: Prevent segfault when reading probe point with absolute address
@ 2015-08-25 13:27 Wang Nan
  2015-08-25 13:27 ` [PATCH 2/2] perf probe: Support probing at " Wang Nan
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Wang Nan @ 2015-08-25 13:27 UTC (permalink / raw)
  To: acme; +Cc: mingo, masami.hiramatsu.pt, namhyung, pi3orama, linux-kernel

'perf probe -l' panic if there is a manually inserted probing point
with absolute address. For example:

 # echo 'p:probe/abs_ffffffff811e6615 0xffffffff811e6615' > /sys/kernel/debug/tracing/kprobe_events
 # perf probe -l
 Segmentation fault (core dumped)

This patch fix this problem by considering the situation that
"tp->symbol == NULL" in find_perf_probe_point_from_dwarf() and
find_perf_probe_point_from_map().

After this patch:

 # perf probe -l
 probe:abs_ffffffff811e6615 (on SyS_write+5@fs/read_write.c)

And when debug info is missing:

 # rm -rf ~/.debug
 # mv /lib/modules/4.2.0-rc1+/build/vmlinux /lib/modules/4.2.0-rc1+/build/vmlinux.bak
 # perf probe -l
 probe:abs_ffffffff811e6615 (on sys_write+5)

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/probe-event.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index f07374b..6c7e538 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -515,7 +515,7 @@ static int find_perf_probe_point_from_dwarf(struct probe_trace_point *tp,
 		if (ret < 0)
 			goto error;
 		addr += stext;
-	} else {
+	} else if (tp->symbol) {
 		addr = kernel_get_symbol_address_by_name(tp->symbol, false);
 		if (addr == 0)
 			goto error;
@@ -1815,17 +1815,17 @@ static int find_perf_probe_point_from_map(struct probe_trace_point *tp,
 {
 	struct symbol *sym = NULL;
 	struct map *map;
-	u64 addr;
+	u64 addr = tp->address;
 	int ret = -ENOENT;
 
 	if (!is_kprobe) {
 		map = dso__new_map(tp->module);
 		if (!map)
 			goto out;
-		addr = tp->address;
 		sym = map__find_symbol(map, addr, NULL);
 	} else {
-		addr = kernel_get_symbol_address_by_name(tp->symbol, true);
+		if (tp->symbol)
+			addr = kernel_get_symbol_address_by_name(tp->symbol, true);
 		if (addr) {
 			addr += tp->offset;
 			sym = __find_kernel_function(addr, &map);
-- 
1.8.3.4


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

* [PATCH 2/2] perf probe: Support probing at absolute address
  2015-08-25 13:27 [PATCH 1/2] perf probe: Prevent segfault when reading probe point with absolute address Wang Nan
@ 2015-08-25 13:27 ` Wang Nan
  2015-08-26  0:02   ` 平松雅巳 / HIRAMATU,MASAMI
  2015-08-25 15:32 ` [PATCH 1/2] perf probe: Prevent segfault when reading probe point with " Arnaldo Carvalho de Melo
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Wang Nan @ 2015-08-25 13:27 UTC (permalink / raw)
  To: acme; +Cc: mingo, masami.hiramatsu.pt, namhyung, pi3orama, linux-kernel

It should be useful to allow 'perf probe' probe at absolute offset of
a target. For example, when (u)probing at a instruction of a shared
object in a embedded system where debuginfo is not avaliable but we
know the offset of that instruction by manually digging.

This patch enables following perf probe command syntax:

 # perf probe +0xffffffff811e6615

And

 # perf probe /lib/x86_64-linux-gnu/libc-2.19.so +0xeb860

In the above example, we don't need a anchor symbol, so it is possible
to compute absolute addresses using other methods and then use
'perf probe' to create the probing points.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/probe-event.c  | 144 +++++++++++++++++++++++++++++++++++++----
 tools/perf/util/probe-event.h  |   3 +
 tools/perf/util/probe-finder.c |  21 +-----
 3 files changed, 138 insertions(+), 30 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 6c7e538..59de69a4 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1194,9 +1194,13 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
 		*ptr++ = '\0';
 	}
 
-	tmp = strdup(arg);
-	if (tmp == NULL)
-		return -ENOMEM;
+	if (arg[0] == '\0')
+		tmp = NULL;
+	else {
+		tmp = strdup(arg);
+		if (tmp == NULL)
+			return -ENOMEM;
+	}
 
 	if (file_spec)
 		pp->file = tmp;
@@ -1283,11 +1287,6 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
 		return -EINVAL;
 	}
 
-	if (pp->offset && !pp->function) {
-		semantic_error("Offset requires an entry function.\n");
-		return -EINVAL;
-	}
-
 	if (pp->retprobe && !pp->function) {
 		semantic_error("Return probe requires an entry function.\n");
 		return -EINVAL;
@@ -1299,6 +1298,11 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
 		return -EINVAL;
 	}
 
+	if (!pp->function && !pp->offset && !pp->file) {
+		semantic_error("Absolute address should not be zero.\n");
+		return -EINVAL;
+	}
+
 	pr_debug("symbol:%s file:%s line:%d offset:%lu return:%d lazy:%s\n",
 		 pp->function, pp->file, pp->line, pp->offset, pp->retprobe,
 		 pp->lazy_line);
@@ -1609,7 +1613,7 @@ error:
 static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
 {
 	char *buf, *tmp;
-	char offs[32] = "", line[32] = "", file[32] = "";
+	char offs[32] = "", line[32] = "", file[32] = "", addr[32] = "";
 	int ret, len;
 
 	buf = zalloc(MAX_CMDLEN);
@@ -1622,6 +1626,11 @@ static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
 		if (ret <= 0)
 			goto error;
 	}
+	if (!pp->function) {
+		ret = e_snprintf(addr, 32, "0x%lx", pp->offset);
+		if (ret <= 0)
+			goto error;
+	}
 	if (pp->line) {
 		ret = e_snprintf(line, 32, ":%d", pp->line);
 		if (ret <= 0)
@@ -1639,9 +1648,11 @@ static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
 			goto error;
 	}
 
-	if (pp->function)
-		ret = e_snprintf(buf, MAX_CMDLEN, "%s%s%s%s%s", pp->function,
-				 offs, pp->retprobe ? "%return" : "", line,
+	if (pp->function || pp->offset)
+		ret = e_snprintf(buf, MAX_CMDLEN, "%s%s%s%s%s",
+				 pp->function ? : addr,
+				 pp->function ? offs : "",
+				 pp->retprobe ? "%return" : "", line,
 				 file);
 	else
 		ret = e_snprintf(buf, MAX_CMDLEN, "%s%s", file, line);
@@ -1786,6 +1797,11 @@ char *synthesize_probe_trace_command(struct probe_trace_event *tev)
 	if (tev->uprobes)
 		ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s:0x%lx",
 				 tp->module, tp->address);
+	else if (tp->symbol[0] == '0' && tp->symbol[1] == 'x')
+		/* Absolute address. See try_to_find_absolute_address() */
+		ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s%s0x%lx",
+				 tp->module ?: "", tp->module ? ":" : "",
+				 tp->address);
 	else
 		ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s%s%s+%lu",
 				 tp->module ?: "", tp->module ? ":" : "",
@@ -2572,6 +2588,87 @@ err_out:
 	goto out;
 }
 
+static int try_to_find_absolute_address(struct perf_probe_event *pev,
+					struct probe_trace_event **tevs)
+{
+	struct perf_probe_point *pp = &pev->point;
+	struct probe_trace_event *tev;
+	struct probe_trace_point *tp;
+	int i, err;
+
+	if (perf_probe_event_need_dwarf(pev) || pev->point.function)
+		return -EINVAL;
+
+	/*
+	 * This is 'perf probe /lib/libc.so +0xabcd'. Try to probe at
+	 * absolute address.
+	 *
+	 * Only one tev can be generated by this.
+	 */
+	*tevs = zalloc(sizeof(*tev));
+	if (!*tevs)
+		return -ENOMEM;
+
+	tev = *tevs;
+	tp = &tev->point;
+
+	/*
+	 * Don't use tp->offset, use address directly, because
+	 * in synthesize_probe_trace_command() address cannot be
+	 * zero.
+	 */
+	tp->address = pev->point.offset;
+	tp->retprobe = pp->retprobe;
+	tev->uprobes = pev->uprobes;
+
+	err = -ENOMEM;
+	/* Give it a '0x' leading symbol name */
+	if (asprintf(&tp->symbol, "0x%lx", tp->address) < 0)
+		goto errout;
+
+	/* For kprobe, check range */
+	if ((!tev->uprobes) &&
+	    (kprobe_warn_out_range(tev->point.symbol,
+				   tev->point.address))) {
+		err = -EACCES;
+		goto errout;
+	}
+
+	if (asprintf(&tp->realname, "abs_%lx", tp->address) < 0)
+		goto errout;
+
+	if (pev->target) {
+		tp->module = strdup(pev->target);
+		if (!tp->module)
+			goto errout;
+	}
+
+	if (tev->group) {
+		tev->group = strdup(pev->group);
+		if (!tev->group)
+			goto errout;
+	}
+
+	if (pev->event) {
+		tev->event = strdup(pev->event);
+		if (!tev->event)
+			goto errout;
+	}
+
+	tev->nargs = pev->nargs;
+	for (i = 0; i < tev->nargs; i++)
+		copy_to_probe_trace_arg(&tev->args[i], &pev->args[i]);
+
+	return 1;
+
+errout:
+	if (*tevs) {
+		clear_probe_trace_events(*tevs, 1);
+		*tevs = NULL;
+	}
+	return err;
+}
+
 bool __weak arch__prefers_symtab(void) { return false; }
 
 static int convert_to_probe_trace_events(struct perf_probe_event *pev,
@@ -2588,6 +2685,10 @@ static int convert_to_probe_trace_events(struct perf_probe_event *pev,
 		}
 	}
 
+	ret = try_to_find_absolute_address(pev, tevs);
+	if (ret > 0)
+		return ret;
+
 	if (arch__prefers_symtab() && !perf_probe_event_need_dwarf(pev)) {
 		ret = find_probe_trace_events_from_map(pev, tevs);
 		if (ret > 0)
@@ -2758,3 +2859,22 @@ end:
 	return ret;
 }
 
+int copy_to_probe_trace_arg(struct probe_trace_arg *tvar,
+			    struct perf_probe_arg *pvar)
+{
+	tvar->value = strdup(pvar->var);
+	if (tvar->value == NULL)
+		return -ENOMEM;
+	if (pvar->type) {
+		tvar->type = strdup(pvar->type);
+		if (tvar->type == NULL)
+			return -ENOMEM;
+	}
+	if (pvar->name) {
+		tvar->name = strdup(pvar->name);
+		if (tvar->name == NULL)
+			return -ENOMEM;
+	} else
+		tvar->name = NULL;
+	return 0;
+}
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index 83ee95e..174a3cf 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -156,4 +156,7 @@ int e_snprintf(char *str, size_t size, const char *format, ...)
 /* Maximum index number of event-name postfix */
 #define MAX_EVENT_INDEX	1024
 
+int copy_to_probe_trace_arg(struct probe_trace_arg *tvar,
+			    struct perf_probe_arg *pvar);
+
 #endif /*_PROBE_EVENT_H */
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 7b80f8c..29c43c068 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -553,24 +553,9 @@ static int find_variable(Dwarf_Die *sc_die, struct probe_finder *pf)
 	char buf[32], *ptr;
 	int ret = 0;
 
-	if (!is_c_varname(pf->pvar->var)) {
-		/* Copy raw parameters */
-		pf->tvar->value = strdup(pf->pvar->var);
-		if (pf->tvar->value == NULL)
-			return -ENOMEM;
-		if (pf->pvar->type) {
-			pf->tvar->type = strdup(pf->pvar->type);
-			if (pf->tvar->type == NULL)
-				return -ENOMEM;
-		}
-		if (pf->pvar->name) {
-			pf->tvar->name = strdup(pf->pvar->name);
-			if (pf->tvar->name == NULL)
-				return -ENOMEM;
-		} else
-			pf->tvar->name = NULL;
-		return 0;
-	}
+	/* Copy raw parameters */
+	if (!is_c_varname(pf->pvar->var))
+		return copy_to_probe_trace_arg(pf->tvar, pf->pvar);
 
 	if (pf->pvar->name)
 		pf->tvar->name = strdup(pf->pvar->name);
-- 
1.8.3.4


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

* Re: [PATCH 1/2] perf probe: Prevent segfault when reading probe point with absolute address
  2015-08-25 13:27 [PATCH 1/2] perf probe: Prevent segfault when reading probe point with absolute address Wang Nan
  2015-08-25 13:27 ` [PATCH 2/2] perf probe: Support probing at " Wang Nan
@ 2015-08-25 15:32 ` Arnaldo Carvalho de Melo
  2015-08-26  0:08 ` 平松雅巳 / HIRAMATU,MASAMI
  2015-08-28  6:40 ` [tip:perf/core] " tip-bot for Wang Nan
  3 siblings, 0 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-08-25 15:32 UTC (permalink / raw)
  To: Wang Nan; +Cc: mingo, masami.hiramatsu.pt, namhyung, pi3orama, linux-kernel

Em Tue, Aug 25, 2015 at 01:27:35PM +0000, Wang Nan escreveu:
> 'perf probe -l' panic if there is a manually inserted probing point
> with absolute address. For example:

Thanks, tested and applied.

- Arnaldo

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

* RE: [PATCH 2/2] perf probe: Support probing at absolute address
  2015-08-25 13:27 ` [PATCH 2/2] perf probe: Support probing at " Wang Nan
@ 2015-08-26  0:02   ` 平松雅巳 / HIRAMATU,MASAMI
  2015-08-26  2:38     ` Wangnan (F)
  2015-08-26 13:00     ` acme
  0 siblings, 2 replies; 12+ messages in thread
From: 平松雅巳 / HIRAMATU,MASAMI @ 2015-08-26  0:02 UTC (permalink / raw)
  To: 'Wang Nan', acme; +Cc: mingo, namhyung, pi3orama, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 9118 bytes --]

> From: Wang Nan [mailto:wangnan0@huawei.com]
> 
> It should be useful to allow 'perf probe' probe at absolute offset of
> a target. For example, when (u)probing at a instruction of a shared
> object in a embedded system where debuginfo is not avaliable but we
> know the offset of that instruction by manually digging.
> 
> This patch enables following perf probe command syntax:
> 
>  # perf probe +0xffffffff811e6615
> 
> And
> 
>  # perf probe /lib/x86_64-linux-gnu/libc-2.19.so +0xeb860

Why do we need "+" for the absolute address?
It seems that we can do it if we find that the given probe point
starts with "0x".

Thanks,

> 
> In the above example, we don't need a anchor symbol, so it is possible
> to compute absolute addresses using other methods and then use
> 'perf probe' to create the probing points.
> 
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/probe-event.c  | 144 +++++++++++++++++++++++++++++++++++++----
>  tools/perf/util/probe-event.h  |   3 +
>  tools/perf/util/probe-finder.c |  21 +-----
>  3 files changed, 138 insertions(+), 30 deletions(-)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 6c7e538..59de69a4 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -1194,9 +1194,13 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
>  		*ptr++ = '\0';
>  	}
> 
> -	tmp = strdup(arg);
> -	if (tmp == NULL)
> -		return -ENOMEM;
> +	if (arg[0] == '\0')
> +		tmp = NULL;
> +	else {
> +		tmp = strdup(arg);
> +		if (tmp == NULL)
> +			return -ENOMEM;
> +	}
> 
>  	if (file_spec)
>  		pp->file = tmp;
> @@ -1283,11 +1287,6 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
>  		return -EINVAL;
>  	}
> 
> -	if (pp->offset && !pp->function) {
> -		semantic_error("Offset requires an entry function.\n");
> -		return -EINVAL;
> -	}
> -
>  	if (pp->retprobe && !pp->function) {
>  		semantic_error("Return probe requires an entry function.\n");
>  		return -EINVAL;
> @@ -1299,6 +1298,11 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
>  		return -EINVAL;
>  	}
> 
> +	if (!pp->function && !pp->offset && !pp->file) {
> +		semantic_error("Absolute address should not be zero.\n");
> +		return -EINVAL;
> +	}
> +
>  	pr_debug("symbol:%s file:%s line:%d offset:%lu return:%d lazy:%s\n",
>  		 pp->function, pp->file, pp->line, pp->offset, pp->retprobe,
>  		 pp->lazy_line);
> @@ -1609,7 +1613,7 @@ error:
>  static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
>  {
>  	char *buf, *tmp;
> -	char offs[32] = "", line[32] = "", file[32] = "";
> +	char offs[32] = "", line[32] = "", file[32] = "", addr[32] = "";
>  	int ret, len;
> 
>  	buf = zalloc(MAX_CMDLEN);
> @@ -1622,6 +1626,11 @@ static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
>  		if (ret <= 0)
>  			goto error;
>  	}
> +	if (!pp->function) {
> +		ret = e_snprintf(addr, 32, "0x%lx", pp->offset);
> +		if (ret <= 0)
> +			goto error;
> +	}
>  	if (pp->line) {
>  		ret = e_snprintf(line, 32, ":%d", pp->line);
>  		if (ret <= 0)
> @@ -1639,9 +1648,11 @@ static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
>  			goto error;
>  	}
> 
> -	if (pp->function)
> -		ret = e_snprintf(buf, MAX_CMDLEN, "%s%s%s%s%s", pp->function,
> -				 offs, pp->retprobe ? "%return" : "", line,
> +	if (pp->function || pp->offset)
> +		ret = e_snprintf(buf, MAX_CMDLEN, "%s%s%s%s%s",
> +				 pp->function ? : addr,
> +				 pp->function ? offs : "",
> +				 pp->retprobe ? "%return" : "", line,
>  				 file);
>  	else
>  		ret = e_snprintf(buf, MAX_CMDLEN, "%s%s", file, line);
> @@ -1786,6 +1797,11 @@ char *synthesize_probe_trace_command(struct probe_trace_event *tev)
>  	if (tev->uprobes)
>  		ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s:0x%lx",
>  				 tp->module, tp->address);
> +	else if (tp->symbol[0] == '0' && tp->symbol[1] == 'x')
> +		/* Absolute address. See try_to_find_absolute_address() */
> +		ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s%s0x%lx",
> +				 tp->module ?: "", tp->module ? ":" : "",
> +				 tp->address);
>  	else
>  		ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s%s%s+%lu",
>  				 tp->module ?: "", tp->module ? ":" : "",
> @@ -2572,6 +2588,87 @@ err_out:
>  	goto out;
>  }
> 
> +static int try_to_find_absolute_address(struct perf_probe_event *pev,
> +					struct probe_trace_event **tevs)
> +{
> +	struct perf_probe_point *pp = &pev->point;
> +	struct probe_trace_event *tev;
> +	struct probe_trace_point *tp;
> +	int i, err;
> +
> +	if (perf_probe_event_need_dwarf(pev) || pev->point.function)
> +		return -EINVAL;
> +
> +	/*
> +	 * This is 'perf probe /lib/libc.so +0xabcd'. Try to probe at
> +	 * absolute address.
> +	 *
> +	 * Only one tev can be generated by this.
> +	 */
> +	*tevs = zalloc(sizeof(*tev));
> +	if (!*tevs)
> +		return -ENOMEM;
> +
> +	tev = *tevs;
> +	tp = &tev->point;
> +
> +	/*
> +	 * Don't use tp->offset, use address directly, because
> +	 * in synthesize_probe_trace_command() address cannot be
> +	 * zero.
> +	 */
> +	tp->address = pev->point.offset;
> +	tp->retprobe = pp->retprobe;
> +	tev->uprobes = pev->uprobes;
> +
> +	err = -ENOMEM;
> +	/* Give it a '0x' leading symbol name */
> +	if (asprintf(&tp->symbol, "0x%lx", tp->address) < 0)
> +		goto errout;
> +
> +	/* For kprobe, check range */
> +	if ((!tev->uprobes) &&
> +	    (kprobe_warn_out_range(tev->point.symbol,
> +				   tev->point.address))) {
> +		err = -EACCES;
> +		goto errout;
> +	}
> +
> +	if (asprintf(&tp->realname, "abs_%lx", tp->address) < 0)
> +		goto errout;
> +
> +	if (pev->target) {
> +		tp->module = strdup(pev->target);
> +		if (!tp->module)
> +			goto errout;
> +	}
> +
> +	if (tev->group) {
> +		tev->group = strdup(pev->group);
> +		if (!tev->group)
> +			goto errout;
> +	}
> +
> +	if (pev->event) {
> +		tev->event = strdup(pev->event);
> +		if (!tev->event)
> +			goto errout;
> +	}
> +
> +	tev->nargs = pev->nargs;
> +	for (i = 0; i < tev->nargs; i++)
> +		copy_to_probe_trace_arg(&tev->args[i], &pev->args[i]);
> +
> +	return 1;
> +
> +errout:
> +	if (*tevs) {
> +		clear_probe_trace_events(*tevs, 1);
> +		*tevs = NULL;
> +	}
> +	return err;
> +}
> +
>  bool __weak arch__prefers_symtab(void) { return false; }
> 
>  static int convert_to_probe_trace_events(struct perf_probe_event *pev,
> @@ -2588,6 +2685,10 @@ static int convert_to_probe_trace_events(struct perf_probe_event *pev,
>  		}
>  	}
> 
> +	ret = try_to_find_absolute_address(pev, tevs);
> +	if (ret > 0)
> +		return ret;
> +
>  	if (arch__prefers_symtab() && !perf_probe_event_need_dwarf(pev)) {
>  		ret = find_probe_trace_events_from_map(pev, tevs);
>  		if (ret > 0)
> @@ -2758,3 +2859,22 @@ end:
>  	return ret;
>  }
> 
> +int copy_to_probe_trace_arg(struct probe_trace_arg *tvar,
> +			    struct perf_probe_arg *pvar)
> +{
> +	tvar->value = strdup(pvar->var);
> +	if (tvar->value == NULL)
> +		return -ENOMEM;
> +	if (pvar->type) {
> +		tvar->type = strdup(pvar->type);
> +		if (tvar->type == NULL)
> +			return -ENOMEM;
> +	}
> +	if (pvar->name) {
> +		tvar->name = strdup(pvar->name);
> +		if (tvar->name == NULL)
> +			return -ENOMEM;
> +	} else
> +		tvar->name = NULL;
> +	return 0;
> +}
> diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> index 83ee95e..174a3cf 100644
> --- a/tools/perf/util/probe-event.h
> +++ b/tools/perf/util/probe-event.h
> @@ -156,4 +156,7 @@ int e_snprintf(char *str, size_t size, const char *format, ...)
>  /* Maximum index number of event-name postfix */
>  #define MAX_EVENT_INDEX	1024
> 
> +int copy_to_probe_trace_arg(struct probe_trace_arg *tvar,
> +			    struct perf_probe_arg *pvar);
> +
>  #endif /*_PROBE_EVENT_H */
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index 7b80f8c..29c43c068 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -553,24 +553,9 @@ static int find_variable(Dwarf_Die *sc_die, struct probe_finder *pf)
>  	char buf[32], *ptr;
>  	int ret = 0;
> 
> -	if (!is_c_varname(pf->pvar->var)) {
> -		/* Copy raw parameters */
> -		pf->tvar->value = strdup(pf->pvar->var);
> -		if (pf->tvar->value == NULL)
> -			return -ENOMEM;
> -		if (pf->pvar->type) {
> -			pf->tvar->type = strdup(pf->pvar->type);
> -			if (pf->tvar->type == NULL)
> -				return -ENOMEM;
> -		}
> -		if (pf->pvar->name) {
> -			pf->tvar->name = strdup(pf->pvar->name);
> -			if (pf->tvar->name == NULL)
> -				return -ENOMEM;
> -		} else
> -			pf->tvar->name = NULL;
> -		return 0;
> -	}
> +	/* Copy raw parameters */
> +	if (!is_c_varname(pf->pvar->var))
> +		return copy_to_probe_trace_arg(pf->tvar, pf->pvar);
> 
>  	if (pf->pvar->name)
>  		pf->tvar->name = strdup(pf->pvar->name);
> --
> 1.8.3.4

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

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

* RE: [PATCH 1/2] perf probe: Prevent segfault when reading probe point with absolute address
  2015-08-25 13:27 [PATCH 1/2] perf probe: Prevent segfault when reading probe point with absolute address Wang Nan
  2015-08-25 13:27 ` [PATCH 2/2] perf probe: Support probing at " Wang Nan
  2015-08-25 15:32 ` [PATCH 1/2] perf probe: Prevent segfault when reading probe point with " Arnaldo Carvalho de Melo
@ 2015-08-26  0:08 ` 平松雅巳 / HIRAMATU,MASAMI
  2015-08-28  6:40 ` [tip:perf/core] " tip-bot for Wang Nan
  3 siblings, 0 replies; 12+ messages in thread
From: 平松雅巳 / HIRAMATU,MASAMI @ 2015-08-26  0:08 UTC (permalink / raw)
  To: 'Wang Nan', acme; +Cc: mingo, namhyung, pi3orama, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2546 bytes --]

> From: Wang Nan [mailto:wangnan0@huawei.com]
> 
> 'perf probe -l' panic if there is a manually inserted probing point
> with absolute address. For example:
> 
>  # echo 'p:probe/abs_ffffffff811e6615 0xffffffff811e6615' > /sys/kernel/debug/tracing/kprobe_events
>  # perf probe -l
>  Segmentation fault (core dumped)
> 
> This patch fix this problem by considering the situation that
> "tp->symbol == NULL" in find_perf_probe_point_from_dwarf() and
> find_perf_probe_point_from_map().

Oops, OK this is a bug.

Thanks!


Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>

> 
> After this patch:
> 
>  # perf probe -l
>  probe:abs_ffffffff811e6615 (on SyS_write+5@fs/read_write.c)
> 
> And when debug info is missing:
> 
>  # rm -rf ~/.debug
>  # mv /lib/modules/4.2.0-rc1+/build/vmlinux /lib/modules/4.2.0-rc1+/build/vmlinux.bak
>  # perf probe -l
>  probe:abs_ffffffff811e6615 (on sys_write+5)
> 
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/probe-event.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index f07374b..6c7e538 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -515,7 +515,7 @@ static int find_perf_probe_point_from_dwarf(struct probe_trace_point *tp,
>  		if (ret < 0)
>  			goto error;
>  		addr += stext;
> -	} else {
> +	} else if (tp->symbol) {
>  		addr = kernel_get_symbol_address_by_name(tp->symbol, false);
>  		if (addr == 0)
>  			goto error;
> @@ -1815,17 +1815,17 @@ static int find_perf_probe_point_from_map(struct probe_trace_point *tp,
>  {
>  	struct symbol *sym = NULL;
>  	struct map *map;
> -	u64 addr;
> +	u64 addr = tp->address;
>  	int ret = -ENOENT;
> 
>  	if (!is_kprobe) {
>  		map = dso__new_map(tp->module);
>  		if (!map)
>  			goto out;
> -		addr = tp->address;
>  		sym = map__find_symbol(map, addr, NULL);
>  	} else {
> -		addr = kernel_get_symbol_address_by_name(tp->symbol, true);
> +		if (tp->symbol)
> +			addr = kernel_get_symbol_address_by_name(tp->symbol, true);
>  		if (addr) {
>  			addr += tp->offset;
>  			sym = __find_kernel_function(addr, &map);
> --
> 1.8.3.4

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

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

* Re: [PATCH 2/2] perf probe: Support probing at absolute address
  2015-08-26  0:02   ` 平松雅巳 / HIRAMATU,MASAMI
@ 2015-08-26  2:38     ` Wangnan (F)
  2015-08-26  9:02       ` 平松雅巳 / HIRAMATU,MASAMI
  2015-08-26 13:02       ` acme
  2015-08-26 13:00     ` acme
  1 sibling, 2 replies; 12+ messages in thread
From: Wangnan (F) @ 2015-08-26  2:38 UTC (permalink / raw)
  To: 平松雅巳 / HIRAMATU,MASAMI, acme
  Cc: mingo, namhyung, pi3orama, linux-kernel


On 2015/8/26 8:02, 平松雅巳 / HIRAMATU,MASAMI wrote:
>> From: Wang Nan [mailto:wangnan0@huawei.com]
>>
>> It should be useful to allow 'perf probe' probe at absolute offset of
>> a target. For example, when (u)probing at a instruction of a shared
>> object in a embedded system where debuginfo is not avaliable but we
>> know the offset of that instruction by manually digging.
>>
>> This patch enables following perf probe command syntax:
>>
>>   # perf probe +0xffffffff811e6615
>>
>> And
>>
>>   # perf probe /lib/x86_64-linux-gnu/libc-2.19.so +0xeb860
> Why do we need "+" for the absolute address?
> It seems that we can do it if we find that the given probe point
> starts with "0x".
>
> Thanks,

I will change 2/2 as you suggection.

However, we can only ensure that in kernel side symbol never leading
with '0x'. Although I don't think symbol leading with 0x is useful,
it is still possible for a userspace program compiled and linked by
a language other than C produces such symbol. '+' helps us separate
address and function name semantically, make us don't rely on assumption
on function names. If in future we do meet '0x' symbols, I think we still
need the '+' syntax back. But we can do it at that time.

Thank you.


>> In the above example, we don't need a anchor symbol, so it is possible
>> to compute absolute addresses using other methods and then use
>> 'perf probe' to create the probing points.
>>
>> Signed-off-by: Wang Nan <wangnan0@huawei.com>
>> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>> Cc: Namhyung Kim <namhyung@kernel.org>
>> ---
>>   tools/perf/util/probe-event.c  | 144 +++++++++++++++++++++++++++++++++++++----
>>   tools/perf/util/probe-event.h  |   3 +
>>   tools/perf/util/probe-finder.c |  21 +-----
>>   3 files changed, 138 insertions(+), 30 deletions(-)
>>
>> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
>> index 6c7e538..59de69a4 100644
>> --- a/tools/perf/util/probe-event.c
>> +++ b/tools/perf/util/probe-event.c
>> @@ -1194,9 +1194,13 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
>>   		*ptr++ = '\0';
>>   	}
>>
>> -	tmp = strdup(arg);
>> -	if (tmp == NULL)
>> -		return -ENOMEM;
>> +	if (arg[0] == '\0')
>> +		tmp = NULL;
>> +	else {
>> +		tmp = strdup(arg);
>> +		if (tmp == NULL)
>> +			return -ENOMEM;
>> +	}
>>
>>   	if (file_spec)
>>   		pp->file = tmp;
>> @@ -1283,11 +1287,6 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
>>   		return -EINVAL;
>>   	}
>>
>> -	if (pp->offset && !pp->function) {
>> -		semantic_error("Offset requires an entry function.\n");
>> -		return -EINVAL;
>> -	}
>> -
>>   	if (pp->retprobe && !pp->function) {
>>   		semantic_error("Return probe requires an entry function.\n");
>>   		return -EINVAL;
>> @@ -1299,6 +1298,11 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
>>   		return -EINVAL;
>>   	}
>>
>> +	if (!pp->function && !pp->offset && !pp->file) {
>> +		semantic_error("Absolute address should not be zero.\n");
>> +		return -EINVAL;
>> +	}
>> +
>>   	pr_debug("symbol:%s file:%s line:%d offset:%lu return:%d lazy:%s\n",
>>   		 pp->function, pp->file, pp->line, pp->offset, pp->retprobe,
>>   		 pp->lazy_line);
>> @@ -1609,7 +1613,7 @@ error:
>>   static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
>>   {
>>   	char *buf, *tmp;
>> -	char offs[32] = "", line[32] = "", file[32] = "";
>> +	char offs[32] = "", line[32] = "", file[32] = "", addr[32] = "";
>>   	int ret, len;
>>
>>   	buf = zalloc(MAX_CMDLEN);
>> @@ -1622,6 +1626,11 @@ static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
>>   		if (ret <= 0)
>>   			goto error;
>>   	}
>> +	if (!pp->function) {
>> +		ret = e_snprintf(addr, 32, "0x%lx", pp->offset);
>> +		if (ret <= 0)
>> +			goto error;
>> +	}
>>   	if (pp->line) {
>>   		ret = e_snprintf(line, 32, ":%d", pp->line);
>>   		if (ret <= 0)
>> @@ -1639,9 +1648,11 @@ static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
>>   			goto error;
>>   	}
>>
>> -	if (pp->function)
>> -		ret = e_snprintf(buf, MAX_CMDLEN, "%s%s%s%s%s", pp->function,
>> -				 offs, pp->retprobe ? "%return" : "", line,
>> +	if (pp->function || pp->offset)
>> +		ret = e_snprintf(buf, MAX_CMDLEN, "%s%s%s%s%s",
>> +				 pp->function ? : addr,
>> +				 pp->function ? offs : "",
>> +				 pp->retprobe ? "%return" : "", line,
>>   				 file);
>>   	else
>>   		ret = e_snprintf(buf, MAX_CMDLEN, "%s%s", file, line);
>> @@ -1786,6 +1797,11 @@ char *synthesize_probe_trace_command(struct probe_trace_event *tev)
>>   	if (tev->uprobes)
>>   		ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s:0x%lx",
>>   				 tp->module, tp->address);
>> +	else if (tp->symbol[0] == '0' && tp->symbol[1] == 'x')
>> +		/* Absolute address. See try_to_find_absolute_address() */
>> +		ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s%s0x%lx",
>> +				 tp->module ?: "", tp->module ? ":" : "",
>> +				 tp->address);
>>   	else
>>   		ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s%s%s+%lu",
>>   				 tp->module ?: "", tp->module ? ":" : "",
>> @@ -2572,6 +2588,87 @@ err_out:
>>   	goto out;
>>   }
>>
>> +static int try_to_find_absolute_address(struct perf_probe_event *pev,
>> +					struct probe_trace_event **tevs)
>> +{
>> +	struct perf_probe_point *pp = &pev->point;
>> +	struct probe_trace_event *tev;
>> +	struct probe_trace_point *tp;
>> +	int i, err;
>> +
>> +	if (perf_probe_event_need_dwarf(pev) || pev->point.function)
>> +		return -EINVAL;
>> +
>> +	/*
>> +	 * This is 'perf probe /lib/libc.so +0xabcd'. Try to probe at
>> +	 * absolute address.
>> +	 *
>> +	 * Only one tev can be generated by this.
>> +	 */
>> +	*tevs = zalloc(sizeof(*tev));
>> +	if (!*tevs)
>> +		return -ENOMEM;
>> +
>> +	tev = *tevs;
>> +	tp = &tev->point;
>> +
>> +	/*
>> +	 * Don't use tp->offset, use address directly, because
>> +	 * in synthesize_probe_trace_command() address cannot be
>> +	 * zero.
>> +	 */
>> +	tp->address = pev->point.offset;
>> +	tp->retprobe = pp->retprobe;
>> +	tev->uprobes = pev->uprobes;
>> +
>> +	err = -ENOMEM;
>> +	/* Give it a '0x' leading symbol name */
>> +	if (asprintf(&tp->symbol, "0x%lx", tp->address) < 0)
>> +		goto errout;
>> +
>> +	/* For kprobe, check range */
>> +	if ((!tev->uprobes) &&
>> +	    (kprobe_warn_out_range(tev->point.symbol,
>> +				   tev->point.address))) {
>> +		err = -EACCES;
>> +		goto errout;
>> +	}
>> +
>> +	if (asprintf(&tp->realname, "abs_%lx", tp->address) < 0)
>> +		goto errout;
>> +
>> +	if (pev->target) {
>> +		tp->module = strdup(pev->target);
>> +		if (!tp->module)
>> +			goto errout;
>> +	}
>> +
>> +	if (tev->group) {
>> +		tev->group = strdup(pev->group);
>> +		if (!tev->group)
>> +			goto errout;
>> +	}
>> +
>> +	if (pev->event) {
>> +		tev->event = strdup(pev->event);
>> +		if (!tev->event)
>> +			goto errout;
>> +	}
>> +
>> +	tev->nargs = pev->nargs;
>> +	for (i = 0; i < tev->nargs; i++)
>> +		copy_to_probe_trace_arg(&tev->args[i], &pev->args[i]);
>> +
>> +	return 1;
>> +
>> +errout:
>> +	if (*tevs) {
>> +		clear_probe_trace_events(*tevs, 1);
>> +		*tevs = NULL;
>> +	}
>> +	return err;
>> +}
>> +
>>   bool __weak arch__prefers_symtab(void) { return false; }
>>
>>   static int convert_to_probe_trace_events(struct perf_probe_event *pev,
>> @@ -2588,6 +2685,10 @@ static int convert_to_probe_trace_events(struct perf_probe_event *pev,
>>   		}
>>   	}
>>
>> +	ret = try_to_find_absolute_address(pev, tevs);
>> +	if (ret > 0)
>> +		return ret;
>> +
>>   	if (arch__prefers_symtab() && !perf_probe_event_need_dwarf(pev)) {
>>   		ret = find_probe_trace_events_from_map(pev, tevs);
>>   		if (ret > 0)
>> @@ -2758,3 +2859,22 @@ end:
>>   	return ret;
>>   }
>>
>> +int copy_to_probe_trace_arg(struct probe_trace_arg *tvar,
>> +			    struct perf_probe_arg *pvar)
>> +{
>> +	tvar->value = strdup(pvar->var);
>> +	if (tvar->value == NULL)
>> +		return -ENOMEM;
>> +	if (pvar->type) {
>> +		tvar->type = strdup(pvar->type);
>> +		if (tvar->type == NULL)
>> +			return -ENOMEM;
>> +	}
>> +	if (pvar->name) {
>> +		tvar->name = strdup(pvar->name);
>> +		if (tvar->name == NULL)
>> +			return -ENOMEM;
>> +	} else
>> +		tvar->name = NULL;
>> +	return 0;
>> +}
>> diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
>> index 83ee95e..174a3cf 100644
>> --- a/tools/perf/util/probe-event.h
>> +++ b/tools/perf/util/probe-event.h
>> @@ -156,4 +156,7 @@ int e_snprintf(char *str, size_t size, const char *format, ...)
>>   /* Maximum index number of event-name postfix */
>>   #define MAX_EVENT_INDEX	1024
>>
>> +int copy_to_probe_trace_arg(struct probe_trace_arg *tvar,
>> +			    struct perf_probe_arg *pvar);
>> +
>>   #endif /*_PROBE_EVENT_H */
>> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
>> index 7b80f8c..29c43c068 100644
>> --- a/tools/perf/util/probe-finder.c
>> +++ b/tools/perf/util/probe-finder.c
>> @@ -553,24 +553,9 @@ static int find_variable(Dwarf_Die *sc_die, struct probe_finder *pf)
>>   	char buf[32], *ptr;
>>   	int ret = 0;
>>
>> -	if (!is_c_varname(pf->pvar->var)) {
>> -		/* Copy raw parameters */
>> -		pf->tvar->value = strdup(pf->pvar->var);
>> -		if (pf->tvar->value == NULL)
>> -			return -ENOMEM;
>> -		if (pf->pvar->type) {
>> -			pf->tvar->type = strdup(pf->pvar->type);
>> -			if (pf->tvar->type == NULL)
>> -				return -ENOMEM;
>> -		}
>> -		if (pf->pvar->name) {
>> -			pf->tvar->name = strdup(pf->pvar->name);
>> -			if (pf->tvar->name == NULL)
>> -				return -ENOMEM;
>> -		} else
>> -			pf->tvar->name = NULL;
>> -		return 0;
>> -	}
>> +	/* Copy raw parameters */
>> +	if (!is_c_varname(pf->pvar->var))
>> +		return copy_to_probe_trace_arg(pf->tvar, pf->pvar);
>>
>>   	if (pf->pvar->name)
>>   		pf->tvar->name = strdup(pf->pvar->name);
>> --
>> 1.8.3.4



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

* RE: [PATCH 2/2] perf probe: Support probing at absolute address
  2015-08-26  2:38     ` Wangnan (F)
@ 2015-08-26  9:02       ` 平松雅巳 / HIRAMATU,MASAMI
  2015-08-26 13:02       ` acme
  1 sibling, 0 replies; 12+ messages in thread
From: 平松雅巳 / HIRAMATU,MASAMI @ 2015-08-26  9:02 UTC (permalink / raw)
  To: 'Wangnan (F)', acme; +Cc: mingo, namhyung, pi3orama, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 10832 bytes --]

> From: Wangnan (F) [mailto:wangnan0@huawei.com] 
> 
> On 2015/8/26 8:02, 平松雅巳 / HIRAMATU,MASAMI wrote:
> >> From: Wang Nan [mailto:wangnan0@huawei.com]
> >>
> >> It should be useful to allow 'perf probe' probe at absolute offset of
> >> a target. For example, when (u)probing at a instruction of a shared
> >> object in a embedded system where debuginfo is not avaliable but we
> >> know the offset of that instruction by manually digging.
> >>
> >> This patch enables following perf probe command syntax:
> >>
> >>   # perf probe +0xffffffff811e6615
> >>
> >> And
> >>
> >>   # perf probe /lib/x86_64-linux-gnu/libc-2.19.so +0xeb860
> > Why do we need "+" for the absolute address?
> > It seems that we can do it if we find that the given probe point
> > starts with "0x".
> >
> > Thanks,
> 
> I will change 2/2 as you suggection.
> 
> However, we can only ensure that in kernel side symbol never leading
> with '0x'. Although I don't think symbol leading with 0x is useful,
> it is still possible for a userspace program compiled and linked by
> a language other than C produces such symbol. '+' helps us separate
> address and function name semantically, make us don't rely on assumption
> on function names. If in future we do meet '0x' symbols, I think we still
> need the '+' syntax back. But we can do it at that time.

Agreed, if we really find the case, we can add '+' prefix support
just for an optional expression :)

Thank you!

> 
> Thank you.
> 
> 
> >> In the above example, we don't need a anchor symbol, so it is possible
> >> to compute absolute addresses using other methods and then use
> >> 'perf probe' to create the probing points.
> >>
> >> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> >> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> >> Cc: Ingo Molnar <mingo@redhat.com>
> >> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> >> Cc: Namhyung Kim <namhyung@kernel.org>
> >> ---
> >>   tools/perf/util/probe-event.c  | 144 +++++++++++++++++++++++++++++++++++++----
> >>   tools/perf/util/probe-event.h  |   3 +
> >>   tools/perf/util/probe-finder.c |  21 +-----
> >>   3 files changed, 138 insertions(+), 30 deletions(-)
> >>
> >> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> >> index 6c7e538..59de69a4 100644
> >> --- a/tools/perf/util/probe-event.c
> >> +++ b/tools/perf/util/probe-event.c
> >> @@ -1194,9 +1194,13 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> >>   		*ptr++ = '\0';
> >>   	}
> >>
> >> -	tmp = strdup(arg);
> >> -	if (tmp == NULL)
> >> -		return -ENOMEM;
> >> +	if (arg[0] == '\0')
> >> +		tmp = NULL;
> >> +	else {
> >> +		tmp = strdup(arg);
> >> +		if (tmp == NULL)
> >> +			return -ENOMEM;
> >> +	}
> >>
> >>   	if (file_spec)
> >>   		pp->file = tmp;
> >> @@ -1283,11 +1287,6 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> >>   		return -EINVAL;
> >>   	}
> >>
> >> -	if (pp->offset && !pp->function) {
> >> -		semantic_error("Offset requires an entry function.\n");
> >> -		return -EINVAL;
> >> -	}
> >> -
> >>   	if (pp->retprobe && !pp->function) {
> >>   		semantic_error("Return probe requires an entry function.\n");
> >>   		return -EINVAL;
> >> @@ -1299,6 +1298,11 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> >>   		return -EINVAL;
> >>   	}
> >>
> >> +	if (!pp->function && !pp->offset && !pp->file) {
> >> +		semantic_error("Absolute address should not be zero.\n");
> >> +		return -EINVAL;
> >> +	}
> >> +
> >>   	pr_debug("symbol:%s file:%s line:%d offset:%lu return:%d lazy:%s\n",
> >>   		 pp->function, pp->file, pp->line, pp->offset, pp->retprobe,
> >>   		 pp->lazy_line);
> >> @@ -1609,7 +1613,7 @@ error:
> >>   static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
> >>   {
> >>   	char *buf, *tmp;
> >> -	char offs[32] = "", line[32] = "", file[32] = "";
> >> +	char offs[32] = "", line[32] = "", file[32] = "", addr[32] = "";
> >>   	int ret, len;
> >>
> >>   	buf = zalloc(MAX_CMDLEN);
> >> @@ -1622,6 +1626,11 @@ static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
> >>   		if (ret <= 0)
> >>   			goto error;
> >>   	}
> >> +	if (!pp->function) {
> >> +		ret = e_snprintf(addr, 32, "0x%lx", pp->offset);
> >> +		if (ret <= 0)
> >> +			goto error;
> >> +	}
> >>   	if (pp->line) {
> >>   		ret = e_snprintf(line, 32, ":%d", pp->line);
> >>   		if (ret <= 0)
> >> @@ -1639,9 +1648,11 @@ static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
> >>   			goto error;
> >>   	}
> >>
> >> -	if (pp->function)
> >> -		ret = e_snprintf(buf, MAX_CMDLEN, "%s%s%s%s%s", pp->function,
> >> -				 offs, pp->retprobe ? "%return" : "", line,
> >> +	if (pp->function || pp->offset)
> >> +		ret = e_snprintf(buf, MAX_CMDLEN, "%s%s%s%s%s",
> >> +				 pp->function ? : addr,
> >> +				 pp->function ? offs : "",
> >> +				 pp->retprobe ? "%return" : "", line,
> >>   				 file);
> >>   	else
> >>   		ret = e_snprintf(buf, MAX_CMDLEN, "%s%s", file, line);
> >> @@ -1786,6 +1797,11 @@ char *synthesize_probe_trace_command(struct probe_trace_event *tev)
> >>   	if (tev->uprobes)
> >>   		ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s:0x%lx",
> >>   				 tp->module, tp->address);
> >> +	else if (tp->symbol[0] == '0' && tp->symbol[1] == 'x')
> >> +		/* Absolute address. See try_to_find_absolute_address() */
> >> +		ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s%s0x%lx",
> >> +				 tp->module ?: "", tp->module ? ":" : "",
> >> +				 tp->address);
> >>   	else
> >>   		ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s%s%s+%lu",
> >>   				 tp->module ?: "", tp->module ? ":" : "",
> >> @@ -2572,6 +2588,87 @@ err_out:
> >>   	goto out;
> >>   }
> >>
> >> +static int try_to_find_absolute_address(struct perf_probe_event *pev,
> >> +					struct probe_trace_event **tevs)
> >> +{
> >> +	struct perf_probe_point *pp = &pev->point;
> >> +	struct probe_trace_event *tev;
> >> +	struct probe_trace_point *tp;
> >> +	int i, err;
> >> +
> >> +	if (perf_probe_event_need_dwarf(pev) || pev->point.function)
> >> +		return -EINVAL;
> >> +
> >> +	/*
> >> +	 * This is 'perf probe /lib/libc.so +0xabcd'. Try to probe at
> >> +	 * absolute address.
> >> +	 *
> >> +	 * Only one tev can be generated by this.
> >> +	 */
> >> +	*tevs = zalloc(sizeof(*tev));
> >> +	if (!*tevs)
> >> +		return -ENOMEM;
> >> +
> >> +	tev = *tevs;
> >> +	tp = &tev->point;
> >> +
> >> +	/*
> >> +	 * Don't use tp->offset, use address directly, because
> >> +	 * in synthesize_probe_trace_command() address cannot be
> >> +	 * zero.
> >> +	 */
> >> +	tp->address = pev->point.offset;
> >> +	tp->retprobe = pp->retprobe;
> >> +	tev->uprobes = pev->uprobes;
> >> +
> >> +	err = -ENOMEM;
> >> +	/* Give it a '0x' leading symbol name */
> >> +	if (asprintf(&tp->symbol, "0x%lx", tp->address) < 0)
> >> +		goto errout;
> >> +
> >> +	/* For kprobe, check range */
> >> +	if ((!tev->uprobes) &&
> >> +	    (kprobe_warn_out_range(tev->point.symbol,
> >> +				   tev->point.address))) {
> >> +		err = -EACCES;
> >> +		goto errout;
> >> +	}
> >> +
> >> +	if (asprintf(&tp->realname, "abs_%lx", tp->address) < 0)
> >> +		goto errout;
> >> +
> >> +	if (pev->target) {
> >> +		tp->module = strdup(pev->target);
> >> +		if (!tp->module)
> >> +			goto errout;
> >> +	}
> >> +
> >> +	if (tev->group) {
> >> +		tev->group = strdup(pev->group);
> >> +		if (!tev->group)
> >> +			goto errout;
> >> +	}
> >> +
> >> +	if (pev->event) {
> >> +		tev->event = strdup(pev->event);
> >> +		if (!tev->event)
> >> +			goto errout;
> >> +	}
> >> +
> >> +	tev->nargs = pev->nargs;
> >> +	for (i = 0; i < tev->nargs; i++)
> >> +		copy_to_probe_trace_arg(&tev->args[i], &pev->args[i]);
> >> +
> >> +	return 1;
> >> +
> >> +errout:
> >> +	if (*tevs) {
> >> +		clear_probe_trace_events(*tevs, 1);
> >> +		*tevs = NULL;
> >> +	}
> >> +	return err;
> >> +}
> >> +
> >>   bool __weak arch__prefers_symtab(void) { return false; }
> >>
> >>   static int convert_to_probe_trace_events(struct perf_probe_event *pev,
> >> @@ -2588,6 +2685,10 @@ static int convert_to_probe_trace_events(struct perf_probe_event *pev,
> >>   		}
> >>   	}
> >>
> >> +	ret = try_to_find_absolute_address(pev, tevs);
> >> +	if (ret > 0)
> >> +		return ret;
> >> +
> >>   	if (arch__prefers_symtab() && !perf_probe_event_need_dwarf(pev)) {
> >>   		ret = find_probe_trace_events_from_map(pev, tevs);
> >>   		if (ret > 0)
> >> @@ -2758,3 +2859,22 @@ end:
> >>   	return ret;
> >>   }
> >>
> >> +int copy_to_probe_trace_arg(struct probe_trace_arg *tvar,
> >> +			    struct perf_probe_arg *pvar)
> >> +{
> >> +	tvar->value = strdup(pvar->var);
> >> +	if (tvar->value == NULL)
> >> +		return -ENOMEM;
> >> +	if (pvar->type) {
> >> +		tvar->type = strdup(pvar->type);
> >> +		if (tvar->type == NULL)
> >> +			return -ENOMEM;
> >> +	}
> >> +	if (pvar->name) {
> >> +		tvar->name = strdup(pvar->name);
> >> +		if (tvar->name == NULL)
> >> +			return -ENOMEM;
> >> +	} else
> >> +		tvar->name = NULL;
> >> +	return 0;
> >> +}
> >> diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> >> index 83ee95e..174a3cf 100644
> >> --- a/tools/perf/util/probe-event.h
> >> +++ b/tools/perf/util/probe-event.h
> >> @@ -156,4 +156,7 @@ int e_snprintf(char *str, size_t size, const char *format, ...)
> >>   /* Maximum index number of event-name postfix */
> >>   #define MAX_EVENT_INDEX	1024
> >>
> >> +int copy_to_probe_trace_arg(struct probe_trace_arg *tvar,
> >> +			    struct perf_probe_arg *pvar);
> >> +
> >>   #endif /*_PROBE_EVENT_H */
> >> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> >> index 7b80f8c..29c43c068 100644
> >> --- a/tools/perf/util/probe-finder.c
> >> +++ b/tools/perf/util/probe-finder.c
> >> @@ -553,24 +553,9 @@ static int find_variable(Dwarf_Die *sc_die, struct probe_finder *pf)
> >>   	char buf[32], *ptr;
> >>   	int ret = 0;
> >>
> >> -	if (!is_c_varname(pf->pvar->var)) {
> >> -		/* Copy raw parameters */
> >> -		pf->tvar->value = strdup(pf->pvar->var);
> >> -		if (pf->tvar->value == NULL)
> >> -			return -ENOMEM;
> >> -		if (pf->pvar->type) {
> >> -			pf->tvar->type = strdup(pf->pvar->type);
> >> -			if (pf->tvar->type == NULL)
> >> -				return -ENOMEM;
> >> -		}
> >> -		if (pf->pvar->name) {
> >> -			pf->tvar->name = strdup(pf->pvar->name);
> >> -			if (pf->tvar->name == NULL)
> >> -				return -ENOMEM;
> >> -		} else
> >> -			pf->tvar->name = NULL;
> >> -		return 0;
> >> -	}
> >> +	/* Copy raw parameters */
> >> +	if (!is_c_varname(pf->pvar->var))
> >> +		return copy_to_probe_trace_arg(pf->tvar, pf->pvar);
> >>
> >>   	if (pf->pvar->name)
> >>   		pf->tvar->name = strdup(pf->pvar->name);
> >> --
> >> 1.8.3.4
> 

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

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

* Re: [PATCH 2/2] perf probe: Support probing at absolute address
  2015-08-26  0:02   ` 平松雅巳 / HIRAMATU,MASAMI
  2015-08-26  2:38     ` Wangnan (F)
@ 2015-08-26 13:00     ` acme
  1 sibling, 0 replies; 12+ messages in thread
From: acme @ 2015-08-26 13:00 UTC (permalink / raw)
  To: 平松雅巳 / HIRAMATU,MASAMI
  Cc: 'Wang Nan', mingo, namhyung, pi3orama, linux-kernel

Em Wed, Aug 26, 2015 at 12:02:40AM +0000, 平松雅巳 / HIRAMATU,MASAMI escreveu:
> > From: Wang Nan [mailto:wangnan0@huawei.com]
> > 
> > It should be useful to allow 'perf probe' probe at absolute offset of
> > a target. For example, when (u)probing at a instruction of a shared
> > object in a embedded system where debuginfo is not avaliable but we
> > know the offset of that instruction by manually digging.
> > 
> > This patch enables following perf probe command syntax:
> > 
> >  # perf probe +0xffffffff811e6615
> > 
> > And
> > 
> >  # perf probe /lib/x86_64-linux-gnu/libc-2.19.so +0xeb860
> 
> Why do we need "+" for the absolute address?

I guess 'absolute' was the misleading term here, 'offset' would be
better, perhaps, or even 'raw' address.

> It seems that we can do it if we find that the given probe point
> starts with "0x".

Which is kinda ok, since that is the most common way of expressing
offsets, but i don't know why we would preclude using, say, decimal,
octal, etc.

- Arnaldo
 
> Thanks,
> 
> > 
> > In the above example, we don't need a anchor symbol, so it is possible
> > to compute absolute addresses using other methods and then use
> > 'perf probe' to create the probing points.
> > 
> > Signed-off-by: Wang Nan <wangnan0@huawei.com>
> > Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> > Cc: Namhyung Kim <namhyung@kernel.org>
> > ---
> >  tools/perf/util/probe-event.c  | 144 +++++++++++++++++++++++++++++++++++++----
> >  tools/perf/util/probe-event.h  |   3 +
> >  tools/perf/util/probe-finder.c |  21 +-----
> >  3 files changed, 138 insertions(+), 30 deletions(-)
> > 
> > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > index 6c7e538..59de69a4 100644
> > --- a/tools/perf/util/probe-event.c
> > +++ b/tools/perf/util/probe-event.c
> > @@ -1194,9 +1194,13 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> >  		*ptr++ = '\0';
> >  	}
> > 
> > -	tmp = strdup(arg);
> > -	if (tmp == NULL)
> > -		return -ENOMEM;
> > +	if (arg[0] == '\0')
> > +		tmp = NULL;
> > +	else {
> > +		tmp = strdup(arg);
> > +		if (tmp == NULL)
> > +			return -ENOMEM;
> > +	}
> > 
> >  	if (file_spec)
> >  		pp->file = tmp;
> > @@ -1283,11 +1287,6 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> >  		return -EINVAL;
> >  	}
> > 
> > -	if (pp->offset && !pp->function) {
> > -		semantic_error("Offset requires an entry function.\n");
> > -		return -EINVAL;
> > -	}
> > -
> >  	if (pp->retprobe && !pp->function) {
> >  		semantic_error("Return probe requires an entry function.\n");
> >  		return -EINVAL;
> > @@ -1299,6 +1298,11 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> >  		return -EINVAL;
> >  	}
> > 
> > +	if (!pp->function && !pp->offset && !pp->file) {
> > +		semantic_error("Absolute address should not be zero.\n");
> > +		return -EINVAL;
> > +	}
> > +
> >  	pr_debug("symbol:%s file:%s line:%d offset:%lu return:%d lazy:%s\n",
> >  		 pp->function, pp->file, pp->line, pp->offset, pp->retprobe,
> >  		 pp->lazy_line);
> > @@ -1609,7 +1613,7 @@ error:
> >  static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
> >  {
> >  	char *buf, *tmp;
> > -	char offs[32] = "", line[32] = "", file[32] = "";
> > +	char offs[32] = "", line[32] = "", file[32] = "", addr[32] = "";
> >  	int ret, len;
> > 
> >  	buf = zalloc(MAX_CMDLEN);
> > @@ -1622,6 +1626,11 @@ static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
> >  		if (ret <= 0)
> >  			goto error;
> >  	}
> > +	if (!pp->function) {
> > +		ret = e_snprintf(addr, 32, "0x%lx", pp->offset);
> > +		if (ret <= 0)
> > +			goto error;
> > +	}
> >  	if (pp->line) {
> >  		ret = e_snprintf(line, 32, ":%d", pp->line);
> >  		if (ret <= 0)
> > @@ -1639,9 +1648,11 @@ static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
> >  			goto error;
> >  	}
> > 
> > -	if (pp->function)
> > -		ret = e_snprintf(buf, MAX_CMDLEN, "%s%s%s%s%s", pp->function,
> > -				 offs, pp->retprobe ? "%return" : "", line,
> > +	if (pp->function || pp->offset)
> > +		ret = e_snprintf(buf, MAX_CMDLEN, "%s%s%s%s%s",
> > +				 pp->function ? : addr,
> > +				 pp->function ? offs : "",
> > +				 pp->retprobe ? "%return" : "", line,
> >  				 file);
> >  	else
> >  		ret = e_snprintf(buf, MAX_CMDLEN, "%s%s", file, line);
> > @@ -1786,6 +1797,11 @@ char *synthesize_probe_trace_command(struct probe_trace_event *tev)
> >  	if (tev->uprobes)
> >  		ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s:0x%lx",
> >  				 tp->module, tp->address);
> > +	else if (tp->symbol[0] == '0' && tp->symbol[1] == 'x')
> > +		/* Absolute address. See try_to_find_absolute_address() */
> > +		ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s%s0x%lx",
> > +				 tp->module ?: "", tp->module ? ":" : "",
> > +				 tp->address);
> >  	else
> >  		ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s%s%s+%lu",
> >  				 tp->module ?: "", tp->module ? ":" : "",
> > @@ -2572,6 +2588,87 @@ err_out:
> >  	goto out;
> >  }
> > 
> > +static int try_to_find_absolute_address(struct perf_probe_event *pev,
> > +					struct probe_trace_event **tevs)
> > +{
> > +	struct perf_probe_point *pp = &pev->point;
> > +	struct probe_trace_event *tev;
> > +	struct probe_trace_point *tp;
> > +	int i, err;
> > +
> > +	if (perf_probe_event_need_dwarf(pev) || pev->point.function)
> > +		return -EINVAL;
> > +
> > +	/*
> > +	 * This is 'perf probe /lib/libc.so +0xabcd'. Try to probe at
> > +	 * absolute address.
> > +	 *
> > +	 * Only one tev can be generated by this.
> > +	 */
> > +	*tevs = zalloc(sizeof(*tev));
> > +	if (!*tevs)
> > +		return -ENOMEM;
> > +
> > +	tev = *tevs;
> > +	tp = &tev->point;
> > +
> > +	/*
> > +	 * Don't use tp->offset, use address directly, because
> > +	 * in synthesize_probe_trace_command() address cannot be
> > +	 * zero.
> > +	 */
> > +	tp->address = pev->point.offset;
> > +	tp->retprobe = pp->retprobe;
> > +	tev->uprobes = pev->uprobes;
> > +
> > +	err = -ENOMEM;
> > +	/* Give it a '0x' leading symbol name */
> > +	if (asprintf(&tp->symbol, "0x%lx", tp->address) < 0)
> > +		goto errout;
> > +
> > +	/* For kprobe, check range */
> > +	if ((!tev->uprobes) &&
> > +	    (kprobe_warn_out_range(tev->point.symbol,
> > +				   tev->point.address))) {
> > +		err = -EACCES;
> > +		goto errout;
> > +	}
> > +
> > +	if (asprintf(&tp->realname, "abs_%lx", tp->address) < 0)
> > +		goto errout;
> > +
> > +	if (pev->target) {
> > +		tp->module = strdup(pev->target);
> > +		if (!tp->module)
> > +			goto errout;
> > +	}
> > +
> > +	if (tev->group) {
> > +		tev->group = strdup(pev->group);
> > +		if (!tev->group)
> > +			goto errout;
> > +	}
> > +
> > +	if (pev->event) {
> > +		tev->event = strdup(pev->event);
> > +		if (!tev->event)
> > +			goto errout;
> > +	}
> > +
> > +	tev->nargs = pev->nargs;
> > +	for (i = 0; i < tev->nargs; i++)
> > +		copy_to_probe_trace_arg(&tev->args[i], &pev->args[i]);
> > +
> > +	return 1;
> > +
> > +errout:
> > +	if (*tevs) {
> > +		clear_probe_trace_events(*tevs, 1);
> > +		*tevs = NULL;
> > +	}
> > +	return err;
> > +}
> > +
> >  bool __weak arch__prefers_symtab(void) { return false; }
> > 
> >  static int convert_to_probe_trace_events(struct perf_probe_event *pev,
> > @@ -2588,6 +2685,10 @@ static int convert_to_probe_trace_events(struct perf_probe_event *pev,
> >  		}
> >  	}
> > 
> > +	ret = try_to_find_absolute_address(pev, tevs);
> > +	if (ret > 0)
> > +		return ret;
> > +
> >  	if (arch__prefers_symtab() && !perf_probe_event_need_dwarf(pev)) {
> >  		ret = find_probe_trace_events_from_map(pev, tevs);
> >  		if (ret > 0)
> > @@ -2758,3 +2859,22 @@ end:
> >  	return ret;
> >  }
> > 
> > +int copy_to_probe_trace_arg(struct probe_trace_arg *tvar,
> > +			    struct perf_probe_arg *pvar)
> > +{
> > +	tvar->value = strdup(pvar->var);
> > +	if (tvar->value == NULL)
> > +		return -ENOMEM;
> > +	if (pvar->type) {
> > +		tvar->type = strdup(pvar->type);
> > +		if (tvar->type == NULL)
> > +			return -ENOMEM;
> > +	}
> > +	if (pvar->name) {
> > +		tvar->name = strdup(pvar->name);
> > +		if (tvar->name == NULL)
> > +			return -ENOMEM;
> > +	} else
> > +		tvar->name = NULL;
> > +	return 0;
> > +}
> > diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> > index 83ee95e..174a3cf 100644
> > --- a/tools/perf/util/probe-event.h
> > +++ b/tools/perf/util/probe-event.h
> > @@ -156,4 +156,7 @@ int e_snprintf(char *str, size_t size, const char *format, ...)
> >  /* Maximum index number of event-name postfix */
> >  #define MAX_EVENT_INDEX	1024
> > 
> > +int copy_to_probe_trace_arg(struct probe_trace_arg *tvar,
> > +			    struct perf_probe_arg *pvar);
> > +
> >  #endif /*_PROBE_EVENT_H */
> > diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> > index 7b80f8c..29c43c068 100644
> > --- a/tools/perf/util/probe-finder.c
> > +++ b/tools/perf/util/probe-finder.c
> > @@ -553,24 +553,9 @@ static int find_variable(Dwarf_Die *sc_die, struct probe_finder *pf)
> >  	char buf[32], *ptr;
> >  	int ret = 0;
> > 
> > -	if (!is_c_varname(pf->pvar->var)) {
> > -		/* Copy raw parameters */
> > -		pf->tvar->value = strdup(pf->pvar->var);
> > -		if (pf->tvar->value == NULL)
> > -			return -ENOMEM;
> > -		if (pf->pvar->type) {
> > -			pf->tvar->type = strdup(pf->pvar->type);
> > -			if (pf->tvar->type == NULL)
> > -				return -ENOMEM;
> > -		}
> > -		if (pf->pvar->name) {
> > -			pf->tvar->name = strdup(pf->pvar->name);
> > -			if (pf->tvar->name == NULL)
> > -				return -ENOMEM;
> > -		} else
> > -			pf->tvar->name = NULL;
> > -		return 0;
> > -	}
> > +	/* Copy raw parameters */
> > +	if (!is_c_varname(pf->pvar->var))
> > +		return copy_to_probe_trace_arg(pf->tvar, pf->pvar);
> > 
> >  	if (pf->pvar->name)
> >  		pf->tvar->name = strdup(pf->pvar->name);
> > --
> > 1.8.3.4
> 

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

* Re: [PATCH 2/2] perf probe: Support probing at absolute address
  2015-08-26  2:38     ` Wangnan (F)
  2015-08-26  9:02       ` 平松雅巳 / HIRAMATU,MASAMI
@ 2015-08-26 13:02       ` acme
  2015-08-26 13:19         ` Wangnan (F)
  1 sibling, 1 reply; 12+ messages in thread
From: acme @ 2015-08-26 13:02 UTC (permalink / raw)
  To: Wangnan (F)
  Cc: 平松雅巳 / HIRAMATU,MASAMI, mingo,
	namhyung, pi3orama, linux-kernel

Em Wed, Aug 26, 2015 at 10:38:18AM +0800, Wangnan (F) escreveu:
> 
> On 2015/8/26 8:02, 平松雅巳 / HIRAMATU,MASAMI wrote:
> >>From: Wang Nan [mailto:wangnan0@huawei.com]
> >>
> >>It should be useful to allow 'perf probe' probe at absolute offset of
> >>a target. For example, when (u)probing at a instruction of a shared
> >>object in a embedded system where debuginfo is not avaliable but we
> >>know the offset of that instruction by manually digging.
> >>
> >>This patch enables following perf probe command syntax:
> >>
> >>  # perf probe +0xffffffff811e6615
> >>
> >>And
> >>
> >>  # perf probe /lib/x86_64-linux-gnu/libc-2.19.so +0xeb860
> >Why do we need "+" for the absolute address?
> >It seems that we can do it if we find that the given probe point
> >starts with "0x".
> >
> >Thanks,
> 
> I will change 2/2 as you suggection.
> 
> However, we can only ensure that in kernel side symbol never leading
> with '0x'. Although I don't think symbol leading with 0x is useful,
> it is still possible for a userspace program compiled and linked by
> a language other than C produces such symbol. '+' helps us separate
> address and function name semantically, make us don't rely on assumption
> on function names. If in future we do meet '0x' symbols, I think we still
> need the '+' syntax back. But we can do it at that time.

Agreed, I also think that using '+' is better, but will not dwell on
this so as to make progress :-)

- Arnaldo
 
> Thank you.
> 
> 
> >>In the above example, we don't need a anchor symbol, so it is possible
> >>to compute absolute addresses using other methods and then use
> >>'perf probe' to create the probing points.
> >>
> >>Signed-off-by: Wang Nan <wangnan0@huawei.com>
> >>Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> >>Cc: Ingo Molnar <mingo@redhat.com>
> >>Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> >>Cc: Namhyung Kim <namhyung@kernel.org>
> >>---
> >>  tools/perf/util/probe-event.c  | 144 +++++++++++++++++++++++++++++++++++++----
> >>  tools/perf/util/probe-event.h  |   3 +
> >>  tools/perf/util/probe-finder.c |  21 +-----
> >>  3 files changed, 138 insertions(+), 30 deletions(-)
> >>
> >>diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> >>index 6c7e538..59de69a4 100644
> >>--- a/tools/perf/util/probe-event.c
> >>+++ b/tools/perf/util/probe-event.c
> >>@@ -1194,9 +1194,13 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> >>  		*ptr++ = '\0';
> >>  	}
> >>
> >>-	tmp = strdup(arg);
> >>-	if (tmp == NULL)
> >>-		return -ENOMEM;
> >>+	if (arg[0] == '\0')
> >>+		tmp = NULL;
> >>+	else {
> >>+		tmp = strdup(arg);
> >>+		if (tmp == NULL)
> >>+			return -ENOMEM;
> >>+	}
> >>
> >>  	if (file_spec)
> >>  		pp->file = tmp;
> >>@@ -1283,11 +1287,6 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> >>  		return -EINVAL;
> >>  	}
> >>
> >>-	if (pp->offset && !pp->function) {
> >>-		semantic_error("Offset requires an entry function.\n");
> >>-		return -EINVAL;
> >>-	}
> >>-
> >>  	if (pp->retprobe && !pp->function) {
> >>  		semantic_error("Return probe requires an entry function.\n");
> >>  		return -EINVAL;
> >>@@ -1299,6 +1298,11 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> >>  		return -EINVAL;
> >>  	}
> >>
> >>+	if (!pp->function && !pp->offset && !pp->file) {
> >>+		semantic_error("Absolute address should not be zero.\n");
> >>+		return -EINVAL;
> >>+	}
> >>+
> >>  	pr_debug("symbol:%s file:%s line:%d offset:%lu return:%d lazy:%s\n",
> >>  		 pp->function, pp->file, pp->line, pp->offset, pp->retprobe,
> >>  		 pp->lazy_line);
> >>@@ -1609,7 +1613,7 @@ error:
> >>  static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
> >>  {
> >>  	char *buf, *tmp;
> >>-	char offs[32] = "", line[32] = "", file[32] = "";
> >>+	char offs[32] = "", line[32] = "", file[32] = "", addr[32] = "";
> >>  	int ret, len;
> >>
> >>  	buf = zalloc(MAX_CMDLEN);
> >>@@ -1622,6 +1626,11 @@ static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
> >>  		if (ret <= 0)
> >>  			goto error;
> >>  	}
> >>+	if (!pp->function) {
> >>+		ret = e_snprintf(addr, 32, "0x%lx", pp->offset);
> >>+		if (ret <= 0)
> >>+			goto error;
> >>+	}
> >>  	if (pp->line) {
> >>  		ret = e_snprintf(line, 32, ":%d", pp->line);
> >>  		if (ret <= 0)
> >>@@ -1639,9 +1648,11 @@ static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
> >>  			goto error;
> >>  	}
> >>
> >>-	if (pp->function)
> >>-		ret = e_snprintf(buf, MAX_CMDLEN, "%s%s%s%s%s", pp->function,
> >>-				 offs, pp->retprobe ? "%return" : "", line,
> >>+	if (pp->function || pp->offset)
> >>+		ret = e_snprintf(buf, MAX_CMDLEN, "%s%s%s%s%s",
> >>+				 pp->function ? : addr,
> >>+				 pp->function ? offs : "",
> >>+				 pp->retprobe ? "%return" : "", line,
> >>  				 file);
> >>  	else
> >>  		ret = e_snprintf(buf, MAX_CMDLEN, "%s%s", file, line);
> >>@@ -1786,6 +1797,11 @@ char *synthesize_probe_trace_command(struct probe_trace_event *tev)
> >>  	if (tev->uprobes)
> >>  		ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s:0x%lx",
> >>  				 tp->module, tp->address);
> >>+	else if (tp->symbol[0] == '0' && tp->symbol[1] == 'x')
> >>+		/* Absolute address. See try_to_find_absolute_address() */
> >>+		ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s%s0x%lx",
> >>+				 tp->module ?: "", tp->module ? ":" : "",
> >>+				 tp->address);
> >>  	else
> >>  		ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s%s%s+%lu",
> >>  				 tp->module ?: "", tp->module ? ":" : "",
> >>@@ -2572,6 +2588,87 @@ err_out:
> >>  	goto out;
> >>  }
> >>
> >>+static int try_to_find_absolute_address(struct perf_probe_event *pev,
> >>+					struct probe_trace_event **tevs)
> >>+{
> >>+	struct perf_probe_point *pp = &pev->point;
> >>+	struct probe_trace_event *tev;
> >>+	struct probe_trace_point *tp;
> >>+	int i, err;
> >>+
> >>+	if (perf_probe_event_need_dwarf(pev) || pev->point.function)
> >>+		return -EINVAL;
> >>+
> >>+	/*
> >>+	 * This is 'perf probe /lib/libc.so +0xabcd'. Try to probe at
> >>+	 * absolute address.
> >>+	 *
> >>+	 * Only one tev can be generated by this.
> >>+	 */
> >>+	*tevs = zalloc(sizeof(*tev));
> >>+	if (!*tevs)
> >>+		return -ENOMEM;
> >>+
> >>+	tev = *tevs;
> >>+	tp = &tev->point;
> >>+
> >>+	/*
> >>+	 * Don't use tp->offset, use address directly, because
> >>+	 * in synthesize_probe_trace_command() address cannot be
> >>+	 * zero.
> >>+	 */
> >>+	tp->address = pev->point.offset;
> >>+	tp->retprobe = pp->retprobe;
> >>+	tev->uprobes = pev->uprobes;
> >>+
> >>+	err = -ENOMEM;
> >>+	/* Give it a '0x' leading symbol name */
> >>+	if (asprintf(&tp->symbol, "0x%lx", tp->address) < 0)
> >>+		goto errout;
> >>+
> >>+	/* For kprobe, check range */
> >>+	if ((!tev->uprobes) &&
> >>+	    (kprobe_warn_out_range(tev->point.symbol,
> >>+				   tev->point.address))) {
> >>+		err = -EACCES;
> >>+		goto errout;
> >>+	}
> >>+
> >>+	if (asprintf(&tp->realname, "abs_%lx", tp->address) < 0)
> >>+		goto errout;
> >>+
> >>+	if (pev->target) {
> >>+		tp->module = strdup(pev->target);
> >>+		if (!tp->module)
> >>+			goto errout;
> >>+	}
> >>+
> >>+	if (tev->group) {
> >>+		tev->group = strdup(pev->group);
> >>+		if (!tev->group)
> >>+			goto errout;
> >>+	}
> >>+
> >>+	if (pev->event) {
> >>+		tev->event = strdup(pev->event);
> >>+		if (!tev->event)
> >>+			goto errout;
> >>+	}
> >>+
> >>+	tev->nargs = pev->nargs;
> >>+	for (i = 0; i < tev->nargs; i++)
> >>+		copy_to_probe_trace_arg(&tev->args[i], &pev->args[i]);
> >>+
> >>+	return 1;
> >>+
> >>+errout:
> >>+	if (*tevs) {
> >>+		clear_probe_trace_events(*tevs, 1);
> >>+		*tevs = NULL;
> >>+	}
> >>+	return err;
> >>+}
> >>+
> >>  bool __weak arch__prefers_symtab(void) { return false; }
> >>
> >>  static int convert_to_probe_trace_events(struct perf_probe_event *pev,
> >>@@ -2588,6 +2685,10 @@ static int convert_to_probe_trace_events(struct perf_probe_event *pev,
> >>  		}
> >>  	}
> >>
> >>+	ret = try_to_find_absolute_address(pev, tevs);
> >>+	if (ret > 0)
> >>+		return ret;
> >>+
> >>  	if (arch__prefers_symtab() && !perf_probe_event_need_dwarf(pev)) {
> >>  		ret = find_probe_trace_events_from_map(pev, tevs);
> >>  		if (ret > 0)
> >>@@ -2758,3 +2859,22 @@ end:
> >>  	return ret;
> >>  }
> >>
> >>+int copy_to_probe_trace_arg(struct probe_trace_arg *tvar,
> >>+			    struct perf_probe_arg *pvar)
> >>+{
> >>+	tvar->value = strdup(pvar->var);
> >>+	if (tvar->value == NULL)
> >>+		return -ENOMEM;
> >>+	if (pvar->type) {
> >>+		tvar->type = strdup(pvar->type);
> >>+		if (tvar->type == NULL)
> >>+			return -ENOMEM;
> >>+	}
> >>+	if (pvar->name) {
> >>+		tvar->name = strdup(pvar->name);
> >>+		if (tvar->name == NULL)
> >>+			return -ENOMEM;
> >>+	} else
> >>+		tvar->name = NULL;
> >>+	return 0;
> >>+}
> >>diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> >>index 83ee95e..174a3cf 100644
> >>--- a/tools/perf/util/probe-event.h
> >>+++ b/tools/perf/util/probe-event.h
> >>@@ -156,4 +156,7 @@ int e_snprintf(char *str, size_t size, const char *format, ...)
> >>  /* Maximum index number of event-name postfix */
> >>  #define MAX_EVENT_INDEX	1024
> >>
> >>+int copy_to_probe_trace_arg(struct probe_trace_arg *tvar,
> >>+			    struct perf_probe_arg *pvar);
> >>+
> >>  #endif /*_PROBE_EVENT_H */
> >>diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> >>index 7b80f8c..29c43c068 100644
> >>--- a/tools/perf/util/probe-finder.c
> >>+++ b/tools/perf/util/probe-finder.c
> >>@@ -553,24 +553,9 @@ static int find_variable(Dwarf_Die *sc_die, struct probe_finder *pf)
> >>  	char buf[32], *ptr;
> >>  	int ret = 0;
> >>
> >>-	if (!is_c_varname(pf->pvar->var)) {
> >>-		/* Copy raw parameters */
> >>-		pf->tvar->value = strdup(pf->pvar->var);
> >>-		if (pf->tvar->value == NULL)
> >>-			return -ENOMEM;
> >>-		if (pf->pvar->type) {
> >>-			pf->tvar->type = strdup(pf->pvar->type);
> >>-			if (pf->tvar->type == NULL)
> >>-				return -ENOMEM;
> >>-		}
> >>-		if (pf->pvar->name) {
> >>-			pf->tvar->name = strdup(pf->pvar->name);
> >>-			if (pf->tvar->name == NULL)
> >>-				return -ENOMEM;
> >>-		} else
> >>-			pf->tvar->name = NULL;
> >>-		return 0;
> >>-	}
> >>+	/* Copy raw parameters */
> >>+	if (!is_c_varname(pf->pvar->var))
> >>+		return copy_to_probe_trace_arg(pf->tvar, pf->pvar);
> >>
> >>  	if (pf->pvar->name)
> >>  		pf->tvar->name = strdup(pf->pvar->name);
> >>--
> >>1.8.3.4
> 

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

* Re: [PATCH 2/2] perf probe: Support probing at absolute address
  2015-08-26 13:02       ` acme
@ 2015-08-26 13:19         ` Wangnan (F)
  2015-08-26 13:32           ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 12+ messages in thread
From: Wangnan (F) @ 2015-08-26 13:19 UTC (permalink / raw)
  To: acme
  Cc: 平松雅巳 / HIRAMATU,MASAMI, mingo,
	namhyung, pi3orama, linux-kernel



On 2015/8/26 21:02, acme@kernel.org wrote:
> Em Wed, Aug 26, 2015 at 10:38:18AM +0800, Wangnan (F) escreveu:
>> On 2015/8/26 8:02, 平松雅巳 / HIRAMATU,MASAMI wrote:
>>>> From: Wang Nan [mailto:wangnan0@huawei.com]
>>>>
>>>> It should be useful to allow 'perf probe' probe at absolute offset of
>>>> a target. For example, when (u)probing at a instruction of a shared
>>>> object in a embedded system where debuginfo is not avaliable but we
>>>> know the offset of that instruction by manually digging.
>>>>
>>>> This patch enables following perf probe command syntax:
>>>>
>>>>   # perf probe +0xffffffff811e6615
>>>>
>>>> And
>>>>
>>>>   # perf probe /lib/x86_64-linux-gnu/libc-2.19.so +0xeb860
>>> Why do we need "+" for the absolute address?
>>> It seems that we can do it if we find that the given probe point
>>> starts with "0x".
>>>
>>> Thanks,
>> I will change 2/2 as you suggection.
>>
>> However, we can only ensure that in kernel side symbol never leading
>> with '0x'. Although I don't think symbol leading with 0x is useful,
>> it is still possible for a userspace program compiled and linked by
>> a language other than C produces such symbol. '+' helps us separate
>> address and function name semantically, make us don't rely on assumption
>> on function names. If in future we do meet '0x' symbols, I think we still
>> need the '+' syntax back. But we can do it at that time.
> Agreed, I also think that using '+' is better, but will not dwell on
> this so as to make progress :-)
>
> - Arnaldo
>   

Maybe we should support both of them, making '+0x1234' the core
syntax, and '0x1234' style as a syntax sugar. However I have worked
on this problem for nearly a full day but my main work should be BPF
related things...

Since Masami has acked all of the 6 v3 patches, if we still need '+' I can
bring it back with a new patch when I have time. However, same to you,
I don't think this should be a blocking problem.

Thank you.

>> Thank you.
>>
>>
>>>> In the above example, we don't need a anchor symbol, so it is possible
>>>> to compute absolute addresses using other methods and then use
>>>> 'perf probe' to create the probing points.
>>>>
>>>> Signed-off-by: Wang Nan <wangnan0@huawei.com>
>>>> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
>>>> Cc: Ingo Molnar <mingo@redhat.com>
>>>> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>>>> Cc: Namhyung Kim <namhyung@kernel.org>
>>>> ---
>>>>   tools/perf/util/probe-event.c  | 144 +++++++++++++++++++++++++++++++++++++----
>>>>   tools/perf/util/probe-event.h  |   3 +
>>>>   tools/perf/util/probe-finder.c |  21 +-----
>>>>   3 files changed, 138 insertions(+), 30 deletions(-)
>>>>
>>>> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
>>>> index 6c7e538..59de69a4 100644
>>>> --- a/tools/perf/util/probe-event.c
>>>> +++ b/tools/perf/util/probe-event.c
>>>> @@ -1194,9 +1194,13 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
>>>>   		*ptr++ = '\0';
>>>>   	}
>>>>
>>>> -	tmp = strdup(arg);
>>>> -	if (tmp == NULL)
>>>> -		return -ENOMEM;
>>>> +	if (arg[0] == '\0')
>>>> +		tmp = NULL;
>>>> +	else {
>>>> +		tmp = strdup(arg);
>>>> +		if (tmp == NULL)
>>>> +			return -ENOMEM;
>>>> +	}
>>>>
>>>>   	if (file_spec)
>>>>   		pp->file = tmp;
>>>> @@ -1283,11 +1287,6 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
>>>>   		return -EINVAL;
>>>>   	}
>>>>
>>>> -	if (pp->offset && !pp->function) {
>>>> -		semantic_error("Offset requires an entry function.\n");
>>>> -		return -EINVAL;
>>>> -	}
>>>> -
>>>>   	if (pp->retprobe && !pp->function) {
>>>>   		semantic_error("Return probe requires an entry function.\n");
>>>>   		return -EINVAL;
>>>> @@ -1299,6 +1298,11 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
>>>>   		return -EINVAL;
>>>>   	}
>>>>
>>>> +	if (!pp->function && !pp->offset && !pp->file) {
>>>> +		semantic_error("Absolute address should not be zero.\n");
>>>> +		return -EINVAL;
>>>> +	}
>>>> +
>>>>   	pr_debug("symbol:%s file:%s line:%d offset:%lu return:%d lazy:%s\n",
>>>>   		 pp->function, pp->file, pp->line, pp->offset, pp->retprobe,
>>>>   		 pp->lazy_line);
>>>> @@ -1609,7 +1613,7 @@ error:
>>>>   static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
>>>>   {
>>>>   	char *buf, *tmp;
>>>> -	char offs[32] = "", line[32] = "", file[32] = "";
>>>> +	char offs[32] = "", line[32] = "", file[32] = "", addr[32] = "";
>>>>   	int ret, len;
>>>>
>>>>   	buf = zalloc(MAX_CMDLEN);
>>>> @@ -1622,6 +1626,11 @@ static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
>>>>   		if (ret <= 0)
>>>>   			goto error;
>>>>   	}
>>>> +	if (!pp->function) {
>>>> +		ret = e_snprintf(addr, 32, "0x%lx", pp->offset);
>>>> +		if (ret <= 0)
>>>> +			goto error;
>>>> +	}
>>>>   	if (pp->line) {
>>>>   		ret = e_snprintf(line, 32, ":%d", pp->line);
>>>>   		if (ret <= 0)
>>>> @@ -1639,9 +1648,11 @@ static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
>>>>   			goto error;
>>>>   	}
>>>>
>>>> -	if (pp->function)
>>>> -		ret = e_snprintf(buf, MAX_CMDLEN, "%s%s%s%s%s", pp->function,
>>>> -				 offs, pp->retprobe ? "%return" : "", line,
>>>> +	if (pp->function || pp->offset)
>>>> +		ret = e_snprintf(buf, MAX_CMDLEN, "%s%s%s%s%s",
>>>> +				 pp->function ? : addr,
>>>> +				 pp->function ? offs : "",
>>>> +				 pp->retprobe ? "%return" : "", line,
>>>>   				 file);
>>>>   	else
>>>>   		ret = e_snprintf(buf, MAX_CMDLEN, "%s%s", file, line);
>>>> @@ -1786,6 +1797,11 @@ char *synthesize_probe_trace_command(struct probe_trace_event *tev)
>>>>   	if (tev->uprobes)
>>>>   		ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s:0x%lx",
>>>>   				 tp->module, tp->address);
>>>> +	else if (tp->symbol[0] == '0' && tp->symbol[1] == 'x')
>>>> +		/* Absolute address. See try_to_find_absolute_address() */
>>>> +		ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s%s0x%lx",
>>>> +				 tp->module ?: "", tp->module ? ":" : "",
>>>> +				 tp->address);
>>>>   	else
>>>>   		ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s%s%s+%lu",
>>>>   				 tp->module ?: "", tp->module ? ":" : "",
>>>> @@ -2572,6 +2588,87 @@ err_out:
>>>>   	goto out;
>>>>   }
>>>>
>>>> +static int try_to_find_absolute_address(struct perf_probe_event *pev,
>>>> +					struct probe_trace_event **tevs)
>>>> +{
>>>> +	struct perf_probe_point *pp = &pev->point;
>>>> +	struct probe_trace_event *tev;
>>>> +	struct probe_trace_point *tp;
>>>> +	int i, err;
>>>> +
>>>> +	if (perf_probe_event_need_dwarf(pev) || pev->point.function)
>>>> +		return -EINVAL;
>>>> +
>>>> +	/*
>>>> +	 * This is 'perf probe /lib/libc.so +0xabcd'. Try to probe at
>>>> +	 * absolute address.
>>>> +	 *
>>>> +	 * Only one tev can be generated by this.
>>>> +	 */
>>>> +	*tevs = zalloc(sizeof(*tev));
>>>> +	if (!*tevs)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	tev = *tevs;
>>>> +	tp = &tev->point;
>>>> +
>>>> +	/*
>>>> +	 * Don't use tp->offset, use address directly, because
>>>> +	 * in synthesize_probe_trace_command() address cannot be
>>>> +	 * zero.
>>>> +	 */
>>>> +	tp->address = pev->point.offset;
>>>> +	tp->retprobe = pp->retprobe;
>>>> +	tev->uprobes = pev->uprobes;
>>>> +
>>>> +	err = -ENOMEM;
>>>> +	/* Give it a '0x' leading symbol name */
>>>> +	if (asprintf(&tp->symbol, "0x%lx", tp->address) < 0)
>>>> +		goto errout;
>>>> +
>>>> +	/* For kprobe, check range */
>>>> +	if ((!tev->uprobes) &&
>>>> +	    (kprobe_warn_out_range(tev->point.symbol,
>>>> +				   tev->point.address))) {
>>>> +		err = -EACCES;
>>>> +		goto errout;
>>>> +	}
>>>> +
>>>> +	if (asprintf(&tp->realname, "abs_%lx", tp->address) < 0)
>>>> +		goto errout;
>>>> +
>>>> +	if (pev->target) {
>>>> +		tp->module = strdup(pev->target);
>>>> +		if (!tp->module)
>>>> +			goto errout;
>>>> +	}
>>>> +
>>>> +	if (tev->group) {
>>>> +		tev->group = strdup(pev->group);
>>>> +		if (!tev->group)
>>>> +			goto errout;
>>>> +	}
>>>> +
>>>> +	if (pev->event) {
>>>> +		tev->event = strdup(pev->event);
>>>> +		if (!tev->event)
>>>> +			goto errout;
>>>> +	}
>>>> +
>>>> +	tev->nargs = pev->nargs;
>>>> +	for (i = 0; i < tev->nargs; i++)
>>>> +		copy_to_probe_trace_arg(&tev->args[i], &pev->args[i]);
>>>> +
>>>> +	return 1;
>>>> +
>>>> +errout:
>>>> +	if (*tevs) {
>>>> +		clear_probe_trace_events(*tevs, 1);
>>>> +		*tevs = NULL;
>>>> +	}
>>>> +	return err;
>>>> +}
>>>> +
>>>>   bool __weak arch__prefers_symtab(void) { return false; }
>>>>
>>>>   static int convert_to_probe_trace_events(struct perf_probe_event *pev,
>>>> @@ -2588,6 +2685,10 @@ static int convert_to_probe_trace_events(struct perf_probe_event *pev,
>>>>   		}
>>>>   	}
>>>>
>>>> +	ret = try_to_find_absolute_address(pev, tevs);
>>>> +	if (ret > 0)
>>>> +		return ret;
>>>> +
>>>>   	if (arch__prefers_symtab() && !perf_probe_event_need_dwarf(pev)) {
>>>>   		ret = find_probe_trace_events_from_map(pev, tevs);
>>>>   		if (ret > 0)
>>>> @@ -2758,3 +2859,22 @@ end:
>>>>   	return ret;
>>>>   }
>>>>
>>>> +int copy_to_probe_trace_arg(struct probe_trace_arg *tvar,
>>>> +			    struct perf_probe_arg *pvar)
>>>> +{
>>>> +	tvar->value = strdup(pvar->var);
>>>> +	if (tvar->value == NULL)
>>>> +		return -ENOMEM;
>>>> +	if (pvar->type) {
>>>> +		tvar->type = strdup(pvar->type);
>>>> +		if (tvar->type == NULL)
>>>> +			return -ENOMEM;
>>>> +	}
>>>> +	if (pvar->name) {
>>>> +		tvar->name = strdup(pvar->name);
>>>> +		if (tvar->name == NULL)
>>>> +			return -ENOMEM;
>>>> +	} else
>>>> +		tvar->name = NULL;
>>>> +	return 0;
>>>> +}
>>>> diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
>>>> index 83ee95e..174a3cf 100644
>>>> --- a/tools/perf/util/probe-event.h
>>>> +++ b/tools/perf/util/probe-event.h
>>>> @@ -156,4 +156,7 @@ int e_snprintf(char *str, size_t size, const char *format, ...)
>>>>   /* Maximum index number of event-name postfix */
>>>>   #define MAX_EVENT_INDEX	1024
>>>>
>>>> +int copy_to_probe_trace_arg(struct probe_trace_arg *tvar,
>>>> +			    struct perf_probe_arg *pvar);
>>>> +
>>>>   #endif /*_PROBE_EVENT_H */
>>>> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
>>>> index 7b80f8c..29c43c068 100644
>>>> --- a/tools/perf/util/probe-finder.c
>>>> +++ b/tools/perf/util/probe-finder.c
>>>> @@ -553,24 +553,9 @@ static int find_variable(Dwarf_Die *sc_die, struct probe_finder *pf)
>>>>   	char buf[32], *ptr;
>>>>   	int ret = 0;
>>>>
>>>> -	if (!is_c_varname(pf->pvar->var)) {
>>>> -		/* Copy raw parameters */
>>>> -		pf->tvar->value = strdup(pf->pvar->var);
>>>> -		if (pf->tvar->value == NULL)
>>>> -			return -ENOMEM;
>>>> -		if (pf->pvar->type) {
>>>> -			pf->tvar->type = strdup(pf->pvar->type);
>>>> -			if (pf->tvar->type == NULL)
>>>> -				return -ENOMEM;
>>>> -		}
>>>> -		if (pf->pvar->name) {
>>>> -			pf->tvar->name = strdup(pf->pvar->name);
>>>> -			if (pf->tvar->name == NULL)
>>>> -				return -ENOMEM;
>>>> -		} else
>>>> -			pf->tvar->name = NULL;
>>>> -		return 0;
>>>> -	}
>>>> +	/* Copy raw parameters */
>>>> +	if (!is_c_varname(pf->pvar->var))
>>>> +		return copy_to_probe_trace_arg(pf->tvar, pf->pvar);
>>>>
>>>>   	if (pf->pvar->name)
>>>>   		pf->tvar->name = strdup(pf->pvar->name);
>>>> --
>>>> 1.8.3.4



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

* Re: [PATCH 2/2] perf probe: Support probing at absolute address
  2015-08-26 13:19         ` Wangnan (F)
@ 2015-08-26 13:32           ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-08-26 13:32 UTC (permalink / raw)
  To: Wangnan (F)
  Cc: 平松雅巳 / HIRAMATU,MASAMI, mingo,
	namhyung, pi3orama, linux-kernel

Em Wed, Aug 26, 2015 at 09:19:27PM +0800, Wangnan (F) escreveu:
> On 2015/8/26 21:02, acme@kernel.org wrote:
> >Em Wed, Aug 26, 2015 at 10:38:18AM +0800, Wangnan (F) escreveu:
> >>On 2015/8/26 8:02, 平松雅巳 / HIRAMATU,MASAMI wrote:
> >>>>From: Wang Nan [mailto:wangnan0@huawei.com]
> >>>>  # perf probe /lib/x86_64-linux-gnu/libc-2.19.so +0xeb860
> >>>Why do we need "+" for the absolute address?
> >>>It seems that we can do it if we find that the given probe point
> >>>starts with "0x".

> >>I will change 2/2 as you suggestion.

> >>However, we can only ensure that in kernel side symbol never leading
> >>with '0x'. Although I don't think symbol leading with 0x is useful,
> >>it is still possible for a userspace program compiled and linked by
> >>a language other than C produces such symbol. '+' helps us separate
> >>address and function name semantically, make us don't rely on assumption
> >>on function names. If in future we do meet '0x' symbols, I think we still
> >>need the '+' syntax back. But we can do it at that time.

> >Agreed, I also think that using '+' is better, but will not dwell on
> >this so as to make progress :-)
 
> Maybe we should support both of them, making '+0x1234' the core
> syntax, and '0x1234' style as a syntax sugar. However I have worked
> on this problem for nearly a full day but my main work should be BPF
> related things...
 
> Since Masami has acked all of the 6 v3 patches, if we still need '+' I can
> bring it back with a new patch when I have time. However, same to you,
> I don't think this should be a blocking problem.

Agreed, we don't have to agree on everything all the time, lets keep
Masami happy this time :-)

- Arnaldo

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

* [tip:perf/core] perf probe: Prevent segfault when reading probe point with absolute address
  2015-08-25 13:27 [PATCH 1/2] perf probe: Prevent segfault when reading probe point with absolute address Wang Nan
                   ` (2 preceding siblings ...)
  2015-08-26  0:08 ` 平松雅巳 / HIRAMATU,MASAMI
@ 2015-08-28  6:40 ` tip-bot for Wang Nan
  3 siblings, 0 replies; 12+ messages in thread
From: tip-bot for Wang Nan @ 2015-08-28  6:40 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, masami.hiramatsu.pt, mingo, wangnan0,
	namhyung, tglx, hpa

Commit-ID:  e486367f0101d43a044404ed2fa35aa043b19667
Gitweb:     http://git.kernel.org/tip/e486367f0101d43a044404ed2fa35aa043b19667
Author:     Wang Nan <wangnan0@huawei.com>
AuthorDate: Tue, 25 Aug 2015 13:27:35 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 26 Aug 2015 10:34:11 -0300

perf probe: Prevent segfault when reading probe point with absolute address

'perf probe -l' panic if there is a manually inserted probing point with
absolute address. For example:

  # echo 'p:probe/abs_ffffffff811e6615 0xffffffff811e6615' > /sys/kernel/debug/tracing/kprobe_events
  # perf probe -l
  Segmentation fault (core dumped)

This patch fix this problem by considering the situation that
"tp->symbol == NULL" in find_perf_probe_point_from_dwarf() and
find_perf_probe_point_from_map().

After this patch:

  # perf probe -l
  probe:abs_ffffffff811e6615 (on SyS_write+5@fs/read_write.c)

And when debug info is missing:

  # rm -rf ~/.debug
  # mv /lib/modules/4.2.0-rc1+/build/vmlinux /lib/modules/4.2.0-rc1+/build/vmlinux.bak
  # perf probe -l
  probe:abs_ffffffff811e6615 (on sys_write+5)

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/r/1440509256-193590-1-git-send-email-wangnan0@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-event.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index f07374b..6c7e538 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -515,7 +515,7 @@ static int find_perf_probe_point_from_dwarf(struct probe_trace_point *tp,
 		if (ret < 0)
 			goto error;
 		addr += stext;
-	} else {
+	} else if (tp->symbol) {
 		addr = kernel_get_symbol_address_by_name(tp->symbol, false);
 		if (addr == 0)
 			goto error;
@@ -1815,17 +1815,17 @@ static int find_perf_probe_point_from_map(struct probe_trace_point *tp,
 {
 	struct symbol *sym = NULL;
 	struct map *map;
-	u64 addr;
+	u64 addr = tp->address;
 	int ret = -ENOENT;
 
 	if (!is_kprobe) {
 		map = dso__new_map(tp->module);
 		if (!map)
 			goto out;
-		addr = tp->address;
 		sym = map__find_symbol(map, addr, NULL);
 	} else {
-		addr = kernel_get_symbol_address_by_name(tp->symbol, true);
+		if (tp->symbol)
+			addr = kernel_get_symbol_address_by_name(tp->symbol, true);
 		if (addr) {
 			addr += tp->offset;
 			sym = __find_kernel_function(addr, &map);

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

end of thread, other threads:[~2015-08-28  6:41 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-25 13:27 [PATCH 1/2] perf probe: Prevent segfault when reading probe point with absolute address Wang Nan
2015-08-25 13:27 ` [PATCH 2/2] perf probe: Support probing at " Wang Nan
2015-08-26  0:02   ` 平松雅巳 / HIRAMATU,MASAMI
2015-08-26  2:38     ` Wangnan (F)
2015-08-26  9:02       ` 平松雅巳 / HIRAMATU,MASAMI
2015-08-26 13:02       ` acme
2015-08-26 13:19         ` Wangnan (F)
2015-08-26 13:32           ` Arnaldo Carvalho de Melo
2015-08-26 13:00     ` acme
2015-08-25 15:32 ` [PATCH 1/2] perf probe: Prevent segfault when reading probe point with " Arnaldo Carvalho de Melo
2015-08-26  0:08 ` 平松雅巳 / HIRAMATU,MASAMI
2015-08-28  6:40 ` [tip:perf/core] " tip-bot for Wang Nan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).