Re: [PATCH 1/1] perf tools: Fix Symbol Address for ET_DYN

[Namhyung Kim <namhyung@kernel.org>]

Hi Tong,

On Thu, 25 Sep 2014 22:53:22 -0700, Tong Shen wrote:
>>>>>> +/**
>>>>>> + * is_non_relocatable_dyn - check if ELF file is ET_DYN, but not relocatable
>>>>>> + * @ehdr: ELF header of the ELF file
>>>>>> + * @shdr: a section header in the ELF file
>>>>>> + *
>>>>>> + * For ELF files with type ET_EXEC and ET_REL, we adjust symbol addresses in
>>>>>> + * symbol table in dso__load_sym() so that later we can always locate symbols
>>>>>> + * by sym.st_value + map_address_of_ELF_file.
>>>>>> + *
>>>>>> + * But ELF files with type ET_DYN may need adjusting as well, if they are
>>>>>> + * not relocatable. There's no standard way to tell if an ELF file with type
>>>>>> + * ET_DYN is relocatable. One possible way to do that is checking if
>>>>>> + * sh_addr == sh_offset for .text section.
>>>>>> + */
>>>>>> +static bool is_non_relocatable_dyn(GElf_Ehdr *ehdr, GElf_Shdr *shdr,
>>>>>> +                                   Elf_Data *secstrs) {
>>>>>> +     return ehdr->e_type == ET_DYN &&
>>>>>> +            elf_sec__is_text(shdr, secstrs) &&
>>>>>> +            shdr->sh_offset != shdr->sh_addr;
>>>>>> +}
>>>>>> +
>>>>>>  int dso__load_sym(struct dso *dso, struct map *map,
>>>>>>                 struct symsrc *syms_ss, struct symsrc *runtime_ss,
>>>>>>                 symbol_filter_t filter, int kmodule)
>>>>>> @@ -914,8 +935,9 @@ int dso__load_sym(struct dso *dso, struct map *map,
>>>>>>                       goto new_symbol;
>>>>>>               }
>>>>>>
>>>>>> -             if ((used_opd && runtime_ss->adjust_symbols)
>>>>>> -                             || (!used_opd && syms_ss->adjust_symbols)) {
>>>>>> +             if ((used_opd && runtime_ss->adjust_symbols) ||
>>>>>> +                 (!used_opd && syms_ss->adjust_symbols) ||
>>>>>> +                 is_non_relocatable_dyn(&ehdr, &shdr, secstrs)) {
>>>>>>                       pr_debug4("%s: adjusting symbol: st_value: %#" PRIx64 " "
>>>>>>                                 "sh_addr: %#" PRIx64 " sh_offset: %#" PRIx64 "\n", __func__,
>>>>>>                                 (u64)sym.st_value, (u64)shdr.sh_addr,

Hmm.. IIUC for normal dso (ET_DYN), shdr->offset == shdr->sh_addr for
text section right?  And we always adjust ET_EXEC and ET_REL..  What
about always trying to adjust symbol address then?  We may precalculate
adjust offset and subtracting it from symbol values.  And the offset of
0 effectively means no adjust.  This way we can simplify the logic IMHO.

Thanks,
Namhyung