This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [ARM] PR 78253 do not resolve weak ref locally
- From: Christophe Lyon <christophe dot lyon at linaro dot org>
- To: Richard Earnshaw <Richard dot Earnshaw at foss dot arm dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 14 Dec 2016 16:29:08 +0100
- Subject: Re: [ARM] PR 78253 do not resolve weak ref locally
- Authentication-results: sourceware.org; auth=none
- References: <CAKdteOY57XHHGSKxTuWesTypp7e-8HS6HBLqcvP50FEssQfqcw@mail.gmail.com> <22e91ae6-7a21-3fd6-c85f-7021571b0c5e@foss.arm.com> <CAKdteOa5nH0a1u1uq0c2P10JLROPjheoTPKfGL=3YR0h3KKB8A@mail.gmail.com> <CAKdteOYPj7mCUD3GEYhriqXgLMMP4WFFF3U-TB_kVF_ATRdKWw@mail.gmail.com> <CAKdteOZpbhP9qm059yksRc0KD1x+r6pzpaPrLUoObA9zXkCUcg@mail.gmail.com>
Ping^2 ?
As a reminder, this patch mimics what aarch64 does wrt to references to weak
symbols such that they are not resolved by the assembler, in case a strong
definition overrides the local one at link time.
Christophe
On 8 December 2016 at 09:17, Christophe Lyon <christophe.lyon@linaro.org> wrote:
> Ping?
>
> On 1 December 2016 at 15:27, Christophe Lyon <christophe.lyon@linaro.org> wrote:
>> Hi,
>>
>>
>> On 10 November 2016 at 15:10, Christophe Lyon
>> <christophe.lyon@linaro.org> wrote:
>>> On 10 November 2016 at 11:05, Richard Earnshaw
>>> <Richard.Earnshaw@foss.arm.com> wrote:
>>>> On 09/11/16 21:29, Christophe Lyon wrote:
>>>>> Hi,
>>>>>
>>>>> PR 78253 shows that the handling of weak references has changed for
>>>>> ARM with gcc-5.
>>>>>
>>>>> When r220674 was committed, default_binds_local_p_2 gained a new
>>>>> parameter (weak_dominate), which, when true, implies that a reference
>>>>> to a weak symbol defined locally will be resolved locally, even though
>>>>> it could be overridden by a strong definition in another object file.
>>>>>
>>>>> With r220674, default_binds_local_p forces weak_dominate=true,
>>>>> effectively changing the previous behavior.
>>>>>
>>>>> The attached patch introduces default_binds_local_p_4 which is a copy
>>>>> of default_binds_local_p_2, but using weak_dominate=false, and updates
>>>>> the ARM target to call default_binds_local_p_4 instead of
>>>>> default_binds_local_p_2.
>>>>>
>>>>> I ran cross-tests on various arm* configurations with no regression,
>>>>> and checked that the test attached to the original bugzilla now works
>>>>> as expected.
>>>>>
>>>>> I am not sure why weak_dominate defaults to true, and I couldn't
>>>>> really understand why by reading the threads related to r220674 and
>>>>> following updates to default_binds_local_p_* which all deal with other
>>>>> corner cases and do not discuss the weak_dominate parameter.
>>>>>
>>>>> Or should this patch be made more generic?
>>>>>
>>>>
>>>> I certainly don't think it should be ARM specific.
>>> That was my feeling too.
>>>
>>>>
>>>> The questions I have are:
>>>>
>>>> 1) What do other targets do today. Are they the same, or different?
>>>
>>> arm, aarch64, s390 use default_binds_local_p_2 since PR 65780, and
>>> default_binds_local_p before that. Both have weak_dominate=true
>>> i386 has its own version, calling default_binds_local_p_3 with true
>>> for weak_dominate
>>>
>>> But the behaviour of default_binds_local_p changed with r220674 as I said above.
>>> See https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=220674 and
>>> notice how weak_dominate was introduced
>>>
>>> The original bug report is about a different case:
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=32219
>>>
>>> The original patch submission is
>>> https://gcc.gnu.org/ml/gcc-patches/2015-02/msg00410.html
>>> and the 1st version with weak_dominate is in:
>>> https://gcc.gnu.org/ml/gcc-patches/2015-02/msg00469.html
>>> but it's not clear to me why this was introduced
>>>
>>>> 2) If different why?
>>> on aarch64, although binds_local_p returns true, the relocations used when
>>> building the function pointer is still the same (still via the GOT).
>>>
>>> aarch64 has different logic than arm when accessing a symbol
>>> (eg aarch64_classify_symbol)
>>>
>>>> 3) Is the current behaviour really what was intended by the patch? ie.
>>>> Was the old behaviour actually wrong?
>>>>
>>> That's what I was wondering.
>>> Before r220674, calling a weak function directly or via a function
>>> pointer had the same effect (in other words, the function pointer
>>> points to the actual implementation: the strong one if any, the weak
>>> one otherwise).
>>>
>>> After r220674, on arm the function pointer points to the weak
>>> definition, which seems wrong to me, it should leave the actual
>>> resolution to the linker.
>>>
>>>
>>
>> After looking at the aarch64 port, I think that references to weak symbols
>> have to be handled carefully, to make sure they cannot be resolved
>> by the assembler, since the weak symbol can be overridden by a strong
>> definition at link-time.
>>
>> Here is a new patch which does that.
>> Validated on arm* targets with no regression, and I checked that the
>> original testcase now executes as expected.
>>
>> Christophe
>>
>>
>>>> R.
>>>>> Thanks,
>>>>>
>>>>> Christophe
>>>>>
>>>>