[committed] MIPS/BFD: Remove extraneous undefined weak symbol visibility check

["Maciej W. Rozycki" <macro at mips dot com>]

Remove an extraneous symbol visibility check made for undefined weak 
symbols in determination whether an R_MIPS_REL32 dynamic relocation has 
to be placed in output, complementing commit ad9512030937 ("mips: Check 
UNDEFWEAK_NO_DYNAMIC_RELOC").  That check duplicates one already made by 
the UNDEFWEAK_NO_DYNAMIC_RELOC macro as a part of a broader condition 
used to decide if to enter undefined weak symbols to the dynamic symbol 
table or not.

	bfd/
	* elfxx-mips.c (allocate_dynrelocs): Remove extraneous symbol 
	visibility check made for undefined weak symbols.
---
On Wed, 18 Oct 2017, Maciej W. Rozycki wrote:

> > diff --git a/bfd/elfxx-mips.c b/bfd/elfxx-mips.c
> > index 6a4d3e1a6a..23fba1bbe0 100644
> > --- a/bfd/elfxx-mips.c
> > +++ b/bfd/elfxx-mips.c
> > @@ -5665,6 +5666,10 @@ mips_elf_calculate_relocation (bfd *abfd, bfd *input_bfd,
> >        addend = 0;
> >      }
> >  
> > +  resolved_to_zero = (h != NULL
> > +		      && UNDEFWEAK_NO_DYNAMIC_RELOC (info,
> > +							  &h->root));
> 
>  Gratuitous line wrapping.
> 
>  I can see you have checked it in last Sat despite Alan's request to wait 
> a few days to give target maintainers an opportunity to chime in.  Please 
> don't do this again.  Not everyone works weekends.
> 
>  I'll look through the change yet and see if there are any issues beyond 
> mere formatting nits.

 I got to this finally, in the course of resolving PR ld/21805.  The MIPS 
backend currently ignores `-z dynamic-undefined-weak' and its complement, 
however this can change once PR ld/21805 has been fixed.  But what is the 
intended interaction between this option and `--export-dynamic' (and its 
complement)?  Should one override the other or ignore it?

 But only exporting undefined weaks makes no sense to me.  So I'm tempted 
to make the MIPS backend only export undefined weaks if `--export-dynamic' 
and `-z dynamic-undefined-weak' are active both at once, even though it 
looks to me like with all the other backends the options ignore each 
other.

 Also I still do not understand the matter of PR ld/22269, that is why 
exporting undefined weaks in PIE (static or dynamic) would cause a 
non-zero run-time value?  They'll still be undefined weaks at load time 
after all (unless, in the dynamic case, satisfied by a DSO), so the 
original value of zero is supposed to stay, and if it is not the case, 
then it looks to me like a run-time loader bug rather than a static linker 
one.

 Am I missing something here?

 Meanwhile I have regression-tested and applied the cleanup included.  
I'll fold a fix for the line wrapping issue pointed out above into the 
upcoming fix for PR ld/21375, as it's going to move the line around 
anyway.

  Maciej
---
 bfd/elfxx-mips.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

binutils-mips-bfd-allocate-dynrelocs-undefweak.diff
Index: binutils/bfd/elfxx-mips.c
===================================================================
--- binutils.orig/bfd/elfxx-mips.c	2018-06-28 21:27:42.000000000 +0100
+++ binutils/bfd/elfxx-mips.c	2018-06-28 21:31:08.123354677 +0100
@@ -8951,10 +8951,9 @@ allocate_dynrelocs (struct elf_link_hash
 
       if (h->root.type == bfd_link_hash_undefweak)
 	{
-	  /* Do not copy relocations for undefined weak symbols with
-	     non-default visibility.  */
-	  if (ELF_ST_VISIBILITY (h->other) != STV_DEFAULT
-	      || UNDEFWEAK_NO_DYNAMIC_RELOC (info, h))
+	  /* Do not copy relocations for undefined weak symbols that
+	     we are not going to export.  */
+	  if (UNDEFWEAK_NO_DYNAMIC_RELOC (info, h))
 	    do_copy = FALSE;
 
 	  /* Make sure undefined weak symbols are output as a dynamic