Re: PATCH: Handle R_X86_64_RELATIVE64 and R_X86_64_64 for x32

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Thu, May 10, 2012 at 1:42 PM, Roland McGrath <roland@hack.frob.com> wrote:
>> +# if !defined RTLD_BOOTSTRAP && defined __ILP32__
>> + ?/* l_addr + r_addend may be > 0xffffffff and R_X86_64_RELATIVE64
>> + ? ? relocation updates the whole 64-bit entry. ?*/
>> + ?if (__builtin_expect (r_type == R_X86_64_RELATIVE64, 0))
>> + ? ?*(Elf64_Addr *) reloc_addr = (Elf64_Addr) map->l_addr + reloc->r_addend;
>> + ?else
>> +# endif
>
> There's no reason x86_64 proper shouldn't handle R_X86_64_RELATIVE64 too.

I will do that.

> So drop the __ILP32__ condition. ?Would it really be a problem for
> R_X86_64_RELATIVE to be treated as a whole 64 bits for x32? ?If not, then
> you can just merge this into the existing case by making the test accept
> either code.

R_X86_64_RELATIVE is generated for ".long foo" in x32 and it updates
a 32-bit field.

>
>> +# ?ifdef __ILP32__
>> + ? ? ? /* value + r_addend may be > 0xffffffff and R_X86_64_64
>> + ? ? ? ? ?relocation updates the whole 64-bit entry. ?*/
>> + ? ? ? *(Elf64_Addr *) reloc_addr = (Elf64_Addr) value + reloc->r_addend;
>> +# ?else
>> ? ? ? ? *reloc_addr = value + reloc->r_addend;
>> +# ?endif
>
> Likewise here, you can drop the #ifdef and just use the version with the
> casts for eitehr case.
>
>> +#ifdef __ILP32__
>> + ?/* l_addr + r_addend may be > 0xffffffff and R_X86_64_RELATIVE64
>> + ? ? relocation updates the whole 64-bit entry. ?*/
>> + ?if (__builtin_expect (ELFW(R_TYPE) (reloc->r_info) == R_X86_64_RELATIVE64, 0))
>> + ? ?{
>> + ? ? ?*(Elf64_Addr *) reloc_addr = (Elf64_Addr) l_addr + reloc->r_addend;
>> + ? ? ?return;
>> + ? ?}
>> +#endif
>> ? ?assert (ELFW(R_TYPE) (reloc->r_info) == R_X86_64_RELATIVE);
>> ? ?*reloc_addr = l_addr + reloc->r_addend;
>
> Again, unless there's a real problem with the 64-bit treatment for x32,
> then just make the assert accept either type.
>

I will make the change,

Thanks.


-- 
H.J.