[PATCH -tip 0/3] perf-probe: Dwarf support for uprobes

[PATCH -tip 0/3] perf-probe: Dwarf support for uprobes

[Masami Hiramatsu]

Hi,

Here is the series of perf-probe patches which adds 
dwarf(debuginfo) support for uprobes.

Currently perf-probe doesn't support debuginfo for uprobes.
The lack of the debuginfo support for uprobes sometimes
confuses users (or makes them cry) because they can't use
perf-probe as for kprobes case, and that is not what I hope.

So I tried to add debuginfo support for uprobes on
perf-probe. Actually, this is not completely done yet.
We still need to add some features for make it perfect.
However, this series can provide minimum debuginfo
support for uprobes.

For example)
- Shows the probe-able lines of the given function
----
# ./perf probe -x perf --line map__load
<map__load@/home/fedora/ksrc/linux-2.6/tools/perf/util/map.c:0>
      0  int map__load(struct map *map, symbol_filter_t filter)
      1  {
      2         const char *name = map->dso->long_name;
                int nr;

      5         if (dso__loaded(map->dso, map->type))
      6                 return 0;

      8         nr = dso__load(map->dso, map, filter);
      9         if (nr < 0) {
     10                 if (map->dso->has_build_id) {
----

- Shows the available variables of the given line
----
# ./perf probe -x perf --vars map__load:8
Available variables at map__load:8
        @<map__load+96>
                char*   name
                struct map*     map
                symbol_filter_t filter
        @<map__find_symbol+112>
                char*   name
                symbol_filter_t filter
        @<map__find_symbol_by_name+136>
                char*   name
                symbol_filter_t filter
        @<map_groups__find_symbol_by_name+176>
                char*   name
                struct map*     map
                symbol_filter_t filter
----

- Set a probe with available vars on the given line
----
# ./perf probe -x perf --add 'map__load:8 $vars'

Added new events:
  probe_perf:map__load (on map__load:8 with $vars)
  probe_perf:map__load_1 (on map__load:8 with $vars)
  probe_perf:map__load_2 (on map__load:8 with $vars)
  probe_perf:map__load_3 (on map__load:8 with $vars)

You can now use it in all perf tools, such as:

        perf record -e probe_perf:map__load_3 -aR sleep 1
----

To complete this requires Namhyung's uprobe
fetch-method updates which is almost done on LKML.

TODO:
 - Convert data symbol name in arguments to address
   offset value.
 - Support distro style debuginfo path (/usr/lib/debug/...)

---

Masami Hiramatsu (3):
      [CLEANUP] perf-probe: Expand given path to absolute path
      perf-probe: Support dwarf on uprobe events
      perf-probe: Use the actual address as a hint for uprobes


 tools/perf/builtin-probe.c     |   11 ++
 tools/perf/util/probe-event.c  |  247 +++++++++++++++++++++++++++-------------
 tools/perf/util/probe-event.h  |    1 
 tools/perf/util/probe-finder.c |    1 
 4 files changed, 176 insertions(+), 84 deletions(-)

-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

[PATCH -tip 1/3] [CLEANUP] perf-probe: Expand given path to absolute path

[Masami Hiramatsu]

Expand given path to absolute path in option parser,
except for a module name. Instead of expanding it later,
this get the absolute path in early stage.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 tools/perf/builtin-probe.c    |    9 +++++++++
 tools/perf/util/probe-event.c |   11 ++---------
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 6ea9e85..b40d064 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -180,6 +180,15 @@ static int opt_set_target(const struct option *opt, const char *str,
 		else
 			return ret;
 
+		/* Expand given path to absolute path, except for modulename */
+		if (params.uprobes || strchr(str, '/')) {
+			str = realpath(str, NULL);
+			if (!str) {
+				pr_warning("Failed to find the path of %s.\n",
+					   str);
+				return ret;
+			}
+		}
 		params.target = str;
 		ret = 0;
 	}
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index d7cff57..05be5de 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2281,7 +2281,7 @@ static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec)
 	struct perf_probe_point *pp = &pev->point;
 	struct symbol *sym;
 	struct map *map = NULL;
-	char *function = NULL, *name = NULL;
+	char *function = NULL;
 	int ret = -EINVAL;
 	unsigned long long vaddr = 0;
 
@@ -2297,12 +2297,7 @@ static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec)
 		goto out;
 	}
 
-	name = realpath(exec, NULL);
-	if (!name) {
-		pr_warning("Cannot find realpath for %s.\n", exec);
-		goto out;
-	}
-	map = dso__new_map(name);
+	map = dso__new_map(exec);
 	if (!map) {
 		pr_warning("Cannot find appropriate DSO for %s.\n", exec);
 		goto out;
@@ -2367,7 +2362,5 @@ out:
 	}
 	if (function)
 		free(function);
-	if (name)
-		free(name);
 	return ret;
 }

[PATCH -tip 2/3] perf-probe: Support dwarf on uprobe events

[Masami Hiramatsu]

Support dwarf(debuginfo) based operations for uprobe events.
With this change, perf probe can analyze debuginfo of user
application binary to set up new uprobe event.
This allows perf-probe --add/--line works with -x option.
(Actually, --vars has already accepted -x option)

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 tools/perf/builtin-probe.c    |    2 
 tools/perf/util/probe-event.c |  230 +++++++++++++++++++++++++++--------------
 2 files changed, 155 insertions(+), 77 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index b40d064..7fc566a 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -420,7 +420,7 @@ int cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 	}
 
 #ifdef HAVE_DWARF_SUPPORT
