lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <554DBA1D.6040702@huawei.com>
Date:	Sat, 9 May 2015 15:41:17 +0800
From:	He Kuang <hekuang@...wei.com>
To:	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
	<a.p.zijlstra@...llo.nl>, <acme@...nel.org>, <jolsa@...nel.org>,
	<mingo@...hat.com>
CC:	<wangnan0@...wei.com>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/3] perf probe: Add --range option to show variable location
 range


On 2015/5/8 22:08, Masami Hiramatsu wrote:
> On 2015/05/08 21:23, 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.
> Interesting idea :)
>
>> For example, there are some variables in function
>> generic_perform_write():
>>
>>    <generic_perform_write@...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.
> Yeah, right. That's why I've introduced --vars option.
>
>> 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 [byte offset]: <324-331>
>>                  [INV]   struct address_space_operations*        a_ops   [byte offset]: <55-61>,<170-176>,<223-246>
>>                  [VAL]   (unknown_type)  fsdata  [byte offset]: <70-307>,<346-411>
>>                  [VAL]   loff_t  pos     [byte offset]: <0-286>,<286-336>,<346-411>
>>                  [VAL]   long int        status  [byte offset]: <83-342>,<346-411>
>>                  [VAL]   long unsigned int       bytes   [byte offset]: <122-311>,<320-338>,<346-403>,<403-411>
>>                  [VAL]   struct address_space*   mapping [byte offset]: <35-344>,<346-411>
>>                  [VAL]   struct iov_iter*        i       [byte offset]: <0-340>,<346-411>
>>                  [VAL]   struct page*    page    [byte offset]: <70-307>,<346-411>
> OK, at first, I don't like frequently repeated "[byte offset]", I prefer to
> show the function name+[offset range], like below :)
>
>   [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]>

OK.

>
> By the way, 'generic_perform_write+170' may be different from
> given line, is that OK?

I think the prefix '[INV]' indicates that difference.

Before this patch, we should reference objdump, dwarf-info, then
dwarf-loc to find the valid range of a variable. Sometimes we
want to view more than one variables by adding one probe event,
to find a place two or more variables all valid is difficult.

This patch gives an overview of the valid ranges of variables in
scope, in most case, if we just want to find a place to probe
variables, the location range result is just enough.  And we can
use the result offset to locate the assembly code of the accurate
meaning of a variable from objdump easily.

