linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] perf probe: Remove length limitation for showing available variables
@ 2015-05-08 12:23 He Kuang
  2015-05-08 12:23 ` [PATCH 2/3] perf probe: Add flag for options when showing variables He Kuang
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: He Kuang @ 2015-05-08 12:23 UTC (permalink / raw)
  To: masami.hiramatsu.pt, a.p.zijlstra, acme, jolsa, mingo
  Cc: wangnan0, linux-kernel

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>
---
 tools/perf/util/dwarf-aux.c    | 50 +++++++++++++++++++-----------------------
 tools/perf/util/dwarf-aux.h    |  4 ++--
 tools/perf/util/probe-finder.c | 16 ++++++++------
 3 files changed, 33 insertions(+), 37 deletions(-)

diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index c34e024..75d264e 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -832,19 +832,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)
@@ -855,8 +853,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;
@@ -867,39 +865,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 af7dbcd..c0d6173 100644
--- a/tools/perf/util/dwarf-aux.h
+++ b/tools/perf/util/dwarf-aux.h
@@ -114,8 +114,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 63d3389..7b645ff40 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1238,17 +1238,16 @@ 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];
+	struct strbuf buf;
 	int tag, ret;
 
 	vl = &af->vls[af->nvls - 1];
+	strbuf_init(&buf, 64);
 
 	tag = dwarf_tag(die_mem);
 	if (tag == DW_TAG_formal_parameter ||
@@ -1257,10 +1256,13 @@ 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);
+			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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/3] perf probe: Add flag for options when showing variables
  2015-05-08 12:23 [PATCH 1/3] perf probe: Remove length limitation for showing available variables He Kuang
@ 2015-05-08 12:23 ` He Kuang
  2015-05-08 13:15   ` Masami Hiramatsu
  2015-05-08 12:23 ` [PATCH 3/3] perf probe: Add --range option to show variable location range He Kuang
  2015-05-08 14:17 ` [PATCH 1/3] perf probe: Remove length limitation for showing available variables Masami Hiramatsu
  2 siblings, 1 reply; 10+ messages in thread
From: He Kuang @ 2015-05-08 12:23 UTC (permalink / raw)
  To: masami.hiramatsu.pt, a.p.zijlstra, acme, jolsa, mingo
  Cc: wangnan0, linux-kernel

Change current boolean flag 'externs' to unsigned long 'var_flags' for
more options related to variables. These options work with --vars only.

Signed-off-by: He Kuang <hekuang@huawei.com>
---
 tools/perf/builtin-probe.c     |  8 +++++++-
 tools/perf/util/probe-event.c  | 12 ++++++------
 tools/perf/util/probe-event.h  |  3 ++-
 tools/perf/util/probe-finder.c |  6 +++---
 tools/perf/util/probe-finder.h |  9 +++++++--
 5 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 9c4cf5e..19ebe93 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -472,18 +472,24 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 			pr_err_with_code("  Error: Failed to show lines.", ret);
 		return ret;
 	case 'V':
+	{
+		unsigned long var_flags;
+
 		if (!params.filter)
 			params.filter = strfilter__new(DEFAULT_VAR_FILTER,
 						       NULL);
 
+		var_flags = params.show_ext_vars << VAR_FLAGS_EXTERN;
+
 		ret = show_available_vars(params.events, params.nevents,
 					  params.max_probe_points,
 					  params.target,
 					  params.filter,
-					  params.show_ext_vars);
+					  var_flags);
 		if (ret < 0)
 			pr_err_with_code("  Error: Failed to show vars.", ret);
 		return ret;
+	}
 #endif
 	case 'd':
 		ret = del_perf_probe_events(params.filter);
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 37a3a8b..4859656 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -824,7 +824,7 @@ int show_line_range(struct line_range *lr, const char *module, bool user)
 static int show_available_vars_at(struct debuginfo *dinfo,
 				  struct perf_probe_event *pev,
 				  int max_vls, struct strfilter *_filter,
-				  bool externs, const char *target)
+				  unsigned long var_flags, const char *target)
 {
 	char *buf;
 	int ret, i, nvars;
@@ -839,12 +839,12 @@ static int show_available_vars_at(struct debuginfo *dinfo,
 	pr_debug("Searching variables at %s\n", buf);
 
 	ret = debuginfo__find_available_vars_at(dinfo, pev, &vls,
-						max_vls, externs);
+						max_vls, var_flags);
 	if (!ret) {  /* Not found, retry with an alternative */
 		ret = get_alternative_probe_event(dinfo, pev, &tmp, target);
 		if (!ret) {
 			ret = debuginfo__find_available_vars_at(dinfo, pev,
-						&vls, max_vls, externs);
+						&vls, max_vls, var_flags);
 			/* Release the old probe_point */
 			clear_perf_probe_point(&tmp);
 		}
@@ -892,7 +892,7 @@ end:
 /* Show available variables on given probe point */
 int show_available_vars(struct perf_probe_event *pevs, int npevs,
 			int max_vls, const char *module,
-			struct strfilter *_filter, bool externs)
+			struct strfilter *_filter, unsigned long var_flags)
 {
 	int i, ret = 0;
 	struct debuginfo *dinfo;
@@ -911,7 +911,7 @@ int show_available_vars(struct perf_probe_event *pevs, int npevs,
 
 	for (i = 0; i < npevs && ret >= 0; i++)
 		ret = show_available_vars_at(dinfo, &pevs[i], max_vls, _filter,
-					     externs, module);
+					     var_flags, module);
 
 	debuginfo__delete(dinfo);
 out:
@@ -954,7 +954,7 @@ int show_available_vars(struct perf_probe_event *pevs __maybe_unused,
 			int npevs __maybe_unused, int max_vls __maybe_unused,
 			const char *module __maybe_unused,
 			struct strfilter *filter __maybe_unused,
-			bool externs __maybe_unused)
+			unsigned long var_flags __maybe_unused)
 {
 	pr_warning("Debuginfo-analysis is not supported.\n");
 	return -ENOSYS;
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index e10aedc..637b59e 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -132,7 +132,8 @@ extern int show_line_range(struct line_range *lr, const char *module,
 			   bool user);
 extern int show_available_vars(struct perf_probe_event *pevs, int npevs,
 			       int max_probe_points, const char *module,
-			       struct strfilter *filter, bool externs);
+			       struct strfilter *filter,
+			       unsigned long var_flags);
 extern int show_available_funcs(const char *module, struct strfilter *filter,
 				bool user);
 bool arch__prefers_symtab(void);
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 7b645ff40..6bbdf3f 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1305,7 +1305,7 @@ static int add_available_vars(Dwarf_Die *sc_die, struct probe_finder *pf)
 	die_find_child(sc_die, collect_variables_cb, (void *)af, &die_mem);
 
 	/* Find external variables */
-	if (!af->externs)
+	if (!test_bit(VAR_FLAGS_EXTERN, &af->var_flags))
 		goto out;
 	/* Don't need to search child DIE for externs. */
 	af->child = false;
@@ -1328,12 +1328,12 @@ out:
 int debuginfo__find_available_vars_at(struct debuginfo *dbg,
 				      struct perf_probe_event *pev,
 				      struct variable_list **vls,
-				      int max_vls, bool externs)
+				      int max_vls, unsigned long var_flags)
 {
 	struct available_var_finder af = {
 			.pf = {.pev = pev, .callback = add_available_vars},
 			.mod = dbg->mod,
-			.max_vls = max_vls, .externs = externs};
+			.max_vls = max_vls, .var_flags = var_flags};
 	int ret;
 
 	/* Allocate result vls array */
diff --git a/tools/perf/util/probe-finder.h b/tools/perf/util/probe-finder.h
index f53553d..95bb79d 100644
--- a/tools/perf/util/probe-finder.h
+++ b/tools/perf/util/probe-finder.h
@@ -10,6 +10,10 @@
 #define MAX_PROBES		 128
 #define MAX_PROBE_ARGS		 128
 
+enum var_flags_bits {
+	VAR_FLAGS_EXTERN
+};
+
 #define PROBE_ARG_VARS		"$vars"
 #define PROBE_ARG_PARAMS	"$params"
 
@@ -56,7 +60,8 @@ extern int debuginfo__find_line_range(struct debuginfo *dbg,
 extern int debuginfo__find_available_vars_at(struct debuginfo *dbg,
 					     struct perf_probe_event *pev,
 					     struct variable_list **vls,
-					     int max_points, bool externs);
+					     int max_points,
+					     unsigned long var_flags);
 
 /* Find a src file from a DWARF tag path */
 int get_real_path(const char *raw_path, const char *comp_dir,
@@ -99,7 +104,7 @@ struct available_var_finder {
 	struct variable_list	*vls;		/* Found variable lists */
 	int			nvls;		/* Number of variable lists */
 	int			max_vls;	/* Max no. of variable lists */
-	bool			externs;	/* Find external vars too */
+	unsigned long		var_flags;	/* Variable flags */
 	bool			child;		/* Search child scopes */
 };
 
-- 
1.8.5.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/3] perf probe: Add --range option to show variable location range
  2015-05-08 12:23 [PATCH 1/3] perf probe: Remove length limitation for showing available variables He Kuang
  2015-05-08 12:23 ` [PATCH 2/3] perf probe: Add flag for options when showing variables He Kuang
