[PATCH v3 1/3] perf probe: Remove length limitation for showing available variables

[PATCH v3 1/3] perf probe: Remove length limitation for showing available variables

[He Kuang]

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 <hekuang@huawei.com>
Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 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

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

[He Kuang]

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 <hekuang@huawei.com>
---
 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));
 			}
-- 
1.8.5.2

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@hitachi.com>

Thank you!

> 
> Signed-off-by: He Kuang <hekuang@huawei.com>
> ---
>  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@hitachi.com

[tip:perf/core] perf probe: Add --range option to show a variable 's location range

[tip-bot for He Kuang]

Commit-ID:  349e8d2611316cce11c0b9d0830ebb585c9b82b8
Gitweb:     http://git.kernel.org/tip/349e8d2611316cce11c0b9d0830ebb585c9b82b8
Author:     He Kuang <hekuang@huawei.com>
AuthorDate: Mon, 11 May 2015 09:25:03 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 12 May 2015 09:59:50 -0300

perf probe: Add --range option to show a variable's location range

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 the scope of the probe point
are showed with a byte offset range, and can be added according to this
range information.

For example, there are some variables in the 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 fail 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 the source code do not match the assembly, so a variable
may not be available in the source code line where it appears.

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 the 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 clear for us to add a 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@huawei.com>
Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1431336304-16863-2-git-send-email-hekuang@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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 d7c2e90..5804086 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))
@@ -1270,13 +1280,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));
 			}

[PATCH v3 3/3] perf probe: Show better error message when failed to find variable

[He Kuang]

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 <hekuang@huawei.com>
Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 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 f032329..8510cde 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -529,7 +529,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

Re: [PATCH v3 3/3] perf probe: Show better error message when failed to find variable

[Ingo Molnar]

* He Kuang <hekuang@huawei.com> 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.

What does this sentence mean? I thought 'address' means 'location of 
bytes'. So the address identifies the location and obviously we know 
that. So this message wants to say something else.

>    Perhaps, it has been optimized out.
>    Use -V with --range option to show variable location range.

This suggestion is useful.

>     Error: Failed to add events.

Thanks,

	Ingo

Re: [PATCH v3 3/3] perf probe: Show better error message when failed to find variable

[He Kuang]

Hi, Ingo

On 2015/5/11 17:30, Ingo Molnar wrote:
> * He Kuang <hekuang@huawei.com> 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.
> What does this sentence mean? I thought 'address' means 'location of
> bytes'. So the address identifies the location and obviously we know
> that. So this message wants to say something else.

'generic_perform_write' is a function name, while 'bytes' is a local
variable in this function. Maybe the variable I chose make you confused.

This maybe clear:
   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.
> This suggestion is useful.
>
>>      Error: Failed to add events.
> Thanks,
>
> 	Ingo
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

Re: [PATCH v3 3/3] perf probe: Show better error message when failed to find variable

[Ingo Molnar]

* He Kuang <hekuang@huawei.com> wrote:

> Hi, Ingo
> 
> On 2015/5/11 17:30, Ingo Molnar wrote:
> >* He Kuang <hekuang@huawei.com> 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.
> >What does this sentence mean? I thought 'address' means 'location of
> >bytes'. So the address identifies the location and obviously we know
> >that. So this message wants to say something else.
> 
> 'generic_perform_write' is a function name, while 'bytes' is a local
> variable in this function. Maybe the variable I chose make you confused.
> 
> This maybe clear:
>   Failed to find the location of 'bytes' at this address.

Yeah, absolutely! This highlights the importance of putting 
user-supplied symbols into quotes and such.

Maybe even write:

    Failed to find the location of the 'bytes' variable at this address.

Another question: what does 'generic_perform_write+118' mean here? Why 
the offset?

Thanks,

	Ingo

Re: [PATCH v3 3/3] perf probe: Show better error message when failed to find variable

[He Kuang]

On 2015/5/11 17:50, Ingo Molnar wrote:
> * He Kuang <hekuang@huawei.com> wrote:
>
>> Hi, Ingo
>>
>> On 2015/5/11 17:30, Ingo Molnar wrote:
>>> * He Kuang <hekuang@huawei.com> 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.
>>> What does this sentence mean? I thought 'address' means 'location of
>>> bytes'. So the address identifies the location and obviously we know
>>> that. So this message wants to say something else.
>> 'generic_perform_write' is a function name, while 'bytes' is a local
>> variable in this function. Maybe the variable I chose make you confused.
>>
>> This maybe clear:
>>    Failed to find the location of 'bytes' at this address.
> Yeah, absolutely! This highlights the importance of putting
> user-supplied symbols into quotes and such.
>
> Maybe even write:
>
>      Failed to find the location of the 'bytes' variable at this address.
>
> Another question: what does 'generic_perform_write+118' mean here? Why
> the offset?

This is for setting a kprobe at the location has an offset of 118
bytes to the entry of the function 'generic_perform_write'.

In the previous patch: https://lkml.org/lkml/2015/5/11/170,
we show a valid byte offset range of variables in scope, 118 is
an invalid address which can be read from the result, this offset
is just for showing the error message.

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

