[PATCH] PR ld/19579/21306 Properly turn common symbol into definition

[PATCH] PR ld/19579/21306 Properly turn common symbol into definition

[H.J. Lu]

Common symbols in executable should be overridden by definitions in
shared objects, similar to definitions in relocatable objects.  But
when common symbols in executable override common symbols in shared
objects, we must make sure that def_regular is set so that ELF linker
sees it as a definition.

Tested with mopac7 on x86-64.  OK for master?


H.J.
---
bfd/

	PR ld/19579
	PR ld/21306
	* elf-bfd.h (_bfd_elf_define_common_symbol): New prototype.
	* elflink.c (_bfd_elf_merge_symbol): Revert commits
	202ac193bbbecc96a4978d1ac3d17148253f9b01 and
	07492f668d2173da7a2bda3707ff0985e0f460b6.
	(_bfd_elf_define_common_symbol): New function.
	* elfxx-target.h (bfd_elfNN_bfd_define_common_symbol): Default
	to _bfd_elf_define_common_symbol.

ld/

	PR ld/19579
	PR ld/21306
	* testsuite/ld-elf/pr19579a.c (main): Updated.

---
 bfd/elf-bfd.h                  |  3 +++
 bfd/elflink.c                  | 29 ++++++++++++++++++++++++-----
 bfd/elfxx-target.h             |  2 +-
 ld/testsuite/ld-elf/pr19579a.c |  2 +-
 4 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index af377ee..5843d55 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -2365,6 +2365,9 @@ extern bfd_boolean bfd_elf_link_record_dynamic_symbol
 extern int bfd_elf_link_record_local_dynamic_symbol
   (struct bfd_link_info *, bfd *, long);
 
+extern bfd_boolean _bfd_elf_define_common_symbol
+  (bfd *, struct bfd_link_info *, struct bfd_link_hash_entry *);
+
 extern bfd_boolean _bfd_elf_close_and_cleanup
   (bfd *);
 
diff --git a/bfd/elflink.c b/bfd/elflink.c
index 9bf75c8..0f1aab7 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -1544,16 +1544,13 @@ _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.  A common
-     symbol in executable also overrides a symbol in a shared object.  */
+     object to override a weak symbol in a shared object.  */
 
   if (newdyn
       && newdef
       && (olddef
 	  || (h->root.type == bfd_link_hash_common
-	      && (newweak
-		  || newfunc
-		  || (!olddyn && bfd_link_executable (info))))))
+	      && (newweak || newfunc))))
     {
       *override = TRUE;
       newdef = FALSE;
@@ -14122,3 +14119,25 @@ elf_append_rel (bfd *abfd, asection *s, Elf_Internal_Rela *rel)
   BFD_ASSERT (loc + bed->s->sizeof_rel <= s->contents + s->size);
   bed->s->swap_reloc_out (abfd, rel, loc);
 }
+
+/* Convert common symbol H into a defined symbol.  Return TRUE on
+   success and FALSE on failure.  */
+
+bfd_boolean
+_bfd_elf_define_common_symbol (bfd *output_bfd,
+			       struct bfd_link_info *info,
+			       struct bfd_link_hash_entry *h)
+{
+  bfd_boolean ret
+    = bfd_generic_define_common_symbol (output_bfd, info, h);
+  if (ret)
+    {
+      /* Since def_regular may not be set if it is overridden by a
+	 dynamic definition, we need to set def_regular when it is
+	 converted to a defined symbol.  */
+      struct elf_link_hash_entry *eh
+	= (struct elf_link_hash_entry *) h;
+      eh->def_regular = 1;
+    }
+  return TRUE;
+}
diff --git a/bfd/elfxx-target.h b/bfd/elfxx-target.h
index 6cc9f3f..9775bfa 100644
--- a/bfd/elfxx-target.h
+++ b/bfd/elfxx-target.h
@@ -194,7 +194,7 @@
 #endif
 
 #ifndef bfd_elfNN_bfd_define_common_symbol
-#define bfd_elfNN_bfd_define_common_symbol bfd_generic_define_common_symbol
+#define bfd_elfNN_bfd_define_common_symbol _bfd_elf_define_common_symbol
 #endif
 
 #ifndef bfd_elfNN_bfd_lookup_section_flags