@ 2015-05-08 12:23 ` He Kuang
  2015-05-08 14:08   ` Masami Hiramatsu
  2015-05-08 14:17 ` [PATCH 1/3] perf probe: Remove length limitation for showing available variables Masami Hiramatsu
  2 siblings, 1 reply; 10+ messages in thread
From: He Kuang @ 2015-05-08 12:23 UTC (permalink / raw)
  To: masami.hiramatsu.pt, a.p.zijlstra, acme, jolsa, mingo
  Cc: wangnan0, linux-kernel

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 [byte offset]: <324-331>
                [INV]   struct address_space_operations*        a_ops   [byte offset]: <55-61>,<170-176>,<223-246>
                [VAL]   (unknown_type)  fsdata  [byte offset]: <70-307>,<346-411>
                [VAL]   loff_t  pos     [byte offset]: <0-286>,<286-336>,<346-411>
                [VAL]   long int        status  [byte offset]: <83-342>,<346-411>
                [VAL]   long unsigned int       bytes   [byte offset]: <122-311>,<320-338>,<346-403>,<403-411>
                [VAL]   struct address_space*   mapping [byte offset]: <35-344>,<346-411>
                [VAL]   struct iov_iter*        i       [byte offset]: <0-340>,<346-411>
                [VAL]   struct page*    page    [byte offset]: <70-307>,<346-411>

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     |   6 ++-
 tools/perf/util/dwarf-aux.c    | 103 +++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/dwarf-aux.h    |   2 +
 tools/perf/util/probe-finder.c |  69 ++++++++++++++++++++-------
 tools/perf/util/probe-finder.h |   3 +-
 5 files changed, 165 insertions(+), 18 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 19ebe93..1b20004 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -52,6 +52,7 @@ static struct {
 	bool list_events;
 	bool force_add;
 	bool show_ext_vars;
+	bool show_location_range;
 	bool uprobes;
 	bool quiet;
 	bool target_used;
@@ -375,6 +376,8 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 		     "Show accessible variables on PROBEDEF", opt_show_vars),
 	OPT_BOOLEAN('\0', "externs", &params.show_ext_vars,
 		    "Show external variables too (with --vars only)"),
+	OPT_BOOLEAN('\0', "range", &params.show_location_range,
+		"Show variables location range in scope (with --vars only)"),
 	OPT_STRING('k', "vmlinux", &symbol_conf.vmlinux_name,
 		   "file", "vmlinux pathname"),
 	OPT_STRING('s', "source", &symbol_conf.source_prefix,
@@ -479,7 +482,8 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 			params.filter = strfilter__new(DEFAULT_VAR_FILTER,
 						       NULL);
 
-		var_flags = params.show_ext_vars << VAR_FLAGS_EXTERN;
+		var_flags = params.show_ext_vars << VAR_FLAGS_EXTERN |
+			params.show_location_range << VAR_FLAGS_LOCATION_RANGE;
 
 		ret = show_available_vars(params.events, params.nevents,
 					  params.max_probe_points,
diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index 75d264e..23053c0 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -897,3 +897,106 @@ int die_get_varname(Dwarf_Die *vr_die, struct strbuf *buf)
 	return 0;
 }
 
+/**
+ * die_get_var_innermost_scope - Get innermost scope range of given variable DIE
+ * @sp_die: a subprogram DIE
+ * @vr_die: a variable DIE
+ * @buf: a strbuf for variable byte offset range
+ *
+ * Get the innermost scope range of @vr_die and stores it in @buf as
+ * "[byte offset]: <NN-NN>,<NN-NN>".
+ */
+static int die_get_var_innermost_scope(Dwarf_Die *sp_die, Dwarf_Die *vr_die,
+				struct strbuf *buf)
+{
+	Dwarf_Die *scopes;
+	int count;
+	size_t offset = 0;
+	Dwarf_Addr base;
+	Dwarf_Addr start, end;
+	Dwarf_Addr entry;
+	int ret;
+	bool first = true;
+
+	ret = dwarf_entrypc(sp_die, &entry);
+	if (ret)
+		return ret;
+
+	count = dwarf_getscopes_die(vr_die, &scopes);
+
+	/* (*SCOPES)[1] is the DIE for the scope containing that scope */
+	if (count <= 1) {
+		ret = -EINVAL;
+		goto end;
+	}
+
+	while ((offset = dwarf_ranges(&scopes[1], offset, &base,
+				&start, &end)) > 0) {
+		start -= entry;
+		end -= entry;
+
+		if (first) {
+			strbuf_addf(buf, "[byte offset]: <%ld-%ld>",
+				start, end);
+			first = false;
+		} else {
+			strbuf_addf(buf, ",<%ld-%ld>",
+				start, end);
+		}
+	}
+end:
+	free(scopes);
+	return ret;
+}
+
+/**
+ * die_get_var_range - Get byte offset range of given variable DIE
+ * @sp_die: a subprogram DIE
+ * @vr_die: a variable DIE
+ * @buf: a strbuf for type and variable name and byte offset range
+ *
+ * Get the byte offset range of @vr_die and stores it in @buf as
+ * "[byte offset]: <NN-NN>,<NN-NN>".
+ */
+int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, struct strbuf *buf)
+{
+	int ret = 0;
+	Dwarf_Addr base;
+	Dwarf_Addr start, end;
+	Dwarf_Addr entry;
+	Dwarf_Op *op;
+	size_t nops;
+	size_t offset = 0;
+	Dwarf_Attribute attr;
+	bool first = true;
+
+	ret = dwarf_entrypc(sp_die, &entry);
+	if (ret)
+		return ret;
+
+	if (dwarf_attr(vr_die, DW_AT_location, &attr) == NULL)
+		return -EINVAL;
+
+	while ((offset = dwarf_getlocations(
+				&attr, offset, &base,
+				&start, &end, &op, &nops)) > 0) {
+		if (start == 0) {
+			/* Single Location Descriptions */
+			ret = die_get_var_innermost_scope(sp_die, vr_die, buf);
+		} else {
+			/* Location Lists */
+			start -= entry;
+			end -= entry;
+			if (first) {
+				strbuf_addf(buf, "[byte offset]: <%ld-%ld>",
+					start, end);
+				first = false;
+			} else {
+				strbuf_addf(buf, ",<%ld-%ld>",
+					start, end);
+			}
+		}
+	}
+
+	return ret;
+}
diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
index c0d6173..8390cde 100644
--- a/tools/perf/util/dwarf-aux.h
+++ b/tools/perf/util/dwarf-aux.h
@@ -118,4 +118,6 @@ extern int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf);
 
 /* Get the name and type of given variable DIE, stored as "type\tname" */
 extern int die_get_varname(Dwarf_Die *vr_die, struct strbuf *buf);
+extern int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die,
+			struct strbuf *buf);
 #endif
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 6bbdf3f..af0a7ea 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -43,6 +43,9 @@
 /* Kprobe tracer basic type is up to u64 */
 #define MAX_BASIC_TYPE_BITS	64
 
+/* Variable location invalid at addr but valid in scope */
+#define VARIABLE_LOCATION_INVALID_AT_ADDR	-10000
+
 /* Dwarf FL wrappers */
 static char *debuginfo_path;	/* Currently dummy */
 
@@ -167,7 +170,8 @@ static struct probe_trace_arg_ref *alloc_trace_arg_ref(long offs)
  */
 static int convert_variable_location(Dwarf_Die *vr_die, Dwarf_Addr addr,
 				     Dwarf_Op *fb_ops, Dwarf_Die *sp_die,
-				     struct probe_trace_arg *tvar)
+				     struct probe_trace_arg *tvar,
+				     unsigned long *var_flags)
 {
 	Dwarf_Attribute attr;
 	Dwarf_Addr tmp = 0;
@@ -177,7 +181,7 @@ static int convert_variable_location(Dwarf_Die *vr_die, Dwarf_Addr addr,
 	Dwarf_Word offs = 0;
 	bool ref = false;
 	const char *regs;
-	int ret;
+	int ret, ret2 = 0;
 
 	if (dwarf_attr(vr_die, DW_AT_external, &attr) != NULL)
 		goto static_var;
@@ -187,9 +191,20 @@ static int convert_variable_location(Dwarf_Die *vr_die, Dwarf_Addr addr,
 		return -EINVAL;	/* Broken DIE ? */
 	if (dwarf_getlocation_addr(&attr, addr, &op, &nops, 1) <= 0) {
 		ret = dwarf_entrypc(sp_die, &tmp);
-		if (ret || addr != tmp ||
-		    dwarf_tag(vr_die) != DW_TAG_formal_parameter ||
-		    dwarf_highpc(sp_die, &tmp))
+		if (ret)
+			return -ENOENT;
+
+		if (var_flags &&
+			test_bit(VAR_FLAGS_LOCATION_RANGE, var_flags) &&
+			(dwarf_tag(vr_die) == DW_TAG_variable)) {
+			ret2 = VARIABLE_LOCATION_INVALID_AT_ADDR;
+		} else if (addr != tmp ||
+			dwarf_tag(vr_die) != DW_TAG_formal_parameter) {
+			return -ENOENT;
+		}
+
+		ret = dwarf_highpc(sp_die, &tmp);
+		if (ret)
 			return -ENOENT;
 		/*
 		 * This is fuzzed by fentry mcount. We try to find the
@@ -210,7 +225,7 @@ found:
 	if (op->atom == DW_OP_addr) {
 static_var:
 		if (!tvar)
-			return 0;
+			return ret2;
 		/* Static variables on memory (not stack), make @varname */
 		ret = strlen(dwarf_diename(vr_die));
 		tvar->value = zalloc(ret + 2);
@@ -220,7 +235,7 @@ static_var:
 		tvar->ref = alloc_trace_arg_ref((long)offs);
 		if (tvar->ref == NULL)
 			return -ENOMEM;
-		return 0;
+		return ret2;
 	}
 
 	/* If this is based on frame buffer, set the offset */
@@ -250,7 +265,7 @@ static_var:
 	}
 
 	if (!tvar)
-		return 0;
+		return ret2;
 
 	regs = get_arch_regstr(regn);
 	if (!regs) {
@@ -269,7 +284,7 @@ static_var:
 		if (tvar->ref == NULL)
 			return -ENOMEM;
 	}
-	return 0;
+	return ret2;
 }
 
 #define BYTES_TO_BITS(nb)	((nb) * BITS_PER_LONG / sizeof(long))
@@ -516,7 +531,7 @@ static int convert_variable(Dwarf_Die *vr_die, struct probe_finder *pf)
 		 dwarf_diename(vr_die));
 
 	ret = convert_variable_location(vr_die, pf->addr, pf->fb_ops,
-					&pf->sp_die, pf->tvar);
+					&pf->sp_die, pf->tvar, NULL);
 	if (ret == -ENOENT || ret == -EINVAL)
 		pr_err("Failed to find the location of %s at this address.\n"
 		       " Perhaps, it has been optimized out.\n", pf->pvar->var);
@@ -1105,7 +1120,7 @@ static int copy_variables_cb(Dwarf_Die *die_mem, void *data)
 	    (tag == DW_TAG_variable && vf->vars)) {
 		if (convert_variable_location(die_mem, vf->pf->addr,
 					      vf->pf->fb_ops, &pf->sp_die,
-					      NULL) == 0) {
+					      NULL, NULL) == 0) {
 			vf->args[vf->nargs].var = (char *)dwarf_diename(die_mem);
 			if (vf->args[vf->nargs].var == NULL) {
 				vf->ret = -ENOMEM;
@@ -1254,11 +1269,33 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data)
 	    tag == DW_TAG_variable) {
 		ret = convert_variable_location(die_mem, af->pf.addr,
 						af->pf.fb_ops, &af->pf.sp_die,
-						NULL);
-		if (ret == 0) {
-			ret = die_get_varname(die_mem, &buf);
-			pr_debug2("Add new var: %s\n", buf.buf);
-			if (ret == 0) {
+						NULL, &af->var_flags);
+		if (ret == 0 || ret == VARIABLE_LOCATION_INVALID_AT_ADDR) {
+			int ret2;
+			bool show_range = test_bit(VAR_FLAGS_LOCATION_RANGE,
+						&af->var_flags);
+			bool externs = !af->child;
+
+			if (show_range) {
+				if (!externs) {
+					if (ret)
+						strbuf_addf(&buf, "[INV]\t");
+					else
+						strbuf_addf(&buf, "[VAL]\t");
+				} else
+					strbuf_addf(&buf, "[EXT]\t");
+			}
+
+			ret2 = die_get_varname(die_mem, &buf);
+
+			if (!ret2 && show_range && !externs) {
+				strbuf_addf(&buf, "\t");
+				ret2 = die_get_var_range(&af->pf.sp_die,
+							die_mem, &buf);
+			}
+
+			pr_debug("Add new var: %s\n", buf.buf);
+			if (ret2 == 0) {
 				strlist__add(vl->vars,
 					strbuf_detach(&buf, NULL));
 			}
diff --git a/tools/perf/util/probe-finder.h b/tools/perf/util/probe-finder.h
index 95bb79d..4b30a9b 100644
--- a/tools/perf/util/probe-finder.h
+++ b/tools/perf/util/probe-finder.h
@@ -11,7 +11,8 @@
 #define MAX_PROBE_ARGS		 128
 
 enum var_flags_bits {
-	VAR_FLAGS_EXTERN
+	VAR_FLAGS_EXTERN,
+	VAR_FLAGS_LOCATION_RANGE
 };
 
 #define PROBE_ARG_VARS		"$vars"
-- 
1.8.5.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] perf probe: Add flag for options when showing variables
  2015-05-08 12:23 ` [PATCH 2/3] perf probe: Add flag for options when showing variables He Kuang