-	if (params.show_lines && !params.uprobes) {
+	if (params.show_lines) {
 		if (params.mod_events) {
 			pr_err("  Error: Don't use --line with"
 			       " --add/--del.\n");
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 05be5de..e27cecb 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -186,6 +186,90 @@ static int init_user_exec(void)
 	return ret;
 }
 
+static const char *__target_symbol;
+static struct symbol *__result_sym;
+
+static int filter_target_symbol(struct map *map __maybe_unused,
+				struct symbol *sym)
+{
+	if (strcmp(__target_symbol, sym->name) == 0) {
+		__result_sym = sym;
+		return 0;
+	}
+	return 1;
+}
+
+/* Find the offset of the symbol in the executable binary */
+static int find_symbol_offset(const char *exec, const char *function,
+			      unsigned long *offs)
+{
+	struct symbol *sym;
+	struct map *map = NULL;
+	int ret = -EINVAL;
+
+	if (!offs)
+		return -EINVAL;
+
+	map = dso__new_map(exec);
+	if (!map) {
+		pr_warning("Cannot find appropriate DSO for %s.\n", exec);
+		goto out;
+	}
+	pr_debug("Search %s in %s\n", function, exec);
+	__target_symbol = function;
+	__result_sym = NULL;
+	if (map__load(map, filter_target_symbol)) {
+		pr_err("Failed to find %s in %s.\n", function, exec);
+		goto out;
+	}
+	sym = __result_sym;
+	if (!sym) {
+		pr_warning("Cannot find %s in DSO %s\n", function, exec);
+		goto out;
+	}
+
+	*offs = (map->start > sym->start) ?  map->start : 0;
+	*offs += sym->start + map->pgoff;
+	ret = 0;
+out:
+	if (map) {
+		dso__delete(map->dso);
+		map__delete(map);
+	}
+	return ret;
+}
+
+static int convert_exec_to_group(const char *exec, char **result)
+{
+	char *ptr1, *ptr2, *exec_copy;
+	char buf[64];
+	int ret;
+
+	exec_copy = strdup(exec);
+	if (!exec_copy)
+		return -ENOMEM;
+
+	ptr1 = basename(exec_copy);
+	if (!ptr1) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	ptr2 = strpbrk(ptr1, "-._");
+	if (ptr2)
+		*ptr2 = '\0';
+	ret = e_snprintf(buf, 64, "%s_%s", PERFPROBE_GROUP, ptr1);
+	if (ret < 0)
+		goto out;
+
+	*result = strdup(buf);
+	ret = *result ? 0 : -ENOMEM;
+
+out:
+	free(exec_copy);
+	return ret;
+}
+
 static int convert_to_perf_probe_point(struct probe_trace_point *tp,
 					struct perf_probe_point *pp)
 {
@@ -261,6 +345,45 @@ static int kprobe_convert_to_perf_probe(struct probe_trace_point *tp,
 	return 0;
 }
 
+static int add_exec_to_probe_trace_events(struct probe_trace_event *tevs,
+					  int ntevs, const char *exec,
+					  const char *group)
+{
+	int i, ret = 0;
+	unsigned long offset;
+	char buf[32];
+
+	if (!exec)
+		return 0;
+
+	for (i = 0; i < ntevs && ret >= 0; i++) {
+		/* Get proper offset */
+		ret = find_symbol_offset(exec, tevs[i].point.symbol, &offset);
+		if (ret < 0)
+			break;
+		offset += tevs[i].point.offset;
+		tevs[i].point.offset = 0;
+		free(tevs[i].point.symbol);
+		ret = e_snprintf(buf, 32, "0x%lx", offset);
+		if (ret < 0)
+			break;
+		tevs[i].point.module = strdup(exec);
+		tevs[i].point.symbol = strdup(buf);
+		if (!tevs[i].point.symbol || !tevs[i].point.module) {
+			ret = -ENOMEM;
+			break;
+		}
+		/* Replace group name if not given */
+		if (!group) {
+			free(tevs[i].group);
+			ret = convert_exec_to_group(exec, &tevs[i].group);
+		}
+		tevs[i].uprobes = true;
+	}
+
+	return ret;
+}
+
 static int add_module_to_probe_trace_events(struct probe_trace_event *tevs,
 					    int ntevs, const char *module)
 {
@@ -305,15 +428,6 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
 	struct debuginfo *dinfo;
 	int ntevs, ret = 0;
 
-	if (pev->uprobes) {
-		if (need_dwarf) {
-			pr_warning("Debuginfo-analysis is not yet supported"
-					" with -x/--exec option.\n");
-			return -ENOSYS;
-		}
-		return convert_name_to_addr(pev, target);
-	}
-
 	dinfo = open_debuginfo(target);
 
 	if (!dinfo) {
@@ -332,9 +446,14 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
 
 	if (ntevs > 0) {	/* Succeeded to find trace events */
 		pr_debug("find %d probe_trace_events.\n", ntevs);
-		if (target)
-			ret = add_module_to_probe_trace_events(*tevs, ntevs,
-							       target);
+		if (target) {
+			if (pev->uprobes)
+				ret = add_exec_to_probe_trace_events(*tevs,
+						 ntevs, target, pev->group);
+			else
+				ret = add_module_to_probe_trace_events(*tevs,
+						 ntevs, target);
+		}
 		return ret < 0 ? ret : ntevs;
 	}
 
@@ -654,9 +773,6 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
 		return -ENOSYS;
 	}
 
-	if (pev->uprobes)
-		return convert_name_to_addr(pev, target);
-
 	return 0;
 }
 
@@ -1916,11 +2032,26 @@ static int convert_to_probe_trace_events(struct perf_probe_event *pev,
 	int ret = 0, i;
 	struct probe_trace_event *tev;
 
+	if (pev->uprobes)
+		if (!pev->group) {
+			ret = convert_exec_to_group(target, &pev->group);
+			if (ret != 0) {
+				pr_warning("Failed to make group name.\n");
+				return ret;
+			}
+		}
+
 	/* Convert perf_probe_event with debuginfo */
 	ret = try_to_find_probe_trace_events(pev, tevs, max_tevs, target);
 	if (ret != 0)
 		return ret;	/* Found in debuginfo or got an error */
 
+	if (pev->uprobes) {
+		ret = convert_name_to_addr(pev, target);
+		if (ret < 0)
+			return ret;
+	}
+
 	/* Allocate trace event buffer */
 	tev = *tevs = zalloc(sizeof(struct probe_trace_event));
 	if (tev == NULL)
@@ -2279,88 +2410,35 @@ int show_available_funcs(const char *target, struct strfilter *_filter,
 static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec)
 {
 	struct perf_probe_point *pp = &pev->point;
-	struct symbol *sym;
-	struct map *map = NULL;
-	char *function = NULL;
 	int ret = -EINVAL;
-	unsigned long long vaddr = 0;
+	unsigned long vaddr = 0;
 
 	if (!pp->function) {
 		pr_warning("No function specified for uprobes");
 		goto out;
 	}
 
-	function = strdup(pp->function);
-	if (!function) {
-		pr_warning("Failed to allocate memory by strdup.\n");
-		ret = -ENOMEM;
-		goto out;
-	}
-
-	map = dso__new_map(exec);
-	if (!map) {
-		pr_warning("Cannot find appropriate DSO for %s.\n", exec);
-		goto out;
-	}
-	available_func_filter = strfilter__new(function, NULL);
-	if (map__load(map, filter_available_functions)) {
-		pr_err("Failed to load map.\n");
-		goto out;
-	}
-
-	sym = map__find_symbol_by_name(map, function, NULL);
-	if (!sym) {
-		pr_warning("Cannot find %s in DSO %s\n", function, exec);
+	ret = find_symbol_offset(exec, pp->function, &vaddr);
+	if (ret < 0)
 		goto out;
-	}
 
-	if (map->start > sym->start)
-		vaddr = map->start;
-	vaddr += sym->start + pp->offset + map->pgoff;
+	vaddr += pp->offset;
 	pp->offset = 0;
 
 	if (!pev->event) {
-		pev->event = function;
-		function = NULL;
-	}
-	if (!pev->group) {
-		char *ptr1, *ptr2, *exec_copy;
-
-		pev->group = zalloc(sizeof(char *) * 64);
-		exec_copy = strdup(exec);
-		if (!exec_copy) {
-			ret = -ENOMEM;
-			pr_warning("Failed to copy exec string.\n");
-			goto out;
-		}
+		pev->event = pp->function;
+	} else
+		free(pp->function);
 
-		ptr1 = strdup(basename(exec_copy));
-		if (ptr1) {
-			ptr2 = strpbrk(ptr1, "-._");
-			if (ptr2)
-				*ptr2 = '\0';
-			e_snprintf(pev->group, 64, "%s_%s", PERFPROBE_GROUP,
-					ptr1);
-			free(ptr1);
-		}
-		free(exec_copy);
-	}
-	free(pp->function);
 	pp->function = zalloc(sizeof(char *) * MAX_PROBE_ARGS);
 	if (!pp->function) {
 		ret = -ENOMEM;
 		pr_warning("Failed to allocate memory by zalloc.\n");
 		goto out;
 	}
-	e_snprintf(pp->function, MAX_PROBE_ARGS, "0x%llx", vaddr);
+	e_snprintf(pp->function, MAX_PROBE_ARGS, "0x%lx", vaddr);
 	ret = 0;
 
 out:
-	if (map) {
-		dso__delete(map->dso);
-		map__delete(map);
-	}
-	if (function)
-		free(function);
 	return ret;
 }

[PATCH -tip 3/3] perf-probe: Use the actual address as a hint for uprobes

[Masami Hiramatsu]

Use the actual address of tracepoint as a hint to find
different local symbols. Since sometimes there are local
symbols which have same name, it is impossible to determine
which symbol should be used. This saves the actual address
from debuginfo and use it as a hint later.

The reason why we don't use the address directly is that
the address stored in the debuginfo has some section offset.
Thus this just uses the last 12 bits of the address as a
hint when searching the symbol in the map.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 tools/perf/util/probe-event.c  |   16 +++++++++++++---
 tools/perf/util/probe-event.h  |    1 +
 tools/perf/util/probe-finder.c |    1 +
 3 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index e27cecb..e5fbda3 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -187,11 +187,18 @@ static int init_user_exec(void)
 }
 
 static const char *__target_symbol;
+static unsigned long __target_hint;
 static struct symbol *__result_sym;
+#define HINT_MASK	0xfff
 
 static int filter_target_symbol(struct map *map __maybe_unused,
 				struct symbol *sym)
 {
+	/* Check the last bits is same */
+	if (__target_hint)
+		if ((sym->start & HINT_MASK) != (__target_hint & HINT_MASK))
+			return 0;
+
 	if (strcmp(__target_symbol, sym->name) == 0) {
 		__result_sym = sym;
 		return 0;
@@ -201,7 +208,7 @@ static int filter_target_symbol(struct map *map __maybe_unused,
 
 /* Find the offset of the symbol in the executable binary */
 static int find_symbol_offset(const char *exec, const char *function,
-			      unsigned long *offs)
+			      unsigned long hint, unsigned long *offs)
 {
 	struct symbol *sym;
 	struct map *map = NULL;
@@ -218,6 +225,7 @@ static int find_symbol_offset(const char *exec, const char *function,
 	pr_debug("Search %s in %s\n", function, exec);
 	__target_symbol = function;
 	__result_sym = NULL;
+	__target_hint = hint;
 	if (map__load(map, filter_target_symbol)) {
 		pr_err("Failed to find %s in %s.\n", function, exec);
 		goto out;
@@ -357,8 +365,10 @@ static int add_exec_to_probe_trace_events(struct probe_trace_event *tevs,
 		return 0;
 
 	for (i = 0; i < ntevs && ret >= 0; i++) {
+		offset = tevs[i].point.address - tevs[i].point.offset;
 		/* Get proper offset */
-		ret = find_symbol_offset(exec, tevs[i].point.symbol, &offset);
+		ret = find_symbol_offset(exec, tevs[i].point.symbol,
+					 offset, &offset);
 		if (ret < 0)
 			break;
 		offset += tevs[i].point.offset;
@@ -2418,7 +2428,7 @@ static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec)
 		goto out;
 	}
 
-	ret = find_symbol_offset(exec, pp->function, &vaddr);
+	ret = find_symbol_offset(exec, pp->function, 0, &vaddr);
 	if (ret < 0)
 		goto out;
 
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index f9f3de8..d481c46 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -12,6 +12,7 @@ struct probe_trace_point {
 	char		*symbol;	/* Base symbol */
 	char		*module;	/* Module name */
 	unsigned long	offset;		/* Offset from symbol */
+	unsigned long	address;	/* Actual address of the trace point */
 	bool		retprobe;	/* Return probe flag */
 };
 
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index ffb657f..7db7e05 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -729,6 +729,7 @@ static int convert_to_trace_point(Dwarf_Die *sp_die, Dwfl_Module *mod,
 		return -ENOENT;
 	}
 	tp->offset = (unsigned long)(paddr - sym.st_value);
+	tp->address = (unsigned long)paddr;
 	tp->symbol = strdup(symbol);
 	if (!tp->symbol)
 		return -ENOMEM;

Re: [PATCH -tip 1/3] [CLEANUP] perf-probe: Expand given path to absolute path

[Arnaldo Carvalho de Melo]

Em Fri, Dec 20, 2013 at 10:02:57AM +0000, Masami Hiramatsu escreveu:
> Expand given path to absolute path in option parser,
> except for a module name. Instead of expanding it later,
> this get the absolute path in early stage.

What is the problem this solves?

Can you provide some output showing the problem, i.e. before you apply
this patch?

Then can you provide the output after the patch is applied?

- Arnaldo
 
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> ---
>  tools/perf/builtin-probe.c    |    9 +++++++++
>  tools/perf/util/probe-event.c |   11 ++---------
>  2 files changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> index 6ea9e85..b40d064 100644
> --- a/tools/perf/builtin-probe.c
> +++ b/tools/perf/builtin-probe.c
> @@ -180,6 +180,15 @@ static int opt_set_target(const struct option *opt, const char *str,
>  		else
>  			return ret;
>  
> +		/* Expand given path to absolute path, except for modulename */
> +		if (params.uprobes || strchr(str, '/')) {
> +			str = realpath(str, NULL);
> +			if (!str) {
> +				pr_warning("Failed to find the path of %s.\n",
> +					   str);
> +				return ret;
> +			}
> +		}
>  		params.target = str;
>  		ret = 0;
>  	}
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index d7cff57..05be5de 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -2281,7 +2281,7 @@ static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec)
>  	struct perf_probe_point *pp = &pev->point;
>  	struct symbol *sym;
>  	struct map *map = NULL;
> -	char *function = NULL, *name = NULL;
> +	char *function = NULL;
>  	int ret = -EINVAL;
>  	unsigned long long vaddr = 0;
>  
> @@ -2297,12 +2297,7 @@ static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec)
>  		goto out;
>  	}
>  
> -	name = realpath(exec, NULL);
> -	if (!name) {
> -		pr_warning("Cannot find realpath for %s.\n", exec);
> -		goto out;
> -	}
> -	map = dso__new_map(name);
> +	map = dso__new_map(exec);
>  	if (!map) {
>  		pr_warning("Cannot find appropriate DSO for %s.\n", exec);
>  		goto out;
> @@ -2367,7 +2362,5 @@ out:
>  	}
>  	if (function)
>  		free(function);
> -	if (name)
> -		free(name);
>  	return ret;
>  }
> 

Re: [PATCH -tip 2/3] perf-probe: Support dwarf on uprobe events

[Arnaldo Carvalho de Melo]

Em Fri, Dec 20, 2013 at 10:02:59AM +0000, Masami Hiramatsu escreveu:
> Support dwarf(debuginfo) based operations for uprobe events.
> With this change, perf probe can analyze debuginfo of user
> application binary to set up new uprobe event.
> This allows perf-probe --add/--line works with -x option.
> (Actually, --vars has already accepted -x option)

Can you provide an example?

Showing output from commands when new features are implemented can speed
up the process of having it used :-)

- Arnaldo
 
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> ---
>  tools/perf/builtin-probe.c    |    2 
>  tools/perf/util/probe-event.c |  230 +++++++++++++++++++++++++++--------------
>  2 files changed, 155 insertions(+), 77 deletions(-)
> 
> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> index b40d064..7fc566a 100644
> --- a/tools/perf/builtin-probe.c
> +++ b/tools/perf/builtin-probe.c
> @@ -420,7 +420,7 @@ int cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
>  	}
>  
>  #ifdef HAVE_DWARF_SUPPORT
> -	if (params.show_lines && !params.uprobes) {
> +	if (params.show_lines) {
>  		if (params.mod_events) {
>  			pr_err("  Error: Don't use --line with"
>  			       " --add/--del.\n");
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 05be5de..e27cecb 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -186,6 +186,90 @@ static int init_user_exec(void)
>  	return ret;
>  }
>  
> +static const char *__target_symbol;
> +static struct symbol *__result_sym;
> +
> +static int filter_target_symbol(struct map *map __maybe_unused,
> +				struct symbol *sym)
> +{
> +	if (strcmp(__target_symbol, sym->name) == 0) {
> +		__result_sym = sym;
> +		return 0;
> +	}
> +	return 1;
> +}
> +
> +/* Find the offset of the symbol in the executable binary */
> +static int find_symbol_offset(const char *exec, const char *function,
> +			      unsigned long *offs)
> +{
> +	struct symbol *sym;
> +	struct map *map = NULL;
> +	int ret = -EINVAL;
> +
> +	if (!offs)
> +		return -EINVAL;
> +
> +	map = dso__new_map(exec);
> +	if (!map) {
> +		pr_warning("Cannot find appropriate DSO for %s.\n", exec);
> +		goto out;
> +	}
> +	pr_debug("Search %s in %s\n", function, exec);
> +	__target_symbol = function;
> +	__result_sym = NULL;
> +	if (map__load(map, filter_target_symbol)) {
> +		pr_err("Failed to find %s in %s.\n", function, exec);
> +		goto out;
> +	}
> +	sym = __result_sym;
> +	if (!sym) {
> +		pr_warning("Cannot find %s in DSO %s\n", function, exec);
> +		goto out;
> +	}
> +
> +	*offs = (map->start > sym->start) ?  map->start : 0;
> +	*offs += sym->start + map->pgoff;
> +	ret = 0;
> +out:
> +	if (map) {
> +		dso__delete(map->dso);
> +		map__delete(map);
> +	}
> +	return ret;
> +}
> +
> +static int convert_exec_to_group(const char *exec, char **result)
> +{
> +	char *ptr1, *ptr2, *exec_copy;
> +	char buf[64];
> +	int ret;
> +
> +	exec_copy = strdup(exec);
> +	if (!exec_copy)
> +		return -ENOMEM;
> +
> +	ptr1 = basename(exec_copy);
> +	if (!ptr1) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	ptr2 = strpbrk(ptr1, "-._");
> +	if (ptr2)
> +		*ptr2 = '\0';
> +	ret = e_snprintf(buf, 64, "%s_%s", PERFPROBE_GROUP, ptr1);
> +	if (ret < 0)
> +		goto out;
> +
> +	*result = strdup(buf);
> +	ret = *result ? 0 : -ENOMEM;
> +
> +out:
> +	free(exec_copy);
> +	return ret;
> +}
> +
>  static int convert_to_perf_probe_point(struct probe_trace_point *tp,
>  					struct perf_probe_point *pp)
>  {
> @@ -261,6 +345,45 @@ static int kprobe_convert_to_perf_probe(struct probe_trace_point *tp,
>  	return 0;
>  }
>  
> +static int add_exec_to_probe_trace_events(struct probe_trace_event *tevs,
> +					  int ntevs, const char *exec,
> +					  const char *group)
> +{
> +	int i, ret = 0;
> +	unsigned long offset;
> +	char buf[32];
> +
> +	if (!exec)
> +		return 0;
> +
> +	for (i = 0; i < ntevs && ret >= 0; i++) {
> +		/* Get proper offset */
> +		ret = find_symbol_offset(exec, tevs[i].point.symbol, &offset);
> +		if (ret < 0)
> +			break;
> +		offset += tevs[i].point.offset;
> +		tevs[i].point.offset = 0;
> +		free(tevs[i].point.symbol);
> +		ret = e_snprintf(buf, 32, "0x%lx", offset);
> +		if (ret < 0)
> +			break;
> +		tevs[i].point.module = strdup(exec);
> +		tevs[i].point.symbol = strdup(buf);
> +		if (!tevs[i].point.symbol || !tevs[i].point.module) {
> +			ret = -ENOMEM;
> +			break;
> +		}
> +		/* Replace group name if not given */
> +		if (!group) {
> +			free(tevs[i].group);
> +			ret = convert_exec_to_group(exec, &tevs[i].group);
> +		}
> +		tevs[i].uprobes = true;
> +	}
> +
> +	return ret;
> +}
> +
>  static int add_module_to_probe_trace_events(struct probe_trace_event *tevs,
>  					    int ntevs, const char *module)
>  {
> @@ -305,15 +428,6 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
>  	struct debuginfo *dinfo;
>  	int ntevs, ret = 0;
>  
> -	if (pev->uprobes) {
> -		if (need_dwarf) {
> -			pr_warning("Debuginfo-analysis is not yet supported"
> -					" with -x/--exec option.\n");
> -			return -ENOSYS;
> -		}
> -		return convert_name_to_addr(pev, target);
> -	}
> -
>  	dinfo = open_debuginfo(target);
>  
>  	if (!dinfo) {
> @@ -332,9 +446,14 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
>  
>  	if (ntevs > 0) {	/* Succeeded to find trace events */
>  		pr_debug("find %d probe_trace_events.\n", ntevs);
> -		if (target)
> -			ret = add_module_to_probe_trace_events(*tevs, ntevs,
> -							       target);
> +		if (target) {
> +			if (pev->uprobes)
> +				ret = add_exec_to_probe_trace_events(*tevs,
> +						 ntevs, target, pev->group);
> +			else
> +				ret = add_module_to_probe_trace_events(*tevs,
> +						 ntevs, target);
> +		}
>  		return ret < 0 ? ret : ntevs;
>  	}
>  
> @@ -654,9 +773,6 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
>  		return -ENOSYS;
>  	}
>  
> -	if (pev->uprobes)
> -		return convert_name_to_addr(pev, target);
> -
>  	return 0;
>  }
>  
> @@ -1916,11 +2032,26 @@ static int convert_to_probe_trace_events(struct perf_probe_event *pev,
>  	int ret = 0, i;
>  	struct probe_trace_event *tev;
>  
> +	if (pev->uprobes)
> +		if (!pev->group) {
> +			ret = convert_exec_to_group(target, &pev->group);
> +			if (ret != 0) {
> +				pr_warning("Failed to make group name.\n");
> +				return ret;
> +			}
> +		}
> +
>  	/* Convert perf_probe_event with debuginfo */
>  	ret = try_to_find_probe_trace_events(pev, tevs, max_tevs, target);
>  	if (ret != 0)
>  		return ret;	/* Found in debuginfo or got an error */
>  
> +	if (pev->uprobes) {
> +		ret = convert_name_to_addr(pev, target);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
>  	/* Allocate trace event buffer */
>  	tev = *tevs = zalloc(sizeof(struct probe_trace_event));
>  	if (tev == NULL)
> @@ -2279,88 +2410,35 @@ int show_available_funcs(const char *target, struct strfilter *_filter,
>  static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec)
>  {
>  	struct perf_probe_point *pp = &pev->point;
> -	struct symbol *sym;
> -	struct map *map = NULL;
> -	char *function = NULL;
>  	int ret = -EINVAL;
> -	unsigned long long vaddr = 0;
> +	unsigned long vaddr = 0;
>  
>  	if (!pp->function) {
>  		pr_warning("No function specified for uprobes");
>  		goto out;
>  	}
>  
> -	function = strdup(pp->function);
> -	if (!function) {
> -		pr_warning("Failed to allocate memory by strdup.\n");
> -		ret = -ENOMEM;
> -		goto out;
> -	}
> -
> -	map = dso__new_map(exec);
> -	if (!map) {
> -		pr_warning("Cannot find appropriate DSO for %s.\n", exec);
> -		goto out;
> -	}
> -	available_func_filter = strfilter__new(function, NULL);
> -	if (map__load(map, filter_available_functions)) {
> -		pr_err("Failed to load map.\n");
> -		goto out;
> -	}
> -
> -	sym = map__find_symbol_by_name(map, function, NULL);
> -	if (!sym) {
> -		pr_warning("Cannot find %s in DSO %s\n", function, exec);
> +	ret = find_symbol_offset(exec, pp->function, &vaddr);
> +	if (ret < 0)
>  		goto out;
> -	}
>  
> -	if (map->start > sym->start)
> -		vaddr = map->start;
> -	vaddr += sym->start + pp->offset + map->pgoff;
> +	vaddr += pp->offset;
>  	pp->offset = 0;
>  
>  	if (!pev->event) {
> -		pev->event = function;
> -		function = NULL;
> -	}
> -	if (!pev->group) {
> -		char *ptr1, *ptr2, *exec_copy;
> -
> -		pev->group = zalloc(sizeof(char *) * 64);
> -		exec_copy = strdup(exec);
> -		if (!exec_copy) {
> -			ret = -ENOMEM;
> -			pr_warning("Failed to copy exec string.\n");
> -			goto out;
> -		}
> +		pev->event = pp->function;
> +	} else
> +		free(pp->function);
>  
> -		ptr1 = strdup(basename(exec_copy));
> -		if (ptr1) {
> -			ptr2 = strpbrk(ptr1, "-._");
> -			if (ptr2)
> -				*ptr2 = '\0';
> -			e_snprintf(pev->group, 64, "%s_%s", PERFPROBE_GROUP,
> -					ptr1);
> -			free(ptr1);
> -		}
> -		free(exec_copy);
> -	}
> -	free(pp->function);
>  	pp->function = zalloc(sizeof(char *) * MAX_PROBE_ARGS);
>  	if (!pp->function) {
>  		ret = -ENOMEM;
>  		pr_warning("Failed to allocate memory by zalloc.\n");
>  		goto out;
>  	}
> -	e_snprintf(pp->function, MAX_PROBE_ARGS, "0x%llx", vaddr);
> +	e_snprintf(pp->function, MAX_PROBE_ARGS, "0x%lx", vaddr);
>  	ret = 0;
>  
>  out:
> -	if (map) {
> -		dso__delete(map->dso);
> -		map__delete(map);
> -	}
> -	if (function)
> -		free(function);
>  	return ret;
>  }
> 

Re: [PATCH -tip 3/3] perf-probe: Use the actual address as a hint for uprobes

[Arnaldo Carvalho de Melo]

Em Fri, Dec 20, 2013 at 10:03:02AM +0000, Masami Hiramatsu escreveu:
> Use the actual address of tracepoint as a hint to find
> different local symbols. Since sometimes there are local
> symbols which have same name, it is impossible to determine
> which symbol should be used. This saves the actual address
> from debuginfo and use it as a hint later.
> 
> The reason why we don't use the address directly is that
> the address stored in the debuginfo has some section offset.
> Thus this just uses the last 12 bits of the address as a
> hint when searching the symbol in the map.

Again, can you provide an example of cases where before this patch the
problem happens, with actual command output?

Then output from the same command with the patch applied, showing how it
works after the fix.

This way users can more quickly use your work and, among others, I can
help you by validating it in my test machines.

Thanks,

- Arnaldo
 
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> ---
>  tools/perf/util/probe-event.c  |   16 +++++++++++++---
>  tools/perf/util/probe-event.h  |    1 +
>  tools/perf/util/probe-finder.c |    1 +
>  3 files changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index e27cecb..e5fbda3 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -187,11 +187,18 @@ static int init_user_exec(void)
>  }
>  
>  static const char *__target_symbol;
> +static unsigned long __target_hint;
>  static struct symbol *__result_sym;
> +#define HINT_MASK	0xfff
>  
>  static int filter_target_symbol(struct map *map __maybe_unused,
>  				struct symbol *sym)
>  {
> +	/* Check the last bits is same */
> +	if (__target_hint)
> +		if ((sym->start & HINT_MASK) != (__target_hint & HINT_MASK))
> +			return 0;
> +
>  	if (strcmp(__target_symbol, sym->name) == 0) {
>  		__result_sym = sym;
>  		return 0;
> @@ -201,7 +208,7 @@ static int filter_target_symbol(struct map *map __maybe_unused,
>  
>  /* Find the offset of the symbol in the executable binary */
>  static int find_symbol_offset(const char *exec, const char *function,
> -			      unsigned long *offs)
> +			      unsigned long hint, unsigned long *offs)
>  {
>  	struct symbol *sym;
>  	struct map *map = NULL;
> @@ -218,6 +225,7 @@ static int find_symbol_offset(const char *exec, const char *function,
>  	pr_debug("Search %s in %s\n", function, exec);
>  	__target_symbol = function;
>  	__result_sym = NULL;
> +	__target_hint = hint;
>  	if (map__load(map, filter_target_symbol)) {
>  		pr_err("Failed to find %s in %s.\n", function, exec);
>  		goto out;
> @@ -357,8 +365,10 @@ static int add_exec_to_probe_trace_events(struct probe_trace_event *tevs,
>  		return 0;
>  
>  	for (i = 0; i < ntevs && ret >= 0; i++) {
> +		offset = tevs[i].point.address - tevs[i].point.offset;
>  		/* Get proper offset */
> -		ret = find_symbol_offset(exec, tevs[i].point.symbol, &offset);
> +		ret = find_symbol_offset(exec, tevs[i].point.symbol,
> +					 offset, &offset);
>  		if (ret < 0)
>  			break;
>  		offset += tevs[i].point.offset;
> @@ -2418,7 +2428,7 @@ static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec)
>  		goto out;
>  	}
>  
> -	ret = find_symbol_offset(exec, pp->function, &vaddr);
> +	ret = find_symbol_offset(exec, pp->function, 0, &vaddr);
>  	if (ret < 0)
>  		goto out;
>  
> diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> index f9f3de8..d481c46 100644
> --- a/tools/perf/util/probe-event.h
> +++ b/tools/perf/util/probe-event.h
> @@ -12,6 +12,7 @@ struct probe_trace_point {
>  	char		*symbol;	/* Base symbol */
>  	char		*module;	/* Module name */
>  	unsigned long	offset;		/* Offset from symbol */
> +	unsigned long	address;	/* Actual address of the trace point */
>  	bool		retprobe;	/* Return probe flag */
>  };
>  
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index ffb657f..7db7e05 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -729,6 +729,7 @@ static int convert_to_trace_point(Dwarf_Die *sp_die, Dwfl_Module *mod,
>  		return -ENOENT;
>  	}
>  	tp->offset = (unsigned long)(paddr - sym.st_value);
> +	tp->address = (unsigned long)paddr;
>  	tp->symbol = strdup(symbol);
>  	if (!tp->symbol)
>  		return -ENOMEM;
> 

Re: [PATCH -tip 1/3] [CLEANUP] perf-probe: Expand given path to absolute path

[Masami Hiramatsu]

(2013/12/21 3:00), Arnaldo Carvalho de Melo wrote:
> Em Fri, Dec 20, 2013 at 10:02:57AM +0000, Masami Hiramatsu escreveu:
>> Expand given path to absolute path in option parser,
>> except for a module name. Instead of expanding it later,
>> this get the absolute path in early stage.
> 
> What is the problem this solves?
> 
> Can you provide some output showing the problem, i.e. before you apply
> this patch?

No, this is just a code cleanup, for the later enhancements.
Should I put it into the next patch?

Thank you,

> 
> Then can you provide the output after the patch is applied?
> 
> - Arnaldo
>  
>> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>> ---
>>  tools/perf/builtin-probe.c    |    9 +++++++++
>>  tools/perf/util/probe-event.c |   11 ++---------
>>  2 files changed, 11 insertions(+), 9 deletions(-)
>>
>> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
>> index 6ea9e85..b40d064 100644
>> --- a/tools/perf/builtin-probe.c
>> +++ b/tools/perf/builtin-probe.c
>> @@ -180,6 +180,15 @@ static int opt_set_target(const struct option *opt, const char *str,
>>  		else
>>  			return ret;
>>  
>> +		/* Expand given path to absolute path, except for modulename */
>> +		if (params.uprobes || strchr(str, '/')) {
>> +			str = realpath(str, NULL);
>> +			if (!str) {
>> +				pr_warning("Failed to find the path of %s.\n",
>> +					   str);
>> +				return ret;
>> +			}
>> +		}
>>  		params.target = str;
>>  		ret = 0;
>>  	}
>> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
>> index d7cff57..05be5de 100644
>> --- a/tools/perf/util/probe-event.c
>> +++ b/tools/perf/util/probe-event.c
>> @@ -2281,7 +2281,7 @@ static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec)
>>  	struct perf_probe_point *pp = &pev->point;
>>  	struct symbol *sym;
>>  	struct map *map = NULL;
>> -	char *function = NULL, *name = NULL;
>> +	char *function = NULL;
>>  	int ret = -EINVAL;
>>  	unsigned long long vaddr = 0;
>>  
>> @@ -2297,12 +2297,7 @@ static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec)
>>  		goto out;
>>  	}
>>  
>> -	name = realpath(exec, NULL);
>> -	if (!name) {
>> -		pr_warning("Cannot find realpath for %s.\n", exec);
>> -		goto out;
>> -	}
>> -	map = dso__new_map(name);
>> +	map = dso__new_map(exec);
>>  	if (!map) {
>>  		pr_warning("Cannot find appropriate DSO for %s.\n", exec);
>>  		goto out;
>> @@ -2367,7 +2362,5 @@ out:
>>  	}
>>  	if (function)
>>  		free(function);
>> -	if (name)
>> -		free(name);
>>  	return ret;
>>  }
>>
> 


-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

Re: [PATCH -tip 2/3] perf-probe: Support dwarf on uprobe events

[Masami Hiramatsu]

(2013/12/21 3:01), Arnaldo Carvalho de Melo wrote:
> Em Fri, Dec 20, 2013 at 10:02:59AM +0000, Masami Hiramatsu escreveu:
>> Support dwarf(debuginfo) based operations for uprobe events.
>> With this change, perf probe can analyze debuginfo of user
>> application binary to set up new uprobe event.
>> This allows perf-probe --add/--line works with -x option.
>> (Actually, --vars has already accepted -x option)
> 
> Can you provide an example?
> 

OK, here is the examples from 0/3. :)
Or, should I better put this into the patch description too?

For example)
- Shows the probe-able lines of the given function
----
# ./perf probe -x perf --line map__load
<map__load@/home/fedora/ksrc/linux-2.6/tools/perf/util/map.c:0>
      0  int map__load(struct map *map, symbol_filter_t filter)
      1  {
      2         const char *name = map->dso->long_name;
                int nr;

      5         if (dso__loaded(map->dso, map->type))
      6                 return 0;

      8         nr = dso__load(map->dso, map, filter);
      9         if (nr < 0) {
     10                 if (map->dso->has_build_id) {
----

- Shows the available variables of the given line
----
# ./perf probe -x perf --vars map__load:8
Available variables at map__load:8
        @<map__load+96>
                char*   name
                struct map*     map
                symbol_filter_t filter
        @<map__find_symbol+112>
                char*   name
                symbol_filter_t filter
        @<map__find_symbol_by_name+136>
                char*   name
                symbol_filter_t filter
        @<map_groups__find_symbol_by_name+176>
                char*   name
                struct map*     map
                symbol_filter_t filter
----

- Set a probe with available vars on the given line
----
# ./perf probe -x perf --add 'map__load:8 $vars'

Added new events:
  probe_perf:map__load (on map__load:8 with $vars)
  probe_perf:map__load_1 (on map__load:8 with $vars)
  probe_perf:map__load_2 (on map__load:8 with $vars)
  probe_perf:map__load_3 (on map__load:8 with $vars)

You can now use it in all perf tools, such as:

        perf record -e probe_perf:map__load_3 -aR sleep 1
----

> Showing output from commands when new features are implemented can speed
> up the process of having it used :-)
> 
> - Arnaldo
>  
>> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>> ---
>>  tools/perf/builtin-probe.c    |    2 
>>  tools/perf/util/probe-event.c |  230 +++++++++++++++++++++++++++--------------
>>  2 files changed, 155 insertions(+), 77 deletions(-)
>>
>> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
>> index b40d064..7fc566a 100644
>> --- a/tools/perf/builtin-probe.c
>> +++ b/tools/perf/builtin-probe.c
>> @@ -420,7 +420,7 @@ int cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
>>  	}
>>  
>>  #ifdef HAVE_DWARF_SUPPORT
>> -	if (params.show_lines && !params.uprobes) {
>> +	if (params.show_lines) {
>>  		if (params.mod_events) {
>>  			pr_err("  Error: Don't use --line with"
>>  			       " --add/--del.\n");
>> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
>> index 05be5de..e27cecb 100644
>> --- a/tools/perf/util/probe-event.c
>> +++ b/tools/perf/util/probe-event.c
>> @@ -186,6 +186,90 @@ static int init_user_exec(void)
>>  	return ret;
>>  }
>>  
>> +static const char *__target_symbol;
>> +static struct symbol *__result_sym;
>> +
>> +static int filter_target_symbol(struct map *map __maybe_unused,
>> +				struct symbol *sym)
>> +{
>> +	if (strcmp(__target_symbol, sym->name) == 0) {
>> +		__result_sym = sym;
>> +		return 0;
>> +	}
>> +	return 1;
>> +}
>> +
>> +/* Find the offset of the symbol in the executable binary */
>> +static int find_symbol_offset(const char *exec, const char *function,
>> +			      unsigned long *offs)
>> +{
>> +	struct symbol *sym;
>> +	struct map *map = NULL;
>> +	int ret = -EINVAL;
>> +
>> +	if (!offs)
>> +		return -EINVAL;
>> +
>> +	map = dso__new_map(exec);
>> +	if (!map) {
>> +		pr_warning("Cannot find appropriate DSO for %s.\n", exec);
>> +		goto out;
>> +	}
>> +	pr_debug("Search %s in %s\n", function, exec);
>> +	__target_symbol = function;
>> +	__result_sym = NULL;
>> +	if (map__load(map, filter_target_symbol)) {
>> +		pr_err("Failed to find %s in %s.\n", function, exec);
>> +		goto out;
>> +	}
>> +	sym = __result_sym;
>> +	if (!sym) {
>> +		pr_warning("Cannot find %s in DSO %s\n", function, exec);
>> +		goto out;
>> +	}
>> +
>> +	*offs = (map->start > sym->start) ?  map->start : 0;
>> +	*offs += sym->start + map->pgoff;
>> +	ret = 0;
>> +out:
>> +	if (map) {
>> +		dso__delete(map->dso);
>> +		map__delete(map);
>> +	}
>> +	return ret;
>> +}
>> +
>> +static int convert_exec_to_group(const char *exec, char **result)
>> +{
>> +	char *ptr1, *ptr2, *exec_copy;
>> +	char buf[64];
>> +	int ret;
>> +
>> +	exec_copy = strdup(exec);
>> +	if (!exec_copy)
>> +		return -ENOMEM;
>> +
>> +	ptr1 = basename(exec_copy);
>> +	if (!ptr1) {
>> +		ret = -EINVAL;
>> +		goto out;
>> +	}
>> +
>> +	ptr2 = strpbrk(ptr1, "-._");
>> +	if (ptr2)
>> +		*ptr2 = '\0';
>> +	ret = e_snprintf(buf, 64, "%s_%s", PERFPROBE_GROUP, ptr1);
>> +	if (ret < 0)
>> +		goto out;
>> +
>> +	*result = strdup(buf);
>> +	ret = *result ? 0 : -ENOMEM;
>> +
>> +out:
>> +	free(exec_copy);
>> +	return ret;
>> +}
>> +
>>  static int convert_to_perf_probe_point(struct probe_trace_point *tp,
>>  					struct perf_probe_point *pp)
>>  {
>> @@ -261,6 +345,45 @@ static int kprobe_convert_to_perf_probe(struct probe_trace_point *tp,
>>  	return 0;
>>  }
>>  
>> +static int add_exec_to_probe_trace_events(struct probe_trace_event *tevs,
>> +					  int ntevs, const char *exec,
>> +					  const char *group)
>> +{
>> +	int i, ret = 0;
>> +	unsigned long offset;
>> +	char buf[32];
>> +
>> +	if (!exec)
>> +		return 0;
>> +
>> +	for (i = 0; i < ntevs && ret >= 0; i++) {
>> +		/* Get proper offset */
>> +		ret = find_symbol_offset(exec, tevs[i].point.symbol, &offset);
>> +		if (ret < 0)
>> +			break;
>> +		offset += tevs[i].point.offset;
>> +		tevs[i].point.offset = 0;
>> +		free(tevs[i].point.symbol);
>> +		ret = e_snprintf(buf, 32, "0x%lx", offset);
>> +		if (ret < 0)
>> +			break;
>> +		tevs[i].point.module = strdup(exec);
>> +		tevs[i].point.symbol = strdup(buf);
>> +		if (!tevs[i].point.symbol || !tevs[i].point.module) {
>> +			ret = -ENOMEM;
>> +			break;
>> +		}
>> +		/* Replace group name if not given */
>> +		if (!group) {
>> +			free(tevs[i].group);
>> +			ret = convert_exec_to_group(exec, &tevs[i].group);
>> +		}
>> +		tevs[i].uprobes = true;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>>  static int add_module_to_probe_trace_events(struct probe_trace_event *tevs,
>>  					    int ntevs, const char *module)
>>  {
>> @@ -305,15 +428,6 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
>>  	struct debuginfo *dinfo;
>>  	int ntevs, ret = 0;
>>  
>> -	if (pev->uprobes) {
>> -		if (need_dwarf) {
>> -			pr_warning("Debuginfo-analysis is not yet supported"
>> -					" with -x/--exec option.\n");
>> -			return -ENOSYS;
>> -		}
>> -		return convert_name_to_addr(pev, target);
>> -	}
>> -
>>  	dinfo = open_debuginfo(target);
>>  
>>  	if (!dinfo) {
>> @@ -332,9 +446,14 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
>>  
>>  	if (ntevs > 0) {	/* Succeeded to find trace events */
>>  		pr_debug("find %d probe_trace_events.\n", ntevs);
>> -		if (target)
>> -			ret = add_module_to_probe_trace_events(*tevs, ntevs,
>> -							       target);
>> +		if (target) {
>> +			if (pev->uprobes)
>> +				ret = add_exec_to_probe_trace_events(*tevs,
>> +						 ntevs, target, pev->group);
>> +			else
>> +				ret = add_module_to_probe_trace_events(*tevs,
>> +						 ntevs, target);
>> +		}
>>  		return ret < 0 ? ret : ntevs;
>>  	}
>>  
>> @@ -654,9 +773,6 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
>>  		return -ENOSYS;
>>  	}
>>  
>> -	if (pev->uprobes)
>> -		return convert_name_to_addr(pev, target);
>> -
>>  	return 0;
>>  }
>>  
>> @@ -1916,11 +2032,26 @@ static int convert_to_probe_trace_events(struct perf_probe_event *pev,
>>  	int ret = 0, i;
>>  	struct probe_trace_event *tev;
>>  
>> +	if (pev->uprobes)
>> +		if (!pev->group) {
>> +			ret = convert_exec_to_group(target, &pev->group);
>> +			if (ret != 0) {
>> +				pr_warning("Failed to make group name.\n");
>> +				return ret;
>> +			}
>> +		}
>> +
>>  	/* Convert perf_probe_event with debuginfo */
>>  	ret = try_to_find_probe_trace_events(pev, tevs, max_tevs, target);
>>  	if (ret != 0)
>>  		return ret;	/* Found in debuginfo or got an error */
>>  
>> +	if (pev->uprobes) {
>> +		ret = convert_name_to_addr(pev, target);
>> +		if (ret < 0)
>> +			return ret;
>> +	}
>> +
>>  	/* Allocate trace event buffer */
>>  	tev = *tevs = zalloc(sizeof(struct probe_trace_event));
>>  	if (tev == NULL)
>> @@ -2279,88 +2410,35 @@ int show_available_funcs(const char *target, struct strfilter *_filter,
>>  static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec)
>>  {
>>  	struct perf_probe_point *pp = &pev->point;
>> -	struct symbol *sym;
>> -	struct map *map = NULL;
>> -	char *function = NULL;
>>  	int ret = -EINVAL;
>> -	unsigned long long vaddr = 0;
>> +	unsigned long vaddr = 0;
>>  
>>  	if (!pp->function) {
>>  		pr_warning("No function specified for uprobes");
>>  		goto out;
>>  	}
>>  
>> -	function = strdup(pp->function);
>> -	if (!function) {
>> -		pr_warning("Failed to allocate memory by strdup.\n");
>> -		ret = -ENOMEM;
>> -		goto out;
>> -	}
>> -
>> -	map = dso__new_map(exec);
>> -	if (!map) {
>> -		pr_warning("Cannot find appropriate DSO for %s.\n", exec);
>> -		goto out;
>> -	}
>> -	available_func_filter = strfilter__new(function, NULL);
>> -	if (map__load(map, filter_available_functions)) {
>> -		pr_err("Failed to load map.\n");
>> -		goto out;
>> -	}
>> -
>> -	sym = map__find_symbol_by_name(map, function, NULL);
>> -	if (!sym) {
>> -		pr_warning("Cannot find %s in DSO %s\n", function, exec);
>> +	ret = find_symbol_offset(exec, pp->function, &vaddr);
>> +	if (ret < 0)
>>  		goto out;
>> -	}
>>  
>> -	if (map->start > sym->start)
>> -		vaddr = map->start;
>> -	vaddr += sym->start + pp->offset + map->pgoff;
>> +	vaddr += pp->offset;
>>  	pp->offset = 0;
>>  
>>  	if (!pev->event) {
>> -		pev->event = function;
>> -		function = NULL;
>> -	}
>> -	if (!pev->group) {
>> -		char *ptr1, *ptr2, *exec_copy;
>> -
>> -		pev->group = zalloc(sizeof(char *) * 64);
>> -		exec_copy = strdup(exec);
>> -		if (!exec_copy) {
>> -			ret = -ENOMEM;
>> -			pr_warning("Failed to copy exec string.\n");
>> -			goto out;
>> -		}
>> +		pev->event = pp->function;
>> +	} else
>> +		free(pp->function);
>>  
>> -		ptr1 = strdup(basename(exec_copy));
>> -		if (ptr1) {
>> -			ptr2 = strpbrk(ptr1, "-._");
>> -			if (ptr2)
>> -				*ptr2 = '\0';
>> -			e_snprintf(pev->group, 64, "%s_%s", PERFPROBE_GROUP,
>> -					ptr1);
>> -			free(ptr1);
>> -		}
>> -		free(exec_copy);
>> -	}
>> -	free(pp->function);
>>  	pp->function = zalloc(sizeof(char *) * MAX_PROBE_ARGS);
>>  	if (!pp->function) {
>>  		ret = -ENOMEM;
>>  		pr_warning("Failed to allocate memory by zalloc.\n");
>>  		goto out;
>>  	}
>> -	e_snprintf(pp->function, MAX_PROBE_ARGS, "0x%llx", vaddr);
>> +	e_snprintf(pp->function, MAX_PROBE_ARGS, "0x%lx", vaddr);
>>  	ret = 0;
>>  
>>  out:
>> -	if (map) {
>> -		dso__delete(map->dso);
>> -		map__delete(map);
>> -	}
>> -	if (function)
>> -		free(function);
>>  	return ret;
>>  }
>>
> 


-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

Re: [PATCH -tip 3/3] perf-probe: Use the actual address as a hint for uprobes

[Masami Hiramatsu]

(2013/12/21 3:03), Arnaldo Carvalho de Melo wrote:
> Em Fri, Dec 20, 2013 at 10:03:02AM +0000, Masami Hiramatsu escreveu:
>> Use the actual address of tracepoint as a hint to find
>> different local symbols. Since sometimes there are local
>> symbols which have same name, it is impossible to determine
>> which symbol should be used. This saves the actual address
>> from debuginfo and use it as a hint later.
>>
>> The reason why we don't use the address directly is that
>> the address stored in the debuginfo has some section offset.
>> Thus this just uses the last 12 bits of the address as a
>> hint when searching the symbol in the map.
> 
> Again, can you provide an example of cases where before this patch the
> problem happens, with actual command output?

Ok, here is what the problem I've met,

Without this patch;
# perf probe -vfn -x perf scnprintf 2>&1
[...]
Writing event: p:probe_perf/scnprintf /kbuild/ksrc/linux-2.6/tools/perf/perf:0x8fd10
Writing event: p:probe_perf/scnprintf_1 /kbuild/ksrc/linux-2.6/tools/perf/perf:0xc02d0
Writing event: p:probe_perf/scnprintf_2 /kbuild/ksrc/linux-2.6/tools/perf/perf:0xc02d0
Writing event: p:probe_perf/scnprintf_3 /kbuild/ksrc/linux-2.6/tools/perf/perf:0x9ea00
Writing event: p:probe_perf/scnprintf_4 /kbuild/ksrc/linux-2.6/tools/perf/perf:0x25f36
Writing event: p:probe_perf/scnprintf_5 /kbuild/ksrc/linux-2.6/tools/perf/perf:0x29f90
Writing event: p:probe_perf/scnprintf_6 /kbuild/ksrc/linux-2.6/tools/perf/perf:0x29f90
Writing event: p:probe_perf/scnprintf_7 /kbuild/ksrc/linux-2.6/tools/perf/perf:0xc02d0
Writing event: p:probe_perf/scnprintf_8 /kbuild/ksrc/linux-2.6/tools/perf/perf:0xc02d0
Writing event: p:probe_perf/scnprintf_9 /kbuild/ksrc/linux-2.6/tools/perf/perf:0xc02d0

You can see there are many 0xc02d0, which is the last symbol of scnprintf in perf.
# nm perf | grep scnprintf
[...]
00000000004a3a30 t scnprintf
000000000041dad0 t scnprintf
0000000000425480 t scnprintf
0000000000442b20 t scnprintf
0000000000448b20 t scnprintf
00000000004525e0 t scnprintf
00000000004555f0 t scnprintf
00000000004678c0 t scnprintf
00000000004c02d0 t scnprintf

With this patch,
# perf probe -vfn -x perf scnprintf 2>&1
[...]
Writing event: p:probe_perf/scnprintf /kbuild/ksrc/linux-2.6/tools/perf/perf:0x1a820
Writing event: p:probe_perf/scnprintf_1 /kbuild/ksrc/linux-2.6/tools/perf/perf:0x1dad0
Writing event: p:probe_perf/scnprintf_2 /kbuild/ksrc/linux-2.6/tools/perf/perf:0x25480
Writing event: p:probe_perf/scnprintf_3 /kbuild/ksrc/linux-2.6/tools/perf/perf:0x25560
Writing event: p:probe_perf/scnprintf_4 /kbuild/ksrc/linux-2.6/tools/perf/perf:0x25f36
Writing event: p:probe_perf/scnprintf_5 /kbuild/ksrc/linux-2.6/tools/perf/perf:0x29f90
Writing event: p:probe_perf/scnprintf_6 /kbuild/ksrc/linux-2.6/tools/perf/perf:0x3e050
Writing event: p:probe_perf/scnprintf_7 /kbuild/ksrc/linux-2.6/tools/perf/perf:0x48b20
Writing event: p:probe_perf/scnprintf_8 /kbuild/ksrc/linux-2.6/tools/perf/perf:0x48b20
Writing event: p:probe_perf/scnprintf_9 /kbuild/ksrc/linux-2.6/tools/perf/perf:0x525e0

Oops, I thought 12bits are enough, but it seems not...

BTW, I'm not sure why debuginfo and nm shows symbol address + 0x400000,
and why the perf's map/symbol can remove this offset. Could you tell me
how it works?
If I can get the offset (0x400000) from binary, I don't need this kind
of ugly hacks...

Thank you,

> 
> Then output from the same command with the patch applied, showing how it
> works after the fix.
> 
> This way users can more quickly use your work and, among others, I can
> help you by validating it in my test machines.
> 
> Thanks,
> 
> - Arnaldo
>  
>> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>> ---
>>  tools/perf/util/probe-event.c  |   16 +++++++++++++---
>>  tools/perf/util/probe-event.h  |    1 +
>>  tools/perf/util/probe-finder.c |    1 +
>>  3 files changed, 15 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
>> index e27cecb..e5fbda3 100644
>> --- a/tools/perf/util/probe-event.c
>> +++ b/tools/perf/util/probe-event.c
>> @@ -187,11 +187,18 @@ static int init_user_exec(void)
>>  }
>>  
>>  static const char *__target_symbol;
>> +static unsigned long __target_hint;
>>  static struct symbol *__result_sym;
>> +#define HINT_MASK	0xfff
>>  
>>  static int filter_target_symbol(struct map *map __maybe_unused,
>>  				struct symbol *sym)
>>  {
>> +	/* Check the last bits is same */
>> +	if (__target_hint)
>> +		if ((sym->start & HINT_MASK) != (__target_hint & HINT_MASK))
>> +			return 0;
>> +
>>  	if (strcmp(__target_symbol, sym->name) == 0) {
>>  		__result_sym = sym;
>>  		return 0;
>> @@ -201,7 +208,7 @@ static int filter_target_symbol(struct map *map __maybe_unused,
>>  
>>  /* Find the offset of the symbol in the executable binary */
>>  static int find_symbol_offset(const char *exec, const char *function,
>> -			      unsigned long *offs)
>> +			      unsigned long hint, unsigned long *offs)
>>  {
>>  	struct symbol *sym;
>>  	struct map *map = NULL;
>> @@ -218,6 +225,7 @@ static int find_symbol_offset(const char *exec, const char *function,
>>  	pr_debug("Search %s in %s\n", function, exec);
>>  	__target_symbol = function;
>>  	__result_sym = NULL;
>> +	__target_hint = hint;
>>  	if (map__load(map, filter_target_symbol)) {
>>  		pr_err("Failed to find %s in %s.\n", function, exec);
>>  		goto out;
>> @@ -357,8 +365,10 @@ static int add_exec_to_probe_trace_events(struct probe_trace_event *tevs,
>>  		return 0;
>>  
>>  	for (i = 0; i < ntevs && ret >= 0; i++) {
>> +		offset = tevs[i].point.address - tevs[i].point.offset;
>>  		/* Get proper offset */
>> -		ret = find_symbol_offset(exec, tevs[i].point.symbol, &offset);
>> +		ret = find_symbol_offset(exec, tevs[i].point.symbol,
>> +					 offset, &offset);
>>  		if (ret < 0)
>>  			break;
>>  		offset += tevs[i].point.offset;
>> @@ -2418,7 +2428,7 @@ static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec)
>>  		goto out;
>>  	}
>>  
>> -	ret = find_symbol_offset(exec, pp->function, &vaddr);
>> +	ret = find_symbol_offset(exec, pp->function, 0, &vaddr);
>>  	if (ret < 0)
>>  		goto out;
>>  
>> diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
>> index f9f3de8..d481c46 100644
>> --- a/tools/perf/util/probe-event.h
>> +++ b/tools/perf/util/probe-event.h
>> @@ -12,6 +12,7 @@ struct probe_trace_point {
>>  	char		*symbol;	/* Base symbol */
>>  	char		*module;	/* Module name */
>>  	unsigned long	offset;		/* Offset from symbol */
>> +	unsigned long	address;	/* Actual address of the trace point */
>>  	bool		retprobe;	/* Return probe flag */
>>  };
>>  
>> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
>> index ffb657f..7db7e05 100644
>> --- a/tools/perf/util/probe-finder.c
>> +++ b/tools/perf/util/probe-finder.c
>> @@ -729,6 +729,7 @@ static int convert_to_trace_point(Dwarf_Die *sp_die, Dwfl_Module *mod,
>>  		return -ENOENT;
>>  	}
>>  	tp->offset = (unsigned long)(paddr - sym.st_value);
>> +	tp->address = (unsigned long)paddr;
>>  	tp->symbol = strdup(symbol);
>>  	if (!tp->symbol)
>>  		return -ENOMEM;
>>
> 


-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

Re: [PATCH -tip 1/3] [CLEANUP] perf-probe: Expand given path to absolute path

[Namhyung Kim]

Hi Masami,

On Fri, 20 Dec 2013 10:02:57 +0000, Masami Hiramatsu wrote:
> Expand given path to absolute path in option parser,
> except for a module name. Instead of expanding it later,
> this get the absolute path in early stage.
>
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> ---
>  tools/perf/builtin-probe.c    |    9 +++++++++
>  tools/perf/util/probe-event.c |   11 ++---------
>  2 files changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> index 6ea9e85..b40d064 100644
> --- a/tools/perf/builtin-probe.c
> +++ b/tools/perf/builtin-probe.c
> @@ -180,6 +180,15 @@ static int opt_set_target(const struct option *opt, const char *str,
>  		else
>  			return ret;
>  
> +		/* Expand given path to absolute path, except for modulename */
> +		if (params.uprobes || strchr(str, '/')) {
> +			str = realpath(str, NULL);
> +			if (!str) {
> +				pr_warning("Failed to find the path of %s.\n",
> +					   str);

It won't print the path since the str already was overwritten to NULL.


> +				return ret;
> +			}
> +		}
>  		params.target = str;

It now points either a dynamically allocated string or not..

Thanks,
Namhyung


>  		ret = 0;
>  	}

Re: [PATCH -tip 3/3] perf-probe: Use the actual address as a hint for uprobes

[Namhyung Kim]

On Mon, 23 Dec 2013 06:54:38 +0900, Masami Hiramatsu wrote:
> (2013/12/21 3:03), Arnaldo Carvalho de Melo wrote:
>> Em Fri, Dec 20, 2013 at 10:03:02AM +0000, Masami Hiramatsu escreveu:
> BTW, I'm not sure why debuginfo and nm shows symbol address + 0x400000,
> and why the perf's map/symbol can remove this offset. Could you tell me
> how it works?
> If I can get the offset (0x400000) from binary, I don't need this kind
> of ugly hacks...

AFAIK the actual symbol address is what nm (and debuginfo) shows.  But
perf adjusts symbol address to have a relative address from the start of
mapping (i.e. file offset) like below:

	sym.st_value -= shdr.sh_addr - shdr.sh_offset;

This way, we can handle mmap and symbol address almost uniformly
(i.e. ip = map->start + symbol->address).  But this requires the mmap
event during perf record.  For perf probe, we might need to synthesize
mapping info from the section/segment header since it doesn't have the
mmap event.  Currently, the dso__new_map() just creates a map starts
from 0.

Thanks,
Namhyung

Re: [PATCH -tip 1/3] [CLEANUP] perf-probe: Expand given path to absolute path

[Masami Hiramatsu]

(2013/12/23 15:17), Namhyung Kim wrote:
> Hi Masami,
> 
> On Fri, 20 Dec 2013 10:02:57 +0000, Masami Hiramatsu wrote:
>> Expand given path to absolute path in option parser,
>> except for a module name. Instead of expanding it later,
>> this get the absolute path in early stage.
>>
>> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>> ---
>>  tools/perf/builtin-probe.c    |    9 +++++++++
>>  tools/perf/util/probe-event.c |   11 ++---------
>>  2 files changed, 11 insertions(+), 9 deletions(-)
>>
>> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
>> index 6ea9e85..b40d064 100644
>> --- a/tools/perf/builtin-probe.c
>> +++ b/tools/perf/builtin-probe.c
>> @@ -180,6 +180,15 @@ static int opt_set_target(const struct option *opt, const char *str,
>>  		else
>>  			return ret;
>>  
>> +		/* Expand given path to absolute path, except for modulename */
>> +		if (params.uprobes || strchr(str, '/')) {
>> +			str = realpath(str, NULL);
>> +			if (!str) {
>> +				pr_warning("Failed to find the path of %s.\n",
>> +					   str);
> 
> It won't print the path since the str already was overwritten to NULL.

Oops ...

>> +				return ret;
>> +			}
>> +		}
>>  		params.target = str;
> 
> It now points either a dynamically allocated string or not..

Ah, right. OK, I'll use strdup for such case.

Thank you!

-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

Re: [PATCH -tip 3/3] perf-probe: Use the actual address as a hint for uprobes

[Masami Hiramatsu]

(2013/12/23 16:46), Namhyung Kim wrote:
> On Mon, 23 Dec 2013 06:54:38 +0900, Masami Hiramatsu wrote:
>> (2013/12/21 3:03), Arnaldo Carvalho de Melo wrote:
>>> Em Fri, Dec 20, 2013 at 10:03:02AM +0000, Masami Hiramatsu escreveu:
>> BTW, I'm not sure why debuginfo and nm shows symbol address + 0x400000,
>> and why the perf's map/symbol can remove this offset. Could you tell me
>> how it works?
>> If I can get the offset (0x400000) from binary, I don't need this kind
>> of ugly hacks...
> 
> AFAIK the actual symbol address is what nm (and debuginfo) shows.  But
> perf adjusts symbol address to have a relative address from the start of
> mapping (i.e. file offset) like below:
> 
> 	sym.st_value -= shdr.sh_addr - shdr.sh_offset;

Thanks! this is what I really need!

> This way, we can handle mmap and symbol address almost uniformly
> (i.e. ip = map->start + symbol->address).  But this requires the mmap
> event during perf record.  For perf probe, we might need to synthesize
> mapping info from the section/segment header since it doesn't have the
> mmap event.  Currently, the dso__new_map() just creates a map starts
> from 0.

I think the uprobe requires only the relative address, doesn't that?

Thank you,

-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

Re: [PATCH -tip 1/3] [CLEANUP] perf-probe: Expand given path to absolute path

[Arnaldo Carvalho de Melo]

Em Mon, Dec 23, 2013 at 06:35:09AM +0900, Masami Hiramatsu escreveu:
> (2013/12/21 3:00), Arnaldo Carvalho de Melo wrote:
> > Em Fri, Dec 20, 2013 at 10:02:57AM +0000, Masami Hiramatsu escreveu:
> >> Expand given path to absolute path in option parser,
> >> except for a module name. Instead of expanding it later,
> >> this get the absolute path in early stage.
> > 
> > What is the problem this solves?
> > 
> > Can you provide some output showing the problem, i.e. before you apply
> > this patch?
> 
> No, this is just a code cleanup, for the later enhancements.

Ok, this is just a cleanup, but what does this cleanup achieves? Why is
it better to "getting the absolute path in early stage"?

I.e. you're describing what the patch does, and I can see it from
reading code, but why is it good to do it in an early stage?

> Should I put it into the next patch?

No need for that, just, please, clarify why it is needed.
 
> Thank you,
> 
> > 
> > Then can you provide the output after the patch is applied?
> > 
> > - Arnaldo
> >  
> >> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> >> ---
> >>  tools/perf/builtin-probe.c    |    9 +++++++++
> >>  tools/perf/util/probe-event.c |   11 ++---------
> >>  2 files changed, 11 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> >> index 6ea9e85..b40d064 100644
> >> --- a/tools/perf/builtin-probe.c
> >> +++ b/tools/perf/builtin-probe.c
> >> @@ -180,6 +180,15 @@ static int opt_set_target(const struct option *opt, const char *str,
> >>  		else
> >>  			return ret;
> >>  
> >> +		/* Expand given path to absolute path, except for modulename */
> >> +		if (params.uprobes || strchr(str, '/')) {
> >> +			str = realpath(str, NULL);
> >> +			if (!str) {
> >> +				pr_warning("Failed to find the path of %s.\n",
> >> +					   str);
> >> +				return ret;
> >> +			}
> >> +		}
> >>  		params.target = str;
> >>  		ret = 0;
> >>  	}
> >> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> >> index d7cff57..05be5de 100644
> >> --- a/tools/perf/util/probe-event.c
> >> +++ b/tools/perf/util/probe-event.c
> >> @@ -2281,7 +2281,7 @@ static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec)
> >>  	struct perf_probe_point *pp = &pev->point;
> >>  	struct symbol *sym;
> >>  	struct map *map = NULL;
> >> -	char *function = NULL, *name = NULL;
> >> +	char *function = NULL;
> >>  	int ret = -EINVAL;
> >>  	unsigned long long vaddr = 0;
> >>  
> >> @@ -2297,12 +2297,7 @@ static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec)
> >>  		goto out;
> >>  	}
> >>  
> >> -	name = realpath(exec, NULL);
> >> -	if (!name) {
> >> -		pr_warning("Cannot find realpath for %s.\n", exec);
> >> -		goto out;
> >> -	}
> >> -	map = dso__new_map(name);
> >> +	map = dso__new_map(exec);
> >>  	if (!map) {
> >>  		pr_warning("Cannot find appropriate DSO for %s.\n", exec);
> >>  		goto out;
> >> @@ -2367,7 +2362,5 @@ out:
> >>  	}
> >>  	if (function)
> >>  		free(function);
> >> -	if (name)
> >> -		free(name);
> >>  	return ret;
> >>  }
> >>
> > 
> 
> 
> -- 
> Masami HIRAMATSU
> IT Management Research Dept. Linux Technology Center
> Hitachi, Ltd., Yokohama Research Laboratory
> E-mail: masami.hiramatsu.pt@hitachi.com
> 

Re: [PATCH -tip 2/3] perf-probe: Support dwarf on uprobe events

[Arnaldo Carvalho de Melo]

Em Mon, Dec 23, 2013 at 06:39:16AM +0900, Masami Hiramatsu escreveu:
> (2013/12/21 3:01), Arnaldo Carvalho de Melo wrote:
> > Em Fri, Dec 20, 2013 at 10:02:59AM +0000, Masami Hiramatsu escreveu:
> >> Support dwarf(debuginfo) based operations for uprobe events.
> >> With this change, perf probe can analyze debuginfo of user
> >> application binary to set up new uprobe event.
> >> This allows perf-probe --add/--line works with -x option.
> >> (Actually, --vars has already accepted -x option)
> > 
> > Can you provide an example?
> > 
> 
> OK, here is the examples from 0/3. :)

Pleasn readnthis message:

http://lkml.kernel.org/r/20131218141125.GT21999@twins.programming.kicks-ass.net

It is unrelated to this code being discussed in this thread, but
provides another request for having better patch descriptions and to
have relevant discussion in the corresponding patch, not just on "0/N".

> Or, should I better put this into the patch description too?

Sure thing, please!

> For example)
> - Shows the probe-able lines of the given function

The above line would be more clear as:

"The following command shows the probe-able lines of a given user space
 function, something that so far was only available in the 'perf probe'
 tool for kernel space functions:"

- Arnaldo

> ----
> # ./perf probe -x perf --line map__load
> <map__load@/home/fedora/ksrc/linux-2.6/tools/perf/util/map.c:0>
>       0  int map__load(struct map *map, symbol_filter_t filter)
>       1  {
>       2         const char *name = map->dso->long_name;
>                 int nr;
> 
>       5         if (dso__loaded(map->dso, map->type))
>       6                 return 0;
> 
>       8         nr = dso__load(map->dso, map, filter);
>       9         if (nr < 0) {
>      10                 if (map->dso->has_build_id) {
> ----
> 
> - Shows the available variables of the given line
> ----
> # ./perf probe -x perf --vars map__load:8
> Available variables at map__load:8
>         @<map__load+96>
>                 char*   name
>                 struct map*     map
>                 symbol_filter_t filter
>         @<map__find_symbol+112>
>                 char*   name
>                 symbol_filter_t filter
>         @<map__find_symbol_by_name+136>
>                 char*   name
>                 symbol_filter_t filter
>         @<map_groups__find_symbol_by_name+176>
>                 char*   name
>                 struct map*     map
>                 symbol_filter_t filter
> ----
> 
> - Set a probe with available vars on the given line
> ----
> # ./perf probe -x perf --add 'map__load:8 $vars'
> 
> Added new events:
>   probe_perf:map__load (on map__load:8 with $vars)
>   probe_perf:map__load_1 (on map__load:8 with $vars)
>   probe_perf:map__load_2 (on map__load:8 with $vars)
>   probe_perf:map__load_3 (on map__load:8 with $vars)
> 
> You can now use it in all perf tools, such as:
> 
>         perf record -e probe_perf:map__load_3 -aR sleep 1
> ----
> 
> > Showing output from commands when new features are implemented can speed
> > up the process of having it used :-)
> > 
> > - Arnaldo
> >  
> >> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> >> ---
> >>  tools/perf/builtin-probe.c    |    2 
> >>  tools/perf/util/probe-event.c |  230 +++++++++++++++++++++++++++--------------
> >>  2 files changed, 155 insertions(+), 77 deletions(-)
> >>
> >> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> >> index b40d064..7fc566a 100644
> >> --- a/tools/perf/builtin-probe.c
> >> +++ b/tools/perf/builtin-probe.c
> >> @@ -420,7 +420,7 @@ int cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> >>  	}
> >>  
> >>  #ifdef HAVE_DWARF_SUPPORT
> >> -	if (params.show_lines && !params.uprobes) {
> >> +	if (params.show_lines) {
> >>  		if (params.mod_events) {
> >>  			pr_err("  Error: Don't use --line with"
> >>  			       " --add/--del.\n");
> >> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> >> index 05be5de..e27cecb 100644
> >> --- a/tools/perf/util/probe-event.c
> >> +++ b/tools/perf/util/probe-event.c
> >> @@ -186,6 +186,90 @@ static int init_user_exec(void)
> >>  	return ret;
> >>  }
> >>  
> >> +static const char *__target_symbol;
> >> +static struct symbol *__result_sym;
> >> +
> >> +static int filter_target_symbol(struct map *map __maybe_unused,
> >> +				struct symbol *sym)
> >> +{
> >> +	if (strcmp(__target_symbol, sym->name) == 0) {
> >> +		__result_sym = sym;
> >> +		return 0;
> >> +	}
> >> +	return 1;
> >> +}
> >> +
> >> +/* Find the offset of the symbol in the executable binary */
> >> +static int find_symbol_offset(const char *exec, const char *function,
> >> +			      unsigned long *offs)
> >> +{
> >> +	struct symbol *sym;
> >> +	struct map *map = NULL;
> >> +	int ret = -EINVAL;
> >> +
> >> +	if (!offs)
> >> +		return -EINVAL;
> >> +
> >> +	map = dso__new_map(exec);
> >> +	if (!map) {
> >> +		pr_warning("Cannot find appropriate DSO for %s.\n", exec);
> >> +		goto out;
> >> +	}
> >> +	pr_debug("Search %s in %s\n", function, exec);
> >> +	__target_symbol = function;
> >> +	__result_sym = NULL;
> >> +	if (map__load(map, filter_target_symbol)) {
> >> +		pr_err("Failed to find %s in %s.\n", function, exec);
> >> +		goto out;
> >> +	}
> >> +	sym = __result_sym;
> >> +	if (!sym) {
> >> +		pr_warning("Cannot find %s in DSO %s\n", function, exec);
> >> +		goto out;
> >> +	}
> >> +
> >> +	*offs = (map->start > sym->start) ?  map->start : 0;
> >> +	*offs += sym->start + map->pgoff;
> >> +	ret = 0;
> >> +out:
> >> +	if (map) {
> >> +		dso__delete(map->dso);
> >> +		map__delete(map);
> >> +	}
> >> +	return ret;
> >> +}
> >> +
> >> +static int convert_exec_to_group(const char *exec, char **result)
> >> +{
> >> +	char *ptr1, *ptr2, *exec_copy;
> >> +	char buf[64];
> >> +	int ret;
> >> +
> >> +	exec_copy = strdup(exec);
> >> +	if (!exec_copy)
> >> +		return -ENOMEM;
> >> +
> >> +	ptr1 = basename(exec_copy);
> >> +	if (!ptr1) {
> >> +		ret = -EINVAL;
> >> +		goto out;
> >> +	}
> >> +
> >> +	ptr2 = strpbrk(ptr1, "-._");
> >> +	if (ptr2)
> >> +		*ptr2 = '\0';
> >> +	ret = e_snprintf(buf, 64, "%s_%s", PERFPROBE_GROUP, ptr1);
> >> +	if (ret < 0)
> >> +		goto out;
> >> +
> >> +	*result = strdup(buf);
> >> +	ret = *result ? 0 : -ENOMEM;
> >> +
> >> +out:
> >> +	free(exec_copy);
> >> +	return ret;
> >> +}
> >> +
> >>  static int convert_to_perf_probe_point(struct probe_trace_point *tp,
> >>  					struct perf_probe_point *pp)
> >>  {
> >> @@ -261,6 +345,45 @@ static int kprobe_convert_to_perf_probe(struct probe_trace_point *tp,
> >>  	return 0;
> >>  }
> >>  
> >> +static int add_exec_to_probe_trace_events(struct probe_trace_event *tevs,
> >> +					  int ntevs, const char *exec,
> >> +					  const char *group)
> >> +{
> >> +	int i, ret = 0;
> >> +	unsigned long offset;
> >> +	char buf[32];
> >> +
> >> +	if (!exec)
> >> +		return 0;
> >> +
> >> +	for (i = 0; i < ntevs && ret >= 0; i++) {
> >> +		/* Get proper offset */
> >> +		ret = find_symbol_offset(exec, tevs[i].point.symbol, &offset);
> >> +		if (ret < 0)
> >> +			break;
> >> +		offset += tevs[i].point.offset;
> >> +		tevs[i].point.offset = 0;
> >> +		free(tevs[i].point.symbol);
> >> +		ret = e_snprintf(buf, 32, "0x%lx", offset);
> >> +		if (ret < 0)
> >> +			break;
> >> +		tevs[i].point.module = strdup(exec);
> >> +		tevs[i].point.symbol = strdup(buf);
> >> +		if (!tevs[i].point.symbol || !tevs[i].point.module) {
> >> +			ret = -ENOMEM;
> >> +			break;
> >> +		}
> >> +		/* Replace group name if not given */
> >> +		if (!group) {
> >> +			free(tevs[i].group);
> >> +			ret = convert_exec_to_group(exec, &tevs[i].group);
> >> +		}
> >> +		tevs[i].uprobes = true;
> >> +	}
> >> +
> >> +	return ret;
> >> +}
> >> +
> >>  static int add_module_to_probe_trace_events(struct probe_trace_event *tevs,
> >>  					    int ntevs, const char *module)
> >>  {
> >> @@ -305,15 +428,6 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
> >>  	struct debuginfo *dinfo;
> >>  	int ntevs, ret = 0;
> >>  
> >> -	if (pev->uprobes) {
> >> -		if (need_dwarf) {
> >> -			pr_warning("Debuginfo-analysis is not yet supported"
> >> -					" with -x/--exec option.\n");
> >> -			return -ENOSYS;
> >> -		}
> >> -		return convert_name_to_addr(pev, target);
> >> -	}
> >> -
> >>  	dinfo = open_debuginfo(target);
> >>  
> >>  	if (!dinfo) {
> >> @@ -332,9 +446,14 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
> >>  
> >>  	if (ntevs > 0) {	/* Succeeded to find trace events */
> >>  		pr_debug("find %d probe_trace_events.\n", ntevs);
> >> -		if (target)
> >> -			ret = add_module_to_probe_trace_events(*tevs, ntevs,
> >> -							       target);
> >> +		if (target) {
> >> +			if (pev->uprobes)
> >> +				ret = add_exec_to_probe_trace_events(*tevs,
> >> +						 ntevs, target, pev->group);
> >> +			else
> >> +				ret = add_module_to_probe_trace_events(*tevs,
> >> +						 ntevs, target);
> >> +		}
> >>  		return ret < 0 ? ret : ntevs;
> >>  	}
> >>  
> >> @@ -654,9 +773,6 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
> >>  		return -ENOSYS;
> >>  	}
> >>  
> >> -	if (pev->uprobes)
> >> -		return convert_name_to_addr(pev, target);
> >> -
> >>  	return 0;
> >>  }
> >>  
> >> @@ -1916,11 +2032,26 @@ static int convert_to_probe_trace_events(struct perf_probe_event *pev,
> >>  	int ret = 0, i;
> >>  	struct probe_trace_event *tev;
> >>  
> >> +	if (pev->uprobes)
> >> +		if (!pev->group) {
> >> +			ret = convert_exec_to_group(target, &pev->group);
> >> +			if (ret != 0) {
> >> +				pr_warning("Failed to make group name.\n");
> >> +				return ret;
> >> +			}
> >> +		}
> >> +
> >>  	/* Convert perf_probe_event with debuginfo */
> >>  	ret = try_to_find_probe_trace_events(pev, tevs, max_tevs, target);
> >>  	if (ret != 0)
> >>  		return ret;	/* Found in debuginfo or got an error */
> >>  
> >> +	if (pev->uprobes) {
> >> +		ret = convert_name_to_addr(pev, target);
> >> +		if (ret < 0)
> >> +			return ret;
> >> +	}
> >> +
> >>  	/* Allocate trace event buffer */
> >>  	tev = *tevs = zalloc(sizeof(struct probe_trace_event));
> >>  	if (tev == NULL)
> >> @@ -2279,88 +2410,35 @@ int show_available_funcs(const char *target, struct strfilter *_filter,
> >>  static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec)
> >>  {
> >>  	struct perf_probe_point *pp = &pev->point;
> >> -	struct symbol *sym;
> >> -	struct map *map = NULL;
> >> -	char *function = NULL;
> >>  	int ret = -EINVAL;
> >> -	unsigned long long vaddr = 0;
> >> +	unsigned long vaddr = 0;
> >>  
> >>  	if (!pp->function) {
> >>  		pr_warning("No function specified for uprobes");
> >>  		goto out;
> >>  	}
> >>  
> >> -	function = strdup(pp->function);
> >> -	if (!function) {
> >> -		pr_warning("Failed to allocate memory by strdup.\n");
> >> -		ret = -ENOMEM;
> >> -		goto out;
> >> -	}
> >> -
> >> -	map = dso__new_map(exec);
> >> -	if (!map) {
> >> -		pr_warning("Cannot find appropriate DSO for %s.\n", exec);
> >> -		goto out;
> >> -	}
> >> -	available_func_filter = strfilter__new(function, NULL);
> >> -	if (map__load(map, filter_available_functions)) {
> >> -		pr_err("Failed to load map.\n");
> >> -		goto out;
> >> -	}
> >> -
> >> -	sym = map__find_symbol_by_name(map, function, NULL);
> >> -	if (!sym) {
> >> -		pr_warning("Cannot find %s in DSO %s\n", function, exec);
> >> +	ret = find_symbol_offset(exec, pp->function, &vaddr);
> >> +	if (ret < 0)
> >>  		goto out;
> >> -	}
> >>  
> >> -	if (map->start > sym->start)
> >> -		vaddr = map->start;
> >> -	vaddr += sym->start + pp->offset + map->pgoff;
> >> +	vaddr += pp->offset;
> >>  	pp->offset = 0;
> >>  
> >>  	if (!pev->event) {
> >> -		pev->event = function;
> >> -		function = NULL;
> >> -	}
> >> -	if (!pev->group) {
> >> -		char *ptr1, *ptr2, *exec_copy;
> >> -
> >> -		pev->group = zalloc(sizeof(char *) * 64);
> >> -		exec_copy = strdup(exec);
> >> -		if (!exec_copy) {
> >> -			ret = -ENOMEM;
> >> -			pr_warning("Failed to copy exec string.\n");
> >> -			goto out;
> >> -		}
> >> +		pev->event = pp->function;
> >> +	} else
> >> +		free(pp->function);
> >>  
> >> -		ptr1 = strdup(basename(exec_copy));
> >> -		if (ptr1) {
> >> -			ptr2 = strpbrk(ptr1, "-._");
> >> -			if (ptr2)
> >> -				*ptr2 = '\0';
> >> -			e_snprintf(pev->group, 64, "%s_%s", PERFPROBE_GROUP,
> >> -					ptr1);
> >> -			free(ptr1);
> >> -		}
> >> -		free(exec_copy);
> >> -	}
> >> -	free(pp->function);
> >>  	pp->function = zalloc(sizeof(char *) * MAX_PROBE_ARGS);
> >>  	if (!pp->function) {
> >>  		ret = -ENOMEM;
> >>  		pr_warning("Failed to allocate memory by zalloc.\n");
> >>  		goto out;
> >>  	}
> >> -	e_snprintf(pp->function, MAX_PROBE_ARGS, "0x%llx", vaddr);
> >> +	e_snprintf(pp->function, MAX_PROBE_ARGS, "0x%lx", vaddr);
> >>  	ret = 0;
> >>  
> >>  out:
> >> -	if (map) {
> >> -		dso__delete(map->dso);
> >> -		map__delete(map);
> >> -	}
> >> -	if (function)
> >> -		free(function);
> >>  	return ret;
> >>  }
> >>
> > 
> 
> 
> -- 
> Masami HIRAMATSU
> IT Management Research Dept. Linux Technology Center
> Hitachi, Ltd., Yokohama Research Laboratory
> E-mail: masami.hiramatsu.pt@hitachi.com
> 

Re: Re: [PATCH -tip 2/3] perf-probe: Support dwarf on uprobe events

[Masami Hiramatsu]

(2013/12/23 23:34), Arnaldo Carvalho de Melo wrote:
> Em Mon, Dec 23, 2013 at 06:39:16AM +0900, Masami Hiramatsu escreveu:
>> (2013/12/21 3:01), Arnaldo Carvalho de Melo wrote:
>>> Em Fri, Dec 20, 2013 at 10:02:59AM +0000, Masami Hiramatsu escreveu:
>>>> Support dwarf(debuginfo) based operations for uprobe events.
>>>> With this change, perf probe can analyze debuginfo of user
>>>> application binary to set up new uprobe event.
>>>> This allows perf-probe --add/--line works with -x option.
>>>> (Actually, --vars has already accepted -x option)
>>>
>>> Can you provide an example?
>>>
>>
>> OK, here is the examples from 0/3. :)
> 
> Pleasn readnthis message:
> 
> http://lkml.kernel.org/r/20131218141125.GT21999@twins.programming.kicks-ass.net
> 
> It is unrelated to this code being discussed in this thread, but
> provides another request for having better patch descriptions and to
> have relevant discussion in the corresponding patch, not just on "0/N".

OK, I'll take care of it.

> 
>> Or, should I better put this into the patch description too?
> 
> Sure thing, please!

I see, I'll update it with the examples in the next version.

> 
>> For example)
>> - Shows the probe-able lines of the given function
> 
> The above line would be more clear as:
> 
> "The following command shows the probe-able lines of a given user space
>  function, something that so far was only available in the 'perf probe'
>  tool for kernel space functions:"

Thanks! :)


-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

Re: [PATCH -tip 1/3] [CLEANUP] perf-probe: Expand given path to absolute path

[Masami Hiramatsu]

(2013/12/23 23:28), Arnaldo Carvalho de Melo wrote:
> Em Mon, Dec 23, 2013 at 06:35:09AM +0900, Masami Hiramatsu escreveu:
>> (2013/12/21 3:00), Arnaldo Carvalho de Melo wrote:
>>> Em Fri, Dec 20, 2013 at 10:02:57AM +0000, Masami Hiramatsu escreveu:
>>>> Expand given path to absolute path in option parser,
>>>> except for a module name. Instead of expanding it later,
>>>> this get the absolute path in early stage.
>>>
>>> What is the problem this solves?
>>>
>>> Can you provide some output showing the problem, i.e. before you apply
>>> this patch?
>>
>> No, this is just a code cleanup, for the later enhancements.
> 
> Ok, this is just a cleanup, but what does this cleanup achieves? Why is
> it better to "getting the absolute path in early stage"?
> 
> I.e. you're describing what the patch does, and I can see it from
> reading code, but why is it good to do it in an early stage?

---
Since realpath at the later stage in processing several probe point
can be called several times(even if currently doesn't, it can happen
when we expands the feature), it is waste of the performance.
Processing it once at the early stage can avoid that.
---

Is that good enough for you? :)

> 
>> Should I put it into the next patch?
> 
> No need for that, just, please, clarify why it is needed.
>  

OK, I'll do that:)

Thank you,

-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

Re: [PATCH -tip 3/3] perf-probe: Use the actual address as a hint for uprobes

[Namhyung Kim]

Hi Masami,

On Mon, 23 Dec 2013 19:50:10 +0900, Masami Hiramatsu wrote:
> (2013/12/23 16:46), Namhyung Kim wrote:
>> On Mon, 23 Dec 2013 06:54:38 +0900, Masami Hiramatsu wrote:
>>> (2013/12/21 3:03), Arnaldo Carvalho de Melo wrote:
>>>> Em Fri, Dec 20, 2013 at 10:03:02AM +0000, Masami Hiramatsu escreveu:
>>> BTW, I'm not sure why debuginfo and nm shows symbol address + 0x400000,
>>> and why the perf's map/symbol can remove this offset. Could you tell me
>>> how it works?
>>> If I can get the offset (0x400000) from binary, I don't need this kind
>>> of ugly hacks...
>> 
>> AFAIK the actual symbol address is what nm (and debuginfo) shows.  But
>> perf adjusts symbol address to have a relative address from the start of
>> mapping (i.e. file offset) like below:
>> 
>> 	sym.st_value -= shdr.sh_addr - shdr.sh_offset;
>
> Thanks! this is what I really need!
>
>> This way, we can handle mmap and symbol address almost uniformly
>> (i.e. ip = map->start + symbol->address).  But this requires the mmap
>> event during perf record.  For perf probe, we might need to synthesize
>> mapping info from the section/segment header since it doesn't have the
>> mmap event.  Currently, the dso__new_map() just creates a map starts
>> from 0.
>
> I think the uprobe requires only the relative address, doesn't that?

Yes, but fetching arguments is little different than a normal relative
address, I think.

An offset of an argument bases on the mapping address of text segment.
This fits naturally for a shared library case - base address is 0.  So
we can use the symbol address (st_value) directly.  But for executables,
the base address of text segment is 0x400000 on x86-64 and data symbol
is on 0x6XXXXX typically.  So in this case the offset given to uprobe
should be "@+0x2XXXXX" (st_value - text_base).

Thanks,
Namhyung

Re: [PATCH -tip 3/3] perf-probe: Use the actual address as a hint for uprobes

[Masami Hiramatsu]

(2013/12/24 16:54), Namhyung Kim wrote:
> Hi Masami,
> 
> On Mon, 23 Dec 2013 19:50:10 +0900, Masami Hiramatsu wrote:
>> (2013/12/23 16:46), Namhyung Kim wrote:
>>> On Mon, 23 Dec 2013 06:54:38 +0900, Masami Hiramatsu wrote:
>>>> (2013/12/21 3:03), Arnaldo Carvalho de Melo wrote:
>>>>> Em Fri, Dec 20, 2013 at 10:03:02AM +0000, Masami Hiramatsu escreveu:
>>>> BTW, I'm not sure why debuginfo and nm shows symbol address + 0x400000,
>>>> and why the perf's map/symbol can remove this offset. Could you tell me
>>>> how it works?
>>>> If I can get the offset (0x400000) from binary, I don't need this kind
>>>> of ugly hacks...
>>>
>>> AFAIK the actual symbol address is what nm (and debuginfo) shows.  But
>>> perf adjusts symbol address to have a relative address from the start of
>>> mapping (i.e. file offset) like below:
>>>
>>> 	sym.st_value -= shdr.sh_addr - shdr.sh_offset;
>>
>> Thanks! this is what I really need!

BTW, what I've found is that the perf's map has start, end and pgoffs
but those are not initialized when we load user-binary (see dso__load_sym).
I'm not sure why.

>>> This way, we can handle mmap and symbol address almost uniformly
>>> (i.e. ip = map->start + symbol->address).  But this requires the mmap
>>> event during perf record.  For perf probe, we might need to synthesize
>>> mapping info from the section/segment header since it doesn't have the
>>> mmap event.  Currently, the dso__new_map() just creates a map starts
>>> from 0.
>>
>> I think the uprobe requires only the relative address, doesn't that?
> 
> Yes, but fetching arguments is little different than a normal relative
> address, I think.

Is this for uprobe probing address? or fetching symbol(global variables)?
I'd like to support uprobes probing address first.

> An offset of an argument bases on the mapping address of text segment.
> This fits naturally for a shared library case - base address is 0.  So
> we can use the symbol address (st_value) directly.  But for executables,
> the base address of text segment is 0x400000 on x86-64 and data symbol
> is on 0x6XXXXX typically.  So in this case the offset given to uprobe
> should be "@+0x2XXXXX" (st_value - text_base).

Oh, I see. I'd better make a testcase for checking what the best
way to get such offsets.

Thank you,

-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

Re: [PATCH -tip 3/3] perf-probe: Use the actual address as a hint for uprobes

[Namhyung Kim]

On Tue, 24 Dec 2013 17:27:45 +0900, Masami Hiramatsu wrote:
> (2013/12/24 16:54), Namhyung Kim wrote:
>> Hi Masami,
>> 
>> On Mon, 23 Dec 2013 19:50:10 +0900, Masami Hiramatsu wrote:
>>> (2013/12/23 16:46), Namhyung Kim wrote:
>>>> On Mon, 23 Dec 2013 06:54:38 +0900, Masami Hiramatsu wrote:
>>>>> (2013/12/21 3:03), Arnaldo Carvalho de Melo wrote:
>>>>>> Em Fri, Dec 20, 2013 at 10:03:02AM +0000, Masami Hiramatsu escreveu:
>>>>> BTW, I'm not sure why debuginfo and nm shows symbol address + 0x400000,
>>>>> and why the perf's map/symbol can remove this offset. Could you tell me
>>>>> how it works?
>>>>> If I can get the offset (0x400000) from binary, I don't need this kind
>>>>> of ugly hacks...
>>>>
>>>> AFAIK the actual symbol address is what nm (and debuginfo) shows.  But
>>>> perf adjusts symbol address to have a relative address from the start of
>>>> mapping (i.e. file offset) like below:
>>>>
>>>> 	sym.st_value -= shdr.sh_addr - shdr.sh_offset;
>>>
>>> Thanks! this is what I really need!
>
> BTW, what I've found is that the perf's map has start, end and pgoffs
> but those are not initialized when we load user-binary (see dso__load_sym).
> I'm not sure why.

It's only set from a mmap event either sent from kernel or synthesized
using /proc/<pid>/maps.  We cannot know the load address of a library
until it gets loaded but for an executable, we could use the address of
ELF segments/sections.

>
>>>> This way, we can handle mmap and symbol address almost uniformly
>>>> (i.e. ip = map->start + symbol->address).  But this requires the mmap
>>>> event during perf record.  For perf probe, we might need to synthesize
>>>> mapping info from the section/segment header since it doesn't have the
>>>> mmap event.  Currently, the dso__new_map() just creates a map starts
>>>> from 0.
>>>
>>> I think the uprobe requires only the relative address, doesn't that?
>> 
>> Yes, but fetching arguments is little different than a normal relative
>> address, I think.
>
> Is this for uprobe probing address? or fetching symbol(global variables)?
> I'd like to support uprobes probing address first.

It's for argument fetching.  For probing, you can simply use a relative
address.

>
>> An offset of an argument bases on the mapping address of text segment.
>> This fits naturally for a shared library case - base address is 0.  So
>> we can use the symbol address (st_value) directly.  But for executables,
>> the base address of text segment is 0x400000 on x86-64 and data symbol
>> is on 0x6XXXXX typically.  So in this case the offset given to uprobe
>> should be "@+0x2XXXXX" (st_value - text_base).
>
> Oh, I see. I'd better make a testcase for checking what the best
> way to get such offsets.

Okay, please share the result then. :)

Thanks,
Namhyung

Re: Re: [PATCH -tip 3/3] perf-probe: Use the actual address as a hint for uprobes

[Masami Hiramatsu]

(2013/12/24 17:46), Namhyung Kim wrote:
> On Tue, 24 Dec 2013 17:27:45 +0900, Masami Hiramatsu wrote:
>> (2013/12/24 16:54), Namhyung Kim wrote:
>>> Hi Masami,
>>>
>>> On Mon, 23 Dec 2013 19:50:10 +0900, Masami Hiramatsu wrote:
>>>> (2013/12/23 16:46), Namhyung Kim wrote:
>>>>> On Mon, 23 Dec 2013 06:54:38 +0900, Masami Hiramatsu wrote:
>>>>>> (2013/12/21 3:03), Arnaldo Carvalho de Melo wrote:
>>>>>>> Em Fri, Dec 20, 2013 at 10:03:02AM +0000, Masami Hiramatsu escreveu:
>>>>>> BTW, I'm not sure why debuginfo and nm shows symbol address + 0x400000,
>>>>>> and why the perf's map/symbol can remove this offset. Could you tell me
>>>>>> how it works?
>>>>>> If I can get the offset (0x400000) from binary, I don't need this kind
>>>>>> of ugly hacks...
>>>>>
>>>>> AFAIK the actual symbol address is what nm (and debuginfo) shows.  But
>>>>> perf adjusts symbol address to have a relative address from the start of
>>>>> mapping (i.e. file offset) like below:
>>>>>
>>>>> 	sym.st_value -= shdr.sh_addr - shdr.sh_offset;
>>>>
>>>> Thanks! this is what I really need!
>>
>> BTW, what I've found is that the perf's map has start, end and pgoffs
>> but those are not initialized when we load user-binary (see dso__load_sym).
>> I'm not sure why.
> 
> It's only set from a mmap event either sent from kernel or synthesized
> using /proc/<pid>/maps.  We cannot know the load address of a library
> until it gets loaded but for an executable, we could use the address of
> ELF segments/sections.

I see, the problem is that the address recorded in the debuginfo
is not the relative address. Thus, I think I need to get the
".text" section offset by decoding elf file (not the debuginfo).

>>>>> This way, we can handle mmap and symbol address almost uniformly
>>>>> (i.e. ip = map->start + symbol->address).  But this requires the mmap
>>>>> event during perf record.  For perf probe, we might need to synthesize
>>>>> mapping info from the section/segment header since it doesn't have the
>>>>> mmap event.  Currently, the dso__new_map() just creates a map starts
>>>>> from 0.
>>>>
>>>> I think the uprobe requires only the relative address, doesn't that?
>>>
>>> Yes, but fetching arguments is little different than a normal relative
>>> address, I think.
>>
>> Is this for uprobe probing address? or fetching symbol(global variables)?
>> I'd like to support uprobes probing address first.
> 
> It's for argument fetching.  For probing, you can simply use a relative
> address.

Hm, OK.
BTW, I've found that current uprobe's address calculation routine is
trying to get the absolute address.

        if (map->start > sym->start)
                vaddr = map->start;
        vaddr += sym->start + pp->offset + map->pgoff;

Currently it just returns a relative address because both of map->pgoff
and map->start are zero :) But I think it should be

  vaddr = sym->start + pp->offset;

Since uprobe requires a simple relative offset.

>>> An offset of an argument bases on the mapping address of text segment.
>>> This fits naturally for a shared library case - base address is 0.  So
>>> we can use the symbol address (st_value) directly.  But for executables,
>>> the base address of text segment is 0x400000 on x86-64 and data symbol
>>> is on 0x6XXXXX typically.  So in this case the offset given to uprobe
>>> should be "@+0x2XXXXX" (st_value - text_base).
>>
>> Oh, I see. I'd better make a testcase for checking what the best
>> way to get such offsets.
> 
> Okay, please share the result then. :)

I just wrote a short test program as below;
---
#include <stdio.h>

int global_var = 0xbeef;

int target(int arg1i, char *arg2s)
{
        printf("arg1=%d, arg2=%s\n", arg1i, arg2s);
        return arg1i;
}

int main(int argc, char *argv[])
{
        int ret;

        ret = target(argc, argv[0]);
        if (ret)
                target(global_var, "test");
        return 0;
}
----

And run nm and eu-readelf as below.
---
$ nm a.out | egrep "(target|global_var)"
0000000000601034 D global_var
0000000000400530 T target
$ eu-readelf -S a.out | egrep "\\.(text|data)"
[13] .text                PROGBITS     0000000000400440 00000440 000001b4  0 AX     0   0 16
[24] .data                PROGBITS     0000000000601030 00001030 00000008  0 WA     0   0  4
---
As you can see, the .text and .data offsets will be calculated by
section start - section offset. Thus, we can do

Dwarf's global_var address - (.text start - .text offset)

for the relative global_var address (unless the kernel loads .data
section into different address.)

Thank you,

-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com