Hi, Masami On 2015/5/10 11:21, Masami Hiramatsu wrote:
On 2015/05/09 18:55, 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]>Thanks, this looks easier to understand :) [...]diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c index dcca551..30a1a1b 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 -10000Hmm, could you use -ERANGE instead of this? Other part is OK for me. Thank you!
I've checked libdw, it never returns -ERANGE, but there is an
errno conflict in the function convert_variable_location itself:
268 regs = get_arch_regstr(regn);
269 if (!regs) {
270 /* This should be a bug in DWARF or this tool */
271 pr_warning("Mapping for the register number %u "
272 "missing on this architecture.\n", regn);
273 return -ERANGE;
274 }
So shell we change the above errno to -ENOENT or choose another
errno for current 'VARIABLE_LOCATION_INVALID_AT_ADDR', what's
your opinion?
Thanks!
+ /* Dwarf FL wrappers */ static char *debuginfo_path; /* Currently dummy */@@ -177,7 +180,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 +190,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 = 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 +223,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 +233,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 +263,7 @@ static_var: }if (!tvar)- return 0; + return ret2;regs = get_arch_regstr(regn);if (!regs) { @@ -269,7 +282,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 +1281,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 == VARIABLE_LOCATION_INVALID_AT_ADDR) { + 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)); }
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
