[RFC PATCH] Fix PPC64 ELF ABI v2 symbol address retrieval

[Hemant Kumar]

On 01/23/2015 07:30 PM, Mark Wielaard wrote:
> On Fri, 2015-01-16 at 17:24 +0530, Hemant Kumar wrote:
>> PPC64 ELF ABI v2 has a Global entry point and a local entry point for the
>> functions. We need the Local entry point in order to probe these functions.
>> However, the DIE for these functions in debuginfo return the function.entrypc
>> which is same as the global entry point. The local entry point is not encoded
>> in the debuginfo of the ELFs.
> BTW. Would it make sense to let GCC encode this in the DWARF output?
> DWARF 4, 2.18 Entry Address says:
>
>          Any debugging information entry describing an entity that has a
>          range of code addresses, which includes compilation units,
>          module initialization, subroutines, ordinary blocks, try/catch
>          blocks, and the like, may have a DW_AT_entry_pc attribute to
>          indicate the first executable instruction within that range of
>          addresses. The value of the DW_AT_entry_pc attribute is a
>          relocated address. If no DW_AT_entry_pc attribute is present,
>          then the entry address is assumed to be the same as the value of
>          the DW_AT_low_pc attribute, if present; otherwise, the entry
>          address is unknown.
>
> Which seems to describe the situation exactly. And would make elfutils
> libdw dwarf_entrypc () do the right thing automagically (it will fall
> back to low_pc as described) without needing any extra lookups through
> symbol tables.
>
>> The offset to local entry point is however encoded in the st_other field of
>> these symbols in the symbol table. We need to use this field to adjust the
>> sym.st_value to actually point to the local entry point instead of the global
>> entry point.
>>
>> This patch is in relation to this bug :
>> https://sourceware.org/bugzilla/show_bug.cgi?id=17638
>>
>> So, while adding symbols to the sym_table, we add an offset of
>> PPC64_LOCAL_ENTRY_OFFSET(sym.st_other) to st_value.
>> And when the function address is queried in query_dwarf_func(), we give priority
>> to the cached sym_table, where we can retrieve the adjusted entry address of the
>> function. If we don't get any address from the symbol table, then we proceed to
>> get from the debuginfo.
> The function_entrypc () part looks good (see some small nitpicks below).
> But I think the priority/fixup needs to be done the other way around
> (see also below).
>
>> Signed-off-by: Hemant Kumar <hemant@linux.vnet.ibm.com>
>> ---
>>   tapsets.cxx |   34 +++++++++++++++++++++++++++++++++-
>>   1 file changed, 33 insertions(+), 1 deletion(-)
>>
>> diff --git a/tapsets.cxx b/tapsets.cxx
>> index 85fd76b..d1382e4 100644
>> --- a/tapsets.cxx
>> +++ b/tapsets.cxx
>> @@ -2099,7 +2099,19 @@ query_dwarf_func (Dwarf_Die * func, dwarf_query * q)
>>             q->dw.function_line (&func.decl_line);
>>   
>>             Dwarf_Addr entrypc;
>> -          if (q->dw.function_entrypc (&entrypc))
>> +          func.entrypc = 0;
>> +          /* Giving priority to sym_table */
>> +          if (q->dw.mod_info->sym_table)
>> +            {
>> +              func_info * fi;
>> +              fi = q->dw.mod_info->sym_table->lookup_symbol(func.name);
>> +              if (fi)
>> +                {
>> +                  func.entrypc = fi->addr;
>> +                  q->filtered_functions.push_back(func);
>> +                }
>> +            }
>> +          if (!func.entrypc && q->dw.function_entrypc (&entrypc))
>>               {
>>                 func.entrypc = entrypc;
>>                 q->filtered_functions.push_back (func);
> I think this should be the other way around. q->dw.function_entrypc ()
> will normally do the right thing (and take DW_AT_entrypc into account,
> which might be important for some cases, even if it isn't currently used
> for the PPC64 ELF ABI v2 case). Then when you do get the func.entrypc
> and func.name you look the name up in the symbol table and adjust it if
> the entrypc matches the symbol value.
>
>> @@ -8154,6 +8166,26 @@ symbol_table::get_from_elf()
>>         reject = reject_section(section);
>>   #endif
>>   
>> +/*
>> + * For ELF ABI v2 on PPC64 LE, we need to adjust sym.st_value corresponding
>> + * to the bits of sym.st_other. These bits will tell us what's the offset
>> + * of the local entry point from the global entry point.
>> + */
>> +#if defined(_CALL_ELF) && _CALL_ELF == 2
> This won't work correctly when stap is ran remotely on a different
> architecture. That is not a very common situation, but I believe we do
> want to support that. So you will just have to always inspect the Elf
> ehdr.
>
>> +      Dwarf_Addr bias;
>> +      Elf* elf = (dwarf_getelf (dwfl_module_getdwarf (mod, &bias))
>> +             ?: dwfl_module_getelf (mod, &bias));
>> +
>> +      GElf_Ehdr ehdr_mem;
>> +      GElf_Ehdr* em = gelf_getehdr (elf, &ehdr_mem);
>> +      if (em == 0) { DWFL_ASSERT ("dwfl_getehdr", dwfl_errno()); }
>> +      assert(em);
>> +
>> +      if ((em->e_machine == EM_PPC64) && (GELF_ST_TYPE(sym.st_info) == STT_FUNC)
>> +        && sym.st_other)
>> +        addr = addr + PPC64_LOCAL_ENTRY_OFFSET(sym.st_other);
> Here you also want to have a check for EF_PPC64_ABI in e_flags then.
>
> Note that older elf.h files won't have those defines, so you should
> either check they are defined or explicitly define them here to make
> sure things will keep building on older systems.
>
>> +#endif
>> +
>>         if (name && GELF_ST_TYPE(sym.st_info) == STT_FUNC)
>>           add_symbol(name, (GELF_ST_BIND(sym.st_info) == STB_WEAK),
>>                      reject, addr, &high_addr);
> Thanks,
>
> Mark
>

Sent a v2 for this patch.

-- 
Thanks,
Hemant Kumar