This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH 2/2] bfd/elfnn-aarch64.c: Handle static links with ifunc correctly.
- From: Will Newton <will dot newton at linaro dot org>
- To: Yufeng Zhang <Yufeng dot Zhang at arm dot com>
- Cc: "binutils at sourceware dot org" <binutils at sourceware dot org>, Patch Tracking <patches at linaro dot org>
- Date: Tue, 26 Nov 2013 11:16:12 +0000
- Subject: Re: [PATCH 2/2] bfd/elfnn-aarch64.c: Handle static links with ifunc correctly.
- Authentication-results: sourceware.org; auth=none
- References: <529461B1 dot 8080200 at linaro dot org> <529480BD dot 40707 at arm dot com>
On 26 November 2013 11:06, Yufeng Zhang <Yufeng.Zhang@arm.com> wrote:
> On 11/26/13 08:54, Will Newton wrote:
>>
>> The code for handling GOT references to ifunc symbols in static links
>> was missing.
>>
>> bfd/ChangeLog:
>>
>> 2013-11-25 Will Newton<will.newton@linaro.org>
>>
>> * elfnn-aarch64.c (elfNN_aarch64_finish_dynamic_symbol):
>> Handle STT_GNU_IFUNC symbols correctly in static links.
>>
>> 2013-11-25 Will Newton<will.newton@linaro.org>
>>
>> * ld-aarch64/aarch64-elf.exp: Add ifunc-22.
>> * ld-aarch64/ifunc-22.d: New file.
>> * ld-aarch64/ifunc-22.s: Likewise.
>> ---
>> bfd/elfnn-aarch64.c | 30
>> +++++++++++++++++++++++++++++-
>> ld/testsuite/ld-aarch64/aarch64-elf.exp | 1 +
>> ld/testsuite/ld-aarch64/ifunc-22.d | 11 +++++++++++
>> ld/testsuite/ld-aarch64/ifunc-22.s | 14 ++++++++++++++
>> 4 files changed, 55 insertions(+), 1 deletion(-)
>> create mode 100644 ld/testsuite/ld-aarch64/ifunc-22.d
>> create mode 100644 ld/testsuite/ld-aarch64/ifunc-22.s
>>
>> OK for trunk and binutils_2_24-branch?
>>
>> diff --git a/bfd/elfnn-aarch64.c b/bfd/elfnn-aarch64.c
>> index 7cce6f4..1467f5d 100644
>> --- a/bfd/elfnn-aarch64.c
>> +++ b/bfd/elfnn-aarch64.c
>> @@ -6824,7 +6824,34 @@ elfNN_aarch64_finish_dynamic_symbol (bfd
>> *output_bfd,
>> + htab->root.sgot->output_offset
>> + (h->got.offset& ~(bfd_vma) 1));
>>
>> - if (info->shared&& SYMBOL_REFERENCES_LOCAL (info, h))
>>
>> + if (h->def_regular
>> + && h->type == STT_GNU_IFUNC)
>> + {
>> + if (info->shared)
>> + {
>> + /* Generate R_AARCH64_GLOB_DAT. */
>> + goto do_glob_dat;
>> + }
>
>
> Can the control flow be optimized so that the outer if condition also checks
> !info->shared? I wonder whether the goto statement be avoided.
It would involve altering the else if condition somewhat so I am
inclined to leave as is.
I am aware that the bfd copy and paste model of code reuse is rather
unsatisfactory, but sometimes I think it is better to have the same
bugs as everyone else rather than different ones! And I also I have
dreams that one day some kindly bfd hacker will come along and pull
some of this stuff out into common code and at that point the
similarity between ports will make that job easier.
> + if (h->def_regular
> + && h->type == STT_GNU_IFUNC
> + && !info->shared )
>
>
>
>> + else
>> + {
>> + asection *plt;
>> +
>> + if (!h->pointer_equality_needed)
>> + abort ();
>> +
>> + /* For non-shared object, we can't use .got.plt, which
>> + contains the real function addres if we need pointer
>
>
> addres/address
Thanks, fixed.
>> + equality. We load the GOT entry with the PLT entry. */
>> + plt = htab->root.splt ? htab->root.splt : htab->root.iplt;
>> + bfd_put_NN (output_bfd, (plt->output_section->vma
>> + + plt->output_offset
>> + + h->plt.offset),
>> + htab->root.sgot->contents
>> + + (h->got.offset& ~(bfd_vma) 1));
>>
>> + return TRUE;
>> + }
>> + }
>> + else if (info->shared&& SYMBOL_REFERENCES_LOCAL (info, h))
>>
>> {
>> if (!h->def_regular)
>> return FALSE;
>> @@ -6838,6 +6865,7 @@ elfNN_aarch64_finish_dynamic_symbol (bfd
>> *output_bfd,
>> else
>> {
>> BFD_ASSERT ((h->got.offset& 1) == 0);
>>
>> +do_glob_dat:
>> bfd_put_NN (output_bfd, (bfd_vma) 0,
>> htab->root.sgot->contents + h->got.offset);
>> rela.r_info = ELFNN_R_INFO (h->dynindx, AARCH64_R (GLOB_DAT));
>
>
> Is do_glob_dat placed deliberately after the assertion?
I don't know. But I share your concern about this code, I'll move the
label up above the assert as I don't see a case where the assert could
fail but the code below remain valid.
--
Will Newton
Toolchain Working Group, Linaro