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

[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?


> 
> 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);
>>>
>>
> 
> 
>