This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] PR ld/19579: [Regression] link error linking fortran code with PIE
- From: "H.J. Lu" <hjl dot tools at gmail dot com>
- To: Alan Modra <amodra at gmail dot com>
- Cc: Binutils <binutils at sourceware dot org>
- Date: Tue, 8 Mar 2016 05:24:45 -0800
- Subject: Re: [PATCH] PR ld/19579: [Regression] link error linking fortran code with PIE
- Authentication-results: sourceware.org; auth=none
- References: <20160304134833 dot GA11350 at gmail dot com> <20160305015242 dot GE9617 at bubble dot grove dot modra dot org> <CAMe9rOq=mgfjaFgAdw=u3dzvtt3sG519Pipy4VyyPS1T3+GJSA at mail dot gmail dot com> <20160307152818 dot GH9617 at bubble dot grove dot modra dot org> <CAMe9rOpZh-Qem3Vt3vwsQG9K4T+9=Me3V_7QtW7_brc_pPo+Cg at mail dot gmail dot com> <20160308005840 dot GI9617 at bubble dot grove dot modra dot org>
On Mon, Mar 7, 2016 at 4:58 PM, Alan Modra <amodra@gmail.com> wrote:
> 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.
>
I am checking in this.
--
H.J.
---
diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 8c2da68..8823c47 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,9 @@
+2016-03-08 H.J. Lu <hongjiu.lu@intel.com>
+
+ PR ld/19579
+ * elflink.c (_bfd_elf_merge_symbol): Group common symbol checking
+ together.
+
2016-03-07 Nick Clifton <nickc@redhat.com>
PR binutils/19775
diff --git a/bfd/elflink.c b/bfd/elflink.c
index 39157bf..ccff780 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -1172,12 +1172,9 @@ _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
- || (!olddyn && newdyn && bfd_link_executable (info))));
+ && h->root.type != bfd_link_hash_common);
/* NEWFUNC and OLDFUNC indicate whether the new or old symbol,
respectively, appear to be a function. */
@@ -1489,13 +1486,16 @@ _bfd_elf_merge_symbol (bfd *abfd,
represent variables; this can cause confusion in principle, but
any such confusion would seem to indicate an erroneous program or
shared library. We also permit a common symbol in a regular
- object to override a weak symbol in a shared object. */
+ object to override a weak symbol in a shared object. A common
+ symbol in executable also overrides a symbol in a shared object. */
if (newdyn
&& newdef
&& (olddef
|| (h->root.type == bfd_link_hash_common
- && (newweak || newfunc))))
+ && (newweak
+ || newfunc
+ || (!olddyn && bfd_link_executable (info))))))
{
*override = TRUE;
newdef = FALSE;
gnu-6:pts/24[44]> vi bfd/elflink.c /export/gnu/import/git/sources/binutils-gdb
gnu-6:pts/24[45]> cat /tmp/x /export/gnu/import/git/sources/binutils-gdb
diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 8c2da68..8823c47 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,9 @@
+2016-03-08 H.J. Lu <hongjiu.lu@intel.com>
+
+ PR ld/19579
+ * elflink.c (_bfd_elf_merge_symbol): Group common symbol checking
+ together.
+
2016-03-07 Nick Clifton <nickc@redhat.com>
PR binutils/19775
diff --git a/bfd/elflink.c b/bfd/elflink.c
index 39157bf..ccff780 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -1172,12 +1172,9 @@ _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
- || (!olddyn && newdyn && bfd_link_executable (info))));
+ && h->root.type != bfd_link_hash_common);
/* NEWFUNC and OLDFUNC indicate whether the new or old symbol,
respectively, appear to be a function. */
@@ -1489,13 +1486,16 @@ _bfd_elf_merge_symbol (bfd *abfd,
represent variables; this can cause confusion in principle, but
any such confusion would seem to indicate an erroneous program or
shared library. We also permit a common symbol in a regular
- object to override a weak symbol in a shared object. */
+ object to override a weak symbol in a shared object. A common
+ symbol in executable also overrides a symbol in a shared object. */
if (newdyn
&& newdef
&& (olddef
|| (h->root.type == bfd_link_hash_common
- && (newweak || newfunc))))
+ && (newweak
+ || newfunc
+ || (!olddyn && bfd_link_executable (info))))))
{
*override = TRUE;
newdef = FALSE;