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

From: Arnaldo Carvalho de Melo
Date: Fri Jun 22 2012 - 08:43:29 EST

Next message: Mimi Zohar: "deferring __fput()"
Previous message: Arnaldo Carvalho de Melo: "Re: [PATCH] perf symbols: Follow .gnu_debuglink section to findseparate symbols"
In reply to: Namhyung Kim: "[PATCH 5/8] perf symbols: Do not use ELF's symbol binding constants"
Next in thread: Namhyung Kim: "Re: [PATCH 5/8] perf symbols: Do not use ELF's symbol bindingconstants"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Em Fri, Jun 22, 2012 at 02:37:39PM +0900, Namhyung Kim escreveu:
> From: Namhyung Kim <namhyung.kim@xxxxxxx>
>
> 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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxx\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
--
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/

Next message: Mimi Zohar: "deferring __fput()"
Previous message: Arnaldo Carvalho de Melo: "Re: [PATCH] perf symbols: Follow .gnu_debuglink section to findseparate symbols"
In reply to: Namhyung Kim: "[PATCH 5/8] perf symbols: Do not use ELF's symbol binding constants"
Next in thread: Namhyung Kim: "Re: [PATCH 5/8] perf symbols: Do not use ELF's symbol bindingconstants"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]