Use struct strbuf instead of bare char[] to remove the length limitation
of variables in variable_list, so they will not disappear due to
overlength, and make preparation for adding more description for
variables.
Signed-off-by: He Kuang <[email protected]>
---
tools/perf/util/dwarf-aux.c | 50 +++++++++++++++++++-----------------------
tools/perf/util/dwarf-aux.h | 4 ++--
tools/perf/util/probe-finder.c | 17 ++++++++------
3 files changed, 34 insertions(+), 37 deletions(-)
diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index 16d46e2..737c9db 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -848,19 +848,17 @@ Dwarf_Die *die_find_member(Dwarf_Die *st_die, const char *name,
/**
* die_get_typename - Get the name of given variable DIE
* @vr_die: a variable DIE
- * @buf: a buffer for result type name
- * @len: a max-length of @buf
+ * @buf: a strbuf for result type name
*
- * Get the name of @vr_die and stores it to @buf. Return the actual length
- * of type name if succeeded. Return -E2BIG if @len is not enough long, and
- * Return -ENOENT if failed to find type name.
+ * Get the name of @vr_die and stores it to @buf. Return 0 if succeeded.
+ * and Return -ENOENT if failed to find type name.
* Note that the result will stores typedef name if possible, and stores
* "*(function_type)" if the type is a function pointer.
*/
-int die_get_typename(Dwarf_Die *vr_die, char *buf, int len)
+int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf)
{
Dwarf_Die type;
- int tag, ret, ret2;
+ int tag, ret;
const char *tmp = "";
if (__die_get_real_type(vr_die, &type) == NULL)
@@ -871,8 +869,8 @@ int die_get_typename(Dwarf_Die *vr_die, char *buf, int len)
tmp = "*";
else if (tag == DW_TAG_subroutine_type) {
/* Function pointer */
- ret = snprintf(buf, len, "(function_type)");
- return (ret >= len) ? -E2BIG : ret;
+ strbuf_addf(buf, "(function_type)");
+ return 0;
} else {
if (!dwarf_diename(&type))
return -ENOENT;
@@ -883,39 +881,35 @@ int die_get_typename(Dwarf_Die *vr_die, char *buf, int len)
else if (tag == DW_TAG_enumeration_type)
tmp = "enum ";
/* Write a base name */
- ret = snprintf(buf, len, "%s%s", tmp, dwarf_diename(&type));
- return (ret >= len) ? -E2BIG : ret;
- }
- ret = die_get_typename(&type, buf, len);
- if (ret > 0) {
- ret2 = snprintf(buf + ret, len - ret, "%s", tmp);
- ret = (ret2 >= len - ret) ? -E2BIG : ret2 + ret;
+ strbuf_addf(buf, "%s%s", tmp, dwarf_diename(&type));
+ return 0;
}
+ ret = die_get_typename(&type, buf);
+ if (ret == 0)
+ strbuf_addf(buf, "%s", tmp);
+
return ret;
}
/**
* die_get_varname - Get the name and type of given variable DIE
* @vr_die: a variable DIE
- * @buf: a buffer for type and variable name
- * @len: the max-length of @buf
+ * @buf: a strbuf for type and variable name
*
* Get the name and type of @vr_die and stores it in @buf as "type\tname".
*/
-int die_get_varname(Dwarf_Die *vr_die, char *buf, int len)
+int die_get_varname(Dwarf_Die *vr_die, struct strbuf *buf)
{
- int ret, ret2;
+ int ret;
- ret = die_get_typename(vr_die, buf, len);
+ ret = die_get_typename(vr_die, buf);
if (ret < 0) {
pr_debug("Failed to get type, make it unknown.\n");
- ret = snprintf(buf, len, "(unknown_type)");
- }
- if (ret > 0) {
- ret2 = snprintf(buf + ret, len - ret, "\t%s",
- dwarf_diename(vr_die));
- ret = (ret2 >= len - ret) ? -E2BIG : ret2 + ret;
+ strbuf_addf(buf, "(unknown_type)");
}
- return ret;
+
+ strbuf_addf(buf, "\t%s", dwarf_diename(vr_die));
+
+ return 0;
}
diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
index 50a3cdc..60676fd 100644
--- a/tools/perf/util/dwarf-aux.h
+++ b/tools/perf/util/dwarf-aux.h
@@ -117,8 +117,8 @@ extern Dwarf_Die *die_find_member(Dwarf_Die *st_die, const char *name,
Dwarf_Die *die_mem);
/* Get the name of given variable DIE */
-extern int die_get_typename(Dwarf_Die *vr_die, char *buf, int len);
+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, char *buf, int len);
+extern int die_get_varname(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 d5f60c0..dcca551 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1253,14 +1253,11 @@ int debuginfo__find_trace_events(struct debuginfo *dbg,
return (ret < 0) ? ret : tf.ntevs;
}
-#define MAX_VAR_LEN 64
-
/* Collect available variables in this scope */
static int collect_variables_cb(Dwarf_Die *die_mem, void *data)
{
struct available_var_finder *af = data;
struct variable_list *vl;
- char buf[MAX_VAR_LEN];
int tag, ret;
vl = &af->vls[af->nvls - 1];
@@ -1272,10 +1269,16 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data)
af->pf.fb_ops, &af->pf.sp_die,
NULL);
if (ret == 0) {
- ret = die_get_varname(die_mem, buf, MAX_VAR_LEN);
- pr_debug2("Add new var: %s\n", buf);
- if (ret > 0)
- strlist__add(vl->vars, buf);
+ 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) {
+ strlist__add(vl->vars,
+ strbuf_detach(&buf, NULL));
+ }
+ strbuf_release(&buf);
}
}
--
1.8.5.2
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)
Signed-off-by: He Kuang <[email protected]>
---
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 | 58 ++++++++++++++++----
5 files changed, 172 insertions(+), 12 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..92fc1a0 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
+ * "[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;
+ 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
+ * "[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;
+ 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..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 -10000
+
/* 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));
}
--
1.8.5.2
Indicate to check variable location range in error message when we got
failed to find the variable.
Before this patch:
$ perf probe --add 'generic_perform_write+118 bytes'
Failed to find the location of bytes at this address.
Perhaps, it has been optimized out.
Error: Failed to add events.
After this patch:
$ perf probe --add 'generic_perform_write+118 bytes'
Failed to find the location of bytes at this address.
Perhaps, it has been optimized out.
Use -V with --range option to show variable location range.
Error: Failed to add events.
Signed-off-by: He Kuang <[email protected]>
---
tools/perf/util/probe-finder.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 30a1a1b..2b91323 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -532,7 +532,9 @@ static int convert_variable(Dwarf_Die *vr_die, struct probe_finder *pf)
&pf->sp_die, pf->tvar);
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);
+ " Perhaps, it has been optimized out.\n"
+ " Use -V with --range option to show variable location range.\n",
+ pf->pvar->var);
else if (ret == -ENOTSUP)
pr_err("Sorry, we don't support this variable location yet.\n");
else if (ret == 0 && pf->pvar->field) {
--
1.8.5.2
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 -10000
Hmm, could you use -ERANGE instead of this?
Other part is OK for me.
Thank you!
> +
> /* 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));
> }
>
--
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: [email protected]
On 2015/05/09 18:55, He Kuang wrote:
> Indicate to check variable location range in error message when we got
> failed to find the variable.
>
> Before this patch:
>
> $ perf probe --add 'generic_perform_write+118 bytes'
> Failed to find the location of bytes at this address.
> Perhaps, it has been optimized out.
> Error: Failed to add events.
>
> After this patch:
> $ perf probe --add 'generic_perform_write+118 bytes'
> Failed to find the location of bytes at this address.
> Perhaps, it has been optimized out.
> Use -V with --range option to show variable location range.
> Error: Failed to add events.
OK, this helps users :)
Acked-by: Masami Hiramatsu <[email protected]>
Thank you,
>
> Signed-off-by: He Kuang <[email protected]>
> ---
> tools/perf/util/probe-finder.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index 30a1a1b..2b91323 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -532,7 +532,9 @@ static int convert_variable(Dwarf_Die *vr_die, struct probe_finder *pf)
> &pf->sp_die, pf->tvar);
> 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);
> + " Perhaps, it has been optimized out.\n"
> + " Use -V with --range option to show variable location range.\n",
> + pf->pvar->var);
> else if (ret == -ENOTSUP)
> pr_err("Sorry, we don't support this variable location yet.\n");
> else if (ret == 0 && pf->pvar->field) {
>
--
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: [email protected]
On 2015/05/09 18:55, He Kuang wrote:
> Use struct strbuf instead of bare char[] to remove the length limitation
> of variables in variable_list, so they will not disappear due to
> overlength, and make preparation for adding more description for
> variables.
>
Looks good to me!
Acked-by: Masami Hiramatsu <[email protected]>
Thank you,
> Signed-off-by: He Kuang <[email protected]>
> ---
> tools/perf/util/dwarf-aux.c | 50 +++++++++++++++++++-----------------------
> tools/perf/util/dwarf-aux.h | 4 ++--
> tools/perf/util/probe-finder.c | 17 ++++++++------
> 3 files changed, 34 insertions(+), 37 deletions(-)
>
> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> index 16d46e2..737c9db 100644
> --- a/tools/perf/util/dwarf-aux.c
> +++ b/tools/perf/util/dwarf-aux.c
> @@ -848,19 +848,17 @@ Dwarf_Die *die_find_member(Dwarf_Die *st_die, const char *name,
> /**
> * die_get_typename - Get the name of given variable DIE
> * @vr_die: a variable DIE
> - * @buf: a buffer for result type name
> - * @len: a max-length of @buf
> + * @buf: a strbuf for result type name
> *
> - * Get the name of @vr_die and stores it to @buf. Return the actual length
> - * of type name if succeeded. Return -E2BIG if @len is not enough long, and
> - * Return -ENOENT if failed to find type name.
> + * Get the name of @vr_die and stores it to @buf. Return 0 if succeeded.
> + * and Return -ENOENT if failed to find type name.
> * Note that the result will stores typedef name if possible, and stores
> * "*(function_type)" if the type is a function pointer.
> */
> -int die_get_typename(Dwarf_Die *vr_die, char *buf, int len)
> +int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf)
> {
> Dwarf_Die type;
> - int tag, ret, ret2;
> + int tag, ret;
> const char *tmp = "";
>
> if (__die_get_real_type(vr_die, &type) == NULL)
> @@ -871,8 +869,8 @@ int die_get_typename(Dwarf_Die *vr_die, char *buf, int len)
> tmp = "*";
> else if (tag == DW_TAG_subroutine_type) {
> /* Function pointer */
> - ret = snprintf(buf, len, "(function_type)");
> - return (ret >= len) ? -E2BIG : ret;
> + strbuf_addf(buf, "(function_type)");
> + return 0;
> } else {
> if (!dwarf_diename(&type))
> return -ENOENT;
> @@ -883,39 +881,35 @@ int die_get_typename(Dwarf_Die *vr_die, char *buf, int len)
> else if (tag == DW_TAG_enumeration_type)
> tmp = "enum ";
> /* Write a base name */
> - ret = snprintf(buf, len, "%s%s", tmp, dwarf_diename(&type));
> - return (ret >= len) ? -E2BIG : ret;
> - }
> - ret = die_get_typename(&type, buf, len);
> - if (ret > 0) {
> - ret2 = snprintf(buf + ret, len - ret, "%s", tmp);
> - ret = (ret2 >= len - ret) ? -E2BIG : ret2 + ret;
> + strbuf_addf(buf, "%s%s", tmp, dwarf_diename(&type));
> + return 0;
> }
> + ret = die_get_typename(&type, buf);
> + if (ret == 0)
> + strbuf_addf(buf, "%s", tmp);
> +
> return ret;
> }
>
> /**
> * die_get_varname - Get the name and type of given variable DIE
> * @vr_die: a variable DIE
> - * @buf: a buffer for type and variable name
> - * @len: the max-length of @buf
> + * @buf: a strbuf for type and variable name
> *
> * Get the name and type of @vr_die and stores it in @buf as "type\tname".
> */
> -int die_get_varname(Dwarf_Die *vr_die, char *buf, int len)
> +int die_get_varname(Dwarf_Die *vr_die, struct strbuf *buf)
> {
> - int ret, ret2;
> + int ret;
>
> - ret = die_get_typename(vr_die, buf, len);
> + ret = die_get_typename(vr_die, buf);
> if (ret < 0) {
> pr_debug("Failed to get type, make it unknown.\n");
> - ret = snprintf(buf, len, "(unknown_type)");
> - }
> - if (ret > 0) {
> - ret2 = snprintf(buf + ret, len - ret, "\t%s",
> - dwarf_diename(vr_die));
> - ret = (ret2 >= len - ret) ? -E2BIG : ret2 + ret;
> + strbuf_addf(buf, "(unknown_type)");
> }
> - return ret;
> +
> + strbuf_addf(buf, "\t%s", dwarf_diename(vr_die));
> +
> + return 0;
> }
>
> diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
> index 50a3cdc..60676fd 100644
> --- a/tools/perf/util/dwarf-aux.h
> +++ b/tools/perf/util/dwarf-aux.h
> @@ -117,8 +117,8 @@ extern Dwarf_Die *die_find_member(Dwarf_Die *st_die, const char *name,
> Dwarf_Die *die_mem);
>
> /* Get the name of given variable DIE */
> -extern int die_get_typename(Dwarf_Die *vr_die, char *buf, int len);
> +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, char *buf, int len);
> +extern int die_get_varname(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 d5f60c0..dcca551 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -1253,14 +1253,11 @@ int debuginfo__find_trace_events(struct debuginfo *dbg,
> return (ret < 0) ? ret : tf.ntevs;
> }
>
> -#define MAX_VAR_LEN 64
> -
> /* Collect available variables in this scope */
> static int collect_variables_cb(Dwarf_Die *die_mem, void *data)
> {
> struct available_var_finder *af = data;
> struct variable_list *vl;
> - char buf[MAX_VAR_LEN];
> int tag, ret;
>
> vl = &af->vls[af->nvls - 1];
> @@ -1272,10 +1269,16 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data)
> af->pf.fb_ops, &af->pf.sp_die,
> NULL);
> if (ret == 0) {
> - ret = die_get_varname(die_mem, buf, MAX_VAR_LEN);
> - pr_debug2("Add new var: %s\n", buf);
> - if (ret > 0)
> - strlist__add(vl->vars, buf);
> + 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) {
> + strlist__add(vl->vars,
> + strbuf_detach(&buf, NULL));
> + }
> + strbuf_release(&buf);
> }
> }
>
>
--
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: [email protected]
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 -10000
> Hmm, 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));
>> }
>>
>
On 2015/05/11 11:22, He Kuang wrote:
> 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 -10000
>> Hmm, 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?
OK, above should be changed to -ENOTSUP, since it does not seem to come
from an invalid range :)
Thank you,
--
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: [email protected]