This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [ARM] PR 78253 do not resolve weak ref locally


Ping?

The patch is at https://gcc.gnu.org/ml/gcc-patches/2016-12/msg00078.html


On 14 December 2016 at 16:29, Christophe Lyon
<christophe.lyon@linaro.org> wrote:
> 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
>>>>>>
>>>>>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]