This is the mail archive of the binutils@sourceware.org mailing list for the binutils 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: [PATCH] PR ld/19579: [Regression] link error linking fortran code with PIE


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;


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