diff --git a/ld/testsuite/ld-elf/pr19579a.c b/ld/testsuite/ld-elf/pr19579a.c
index e4a6eb1..69d0f35 100644
--- a/ld/testsuite/ld-elf/pr19579a.c
+++ b/ld/testsuite/ld-elf/pr19579a.c
@@ -9,7 +9,7 @@ extern int *bar_p (void);
 int
 main ()
 {
-  if (foo[0] == 0 && foo == foo_p () && bar[0] == 0 && bar == bar_p ())
+  if (foo[0] == 0 && foo == foo_p () && bar[0] == -1 && bar == bar_p ())
     printf ("PASS\n");
   return 0;
 }
-- 
2.9.3

Re: [PATCH] PR ld/19579/21306 Properly turn common symbol into definition

[Alan Modra]

On Wed, Apr 05, 2017 at 09:46:38AM -0700, H.J. Lu wrote:
> +bfd_boolean
> +_bfd_elf_define_common_symbol (bfd *output_bfd,
> +			       struct bfd_link_info *info,
> +			       struct bfd_link_hash_entry *h)
> +{
> +  bfd_boolean ret
> +    = bfd_generic_define_common_symbol (output_bfd, info, h);
> +  if (ret)
> +    {
> +      /* Since def_regular may not be set if it is overridden by a
> +	 dynamic definition, we need to set def_regular when it is
> +	 converted to a defined symbol.  */
> +      struct elf_link_hash_entry *eh
> +	= (struct elf_link_hash_entry *) h;
> +      eh->def_regular = 1;
> +    }
> +  return TRUE;
> +}

This fails for ELF targets using the generic linker, like d30v-elf.

Isn't the real bug that somewhere we have code lacking an
ELF_COMMON_DEF_P test?

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] PR ld/19579/21306 Properly turn common symbol into definition

[H.J. Lu]

On Thu, Apr 6, 2017 at 3:43 AM, Alan Modra <amodra@gmail.com> wrote:
> On Wed, Apr 05, 2017 at 09:46:38AM -0700, H.J. Lu wrote:
>> +bfd_boolean
>> +_bfd_elf_define_common_symbol (bfd *output_bfd,
>> +                            struct bfd_link_info *info,
>> +                            struct bfd_link_hash_entry *h)
>> +{
>> +  bfd_boolean ret
>> +    = bfd_generic_define_common_symbol (output_bfd, info, h);
>> +  if (ret)
>> +    {
>> +      /* Since def_regular may not be set if it is overridden by a
>> +      dynamic definition, we need to set def_regular when it is
>> +      converted to a defined symbol.  */
>> +      struct elf_link_hash_entry *eh
>> +     = (struct elf_link_hash_entry *) h;
>> +      eh->def_regular = 1;
>> +    }
>> +  return TRUE;
>> +}
>
> This fails for ELF targets using the generic linker, like d30v-elf.

Does such target support shared object, which is the issue here?

> Isn't the real bug that somewhere we have code lacking an
> ELF_COMMON_DEF_P test?

ELF targets check def_regular for definition in relocatable object.
It should be set for all definitions in relocatable objects

-- 
H.J.

Re: [PATCH] PR ld/19579/21306 Properly turn common symbol into definition

[Alan Modra]

On Thu, Apr 06, 2017 at 07:38:54AM -0700, H.J. Lu wrote:
> On Thu, Apr 6, 2017 at 3:43 AM, Alan Modra <amodra@gmail.com> wrote:
> > On Wed, Apr 05, 2017 at 09:46:38AM -0700, H.J. Lu wrote:
> >> +bfd_boolean
> >> +_bfd_elf_define_common_symbol (bfd *output_bfd,
> >> +                            struct bfd_link_info *info,
> >> +                            struct bfd_link_hash_entry *h)
> >> +{
> >> +  bfd_boolean ret
> >> +    = bfd_generic_define_common_symbol (output_bfd, info, h);
> >> +  if (ret)
> >> +    {
> >> +      /* Since def_regular may not be set if it is overridden by a
> >> +      dynamic definition, we need to set def_regular when it is
> >> +      converted to a defined symbol.  */
> >> +      struct elf_link_hash_entry *eh
> >> +     = (struct elf_link_hash_entry *) h;
> >> +      eh->def_regular = 1;
> >> +    }
> >> +  return TRUE;
> >> +}
> >
> > This fails for ELF targets using the generic linker, like d30v-elf.
> 
> Does such target support shared object, which is the issue here?