@ 2015-05-08 13:15   ` Masami Hiramatsu
  2015-05-09  6:30     ` He Kuang
  0 siblings, 1 reply; 10+ messages in thread
From: Masami Hiramatsu @ 2015-05-08 13:15 UTC (permalink / raw)
  To: He Kuang, a.p.zijlstra, acme, jolsa, mingo; +Cc: wangnan0, linux-kernel

On 2015/05/08 21:23, He Kuang wrote:
> Change current boolean flag 'externs' to unsigned long 'var_flags' for
> more options related to variables. These options work with --vars only.

NAK, please check my latest patch.

https://lkml.org/lkml/2015/5/7/1024

If you need to add some configurations, please considering to add something
on probe_conf to pass.

Thanks,

> 
> Signed-off-by: He Kuang <hekuang@huawei.com>
> ---
>  tools/perf/builtin-probe.c     |  8 +++++++-
>  tools/perf/util/probe-event.c  | 12 ++++++------
>  tools/perf/util/probe-event.h  |  3 ++-
>  tools/perf/util/probe-finder.c |  6 +++---
>  tools/perf/util/probe-finder.h |  9 +++++++--
>  5 files changed, 25 insertions(+), 13 deletions(-)
> 
> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> index 9c4cf5e..19ebe93 100644
> --- a/tools/perf/builtin-probe.c
> +++ b/tools/perf/builtin-probe.c
> @@ -472,18 +472,24 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
>  			pr_err_with_code("  Error: Failed to show lines.", ret);
>  		return ret;
>  	case 'V':
> +	{
> +		unsigned long var_flags;
> +
>  		if (!params.filter)
>  			params.filter = strfilter__new(DEFAULT_VAR_FILTER,
>  						       NULL);
>  
> +		var_flags = params.show_ext_vars << VAR_FLAGS_EXTERN;
> +
>  		ret = show_available_vars(params.events, params.nevents,
>  					  params.max_probe_points,
>  					  params.target,
>  					  params.filter,
> -					  params.show_ext_vars);
> +					  var_flags);
>  		if (ret < 0)
>  			pr_err_with_code("  Error: Failed to show vars.", ret);
>  		return ret;
> +	}
>  #endif
>  	case 'd':
>  		ret = del_perf_probe_events(params.filter);
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 37a3a8b..4859656 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -824,7 +824,7 @@ int show_line_range(struct line_range *lr, const char *module, bool user)
>  static int show_available_vars_at(struct debuginfo *dinfo,
>  				  struct perf_probe_event *pev,
>  				  int max_vls, struct strfilter *_filter,
> -				  bool externs, const char *target)
> +				  unsigned long var_flags, const char *target)
>  {
>  	char *buf;
>  	int ret, i, nvars;
> @@ -839,12 +839,12 @@ static int show_available_vars_at(struct debuginfo *dinfo,
>  	pr_debug("Searching variables at %s\n", buf);
>  
>  	ret = debuginfo__find_available_vars_at(dinfo, pev, &vls,
> -						max_vls, externs);
> +						max_vls, var_flags);
>  	if (!ret) {  /* Not found, retry with an alternative */
>  		ret = get_alternative_probe_event(dinfo, pev, &tmp, target);
>  		if (!ret) {
>  			ret = debuginfo__find_available_vars_at(dinfo, pev,
> -						&vls, max_vls, externs);
> +						&vls, max_vls, var_flags);
>  			/* Release the old probe_point */
>  			clear_perf_probe_point(&tmp);
>  		}
> @@ -892,7 +892,7 @@ end:
>  /* Show available variables on given probe point */
>  int show_available_vars(struct perf_probe_event *pevs, int npevs,
>  			int max_vls, const char *module,
> -			struct strfilter *_filter, bool externs)
> +			struct strfilter *_filter, unsigned long var_flags)
>  {
>  	int i, ret = 0;
>  	struct debuginfo *dinfo;
> @@ -911,7 +911,7 @@ int show_available_vars(struct perf_probe_event *pevs, int npevs,
>  
>  	for (i = 0; i < npevs && ret >= 0; i++)
>  		ret = show_available_vars_at(dinfo, &pevs[i], max_vls, _filter,
> -					     externs, module);
> +					     var_flags, module);
>  
>  	debuginfo__delete(dinfo);
>  out:
> @@ -954,7 +954,7 @@ int show_available_vars(struct perf_probe_event *pevs __maybe_unused,
>  			int npevs __maybe_unused, int max_vls __maybe_unused,
>  			const char *module __maybe_unused,
>  			struct strfilter *filter __maybe_unused,
> -			bool externs __maybe_unused)
> +			unsigned long var_flags __maybe_unused)
>  {
>  	pr_warning("Debuginfo-analysis is not supported.\n");
>  	return -ENOSYS;
> diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> index e10aedc..637b59e 100644
> --- a/tools/perf/util/probe-event.h
> +++ b/tools/perf/util/probe-event.h
> @@ -132,7 +132,8 @@ extern int show_line_range(struct line_range *lr, const char *module,
>  			   bool user);
>  extern int show_available_vars(struct perf_probe_event *pevs, int npevs,
>  			       int max_probe_points, const char *module,
> -			       struct strfilter *filter, bool externs);
> +			       struct strfilter *filter,
> +			       unsigned long var_flags);
>  extern int show_available_funcs(const char *module, struct strfilter *filter,
>  				bool user);
>  bool arch__prefers_symtab(void);
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index 7b645ff40..6bbdf3f 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -1305,7 +1305,7 @@ static int add_available_vars(Dwarf_Die *sc_die, struct probe_finder *pf)
>  	die_find_child(sc_die, collect_variables_cb, (void *)af, &die_mem);
>  
>  	/* Find external variables */
> -	if (!af->externs)
> +	if (!test_bit(VAR_FLAGS_EXTERN, &af->var_flags))
>  		goto out;
>  	/* Don't need to search child DIE for externs. */
>  	af->child = false;
> @@ -1328,12 +1328,12 @@ out:
>  int debuginfo__find_available_vars_at(struct debuginfo *dbg,
>  				      struct perf_probe_event *pev,
>  				      struct variable_list **vls,
> -				      int max_vls, bool externs)
> +				      int max_vls, unsigned long var_flags)
>  {
>  	struct available_var_finder af = {
>  			.pf = {.pev = pev, .callback = add_available_vars},
>  			.mod = dbg->mod,
> -			.max_vls = max_vls, .externs = externs};
> +			.max_vls = max_vls, .var_flags = var_flags};
>  	int ret;
>  
>  	/* Allocate result vls array */
> diff --git a/tools/perf/util/probe-finder.h b/tools/perf/util/probe-finder.h
> index f53553d..95bb79d 100644
> --- a/tools/perf/util/probe-finder.h
> +++ b/tools/perf/util/probe-finder.h
> @@ -10,6 +10,10 @@
>  #define MAX_PROBES		 128
>  #define MAX_PROBE_ARGS		 128
>  
> +enum var_flags_bits {
> +	VAR_FLAGS_EXTERN
> +};
> +
>  #define PROBE_ARG_VARS		"$vars"
>  #define PROBE_ARG_PARAMS	"$params"
>  
> @@ -56,7 +60,8 @@ extern int debuginfo__find_line_range(struct debuginfo *dbg,
>  extern int debuginfo__find_available_vars_at(struct debuginfo *dbg,
>  					     struct perf_probe_event *pev,
>  					     struct variable_list **vls,
> -					     int max_points, bool externs);
> +					     int max_points,
> +					     unsigned long var_flags);
>  
>  /* Find a src file from a DWARF tag path */
>  int get_real_path(const char *raw_path, const char *comp_dir,
> @@ -99,7 +104,7 @@ struct available_var_finder {
>  	struct variable_list	*vls;		/* Found variable lists */
>  	int			nvls;		/* Number of variable lists */
>  	int			max_vls;	/* Max no. of variable lists */
> -	bool			externs;	/* Find external vars too */
> +	unsigned long		var_flags;	/* Variable flags */
>  	bool			child;		/* Search child scopes */
>  };
>  
> 


-- 
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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3] perf probe: Add --range option to show variable location range
  2015-05-08 12:23 ` [PATCH 3/3] perf probe: Add --range option to show variable location range He Kuang
@ 2015-05-08 14:08   ` Masami Hiramatsu
  2015-05-09  7:41     ` He Kuang
  0 siblings, 1 reply; 10+ messages in thread
From: Masami Hiramatsu @ 2015-05-08 14:08 UTC (permalink / raw)
  To: He Kuang, a.p.zijlstra, acme, jolsa, mingo; +Cc: wangnan0, linux-kernel

On 2015/05/08 21:23, He Kuang wrote:
> It is not easy for users to get the accurate byte offset or the line
> number where a local variable can be probed. With '--range' option,
> local variables in scope of the probe point are showed with byte offset
> range, and can be added according to this range information.

Interesting idea :)

> 
> For example, there are some variables in function
> generic_perform_write():
> 
>   <generic_perform_write@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.

Yeah, right. That's why I've introduced --vars option.

> 
> This is because source code do not match assembly, so a variable may not
> be available in the sourcecode line where it presents. After this patch,
> we can lookup the accurate byte offset range of a variable, 'INV'
> indicates that this variable is not valid at the given point, but
> available in scope:
> 
>   $ perf probe --vars 'generic_perform_write:42' --range
>   Available variables at generic_perform_write:42
>         @<generic_perform_write+141>
>                 [INV]   ssize_t written [byte offset]: <324-331>
>                 [INV]   struct address_space_operations*        a_ops   [byte offset]: <55-61>,<170-176>,<223-246>
>                 [VAL]   (unknown_type)  fsdata  [byte offset]: <70-307>,<346-411>
>                 [VAL]   loff_t  pos     [byte offset]: <0-286>,<286-336>,<346-411>
>                 [VAL]   long int        status  [byte offset]: <83-342>,<346-411>
>                 [VAL]   long unsigned int       bytes   [byte offset]: <122-311>,<320-338>,<346-403>,<403-411>
>                 [VAL]   struct address_space*   mapping [byte offset]: <35-344>,<346-411>
>                 [VAL]   struct iov_iter*        i       [byte offset]: <0-340>,<346-411>
>                 [VAL]   struct page*    page    [byte offset]: <70-307>,<346-411>

OK, at first, I don't like frequently repeated "[byte offset]", I prefer to
show the function name+[offset range], like below :)

 [INV]   ssize_t written @<generic_perform_write+[324-331]>
 [INV]   struct address_space_operations*        a_ops   <@generic_perform_write+[55-61,170-176,223-246]>

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

Thank you,

> 
> Then it is more clearly for us to do the probe with this variable:
> 
>   $ perf probe --add 'generic_perform_write+170 a_ops'
>   Added new event:
>     probe:generic_perform_write (on generic_perform_write+170 with a_ops)
> 
> Signed-off-by: He Kuang <hekuang@huawei.com>
> ---
>  tools/perf/builtin-probe.c     |   6 ++-
>  tools/perf/util/dwarf-aux.c    | 103 +++++++++++++++++++++++++++++++++++++++++
>  tools/perf/util/dwarf-aux.h    |   2 +
>  tools/perf/util/probe-finder.c |  69 ++++++++++++++++++++-------
>  tools/perf/util/probe-finder.h |   3 +-
>  5 files changed, 165 insertions(+), 18 deletions(-)
> 
> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> index 19ebe93..1b20004 100644
> --- a/tools/perf/builtin-probe.c
> +++ b/tools/perf/builtin-probe.c
> @@ -52,6 +52,7 @@ static struct {
>  	bool list_events;
>  	bool force_add;
>  	bool show_ext_vars;
> +	bool show_location_range;
>  	bool uprobes;
>  	bool quiet;
>  	bool target_used;
> @@ -375,6 +376,8 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
>  		     "Show accessible variables on PROBEDEF", opt_show_vars),
>  	OPT_BOOLEAN('\0', "externs", &params.show_ext_vars,
>  		    "Show external variables too (with --vars only)"),
> +	OPT_BOOLEAN('\0', "range", &params.show_location_range,
> +		"Show variables location range in scope (with --vars only)"),
>  	OPT_STRING('k', "vmlinux", &symbol_conf.vmlinux_name,
>  		   "file", "vmlinux pathname"),
>  	OPT_STRING('s', "source", &symbol_conf.source_prefix,
> @@ -479,7 +482,8 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
>  			params.filter = strfilter__new(DEFAULT_VAR_FILTER,
>  						       NULL);
>  
> -		var_flags = params.show_ext_vars << VAR_FLAGS_EXTERN;
> +		var_flags = params.show_ext_vars << VAR_FLAGS_EXTERN |
> +			params.show_location_range << VAR_FLAGS_LOCATION_RANGE;
>  
>  		ret = show_available_vars(params.events, params.nevents,
>  					  params.max_probe_points,
> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> index 75d264e..23053c0 100644
> --- a/tools/perf/util/dwarf-aux.c
> +++ b/tools/perf/util/dwarf-aux.c
> @@ -897,3 +897,106 @@ int die_get_varname(Dwarf_Die *vr_die, struct strbuf *buf)
>  	return 0;
>  }
>  
> +/**
> + * die_get_var_innermost_scope - Get innermost scope range of given variable DIE
> + * @sp_die: a subprogram DIE
> + * @vr_die: a variable DIE
> + * @buf: a strbuf for variable byte offset range
> + *
> + * Get the innermost scope range of @vr_die and stores it in @buf as
> + * "[byte offset]: <NN-NN>,<NN-NN>".
> + */
> +static int die_get_var_innermost_scope(Dwarf_Die *sp_die, Dwarf_Die *vr_die,
> +				struct strbuf *buf)
> +{
> +	Dwarf_Die *scopes;
> +	int count;
> +	size_t offset = 0;
> +	Dwarf_Addr base;
> +	Dwarf_Addr start, end;
> +	Dwarf_Addr entry;
> +	int ret;
> +	bool first = true;
> +
> +	ret = dwarf_entrypc(sp_die, &entry);
> +	if (ret)
> +		return ret;
> +
> +	count = dwarf_getscopes_die(vr_die, &scopes);
> +
> +	/* (*SCOPES)[1] is the DIE for the scope containing that scope */
> +	if (count <= 1) {
> +		ret = -EINVAL;
> +		goto end;
> +	}
> +
> +	while ((offset = dwarf_ranges(&scopes[1], offset, &base,
> +				&start, &end)) > 0) {
> +		start -= entry;
> +		end -= entry;
> +
> +		if (first) {
> +			strbuf_addf(buf, "[byte offset]: <%ld-%ld>",
> +				start, end);
> +			first = false;
> +		} else {
> +			strbuf_addf(buf, ",<%ld-%ld>",
> +				start, end);
> +		}
> +	}
> +end:
> +	free(scopes);
> +	return ret;
> +}
> +
> +/**
> + * die_get_var_range - Get byte offset range of given variable DIE
> + * @sp_die: a subprogram DIE
> + * @vr_die: a variable DIE
> + * @buf: a strbuf for type and variable name and byte offset range
> + *
> + * Get the byte offset range of @vr_die and stores it in @buf as
> + * "[byte offset]: <NN-NN>,<NN-NN>".
> + */
> +int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, struct strbuf *buf)
> +{
> +	int ret = 0;
> +	Dwarf_Addr base;
> +	Dwarf_Addr start, end;
> +	Dwarf_Addr entry;
> +	Dwarf_Op *op;
> +	size_t nops;
> +	size_t offset = 0;
> +	Dwarf_Attribute attr;
> +	bool first = true;
> +
> +	ret = dwarf_entrypc(sp_die, &entry);
> +	if (ret)
> +		return ret;
> +
> +	if (dwarf_attr(vr_die, DW_AT_location, &attr) == NULL)
> +		return -EINVAL;
> +
> +	while ((offset = dwarf_getlocations(
> +				&attr, offset, &base,
> +				&start, &end, &op, &nops)) > 0) {
> +		if (start == 0) {
> +			/* Single Location Descriptions */
> +			ret = die_get_var_innermost_scope(sp_die, vr_die, buf);
> +		} else {
> +			/* Location Lists */
> +			start -= entry;
> +			end -= entry;
> +			if (first) {
> +				strbuf_addf(buf, "[byte offset]: <%ld-%ld>",
> +					start, end);
> +				first = false;
> +			} else {
> +				strbuf_addf(buf, ",<%ld-%ld>",
> +					start, end);
> +			}
> +		}
> +	}
> +
> +	return ret;
> +}
> diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
> index c0d6173..8390cde 100644
> --- a/tools/perf/util/dwarf-aux.h
> +++ b/tools/perf/util/dwarf-aux.h
> @@ -118,4 +118,6 @@ extern int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf);
>  
>  /* Get the name and type of given variable DIE, stored as "type\tname" */
>  extern int die_get_varname(Dwarf_Die *vr_die, struct strbuf *buf);
> +extern int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die,
> +			struct strbuf *buf);
>  #endif
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index 6bbdf3f..af0a7ea 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -43,6 +43,9 @@
>  /* Kprobe tracer basic type is up to u64 */
>  #define MAX_BASIC_TYPE_BITS	64
>  
> +/* Variable location invalid at addr but valid in scope */
> +#define VARIABLE_LOCATION_INVALID_AT_ADDR	-10000
> +
>  /* Dwarf FL wrappers */
>  static char *debuginfo_path;	/* Currently dummy */
>  
> @@ -167,7 +170,8 @@ static struct probe_trace_arg_ref *alloc_trace_arg_ref(long offs)
>   */
>  static int convert_variable_location(Dwarf_Die *vr_die, Dwarf_Addr addr,
>  				     Dwarf_Op *fb_ops, Dwarf_Die *sp_die,
> -				     struct probe_trace_arg *tvar)
> +				     struct probe_trace_arg *tvar,
> +				     unsigned long *var_flags)
>  {
>  	Dwarf_Attribute attr;
>  	Dwarf_Addr tmp = 0;
> @@ -177,7 +181,7 @@ static int convert_variable_location(Dwarf_Die *vr_die, Dwarf_Addr addr,
>  	Dwarf_Word offs = 0;
>  	bool ref = false;
>  	const char *regs;
> -	int ret;
> +	int ret, ret2 = 0;
>  
>  	if (dwarf_attr(vr_die, DW_AT_external, &attr) != NULL)
>  		goto static_var;
> @@ -187,9 +191,20 @@ static int convert_variable_location(Dwarf_Die *vr_die, Dwarf_Addr addr,
>  		return -EINVAL;	/* Broken DIE ? */
>  	if (dwarf_getlocation_addr(&attr, addr, &op, &nops, 1) <= 0) {
>  		ret = dwarf_entrypc(sp_die, &tmp);
> -		if (ret || addr != tmp ||
> -		    dwarf_tag(vr_die) != DW_TAG_formal_parameter ||
> -		    dwarf_highpc(sp_die, &tmp))
> +		if (ret)
> +			return -ENOENT;
> +
> +		if (var_flags &&
> +			test_bit(VAR_FLAGS_LOCATION_RANGE, var_flags) &&
> +			(dwarf_tag(vr_die) == DW_TAG_variable)) {
> +			ret2 = VARIABLE_LOCATION_INVALID_AT_ADDR;
> +		} else if (addr != tmp ||
> +			dwarf_tag(vr_die) != DW_TAG_formal_parameter) {
> +			return -ENOENT;
> +		}
> +
> +		ret = dwarf_highpc(sp_die, &tmp);
> +		if (ret)
>  			return -ENOENT;
>  		/*
>  		 * This is fuzzed by fentry mcount. We try to find the
> @@ -210,7 +225,7 @@ found:
>  	if (op->atom == DW_OP_addr) {
>  static_var:
>  		if (!tvar)
> -			return 0;
> +			return ret2;
>  		/* Static variables on memory (not stack), make @varname */
>  		ret = strlen(dwarf_diename(vr_die));
>  		tvar->value = zalloc(ret + 2);
> @@ -220,7 +235,7 @@ static_var:
>  		tvar->ref = alloc_trace_arg_ref((long)offs);
>  		if (tvar->ref == NULL)
>  			return -ENOMEM;
> -		return 0;
> +		return ret2;
>  	}
>  
>  	/* If this is based on frame buffer, set the offset */
> @@ -250,7 +265,7 @@ static_var:
>  	}
>  
>  	if (!tvar)
> -		return 0;
> +		return ret2;
>  
>  	regs = get_arch_regstr(regn);
>  	if (!regs) {
> @@ -269,7 +284,7 @@ static_var:
>  		if (tvar->ref == NULL)
>  			return -ENOMEM;
>  	}
> -	return 0;
> +	return ret2;
>  }
>  
>  #define BYTES_TO_BITS(nb)	((nb) * BITS_PER_LONG / sizeof(long))
> @@ -516,7 +531,7 @@ static int convert_variable(Dwarf_Die *vr_die, struct probe_finder *pf)
>  		 dwarf_diename(vr_die));
>  
>  	ret = convert_variable_location(vr_die, pf->addr, pf->fb_ops,
> -					&pf->sp_die, pf->tvar);
> +					&pf->sp_die, pf->tvar, NULL);
>  	if (ret == -ENOENT || ret == -EINVAL)
>  		pr_err("Failed to find the location of %s at this address.\n"
>  		       " Perhaps, it has been optimized out.\n", pf->pvar->var);
> @@ -1105,7 +1120,7 @@ static int copy_variables_cb(Dwarf_Die *die_mem, void *data)
>  	    (tag == DW_TAG_variable && vf->vars)) {
>  		if (convert_variable_location(die_mem, vf->pf->addr,
>  					      vf->pf->fb_ops, &pf->sp_die,
> -					      NULL) == 0) {
> +					      NULL, NULL) == 0) {
>  			vf->args[vf->nargs].var = (char *)dwarf_diename(die_mem);
>  			if (vf->args[vf->nargs].var == NULL) {
>  				vf->ret = -ENOMEM;
> @@ -1254,11 +1269,33 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data)
>  	    tag == DW_TAG_variable) {
>  		ret = convert_variable_location(die_mem, af->pf.addr,
>  						af->pf.fb_ops, &af->pf.sp_die,
> -						NULL);
> -		if (ret == 0) {
> -			ret = die_get_varname(die_mem, &buf);
> -			pr_debug2("Add new var: %s\n", buf.buf);
> -			if (ret == 0) {
> +						NULL, &af->var_flags);
> +		if (ret == 0 || ret == VARIABLE_LOCATION_INVALID_AT_ADDR) {
> +			int ret2;
> +			bool show_range = test_bit(VAR_FLAGS_LOCATION_RANGE,
> +						&af->var_flags);
> +			bool externs = !af->child;
> +
> +			if (show_range) {
> +				if (!externs) {
> +					if (ret)
> +						strbuf_addf(&buf, "[INV]\t");
> +					else
> +						strbuf_addf(&buf, "[VAL]\t");
> +				} else
> +					strbuf_addf(&buf, "[EXT]\t");
> +			}
> +
> +			ret2 = die_get_varname(die_mem, &buf);
> +
> +			if (!ret2 && show_range && !externs) {
> +				strbuf_addf(&buf, "\t");
> +				ret2 = die_get_var_range(&af->pf.sp_die,
> +							die_mem, &buf);
> +			}
> +
> +			pr_debug("Add new var: %s\n", buf.buf);
> +			if (ret2 == 0) {
>  				strlist__add(vl->vars,
>  					strbuf_detach(&buf, NULL));
>  			}
> diff --git a/tools/perf/util/probe-finder.h b/tools/perf/util/probe-finder.h
> index 95bb79d..4b30a9b 100644
> --- a/tools/perf/util/probe-finder.h
> +++ b/tools/perf/util/probe-finder.h
> @@ -11,7 +11,8 @@
>  #define MAX_PROBE_ARGS		 128
>  
>  enum var_flags_bits {
> -	VAR_FLAGS_EXTERN
> +	VAR_FLAGS_EXTERN,
> +	VAR_FLAGS_LOCATION_RANGE
>  };
>  
>  #define PROBE_ARG_VARS		"$vars"
> 


-- 
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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] perf probe: Remove length limitation for showing available variables
  2015-05-08 12:23 [PATCH 1/3] perf probe: Remove length limitation for showing available variables He Kuang
  2015-05-08 12:23 ` [PATCH 2/3] perf probe: Add flag for options when showing variables He Kuang
  2015-05-08 12:23 ` [PATCH 3/3] perf probe: Add --range option to show variable location range He Kuang
@ 2015-05-08 14:17 ` Masami Hiramatsu
  2015-05-09  7:16   ` He Kuang
  2 siblings, 1 reply; 10+ messages in thread
From: Masami Hiramatsu @ 2015-05-08 14:17 UTC (permalink / raw)
  To: He Kuang, a.p.zijlstra, acme, jolsa, mingo; +Cc: wangnan0, linux-kernel

On 2015/05/08 21:23, 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, except one memory leak. please see below.

> 
> Signed-off-by: He Kuang <hekuang@huawei.com>
> ---
>  tools/perf/util/dwarf-aux.c    | 50 +++++++++++++++++++-----------------------
>  tools/perf/util/dwarf-aux.h    |  4 ++--
>  tools/perf/util/probe-finder.c | 16 ++++++++------
>  3 files changed, 33 insertions(+), 37 deletions(-)
> 
> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> index c34e024..75d264e 100644
> --- a/tools/perf/util/dwarf-aux.c
> +++ b/tools/perf/util/dwarf-aux.c
> @@ -832,19 +832,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)
> @@ -855,8 +853,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;
> @@ -867,39 +865,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 af7dbcd..c0d6173 100644
> --- a/tools/perf/util/dwarf-aux.h
> +++ b/tools/perf/util/dwarf-aux.h
> @@ -114,8 +114,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 63d3389..7b645ff40 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -1238,17 +1238,16 @@ 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];
> +	struct strbuf buf;
>  	int tag, ret;
>  
>  	vl = &af->vls[af->nvls - 1];
> +	strbuf_init(&buf, 64);
>  
>  	tag = dwarf_tag(die_mem);
>  	if (tag == DW_TAG_formal_parameter ||
> @@ -1257,10 +1256,13 @@ 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);
> +			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);

It seems that this strbuf_release() should be called in any case,
since strbuf_init already allocated buffer.

>  		}
>  	}

so here is the good place to call, isn't it?

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] perf probe: Add flag for options when showing variables
  2015-05-08 13:15   ` Masami Hiramatsu
