This is the mail archive of the
mailing list for the binutils project.
Re: [PATCH, ARM] Fix handling of GOT and PLT access to IFUNC symbols
- From: Will Newton <will dot newton at linaro dot org>
- To: Richard Earnshaw <rearnsha at arm dot com>, Will Newton <will dot newton at linaro dot org>, "binutils at sourceware dot org" <binutils at sourceware dot org>, rdsandiford at googlemail dot com
- Date: Wed, 24 Apr 2013 09:59:09 +0100
- Subject: Re: [PATCH, ARM] Fix handling of GOT and PLT access to IFUNC symbols
- References: <5168395D dot 5070406 at linaro dot org> <516C1A3A dot 5090004 at arm dot com> <87bo95jfpt dot fsf at talisman dot default>
On 23 April 2013 18:53, Richard Sandiford <email@example.com> wrote:
> Richard Earnshaw <firstname.lastname@example.org> writes:
>> On 12/04/13 17:42, Will Newton wrote:
>>> Hi all,
>>> The current ARM IFUNC code appears to have a bug when an access is made via
>>> the PLT and GOT in the same object. This results in two relocs being swapped
>>> out into the same slot so one R_ARM_IRELATIVE reloc goes missing.
>>> This patch changes the behaviour to use an incremented reloc count rather
>>> than the calculated PLT index to match the behaviour of elf32_arm_add_dynreloc
>>> and as a result requires the order of relocs in the ifunc tests to be
>>> 2013-04-12 Will Newton <email@example.com>
>>> * elf32-arm.c (elf32_arm_populate_plt_entry): Increment reloc_count
>>> when emitting R_ARM_IRELATIVE relocs.
>> Hmm, it looks somewhat suspicious to me to be changing the reloc count
>> this late on.
>> Richard, do you know if this is safe? If not, can you recommend a
>> better approach?
> Sorry for the slow reply, been on holiday. The patch looks good to me.
> At this stage, ->reloc_count is the number of relocations that have been
> filled in, whereas ->size is the number of bytes that have been allocated
> in the relocation section. .rel.plt is special in that the relocation
> index has to match the PLT index, but as Will says, .rel.iplt is more
> like a normal relocation section, and can have a mixture of both "PLT"
> and non-PLT relocations.
> In some ways it might be clearer to use elf32_arm_add_dynreloc in
> the dynindx == -1 case, which would also give us the assert that
> we have allocated enough relocation entries. Both should be
> correct though.
> BTW, there's a typo in the testcase:
>>+#------ libfunc1's .iplt entry
> should be "appfunc1".
Thanks for the review! I'll post a new version of the patch with these changes.
Toolchain Working Group, Linaro