Re: PATCH: PR ld/13302: IRELATIVE relocation should come last

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

On Tue, Mar 6, 2012 at 1:00 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> On Fri, Oct 21, 2011 at 12:49:12PM -0700, H.J. Lu wrote:
>> Here is a patch to replace IRELATIVE with RELATIVE in .rel.dyn. We now
>> use PLT entry, instead of real function, for non-GOT reference to locally
>> defined IFUNC symbols.
>
>> --- a/bfd/ChangeLog
>> +++ b/bfd/ChangeLog
>> @@ -1,3 +1,12 @@
>> +2011-10-21 ?H.J. Lu ?<hongjiu.lu@intel.com>
>> +
>> + ? ? PR ld/13302
>> + ? ? * elf32-i386.c (elf_i386_relocate_section): Replace
>> + ? ? R_386_IRELATIVE with R_386_RELATIVE.
>> +
>> + ? ? * elf64-x86-64.c (elf_x86_64_relocate_section): Replace
>> + ? ? R_X86_64_IRELATIVE with R_X86_64_RELATIVE.
>
> This patch apparently broke prelink, which on x86_64 expects that the added
> for R_X86_64_RELATIVE is always applied (i.e. the same number is in both
> addend and memory and it normally is for all other kinds of
> R_X86_64_RELATIVE, just not this spot now).
>
>> --- a/bfd/elf64-x86-64.c
>> +++ b/bfd/elf64-x86-64.c
>> @@ -3161,6 +3161,7 @@ elf_x86_64_relocate_section (bfd *output_bfd,
>> ? ? ? ? ? ? ? {
>> ? ? ? ? ? ? ? ? Elf_Internal_Rela outrel;
>> ? ? ? ? ? ? ? ? asection *sreloc;
>> + ? ? ? ? ? ? ? bfd_boolean relocate;
>>
>> ? ? ? ? ? ? ? ? /* Need a dynamic relocation to get the real function
>> ? ? ? ? ? ? ? ? ? ?address. ?*/
>> @@ -3180,15 +3181,15 @@ elf_x86_64_relocate_section (bfd *output_bfd,
>> ? ? ? ? ? ? ? ? ? ? || info->executable)
>> ? ? ? ? ? ? ? ? ? {
>> ? ? ? ? ? ? ? ? ? ? /* This symbol is resolved locally. ?*/
>> - ? ? ? ? ? ? ? ? ? outrel.r_info = htab->r_info (0, R_X86_64_IRELATIVE);
>> - ? ? ? ? ? ? ? ? ? outrel.r_addend = (h->root.u.def.value
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?+ h->root.u.def.section->output_section->vma
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?+ h->root.u.def.section->output_offset);
>> + ? ? ? ? ? ? ? ? ? outrel.r_info = htab->r_info (0, R_X86_64_RELATIVE);
>> + ? ? ? ? ? ? ? ? ? outrel.r_addend = relocation;
>> + ? ? ? ? ? ? ? ? ? relocate = FALSE;
>> ? ? ? ? ? ? ? ? ? }
>> ? ? ? ? ? ? ? ? else
>> ? ? ? ? ? ? ? ? ? {
>> ? ? ? ? ? ? ? ? ? ? outrel.r_info = htab->r_info (h->dynindx, r_type);
>> ? ? ? ? ? ? ? ? ? ? outrel.r_addend = 0;
>> + ? ? ? ? ? ? ? ? ? relocate = FALSE;
>> ? ? ? ? ? ? ? ? ? }
>>
>> ? ? ? ? ? ? ? ? sreloc = htab->elf.irelifunc;
>> @@ -3199,7 +3200,8 @@ elf_x86_64_relocate_section (bfd *output_bfd,
>> ? ? ? ? ? ? ? ? ? ?we need to include the symbol value so that it
>> ? ? ? ? ? ? ? ? ? ?becomes an addend for the dynamic reloc. ?For an
>> ? ? ? ? ? ? ? ? ? ?internal symbol, we have updated addend. ?*/
>> - ? ? ? ? ? ? ? continue;
>> + ? ? ? ? ? ? ? if (! relocate)
>> + ? ? ? ? ? ? ? ? continue;
>> ? ? ? ? ? ? ? }
>> ? ? ? ? ? ? /* FALLTHROUGH */
>> ? ? ? ? ? case R_X86_64_PC32:
>
> The above looks like typo to me, if relocate was intentionally
> always false, why add the boolean at all? ?Here is a fix. ?Ok for trunk?
>
> 2012-03-06 ?Jakub Jelinek ?<jakub@redhat.com>
>
> ? ? ? ?* elf64-x86-64.c (elf_x86_64_relocate_section): For R_X86_64_RELATIVE
> ? ? ? ?set relocate to TRUE.
>
> --- bfd/elf64-x86-64.c.jj ? ? ? 2012-02-08 22:12:43.000000000 +0100
> +++ bfd/elf64-x86-64.c ?2012-03-06 09:52:16.760752214 +0100
> @@ -1,6 +1,6 @@
> ?/* X86-64 specific support for ELF
> ? ?Copyright 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009,
> - ? 2010, 2011
> + ? 2010, 2011, 2012
> ? ?Free Software Foundation, Inc.
> ? ?Contributed by Jan Hubicka <jh@suse.cz>.
>
> @@ -3178,7 +3178,7 @@ elf_x86_64_relocate_section (bfd *output
> ? ? ? ? ? ? ? ? ? ? ?/* This symbol is resolved locally. ?*/
> ? ? ? ? ? ? ? ? ? ? ?outrel.r_info = htab->r_info (0, R_X86_64_RELATIVE);
> ? ? ? ? ? ? ? ? ? ? ?outrel.r_addend = relocation;
> - ? ? ? ? ? ? ? ? ? ? relocate = FALSE;
> + ? ? ? ? ? ? ? ? ? ? relocate = TRUE;
> ? ? ? ? ? ? ? ? ? ?}
> ? ? ? ? ? ? ? ? ?else
> ? ? ? ? ? ? ? ? ? ?{
>
>
> ? ? ? ?Jakub

It is a typo.  OK for trunk.  I think it should also be applied on 2.22 branch.

Thanks.


-- 
H.J.