Re: [PATCH] PR ld/19636: [x86] Resolve undefweak and defined symbols in executable

[Jiong Wang <jiong dot wang at foss dot arm dot com>]

On 02/03/16 12:50, H.J. Lu wrote:
> On Wed, Mar 2, 2016 at 4:28 AM, Jiong Wang <jiong.wang@foss.arm.com> wrote:
> > On 02/03/16 12:22, H.J. Lu wrote:
> > > On Wed, Mar 2, 2016 at 4:03 AM, Jiong Wang <jiong.wang@foss.arm.com>
> > > wrote:
> > > > On 01/03/16 14:37, H.J. Lu wrote:
> > > > > On Tue, Mar 1, 2016 at 6:02 AM, Kyrill Tkachov
> > > > > <kyrylo.tkachov@foss.arm.com> wrote:
> > > > > > Hi HJ,
> > > > > >
> > > > > > On 26/02/16 12:51, H.J. Lu wrote:
> > > > > > > On Thu, Feb 25, 2016 at 10:59 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > > > > > Here is the updated patch I am testing.  The linker behavior is
> > > > > > > > changed
> > > > > > > > in 2 cases when creating executable:
> > > > > > > >
> > > > > > > > 1. When there are mixed PIC and non-PIC references to undefined
> > > > > > > > weak symbols, undefined weak symbols are resolved to 0 at link-time.
> > > > > > > > 2. If all references to undefined weak symbols are PIC, dynamic
> > > > > > > > relocations against undefined weak symbols will be generated unless
> > > > > > > > -z nodynamic-undefined-weak is passed to linker.
> > > > > > > >
> > > > > > > > BTW,  We have to resolve R_X86_64_32/R_X86_64_PC32 relocations
> > > > > > > > against undefined weak symbols to zero.  Otherwise, we will get
> > > > > > > > run-time
> > > > > > > > relocation overflow for dynamic R_X86_64_32/R_X86_64_PC32
> > > > > > > > relocations.
> > > > > > > This is what I am checking in.
> > > > > > I'm seeing:
> > > > > >
> > > > > > NA->FAIL: Mixing PIC and non-PIC
> > > > > > on aarch64-none-linux-gnu.
> > > > > You can either fix aarch64 backend or skip the test for aarch64.
> > > >
> > > > H.J,
> > > >
> > > >     For your testcase, AArch64 is not generating dynamic relocation for
> > > >     weak undefined symbol referenced from non-pic code when linking
> > > >     exectuable, instead, it's resolved to zero during static linking
> > > > stage.
> > > >     As far as I know, this behavior is exactly what's described here at
> > > >
> > > >       https://sourceware.org/ml/binutils/2008-04/msg00269.html
> > > >
> > > >     And reading those historical discussions,
> > > >
> > > >      https://sourceware.org/ml/binutils/2008-04/msg00032.html
> > > >      https://sourceware.org/ml/binutils/2008-02/msg00264.html
> > > >
> > > >     Looks to me the ld behavior changes introduced by your patch is quite
> > > >     sensitive and there still be lack of consensus.
> > > What linker change were you referring to?  I only added a testcase.
> >
> > I mean those linker changes added together with this testcase.
> >
> > commit aec6b87e0b66d707ead62ca40d220ee78b4cf5a5
> > Author: H.J. Lu <hjl.tools@gmail.com>
> > Date:   Fri Feb 26 04:16:15 2016 -0800
> >
> >      [x86] Resolve non-PIC undefweak symbols in executable
> As far as aarch64 backend is concerned, I only added a testcase.

Well, then why you put it under generic directory? and without 
restricting it
on x86? this is implicitly affect all targets, and enforcing all targets 
to follow
the changes on x86.  I think similar changes should only be encouraged 
to other target
if it's conventional rules, or it's clearly documented by generic or 
that target's ELF
specification.

While reading from http://www.skyfree.org/linux/references/ELF_Format.pdf,

  "The link editor does not extract archive members to resolve undefined
  weak symbols. Unresolved weak symbols have a zero value."

Looks to me the spec is even more strict that weak symbol's life is 
defined to be ended
after static linking stage. All unresolved weak symbols are assigned 
zero value.

IMO, the support of weak symbol under various rare and complex scenarios 
are very
target specific, thus I'd either move this testcase under x86 directory 
or put it under
generic directory but enabling it on x86 only initially.  If other 
targets want and start
to support similar features like x86 on weak symbol, then they can be 
enabled seperately.
This looks to me is a more clean & acceptable way to other targets.

Thanks.

Regards,
Jiong