[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.hi...@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;


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

[Masami Hiramatsu]

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

[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.hi...@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]

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.hi...@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;

[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

[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

[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

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

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

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

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

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

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

[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.hi...@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;

[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

[Masami Hiramatsu]

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

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

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

[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/20131218141...@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

[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/20131218141...@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]

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

[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

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

[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

[Masami Hiramatsu]

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

[Masami Hiramatsu]

Expand given path to absolute path in the option parser,

except for a module name.

Since realpath at 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.

Changes from previous one:
- Fix not to print null string.
- Allocate memory for given path/module name everytime.

Signed-off-by: Masami Hiramatsu <masami.hi...@hitachi.com>
---

tools/perf/builtin-probe.c | 16 +++++++++++++++-
tools/perf/util/probe-event.c | 11 ++---------
2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 6ea9e85..5681266 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -169,6 +169,7 @@ static int opt_set_target(const struct option *opt, const char *str,
int unset __maybe_unused)
{
int ret = -ENOENT;
+ char *tmp;

if (str && !params.target) {
if (!strcmp(opt->long_name, "exec"))
@@ -180,7 +181,20 @@ static int opt_set_target(const struct option *opt, const char *str,
else
return ret;

- params.target = str;

+ /* Expand given path to absolute path, except for modulename */
+ if (params.uprobes || strchr(str, '/')) {

+ tmp = realpath(str, NULL);
+ if (!tmp) {

+ pr_warning("Failed to find the path of %s.\n",
+ str);

+ return ret;
+ }
+ } else {
+ tmp = strdup(str);
+ if (!tmp)
+ return -ENOMEM;
+ }
+ params.target = tmp;
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]

Hi,

Here is the 2nd version of the series of perf-probe patches
which adds basic 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, 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:

----
# ./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) {
----

And this shows the available variables at the given line of
the function.

----
# ./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
----

At last, we can now define a probe(s) with all available
variables 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/...)

- Support --list to show actual lines and executable names.

Changes from previous one:
- Fix not to print null string.
- Allocate memory for given path/module name everytime.

- Fix patch descriptions.
- Add examples in the patch description.
- Use .text section start address and dwarf symbol address
for calculating the offset of given symbol, instead of
searching the symbol in symtab again.
With this change, we can safely handle multiple local
function instances (e.g. scnprintf in perf).

---

Masami Hiramatsu (2):
[CLEANUP] perf-probe: Expand given path to absolute path
perf-probe: Support basic dwarf-based operations on uprobe events


tools/perf/builtin-probe.c | 18 ++++
tools/perf/util/probe-event.c | 162 ++++++++++++++++++++++++++++++++++------
tools/perf/util/probe-event.h | 1
tools/perf/util/probe-finder.c | 1
4 files changed, 155 insertions(+), 27 deletions(-)

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

[Masami Hiramatsu]

Support basic 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(with local variables, line numbers)
and --line works with -x option.

(Actually, --vars has already accepted -x option)

Changes from previous version:

- Add examples in the patch description.
- Use .text section start address and dwarf symbol address
for calculating the offset of given symbol, instead of
searching the symbol in symtab again.
With this change, we can safely handle multiple local
function instances (e.g. scnprintf in perf).

Signed-off-by: Masami Hiramatsu <masami.hi...@hitachi.com>
---

tools/perf/builtin-probe.c | 2 -
tools/perf/util/probe-event.c | 151 ++++++++++++++++++++++++++++++++++++----
tools/perf/util/probe-event.h | 1
tools/perf/util/probe-finder.c | 1
4 files changed, 138 insertions(+), 17 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 5681266..20b3e15 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -425,7 +425,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..2f82267 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -172,6 +172,52 @@ const char *kernel_get_module_path(const char *module)
return (dso) ? dso->long_name : NULL;
}

+/* Copied from unwind.c */
+static Elf_Scn *elf_section_by_name(Elf *elf, GElf_Ehdr *ep,
+ GElf_Shdr *shp, const char *name)
+{
+ Elf_Scn *sec = NULL;
+
+ while ((sec = elf_nextscn(elf, sec)) != NULL) {
+ char *str;
+
+ gelf_getshdr(sec, shp);
+ str = elf_strptr(elf, ep->e_shstrndx, shp->sh_name);
+ if (!strcmp(name, str))
+ break;
+ }
+
+ return sec;
+}
+
+static int get_text_start_address(const char *exec, unsigned long *address)
+{
+ Elf *elf;
+ GElf_Ehdr ehdr;
+ GElf_Shdr shdr;
+ int fd, ret = -ENOENT;
+
+ fd = open(exec, O_RDONLY);
+ if (fd < 0)
+ return -errno;
+
+ elf = elf_begin(fd, PERF_ELF_C_READ_MMAP, NULL);
+ if (elf == NULL)
+ return -EINVAL;
+
+ if (gelf_getehdr(elf, &ehdr) == NULL)
+ goto out;
+
+ if (!elf_section_by_name(elf, &ehdr, &shdr, ".text"))
+ goto out;
+
+ *address = shdr.sh_addr - shdr.sh_offset;

+ ret = 0;
+out:

+ elf_end(elf);

+ return ret;
+}
+

static int init_user_exec(void)
{
int ret = 0;
@@ -186,6 +232,37 @@ static int init_user_exec(void)
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 +338,40 @@ 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)

+{
+ int i, ret = 0;

+ unsigned long offset, stext = 0;

+ char buf[32];
+
+ if (!exec)
+ return 0;
+

+ ret = get_text_start_address(exec, &stext);
+ if (ret < 0)
+ return ret;
+

+ for (i = 0; i < ntevs && ret >= 0; i++) {

+ offset = tevs[i].point.address - stext;

+ 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;
+ }

+ 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 +416,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 +434,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);

+ else
+ ret = add_module_to_probe_trace_events(*tevs,
+ ntevs, target);
+ }
return ret < 0 ? ret : ntevs;
}

@@ -654,9 +761,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;
}

@@ -1913,14 +2017,29 @@ static int convert_to_probe_trace_events(struct perf_probe_event *pev,
int max_tevs, const char *target)
{
struct symbol *sym;
- int ret = 0, i;
+ int ret, i;
struct probe_trace_event *tev;

+ if (pev->uprobes && !pev->group) {

+ /* Replace group name if not given */

+ ret = convert_exec_to_group(target, &pev->group);
+ if (ret != 0) {

+ pr_warning("Failed to make a 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)

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;

[David Ahern]

On 12/26/13, 12:41 AM, Masami Hiramatsu wrote:
> @@ -180,7 +181,20 @@ static int opt_set_target(const struct option *opt, const char *str,
> else
> return ret;
>
> - params.target = str;
> + /* Expand given path to absolute path, except for modulename */
> + if (params.uprobes || strchr(str, '/')) {
> + tmp = realpath(str, NULL);
> + if (!tmp) {
> + pr_warning("Failed to find the path of %s.\n",
> + str);

That error message will be misleading if it is generated. How about:
Failed to get the absolute path of %s: %d\n", str, errno.

> + return ret;
> + }
> + } else {
> + tmp = strdup(str);
> + if (!tmp)
> + return -ENOMEM;
> + }
> + params.target = tmp;

When is params.target freed?

David

[Arnaldo Carvalho de Melo]

Em Thu, Dec 26, 2013 at 09:14:46AM -0500, David Ahern escreveu:
> On 12/26/13, 12:41 AM, Masami Hiramatsu wrote:
> >@@ -180,7 +181,20 @@ static int opt_set_target(const struct option *opt, const char *str,
> > else
> > return ret;
> >
> >- params.target = str;
> >+ /* Expand given path to absolute path, except for modulename */
> >+ if (params.uprobes || strchr(str, '/')) {
> >+ tmp = realpath(str, NULL);
> >+ if (!tmp) {
> >+ pr_warning("Failed to find the path of %s.\n",
> >+ str);
>
> That error message will be misleading if it is generated. How about:
> Failed to get the absolute path of %s: %d\n", str, errno.

Changed it to:

pr_warning("Failed to get the absolute path of %s: %m\n", str);

[David Ahern]

On 12/26/13, 12:41 AM, Masami Hiramatsu wrote:

> And this shows the available variables at the given line of
> the function.
> ----
> # ./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

Still limitations. This is Fedora 18:

# rpm -qa | grep debug
glibc-debuginfo-common-2.16-34.fc18.x86_64
glibc-debuginfo-2.16-34.fc18.x86_64

# /tmp/perf/perf probe -V malloc -x /lib64/libc-2.16.so
Failed to find variables at malloc (0)

So probing on system libraries does not benefit from this patch.

> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> index 5681266..20b3e15 100644
> --- a/tools/perf/builtin-probe.c
> +++ b/tools/perf/builtin-probe.c
> @@ -425,7 +425,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");

Unrelated change.

> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 05be5de..2f82267 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -172,6 +172,52 @@ const char *kernel_get_module_path(const char *module)
> return (dso) ? dso->long_name : NULL;
> }
>
> +/* Copied from unwind.c */
> +static Elf_Scn *elf_section_by_name(Elf *elf, GElf_Ehdr *ep,
> + GElf_Shdr *shp, const char *name)
> +{
> + Elf_Scn *sec = NULL;
> +
> + while ((sec = elf_nextscn(elf, sec)) != NULL) {
> + char *str;
> +
> + gelf_getshdr(sec, shp);
> + str = elf_strptr(elf, ep->e_shstrndx, shp->sh_name);
> + if (!strcmp(name, str))
> + break;
> + }
> +
> + return sec;
> +}

Why copy it? With unwind.c and util/symbol-elf.c we now have 2 copies.
How about exporting one of those?

More strdup's. This is library code and we need methods to free that
memory as well.

David

[Arnaldo Carvalho de Melo]

Yeah, it needs to use symbol.c functions to find the right file to get
the detached DWARF info, just like annotate, etc does.

- Arnaldo

[Masami Hiramatsu]

(2013/12/26 23:22), Arnaldo Carvalho de Melo wrote:
> Em Thu, Dec 26, 2013 at 09:14:46AM -0500, David Ahern escreveu:
>> On 12/26/13, 12:41 AM, Masami Hiramatsu wrote:
>>> @@ -180,7 +181,20 @@ static int opt_set_target(const struct option *opt, const char *str,
>>> else
>>> return ret;
>>>
>>> - params.target = str;
>>> + /* Expand given path to absolute path, except for modulename */
>>> + if (params.uprobes || strchr(str, '/')) {
>>> + tmp = realpath(str, NULL);
>>> + if (!tmp) {
>>> + pr_warning("Failed to find the path of %s.\n",
>>> + str);
>>
>> That error message will be misleading if it is generated. How about:
>> Failed to get the absolute path of %s: %d\n", str, errno.
>
> Changed it to:
>
> pr_warning("Failed to get the absolute path of %s: %m\n", str);

Thanks, that looks good to me.

>>> + return ret;
>>> + }
>>> + } else {
>>> + tmp = strdup(str);
>>> + if (!tmp)
>>> + return -ENOMEM;
>>> + }
>>> + params.target = tmp;
>>
>> When is params.target freed?

Nowhere, since there is no terminal code for user
command interface.

Those memories are released when the program terminated.
I think it is just a waste of the time to free the memory
pieces which are not used(and allocated) repeatedly.
Or, is there any chance to call this part directly from
other command?


Thank you,

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

[Masami Hiramatsu]

Yes, please see the todo list in 0/2

> - Support distro style debuginfo path (/usr/lib/debug/...)

Not only the libraries but also all distro packaged binaries have
debuginfo as other binaries. We should locate it by using dso.
But since this is just the first step series, I'd like to start with
the "homebrew" debuginfo.

>> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
>> index 5681266..20b3e15 100644
>> --- a/tools/perf/builtin-probe.c
>> +++ b/tools/perf/builtin-probe.c
>> @@ -425,7 +425,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");
>
> Unrelated change.

No, this is required for the --line dwarf support.

Hm, OK, I missed the copy in symbol-elf.c. I'd like to use that, since
the unwind.c can be disabled by NO_LIBUNWIND.

Ah, yes. Still some error handling routine doesn't clean it at all.
And it should be a separated bugfix patch, since not only
add_exec_to_probe_trace_events but also add_module_to_probe_trace_events
does the similar strdups.

Thank you!

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

[David Ahern]

On 12/27/13, 1:14 AM, Masami Hiramatsu wrote:
> Nowhere, since there is no terminal code for user
> command interface.
>
> Those memories are released when the program terminated.
> I think it is just a waste of the time to free the memory
> pieces which are not used(and allocated) repeatedly.
> Or, is there any chance to call this part directly from
> other command?

Most of the functionality has a destructor to clean up memory
allocations. probe code needs to follow suit.

e.g, from builtin-record.c:

err = __cmd_record(&record, argc, argv);

perf_evlist__munmap(evsel_list);
perf_evlist__close(evsel_list);
out_free_fd:
perf_evlist__delete_maps(evsel_list);
out_symbol_exit:
symbol__exit();

and __cmd_record ends by cleaning up the session struct.

David

[Arnaldo Carvalho de Melo]

Em Fri, Dec 27, 2013 at 09:20:11AM -0500, David Ahern escreveu:
> On 12/27/13, 1:14 AM, Masami Hiramatsu wrote:
> >Nowhere, since there is no terminal code for user
> >command interface.

> >Those memories are released when the program terminated.
> >I think it is just a waste of the time to free the memory
> >pieces which are not used(and allocated) repeatedly.
> >Or, is there any chance to call this part directly from
> >other command?

There is always the chance of parts of a tool to be librarized, I think
the rule is: allocator -> lifetime -> destructor, explicit.

We may want to explicitely disable some big destructor call (or lots of
destructors, like symbols + hists, etc) because it may make exit time to
be overly long, but at least we'll know what destructors to call when
such code gets librarized.

- Arnaldo

[Arnaldo Carvalho de Melo]

Em Fri, Dec 27, 2013 at 02:49:02PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Fri, Dec 27, 2013 at 09:20:11AM -0500, David Ahern escreveu:
> > On 12/27/13, 1:14 AM, Masami Hiramatsu wrote:
> > >Nowhere, since there is no terminal code for user
> > >command interface.
>
> > >Those memories are released when the program terminated.
> > >I think it is just a waste of the time to free the memory
> > >pieces which are not used(and allocated) repeatedly.
> > >Or, is there any chance to call this part directly from
> > >other command?
>
> There is always the chance of parts of a tool to be librarized, I think
> the rule is: allocator -> lifetime -> destructor, explicit.
>
> We may want to explicitely disable some big destructor call (or lots of
> destructors, like symbols + hists, etc) because it may make exit time to
> be overly long, but at least we'll know what destructors to call when
> such code gets librarized.

Having said that, I applied the current patch, as it doesn't makes
things much worser than they already are, i.e. we need to audit that
code further, and these patches provided an useful new functionality.

- Arnaldo

[tip-bot for Masami Hiramatsu]

Commit-ID: 8a613d40e389b723fd5889ac8d4033ed4030be31
Gitweb: http://git.kernel.org/tip/8a613d40e389b723fd5889ac8d4033ed4030be31
Author: Masami Hiramatsu <masami.hi...@hitachi.com>
AuthorDate: Thu, 26 Dec 2013 05:41:50 +0000
Committer: Arnaldo Carvalho de Melo <ac...@redhat.com>
CommitDate: Thu, 26 Dec 2013 11:21:19 -0300

perf probe: Expand given path to absolute path

Expand given path to absolute path in the option parser, except for a
module name.

Since realpath at 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.

Changes from previous one:
- Fix not to print null string.
- Allocate memory for given path/module name everytime.

Signed-off-by: Masami Hiramatsu <masami.hi...@hitachi.com>

Cc: "David A. Long" <dave...@linaro.org>
Cc: "Steven Rostedt (Red Hat)" <ros...@goodmis.org>
Cc: David Ahern <dsa...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Namhyung Kim <namh...@kernel.org>
Cc: Oleg Nesterov <ol...@redhat.com>
Cc: Srikar Dronamraju <sri...@linux.vnet.ibm.com>
Cc: syst...@sourceware.org
Cc: yrl.pp-m...@hitachi.com
Link: http://lkml.kernel.org/r/20131226054150.2...@kbuild-fedora.novalocal
[ Clarified the pr_warning message as per David Ahern's suggestion ]
Signed-off-by: Arnaldo Carvalho de Melo <ac...@redhat.com>
---
tools/perf/builtin-probe.c | 15 ++++++++++++++-
tools/perf/util/probe-event.c | 11 ++---------
2 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index c98ccb5..1792a3f 100644

--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -169,6 +169,7 @@ static int opt_set_target(const struct option *opt, const char *str,
int unset __maybe_unused)
{
int ret = -ENOENT;
+ char *tmp;

if (str && !params.target) {
if (!strcmp(opt->long_name, "exec"))

@@ -180,7 +181,19 @@ static int opt_set_target(const struct option *opt, const char *str,

else
return ret;

- params.target = str;
+ /* Expand given path to absolute path, except for modulename */
+ if (params.uprobes || strchr(str, '/')) {
+ tmp = realpath(str, NULL);
+ if (!tmp) {

+ pr_warning("Failed to get the absolute path of %s: %m\n", str);

+ return ret;
+ }
+ } else {
+ tmp = strdup(str);
+ if (!tmp)
+ return -ENOMEM;
+ }
+ params.target = tmp;
ret = 0;
}

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c

index 544ac18..68013b9 100644

[tip-bot for Masami Hiramatsu]

Commit-ID: fb7345bbf7fad9bf72ef63a19c707970b9685812
Gitweb: http://git.kernel.org/tip/fb7345bbf7fad9bf72ef63a19c707970b9685812
Author: Masami Hiramatsu <masami.hi...@hitachi.com>
AuthorDate: Thu, 26 Dec 2013 05:41:53 +0000

Committer: Arnaldo Carvalho de Melo <ac...@redhat.com>

CommitDate: Thu, 26 Dec 2013 11:22:01 -0300

perf probe: Support basic dwarf-based operations on uprobe events

Support basic 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(with local variables, line numbers) and
--line works with -x option. (Actually, --vars has already accepted -x
option)

For example, 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:

# ./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) {

And this shows the available variables at the given line of the
function.

# ./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

And lastly, we can now define probe(s) with all available
variables 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

Changes from previous version:
- Add examples in the patch description.
- Use .text section start address and dwarf symbol address
for calculating the offset of given symbol, instead of
searching the symbol in symtab again.
With this change, we can safely handle multiple local
function instances (e.g. scnprintf in perf).

Signed-off-by: Masami Hiramatsu <masami.hi...@hitachi.com>

Cc: David Ahern <dsa...@gmail.com>

Cc: David A. Long <dave...@linaro.org>

Cc: Ingo Molnar <mi...@kernel.org>
Cc: Namhyung Kim <namh...@kernel.org>
Cc: Oleg Nesterov <ol...@redhat.com>
Cc: Srikar Dronamraju <sri...@linux.vnet.ibm.com>

Cc: Steven Rostedt <ros...@goodmis.org>
Cc: syst...@sourceware.org
Cc: yrl.pp-m...@hitachi.com
Link: http://lkml.kernel.org/r/20131226054152.2...@kbuild-fedora.novalocal

Signed-off-by: Arnaldo Carvalho de Melo <ac...@redhat.com>
---

tools/perf/builtin-probe.c | 2 +-
tools/perf/util/probe-event.c | 151 ++++++++++++++++++++++++++++++++++++-----
tools/perf/util/probe-event.h | 1 +
tools/perf/util/probe-finder.c | 1 +

4 files changed, 138 insertions(+), 17 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c

index 1792a3f..43ff33d 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -424,7 +424,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 68013b9..72b56ae 100644