[PATCH] PR ld/19579: [Regression] link error linking fortran code with PIE

Alan Modra amodra@gmail.com
Tue Mar 8 00:58:00 GMT 2016


On Mon, Mar 07, 2016 at 10:44:29AM -0800, H.J. Lu wrote:
> On Mon, Mar 7, 2016 at 7:28 AM, Alan Modra <amodra@gmail.com> wrote:
> > On Fri, Mar 04, 2016 at 07:41:42PM -0800, H.J. Lu wrote:
> >> On Fri, Mar 4, 2016 at 5:52 PM, Alan Modra <amodra@gmail.com> wrote:
> >> > On Fri, Mar 04, 2016 at 05:48:33AM -0800, H.J. Lu wrote:
> >> >> --- a/bfd/elflink.c
> >> >> +++ b/bfd/elflink.c
> >> >> @@ -1172,9 +1172,12 @@ _bfd_elf_merge_symbol (bfd *abfd,
> >> >>
> >> >>    newdef = !bfd_is_und_section (sec) && !bfd_is_com_section (sec);
> >> >>
> >> >> +  /* The old common symbol in executable is a definition if the new
> >> >> +     definition comes from a shared library.  */
> >> >>    olddef = (h->root.type != bfd_link_hash_undefined
> >> >>           && h->root.type != bfd_link_hash_undefweak
> >> >> -         && h->root.type != bfd_link_hash_common);
> >> >> +         && (h->root.type != bfd_link_hash_common
> >> >> +             || (!olddyn && newdyn && bfd_link_executable (info))));
> >> >>
> >> >>    /* NEWFUNC and OLDFUNC indicate whether the new or old symbol,
> >> >>       respectively, appear to be a function.  */
> >> >
> >> > Why is this the correct place to change, and not code after the
> >> > comment "We treat a common symbol as a definition"?
> >>
> >> olddef has been checked well before that.
> >
> > And do any of those matter?
[snip TLS example]
I don't think an error message difference is particularly relevant.

> >> We need to get it right.
> >
> > That's why I asked.  You haven't yet replied with anything more than a
> > superficial reason for not moving the change to where it ought to go,
> > I think.
> 
> The old common symbol is a definition in this case.  Why shouldn't
> olddef set to yes?

A common is *not* a definition.  We treat a common *as if* it were a
definition in some cases.

The distinction might be subtle, but I'm concerned that in some future
change to _bfd_elf_merge_symbol someone might think that olddef being
true really means we have a definition.

Also, we now have two places in _bfd_elf_merge_symbol where we modify
the code to treat a common as a definition.  I'd rather have just one
place, again for maintainability reasons.

-- 
Alan Modra
Australia Development Lab, IBM



More information about the Binutils mailing list