[PATCH perf/core v2 0/5] perf-probe: improve glibc support

[PATCH perf/core v2 0/5] perf-probe: improve glibc support

[Masami Hiramatsu]

Hi,

Here is a series of patches which improves perf-probe to
handle glibc's aliased symbols and weak symbols more
correctly.

This version includes 2 new patches from Namhyung (Thanks!)
which solves a problem on weak symbols. I added a fix
on his latter patch to modify find_alternative_probe_point,
and dropped a bugfix which is already merged.
So, this series is a merged series of below 2 series.

http://lkml.kernel.org/g/20150302124939.9191.33564.stgit@localhost.localdomain
http://lkml.kernel.org/g/1425477143-5310-1-git-send-email-namhyung@kernel.org

======
A major known issue of probing on glibc is that the
some aliased symbols(e.g. malloc) and weak symbols
(e.g. calloc) can not find by perf-probe.

Actually, glibc's malloc symbol is just an alias of
__libc_malloc. Its debuginfo knows only __libc_malloc,
and perf's symbol map knows only malloc. This difference
always confuses users that they can see malloc by perf
report or annotate, but they can not probe on it, nor
find definitions by --line option.

And weak symbols have been dropped when loading.

Previously, I've made a commit 906451b98b67 which solved
this problem partly, but not completely fixed.
So I decided to solve this issue completely by finding
the symbols like malloc from perf's symbol map, and 
converting the symbol's address into debuginfo's
location infomation.

With this series, you can use --vars, --line and --add
with the aliased symbols and weak symbols on glibc.

Thank you,


---

Masami Hiramatsu (3):
      perf-probe: Fix to handle aliased symbols in glibc
      perf-probe: Fix --line to handle aliased symbols in glibc
      Revert "perf probe: Fix to fall back to find probe point in symbols"

Namhyung Kim (2):
      perf symbols: Allow symbol alias when loading map for symbol name
      perf probe: Allow weak symbols to be probed


 tools/perf/util/machine.c        |    2 
 tools/perf/util/map.c            |    6 +
 tools/perf/util/map.h            |    8 +-
 tools/perf/util/probe-event.c    |  185 +++++++++++++++++++++++++++++++++-----
 tools/perf/util/symbol-elf.c     |    5 +
 tools/perf/util/symbol-minimal.c |    2 
 tools/perf/util/symbol.c         |    8 +-
 tools/perf/util/symbol.h         |    5 +
 8 files changed, 182 insertions(+), 39 deletions(-)

--
Masami HIRAMATSU
Software Platform Research Dpt. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

[PATCH perf/core v2 1/5] perf-probe: Fix to handle aliased symbols in glibc

[Masami Hiramatsu]

Fix perf probe to handle aliased symbols correctly in glibc.
In the glibc, several symbols are defined as an alias of
__libc_XXX, e.g. malloc is an alias of __libc_malloc.
In such cases, dwarf has no subroutine instances of the
alias functions (e.g. no "malloc" instance), but the map
has that symbol and its address.
Thus, if we search the alieased symbol in debuginfo, we
always fail to find it, but it is in the map.

To solve this problem, this fails back to address-based
alternative search, which searches the symbol in the map,
translates its address to alternative (correct) function
name by using debuginfo, and retry to find the alternative
function point from debuginfo.

This adds fail-back process to --vars, --lines and --add
options. So, now you can use those on malloc@libc :)

Without this patch;
  -----
  # ./perf probe -x /usr/lib64/libc-2.17.so -V malloc
  Failed to find the address of malloc
    Error: Failed to show vars.
  # ./perf probe -x /usr/lib64/libc-2.17.so -a "malloc bytes"
  Probe point 'malloc' not found in debuginfo.
    Error: Failed to add events.
  -----

With this patch;
  -----
  # ./perf probe -x /usr/lib64/libc-2.17.so -V malloc
  Available variables at malloc
          @<__libc_malloc+0>
                  size_t  bytes
  # ./perf probe -x /usr/lib64/libc-2.17.so -a "malloc bytes"
  Added new event:
    probe_libc:malloc    (on malloc in /usr/lib64/libc-2.17.so with bytes)

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

          perf record -e probe_libc:malloc -aR sleep 1
  -----

Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 tools/perf/util/probe-event.c |  140 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 124 insertions(+), 16 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 1c570c2fa7..b8f4578 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -178,6 +178,25 @@ static struct map *kernel_get_module_map(const char *module)
 	return NULL;
 }
 
+static struct map *get_target_map(const char *target, bool user)
+{
+	/* Init maps of given executable or kernel */
+	if (user)
+		return dso__new_map(target);
+	else
+		return kernel_get_module_map(target);
+}
+
+static void put_target_map(struct map *map, bool user)
+{
+	if (map && user) {
+		/* Only the user map needs to be released */
+		dso__delete(map->dso);
+		map__delete(map);
+	}
+}
+
+
 static struct dso *kernel_get_module_dso(const char *module)
 {
 	struct dso *dso;
@@ -249,6 +268,13 @@ out:
 	return ret;
 }
 
+static void clear_perf_probe_point(struct perf_probe_point *pp)
+{
+	free(pp->file);
+	free(pp->function);
+	free(pp->lazy_line);
+}
+
 static void clear_probe_trace_events(struct probe_trace_event *tevs, int ntevs)
 {
 	int i;
@@ -258,6 +284,74 @@ static void clear_probe_trace_events(struct probe_trace_event *tevs, int ntevs)
 }
 
 #ifdef HAVE_DWARF_SUPPORT
+/*
+ * Some binaries like glibc have special symbols which are on the symbol
+ * table, but not in the debuginfo. If we can find the address of the
+ * symbol from map, we can translate the address back to the probe point.
+ */
+static int find_alternative_probe_point(struct debuginfo *dinfo,
+					struct perf_probe_point *pp,
+					struct perf_probe_point *result,
+					const char *target, bool uprobes)
+{
+	struct map *map = NULL;
+	struct symbol *sym;
+	u64 address = 0;
+	int ret = -ENOENT;
+
+	/* This can work only for function-name based one */
+	if (!pp->function || pp->file)
+		return -ENOTSUP;
+
+	map = get_target_map(target, uprobes);
+	if (!map)
+		return -EINVAL;
+
+	/* Find the address of given function */
+	map__for_each_symbol_by_name(map, pp->function, sym) {
+		if (sym->binding == STB_GLOBAL || sym->binding == STB_LOCAL) {
+			address = sym->start;
+			break;
+		}
+	}
+	if (!address) {
+		ret = -ENOENT;
+		goto out;
+	}
+	pr_debug("Symbol %s address found : %lx\n", pp->function, address);
+
+	ret = debuginfo__find_probe_point(dinfo, (unsigned long)address,
+					  result);
+	if (ret <= 0)
+		ret = (!ret) ? -ENOENT : ret;
+	else {
+		result->offset += pp->offset;
+		result->line += pp->line;
+		ret = 0;
+	}
+
+out:
+	put_target_map(map, uprobes);
+	return ret;
+
+}
+
+static int get_alternative_probe_event(struct debuginfo *dinfo,
+				       struct perf_probe_event *pev,
+				       struct perf_probe_point *tmp,
+				       const char *target)
+{
+	int ret;
+
+	memcpy(tmp, &pev->point, sizeof(*tmp));
+	memset(&pev->point, 0, sizeof(pev->point));
+	ret = find_alternative_probe_point(dinfo, tmp, &pev->point,
+					   target, pev->uprobes);
+	if (ret < 0)
+		memcpy(&pev->point, tmp, sizeof(*tmp));
+
+	return ret;
+}
 
 /* Open new debuginfo of given module */
 static struct debuginfo *open_debuginfo(const char *module, bool silent)
