Re: [PATCH v3 2/3] perf probe: Add --range option to show variable location range

[Masami Hiramatsu]

On 2015/05/11 18:25, He Kuang wrote:
> It is not easy for users to get the accurate byte offset or the line
> number where a local variable can be probed. With '--range' option,
> local variables in scope of the probe point are showed with byte offset
> range, and can be added according to this range information.
> 
> For example, there are some variables in function
> generic_perform_write():
> 
>   <generic_perform_write@mm/filemap.c:0>
>   0  ssize_t generic_perform_write(struct file *file,
>   1                                 struct iov_iter *i, loff_t pos)
>   2  {
>   3          struct address_space *mapping = file->f_mapping;
>   4          const struct address_space_operations *a_ops = mapping->a_ops;
>   ...
>   42                 status = a_ops->write_begin(file, mapping, pos, bytes, flags,
>                                                &page, &fsdata);
>   44                 if (unlikely(status < 0))
> 
> But we got failed when we try to probe the variable 'a_ops' at line 42
> or 44.
> 
>   $ perf probe --add 'generic_perform_write:42 a_ops'
>   Failed to find the location of a_ops at this address.
>     Perhaps, it has been optimized out.
> 
> This is because source code do not match assembly, so a variable may not
> be available in the sourcecode line where it presents. After this patch,
> we can lookup the accurate byte offset range of a variable, 'INV'
> indicates that this variable is not valid at the given point, but
> available in scope:
> 
>   $ perf probe --vars 'generic_perform_write:42' --range
>   Available variables at generic_perform_write:42
>         @<generic_perform_write+141>
>                 [INV]   ssize_t written @<generic_perform_write+[324-331]>
>                 [INV]   struct address_space_operations*        a_ops   @<generic_perform_write+[55-61,170-176,223-246]>
>                 [VAL]   (unknown_type)  fsdata  @<generic_perform_write+[70-307,346-411]>
>                 [VAL]   loff_t  pos     @<generic_perform_write+[0-286,286-336,346-411]>
>                 [VAL]   long int        status  @<generic_perform_write+[83-342,346-411]>
>                 [VAL]   long unsigned int       bytes   @<generic_perform_write+[122-311,320-338,346-403,403-411]>
>                 [VAL]   struct address_space*   mapping @<generic_perform_write+[35-344,346-411]>
>                 [VAL]   struct iov_iter*        i       @<generic_perform_write+[0-340,346-411]>
>                 [VAL]   struct page*    page    @<generic_perform_write+[70-307,346-411]>
> 
> Then it is more clearly for us to do the probe with this variable:
> 
>   $ perf probe --add 'generic_perform_write+170 a_ops'
>   Added new event:
>     probe:generic_perform_write (on generic_perform_write+170 with a_ops)

Looks good to me:)

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

Thank you!

