Date:	Wed, 26 Aug 2015 09:49:35 +0000
From:	平松雅巳 / HIRAMATU，MASAMI 
	<masami.hiramatsu.pt@...achi.com>
To:	"'Wang Nan'" <wangnan0@...wei.com>,
	"acme@...nel.org" <acme@...nel.org>,
	"rostedt@...dmis.org" <rostedt@...dmis.org>
CC:	"mingo@...hat.com" <mingo@...hat.com>,
	"namhyung@...nel.org" <namhyung@...nel.org>,
	"pi3orama@....com" <pi3orama@....com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	sysp-manager <cti.systems-productivity-manager.ts@...achi.com>
Subject: RE: [PATCH v2 3/4] perf probe: Support probing at absolute address

> From: Wang Nan [mailto:wangnan0@...wei.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
> 
> 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.

OK, this looks good :)

> 
> This patch also fix a bug that, when offset is provided but function is
> skipped, parse_perf_probe_point() will give function a "" string, so
> the checking code at the end of this function is not used. For example:
> 
>  # perf probe +0x1234
>  Failed to find symbol  in kernel
>    Error: Failed to add events.
> 
> After this patch:
> 
>  # perf probe +0x1234
>  Semantic error :Offset requires an entry function.
>    Error: Command Parse Error.

Hmm, could you please split the bugfix from feature addition patch?

Thank you,