No, the issue is that the above code uses a cast that is invalid.  The
generic linker does not use a hash table with elf_link_hash_entry (or
superclass) elements.

> > Isn't the real bug that somewhere we have code lacking an
> > ELF_COMMON_DEF_P test?
> 
> ELF targets check def_regular for definition in relocatable object.
> It should be set for all definitions in relocatable objects

Please read the ELF_COMMON_DEF_P comment.  I disagree that it is
*necessary* to set def_regular when a common symbol becomes a
definition.  It may be desirable to set the flag, to simplify other
code, if there is no place where we want to distinguish symbols that
actually are definitions in relocatable object files from symbols that
*become* definitions.  I'd need to be convinced of that before
accepting a patch that sets def_regular for commons.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] PR ld/19579/21306 Properly turn common symbol into definition

[H.J. Lu]

[-- Attachment #1: Type: text/plain, Size: 2036 bytes --]

On Thu, Apr 6, 2017 at 8:18 AM, Alan Modra <amodra@gmail.com> wrote:
> On Thu, Apr 06, 2017 at 07:38:54AM -0700, H.J. Lu wrote:
>> On Thu, Apr 6, 2017 at 3:43 AM, Alan Modra <amodra@gmail.com> wrote:
>> > On Wed, Apr 05, 2017 at 09:46:38AM -0700, H.J. Lu wrote:
>> >> +bfd_boolean
>> >> +_bfd_elf_define_common_symbol (bfd *output_bfd,
>> >> +                            struct bfd_link_info *info,
>> >> +                            struct bfd_link_hash_entry *h)
>> >> +{
>> >> +  bfd_boolean ret
>> >> +    = bfd_generic_define_common_symbol (output_bfd, info, h);
>> >> +  if (ret)
>> >> +    {
>> >> +      /* Since def_regular may not be set if it is overridden by a
>> >> +      dynamic definition, we need to set def_regular when it is
>> >> +      converted to a defined symbol.  */
>> >> +      struct elf_link_hash_entry *eh
>> >> +     = (struct elf_link_hash_entry *) h;
>> >> +      eh->def_regular = 1;
>> >> +    }
>> >> +  return TRUE;
>> >> +}
>> >
>> > This fails for ELF targets using the generic linker, like d30v-elf.
>>
>> Does such target support shared object, which is the issue here?
>
> No, the issue is that the above code uses a cast that is invalid.  The
> generic linker does not use a hash table with elf_link_hash_entry (or
> superclass) elements.
>
>> > Isn't the real bug that somewhere we have code lacking an
>> > ELF_COMMON_DEF_P test?
>>
>> ELF targets check def_regular for definition in relocatable object.
>> It should be set for all definitions in relocatable objects
>
> Please read the ELF_COMMON_DEF_P comment.  I disagree that it is
> *necessary* to set def_regular when a common symbol becomes a
> definition.  It may be desirable to set the flag, to simplify other
> code, if there is no place where we want to distinguish symbols that
> actually are definitions in relocatable object files from symbols that
> *become* definitions.  I'd need to be convinced of that before
> accepting a patch that sets def_regular for commons.
>

Here is  a patch to use ELF_COMMON_DEF_P.



-- 
H.J.

[-- Attachment #2: 0001-ELF-Check-ELF_COMMON_DEF_P-for-common-symbols.patch --]
[-- Type: text/x-patch, Size: 3933 bytes --]

From fe27591e54758240778acb557f2c786830200bcf Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Wed, 5 Apr 2017 09:30:40 -0700
Subject: [PATCH] ELF: Check ELF_COMMON_DEF_P for common symbols

Since common symbols that are turned into definitions don't have the
DEF_REGULAR flag set, we need to check ELF_COMMON_DEF_P for common
symbols.

bfd/

	PR ld/19579
	PR ld/21306
	* elf32-s390.c (elf_s390_finish_dynamic_symbol): Check
	ELF_COMMON_DEF_P for common symbols.
	* elf64-s390.c (elf_s390_finish_dynamic_symbol): Likewise.
	* elf64-x86-64.c (elf_x86_64_relocate_section): Likewise.
	* elflink.c (_bfd_elf_merge_symbol): Revert commits
	202ac193bbbecc96a4978d1ac3d17148253f9b01 and
	07492f668d2173da7a2bda3707ff0985e0f460b6.

ld/

	PR ld/19579
	PR ld/21306
	* testsuite/ld-elf/pr19579a.c (main): Updated.

xxxx
---
 bfd/elf32-s390.c               | 2 +-
 bfd/elf64-s390.c               | 2 +-
 bfd/elf64-x86-64.c             | 3 ++-
 bfd/elflink.c                  | 7 ++-----
 ld/testsuite/ld-elf/pr19579a.c | 2 +-
 5 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/bfd/elf32-s390.c b/bfd/elf32-s390.c
index fd1bc13..ddb6f5b 100644
--- a/bfd/elf32-s390.c
+++ b/bfd/elf32-s390.c
@@ -3785,7 +3785,7 @@ elf_s390_finish_dynamic_symbol (bfd *output_bfd,
 	     RELATIVE reloc.  The entry in the global offset table
 	     will already have been initialized in the
 	     relocate_section function.  */
-	  if (!h->def_regular)
+	  if (!(h->def_regular || ELF_COMMON_DEF_P (h)))
 	    return FALSE;
 	  BFD_ASSERT((h->got.offset & 1) != 0);
 	  rela.r_info = ELF32_R_INFO (0, R_390_RELATIVE);
diff --git a/bfd/elf64-s390.c b/bfd/elf64-s390.c
index b5fd05f..fbbf8d6 100644
--- a/bfd/elf64-s390.c
+++ b/bfd/elf64-s390.c
@@ -3582,7 +3582,7 @@ elf_s390_finish_dynamic_symbol (bfd *output_bfd,
 	     RELATIVE reloc.  The entry in the global offset table
 	     will already have been initialized in the
 	     relocate_section function.  */
-	  if (!h->def_regular)
+	  if (!(h->def_regular || ELF_COMMON_DEF_P (h)))
 	    return FALSE;
 	  BFD_ASSERT((h->got.offset & 1) != 0);
 	  rela.r_info = ELF64_R_INFO (0, R_390_RELATIVE);
diff --git a/bfd/elf64-x86-64.c b/bfd/elf64-x86-64.c
index 6d92c79..a4048f1 100644
--- a/bfd/elf64-x86-64.c
+++ b/bfd/elf64-x86-64.c
@@ -4926,7 +4926,8 @@ do_ifunc_pointer:
 		{
 		  /* Symbol is referenced locally.  Make sure it is
 		     defined locally or for a branch.  */
-		  fail = !h->def_regular && !branch;
+		  fail = (!(h->def_regular || ELF_COMMON_DEF_P (h))
+			  && !branch);
 		}
 	      else if (!(bfd_link_pie (info)
 			 && (h->needs_copy || eh->needs_copy)))
diff --git a/bfd/elflink.c b/bfd/elflink.c
index 9bf75c8..c00d712 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -1544,16 +1544,13 @@ _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.  A common
-     symbol in executable also overrides a symbol in a shared object.  */
+     object to override a weak symbol in a shared object.  */
 
   if (newdyn
       && newdef
       && (olddef
 	  || (h->root.type == bfd_link_hash_common
-	      && (newweak
-		  || newfunc
-		  || (!olddyn && bfd_link_executable (info))))))
+	      && (newweak || newfunc))))
     {
       *override = TRUE;
       newdef = FALSE;
diff --git a/ld/testsuite/ld-elf/pr19579a.c b/ld/testsuite/ld-elf/pr19579a.c
index e4a6eb1..69d0f35 100644
--- a/ld/testsuite/ld-elf/pr19579a.c
+++ b/ld/testsuite/ld-elf/pr19579a.c
@@ -9,7 +9,7 @@ extern int *bar_p (void);
 int
 main ()
 {
-  if (foo[0] == 0 && foo == foo_p () && bar[0] == 0 && bar == bar_p ())
+  if (foo[0] == 0 && foo == foo_p () && bar[0] == -1 && bar == bar_p ())
     printf ("PASS\n");
   return 0;
 }
-- 
2.9.3

Re: [PATCH] PR ld/19579/21306 Properly turn common symbol into definition

[Alan Modra]

On Thu, Apr 06, 2017 at 10:45:32AM -0700, H.J. Lu wrote:
> Since common symbols that are turned into definitions don't have the
> DEF_REGULAR flag set, we need to check ELF_COMMON_DEF_P for common
> symbols.
> 
> bfd/
> 
> 	PR ld/19579
> 	PR ld/21306
> 	* elf32-s390.c (elf_s390_finish_dynamic_symbol): Check
> 	ELF_COMMON_DEF_P for common symbols.
> 	* elf64-s390.c (elf_s390_finish_dynamic_symbol): Likewise.
> 	* elf64-x86-64.c (elf_x86_64_relocate_section): Likewise.
> 	* elflink.c (_bfd_elf_merge_symbol): Revert commits
> 	202ac193bbbecc96a4978d1ac3d17148253f9b01 and
> 	07492f668d2173da7a2bda3707ff0985e0f460b6.
> 
> ld/
> 
> 	PR ld/19579
> 	PR ld/21306
> 	* testsuite/ld-elf/pr19579a.c (main): Updated.

This is OK.  Thanks for looking at s390 too.  Which other targets do
you have cross-compilers installed or test natively in order to see
19579 failures?  (I'm assuming that's why you made the s390 changes,
and would like to know the targets that might yet need attention.)

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] PR ld/19579/21306 Properly turn common symbol into definition

[H.J. Lu]

On Thu, Apr 6, 2017 at 3:40 PM, Alan Modra <amodra@gmail.com> wrote:
> On Thu, Apr 06, 2017 at 10:45:32AM -0700, H.J. Lu wrote:
>> Since common symbols that are turned into definitions don't have the
>> DEF_REGULAR flag set, we need to check ELF_COMMON_DEF_P for common
>> symbols.
>>
>> bfd/
>>
>>       PR ld/19579
>>       PR ld/21306
>>       * elf32-s390.c (elf_s390_finish_dynamic_symbol): Check
>>       ELF_COMMON_DEF_P for common symbols.
>>       * elf64-s390.c (elf_s390_finish_dynamic_symbol): Likewise.
>>       * elf64-x86-64.c (elf_x86_64_relocate_section): Likewise.
>>       * elflink.c (_bfd_elf_merge_symbol): Revert commits
>>       202ac193bbbecc96a4978d1ac3d17148253f9b01 and
>>       07492f668d2173da7a2bda3707ff0985e0f460b6.
>>
>> ld/
>>
>>       PR ld/19579
>>       PR ld/21306
>>       * testsuite/ld-elf/pr19579a.c (main): Updated.
>
> This is OK.  Thanks for looking at s390 too.  Which other targets do
> you have cross-compilers installed or test natively in order to see
> 19579 failures?  (I'm assuming that's why you made the s390 changes,

19579 test needs a target compiler to compile.  I updated s390
since the s390 code in question is in PR 19579.

> and would like to know the targets that might yet need attention.)
>

It is hard to tell if other targets are affected since we don't need to
check ELF_COMMON_DEF_P everywhere.

-- 
H.J.

Re: [PATCH] PR ld/19579/21306 Properly turn common symbol into definition

[Alan Modra]

On Thu, Apr 06, 2017 at 03:44:47PM -0700, H.J. Lu wrote:
> On Thu, Apr 6, 2017 at 3:40 PM, Alan Modra <amodra@gmail.com> wrote:
> > On Thu, Apr 06, 2017 at 10:45:32AM -0700, H.J. Lu wrote:
> >> Since common symbols that are turned into definitions don't have the
> >> DEF_REGULAR flag set, we need to check ELF_COMMON_DEF_P for common
> >> symbols.
> >>
> >> bfd/
> >>
> >>       PR ld/19579
> >>       PR ld/21306
> >>       * elf32-s390.c (elf_s390_finish_dynamic_symbol): Check
> >>       ELF_COMMON_DEF_P for common symbols.
> >>       * elf64-s390.c (elf_s390_finish_dynamic_symbol): Likewise.
> >>       * elf64-x86-64.c (elf_x86_64_relocate_section): Likewise.
> >>       * elflink.c (_bfd_elf_merge_symbol): Revert commits
> >>       202ac193bbbecc96a4978d1ac3d17148253f9b01 and
> >>       07492f668d2173da7a2bda3707ff0985e0f460b6.
> >>
> >> ld/
> >>
> >>       PR ld/19579
> >>       PR ld/21306
> >>       * testsuite/ld-elf/pr19579a.c (main): Updated.
> >
> > This is OK.  Thanks for looking at s390 too.  Which other targets do
> > you have cross-compilers installed or test natively in order to see
> > 19579 failures?  (I'm assuming that's why you made the s390 changes,
> 
> 19579 test needs a target compiler to compile.  I updated s390
> since the s390 code in question is in PR 19579.
> 
> > and would like to know the targets that might yet need attention.)
> >
> 
> It is hard to tell if other targets are affected since we don't need to
> check ELF_COMMON_DEF_P everywhere.

Right.  If I find myself bored with nothing else to do, I may audit
all the uses of def_regular in code that runs after lang_common, in
order to see whether we can delete ELF_COMMON_DEF_P and set
def_regular for commons instead.  It isn't hard to make your previous
patch work; You just need to define bfd_elfNN_bfd_define_common_symbol
conditionally in elfxx-target.h.  Search for "generic linker" there.

For now, I think adding ELF_COMMON_DEF_P as necessary is the best we
can do.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] PR ld/19579/21306 Properly turn common symbol into definition

