Thierry, will you be able to address these last nits? It's the last
hump before I can merge this without breaking compilation on hosts that
don't know about ELFv2 ABI yet.
Thank you,
PM
Petr Machata <pmach...@redhat.com> writes:
> This looks all pretty good. I compile-tested it on plain PPC64 --it's
> desirable to keep ltrace building on old toolchains as well-- and hit a
> couple problems with the several new symbols that you use. I think we
> should have a fallback that disables the new code if the definitions are
> just unavailable.
>
>> diff --git a/sysdeps/linux-gnu/ppc/fetch.c b/sysdeps/linux-gnu/ppc/fetch.c
>> index ed38336..b42cd95 100644
>> --- a/sysdeps/linux-gnu/ppc/fetch.c
>> +++ b/sysdeps/linux-gnu/ppc/fetch.c
> [...]
>>
>> +static int
>> +get_return_info(struct arg_type_info *info, struct process *proc,
>> + struct fetch_context *context)
>
> For non-ELFv2 host, I'm getting:
> sysdeps/linux-gnu/ppc/fetch.c:119:1: error: 'get_return_info' defined but not
> used [-Werror=unused-function]
> sysdeps/linux-gnu/ppc/fetch.c:392:1: error: 'allocate_hfa' defined but not
> used [-Werror=unused-function]
>
>> diff --git a/sysdeps/linux-gnu/ppc/plt.c b/sysdeps/linux-gnu/ppc/plt.c
>> index 332daa8..a16e182 100644
>> --- a/sysdeps/linux-gnu/ppc/plt.c
>> +++ b/sysdeps/linux-gnu/ppc/plt.c
> [...]
>> @@ -430,7 +448,12 @@ reloc_copy_if_irelative(GElf_Rela *rela, void *data)
>> int
>> arch_elf_init(struct ltelf *lte, struct library *lib)
>> {
>> +
>> + /* Check for ABIv2 in ELF header processor specific flag. */
>> + lte->arch.elfv2_abi = ((lte->ehdr.e_flags & EF_PPC64_ABI) == 2);
>> +
>
> So, I don't think we should turn off detection. It seems appropriate to
> me to do something like:
>
> +#ifndef EF_PPC64_ABI
> +# define EF_PPC64_ABI 3
> +#endif
>
> somewhere around here (or possibly at the top of this file). If
> EF_PPC64_ABI is undefined, and we detect elfv2_abi, we should bail out
> with a message about unsupported ABI.
>
>> @@ -629,9 +652,45 @@ arch_elf_add_func_entry(struct process *proc, struct
>> ltelf *lte,
>> arch_addr_t addr, const char *name,
>> struct library_symbol **ret)
>> {
>> - if (lte->ehdr.e_machine != EM_PPC || lte->ehdr.e_type == ET_DYN)
>> + /* With ABIv2 st_other field contains an offset. */
>> + if (lte->arch.elfv2_abi)
>> + addr += PPC64_LOCAL_ENTRY_OFFSET(sym->st_other);
>
> Similar problem. This should be ifdef'd away I think. I.e. something
> like:
>
> #ifndef PPC64_LOCAL_ENTRY_OFFSET
> assert(! lte->arch.elfv2_abi);
> #else
> if (lte->arch.elfv2_abi)
> addr += ...
> #endif
_______________________________________________
Ltrace-devel mailing list
Ltrace-devel@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/ltrace-devel