Re: Re: [PATCH v3 3/3] perf probe: Show better error message when failed to find variable

[Masami Hiramatsu]

On 2015/05/11 19:15, He Kuang wrote:
> 
> 
> On 2015/5/11 17:50, Ingo Molnar wrote:
>> * He Kuang <hekuang@huawei.com> wrote:
>>
>>> Hi, Ingo
>>>
>>> On 2015/5/11 17:30, Ingo Molnar wrote:
>>>> * He Kuang <hekuang@huawei.com> 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.
>>>> What does this sentence mean? I thought 'address' means 'location of
>>>> bytes'. So the address identifies the location and obviously we know
>>>> that. So this message wants to say something else.
>>> 'generic_perform_write' is a function name, while 'bytes' is a local
>>> variable in this function. Maybe the variable I chose make you confused.
>>>
>>> This maybe clear:
>>>    Failed to find the location of 'bytes' at this address.
>> Yeah, absolutely! This highlights the importance of putting
>> user-supplied symbols into quotes and such.
>>
>> Maybe even write:
>>
>>      Failed to find the location of the 'bytes' variable at this address.

OK, He, could you also include this fix?

Thank you,


>>
>> Another question: what does 'generic_perform_write+118' mean here? Why
>> the offset?
> 
> This is for setting a kprobe at the location has an offset of 118
> bytes to the entry of the function 'generic_perform_write'.
> 
> In the previous patch: https://lkml.org/lkml/2015/5/11/170,
> we show a valid byte offset range of variables in scope, 118 is
> an invalid address which can be read from the result, this offset
> is just for showing the error message.
> 
>>
>> Thanks,
>>
>> 	Ingo
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/
>>
> 
> 
> 


-- 
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@hitachi.com

Re: Re: [PATCH v3 3/3] perf probe: Show better error message when failed to find variable

[Arnaldo Carvalho de Melo]

Em Mon, May 11, 2015 at 09:02:47PM +0900, Masami Hiramatsu escreveu:
> On 2015/05/11 19:15, He Kuang wrote:
> > On 2015/5/11 17:50, Ingo Molnar wrote:
> >> * He Kuang <hekuang@huawei.com> wrote:
> >>> On 2015/5/11 17:30, Ingo Molnar wrote:
> >>>>> After this patch:
> >>>>>    $ perf probe --add 'generic_perform_write+118 bytes'
> >>>>>    Failed to find the location of bytes at this address.
> >>>> What does this sentence mean? I thought 'address' means 'location of
> >>>> bytes'. So the address identifies the location and obviously we know
> >>>> that. So this message wants to say something else.
> >>> 'generic_perform_write' is a function name, while 'bytes' is a local
> >>> variable in this function. Maybe the variable I chose make you confused.

> >>> This maybe clear:
> >>>    Failed to find the location of 'bytes' at this address.
> >> Yeah, absolutely! This highlights the importance of putting
> >> user-supplied symbols into quotes and such.

> >> Maybe even write:

> >>      Failed to find the location of the 'bytes' variable at this address.
 
> OK, He, could you also include this fix?

I agree with the change, makes things clearer, will do the change
myself. If He has any objection to that, I can fix things up before
pushing it to Ingo,

Thanks,

- Arnaldo

Re: Re: [PATCH v3 3/3] perf probe: Show better error message when failed to find variable

[Arnaldo Carvalho de Melo]

Em Mon, May 11, 2015 at 10:35:27AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Mon, May 11, 2015 at 09:02:47PM +0900, Masami Hiramatsu escreveu:
> > On 2015/05/11 19:15, He Kuang wrote:
> > > On 2015/5/11 17:50, Ingo Molnar wrote:
> > >> * He Kuang <hekuang@huawei.com> wrote:
> > >>> On 2015/5/11 17:30, Ingo Molnar wrote:
> > >>>>> After this patch:
> > >>>>>    $ perf probe --add 'generic_perform_write+118 bytes'
> > >>>>>    Failed to find the location of bytes at this address.
> > >>>> What does this sentence mean? I thought 'address' means 'location of
> > >>>> bytes'. So the address identifies the location and obviously we know
> > >>>> that. So this message wants to say something else.
> > >>> 'generic_perform_write' is a function name, while 'bytes' is a local
> > >>> variable in this function. Maybe the variable I chose make you confused.
> 
> > >>> This maybe clear:
> > >>>    Failed to find the location of 'bytes' at this address.
> > >> Yeah, absolutely! This highlights the importance of putting
> > >> user-supplied symbols into quotes and such.
> 
> > >> Maybe even write:
> 
> > >>      Failed to find the location of the 'bytes' variable at this address.
>  
> > OK, He, could you also include this fix?
> 
> I agree with the change, makes things clearer, will do the change
> myself. If He has any objection to that, I can fix things up before
> pushing it to Ingo,

So, this is the end result:

-       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);
-       else if (ret == -ENOTSUP)
+       if (ret == -ENOENT || ret == -EINVAL) {
+               pr_err("Failed to find the location of the '%s' variable at this address.\n"
+                      " Perhaps it has been optimized out.\n"
+                      " Use -V with the --range option to show '%s' location range.\n",
+                      pf->pvar->var, pf->pvar->var);
+       } else if (ret == -ENOTSUP)