@ 2015-05-09  6:30     ` He Kuang
  0 siblings, 0 replies; 10+ messages in thread
From: He Kuang @ 2015-05-09  6:30 UTC (permalink / raw)
  To: Masami Hiramatsu, a.p.zijlstra, acme, jolsa, mingo; +Cc: wangnan0, linux-kernel

hi, Masami

On 2015/5/8 21:15, Masami Hiramatsu wrote:
> On 2015/05/08 21:23, He Kuang wrote:
>> Change current boolean flag 'externs' to unsigned long 'var_flags' for
>> more options related to variables. These options work with --vars only.
> NAK, please check my latest patch.
>
> https://lkml.org/lkml/2015/5/7/1024
>
> If you need to add some configurations, please considering to add something
> on probe_conf to pass.
>
> Thanks,

Ok, I'll use probe_conf for new options.

>> Signed-off-by: He Kuang <hekuang@huawei.com>
>> ---
>>  tools/perf/builtin-probe.c     |  8 +++++++-
>>  tools/perf/util/probe-event.c  | 12 ++++++------
>>  tools/perf/util/probe-event.h  |  3 ++-
>>  tools/perf/util/probe-finder.c |  6 +++---
>>  tools/perf/util/probe-finder.h |  9 +++++++--
>>  5 files changed, 25 insertions(+), 13 deletions(-)
>>
>> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
>> index 9c4cf5e..19ebe93 100644
>> --- a/tools/perf/builtin-probe.c
>> +++ b/tools/perf/builtin-probe.c
>> @@ -472,18 +472,24 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
>>  			pr_err_with_code("  Error: Failed to show lines.", ret);
>>  		return ret;
>>  	case 'V':
>> +	{
>> +		unsigned long var_flags;
>> +
>>  		if (!params.filter)
>>  			params.filter = strfilter__new(DEFAULT_VAR_FILTER,
>>  						       NULL);
>>  
>> +		var_flags = params.show_ext_vars << VAR_FLAGS_EXTERN;
>> +
>>  		ret = show_available_vars(params.events, params.nevents,
>>  					  params.max_probe_points,
>>  					  params.target,
>>  					  params.filter,
>> -					  params.show_ext_vars);
>> +					  var_flags);
>>  		if (ret < 0)
>>  			pr_err_with_code("  Error: Failed to show vars.", ret);
>>  		return ret;
>> +	}
>>  #endif
>>  	case 'd':
>>  		ret = del_perf_probe_events(params.filter);
>> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
>> index 37a3a8b..4859656 100644
>> --- a/tools/perf/util/probe-event.c
>> +++ b/tools/perf/util/probe-event.c
>> @@ -824,7 +824,7 @@ int show_line_range(struct line_range *lr, const char *module, bool user)
>>  static int show_available_vars_at(struct debuginfo *dinfo,
>>  				  struct perf_probe_event *pev,
>>  				  int max_vls, struct strfilter *_filter,
>> -				  bool externs, const char *target)
>> +				  unsigned long var_flags, const char *target)
>>  {
>>  	char *buf;
>>  	int ret, i, nvars;
>> @@ -839,12 +839,12 @@ static int show_available_vars_at(struct debuginfo *dinfo,
>>  	pr_debug("Searching variables at %s\n", buf);
>>  
>>  	ret = debuginfo__find_available_vars_at(dinfo, pev, &vls,
>> -						max_vls, externs);
>> +						max_vls, var_flags);
>>  	if (!ret) {  /* Not found, retry with an alternative */
>>  		ret = get_alternative_probe_event(dinfo, pev, &tmp, target);
>>  		if (!ret) {
>>  			ret = debuginfo__find_available_vars_at(dinfo, pev,
>> -						&vls, max_vls, externs);
>> +						&vls, max_vls, var_flags);
>>  			/* Release the old probe_point */
>>  			clear_perf_probe_point(&tmp);
>>  		}
>> @@ -892,7 +892,7 @@ end:
>>  /* Show available variables on given probe point */
>>  int show_available_vars(struct perf_probe_event *pevs, int npevs,
>>  			int max_vls, const char *module,
>> -			struct strfilter *_filter, bool externs)
>> +			struct strfilter *_filter, unsigned long var_flags)
>>  {
>>  	int i, ret = 0;
>>  	struct debuginfo *dinfo;
>> @@ -911,7 +911,7 @@ int show_available_vars(struct perf_probe_event *pevs, int npevs,
>>  
>>  	for (i = 0; i < npevs && ret >= 0; i++)
>>  		ret = show_available_vars_at(dinfo, &pevs[i], max_vls, _filter,
>> -					     externs, module);
>> +					     var_flags, module);
>>  
>>  	debuginfo__delete(dinfo);
>>  out:
>> @@ -954,7 +954,7 @@ int show_available_vars(struct perf_probe_event *pevs __maybe_unused,
>>  			int npevs __maybe_unused, int max_vls __maybe_unused,
>>  			const char *module __maybe_unused,
>>  			struct strfilter *filter __maybe_unused,
>> -			bool externs __maybe_unused)
>> +			unsigned long var_flags __maybe_unused)
>>  {
>>  	pr_warning("Debuginfo-analysis is not supported.\n");
>>  	return -ENOSYS;
>> diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
>> index e10aedc..637b59e 100644
>> --- a/tools/perf/util/probe-event.h
>> +++ b/tools/perf/util/probe-event.h
>> @@ -132,7 +132,8 @@ extern int show_line_range(struct line_range *lr, const char *module,
>>  			   bool user);
>>  extern int show_available_vars(struct perf_probe_event *pevs, int npevs,
>>  			       int max_probe_points, const char *module,
>> -			       struct strfilter *filter, bool externs);
>> +			       struct strfilter *filter,
>> +			       unsigned long var_flags);
>>  extern int show_available_funcs(const char *module, struct strfilter *filter,
>>  				bool user);
>>  bool arch__prefers_symtab(void);
>> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
>> index 7b645ff40..6bbdf3f 100644
>> --- a/tools/perf/util/probe-finder.c
>> +++ b/tools/perf/util/probe-finder.c
>> @@ -1305,7 +1305,7 @@ static int add_available_vars(Dwarf_Die *sc_die, struct probe_finder *pf)
>>  	die_find_child(sc_die, collect_variables_cb, (void *)af, &die_mem);
>>  
>>  	/* Find external variables */
>> -	if (!af->externs)
>> +	if (!test_bit(VAR_FLAGS_EXTERN, &af->var_flags))
>>  		goto out;
>>  	/* Don't need to search child DIE for externs. */
>>  	af->child = false;
>> @@ -1328,12 +1328,12 @@ out:
>>  int debuginfo__find_available_vars_at(struct debuginfo *dbg,
>>  				      struct perf_probe_event *pev,
>>  				      struct variable_list **vls,
>> -				      int max_vls, bool externs)
>> +				      int max_vls, unsigned long var_flags)
>>  {
>>  	struct available_var_finder af = {
>>  			.pf = {.pev = pev, .callback = add_available_vars},
>>  			.mod = dbg->mod,
>> -			.max_vls = max_vls, .externs = externs};
>> +			.max_vls = max_vls, .var_flags = var_flags};
>>  	int ret;
>>  
>>  	/* Allocate result vls array */
>> diff --git a/tools/perf/util/probe-finder.h b/tools/perf/util/probe-finder.h
>> index f53553d..95bb79d 100644
>> --- a/tools/perf/util/probe-finder.h
>> +++ b/tools/perf/util/probe-finder.h
>> @@ -10,6 +10,10 @@
>>  #define MAX_PROBES		 128
>>  #define MAX_PROBE_ARGS		 128
>>  
>> +enum var_flags_bits {
>> +	VAR_FLAGS_EXTERN
>> +};
>> +
>>  #define PROBE_ARG_VARS		"$vars"
>>  #define PROBE_ARG_PARAMS	"$params"
>>  
>> @@ -56,7 +60,8 @@ extern int debuginfo__find_line_range(struct debuginfo *dbg,
>>  extern int debuginfo__find_available_vars_at(struct debuginfo *dbg,
>>  					     struct perf_probe_event *pev,
>>  					     struct variable_list **vls,
>> -					     int max_points, bool externs);
>> +					     int max_points,
>> +					     unsigned long var_flags);
>>  
>>  /* Find a src file from a DWARF tag path */
>>  int get_real_path(const char *raw_path, const char *comp_dir,
>> @@ -99,7 +104,7 @@ struct available_var_finder {
>>  	struct variable_list	*vls;		/* Found variable lists */
>>  	int			nvls;		/* Number of variable lists */
>>  	int			max_vls;	/* Max no. of variable lists */
>> -	bool			externs;	/* Find external vars too */
>> +	unsigned long		var_flags;	/* Variable flags */
>>  	bool			child;		/* Search child scopes */
>>  };
>>  
>>
>



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] perf probe: Remove length limitation for showing available variables
  2015-05-08 14:17 ` [PATCH 1/3] perf probe: Remove length limitation for showing available variables Masami Hiramatsu
