Ping?
On 3 January 2017 at 16:45, Christophe Lyon <christophe.l...@linaro.org> wrote: > 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.l...@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.l...@linaro.org> >> wrote: >>> Ping? >>> >>> On 1 December 2016 at 15:27, Christophe Lyon <christophe.l...@linaro.org> >>> wrote: >>>> Hi, >>>> >>>> >>>> On 10 November 2016 at 15:10, Christophe Lyon >>>> <christophe.l...@linaro.org> wrote: >>>>> On 10 November 2016 at 11:05, Richard Earnshaw >>>>> <richard.earns...@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 >>>>>>> >>>>>>