> 
> v1 -> v2:
>   Drop the leading '+' in cmdline;
>   Allow uprobing at offset 0x0;
>   Improve 'perf probe -l' result when uprobe at area without debuginfo.
> 
> Test result:
> 
>  # perf probe 0xffffffff8119d175 %ax
>  # perf probe sys_write %ax
>  # perf probe /lib64/libc-2.18.so 0x0 %ax
>  # perf probe /lib64/libc-2.18.so 0x5 %ax
>  # perf probe /lib64/libc-2.18.so 0xd8e40 %ax
>  # perf probe /lib64/libc-2.18.so __write %ax
>  # perf probe /lib64/libc-2.18.so 0xd8e49 %ax
>  # cat /sys/kernel/debug/tracing/uprobe_events
> 
>  p:probe_libc/abs_0 /lib64/libc-2.18.so:0x          (null) arg1=%ax
>  p:probe_libc/abs_5 /lib64/libc-2.18.so:0x0000000000000005 arg1=%ax
>  p:probe_libc/abs_d8e40 /lib64/libc-2.18.so:0x00000000000d8e40 arg1=%ax
>  p:probe_libc/__write /lib64/libc-2.18.so:0x00000000000d8e40 arg1=%ax
>  p:probe_libc/abs_d8e49 /lib64/libc-2.18.so:0x00000000000d8e49 arg1=%ax
> 
>  # cat /sys/kernel/debug/tracing/kprobe_events
> 
>  p:probe/abs_ffffffff8119d175 0xffffffff8119d175 arg1=%ax
>  p:probe/sys_write _text+1692016 arg1=%ax
> 
>  # perf probe -l
> 
>  Failed to find debug information for address 5
>    probe:abs_ffffffff8119d175 (on sys_write+5 with arg1)
>    probe:sys_write      (on sys_write with arg1)
>    probe_libc:__write   (on @unix/syscall-template.S:81 in /lib64/libc-2.18.so with arg1)
>    probe_libc:abs_0     (on 0x0 in /lib64/libc-2.18.so with arg1)
>    probe_libc:abs_5     (on 0x5 in /lib64/libc-2.18.so with arg1)
>    probe_libc:abs_d8e40 (on @unix/syscall-template.S:81 in /lib64/libc-2.18.so with arg1)
>    probe_libc:abs_d8e49 (on __GI___libc_write+9 in /lib64/libc-2.18.so with arg1)
> 
> Signed-off-by: Wang Nan <wangnan0@...wei.com>
> Cc: Arnaldo Carvalho de Melo <acme@...nel.org>
> Cc: Ingo Molnar <mingo@...hat.com>
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
> Cc: Namhyung Kim <namhyung@...nel.org>
> Cc: Steven Rostedt <rostedt@...dmis.org>
> ---
>  tools/perf/util/probe-event.c  | 172 ++++++++++++++++++++++++++++++++++++++---
>  tools/perf/util/probe-event.h  |   4 +
>  tools/perf/util/probe-finder.c |  21 +----
>  3 files changed, 170 insertions(+), 27 deletions(-)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 3ce223d..b94a8d7 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -1196,15 +1196,37 @@ 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;
> -	else
> +	else {
>  		pp->function = tmp;
> 
> +		/*
> +		 * Keep pp->function even if this is absolute address,
> +		 * so it can mark whether abs_address is valid.
> +		 * Which make 'perf probe lib.bin 0x0' possible.
> +		 *
> +		 * Note that checking length of tmp is not needed
> +		 * because when we access tmp[1] we know tmp[0] is '0',
> +		 * so tmp[1] should always valid (but could be '\0').
> +		 */
> +		if (tmp && !strncmp(tmp, "0x", 2)) {
> +			pp->abs_address = strtoul(pp->function, &tmp, 0);
> +			if (*tmp != '\0') {
> +				semantic_error("Invalid absolute address.\n");
> +				return -EINVAL;
> +			}
> +		}
> +	}
> +
>  	/* Parse other options */
>  	while (ptr) {
>  		arg = ptr;
> @@ -1802,14 +1824,29 @@ char *synthesize_probe_trace_command(struct probe_trace_event *tev)
>  	if (len <= 0)
>  		goto error;
> 
> -	/* Uprobes must have tp->address and tp->module */
> -	if (tev->uprobes && (!tp->address || !tp->module))
> +	/* Uprobes must have tp->module */
> +	if (tev->uprobes && !tp->module)
>  		goto error;
> +	/*
> +	 * If tp->address == 0, then this point must be a
> +	 * absolute address uprobe.
> +	 * try_to_find_absolute_address() should have made
> +	 * tp->symbol to "0x0".
> +	 */
> +	if (tev->uprobes && !tp->address) {
> +		if (!tp->symbol || strcmp(tp->symbol, "0x0"))
> +			goto error;
> +	}
> 
>  	/* Use the tp->address for uprobes */
>  	if (tev->uprobes)
>  		ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s:0x%lx",
>  				 tp->module, tp->address);
> +	else if (!strncmp(tp->symbol, "0x", 2))
> +		/* 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 ? ":" : "",
> @@ -1872,8 +1909,8 @@ out:
>  }
> 
>  static int convert_to_perf_probe_point(struct probe_trace_point *tp,
> -					struct perf_probe_point *pp,
> -					bool is_kprobe)
> +				       struct perf_probe_point *pp,
> +				       bool is_kprobe)
>  {
>  	char buf[128];
>  	int ret;
> @@ -2334,7 +2371,9 @@ static int probe_trace_event__set_name(struct probe_trace_event *tev,
>  	if (pev->event)
>  		event = pev->event;
>  	else
> -		if (pev->point.function && !strisglob(pev->point.function))
> +		if (pev->point.function &&
> +			(strncmp(pev->point.function, "0x", 2) != 0) &&
> +			!strisglob(pev->point.function))
>  			event = pev->point.function;
>  		else
>  			event = tev->point.realname;
> @@ -2604,6 +2643,98 @@ 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 (!(pev->point.function && !strncmp(pev->point.function, "0x", 2)))
> +		return -EINVAL;
> +	if (perf_probe_event_need_dwarf(pev))
> +		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.abs_address;
> +	tp->retprobe = pp->retprobe;
> +	tev->uprobes = pev->uprobes;
> +
> +	err = -ENOMEM;
> +	/*
> +	 * Give it a '0x' leading symbol name.
> +	 * In __add_probe_trace_events, a NULL symbol is interpreted as
> +	 * invalud.
> +	 */
> +	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;
> +	tev->args = zalloc(sizeof(struct probe_trace_arg) * tev->nargs);
> +	if (!tev->args) {
> +		err = -ENOMEM;
> +		goto errout;
> +	}
> +	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,
> @@ -2620,6 +2751,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)
> @@ -2787,3 +2922,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 9c7b99e..de0dd13 100644
> --- a/tools/perf/util/probe-event.h
> +++ b/tools/perf/util/probe-event.h
> @@ -61,6 +61,7 @@ struct perf_probe_point {
>  	bool		retprobe;	/* Return probe flag */
>  	char		*lazy_line;	/* Lazy matching pattern */
>  	unsigned long	offset;		/* Offset from function entry */
> +	unsigned long	abs_address;	/* Absolute address of the point */
>  };
> 
>  /* Perf probe probing argument field chain */
> @@ -162,4 +163,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 4b1e074..5ab9cd6 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

Powered by blists - more mailing lists