Re: [PATCH 5/8] perf symbols: Do not use ELF's symbol binding constants

[Arnaldo Carvalho de Melo <acme@ghostprotocols.net>]

Em Fri, Jun 22, 2012 at 02:37:39PM +0900, Namhyung Kim escreveu:
> From: Namhyung Kim <namhyung.kim@lge.com>
> 
> There's no need to use the ELF's internal value directly.
> Define and use our own - it's required to eliminated the
> dependency of libelf.

Why don't you set STB_GLOBAL, etc to the expected values when libelf is
not present? That way no changes need to be made to symbol.c

Ditto for GELF_ST_BIND.

I.e. keep the subset of libelf.h that we use, providing those
definitions on the poor man's libelf.h we should use when the "real
thing" is not available.

- Arnaldo
 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/builtin-top.c     |    5 ++---
>  tools/perf/ui/browsers/map.c |    5 ++---
>  tools/perf/util/symbol.c     |   40 ++++++++++++++++++++++++++--------------
>  tools/perf/util/symbol.h     |    4 ++++
>  4 files changed, 34 insertions(+), 20 deletions(-)
> 
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index e3cab5f088f8..e0977175f689 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -42,7 +42,6 @@
>  #include "util/debug.h"
>  
>  #include <assert.h>
> -#include <elf.h>
>  #include <fcntl.h>
>  
>  #include <stdio.h>
> @@ -181,8 +180,8 @@ static void ui__warn_map_erange(struct map *map, struct symbol *sym, u64 ip)
>  		    "Please report to linux-kernel@vger.kernel.org\n",
>  		    ip, map->dso->long_name, dso__symtab_origin(map->dso),
>  		    map->start, map->end, sym->start, sym->end,
> -		    sym->binding == STB_GLOBAL ? 'g' :
> -		    sym->binding == STB_LOCAL  ? 'l' : 'w', sym->name,
> +		    sym->binding == SYMBIND_GLOBAL ? 'g' :
> +		    sym->binding == SYMBIND_LOCAL  ? 'l' : 'w', sym->name,
>  		    err ? "[unknown]" : uts.machine,
>  		    err ? "[unknown]" : uts.release, perf_version_string);
>  	if (use_browser <= 0)
> diff --git a/tools/perf/ui/browsers/map.c b/tools/perf/ui/browsers/map.c
> index 98851d55a53e..f2059664b23f 100644
> --- a/tools/perf/ui/browsers/map.c
> +++ b/tools/perf/ui/browsers/map.c
> @@ -1,5 +1,4 @@
>  #include "../libslang.h"
> -#include <elf.h>
>  #include <newt.h>
>  #include <inttypes.h>
>  #include <sys/ttydefaults.h>
> @@ -61,8 +60,8 @@ static void map_browser__write(struct ui_browser *self, void *nd, int row)
>  	ui_browser__set_percent_color(self, 0, current_entry);
>  	slsmg_printf("%*" PRIx64 " %*" PRIx64 " %c ",
>  		     mb->addrlen, sym->start, mb->addrlen, sym->end,
> -		     sym->binding == STB_GLOBAL ? 'g' :
> -		     sym->binding == STB_LOCAL  ? 'l' : 'w');
> +		     sym->binding == SYMBIND_GLOBAL ? 'g' :
> +		     sym->binding == SYMBIND_LOCAL  ? 'l' : 'w');
>  	width = self->width - ((mb->addrlen * 2) + 4);
>  	if (width > 0)
>  		slsmg_write_nstring(sym->name, width);
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 38447ad42ad1..db807a8535d1 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -114,16 +114,16 @@ static int choose_best_symbol(struct symbol *syma, struct symbol *symb)
>  		return SYMBOL_B;
>  
>  	/* Prefer a non weak symbol over a weak one */
> -	a = syma->binding == STB_WEAK;
> -	b = symb->binding == STB_WEAK;
> +	a = syma->binding == SYMBIND_WEAK;
> +	b = symb->binding == SYMBIND_WEAK;
>  	if (b && !a)
>  		return SYMBOL_A;
>  	if (a && !b)
>  		return SYMBOL_B;
>  
>  	/* Prefer a global symbol over a non global one */
> -	a = syma->binding == STB_GLOBAL;
> -	b = symb->binding == STB_GLOBAL;
> +	a = syma->binding == SYMBIND_GLOBAL;
> +	b = symb->binding == SYMBIND_GLOBAL;
>  	if (a && !b)
>  		return SYMBOL_A;
>  	if (b && !a)
> @@ -259,8 +259,8 @@ static size_t symbol__fprintf(struct symbol *sym, FILE *fp)
>  {
>  	return fprintf(fp, " %" PRIx64 "-%" PRIx64 " %c %s\n",
>  		       sym->start, sym->end,
> -		       sym->binding == STB_GLOBAL ? 'g' :
> -		       sym->binding == STB_LOCAL  ? 'l' : 'w',
> +		       sym->binding == SYMBIND_GLOBAL ? 'g' :
> +		       sym->binding == SYMBIND_LOCAL  ? 'l' : 'w',
>  		       sym->name);
>  }
>  
> @@ -609,12 +609,12 @@ struct process_kallsyms_args {
>  	struct dso *dso;
>  };
>  
> -static u8 kallsyms2elf_type(char type)
> +static u8 kallsyms_binding(char type)
>  {
>  	if (type == 'W')
> -		return STB_WEAK;
> +		return SYMBIND_WEAK;
>  
> -	return isupper(type) ? STB_GLOBAL : STB_LOCAL;
> +	return isupper(type) ? SYMBIND_GLOBAL : SYMBIND_LOCAL;
>  }
>  
>  static int map__process_kallsym_symbol(void *arg, const char *name,
> @@ -628,7 +628,7 @@ static int map__process_kallsym_symbol(void *arg, const char *name,
>  		return 0;
>  
>  	sym = symbol__new(start, end - start + 1,
> -			  kallsyms2elf_type(type), name);
> +			  kallsyms_binding(type), name);
>  	if (sym == NULL)
>  		return -ENOMEM;
>  	/*
> @@ -851,7 +851,7 @@ static int dso__load_perf_map(struct dso *dso, struct map *map,
>  		if (len + 2 >= line_len)
>  			continue;
>  
> -		sym = symbol__new(start, size, STB_GLOBAL, line + len);
> +		sym = symbol__new(start, size, SYMBIND_GLOBAL, line + len);
>  
>  		if (sym == NULL)
>  			goto out_delete_line;
> @@ -1064,7 +1064,7 @@ dso__synthesize_plt_symbols(struct dso *dso, char *name, struct map *map,
>  				 "%s@plt", elf_sym__name(&sym, symstrs));
>  
>  			f = symbol__new(plt_offset, shdr_plt.sh_entsize,
> -					STB_GLOBAL, sympltname);
> +					SYMBIND_GLOBAL, sympltname);
>  			if (!f)
>  				goto out_elf_end;
>  
> @@ -1086,7 +1086,7 @@ dso__synthesize_plt_symbols(struct dso *dso, char *name, struct map *map,
>  				 "%s@plt", elf_sym__name(&sym, symstrs));
>  
>  			f = symbol__new(plt_offset, shdr_plt.sh_entsize,
> -					STB_GLOBAL, sympltname);
> +					SYMBIND_GLOBAL, sympltname);
>  			if (!f)
>  				goto out_elf_end;
>  
> @@ -1157,6 +1157,18 @@ static size_t elf_addr_to_index(Elf *elf, GElf_Addr addr)
>  	return -1;
>  }
>  
> +static int elf_sym__binding(const GElf_Sym *sym)
> +{
> +	switch (GELF_ST_BIND(sym->st_info)) {
> +	case STB_GLOBAL:
> +		return SYMBIND_GLOBAL;
> +	case STB_WEAK:
> +		return SYMBIND_WEAK;
> +	default:
> +		return SYMBIND_LOCAL;
> +	}
> +}
> +
>  static int dso__swap_init(struct dso *dso, unsigned char eidata)
>  {
>  	static unsigned int const endian = 1;
> @@ -1390,7 +1402,7 @@ static int dso__load_sym(struct dso *dso, struct map *map, const char *name,
>  			elf_name = demangled;
>  new_symbol:
>  		f = symbol__new(sym.st_value, sym.st_size,
> -				GELF_ST_BIND(sym.st_info), elf_name);
> +				elf_sym__binding(&sym), elf_name);
>  		free(demangled);
>  		if (!f)
>  			goto out_elf_end;
> diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> index 295a9aa1cc78..bd91d2aa8262 100644
> --- a/tools/perf/util/symbol.h
> +++ b/tools/perf/util/symbol.h
> @@ -50,6 +50,10 @@ char *strxfrchar(char *s, char from, char to);
>  
>  #define BUILD_ID_SIZE 20
>  
> +#define SYMBIND_LOCAL	0
> +#define SYMBIND_GLOBAL	1
> +#define SYMBIND_WEAK	2
> +
>  /** struct symbol - symtab entry
>   *
>   * @ignore - resolvable but tools ignore it (e.g. idle routines)
> -- 
> 1.7.10.2