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@xxxxxxxxxx>
> 
> 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@xxxxxxxxxxx>
> Signed-off-by: Namhyung Kim <namhyung@xxxxxxxxxx>
> ---
>  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);
>  
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/