@ 2015-05-09  7:16   ` He Kuang
  0 siblings, 0 replies; 10+ messages in thread
From: He Kuang @ 2015-05-09  7:16 UTC (permalink / raw)
  To: Masami Hiramatsu, a.p.zijlstra, acme, jolsa, mingo; +Cc: wangnan0, linux-kernel


On 2015/5/8 22:17, Masami Hiramatsu wrote:
> On 2015/05/08 21:23, 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, except one memory leak. please see below.

Oh yes, thanks!
>>  }
>>  
>> -#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];
>> +	struct strbuf buf;
>>  	int tag, ret;
>>  
>>  	vl = &af->vls[af->nvls - 1];
>> +	strbuf_init(&buf, 64);
>>  
>>  	tag = dwarf_tag(die_mem);
>>  	if (tag == DW_TAG_formal_parameter ||
>> @@ -1257,10 +1256,13 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data)
>>  						af->pf.fb_ops, &af->pf.sp_die,
>>  						NULL);
>>  		if (ret == 0) {

strbuf_init() should be called here to avoid useless malloc.

>> -			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);
>> +			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);
> It seems that this strbuf_release() should be called in any case,
> since strbuf_init already allocated buffer.
>
>>  		}
>>  	}
> so here is the good place to call, isn't it?
>
> Thank you,
>
>>  
>>
>



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3] perf probe: Add --range option to show variable location range
  2015-05-08 14:08   ` Masami Hiramatsu