Re: Re: Re: [PATCH v3 3/3] perf probe: Show better error message when failed to find variable

[Masami Hiramatsu]

On 2015/05/11 22:44, Arnaldo Carvalho de Melo wrote:
> Em Mon, May 11, 2015 at 10:35:27AM -0300, Arnaldo Carvalho de Melo escreveu:
>> Em Mon, May 11, 2015 at 09:02:47PM +0900, Masami Hiramatsu escreveu:
>>> On 2015/05/11 19:15, He Kuang wrote:
>>>> On 2015/5/11 17:50, Ingo Molnar wrote:
>>>>> * He Kuang <hekuang@huawei.com> wrote:
>>>>>> On 2015/5/11 17:30, Ingo Molnar wrote:
>>>>>>>> After this patch:
>>>>>>>>    $ perf probe --add 'generic_perform_write+118 bytes'
>>>>>>>>    Failed to find the location of bytes at this address.
>>>>>>> What does this sentence mean? I thought 'address' means 'location of
>>>>>>> bytes'. So the address identifies the location and obviously we know
>>>>>>> that. So this message wants to say something else.
>>>>>> 'generic_perform_write' is a function name, while 'bytes' is a local
>>>>>> variable in this function. Maybe the variable I chose make you confused.
>>
>>>>>> This maybe clear:
>>>>>>    Failed to find the location of 'bytes' at this address.
>>>>> Yeah, absolutely! This highlights the importance of putting
>>>>> user-supplied symbols into quotes and such.
>>
>>>>> Maybe even write:
>>
>>>>>      Failed to find the location of the 'bytes' variable at this address.
>>  
>>> OK, He, could you also include this fix?
>>
>> I agree with the change, makes things clearer, will do the change
>> myself. If He has any objection to that, I can fix things up before
>> pushing it to Ingo,

Opps, I missed this...

> 
> So, this is the end result:
> 
> -       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);
> -       else if (ret == -ENOTSUP)
> +       if (ret == -ENOENT || ret == -EINVAL) {
> +               pr_err("Failed to find the location of the '%s' variable at this address.\n"
> +                      " Perhaps it has been optimized out.\n"
> +                      " Use -V with the --range option to show '%s' location range.\n",
> +                      pf->pvar->var, pf->pvar->var);
> +       } else if (ret == -ENOTSUP)

OK, I ack this change.

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: masami.hiramatsu.pt@hitachi.com

Re: [PATCH v3 3/3] perf probe: Show better error message when failed to find variable

[Ingo Molnar]

* He Kuang <hekuang@huawei.com> 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.

Btw., you talk about 'variables', but generic_perform_write() is a 
function name?

Thanks,

	Ingo

[tip:perf/core] perf probe: Show better error message when failed to find variable

[tip-bot for He Kuang]

Commit-ID:  7d5eaba9b33682b734e3a79c21c4a9a5f91624b1
Gitweb:     http://git.kernel.org/tip/7d5eaba9b33682b734e3a79c21c4a9a5f91624b1
Author:     He Kuang <hekuang@huawei.com>
AuthorDate: Mon, 11 May 2015 09:25:04 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 12 May 2015 09:59:50 -0300

perf probe: Show better error message when failed to find variable

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 the 'bytes' variable at this address.
   Perhaps it has been optimized out.
   Use -V with the --range option to show 'bytes' location range.
    Error: Failed to add events.

Signed-off-by: He Kuang <hekuang@huawei.com>
Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1431336304-16863-3-git-send-email-hekuang@huawei.com
[ Improve the error message based on lkml thread ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-finder.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 5804086..590a24a 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -527,10 +527,12 @@ static int convert_variable(Dwarf_Die *vr_die, struct probe_finder *pf)
 
 	ret = convert_variable_location(vr_die, pf->addr, pf->fb_ops,
 					&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);
-	else if (ret == -ENOTSUP)
+	if (ret == -ENOENT || ret == -EINVAL) {
+		pr_err("Failed to find the location of the '%s' variable at this address.\n"
+		       " Perhaps it has been optimized out.\n"
+		       " Use -V with the --range option to show '%s' location range.\n",
+		       pf->pvar->var, 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) {
 		ret = convert_variable_fields(vr_die, pf->pvar->var,

[tip:perf/core] perf probe: Remove length limitation for showing available variables

[tip-bot for He Kuang]

Commit-ID:  fb9596d1731cc37da6489de439f8b876f3a12db2
Gitweb:     http://git.kernel.org/tip/fb9596d1731cc37da6489de439f8b876f3a12db2
Author:     He Kuang <hekuang@huawei.com>
AuthorDate: Mon, 11 May 2015 09:25:02 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 12 May 2015 09:59:49 -0300

perf probe: Remove length limitation for showing available variables

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 <hekuang@huawei.com>
Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1431336304-16863-1-git-send-email-hekuang@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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 8b9e274..d7c2e90 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1255,14 +1255,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];
@@ -1274,10 +1271,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);
 		}
 	}