PATCH: Sign extend relocation result to 64bit for R_X86_64_DTPOFF64/R_X86_64_TPOFF64

[H.J. Lu]

On Tue, May 8, 2012 at 10:07 AM, Roland McGrath <roland@hack.frob.com> wrote:
> Actually, shouldn't reloc_addr actually be Elf64_Addr regardless?
> All the reloc types indicate 64-bit quantities except for those that
> already use *(unsigned int *) reloc_addr.

No, there is

ElfW(Addr) *const reloc_addr = reloc_addr_arg;

According to x32 psABI at

https://sites.google.com/site/x32abi/documents

most of relocations in x32 are applied to 32bit fields.  There are 4
exceptions. R_X86_64_DTPOFF64/R_X86_64_TPOFF64 relocations
for GOT are 2 of them.  X32/x86-64 TLS data structure uses negative
offset.  That is why sign-extension is needed here.  Otherwise,
their GOT entries will be wrong.

> The high bits will be zero because the st_value and r_addend types are
> smaller, but that's no reason not to write the full 64 bits for all the
> 64-bit reloc types.
>
>> +#   ifdef __ILP32__
>> +         *(Elf64_Sxword *) reloc_addr
>> +           = (Elf64_Sxword)
>> +               ((Elf32_Sword) (sym->st_value + reloc->r_addend));
>> +#   else
>>           *reloc_addr = sym->st_value + reloc->r_addend;
>> +#   endif
>
> Put the calculation in a temporary rather than repeating it.
> (Same below.)
>
> Then it can be just:
>
>        *reloc_addr = (Elf64_Sxword) (Elf32_Sword) value;

Not all relocations use this computation.  I don't think we
should compute a value which is only used by a few relocations.

> But it needs a comment saying why this particular reloc type gets sign
> extension when all the others get zero extension.
>

Here is the updated patch with comments.  OK for trunk?

Thanks.


-- 
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: libc-x32-tls-2.patch
Type: application/octet-stream
Size: 1476 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20120508/47ddf2a2/attachment.obj>