Relax PR 15228 protected visibility restriction

Allows .dynbss copy of shared library protected visibility variables
if they are read-only.

To recap: Copying a variable from a shared library into an executable's
.dynbss is an old hack invented for non-PIC executables, to avoid the
text relocations you'd otherwise need to access a shared library
variable.  This works with ELF shared libraries because global
symbols can be overridden.  The trouble is that protected visibility
symbols can't be overridden.  A shared library will continue to access
it's own protected visibility variable while the executable accesses a
copy.  If either the shared library or the executable updates the
value then the copy diverges from the original.  This is wrong since
there is only one definition of the variable in the application.

So I made the linker report an error on attempting to copy protected
visibility variables into .dynbss.  However, you'll notice the above
paragraph contains an "If".  An application that does not modify the
variable value remains correct even though two copies of the variable
exist.  The linker can detect this situation if the variable was
defined in a read-only section.

	PR ld/15228
	PR ld/18167
	* elflink.c (elf_merge_st_other): Add "sec" parameter.  Don't set
	protected_def when symbol section is read-only.  Adjust all calls.
	* elf-bfd.h (struct elf_link_hash_entry): Update protected_def comment.

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 15e957a1ba05b137dcc308e0cd20a0c1c54e9e6e..4b5143ea213110f1b35d125b0fd4650cfbd4ede5 100644 (file)
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,11 @@
+2015-03-27  Alan Modra  <amodra@gmail.com>
+
+       PR ld/15228
+       PR ld/18167
+       * elflink.c (elf_merge_st_other): Add "sec" parameter.  Don't set
+       protected_def when symbol section is read-only.  Adjust all calls.
+       * elf-bfd.h (struct elf_link_hash_entry): Update protected_def comment.
+
 2015-03-26  Tejas Belagod  <tejas.belagod@arm.com>
 
        * elfnn-aarch64.c (aarch64_build_one_stub): Replace the call to generic

diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index c84bbc4583bea0ad890a622e9e0d621564b71af8..fb1a892b12ba419c408d4da75e90ca4b0b9d85a3 100644 (file)
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -196,7 +196,8 @@ struct elf_link_hash_entry
   unsigned int pointer_equality_needed : 1;
   /* Symbol is a unique global symbol.  */
   unsigned int unique_global : 1;
-  /* Symbol is defined with non-default visibility.  */
+  /* Symbol is defined by a shared library with non-default visibility
+     in a read/write section.  */
   unsigned int protected_def : 1;
 
   /* String table index in .dynstr if this is a dynamic symbol.  */

diff --git a/bfd/elflink.c b/bfd/elflink.c
index 8298124bec77df38ee9a71e60ef60e65d453c973..0dd53c26ad13f07a517150ec3607e17b0fd74558 100644 (file)
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -845,7 +845,7 @@ _bfd_elf_link_renumber_dynsyms (bfd *output_bfd,
 
 static void
 elf_merge_st_other (bfd *abfd, struct elf_link_hash_entry *h,
-                   const Elf_Internal_Sym *isym,
+                   const Elf_Internal_Sym *isym, asection *sec,
                    bfd_boolean definition, bfd_boolean dynamic)
 {
   const struct elf_backend_data *bed = get_elf_backend_data (abfd);
@@ -866,7 +866,9 @@ elf_merge_st_other (bfd *abfd, struct elf_link_hash_entry *h,
       if (symvis - 1 < hvis - 1)
        h->other = symvis | (h->other & ~ELF_ST_VISIBILITY (-1));
     }
-  else if (definition && ELF_ST_VISIBILITY (isym->st_other) != STV_DEFAULT)
+  else if (definition
+          && ELF_ST_VISIBILITY (isym->st_other) != STV_DEFAULT
+          && (sec->flags & SEC_READONLY) == 0)
     h->protected_def = 1;
 }
 
@@ -1418,7 +1420,7 @@ _bfd_elf_merge_symbol (bfd *abfd,
       /* Merge st_other.  If the symbol already has a dynamic index,
         but visibility says it should not be visible, turn it into a
         local symbol.  */
-      elf_merge_st_other (abfd, h, sym, newdef, newdyn);
+      elf_merge_st_other (abfd, h, sym, sec, newdef, newdyn);
       if (h->dynindx != -1)
        switch (ELF_ST_VISIBILITY (h->other))
          {
@@ -4360,7 +4362,7 @@ error_free_dyn:
            }
 
          /* Merge st_other field.  */
-         elf_merge_st_other (abfd, h, isym, definition, dynamic);
+         elf_merge_st_other (abfd, h, isym, sec, definition, dynamic);
 
          /* We don't want to make debug symbol dynamic.  */
          if (definition && (sec->flags & SEC_DEBUGGING) && !info->relocatable)
@@ -13207,7 +13209,7 @@ _bfd_elf_copy_link_hash_symbol_type (bfd *abfd,
   ehdest->target_internal = ehsrc->target_internal;
 
   isym.st_other = ehsrc->other;
-  elf_merge_st_other (abfd, ehdest, &isym, TRUE, FALSE);
+  elf_merge_st_other (abfd, ehdest, &isym, NULL, TRUE, FALSE);
 }
 
 /* Append a RELA relocation REL to section S in BFD.  */