Re: [PATCH v2 5/9] perf tools: Add methods to test dso is 64-bit or 32-bit

[Hekuang]

å 2016/5/10 18:34, Adrian Hunter åé:
> On 10/05/16 12:49, Hekuang wrote:
> > hi
> >
> > å 2016/5/10 16:08, Adrian Hunter åé:
> > > On 10/05/16 10:40, He Kuang wrote:
> > > > 32-bit programs can be run on 64-bit machines, so we should choose
> > > > unwind methods according to 'thread->map' instead of the host
> > > > architecture.
> > > >
> > > > This patch adds methods to test whether a dso is 64-bit or 32-bit by
> > > > the class info in elf.
> > > What about using dso->is_64_bit set by dso__load_sym() ?
> > I've noticed this variable, but it's value is not as its name said:
> >
> > util/dso.c: 1067    dso->is_64_bit = (sizeof(void *) == 8);
> That is just initialization i.e. before we know what it is we assume it is
> the same as the host.
>
> > This is only related to the host architecture.
> >
> > A closer one is 'is_64_bit' in 'struct symsrc', but the value is assigned
> > after dso
> > loaded. So I think we should provide individual methods to get that value.
> Are you saying you don't load dsos?  Or that is_64_bit is set incorrectly?

Yes, I know it's the inital value, but the correct value is
assigned in function dso__load_sym(), and have a look at the call
stack(gdb):

#0  dso__load_sym
#1  in dso__load
#2  in map__load
#3  in map__find_symbol
#4  in thread__find_addr_location
#5  in entry
#6  in get_entries
#7  in _Ux86__unwind__get_entries
#8  in thread__resolve_callchain

I think we should choose the right unwind method before
dso__load_sym(). i.e. line#7, which is called before dso__load_sym().

I'm not very familiar with this, what's your opinion?

Thanks.
> > Thanks.
> >
> > > > Signed-off-by: He Kuang <hekuang@xxxxxxxxxx>
> > > > ---
> > > >    tools/perf/util/symbol-elf.c | 16 +++++++++++++++
> > > >    tools/perf/util/symbol.c     | 49
> > > > ++++++++++++++++++++++++++++++++++++++++++++
> > > >    tools/perf/util/symbol.h     |  2 ++
> > > >    3 files changed, 67 insertions(+)
> > > >
> > > > diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> > > > index 3f9d679..9f290b9 100644
> > > > --- a/tools/perf/util/symbol-elf.c
> > > > +++ b/tools/perf/util/symbol-elf.c
> > > > @@ -636,6 +636,22 @@ bool __weak elf__needs_adjust_symbols(GElf_Ehdr ehdr)
> > > >        return ehdr.e_type == ET_EXEC || ehdr.e_type == ET_REL;
> > > >    }
> > > >    +int elf_is_64_bit(char *name)
> > > > +{
> > > > +    Elf *elf;
> > > > +    int fd;
> > > > +
> > > > +    fd = open(name, O_RDONLY);
> > > > +    if (fd < 0)
> > > > +        return -1;
> > > > +
> > > > +    elf = elf_begin(fd, PERF_ELF_C_READ_MMAP, NULL);
> > > > +    if (elf == NULL)
> > > > +        return -1;
> > > > +
> > > > +    return (gelf_getclass(elf) == ELFCLASS64);
> > > > +}
> > > > +
> > > >    int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
> > > >             enum dso_binary_type type)
> > > >    {
> > > > diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> > > > index 4630751..592bf8c 100644
> > > > --- a/tools/perf/util/symbol.c
> > > > +++ b/tools/perf/util/symbol.c
> > > > @@ -1395,6 +1395,55 @@ static bool dso__is_compatible_symtab_type(struct
> > > > dso *dso, bool kmod,
> > > >        }
> > > >    }
> > > >    +int dso_is_64_bit(struct dso *dso, struct map *map)
> > > > +{
> > > > +    char *name;
> > > > +    u_int i;
> > > > +    bool kmod;
> > > > +    char *root_dir = (char *) "";
> > > > +    struct machine *machine;
> > > > +
> > > > +    if (map->groups && map->groups->machine)
> > > > +        machine = map->groups->machine;
> > > > +    else
> > > > +        machine = NULL;
> > > > +
> > > > +    if (machine)
> > > > +        root_dir = machine->root_dir;
> > > > +
> > > > +    name = malloc(PATH_MAX);
> > > > +    if (!name)
> > > > +        return -1;
> > > > +
> > > > +    kmod = dso->symtab_type == DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE ||
> > > > +        dso->symtab_type == DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE_COMP ||
> > > > +        dso->symtab_type == DSO_BINARY_TYPE__GUEST_KMODULE ||
> > > > +        dso->symtab_type == DSO_BINARY_TYPE__GUEST_KMODULE_COMP;
> > > > +
> > > > +    /*
> > > > +     * Iterate over candidate debug images.
> > > > +     * Keep track of "interesting" ones (those which have a symtab, dynsym,
> > > > +     * and/or opd section) for processing.
> > > > +     */
> > > > +    for (i = 0; i < DSO_BINARY_TYPE__SYMTAB_CNT; i++) {
> > > > +        enum dso_binary_type symtab_type = binary_type_symtab[i];
> > > > +
> > > > +        if (!dso__is_compatible_symtab_type(dso, kmod, symtab_type))
> > > > +            continue;
> > > > +
> > > > +        if (dso__read_binary_type_filename(dso, symtab_type,
> > > > +                           root_dir, name, PATH_MAX))
> > > > +            continue;
> > > > +
> > > > +        if (!is_regular_file(name))
> > > > +            continue;
> > > > +
> > > > +        return elf_is_64_bit(name);
> > > > +    }
> > > > +
> > > > +    return -1;
> > > > +}
> > > > +
> > > >    int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter)
> > > >    {
> > > >        char *name;
> > > > diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> > > > index 4e6910e..d33fbf4 100644
> > > > --- a/tools/perf/util/symbol.h
> > > > +++ b/tools/perf/util/symbol.h
> > > > @@ -308,6 +308,8 @@ int setup_list(struct strlist **list, const char
> > > > *list_str,
> > > >               const char *list_name);
> > > >    int setup_intlist(struct intlist **list, const char *list_str,
> > > >              const char *list_name);
> > > > +int elf_is_64_bit(char *name);
> > > > +int dso_is_64_bit(struct dso *dso, struct map *map);
> > > >      #ifdef HAVE_LIBELF_SUPPORT
> > > >    bool elf__needs_adjust_symbols(GElf_Ehdr ehdr);