@@ -466,6 +560,7 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
 					  int max_tevs, const char *target)
 {
 	bool need_dwarf = perf_probe_event_need_dwarf(pev);
+	struct perf_probe_point tmp;
 	struct debuginfo *dinfo;
 	int ntevs, ret = 0;
 
@@ -482,6 +577,20 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
 	/* Searching trace events corresponding to a probe event */
 	ntevs = debuginfo__find_trace_events(dinfo, pev, tevs, max_tevs);
 
+	if (ntevs == 0)	{  /* Not found, retry with an alternative */
+		ret = get_alternative_probe_event(dinfo, pev, &tmp, target);
+		if (!ret) {
+			ntevs = debuginfo__find_trace_events(dinfo, pev,
+							     tevs, max_tevs);
+			/*
+			 * Write back to the original probe_event for
+			 * setting appropriate (user given) event name
+			 */
+			clear_perf_probe_point(&pev->point);
+			memcpy(&pev->point, &tmp, sizeof(tmp));
+		}
+	}
+
 	debuginfo__delete(dinfo);
 
 	if (ntevs > 0) {	/* Succeeded to find trace events */
@@ -719,12 +828,13 @@ int show_line_range(struct line_range *lr, const char *module, bool user)
 static int show_available_vars_at(struct debuginfo *dinfo,
 				  struct perf_probe_event *pev,
 				  int max_vls, struct strfilter *_filter,
-				  bool externs)
+				  bool externs, const char *target)
 {
 	char *buf;
 	int ret, i, nvars;
 	struct str_node *node;
 	struct variable_list *vls = NULL, *vl;
+	struct perf_probe_point tmp;
 	const char *var;
 
 	buf = synthesize_perf_probe_point(&pev->point);
@@ -734,6 +844,15 @@ static int show_available_vars_at(struct debuginfo *dinfo,
 
 	ret = debuginfo__find_available_vars_at(dinfo, pev, &vls,
 						max_vls, externs);
+	if (!ret) {  /* Not found, retry with an alternative */
+		ret = get_alternative_probe_event(dinfo, pev, &tmp, target);
+		if (!ret) {
+			ret = debuginfo__find_available_vars_at(dinfo, pev,
+						&vls, max_vls, externs);
+			/* Release the old probe_point */
+			clear_perf_probe_point(&tmp);
+		}
+	}
 	if (ret <= 0) {
 		if (ret == 0 || ret == -ENOENT) {
 			pr_err("Failed to find the address of %s\n", buf);
@@ -796,7 +915,7 @@ int show_available_vars(struct perf_probe_event *pevs, int npevs,
 
 	for (i = 0; i < npevs && ret >= 0; i++)
 		ret = show_available_vars_at(dinfo, &pevs[i], max_vls, _filter,
-					     externs);
+					     externs, module);
 
 	debuginfo__delete(dinfo);
 out:
@@ -1742,15 +1861,12 @@ static int convert_to_perf_probe_event(struct probe_trace_event *tev,
 
 void clear_perf_probe_event(struct perf_probe_event *pev)
 {
-	struct perf_probe_point *pp = &pev->point;
 	struct perf_probe_arg_field *field, *next;
 	int i;
 
 	free(pev->event);
 	free(pev->group);
-	free(pp->file);
-	free(pp->function);
-	free(pp->lazy_line);
+	clear_perf_probe_point(&pev->point);
 
 	for (i = 0; i < pev->nargs; i++) {
 		free(pev->args[i].name);
@@ -2367,11 +2483,7 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
 	int num_matched_functions;
 	int ret, i;
 
-	/* Init maps of given executable or kernel */
-	if (pev->uprobes)
-		map = dso__new_map(target);
-	else
-		map = kernel_get_module_map(target);
+	map = get_target_map(target, pev->uprobes);
 	if (!map) {
 		ret = -EINVAL;
 		goto out;
@@ -2464,11 +2576,7 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
 	}
 
 out:
-	if (map && pev->uprobes) {
-		/* Only when using uprobe(exec) map needs to be released */
-		dso__delete(map->dso);
-		map__delete(map);
-	}
+	put_target_map(map, pev->uprobes);
 	return ret;
 
 nomem_out:

Re: [PATCH perf/core v2 1/5] perf-probe: Fix to handle aliased symbols in glibc

[Arnaldo Carvalho de Melo]

Em Fri, Mar 06, 2015 at 04:31:20PM +0900, Masami Hiramatsu escreveu:
> Fix perf probe to handle aliased symbols correctly in glibc.
> In the glibc, several symbols are defined as an alias of
> __libc_XXX, e.g. malloc is an alias of __libc_malloc.
> In such cases, dwarf has no subroutine instances of the
> alias functions (e.g. no "malloc" instance), but the map
> has that symbol and its address.
> Thus, if we search the alieased symbol in debuginfo, we
> always fail to find it, but it is in the map.
> 
> To solve this problem, this fails back to address-based
> alternative search, which searches the symbol in the map,
> translates its address to alternative (correct) function
> name by using debuginfo, and retry to find the alternative
> function point from debuginfo.
> 
> This adds fail-back process to --vars, --lines and --add
> options. So, now you can use those on malloc@libc :)

--vars and --add works, but not --lines:

[root@ssdandy ~]# perf probe -x /usr/lib64/libc-2.17.so -V malloc
Available variables at malloc
        @<__libc_malloc+96>
                size_t  bytes

[root@ssdandy ~]# perf probe -x /usr/lib64/libc-2.17.so -L malloc
Specified source line is not found.
  Error: Failed to show lines.
[root@ssdandy ~]# 

[root@ssdandy ~]# perf probe -x /usr/lib64/libc-2.17.so -a "malloc
bytes"
Added new event:
  probe_libc:malloc    (on malloc in /usr/lib64/libc-2.17.so with bytes)

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

	perf record -e probe_libc:malloc -aR sleep 1

[root@ssdandy ~]#

[root@ssdandy ~]# cat /t/events/probe_libc/malloc/format 
name: malloc
ID: 1921
format:
	field:unsigned short common_type;	offset:0;	size:2;	signed:0;
	field:unsigned char common_flags;	offset:2;	size:1;	signed:0;
	field:unsigned char common_preempt_count;	offset:3;	size:1;	signed:0;
	field:int common_pid;	offset:4;	size:4;	signed:1;

	field:unsigned long __probe_ip;	offset:8;	size:8;	signed:0;
	field:u64 bytes;	offset:16;	size:8;	signed:0;

print fmt: "(%lx) bytes=0x%Lx", REC->__probe_ip, REC->bytes
[root@ssdandy ~]# 

Works for the aliased symbol, anyway, applying as it makes progress.

[root@ssdandy ~]# perf probe -x /usr/lib64/libc-2.17.so -L __libc_malloc
<__libc_malloc@/usr/src/debug/glibc-2.17-c758a686/malloc/malloc.c:0>
      0  __libc_malloc(size_t bytes)
      1  {
           mstate ar_ptr;
           void *victim;
         
           __malloc_ptr_t (*hook) (size_t, const __malloc_ptr_t)
      6      = force_reg (__malloc_hook);
      7    if (__builtin_expect (hook != NULL, 0))
      8      return (*hook)(bytes, RETURN_ADDRESS (0));
         
     10    arena_lookup(ar_ptr);
         
     12    arena_lock(ar_ptr, bytes);
     13    if(!ar_ptr)
     14      return 0;
     15    victim = _int_malloc(ar_ptr, bytes);
     16    if(!victim) {
     17      LIBC_PROBE (memory_malloc_retry, 1, bytes);
     18      ar_ptr = arena_get_retry(ar_ptr, bytes);
     19      if (__builtin_expect(ar_ptr != NULL, 1)) {
     20        victim = _int_malloc(ar_ptr, bytes);
     21        (void)mutex_unlock(&ar_ptr->mutex);
             }
           } else
     24      (void)mutex_unlock(&ar_ptr->mutex);
           assert(!victim || chunk_is_mmapped(mem2chunk(victim)) ||
                 ar_ptr == arena_for_chunk(mem2chunk(victim)));
           return victim;
     28  }
         libc_hidden_def(__libc_malloc)
         
         void

[root@ssdandy ~]#

Re: [PATCH perf/core v2 1/5] perf-probe: Fix to handle aliased symbols in glibc

[Arnaldo Carvalho de Melo]

Em Fri, Mar 06, 2015 at 02:59:19PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Fri, Mar 06, 2015 at 04:31:20PM +0900, Masami Hiramatsu escreveu:
> > Fix perf probe to handle aliased symbols correctly in glibc.
> > In the glibc, several symbols are defined as an alias of
> > __libc_XXX, e.g. malloc is an alias of __libc_malloc.
> > In such cases, dwarf has no subroutine instances of the
> > alias functions (e.g. no "malloc" instance), but the map
> > has that symbol and its address.
> > Thus, if we search the alieased symbol in debuginfo, we
> > always fail to find it, but it is in the map.
> > 
> > To solve this problem, this fails back to address-based
> > alternative search, which searches the symbol in the map,
> > translates its address to alternative (correct) function
> > name by using debuginfo, and retry to find the alternative
> > function point from debuginfo.
> > 
> > This adds fail-back process to --vars, --lines and --add
> > options. So, now you can use those on malloc@libc :)
> 
> --vars and --add works, but not --lines:

Ok, it works after applying 2/5, so it was just me confused by the above
statement, that a fallback was added that would make --lines work, it
works, but not with this patch, the next one is needed.

Anyway, works now, I'm happy, thanks!

- Arnaldo
 
> [root@ssdandy ~]# perf probe -x /usr/lib64/libc-2.17.so -V malloc
> Available variables at malloc
>         @<__libc_malloc+96>
>                 size_t  bytes
> 
> [root@ssdandy ~]# perf probe -x /usr/lib64/libc-2.17.so -L malloc
> Specified source line is not found.
>   Error: Failed to show lines.
> [root@ssdandy ~]# 
> 
> [root@ssdandy ~]# perf probe -x /usr/lib64/libc-2.17.so -a "malloc
> bytes"
> Added new event:
>   probe_libc:malloc    (on malloc in /usr/lib64/libc-2.17.so with bytes)
> 
> You can now use it in all perf tools, such as:
> 
> 	perf record -e probe_libc:malloc -aR sleep 1
> 
> [root@ssdandy ~]#
> 
> [root@ssdandy ~]# cat /t/events/probe_libc/malloc/format 
> name: malloc
> ID: 1921
> format:
> 	field:unsigned short common_type;	offset:0;	size:2;	signed:0;
> 	field:unsigned char common_flags;	offset:2;	size:1;	signed:0;
> 	field:unsigned char common_preempt_count;	offset:3;	size:1;	signed:0;
> 	field:int common_pid;	offset:4;	size:4;	signed:1;
> 
> 	field:unsigned long __probe_ip;	offset:8;	size:8;	signed:0;
> 	field:u64 bytes;	offset:16;	size:8;	signed:0;
> 
> print fmt: "(%lx) bytes=0x%Lx", REC->__probe_ip, REC->bytes
> [root@ssdandy ~]# 
> 
> Works for the aliased symbol, anyway, applying as it makes progress.
> 
> [root@ssdandy ~]# perf probe -x /usr/lib64/libc-2.17.so -L __libc_malloc
> <__libc_malloc@/usr/src/debug/glibc-2.17-c758a686/malloc/malloc.c:0>
>       0  __libc_malloc(size_t bytes)
>       1  {
>            mstate ar_ptr;
>            void *victim;
>          
>            __malloc_ptr_t (*hook) (size_t, const __malloc_ptr_t)
>       6      = force_reg (__malloc_hook);
>       7    if (__builtin_expect (hook != NULL, 0))
>       8      return (*hook)(bytes, RETURN_ADDRESS (0));
>          
>      10    arena_lookup(ar_ptr);
>          
>      12    arena_lock(ar_ptr, bytes);
>      13    if(!ar_ptr)
>      14      return 0;
>      15    victim = _int_malloc(ar_ptr, bytes);
>      16    if(!victim) {
>      17      LIBC_PROBE (memory_malloc_retry, 1, bytes);
>      18      ar_ptr = arena_get_retry(ar_ptr, bytes);
>      19      if (__builtin_expect(ar_ptr != NULL, 1)) {
>      20        victim = _int_malloc(ar_ptr, bytes);
>      21        (void)mutex_unlock(&ar_ptr->mutex);
>              }
>            } else
>      24      (void)mutex_unlock(&ar_ptr->mutex);
>            assert(!victim || chunk_is_mmapped(mem2chunk(victim)) ||
>                  ar_ptr == arena_for_chunk(mem2chunk(victim)));
>            return victim;
>      28  }
>          libc_hidden_def(__libc_malloc)
>          
>          void
> 
> [root@ssdandy ~]#

[tip:perf/core] perf probe: Fix to handle aliased symbols in glibc

[tip-bot for Masami Hiramatsu]

Commit-ID:  9b118acae310f57baee770b5db402500d8695e50
Gitweb:     http://git.kernel.org/tip/9b118acae310f57baee770b5db402500d8695e50
Author:     Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
AuthorDate: Fri, 6 Mar 2015 16:31:20 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 12 Mar 2015 12:39:52 -0300

perf probe: Fix to handle aliased symbols in glibc

Fix perf probe to handle aliased symbols correctly in glibc.  In the
glibc, several symbols are defined as an alias of __libc_XXX, e.g.
malloc is an alias of __libc_malloc.

In such cases, dwarf has no subroutine instances of the alias functions
(e.g. no "malloc" instance), but the map has that symbol and its
address.

Thus, if we search the alieased symbol in debuginfo, we always fail to
find it, but it is in the map.

To solve this problem, this fails back to address-based alternative
search, which searches the symbol in the map, translates its address to
alternative (correct) function name by using debuginfo, and retry to
find the alternative function point from debuginfo.

This adds fail-back process to --vars, --lines and --add options. So,
now you can use those on malloc@libc :)

Without this patch;
  -----
  # ./perf probe -x /usr/lib64/libc-2.17.so -V malloc
  Failed to find the address of malloc
    Error: Failed to show vars.
  # ./perf probe -x /usr/lib64/libc-2.17.so -a "malloc bytes"
  Probe point 'malloc' not found in debuginfo.
    Error: Failed to add events.
  -----

With this patch;
  -----
  # ./perf probe -x /usr/lib64/libc-2.17.so -V malloc
  Available variables at malloc
          @<__libc_malloc+0>
                  size_t  bytes
  # ./perf probe -x /usr/lib64/libc-2.17.so -a "malloc bytes"
  Added new event:
    probe_libc:malloc    (on malloc in /usr/lib64/libc-2.17.so with bytes)

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

          perf record -e probe_libc:malloc -aR sleep 1
  -----

Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Naohiro Aota <naota@elisp.net>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20150306073120.6904.13779.stgit@localhost.localdomain
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-event.c | 140 +++++++++++++++++++++++++++++++++++++-----
 1 file changed, 124 insertions(+), 16 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 1c570c2..b8f4578 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -178,6 +178,25 @@ static struct map *kernel_get_module_map(const char *module)
 	return NULL;
 }
 
+static struct map *get_target_map(const char *target, bool user)
+{
+	/* Init maps of given executable or kernel */
+	if (user)
+		return dso__new_map(target);
+	else
+		return kernel_get_module_map(target);
+}
+
+static void put_target_map(struct map *map, bool user)
+{
+	if (map && user) {
+		/* Only the user map needs to be released */
+		dso__delete(map->dso);
+		map__delete(map);
+	}
+}
+
+
 static struct dso *kernel_get_module_dso(const char *module)
 {
 	struct dso *dso;
@@ -249,6 +268,13 @@ out:
 	return ret;
 }
 
+static void clear_perf_probe_point(struct perf_probe_point *pp)
+{
+	free(pp->file);
+	free(pp->function);
+	free(pp->lazy_line);
+}
+
 static void clear_probe_trace_events(struct probe_trace_event *tevs, int ntevs)
 {
 	int i;
@@ -258,6 +284,74 @@ static void clear_probe_trace_events(struct probe_trace_event *tevs, int ntevs)
 }
 
 #ifdef HAVE_DWARF_SUPPORT
+/*
+ * Some binaries like glibc have special symbols which are on the symbol
+ * table, but not in the debuginfo. If we can find the address of the
+ * symbol from map, we can translate the address back to the probe point.
+ */
+static int find_alternative_probe_point(struct debuginfo *dinfo,
+					struct perf_probe_point *pp,
+					struct perf_probe_point *result,
+					const char *target, bool uprobes)
+{
+	struct map *map = NULL;
+	struct symbol *sym;
+	u64 address = 0;
+	int ret = -ENOENT;
+
+	/* This can work only for function-name based one */
+	if (!pp->function || pp->file)
+		return -ENOTSUP;
+
+	map = get_target_map(target, uprobes);
+	if (!map)
+		return -EINVAL;
+
+	/* Find the address of given function */
+	map__for_each_symbol_by_name(map, pp->function, sym) {
+		if (sym->binding == STB_GLOBAL || sym->binding == STB_LOCAL) {
+			address = sym->start;
+			break;
+		}
+	}
+	if (!address) {
+		ret = -ENOENT;
+		goto out;
+	}
+	pr_debug("Symbol %s address found : %lx\n", pp->function, address);
+
+	ret = debuginfo__find_probe_point(dinfo, (unsigned long)address,
+					  result);
+	if (ret <= 0)
+		ret = (!ret) ? -ENOENT : ret;
+	else {
+		result->offset += pp->offset;
+		result->line += pp->line;
+		ret = 0;
+	}
+
+out:
+	put_target_map(map, uprobes);
+	return ret;
+
+}
+
+static int get_alternative_probe_event(struct debuginfo *dinfo,
+				       struct perf_probe_event *pev,
+				       struct perf_probe_point *tmp,
+				       const char *target)
+{
+	int ret;
+
+	memcpy(tmp, &pev->point, sizeof(*tmp));
+	memset(&pev->point, 0, sizeof(pev->point));
+	ret = find_alternative_probe_point(dinfo, tmp, &pev->point,
+					   target, pev->uprobes);
+	if (ret < 0)
+		memcpy(&pev->point, tmp, sizeof(*tmp));
+
+	return ret;
+}
 
 /* Open new debuginfo of given module */
 static struct debuginfo *open_debuginfo(const char *module, bool silent)
@@ -466,6 +560,7 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
 					  int max_tevs, const char *target)
 {
 	bool need_dwarf = perf_probe_event_need_dwarf(pev);
+	struct perf_probe_point tmp;
 	struct debuginfo *dinfo;
 	int ntevs, ret = 0;
 
@@ -482,6 +577,20 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
 	/* Searching trace events corresponding to a probe event */
 	ntevs = debuginfo__find_trace_events(dinfo, pev, tevs, max_tevs);
 
+	if (ntevs == 0)	{  /* Not found, retry with an alternative */
+		ret = get_alternative_probe_event(dinfo, pev, &tmp, target);
+		if (!ret) {
+			ntevs = debuginfo__find_trace_events(dinfo, pev,
+							     tevs, max_tevs);
+			/*
+			 * Write back to the original probe_event for
+			 * setting appropriate (user given) event name
+			 */
+			clear_perf_probe_point(&pev->point);
+			memcpy(&pev->point, &tmp, sizeof(tmp));
+		}
+	}
+
 	debuginfo__delete(dinfo);
 
 	if (ntevs > 0) {	/* Succeeded to find trace events */
@@ -719,12 +828,13 @@ int show_line_range(struct line_range *lr, const char *module, bool user)
 static int show_available_vars_at(struct debuginfo *dinfo,
 				  struct perf_probe_event *pev,
 				  int max_vls, struct strfilter *_filter,
-				  bool externs)
+				  bool externs, const char *target)
 {
 	char *buf;
 	int ret, i, nvars;
 	struct str_node *node;
 	struct variable_list *vls = NULL, *vl;
+	struct perf_probe_point tmp;
 	const char *var;
 
 	buf = synthesize_perf_probe_point(&pev->point);
@@ -734,6 +844,15 @@ static int show_available_vars_at(struct debuginfo *dinfo,
 
 	ret = debuginfo__find_available_vars_at(dinfo, pev, &vls,
 						max_vls, externs);
+	if (!ret) {  /* Not found, retry with an alternative */
+		ret = get_alternative_probe_event(dinfo, pev, &tmp, target);
+		if (!ret) {
+			ret = debuginfo__find_available_vars_at(dinfo, pev,
+						&vls, max_vls, externs);
+			/* Release the old probe_point */
+			clear_perf_probe_point(&tmp);
+		}
+	}
 	if (ret <= 0) {
 		if (ret == 0 || ret == -ENOENT) {
 			pr_err("Failed to find the address of %s\n", buf);
@@ -796,7 +915,7 @@ int show_available_vars(struct perf_probe_event *pevs, int npevs,
 
 	for (i = 0; i < npevs && ret >= 0; i++)
 		ret = show_available_vars_at(dinfo, &pevs[i], max_vls, _filter,
-					     externs);
+					     externs, module);
 
 	debuginfo__delete(dinfo);
 out:
@@ -1742,15 +1861,12 @@ static int convert_to_perf_probe_event(struct probe_trace_event *tev,
 
 void clear_perf_probe_event(struct perf_probe_event *pev)
 {
-	struct perf_probe_point *pp = &pev->point;
 	struct perf_probe_arg_field *field, *next;
 	int i;
 
 	free(pev->event);
 	free(pev->group);
-	free(pp->file);
-	free(pp->function);
-	free(pp->lazy_line);
+	clear_perf_probe_point(&pev->point);
 
 	for (i = 0; i < pev->nargs; i++) {
 		free(pev->args[i].name);
@@ -2367,11 +2483,7 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
 	int num_matched_functions;
 	int ret, i;
 
-	/* Init maps of given executable or kernel */
-	if (pev->uprobes)
-		map = dso__new_map(target);
-	else
-		map = kernel_get_module_map(target);
+	map = get_target_map(target, pev->uprobes);
 	if (!map) {
 		ret = -EINVAL;
 		goto out;
@@ -2464,11 +2576,7 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
 	}
 
 out:
-	if (map && pev->uprobes) {
-		/* Only when using uprobe(exec) map needs to be released */
-		dso__delete(map->dso);
-		map__delete(map);
-	}
+	put_target_map(map, pev->uprobes);
 	return ret;
 
 nomem_out:

[PATCH perf/core v2 2/5] perf-probe: Fix --line to handle aliased symbols in glibc

[Masami Hiramatsu]

Fix perf probe --line to handle aliased symbols correctly
in glibc.

This makes line_range search failing back to address-based
alternative search as same as --add and --vars.

Without this patch;
  -----
  # ./perf probe -x /usr/lib64/libc-2.17.so -L malloc
  Specified source line is not found.
    Error: Failed to show lines.
  -----

With this patch;
  -----
  # ./perf probe -x /usr/lib64/libc-2.17.so -L malloc
  <__libc_malloc@/usr/src/debug/glibc-2.17-c758a686/malloc/malloc.c:0>
        0  __libc_malloc(size_t bytes)
        1  {
             mstate ar_ptr;
             void *victim;

             __malloc_ptr_t (*hook) (size_t, const __malloc_ptr_t)
        6      = force_reg (__malloc_hook);
        7    if (__builtin_expect (hook != NULL, 0))
        8      return (*hook)(bytes, RETURN_ADDRESS (0));

       10    arena_lookup(ar_ptr);

       12    arena_lock(ar_ptr, bytes);
  -----

Note that this actually shows __libc_malloc, since it is
the real instance of malloc. User can use both __libc_malloc
and malloc for --line.

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

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index b8f4578..4cfd121 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -353,6 +353,31 @@ static int get_alternative_probe_event(struct debuginfo *dinfo,
 	return ret;
 }
 
+static int get_alternative_line_range(struct debuginfo *dinfo,
+				      struct line_range *lr,
+				      const char *target, bool user)
+{
+	struct perf_probe_point pp = { 0 }, result = { 0 };
+	int ret, len = 0;
+
+	pp.function = lr->function;
+	pp.file = lr->file;
+	pp.line = lr->start;
+	if (lr->end != INT_MAX)
+		len = lr->end - lr->start;
+	ret = find_alternative_probe_point(dinfo, &pp, &result,
+					   target, user);
+	if (!ret) {
+		lr->function = result.function;
+		lr->file = result.file;
+		lr->start = result.line;
+		if (lr->end != INT_MAX)
+			lr->end = lr->start + len;
+		clear_perf_probe_point(&pp);
+	}
+	return ret;
+}
+
 /* Open new debuginfo of given module */
 static struct debuginfo *open_debuginfo(const char *module, bool silent)
 {
@@ -734,7 +759,8 @@ static int _show_one_line(FILE *fp, int l, bool skip, bool show_num)
  * Show line-range always requires debuginfo to find source file and
  * line number.
  */
-static int __show_line_range(struct line_range *lr, const char *module)
+static int __show_line_range(struct line_range *lr, const char *module,
+			     bool user)
 {
 	int l = 1;
 	struct int_node *ln;
@@ -750,6 +776,11 @@ static int __show_line_range(struct line_range *lr, const char *module)
 		return -ENOENT;
 
 	ret = debuginfo__find_line_range(dinfo, lr);
+	if (!ret) {	/* Not found, retry with an alternative */
+		ret = get_alternative_line_range(dinfo, lr, module, user);
+		if (!ret)
+			ret = debuginfo__find_line_range(dinfo, lr);
+	}
 	debuginfo__delete(dinfo);
 	if (ret == 0 || ret == -ENOENT) {
 		pr_warning("Specified source line is not found.\n");
@@ -819,7 +850,7 @@ int show_line_range(struct line_range *lr, const char *module, bool user)
 	ret = init_symbol_maps(user);
 	if (ret < 0)
 		return ret;
-	ret = __show_line_range(lr, module);
+	ret = __show_line_range(lr, module, user);
 	exit_symbol_maps();
 
 	return ret;

[tip:perf/core] perf probe: Fix --line to handle aliased symbols in glibc

[tip-bot for Masami Hiramatsu]

Commit-ID:  811dd2ae7cd670fefbb3b220b529bb9876edde70
Gitweb:     http://git.kernel.org/tip/811dd2ae7cd670fefbb3b220b529bb9876edde70
Author:     Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
AuthorDate: Fri, 6 Mar 2015 16:31:22 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 12 Mar 2015 12:39:53 -0300

perf probe: Fix --line to handle aliased symbols in glibc

Fix perf probe --line to handle aliased symbols correctly in glibc.

This makes line_range search failing back to address-based alternative
search as same as --add and --vars.

Without this patch;
  -----
  # ./perf probe -x /usr/lib64/libc-2.17.so -L malloc
  Specified source line is not found.
    Error: Failed to show lines.
  -----

With this patch;
  -----
  # ./perf probe -x /usr/lib64/libc-2.17.so -L malloc
  <__libc_malloc@/usr/src/debug/glibc-2.17-c758a686/malloc/malloc.c:0>
        0  __libc_malloc(size_t bytes)
        1  {
             mstate ar_ptr;
             void *victim;

             __malloc_ptr_t (*hook) (size_t, const __malloc_ptr_t)
        6      = force_reg (__malloc_hook);
        7    if (__builtin_expect (hook != NULL, 0))
        8      return (*hook)(bytes, RETURN_ADDRESS (0));

       10    arena_lookup(ar_ptr);

       12    arena_lock(ar_ptr, bytes);
  -----

Note that this actually shows __libc_malloc, since it is the real
instance of malloc. User can use both __libc_malloc and malloc for
--line.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Naohiro Aota <naota@elisp.net>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20150306073122.6904.18540.stgit@localhost.localdomain
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-event.c | 35 +++++++++++++++++++++++++++++++++--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index b8f4578..4cfd121 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -353,6 +353,31 @@ static int get_alternative_probe_event(struct debuginfo *dinfo,
 	return ret;
 }
 
+static int get_alternative_line_range(struct debuginfo *dinfo,
+				      struct line_range *lr,
+				      const char *target, bool user)
+{
+	struct perf_probe_point pp = { 0 }, result = { 0 };
+	int ret, len = 0;
+
+	pp.function = lr->function;
+	pp.file = lr->file;
+	pp.line = lr->start;
+	if (lr->end != INT_MAX)
+		len = lr->end - lr->start;
+	ret = find_alternative_probe_point(dinfo, &pp, &result,
+					   target, user);
+	if (!ret) {
+		lr->function = result.function;
+		lr->file = result.file;
+		lr->start = result.line;
+		if (lr->end != INT_MAX)
+			lr->end = lr->start + len;
+		clear_perf_probe_point(&pp);
+	}
+	return ret;
+}
+
 /* Open new debuginfo of given module */
 static struct debuginfo *open_debuginfo(const char *module, bool silent)
 {
@@ -734,7 +759,8 @@ static int _show_one_line(FILE *fp, int l, bool skip, bool show_num)
  * Show line-range always requires debuginfo to find source file and
  * line number.
  */
-static int __show_line_range(struct line_range *lr, const char *module)
+static int __show_line_range(struct line_range *lr, const char *module,
+			     bool user)
 {
 	int l = 1;
 	struct int_node *ln;
@@ -750,6 +776,11 @@ static int __show_line_range(struct line_range *lr, const char *module)
 		return -ENOENT;
 
 	ret = debuginfo__find_line_range(dinfo, lr);
+	if (!ret) {	/* Not found, retry with an alternative */
+		ret = get_alternative_line_range(dinfo, lr, module, user);
+		if (!ret)
+			ret = debuginfo__find_line_range(dinfo, lr);
+	}
 	debuginfo__delete(dinfo);
 	if (ret == 0 || ret == -ENOENT) {
 		pr_warning("Specified source line is not found.\n");
@@ -819,7 +850,7 @@ int show_line_range(struct line_range *lr, const char *module, bool user)
 	ret = init_symbol_maps(user);
 	if (ret < 0)
 		return ret;
-	ret = __show_line_range(lr, module);
+	ret = __show_line_range(lr, module, user);
 	exit_symbol_maps();
 
 	return ret;

[PATCH perf/core v2 3/5] Revert "perf probe: Fix to fall back to find probe point in symbols"

[Masami Hiramatsu]

This reverts commit 906451b98b67 ("perf probe: Fix to fall back to find probe point in symbols").

Since perf-probe retries with the address of given symbol
searched from map before this path, this fall back routine
doesn't need anymore.

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

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 4cfd121..c379ea0 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -630,11 +630,9 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
 	}
 
 	if (ntevs == 0)	{	/* No error but failed to find probe point. */
-		pr_warning("Probe point '%s' not found in debuginfo.\n",
+		pr_warning("Probe point '%s' not found.\n",
 			   synthesize_perf_probe_point(&pev->point));
-		if (need_dwarf)
-			return -ENOENT;
-		return 0;
+		return -ENOENT;
 	}
 	/* Error path : ntevs < 0 */
 	pr_debug("An error occurred in debuginfo analysis (%d).\n", ntevs);

[tip:perf/core] Revert "perf probe: Fix to fall back to find probe point in symbols"

[tip-bot for Masami Hiramatsu]

Commit-ID:  0687eba7872d7dbe01b074c54359315e97502360
Gitweb:     http://git.kernel.org/tip/0687eba7872d7dbe01b074c54359315e97502360
Author:     Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
AuthorDate: Fri, 6 Mar 2015 16:31:25 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 12 Mar 2015 12:39:53 -0300

Revert "perf probe: Fix to fall back to find probe point in symbols"

This reverts commit 906451b98b67 ("perf probe: Fix to fall back to find probe point in symbols").

Since 'perf probe' now retries with the address of given symbol searched from
map before this path, this fall back routine isn't needed anymore.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Naohiro Aota <naota@elisp.net>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20150306073124.6904.1751.stgit@localhost.localdomain
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-event.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 4cfd121..c379ea0 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -630,11 +630,9 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
 	}
 
 	if (ntevs == 0)	{	/* No error but failed to find probe point. */
-		pr_warning("Probe point '%s' not found in debuginfo.\n",
+		pr_warning("Probe point '%s' not found.\n",
 			   synthesize_perf_probe_point(&pev->point));
-		if (need_dwarf)
-			return -ENOENT;
-		return 0;
+		return -ENOENT;
 	}
 	/* Error path : ntevs < 0 */
 	pr_debug("An error occurred in debuginfo analysis (%d).\n", ntevs);

[PATCH perf/core v2 4/5] perf symbols: Allow symbol alias when loading map for symbol name

[Masami Hiramatsu]

From: Namhyung Kim <namhyung@kernel.org>

When perf probe tries to add a probe in a binary using symbol name, it
sometimes failed since some symbols were discard during loading dso.
When it resolves an address to symbol, it'd be better to have just one
symbol at given address.  But for finding address from symbol, it'd be
better to keep all names (including aliases).

Add and propagate a new allow_alias argument to dso (and map) load
functions so that it can keep those duplicate symbol aliases.

Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/machine.c        |    2 +-
 tools/perf/util/map.c            |    6 +++---
 tools/perf/util/map.h            |    8 +++++++-
 tools/perf/util/symbol-elf.c     |    5 +++--
 tools/perf/util/symbol-minimal.c |    2 +-
 tools/perf/util/symbol.c         |    8 +++++---
 tools/perf/util/symbol.h         |    5 +++--
 7 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 24f8c97..01ba9b6 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1128,7 +1128,7 @@ static int machine__process_kernel_mmap_event(struct machine *machine,
 			 * preload dso of guest kernel and modules
 			 */
 			dso__load(kernel, machine->vmlinux_maps[MAP__FUNCTION],
-				  NULL);
+				  NULL, false);
 		}
 	}
 	return 0;
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 62ca9f2..711e072 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -248,7 +248,7 @@ void map__fixup_end(struct map *map)
 
 #define DSO__DELETED "(deleted)"
 
-int map__load(struct map *map, symbol_filter_t filter)
+int __map__load(struct map *map, symbol_filter_t filter, bool allow_alias)
 {
 	const char *name = map->dso->long_name;
 	int nr;
@@ -256,7 +256,7 @@ int map__load(struct map *map, symbol_filter_t filter)
 	if (dso__loaded(map->dso, map->type))
 		return 0;
 
-	nr = dso__load(map->dso, map, filter);
+	nr = dso__load(map->dso, map, filter, allow_alias);
 	if (nr < 0) {
 		if (map->dso->has_build_id) {
 			char sbuild_id[BUILD_ID_SIZE * 2 + 1];
@@ -304,7 +304,7 @@ struct symbol *map__find_symbol(struct map *map, u64 addr,
 struct symbol *map__find_symbol_by_name(struct map *map, const char *name,
 					symbol_filter_t filter)
 {
-	if (map__load(map, filter) < 0)
+	if (__map__load(map, filter, true) < 0)
 		return NULL;
 
 	if (!dso__sorted_by_name(map->dso, map->type))
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index 0e42438..ba15607 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -149,7 +149,13 @@ size_t map__fprintf_dsoname(struct map *map, FILE *fp);
 int map__fprintf_srcline(struct map *map, u64 addr, const char *prefix,
 			 FILE *fp);
 
-int map__load(struct map *map, symbol_filter_t filter);
+int __map__load(struct map *map, symbol_filter_t filter, bool allow_alias);
+
+static inline int map__load(struct map *map, symbol_filter_t filter)
+{
+	return __map__load(map, filter, false);
+}
+
 struct symbol *map__find_symbol(struct map *map,
 				u64 addr, symbol_filter_t filter);
 struct symbol *map__find_symbol_by_name(struct map *map, const char *name,
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index ada1676..fb630f8 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -754,7 +754,7 @@ static bool want_demangle(bool is_kernel_sym)
 
 int dso__load_sym(struct dso *dso, struct map *map,
 		  struct symsrc *syms_ss, struct symsrc *runtime_ss,
-		  symbol_filter_t filter, int kmodule)
+		  symbol_filter_t filter, int kmodule, bool allow_alias)
 {
 	struct kmap *kmap = dso->kernel ? map__kmap(map) : NULL;
 	struct map *curr_map = map;
@@ -1048,7 +1048,8 @@ new_symbol:
 	 * For misannotated, zeroed, ASM function sizes.
 	 */
 	if (nr > 0) {
-		symbols__fixup_duplicate(&dso->symbols[map->type]);
+		if (!allow_alias)
+			symbols__fixup_duplicate(&dso->symbols[map->type]);
 		symbols__fixup_end(&dso->symbols[map->type]);
 		if (kmap) {
 			/*
diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
index d7efb03..fefeeb3 100644
--- a/tools/perf/util/symbol-minimal.c
+++ b/tools/perf/util/symbol-minimal.c
@@ -334,7 +334,7 @@ int dso__load_sym(struct dso *dso, struct map *map __maybe_unused,
 		  struct symsrc *ss,
 		  struct symsrc *runtime_ss __maybe_unused,
 		  symbol_filter_t filter __maybe_unused,
-		  int kmodule __maybe_unused)
+		  int kmodule __maybe_unused, bool allow_alias __maybe_unused)
 {
 	unsigned char *build_id[BUILD_ID_SIZE];
 	int ret;
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index a690668..de72a16 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1345,7 +1345,8 @@ static bool dso__is_compatible_symtab_type(struct dso *dso, bool kmod,
 	}
 }
 
-int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter)
+int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter,
+	      bool allow_alias)
 {
 	char *name;
 	int ret = -1;
@@ -1458,7 +1459,8 @@ int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter)
 		runtime_ss = syms_ss;
 
 	if (syms_ss)
-		ret = dso__load_sym(dso, map, syms_ss, runtime_ss, filter, kmod);
+		ret = dso__load_sym(dso, map, syms_ss, runtime_ss, filter, kmod,
+				    allow_alias);
 	else
 		ret = -1;
 
@@ -1516,7 +1518,7 @@ int dso__load_vmlinux(struct dso *dso, struct map *map,
 	if (symsrc__init(&ss, dso, symfs_vmlinux, symtab_type))
 		return -1;
 
-	err = dso__load_sym(dso, map, &ss, &ss, filter, 0);
+	err = dso__load_sym(dso, map, &ss, &ss, filter, 0, false);
 	symsrc__destroy(&ss);
 
 	if (err > 0) {
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 1650dcb..8299038 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -218,7 +218,8 @@ int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
 bool symsrc__has_symtab(struct symsrc *ss);
 bool symsrc__possibly_runtime(struct symsrc *ss);
 
-int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter);
+int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter,
+	      bool allow_alias);
 int dso__load_vmlinux(struct dso *dso, struct map *map,
 		      const char *vmlinux, bool vmlinux_allocated,
 		      symbol_filter_t filter);
@@ -262,7 +263,7 @@ bool symbol__is_idle(struct symbol *sym);
 
 int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *syms_ss,
 		  struct symsrc *runtime_ss, symbol_filter_t filter,
-		  int kmodule);
+		  int kmodule, bool allow_alias);
 int dso__synthesize_plt_symbols(struct dso *dso, struct symsrc *ss,
 				struct map *map, symbol_filter_t filter);
 

Re: [PATCH perf/core v2 4/5] perf symbols: Allow symbol alias when loading map for symbol name

[Arnaldo Carvalho de Melo]

Em Fri, Mar 06, 2015 at 04:31:27PM +0900, Masami Hiramatsu escreveu:
> From: Namhyung Kim <namhyung@kernel.org>
> 
> When perf probe tries to add a probe in a binary using symbol name, it
> sometimes failed since some symbols were discard during loading dso.
> When it resolves an address to symbol, it'd be better to have just one
> symbol at given address.  But for finding address from symbol, it'd be
> better to keep all names (including aliases).
> 
> Add and propagate a new allow_alias argument to dso (and map) load
> functions so that it can keep those duplicate symbol aliases.

Isn't this a global knob, i.e. one that a tool, such as 'perf probe'
flips at startup and leave it turned on while other tools don't need to
care about it?

If so, we should use symbol_conf for that, no? I.e.
symbol_conf.allow_aliases?

Looking at doing that now...

- Arnaldo
 
> Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/machine.c        |    2 +-
>  tools/perf/util/map.c            |    6 +++---
>  tools/perf/util/map.h            |    8 +++++++-
>  tools/perf/util/symbol-elf.c     |    5 +++--
>  tools/perf/util/symbol-minimal.c |    2 +-
>  tools/perf/util/symbol.c         |    8 +++++---
>  tools/perf/util/symbol.h         |    5 +++--
>  7 files changed, 23 insertions(+), 13 deletions(-)
> 
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index 24f8c97..01ba9b6 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -1128,7 +1128,7 @@ static int machine__process_kernel_mmap_event(struct machine *machine,
>  			 * preload dso of guest kernel and modules
>  			 */
>  			dso__load(kernel, machine->vmlinux_maps[MAP__FUNCTION],
> -				  NULL);
> +				  NULL, false);
>  		}
>  	}
>  	return 0;
> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> index 62ca9f2..711e072 100644
> --- a/tools/perf/util/map.c
> +++ b/tools/perf/util/map.c
> @@ -248,7 +248,7 @@ void map__fixup_end(struct map *map)
>  
>  #define DSO__DELETED "(deleted)"
>  
> -int map__load(struct map *map, symbol_filter_t filter)
> +int __map__load(struct map *map, symbol_filter_t filter, bool allow_alias)
>  {
>  	const char *name = map->dso->long_name;
>  	int nr;
> @@ -256,7 +256,7 @@ int map__load(struct map *map, symbol_filter_t filter)
>  	if (dso__loaded(map->dso, map->type))
>  		return 0;
>  
> -	nr = dso__load(map->dso, map, filter);
> +	nr = dso__load(map->dso, map, filter, allow_alias);
>  	if (nr < 0) {
>  		if (map->dso->has_build_id) {
>  			char sbuild_id[BUILD_ID_SIZE * 2 + 1];
> @@ -304,7 +304,7 @@ struct symbol *map__find_symbol(struct map *map, u64 addr,
>  struct symbol *map__find_symbol_by_name(struct map *map, const char *name,
>  					symbol_filter_t filter)
>  {
> -	if (map__load(map, filter) < 0)
> +	if (__map__load(map, filter, true) < 0)
>  		return NULL;
>  
>  	if (!dso__sorted_by_name(map->dso, map->type))
> diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
> index 0e42438..ba15607 100644
> --- a/tools/perf/util/map.h
> +++ b/tools/perf/util/map.h
> @@ -149,7 +149,13 @@ size_t map__fprintf_dsoname(struct map *map, FILE *fp);
>  int map__fprintf_srcline(struct map *map, u64 addr, const char *prefix,
>  			 FILE *fp);
>  
> -int map__load(struct map *map, symbol_filter_t filter);
> +int __map__load(struct map *map, symbol_filter_t filter, bool allow_alias);
> +
> +static inline int map__load(struct map *map, symbol_filter_t filter)
> +{
> +	return __map__load(map, filter, false);
> +}
> +
>  struct symbol *map__find_symbol(struct map *map,
>  				u64 addr, symbol_filter_t filter);
>  struct symbol *map__find_symbol_by_name(struct map *map, const char *name,
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index ada1676..fb630f8 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -754,7 +754,7 @@ static bool want_demangle(bool is_kernel_sym)
>  
>  int dso__load_sym(struct dso *dso, struct map *map,
>  		  struct symsrc *syms_ss, struct symsrc *runtime_ss,
> -		  symbol_filter_t filter, int kmodule)
> +		  symbol_filter_t filter, int kmodule, bool allow_alias)
>  {
>  	struct kmap *kmap = dso->kernel ? map__kmap(map) : NULL;
>  	struct map *curr_map = map;
> @@ -1048,7 +1048,8 @@ new_symbol:
>  	 * For misannotated, zeroed, ASM function sizes.
>  	 */
>  	if (nr > 0) {
> -		symbols__fixup_duplicate(&dso->symbols[map->type]);
> +		if (!allow_alias)
> +			symbols__fixup_duplicate(&dso->symbols[map->type]);
>  		symbols__fixup_end(&dso->symbols[map->type]);
>  		if (kmap) {
>  			/*
> diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
> index d7efb03..fefeeb3 100644
> --- a/tools/perf/util/symbol-minimal.c
> +++ b/tools/perf/util/symbol-minimal.c
> @@ -334,7 +334,7 @@ int dso__load_sym(struct dso *dso, struct map *map __maybe_unused,
>  		  struct symsrc *ss,
>  		  struct symsrc *runtime_ss __maybe_unused,
>  		  symbol_filter_t filter __maybe_unused,
> -		  int kmodule __maybe_unused)
> +		  int kmodule __maybe_unused, bool allow_alias __maybe_unused)
>  {
>  	unsigned char *build_id[BUILD_ID_SIZE];
>  	int ret;
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index a690668..de72a16 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -1345,7 +1345,8 @@ static bool dso__is_compatible_symtab_type(struct dso *dso, bool kmod,
>  	}
>  }
>  
> -int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter)
> +int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter,
> +	      bool allow_alias)
>  {
>  	char *name;
>  	int ret = -1;
> @@ -1458,7 +1459,8 @@ int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter)
>  		runtime_ss = syms_ss;
>  
>  	if (syms_ss)
> -		ret = dso__load_sym(dso, map, syms_ss, runtime_ss, filter, kmod);
> +		ret = dso__load_sym(dso, map, syms_ss, runtime_ss, filter, kmod,
> +				    allow_alias);
>  	else
>  		ret = -1;
>  
> @@ -1516,7 +1518,7 @@ int dso__load_vmlinux(struct dso *dso, struct map *map,
>  	if (symsrc__init(&ss, dso, symfs_vmlinux, symtab_type))
>  		return -1;
>  
> -	err = dso__load_sym(dso, map, &ss, &ss, filter, 0);
> +	err = dso__load_sym(dso, map, &ss, &ss, filter, 0, false);
>  	symsrc__destroy(&ss);
>  
>  	if (err > 0) {
> diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> index 1650dcb..8299038 100644
> --- a/tools/perf/util/symbol.h
> +++ b/tools/perf/util/symbol.h
> @@ -218,7 +218,8 @@ int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
>  bool symsrc__has_symtab(struct symsrc *ss);
>  bool symsrc__possibly_runtime(struct symsrc *ss);
>  
> -int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter);
> +int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter,
> +	      bool allow_alias);
>  int dso__load_vmlinux(struct dso *dso, struct map *map,
>  		      const char *vmlinux, bool vmlinux_allocated,
>  		      symbol_filter_t filter);
> @@ -262,7 +263,7 @@ bool symbol__is_idle(struct symbol *sym);
>  
>  int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *syms_ss,
>  		  struct symsrc *runtime_ss, symbol_filter_t filter,
> -		  int kmodule);
> +		  int kmodule, bool allow_alias);
>  int dso__synthesize_plt_symbols(struct dso *dso, struct symsrc *ss,
>  				struct map *map, symbol_filter_t filter);
>  

Re: [PATCH perf/core v2 4/5] perf symbols: Allow symbol alias when loading map for symbol name

[Arnaldo Carvalho de Melo]

Em Fri, Mar 06, 2015 at 03:06:04PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Fri, Mar 06, 2015 at 04:31:27PM +0900, Masami Hiramatsu escreveu:
> > From: Namhyung Kim <namhyung@kernel.org>
> > 
> > When perf probe tries to add a probe in a binary using symbol name, it
> > sometimes failed since some symbols were discard during loading dso.
> > When it resolves an address to symbol, it'd be better to have just one
> > symbol at given address.  But for finding address from symbol, it'd be
> > better to keep all names (including aliases).
> > 
> > Add and propagate a new allow_alias argument to dso (and map) load
> > functions so that it can keep those duplicate symbol aliases.
> 
> Isn't this a global knob, i.e. one that a tool, such as 'perf probe'
> flips at startup and leave it turned on while other tools don't need to
> care about it?
> 
> If so, we should use symbol_conf for that, no? I.e.
> symbol_conf.allow_aliases?
> 
> Looking at doing that now...

Ugh, I was looking at when was that knob to flipped on in this patch, it
is when one calls map__find_symbol_by_name(), when it assumes that the
map is not loaded yet and thus asks for duplicates not to be eliminated.

But what happens if the map was already loaded by, say,
map__find_symbol(), i.e. to find by address? Then that 'true' in the
__map__load() call from map__find_symbol_by_name will be a nop.

So I think it is really better for this to be a
symbol_conf.allow_aliases that tools set at startup, and perhaps even
this could even be governed by symbol_conf.sort_by_name, but I'll leave
alow_aliases for now, its more flexible.

- Arnaldo
 
> - Arnaldo
>  
> > Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> >  tools/perf/util/machine.c        |    2 +-
> >  tools/perf/util/map.c            |    6 +++---
> >  tools/perf/util/map.h            |    8 +++++++-
> >  tools/perf/util/symbol-elf.c     |    5 +++--
> >  tools/perf/util/symbol-minimal.c |    2 +-
> >  tools/perf/util/symbol.c         |    8 +++++---
> >  tools/perf/util/symbol.h         |    5 +++--
> >  7 files changed, 23 insertions(+), 13 deletions(-)
> > 
> > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> > index 24f8c97..01ba9b6 100644
> > --- a/tools/perf/util/machine.c
> > +++ b/tools/perf/util/machine.c
> > @@ -1128,7 +1128,7 @@ static int machine__process_kernel_mmap_event(struct machine *machine,
> >  			 * preload dso of guest kernel and modules
> >  			 */
> >  			dso__load(kernel, machine->vmlinux_maps[MAP__FUNCTION],
> > -				  NULL);
> > +				  NULL, false);
> >  		}
> >  	}
> >  	return 0;
> > diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> > index 62ca9f2..711e072 100644
> > --- a/tools/perf/util/map.c
> > +++ b/tools/perf/util/map.c
> > @@ -248,7 +248,7 @@ void map__fixup_end(struct map *map)
> >  
> >  #define DSO__DELETED "(deleted)"
> >  
> > -int map__load(struct map *map, symbol_filter_t filter)
> > +int __map__load(struct map *map, symbol_filter_t filter, bool allow_alias)
> >  {
> >  	const char *name = map->dso->long_name;
> >  	int nr;
> > @@ -256,7 +256,7 @@ int map__load(struct map *map, symbol_filter_t filter)
> >  	if (dso__loaded(map->dso, map->type))
> >  		return 0;
> >  
> > -	nr = dso__load(map->dso, map, filter);
> > +	nr = dso__load(map->dso, map, filter, allow_alias);
> >  	if (nr < 0) {
> >  		if (map->dso->has_build_id) {
> >  			char sbuild_id[BUILD_ID_SIZE * 2 + 1];
> > @@ -304,7 +304,7 @@ struct symbol *map__find_symbol(struct map *map, u64 addr,
> >  struct symbol *map__find_symbol_by_name(struct map *map, const char *name,
> >  					symbol_filter_t filter)
> >  {
> > -	if (map__load(map, filter) < 0)
> > +	if (__map__load(map, filter, true) < 0)
> >  		return NULL;
> >  
> >  	if (!dso__sorted_by_name(map->dso, map->type))
> > diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
> > index 0e42438..ba15607 100644
> > --- a/tools/perf/util/map.h
> > +++ b/tools/perf/util/map.h
> > @@ -149,7 +149,13 @@ size_t map__fprintf_dsoname(struct map *map, FILE *fp);
> >  int map__fprintf_srcline(struct map *map, u64 addr, const char *prefix,
> >  			 FILE *fp);
> >  
> > -int map__load(struct map *map, symbol_filter_t filter);
> > +int __map__load(struct map *map, symbol_filter_t filter, bool allow_alias);
> > +
> > +static inline int map__load(struct map *map, symbol_filter_t filter)
> > +{
> > +	return __map__load(map, filter, false);
> > +}
> > +
> >  struct symbol *map__find_symbol(struct map *map,
> >  				u64 addr, symbol_filter_t filter);
> >  struct symbol *map__find_symbol_by_name(struct map *map, const char *name,
> > diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> > index ada1676..fb630f8 100644
> > --- a/tools/perf/util/symbol-elf.c
> > +++ b/tools/perf/util/symbol-elf.c
> > @@ -754,7 +754,7 @@ static bool want_demangle(bool is_kernel_sym)
> >  
> >  int dso__load_sym(struct dso *dso, struct map *map,
> >  		  struct symsrc *syms_ss, struct symsrc *runtime_ss,
> > -		  symbol_filter_t filter, int kmodule)
> > +		  symbol_filter_t filter, int kmodule, bool allow_alias)
> >  {
> >  	struct kmap *kmap = dso->kernel ? map__kmap(map) : NULL;
> >  	struct map *curr_map = map;
> > @@ -1048,7 +1048,8 @@ new_symbol:
> >  	 * For misannotated, zeroed, ASM function sizes.
> >  	 */
> >  	if (nr > 0) {
> > -		symbols__fixup_duplicate(&dso->symbols[map->type]);
> > +		if (!allow_alias)
> > +			symbols__fixup_duplicate(&dso->symbols[map->type]);
> >  		symbols__fixup_end(&dso->symbols[map->type]);
> >  		if (kmap) {
> >  			/*
> > diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
> > index d7efb03..fefeeb3 100644
> > --- a/tools/perf/util/symbol-minimal.c
> > +++ b/tools/perf/util/symbol-minimal.c
> > @@ -334,7 +334,7 @@ int dso__load_sym(struct dso *dso, struct map *map __maybe_unused,
> >  		  struct symsrc *ss,
> >  		  struct symsrc *runtime_ss __maybe_unused,
> >  		  symbol_filter_t filter __maybe_unused,
> > -		  int kmodule __maybe_unused)
> > +		  int kmodule __maybe_unused, bool allow_alias __maybe_unused)
> >  {
> >  	unsigned char *build_id[BUILD_ID_SIZE];
> >  	int ret;
> > diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> > index a690668..de72a16 100644
> > --- a/tools/perf/util/symbol.c
> > +++ b/tools/perf/util/symbol.c
> > @@ -1345,7 +1345,8 @@ static bool dso__is_compatible_symtab_type(struct dso *dso, bool kmod,
> >  	}
> >  }
> >  
> > -int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter)
> > +int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter,
> > +	      bool allow_alias)
> >  {
> >  	char *name;
> >  	int ret = -1;
> > @@ -1458,7 +1459,8 @@ int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter)
> >  		runtime_ss = syms_ss;
> >  
> >  	if (syms_ss)
> > -		ret = dso__load_sym(dso, map, syms_ss, runtime_ss, filter, kmod);
> > +		ret = dso__load_sym(dso, map, syms_ss, runtime_ss, filter, kmod,
> > +				    allow_alias);
> >  	else
> >  		ret = -1;
> >  
> > @@ -1516,7 +1518,7 @@ int dso__load_vmlinux(struct dso *dso, struct map *map,
> >  	if (symsrc__init(&ss, dso, symfs_vmlinux, symtab_type))
> >  		return -1;
> >  
> > -	err = dso__load_sym(dso, map, &ss, &ss, filter, 0);
> > +	err = dso__load_sym(dso, map, &ss, &ss, filter, 0, false);
> >  	symsrc__destroy(&ss);
> >  
> >  	if (err > 0) {
> > diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> > index 1650dcb..8299038 100644
> > --- a/tools/perf/util/symbol.h
> > +++ b/tools/perf/util/symbol.h
> > @@ -218,7 +218,8 @@ int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
> >  bool symsrc__has_symtab(struct symsrc *ss);
> >  bool symsrc__possibly_runtime(struct symsrc *ss);
> >  
> > -int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter);
> > +int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter,
> > +	      bool allow_alias);
> >  int dso__load_vmlinux(struct dso *dso, struct map *map,
> >  		      const char *vmlinux, bool vmlinux_allocated,
> >  		      symbol_filter_t filter);
> > @@ -262,7 +263,7 @@ bool symbol__is_idle(struct symbol *sym);
> >  
> >  int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *syms_ss,
> >  		  struct symsrc *runtime_ss, symbol_filter_t filter,
> > -		  int kmodule);
> > +		  int kmodule, bool allow_alias);
> >  int dso__synthesize_plt_symbols(struct dso *dso, struct symsrc *ss,
> >  				struct map *map, symbol_filter_t filter);
> >  

Re: [PATCH perf/core v2 4/5] perf symbols: Allow symbol alias when loading map for symbol name

[Arnaldo Carvalho de Melo]

Em Fri, Mar 06, 2015 at 03:26:30PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Fri, Mar 06, 2015 at 03:06:04PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Fri, Mar 06, 2015 at 04:31:27PM +0900, Masami Hiramatsu escreveu:
> > > From: Namhyung Kim <namhyung@kernel.org>
> > > 
> > > When perf probe tries to add a probe in a binary using symbol name, it
> > > sometimes failed since some symbols were discard during loading dso.
> > > When it resolves an address to symbol, it'd be better to have just one
> > > symbol at given address.  But for finding address from symbol, it'd be
> > > better to keep all names (including aliases).
> > > 
> > > Add and propagate a new allow_alias argument to dso (and map) load
> > > functions so that it can keep those duplicate symbol aliases.
> > 
> > Isn't this a global knob, i.e. one that a tool, such as 'perf probe'
> > flips at startup and leave it turned on while other tools don't need to
> > care about it?
> > 
> > If so, we should use symbol_conf for that, no? I.e.
> > symbol_conf.allow_aliases?
> > 
> > Looking at doing that now...
> 
> Ugh, I was looking at when was that knob to flipped on in this patch, it
> is when one calls map__find_symbol_by_name(), when it assumes that the
> map is not loaded yet and thus asks for duplicates not to be eliminated.
> 
> But what happens if the map was already loaded by, say,
> map__find_symbol(), i.e. to find by address? Then that 'true' in the
> __map__load() call from map__find_symbol_by_name will be a nop.
> 
> So I think it is really better for this to be a
> symbol_conf.allow_aliases that tools set at startup, and perhaps even
> this could even be governed by symbol_conf.sort_by_name, but I'll leave
> alow_aliases for now, its more flexible.

For reference, below is what I have now in my tree, if you are OK with
it that is what I'll push to Ingo,

- Arnaldo

commit 3dae901ca3eddbd76bc20f47e0019c712ade7d14
Author: Namhyung Kim <namhyung@kernel.org>
Date:   Fri Mar 6 16:31:27 2015 +0900

    perf symbols: Allow symbol alias when loading map for symbol name
    
    When perf probe tries to add a probe in a binary using symbol name, it
    sometimes failed since some symbols were discard during loading dso.
    
    When it resolves an address to symbol, it'd be better to have just one
    symbol at given address.  But for finding address from symbol, it'd be
    better to keep all names (including aliases).
    
    So allow tools to state that they want to allow aliases via
    symbol_conf.allow_aliases.
    
    Signed-off-by: Namhyung Kim <namhyung@kernel.org>
    Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
    Cc: David Ahern <dsahern@gmail.com>
    Cc: Ingo Molnar <mingo@kernel.org>
    Cc: Jiri Olsa <jolsa@redhat.com>
    Cc: Naohiro Aota <naota@elisp.net>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Link: http://lkml.kernel.org/r/20150306073127.6904.3232.stgit@localhost.localdomain
    [ Original patch passwd allow_alias to many functions, use symbol_conf.allow_aliases instead ]
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index c379ea0edfd5..9feba0e3343e 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -80,6 +80,7 @@ static int init_symbol_maps(bool user_only)
 	int ret;
 
 	symbol_conf.sort_by_name = true;
+	symbol_conf.allow_aliases = true;
 	ret = symbol__init(NULL);
 	if (ret < 0) {
 		pr_debug("Failed to init symbol map.\n");
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index ada16762fac2..62742e46c010 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1048,7 +1048,8 @@ new_symbol:
 	 * For misannotated, zeroed, ASM function sizes.
 	 */
 	if (nr > 0) {
-		symbols__fixup_duplicate(&dso->symbols[map->type]);
+		if (!symbol_conf.allow_aliases)
+			symbols__fixup_duplicate(&dso->symbols[map->type]);
 		symbols__fixup_end(&dso->symbols[map->type]);
 		if (kmap) {
 			/*
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 1650dcb3a67b..efdaaa544041 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -87,6 +87,7 @@ struct symbol_conf {
 			ignore_vmlinux_buildid,
 			show_kernel_path,
 			use_modules,
+			allow_aliases,
 			sort_by_name,
 			show_nr_samples,
 			show_total_period,

[tip:perf/core] perf symbols: Allow symbol alias when loading map for symbol name

[tip-bot for Namhyung Kim]

Commit-ID:  680d926a8cb08dd9cf173e2bb93d4a4477771949
Gitweb:     http://git.kernel.org/tip/680d926a8cb08dd9cf173e2bb93d4a4477771949
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Fri, 6 Mar 2015 16:31:27 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 12 Mar 2015 12:39:54 -0300

perf symbols: Allow symbol alias when loading map for symbol name

When perf probe tries to add a probe in a binary using symbol name, it
sometimes failed since some symbols were discard during loading dso.

When it resolves an address to symbol, it'd be better to have just one
symbol at given address.  But for finding address from symbol, it'd be
better to keep all names (including aliases).

So allow tools to state that they want to allow aliases via
symbol_conf.allow_aliases.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Naohiro Aota <naota@elisp.net>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20150306073127.6904.3232.stgit@localhost.localdomain
[ Original patch passwd allow_alias to many functions, use symbol_conf.allow_aliases instead ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-event.c | 1 +
 tools/perf/util/symbol-elf.c  | 3 ++-
 tools/perf/util/symbol.h      | 1 +
 3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index c379ea0..9feba0e 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -80,6 +80,7 @@ static int init_symbol_maps(bool user_only)
 	int ret;
 
 	symbol_conf.sort_by_name = true;
+	symbol_conf.allow_aliases = true;
 	ret = symbol__init(NULL);
 	if (ret < 0) {
 		pr_debug("Failed to init symbol map.\n");
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index ada1676..62742e4 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1048,7 +1048,8 @@ new_symbol:
 	 * For misannotated, zeroed, ASM function sizes.
 	 */
 	if (nr > 0) {
-		symbols__fixup_duplicate(&dso->symbols[map->type]);
+		if (!symbol_conf.allow_aliases)
+			symbols__fixup_duplicate(&dso->symbols[map->type]);
 		symbols__fixup_end(&dso->symbols[map->type]);
 		if (kmap) {
 			/*
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 1650dcb..efdaaa5 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -87,6 +87,7 @@ struct symbol_conf {
 			ignore_vmlinux_buildid,
 			show_kernel_path,
 			use_modules,
+			allow_aliases,
 			sort_by_name,
 			show_nr_samples,
 			show_total_period,

[PATCH perf/core v2 5/5] perf probe: Allow weak symbols to be probed

[Masami Hiramatsu]

From: Namhyung Kim <namhyung@kernel.org>

It currently prevents adding probes in weak symbols.  But there're cases
that given name is an only weak symbol so that we cannot add probe.

  $ perf probe -x /usr/lib/libc.so.6 -a calloc
  Failed to find symbol calloc in /usr/lib/libc-2.21.so
    Error: Failed to add events.

  $ nm /usr/lib/libc.so.6 | grep calloc
  000000000007b1f0 t __calloc
  000000000007b1f0 T __libc_calloc
  000000000007b1f0 W calloc

This change will result in duplicate probes when strong and weak symbols
co-exist in a binary.  But I think it's not a big problem since probes
at the weak symbol will never be hit anyway.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/probe-event.c |   12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index c379ea0..f9c1e53 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -309,10 +309,8 @@ static int find_alternative_probe_point(struct debuginfo *dinfo,
 
 	/* Find the address of given function */
 	map__for_each_symbol_by_name(map, pp->function, sym) {
-		if (sym->binding == STB_GLOBAL || sym->binding == STB_LOCAL) {
-			address = sym->start;
-			break;
-		}
+		address = sym->start;
+		break;
 	}
 	if (!address) {
 		ret = -ENOENT;
@@ -2484,8 +2482,7 @@ static int find_probe_functions(struct map *map, char *name)
 	struct symbol *sym;
 
 	map__for_each_symbol_by_name(map, name, sym) {
-		if (sym->binding == STB_GLOBAL || sym->binding == STB_LOCAL)
-			found++;
+		found++;
 	}
 
 	return found;
@@ -2845,8 +2842,7 @@ static struct strfilter *available_func_filter;
 static int filter_available_functions(struct map *map __maybe_unused,
 				      struct symbol *sym)
 {
-	if ((sym->binding == STB_GLOBAL || sym->binding == STB_LOCAL) &&
-	    strfilter__compare(available_func_filter, sym->name))
+	if (strfilter__compare(available_func_filter, sym->name))
 		return 0;
 	return 1;
 }

[tip:perf/core] perf probe: Allow weak symbols to be probed

[tip-bot for Namhyung Kim]

Commit-ID:  e578da3b2009da2a9ae2d25fd0f78c7b76ca5e56
Gitweb:     http://git.kernel.org/tip/e578da3b2009da2a9ae2d25fd0f78c7b76ca5e56
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Fri, 6 Mar 2015 16:31:29 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 12 Mar 2015 12:39:55 -0300

perf probe: Allow weak symbols to be probed

It currently prevents adding probes in weak symbols.  But there're cases
that given name is an only weak symbol so that we cannot add probe.

  $ perf probe -x /usr/lib/libc.so.6 -a calloc
  Failed to find symbol calloc in /usr/lib/libc-2.21.so
    Error: Failed to add events.

  $ nm /usr/lib/libc.so.6 | grep calloc
  000000000007b1f0 t __calloc
  000000000007b1f0 T __libc_calloc
  000000000007b1f0 W calloc

This change will result in duplicate probes when strong and weak symbols
co-exist in a binary.  But I think it's not a big problem since probes
at the weak symbol will never be hit anyway.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Naohiro Aota <naota@elisp.net>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20150306073129.6904.41078.stgit@localhost.localdomain
Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-event.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 9feba0e..8af8e7f 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -310,10 +310,8 @@ static int find_alternative_probe_point(struct debuginfo *dinfo,
 
 	/* Find the address of given function */
 	map__for_each_symbol_by_name(map, pp->function, sym) {
-		if (sym->binding == STB_GLOBAL || sym->binding == STB_LOCAL) {
-			address = sym->start;
-			break;
-		}
+		address = sym->start;
+		break;
 	}
 	if (!address) {
 		ret = -ENOENT;
@@ -2485,8 +2483,7 @@ static int find_probe_functions(struct map *map, char *name)
 	struct symbol *sym;
 
 	map__for_each_symbol_by_name(map, name, sym) {
-		if (sym->binding == STB_GLOBAL || sym->binding == STB_LOCAL)
-			found++;
+		found++;
 	}
 
 	return found;
@@ -2846,8 +2843,7 @@ static struct strfilter *available_func_filter;
 static int filter_available_functions(struct map *map __maybe_unused,
 				      struct symbol *sym)
 {
-	if ((sym->binding == STB_GLOBAL || sym->binding == STB_LOCAL) &&
-	    strfilter__compare(available_func_filter, sym->name))
+	if (strfilter__compare(available_func_filter, sym->name))
 		return 0;
 	return 1;
 }

Re: [PATCH perf/core v2 0/5] perf-probe: improve glibc support

[Arnaldo Carvalho de Melo]

Em Fri, Mar 06, 2015 at 04:31:18PM +0900, Masami Hiramatsu escreveu:
> Hi,
> 
> Here is a series of patches which improves perf-probe to
> handle glibc's aliased symbols and weak symbols more
> correctly.

Ok, I have this, with the change I mentioned in another message, in my
perf/core branch, please take a look,

Thanks,

- Arnaldo