@ 2015-05-09  7:41     ` He Kuang
  2015-05-10  3:02       ` Masami Hiramatsu
  0 siblings, 1 reply; 10+ messages in thread
From: He Kuang @ 2015-05-09  7:41 UTC (permalink / raw)
  To: Masami Hiramatsu, a.p.zijlstra, acme, jolsa, mingo; +Cc: wangnan0, linux-kernel


On 2015/5/8 22:08, Masami Hiramatsu wrote:
> On 2015/05/08 21:23, He Kuang wrote:
>> It is not easy for users to get the accurate byte offset or the line
>> number where a local variable can be probed. With '--range' option,
>> local variables in scope of the probe point are showed with byte offset
>> range, and can be added according to this range information.
> Interesting idea :)
>
>> For example, there are some variables in function
>> generic_perform_write():
>>
>>    <generic_perform_write@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.
> Yeah, right. That's why I've introduced --vars option.
>
>> This is because source code do not match assembly, so a variable may not
>> be available in the sourcecode line where it presents. After this patch,
>> we can lookup the accurate byte offset range of a variable, 'INV'
>> indicates that this variable is not valid at the given point, but
>> available in scope:
>>
>>    $ perf probe --vars 'generic_perform_write:42' --range
>>    Available variables at generic_perform_write:42
>>          @<generic_perform_write+141>
>>                  [INV]   ssize_t written [byte offset]: <324-331>
>>                  [INV]   struct address_space_operations*        a_ops   [byte offset]: <55-61>,<170-176>,<223-246>
>>                  [VAL]   (unknown_type)  fsdata  [byte offset]: <70-307>,<346-411>
>>                  [VAL]   loff_t  pos     [byte offset]: <0-286>,<286-336>,<346-411>
>>                  [VAL]   long int        status  [byte offset]: <83-342>,<346-411>
>>                  [VAL]   long unsigned int       bytes   [byte offset]: <122-311>,<320-338>,<346-403>,<403-411>
>>                  [VAL]   struct address_space*   mapping [byte offset]: <35-344>,<346-411>
>>                  [VAL]   struct iov_iter*        i       [byte offset]: <0-340>,<346-411>
>>                  [VAL]   struct page*    page    [byte offset]: <70-307>,<346-411>
> OK, at first, I don't like frequently repeated "[byte offset]", I prefer to
> show the function name+[offset range], like below :)
>
>   [INV]   ssize_t written @<generic_perform_write+[324-331]>
>   [INV]   struct address_space_operations*        a_ops   <@generic_perform_write+[55-61,170-176,223-246]>