> 
> Signed-off-by: He Kuang <hekuang@xxxxxxxxxx>
> ---
>  tools/perf/builtin-probe.c     |   2 +
>  tools/perf/util/dwarf-aux.c    | 121 +++++++++++++++++++++++++++++++++++++++++
>  tools/perf/util/dwarf-aux.h    |   2 +
>  tools/perf/util/probe-event.h  |   1 +
>  tools/perf/util/probe-finder.c |  57 ++++++++++++++-----
>  5 files changed, 170 insertions(+), 13 deletions(-)
> 
> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> index 7fa2c7a..1272559 100644
> --- a/tools/perf/builtin-probe.c
> +++ b/tools/perf/builtin-probe.c
> @@ -372,6 +372,8 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
>  		     "Show accessible variables on PROBEDEF", opt_show_vars),
>  	OPT_BOOLEAN('\0', "externs", &probe_conf.show_ext_vars,
>  		    "Show external variables too (with --vars only)"),
> +	OPT_BOOLEAN('\0', "range", &probe_conf.show_location_range,
> +		"Show variables location range in scope (with --vars only)"),
>  	OPT_STRING('k', "vmlinux", &symbol_conf.vmlinux_name,
>  		   "file", "vmlinux pathname"),
>  	OPT_STRING('s', "source", &symbol_conf.source_prefix,
> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> index 737c9db..afa0971 100644
> --- a/tools/perf/util/dwarf-aux.c
> +++ b/tools/perf/util/dwarf-aux.c
> @@ -913,3 +913,124 @@ int die_get_varname(Dwarf_Die *vr_die, struct strbuf *buf)
>  	return 0;
>  }
>  
> +/**
> + * die_get_var_innermost_scope - Get innermost scope range of given variable DIE
> + * @sp_die: a subprogram DIE
> + * @vr_die: a variable DIE
> + * @buf: a strbuf for variable byte offset range
> + *
> + * Get the innermost scope range of @vr_die and stores it in @buf as
> + * "@<function_name+[NN-NN,NN-NN]>".
> + */
> +static int die_get_var_innermost_scope(Dwarf_Die *sp_die, Dwarf_Die *vr_die,
> +				struct strbuf *buf)
> +{
> +	Dwarf_Die *scopes;
> +	int count;
> +	size_t offset = 0;
> +	Dwarf_Addr base;
> +	Dwarf_Addr start, end;
> +	Dwarf_Addr entry;
> +	int ret;
> +	bool first = true;
> +	const char *name;
> +
> +	ret = dwarf_entrypc(sp_die, &entry);
> +	if (ret)
> +		return ret;
> +
> +	name = dwarf_diename(sp_die);
> +	if (!name)
> +		return -ENOENT;
> +
> +	count = dwarf_getscopes_die(vr_die, &scopes);
> +
> +	/* (*SCOPES)[1] is the DIE for the scope containing that scope */
> +	if (count <= 1) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	while ((offset = dwarf_ranges(&scopes[1], offset, &base,
> +				&start, &end)) > 0) {
> +		start -= entry;
> +		end -= entry;
> +
> +		if (first) {
> +			strbuf_addf(buf, "@<%s+[%lu-%lu",
> +				name, start, end);
> +			first = false;
> +		} else {
> +			strbuf_addf(buf, ",%lu-%lu",
> +				start, end);
> +		}
> +	}
> +
> +	if (!first)
> +		strbuf_addf(buf, "]>");
> +
> +out:
> +	free(scopes);
> +	return ret;
> +}
> +
> +/**
> + * die_get_var_range - Get byte offset range of given variable DIE
> + * @sp_die: a subprogram DIE
> + * @vr_die: a variable DIE
> + * @buf: a strbuf for type and variable name and byte offset range
> + *
> + * Get the byte offset range of @vr_die and stores it in @buf as
> + * "@<function_name+[NN-NN,NN-NN]>".
> + */
> +int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, struct strbuf *buf)
> +{
> +	int ret = 0;
> +	Dwarf_Addr base;
> +	Dwarf_Addr start, end;
> +	Dwarf_Addr entry;
> +	Dwarf_Op *op;
> +	size_t nops;
> +	size_t offset = 0;
> +	Dwarf_Attribute attr;
> +	bool first = true;
> +	const char *name;
> +
> +	ret = dwarf_entrypc(sp_die, &entry);
> +	if (ret)
> +		return ret;
> +
> +	name = dwarf_diename(sp_die);
> +	if (!name)
> +		return -ENOENT;
> +
> +	if (dwarf_attr(vr_die, DW_AT_location, &attr) == NULL)
> +		return -EINVAL;
> +
> +	while ((offset = dwarf_getlocations(
> +				&attr, offset, &base,
> +				&start, &end, &op, &nops)) > 0) {
> +		if (start == 0) {
> +			/* Single Location Descriptions */
> +			ret = die_get_var_innermost_scope(sp_die, vr_die, buf);
> +			return ret;
> +		}
> +
> +		/* Location Lists */
> +		start -= entry;
> +		end -= entry;
> +		if (first) {
> +			strbuf_addf(buf, "@<%s+[%lu-%lu",
> +				name, start, end);
> +			first = false;
> +		} else {
> +			strbuf_addf(buf, ",%lu-%lu",
> +				start, end);
> +		}
> +	}
> +
> +	if (!first)
> +		strbuf_addf(buf, "]>");
> +
> +	return ret;
> +}
> diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
> index 60676fd..c154c0b8 100644
> --- a/tools/perf/util/dwarf-aux.h
> +++ b/tools/perf/util/dwarf-aux.h
> @@ -121,4 +121,6 @@ extern int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf);
>  
>  /* Get the name and type of given variable DIE, stored as "type\tname" */
>  extern int die_get_varname(Dwarf_Die *vr_die, struct strbuf *buf);
> +extern int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die,
> +			struct strbuf *buf);
>  #endif
> diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> index 1e2faa3..537eb32 100644
> --- a/tools/perf/util/probe-event.h
> +++ b/tools/perf/util/probe-event.h
> @@ -9,6 +9,7 @@
>  /* Probe related configurations */
>  struct probe_conf {
>  	bool	show_ext_vars;
> +	bool	show_location_range;
>  	bool	force_add;
>  	bool	no_inlines;
>  	int	max_probes;
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index dcca551..f032329 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -177,7 +177,7 @@ static int convert_variable_location(Dwarf_Die *vr_die, Dwarf_Addr addr,
>  	Dwarf_Word offs = 0;
>  	bool ref = false;
>  	const char *regs;
> -	int ret;
> +	int ret, ret2 = 0;
>  
>  	if (dwarf_attr(vr_die, DW_AT_external, &attr) != NULL)
>  		goto static_var;
> @@ -187,9 +187,19 @@ static int convert_variable_location(Dwarf_Die *vr_die, Dwarf_Addr addr,
>  		return -EINVAL;	/* Broken DIE ? */
>  	if (dwarf_getlocation_addr(&attr, addr, &op, &nops, 1) <= 0) {
>  		ret = dwarf_entrypc(sp_die, &tmp);
> -		if (ret || addr != tmp ||
> -		    dwarf_tag(vr_die) != DW_TAG_formal_parameter ||
> -		    dwarf_highpc(sp_die, &tmp))
> +		if (ret)
> +			return -ENOENT;
> +
> +		if (probe_conf.show_location_range &&
> +			(dwarf_tag(vr_die) == DW_TAG_variable)) {
> +			ret2 = -ERANGE;
> +		} else if (addr != tmp ||
> +			dwarf_tag(vr_die) != DW_TAG_formal_parameter) {
> +			return -ENOENT;
> +		}
> +
> +		ret = dwarf_highpc(sp_die, &tmp);
> +		if (ret)
>  			return -ENOENT;
>  		/*
>  		 * This is fuzzed by fentry mcount. We try to find the
> @@ -210,7 +220,7 @@ found:
>  	if (op->atom == DW_OP_addr) {
>  static_var:
>  		if (!tvar)
> -			return 0;
> +			return ret2;
>  		/* Static variables on memory (not stack), make @varname */
>  		ret = strlen(dwarf_diename(vr_die));
>  		tvar->value = zalloc(ret + 2);
> @@ -220,7 +230,7 @@ static_var:
>  		tvar->ref = alloc_trace_arg_ref((long)offs);
>  		if (tvar->ref == NULL)
>  			return -ENOMEM;
> -		return 0;
> +		return ret2;
>  	}
>  
>  	/* If this is based on frame buffer, set the offset */
> @@ -250,14 +260,14 @@ static_var:
>  	}
>  
>  	if (!tvar)
> -		return 0;
> +		return ret2;
>  
>  	regs = get_arch_regstr(regn);
>  	if (!regs) {
>  		/* This should be a bug in DWARF or this tool */
>  		pr_warning("Mapping for the register number %u "
>  			   "missing on this architecture.\n", regn);
> -		return -ERANGE;
> +		return -ENOTSUP;
>  	}
>  
>  	tvar->value = strdup(regs);
> @@ -269,7 +279,7 @@ static_var:
>  		if (tvar->ref == NULL)
>  			return -ENOMEM;
>  	}
> -	return 0;
> +	return ret2;
>  }
>  
>  #define BYTES_TO_BITS(nb)	((nb) * BITS_PER_LONG / sizeof(long))
> @@ -1268,13 +1278,34 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data)
>  		ret = convert_variable_location(die_mem, af->pf.addr,
>  						af->pf.fb_ops, &af->pf.sp_die,
>  						NULL);
> -		if (ret == 0) {
> +		if (ret == 0 || ret == -ERANGE) {
> +			int ret2;
> +			bool externs = !af->child;
>  			struct strbuf buf;
>  
>  			strbuf_init(&buf, 64);
> -			ret = die_get_varname(die_mem, &buf);
> -			pr_debug2("Add new var: %s\n", buf.buf);
> -			if (ret == 0) {
> +
> +			if (probe_conf.show_location_range) {
> +				if (!externs) {
> +					if (ret)
> +						strbuf_addf(&buf, "[INV]\t");
> +					else
> +						strbuf_addf(&buf, "[VAL]\t");
> +				} else
> +					strbuf_addf(&buf, "[EXT]\t");
> +			}
> +
> +			ret2 = die_get_varname(die_mem, &buf);
> +
> +			if (!ret2 && probe_conf.show_location_range &&
> +				!externs) {
> +				strbuf_addf(&buf, "\t");
> +				ret2 = die_get_var_range(&af->pf.sp_die,
> +							die_mem, &buf);
> +			}
> +
> +			pr_debug("Add new var: %s\n", buf.buf);
> +			if (ret2 == 0) {
>  				strlist__add(vl->vars,
>  					strbuf_detach(&buf, NULL));
>  			}
> 


-- 
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: masami.hiramatsu.pt@xxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/