>
> Thank you,
>> 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)
>>
>> Signed-off-by: He Kuang <hekuang@...wei.com>
>> ---
>>   tools/perf/builtin-probe.c     |   6 ++-
>>   tools/perf/util/dwarf-aux.c    | 103 +++++++++++++++++++++++++++++++++++++++++
>>   tools/perf/util/dwarf-aux.h    |   2 +
>>   tools/perf/util/probe-finder.c |  69 ++++++++++++++++++++-------
>>   tools/perf/util/probe-finder.h |   3 +-
>>   5 files changed, 165 insertions(+), 18 deletions(-)
>>
>> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
>> index 19ebe93..1b20004 100644
>> --- a/tools/perf/builtin-probe.c
>> +++ b/tools/perf/builtin-probe.c
>> @@ -52,6 +52,7 @@ static struct {
>>   	bool list_events;
>>   	bool force_add;
>>   	bool show_ext_vars;
>> +	bool show_location_range;
>>   	bool uprobes;
>>   	bool quiet;
>>   	bool target_used;
>> @@ -375,6 +376,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", &params.show_ext_vars,
>>   		    "Show external variables too (with --vars only)"),
>> +	OPT_BOOLEAN('\0', "range", &params.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,
>> @@ -479,7 +482,8 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
>>   			params.filter = strfilter__new(DEFAULT_VAR_FILTER,
>>   						       NULL);
>>   
>> -		var_flags = params.show_ext_vars << VAR_FLAGS_EXTERN;
>> +		var_flags = params.show_ext_vars << VAR_FLAGS_EXTERN |
>> +			params.show_location_range << VAR_FLAGS_LOCATION_RANGE;
>>   
>>   		ret = show_available_vars(params.events, params.nevents,
>>   					  params.max_probe_points,
>> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
>> index 75d264e..23053c0 100644
>> --- a/tools/perf/util/dwarf-aux.c
>> +++ b/tools/perf/util/dwarf-aux.c
>> @@ -897,3 +897,106 @@ 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
>> + * "[byte offset]: <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;
>> +
>> +	ret = dwarf_entrypc(sp_die, &entry);
>> +	if (ret)
>> +		return ret;
>> +
>> +	count = dwarf_getscopes_die(vr_die, &scopes);
>> +
>> +	/* (*SCOPES)[1] is the DIE for the scope containing that scope */
>> +	if (count <= 1) {
>> +		ret = -EINVAL;
>> +		goto end;
>> +	}
>> +
>> +	while ((offset = dwarf_ranges(&scopes[1], offset, &base,
>> +				&start, &end)) > 0) {
>> +		start -= entry;
>> +		end -= entry;
>> +
>> +		if (first) {
>> +			strbuf_addf(buf, "[byte offset]: <%ld-%ld>",
>> +				start, end);
>> +			first = false;
>> +		} else {
>> +			strbuf_addf(buf, ",<%ld-%ld>",
>> +				start, end);
>> +		}
>> +	}
>> +end:
>> +	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
>> + * "[byte offset]: <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;
>> +
>> +	ret = dwarf_entrypc(sp_die, &entry);
>> +	if (ret)
>> +		return ret;
>> +
>> +	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);
>> +		} else {
>> +			/* Location Lists */
>> +			start -= entry;
>> +			end -= entry;
>> +			if (first) {
>> +				strbuf_addf(buf, "[byte offset]: <%ld-%ld>",
>> +					start, end);
>> +				first = false;
>> +			} else {
>> +				strbuf_addf(buf, ",<%ld-%ld>",
>> +					start, end);
>> +			}
>> +		}
>> +	}
>> +
>> +	return ret;
>> +}
>> diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
>> index c0d6173..8390cde 100644
>> --- a/tools/perf/util/dwarf-aux.h
>> +++ b/tools/perf/util/dwarf-aux.h
>> @@ -118,4 +118,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-finder.c b/tools/perf/util/probe-finder.c
>> index 6bbdf3f..af0a7ea 100644
>> --- a/tools/perf/util/probe-finder.c
>> +++ b/tools/perf/util/probe-finder.c
>> @@ -43,6 +43,9 @@
>>   /* Kprobe tracer basic type is up to u64 */
>>   #define MAX_BASIC_TYPE_BITS	64
>>   
>> +/* Variable location invalid at addr but valid in scope */
>> +#define VARIABLE_LOCATION_INVALID_AT_ADDR	-10000
>> +
>>   /* Dwarf FL wrappers */
>>   static char *debuginfo_path;	/* Currently dummy */
>>   
>> @@ -167,7 +170,8 @@ static struct probe_trace_arg_ref *alloc_trace_arg_ref(long offs)
>>    */
>>   static int convert_variable_location(Dwarf_Die *vr_die, Dwarf_Addr addr,
>>   				     Dwarf_Op *fb_ops, Dwarf_Die *sp_die,
>> -				     struct probe_trace_arg *tvar)
>> +				     struct probe_trace_arg *tvar,
>> +				     unsigned long *var_flags)
>>   {
>>   	Dwarf_Attribute attr;
>>   	Dwarf_Addr tmp = 0;
>> @@ -177,7 +181,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 +191,20 @@ 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 (var_flags &&
>> +			test_bit(VAR_FLAGS_LOCATION_RANGE, var_flags) &&
>> +			(dwarf_tag(vr_die) == DW_TAG_variable)) {
>> +			ret2 = VARIABLE_LOCATION_INVALID_AT_ADDR;
>> +		} 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 +225,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 +235,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,7 +265,7 @@ static_var:
>>   	}
>>   
>>   	if (!tvar)
>> -		return 0;
>> +		return ret2;
>>   
>>   	regs = get_arch_regstr(regn);
>>   	if (!regs) {
>> @@ -269,7 +284,7 @@ static_var:
>>   		if (tvar->ref == NULL)
>>   			return -ENOMEM;
>>   	}
>> -	return 0;
>> +	return ret2;
>>   }
>>   
>>   #define BYTES_TO_BITS(nb)	((nb) * BITS_PER_LONG / sizeof(long))
>> @@ -516,7 +531,7 @@ static int convert_variable(Dwarf_Die *vr_die, struct probe_finder *pf)
>>   		 dwarf_diename(vr_die));
>>   
>>   	ret = convert_variable_location(vr_die, pf->addr, pf->fb_ops,
>> -					&pf->sp_die, pf->tvar);
>> +					&pf->sp_die, pf->tvar, NULL);
>>   	if (ret == -ENOENT || ret == -EINVAL)
>>   		pr_err("Failed to find the location of %s at this address.\n"
>>   		       " Perhaps, it has been optimized out.\n", pf->pvar->var);
>> @@ -1105,7 +1120,7 @@ static int copy_variables_cb(Dwarf_Die *die_mem, void *data)
>>   	    (tag == DW_TAG_variable && vf->vars)) {
>>   		if (convert_variable_location(die_mem, vf->pf->addr,
>>   					      vf->pf->fb_ops, &pf->sp_die,
>> -					      NULL) == 0) {
>> +					      NULL, NULL) == 0) {
>>   			vf->args[vf->nargs].var = (char *)dwarf_diename(die_mem);
>>   			if (vf->args[vf->nargs].var == NULL) {
>>   				vf->ret = -ENOMEM;
>> @@ -1254,11 +1269,33 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data)
>>   	    tag == DW_TAG_variable) {
>>   		ret = convert_variable_location(die_mem, af->pf.addr,
>>   						af->pf.fb_ops, &af->pf.sp_die,
>> -						NULL);
>> -		if (ret == 0) {
>> -			ret = die_get_varname(die_mem, &buf);
>> -			pr_debug2("Add new var: %s\n", buf.buf);
>> -			if (ret == 0) {
>> +						NULL, &af->var_flags);
>> +		if (ret == 0 || ret == VARIABLE_LOCATION_INVALID_AT_ADDR) {
>> +			int ret2;
>> +			bool show_range = test_bit(VAR_FLAGS_LOCATION_RANGE,
>> +						&af->var_flags);
>> +			bool externs = !af->child;
>> +
>> +			if (show_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 && show_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));
>>   			}
>> diff --git a/tools/perf/util/probe-finder.h b/tools/perf/util/probe-finder.h
>> index 95bb79d..4b30a9b 100644
>> --- a/tools/perf/util/probe-finder.h
>> +++ b/tools/perf/util/probe-finder.h
>> @@ -11,7 +11,8 @@
>>   #define MAX_PROBE_ARGS		 128
>>   
>>   enum var_flags_bits {
>> -	VAR_FLAGS_EXTERN
>> +	VAR_FLAGS_EXTERN,
>> +	VAR_FLAGS_LOCATION_RANGE
>>   };
>>   
>>   #define PROBE_ARG_VARS		"$vars"
>>
>


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