[H.J. Lu]

On Thu, Apr 6, 2017 at 6:07 PM, Alan Modra <amodra@gmail.com> wrote:
> On Thu, Apr 06, 2017 at 03:44:47PM -0700, H.J. Lu wrote:
>> On Thu, Apr 6, 2017 at 3:40 PM, Alan Modra <amodra@gmail.com> wrote:
>> > On Thu, Apr 06, 2017 at 10:45:32AM -0700, H.J. Lu wrote:
>> >> Since common symbols that are turned into definitions don't have the
>> >> DEF_REGULAR flag set, we need to check ELF_COMMON_DEF_P for common
>> >> symbols.
>> >>
>> >> bfd/
>> >>
>> >>       PR ld/19579
>> >>       PR ld/21306
>> >>       * elf32-s390.c (elf_s390_finish_dynamic_symbol): Check
>> >>       ELF_COMMON_DEF_P for common symbols.
>> >>       * elf64-s390.c (elf_s390_finish_dynamic_symbol): Likewise.
>> >>       * elf64-x86-64.c (elf_x86_64_relocate_section): Likewise.
>> >>       * elflink.c (_bfd_elf_merge_symbol): Revert commits
>> >>       202ac193bbbecc96a4978d1ac3d17148253f9b01 and
>> >>       07492f668d2173da7a2bda3707ff0985e0f460b6.
>> >>
>> >> ld/
>> >>
>> >>       PR ld/19579
>> >>       PR ld/21306
>> >>       * testsuite/ld-elf/pr19579a.c (main): Updated.
>> >
>> > This is OK.  Thanks for looking at s390 too.  Which other targets do
>> > you have cross-compilers installed or test natively in order to see
>> > 19579 failures?  (I'm assuming that's why you made the s390 changes,
>>
>> 19579 test needs a target compiler to compile.  I updated s390
>> since the s390 code in question is in PR 19579.
>>
>> > and would like to know the targets that might yet need attention.)
>> >
>>
>> It is hard to tell if other targets are affected since we don't need to
>> check ELF_COMMON_DEF_P everywhere.
>
> Right.  If I find myself bored with nothing else to do, I may audit
> all the uses of def_regular in code that runs after lang_common, in
> order to see whether we can delete ELF_COMMON_DEF_P and set
> def_regular for commons instead.  It isn't hard to make your previous
> patch work; You just need to define bfd_elfNN_bfd_define_common_symbol
> conditionally in elfxx-target.h.  Search for "generic linker" there.
>
> For now, I think adding ELF_COMMON_DEF_P as necessary is the best we
> can do.
>

I checked it in.  OK to backport to 2,28 branch?

-- 
H.J.

Re: [PATCH] PR ld/19579/21306 Properly turn common symbol into definition

[Alan Modra]

On Fri, Apr 07, 2017 at 07:41:24AM -0700, H.J. Lu wrote:
> On Thu, Apr 6, 2017 at 6:07 PM, Alan Modra <amodra@gmail.com> wrote:
> > For now, I think adding ELF_COMMON_DEF_P as necessary is the best we
> > can do.
> >
> 
> I checked it in.  OK to backport to 2,28 branch?

Yes, I think that is good.

-- 
Alan Modra
Australia Development Lab, IBM