OK.

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

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

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

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

>
> Thank you,
>> Then it is more clearly for us to do the probe with this variable:
>>
>>    $ perf probe --add 'generic_perform_write+170 a_ops'
>>    Added new event:
>>      probe:generic_perform_write (on generic_perform_write+170 with a_ops)
>>
>> Signed-off-by: He Kuang <hekuang@huawei.com>
>> ---
>>   tools/perf/builtin-probe.c     |   6 ++-
>>   tools/perf/util/dwarf-aux.c    | 103 +++++++++++++++++++++++++++++++++++++++++
>>   tools/perf/util/dwarf-aux.h    |   2 +
>>   tools/perf/util/probe-finder.c |  69 ++++++++++++++++++++-------
>>   tools/perf/util/probe-finder.h |   3 +-
>>   5 files changed, 165 insertions(+), 18 deletions(-)
>>
>> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
>> index 19ebe93..1b20004 100644
>> --- a/tools/perf/builtin-probe.c
>> +++ b/tools/perf/builtin-probe.c
>> @@ -52,6 +52,7 @@ static struct {
>>   	bool list_events;
>>   	bool force_add;
>>   	bool show_ext_vars;
>> +	bool show_location_range;
>>   	bool uprobes;
>>   	bool quiet;
>>   	bool target_used;
>> @@ -375,6 +376,8 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
>>   		     "Show accessible variables on PROBEDEF", opt_show_vars),
>>   	OPT_BOOLEAN('\0', "externs", &params.show_ext_vars,
>>   		    "Show external variables too (with --vars only)"),
>> +	OPT_BOOLEAN('\0', "range", &params.show_location_range,
>> +		"Show variables location range in scope (with --vars only)"),
>>   	OPT_STRING('k', "vmlinux", &symbol_conf.vmlinux_name,
>>   		   "file", "vmlinux pathname"),
>>   	OPT_STRING('s', "source", &symbol_conf.source_prefix,
>> @@ -479,7 +482,8 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
>>   			params.filter = strfilter__new(DEFAULT_VAR_FILTER,
>>   						       NULL);
>>   
>> -		var_flags = params.show_ext_vars << VAR_FLAGS_EXTERN;
>> +		var_flags = params.show_ext_vars << VAR_FLAGS_EXTERN |
>> +			params.show_location_range << VAR_FLAGS_LOCATION_RANGE;
>>   
>>   		ret = show_available_vars(params.events, params.nevents,
>>   					  params.max_probe_points,
>> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
>> index 75d264e..23053c0 100644
>> --- a/tools/perf/util/dwarf-aux.c
>> +++ b/tools/perf/util/dwarf-aux.c
>> @@ -897,3 +897,106 @@ int die_get_varname(Dwarf_Die *vr_die, struct strbuf *buf)
>>   	return 0;
>>   }
>>   
>> +/**
>> + * die_get_var_innermost_scope - Get innermost scope range of given variable DIE
>> + * @sp_die: a subprogram DIE
>> + * @vr_die: a variable DIE
>> + * @buf: a strbuf for variable byte offset range
>> + *
>> + * Get the innermost scope range of @vr_die and stores it in @buf as
>> + * "[byte offset]: <NN-NN>,<NN-NN>".
>> + */
>> +static int die_get_var_innermost_scope(Dwarf_Die *sp_die, Dwarf_Die *vr_die,
>> +				struct strbuf *buf)
>> +{
>> +	Dwarf_Die *scopes;
>> +	int count;
>> +	size_t offset = 0;
>> +	Dwarf_Addr base;
>> +	Dwarf_Addr start, end;
>> +	Dwarf_Addr entry;
>> +	int ret;
>> +	bool first = true;
>> +
>> +	ret = dwarf_entrypc(sp_die, &entry);
>> +	if (ret)
>> +		return ret;
>> +
>> +	count = dwarf_getscopes_die(vr_die, &scopes);
>> +
>> +	/* (*SCOPES)[1] is the DIE for the scope containing that scope */
>> +	if (count <= 1) {
>> +		ret = -EINVAL;
>> +		goto end;
>> +	}
>> +
>> +	while ((offset = dwarf_ranges(&scopes[1], offset, &base,
>> +				&start, &end)) > 0) {
>> +		start -= entry;
>> +		end -= entry;
>> +
>> +		if (first) {
>> +			strbuf_addf(buf, "[byte offset]: <%ld-%ld>",
>> +				start, end);
>> +			first = false;
>> +		} else {
>> +			strbuf_addf(buf, ",<%ld-%ld>",
>> +				start, end);
>> +		}
>> +	}
>> +end:
>> +	free(scopes);
>> +	return ret;
>> +}
>> +
>> +/**
>> + * die_get_var_range - Get byte offset range of given variable DIE
>> + * @sp_die: a subprogram DIE
>> + * @vr_die: a variable DIE
>> + * @buf: a strbuf for type and variable name and byte offset range
>> + *
>> + * Get the byte offset range of @vr_die and stores it in @buf as
>> + * "[byte offset]: <NN-NN>,<NN-NN>".
>> + */
>> +int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, struct strbuf *buf)
>> +{
>> +	int ret = 0;
>> +	Dwarf_Addr base;
>> +	Dwarf_Addr start, end;
>> +	Dwarf_Addr entry;
>> +	Dwarf_Op *op;
>> +	size_t nops;
>> +	size_t offset = 0;
>> +	Dwarf_Attribute attr;
>> +	bool first = true;
>> +
>> +	ret = dwarf_entrypc(sp_die, &entry);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (dwarf_attr(vr_die, DW_AT_location, &attr) == NULL)
>> +		return -EINVAL;
>> +
>> +	while ((offset = dwarf_getlocations(
>> +				&attr, offset, &base,
>> +				&start, &end, &op, &nops)) > 0) {
>> +		if (start == 0) {
>> +			/* Single Location Descriptions */
>> +			ret = die_get_var_innermost_scope(sp_die, vr_die, buf);
>> +		} else {
>> +			/* Location Lists */
>> +			start -= entry;
>> +			end -= entry;
>> +			if (first) {
>> +				strbuf_addf(buf, "[byte offset]: <%ld-%ld>",
>> +					start, end);
>> +				first = false;
>> +			} else {
>> +				strbuf_addf(buf, ",<%ld-%ld>",
>> +					start, end);
>> +			}
>> +		}
>> +	}
>> +
>> +	return ret;
>> +}
>> diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
>> index c0d6173..8390cde 100644
>> --- a/tools/perf/util/dwarf-aux.h
>> +++ b/tools/perf/util/dwarf-aux.h
>> @@ -118,4 +118,6 @@ extern int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf);
>>   
>>   /* Get the name and type of given variable DIE, stored as "type\tname" */
>>   extern int die_get_varname(Dwarf_Die *vr_die, struct strbuf *buf);
>> +extern int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die,
>> +			struct strbuf *buf);
>>   #endif
>> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
>> index 6bbdf3f..af0a7ea 100644
>> --- a/tools/perf/util/probe-finder.c
>> +++ b/tools/perf/util/probe-finder.c
>> @@ -43,6 +43,9 @@
>>   /* Kprobe tracer basic type is up to u64 */
>>   #define MAX_BASIC_TYPE_BITS	64
>>   
>> +/* Variable location invalid at addr but valid in scope */
>> +#define VARIABLE_LOCATION_INVALID_AT_ADDR	-10000
>> +
>>   /* Dwarf FL wrappers */
>>   static char *debuginfo_path;	/* Currently dummy */
>>   
>> @@ -167,7 +170,8 @@ static struct probe_trace_arg_ref *alloc_trace_arg_ref(long offs)
>>    */
>>   static int convert_variable_location(Dwarf_Die *vr_die, Dwarf_Addr addr,
>>   				     Dwarf_Op *fb_ops, Dwarf_Die *sp_die,
>> -				     struct probe_trace_arg *tvar)
>> +				     struct probe_trace_arg *tvar,
>> +				     unsigned long *var_flags)
>>   {
>>   	Dwarf_Attribute attr;
>>   	Dwarf_Addr tmp = 0;
>> @@ -177,7 +181,7 @@ static int convert_variable_location(Dwarf_Die *vr_die, Dwarf_Addr addr,
>>   	Dwarf_Word offs = 0;
>>   	bool ref = false;
>>   	const char *regs;
>> -	int ret;
>> +	int ret, ret2 = 0;
>>   
>>   	if (dwarf_attr(vr_die, DW_AT_external, &attr) != NULL)
>>   		goto static_var;
>> @@ -187,9 +191,20 @@ static int convert_variable_location(Dwarf_Die *vr_die, Dwarf_Addr addr,
>>   		return -EINVAL;	/* Broken DIE ? */
>>   	if (dwarf_getlocation_addr(&attr, addr, &op, &nops, 1) <= 0) {
>>   		ret = dwarf_entrypc(sp_die, &tmp);
>> -		if (ret || addr != tmp ||
>> -		    dwarf_tag(vr_die) != DW_TAG_formal_parameter ||
>> -		    dwarf_highpc(sp_die, &tmp))
>> +		if (ret)
>> +			return -ENOENT;
>> +
>> +		if (var_flags &&
>> +			test_bit(VAR_FLAGS_LOCATION_RANGE, var_flags) &&
>> +			(dwarf_tag(vr_die) == DW_TAG_variable)) {
>> +			ret2 = VARIABLE_LOCATION_INVALID_AT_ADDR;
>> +		} else if (addr != tmp ||
>> +			dwarf_tag(vr_die) != DW_TAG_formal_parameter) {
>> +			return -ENOENT;
>> +		}
>> +
>> +		ret = dwarf_highpc(sp_die, &tmp);
>> +		if (ret)
>>   			return -ENOENT;
>>   		/*
>>   		 * This is fuzzed by fentry mcount. We try to find the
>> @@ -210,7 +225,7 @@ found:
>>   	if (op->atom == DW_OP_addr) {
>>   static_var:
>>   		if (!tvar)
>> -			return 0;
>> +			return ret2;
>>   		/* Static variables on memory (not stack), make @varname */
>>   		ret = strlen(dwarf_diename(vr_die));
>>   		tvar->value = zalloc(ret + 2);
>> @@ -220,7 +235,7 @@ static_var:
>>   		tvar->ref = alloc_trace_arg_ref((long)offs);
>>   		if (tvar->ref == NULL)
>>   			return -ENOMEM;
>> -		return 0;
>> +		return ret2;
>>   	}
>>   
>>   	/* If this is based on frame buffer, set the offset */
>> @@ -250,7 +265,7 @@ static_var:
>>   	}
>>   
>>   	if (!tvar)
>> -		return 0;
>> +		return ret2;
>>   
>>   	regs = get_arch_regstr(regn);
>>   	if (!regs) {
>> @@ -269,7 +284,7 @@ static_var:
>>   		if (tvar->ref == NULL)
>>   			return -ENOMEM;
>>   	}
>> -	return 0;
>> +	return ret2;
>>   }
>>   
>>   #define BYTES_TO_BITS(nb)	((nb) * BITS_PER_LONG / sizeof(long))
>> @@ -516,7 +531,7 @@ static int convert_variable(Dwarf_Die *vr_die, struct probe_finder *pf)
>>   		 dwarf_diename(vr_die));
>>   
>>   	ret = convert_variable_location(vr_die, pf->addr, pf->fb_ops,
>> -					&pf->sp_die, pf->tvar);
>> +					&pf->sp_die, pf->tvar, NULL);
>>   	if (ret == -ENOENT || ret == -EINVAL)
>>   		pr_err("Failed to find the location of %s at this address.\n"
>>   		       " Perhaps, it has been optimized out.\n", pf->pvar->var);
>> @@ -1105,7 +1120,7 @@ static int copy_variables_cb(Dwarf_Die *die_mem, void *data)
>>   	    (tag == DW_TAG_variable && vf->vars)) {
>>   		if (convert_variable_location(die_mem, vf->pf->addr,
>>   					      vf->pf->fb_ops, &pf->sp_die,
>> -					      NULL) == 0) {
>> +					      NULL, NULL) == 0) {
>>   			vf->args[vf->nargs].var = (char *)dwarf_diename(die_mem);
>>   			if (vf->args[vf->nargs].var == NULL) {
>>   				vf->ret = -ENOMEM;
>> @@ -1254,11 +1269,33 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data)
>>   	    tag == DW_TAG_variable) {
>>   		ret = convert_variable_location(die_mem, af->pf.addr,
>>   						af->pf.fb_ops, &af->pf.sp_die,
>> -						NULL);
>> -		if (ret == 0) {
>> -			ret = die_get_varname(die_mem, &buf);
>> -			pr_debug2("Add new var: %s\n", buf.buf);
>> -			if (ret == 0) {
>> +						NULL, &af->var_flags);
>> +		if (ret == 0 || ret == VARIABLE_LOCATION_INVALID_AT_ADDR) {
>> +			int ret2;
>> +			bool show_range = test_bit(VAR_FLAGS_LOCATION_RANGE,
>> +						&af->var_flags);
>> +			bool externs = !af->child;
>> +
>> +			if (show_range) {
>> +				if (!externs) {
>> +					if (ret)
>> +						strbuf_addf(&buf, "[INV]\t");
>> +					else
>> +						strbuf_addf(&buf, "[VAL]\t");
>> +				} else
>> +					strbuf_addf(&buf, "[EXT]\t");
>> +			}
>> +
>> +			ret2 = die_get_varname(die_mem, &buf);
>> +
>> +			if (!ret2 && show_range && !externs) {
>> +				strbuf_addf(&buf, "\t");
>> +				ret2 = die_get_var_range(&af->pf.sp_die,
>> +							die_mem, &buf);
>> +			}
>> +
>> +			pr_debug("Add new var: %s\n", buf.buf);
>> +			if (ret2 == 0) {
>>   				strlist__add(vl->vars,
>>   					strbuf_detach(&buf, NULL));
>>   			}
>> diff --git a/tools/perf/util/probe-finder.h b/tools/perf/util/probe-finder.h
>> index 95bb79d..4b30a9b 100644
>> --- a/tools/perf/util/probe-finder.h
>> +++ b/tools/perf/util/probe-finder.h
>> @@ -11,7 +11,8 @@
>>   #define MAX_PROBE_ARGS		 128
>>   
>>   enum var_flags_bits {
>> -	VAR_FLAGS_EXTERN
>> +	VAR_FLAGS_EXTERN,
>> +	VAR_FLAGS_LOCATION_RANGE
>>   };
>>   
>>   #define PROBE_ARG_VARS		"$vars"
>>
>



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3] perf probe: Add --range option to show variable location range
  2015-05-09  7:41     ` He Kuang
@ 2015-05-10  3:02       ` Masami Hiramatsu
  0 siblings, 0 replies; 10+ messages in thread
From: Masami Hiramatsu @ 2015-05-10  3:02 UTC (permalink / raw)
  To: He Kuang, a.p.zijlstra, acme, jolsa, mingo; +Cc: wangnan0, linux-kernel

On 2015/05/09 16:41, He Kuang wrote:
> 
> On 2015/5/8 22:08, Masami Hiramatsu wrote:
>> On 2015/05/08 21:23, He Kuang wrote:
>>> It is not easy for users to get the accurate byte offset or the line
>>> number where a local variable can be probed. With '--range' option,
>>> local variables in scope of the probe point are showed with byte offset
>>> range, and can be added according to this range information.
>> Interesting idea :)
>>
>>> For example, there are some variables in function
>>> generic_perform_write():
>>>
>>>    <generic_perform_write@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.
>> Yeah, right. That's why I've introduced --vars option.
>>
>>> This is because source code do not match assembly, so a variable may not
>>> be available in the sourcecode line where it presents. After this patch,
>>> we can lookup the accurate byte offset range of a variable, 'INV'
>>> indicates that this variable is not valid at the given point, but
>>> available in scope:
>>>
>>>    $ perf probe --vars 'generic_perform_write:42' --range
>>>    Available variables at generic_perform_write:42
>>>          @<generic_perform_write+141>
>>>                  [INV]   ssize_t written [byte offset]: <324-331>
>>>                  [INV]   struct address_space_operations*        a_ops   [byte offset]: <55-61>,<170-176>,<223-246>
>>>                  [VAL]   (unknown_type)  fsdata  [byte offset]: <70-307>,<346-411>
>>>                  [VAL]   loff_t  pos     [byte offset]: <0-286>,<286-336>,<346-411>
>>>                  [VAL]   long int        status  [byte offset]: <83-342>,<346-411>
>>>                  [VAL]   long unsigned int       bytes   [byte offset]: <122-311>,<320-338>,<346-403>,<403-411>
>>>                  [VAL]   struct address_space*   mapping [byte offset]: <35-344>,<346-411>
>>>                  [VAL]   struct iov_iter*        i       [byte offset]: <0-340>,<346-411>
>>>                  [VAL]   struct page*    page    [byte offset]: <70-307>,<346-411>
>> OK, at first, I don't like frequently repeated "[byte offset]", I prefer to
>> show the function name+[offset range], like below :)
>>
>>   [INV]   ssize_t written @<generic_perform_write+[324-331]>
>>   [INV]   struct address_space_operations*        a_ops   <@generic_perform_write+[55-61,170-176,223-246]>
> 
> OK.
> 
>>
>> By the way, 'generic_perform_write+170' may be different from
>> given line, is that OK?
> 
> I think the prefix '[INV]' indicates that difference.
> 
> Before this patch, we should reference objdump, dwarf-info, then
> dwarf-loc to find the valid range of a variable. Sometimes we
> want to view more than one variables by adding one probe event,
> to find a place two or more variables all valid is difficult.

Yes, I think this looks good idea to trace variables :)

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

OK, but please note that the optimizer sometimes arranges code
sequence different from the source code. So I recommend to ensure
the address is actually where you imagine, by using perf probe -l :)

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2015-05-10  3:02 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-08 12:23 [PATCH 1/3] perf probe: Remove length limitation for showing available variables He Kuang
2015-05-08 12:23 ` [PATCH 2/3] perf probe: Add flag for options when showing variables He Kuang
2015-05-08 13:15   ` Masami Hiramatsu
2015-05-09  6:30     ` He Kuang
2015-05-08 12:23 ` [PATCH 3/3] perf probe: Add --range option to show variable location range He Kuang
2015-05-08 14:08   ` Masami Hiramatsu
2015-05-09  7:41     ` He Kuang
2015-05-10  3:02       ` Masami Hiramatsu
2015-05-08 14:17 ` [PATCH 1/3] perf probe: Remove length limitation for showing available variables Masami Hiramatsu
2015-05-09  7:16   ` He Kuang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).