PR26580, Size and alignment of commons vs as-needed shared lib

Two pieces to this puzzle:
1) Revert HJ's fix for PR13250 so that size and alignment isn't
   sticky, instead attack the real underlying problem that
   _bfd_generic_link_add_one_symbol does the wrong thing in making a
   common section in a shared library bfd.
2) Save and restore common u.c.p fields, which hold the section and
   alignment.

A better fix for (2) would be to throw away all of that horrible code
saving and restoring the hash table when loading as-needed library
symbols, and instead do a scan over as-needed library symbols before
adding anything.

bfd/
	PR 13250
	PR 26580
	* elflink.c (_bfd_elf_merge_symbol): Make "override" a bfd**.
	Return oldbfd in override when old common should override new
	common.
	(_bfd_elf_add_default_symbol): Adjust to suit.
	(elf_link_add_object_symbols): Likewise.  Pass "override" to
	_bfd_generic_link_add_one_symbol.  Save and restore common u.c.p
	field for --as-needed shared libraries.  Revert pr13250 changes.
ld/
	* testsuite/ld-elf/pr26580-a.s,
	* testsuite/ld-elf/pr26580-b.s,
	* testsuite/ld-elf/pr26580-1.sd,
	* testsuite/ld-elf/pr26580-2.sd: New tests
	* testsuite/ld-elf/comm-data.exp: Run new tests.
	* testsuite/ld-elf/pr26580-a.c,
	* testsuite/ld-elf/pr26580-b.c,
	* testsuite/ld-elf/pr26580-3.out,
	* testsuite/ld-elf/pr26580-4.out: New tests.
	* testsuite/ld-elf/shared.exp: Run new tests.

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 22cee1d66420c68058189f9b7c415fae482fb776..4faa8f235112de1c0e624f331c4d7e1fca3b77e4 100644 (file)
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,15 @@
+2020-09-08  Alan Modra  <amodra@gmail.com>
+
+       PR 13250
+       PR 26580
+       * elflink.c (_bfd_elf_merge_symbol): Make "override" a bfd**.
+       Return oldbfd in override when old common should override new
+       common.
+       (_bfd_elf_add_default_symbol): Adjust to suit.
+       (elf_link_add_object_symbols): Likewise.  Pass "override" to
+       _bfd_generic_link_add_one_symbol.  Save and restore common u.c.p
+       field for --as-needed shared libraries.  Revert pr13250 changes.
+
 2020-09-08  Nick Clifton  <nickc@redhat.com>
 
        * plugin.c (bfd_plugin_canonicalize_symtab): Handle the case of an

diff --git a/bfd/elflink.c b/bfd/elflink.c
index 1384c1a46b83b55876b6a73dbcba0386a458063b..0e339f3c1ea44f745767115d1373937e5479af29 100644 (file)
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -1058,7 +1058,7 @@ _bfd_elf_merge_symbol (bfd *abfd,
                       bfd_boolean *pold_weak,
                       unsigned int *pold_alignment,
                       bfd_boolean *skip,
-                      bfd_boolean *override,
+                      bfd **override,
                       bfd_boolean *type_change_ok,
                       bfd_boolean *size_change_ok,
                       bfd_boolean *matched)
@@ -1076,7 +1076,7 @@ _bfd_elf_merge_symbol (bfd *abfd,
   bfd_boolean default_sym = *matched;
 
   *skip = FALSE;
-  *override = FALSE;
+  *override = NULL;
 
   sec = *psec;
   bind = ELF_ST_BIND (sym->st_info);
@@ -1652,7 +1652,7 @@ _bfd_elf_merge_symbol (bfd *abfd,
          || (h->root.type == bfd_link_hash_common
              && (newweak || newfunc))))
     {
-      *override = TRUE;
+      *override = abfd;
       newdef = FALSE;
       newdyncommon = FALSE;
 
@@ -1678,7 +1678,7 @@ _bfd_elf_merge_symbol (bfd *abfd,
   if (newdyncommon
       && h->root.type == bfd_link_hash_common)
     {
-      *override = TRUE;
+      *override = oldbfd;
       newdef = FALSE;
       newdyncommon = FALSE;
       *pvalue = sym->st_size;
@@ -1854,7 +1854,7 @@ _bfd_elf_add_default_symbol (bfd *abfd,
   const struct elf_backend_data *bed;
   bfd_boolean collect;
   bfd_boolean dynamic;
-  bfd_boolean override;
+  bfd *override;
   char *p;
   size_t len, shortlen;
   asection *tmp_sec;
@@ -4484,7 +4484,12 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
              h = (struct elf_link_hash_entry *) p;
              entsize += htab->root.table.entsize;
              if (h->root.type == bfd_link_hash_warning)
-               entsize += htab->root.table.entsize;
+               {
+                 entsize += htab->root.table.entsize;
+                 h = (struct elf_link_hash_entry *) h->root.u.i.link;
+               }
+             if (h->root.type == bfd_link_hash_common)
+               entsize += sizeof (*h->root.u.c.p);
            }
        }
 
@@ -4528,14 +4533,20 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
 
          for (p = htab->root.table.table[i]; p != NULL; p = p->next)
            {
-             memcpy (old_ent, p, htab->root.table.entsize);
-             old_ent = (char *) old_ent + htab->root.table.entsize;
              h = (struct elf_link_hash_entry *) p;
+             memcpy (old_ent, h, htab->root.table.entsize);
+             old_ent = (char *) old_ent + htab->root.table.entsize;
              if (h->root.type == bfd_link_hash_warning)
                {
-                 memcpy (old_ent, h->root.u.i.link, htab->root.table.entsize);
+                 h = (struct elf_link_hash_entry *) h->root.u.i.link;
+                 memcpy (old_ent, h, htab->root.table.entsize);
                  old_ent = (char *) old_ent + htab->root.table.entsize;
                }
+             if (h->root.type == bfd_link_hash_common)
+               {
+                 memcpy (old_ent, h->root.u.c.p, sizeof (*h->root.u.c.p));
+                 old_ent = (char *) old_ent + sizeof (*h->root.u.c.p);
+               }
            }
        }
     }
@@ -4578,7 +4589,7 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
       bfd_boolean type_change_ok;
       bfd_boolean new_weak;
       bfd_boolean old_weak;
-      bfd_boolean override;
+      bfd *override;
       bfd_boolean common;
       bfd_boolean discarded;
       unsigned int old_alignment;
@@ -4586,7 +4597,7 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
       bfd *old_bfd;
       bfd_boolean matched;
 
-      override = FALSE;
+      override = NULL;
 
       flags = BSF_NO_FLAGS;
       sec = NULL;
@@ -4906,7 +4917,8 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
        }
 
       if (! (_bfd_generic_link_add_one_symbol
-            (info, abfd, name, flags, sec, value, NULL, FALSE, bed->collect,
+            (info, override ? override : abfd, name, flags, sec, value,
+             NULL, FALSE, bed->collect,
              (struct bfd_link_hash_entry **) sym_hash)))
        goto error_free_vers;
 
@@ -5337,49 +5349,31 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
        {
          struct bfd_hash_entry *p;
          struct elf_link_hash_entry *h;
-         bfd_size_type size;
-         unsigned int alignment_power;
          unsigned int non_ir_ref_dynamic;
 
          for (p = htab->root.table.table[i]; p != NULL; p = p->next)
            {
-             h = (struct elf_link_hash_entry *) p;
-             if (h->root.type == bfd_link_hash_warning)
-               h = (struct elf_link_hash_entry *) h->root.u.i.link;
-
-             /* Preserve the maximum alignment and size for common
-                symbols even if this dynamic lib isn't on DT_NEEDED
-                since it can still be loaded at run time by another
-                dynamic lib.  */
-             if (h->root.type == bfd_link_hash_common)
-               {
-                 size = h->root.u.c.size;
-                 alignment_power = h->root.u.c.p->alignment_power;
-               }
-             else
-               {
-                 size = 0;
-                 alignment_power = 0;
-               }
              /* Preserve non_ir_ref_dynamic so that this symbol
                 will be exported when the dynamic lib becomes needed
                 in the second pass.  */
+             h = (struct elf_link_hash_entry *) p;
+             if (h->root.type == bfd_link_hash_warning)
+               h = (struct elf_link_hash_entry *) h->root.u.i.link;
              non_ir_ref_dynamic = h->root.non_ir_ref_dynamic;
-             memcpy (p, old_ent, htab->root.table.entsize);
-             old_ent = (char *) old_ent + htab->root.table.entsize;
+
              h = (struct elf_link_hash_entry *) p;
+             memcpy (h, old_ent, htab->root.table.entsize);
+             old_ent = (char *) old_ent + htab->root.table.entsize;
              if (h->root.type == bfd_link_hash_warning)
                {
-                 memcpy (h->root.u.i.link, old_ent, htab->root.table.entsize);
-                 old_ent = (char *) old_ent + htab->root.table.entsize;
                  h = (struct elf_link_hash_entry *) h->root.u.i.link;
+                 memcpy (h, old_ent, htab->root.table.entsize);
+                 old_ent = (char *) old_ent + htab->root.table.entsize;
                }
              if (h->root.type == bfd_link_hash_common)
                {
-                 if (size > h->root.u.c.size)
-                   h->root.u.c.size = size;
-                 if (alignment_power > h->root.u.c.p->alignment_power)
-                   h->root.u.c.p->alignment_power = alignment_power;
+                 memcpy (h->root.u.c.p, old_ent, sizeof (*h->root.u.c.p));
+                 old_ent = (char *) old_ent + sizeof (*h->root.u.c.p);
                }
              h->root.non_ir_ref_dynamic = non_ir_ref_dynamic;
            }

diff --git a/ld/ChangeLog b/ld/ChangeLog
index 0a11b79c67443fa8831f65a251b7533b2c773eda..4b42f05171c2eb892dedf7f972b29248450eed62 100644 (file)
--- a/ld/ChangeLog
+++ b/ld/ChangeLog
@@ -1,3 +1,16 @@
+2020-09-08  Alan Modra  <amodra@gmail.com>
+
+       * testsuite/ld-elf/pr26580-a.s,
+       * testsuite/ld-elf/pr26580-b.s,
+       * testsuite/ld-elf/pr26580-1.sd,
+       * testsuite/ld-elf/pr26580-2.sd: New tests
+       * testsuite/ld-elf/comm-data.exp: Run new tests.
+       * testsuite/ld-elf/pr26580-a.c,
+       * testsuite/ld-elf/pr26580-b.c,
+       * testsuite/ld-elf/pr26580-3.out,
+       * testsuite/ld-elf/pr26580-4.out: New tests.
+       * testsuite/ld-elf/shared.exp: Run new tests.
+
 2020-09-04  Alan Modra  <amodra@gmail.com>
 
        * testsuite/ld-plugin/lto.exp: Don't run pr15146 tests.

diff --git a/ld/testsuite/ld-elf/comm-data.exp b/ld/testsuite/ld-elf/comm-data.exp
index 87ae3b299124e615e5b876e4570165a5b79619e8..16b8e2b8ed2587cf715286b9c6687c9f29eb4cef 100644 (file)
--- a/ld/testsuite/ld-elf/comm-data.exp
+++ b/ld/testsuite/ld-elf/comm-data.exp
@@ -48,6 +48,16 @@ if { [istarget "mips*vr4100*-*-elf*"] \
     append LFLAGS " -call_shared"
 }
 
+# Set the pointer size according to the ELF flavor.
+set AFLAGS ""
+if [is_elf64 "tmpdir/libcomm-data.so"] {
+    append AFLAGS " --defsym ELF64=1"
+}
+# HPUX targets use a different .comm syntax.
+if [istarget "*-*-hpux*"] {
+    append AFLAGS " --defsym HPUX=1"
+}
+
 set testname "Common symbol override test"
 
 # Define a global symbol.
@@ -62,18 +72,16 @@ run_ld_link_tests [list \
        } \
        "libcomm-data.so" \
     ] \
+    [list \
+       "libpr26580-1.so" \
+       "$LFLAGS -shared" "" \
+       "$AFLAGS_PIC $AFLAGS" \
+       { pr26580-b.s } \
+       { } \
+       "libpr26580-1.so" \
+    ] \
 ]
 
-# Set the pointer size according to the ELF flavor.
-set AFLAGS ""
-if [is_elf64 "tmpdir/libcomm-data.so"] {
-    append AFLAGS " --defsym ELF64=1"
-}
-# HPUX targets use a different .comm syntax.
-if [istarget "*-*-hpux*"] {
-    append AFLAGS " --defsym HPUX=1"
-}
-
 # bfin-elf does not currently support copy relocs.
 setup_xfail "bfin-*-*"
 clear_xfail "bfin-*-linux-uclibc*"
@@ -113,4 +121,20 @@ run_ld_link_tests [list \
        } \
        "comm-data3b" \
     ] \
+    [list \
+       "pr26580-1" \
+       "$LFLAGS --as-needed -Ltmpdir -lpr26580-1" "" \
+       "$AFLAGS" \
+       { pr26580-a.s } \
+       { {readelf -s pr26580-1.sd} } \
+       "pr26580-1" \
+    ] \
+    [list \
+       "pr26580-2" \
+       "$LFLAGS --no-as-needed -Ltmpdir -lpr26580-1" "" \
+       "$AFLAGS" \
+       { pr26580-a.s } \
+       { {readelf -s pr26580-2.sd} } \
+       "pr26580-2" \
+    ] \
 ]

diff --git a/ld/testsuite/ld-elf/pr26580-1.sd b/ld/testsuite/ld-elf/pr26580-1.sd
new file mode 100644 (file)
index 0000000..b5adab0
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr26580-1.sd
@@ -0,0 +1,4 @@
+
+#...
+.* 1 OBJECT  GLOBAL DEFAULT .* one
+#pass

diff --git a/ld/testsuite/ld-elf/pr26580-2.sd b/ld/testsuite/ld-elf/pr26580-2.sd
new file mode 100644 (file)
index 0000000..265b880
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr26580-2.sd
@@ -0,0 +1,4 @@
+
+#...
+.* 8 OBJECT  GLOBAL DEFAULT .* one
+#pass

diff --git a/ld/testsuite/ld-elf/pr26580-3.out b/ld/testsuite/ld-elf/pr26580-3.out
new file mode 100644 (file)
index 0000000..566696f
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr26580-3.out
@@ -0,0 +1,2 @@
+library not loaded
+alignment 1

diff --git a/ld/testsuite/ld-elf/pr26580-4.out b/ld/testsuite/ld-elf/pr26580-4.out
new file mode 100644 (file)
index 0000000..d91deea
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr26580-4.out
@@ -0,0 +1,2 @@
+library loaded
+alignment 8

diff --git a/ld/testsuite/ld-elf/pr26580-a.c b/ld/testsuite/ld-elf/pr26580-a.c
new file mode 100644 (file)
index 0000000..5557997
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr26580-a.c
@@ -0,0 +1,20 @@
+#include <stdio.h>
+
+extern void __attribute__ ((weak)) foo (void);
+
+char x, y, z;
+
+long
+lowest_align (void *a, void *b, void *c)
+{
+  unsigned long bits = (long) a | (long) b | (long) c;
+  return bits & -bits;
+}
+
+int
+main (void)
+{
+  printf ("library %sloaded\n", &foo ? "" : "not ");
+  printf ("alignment %ld\n", lowest_align (&x, &y, &z));
+  return 0;
+}

diff --git a/ld/testsuite/ld-elf/pr26580-a.s b/ld/testsuite/ld-elf/pr26580-a.s
new file mode 100644 (file)
index 0000000..3b70ccc
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr26580-a.s
@@ -0,0 +1,10 @@
+ .text
+ .globl        _start
+ .globl        __start
+_start:
+__start:
+ .ifdef        HPUX
+one    .comm   1
+ .else
+       .comm   one, 1, 1
+ .endif

diff --git a/ld/testsuite/ld-elf/pr26580-b.c b/ld/testsuite/ld-elf/pr26580-b.c
new file mode 100644 (file)
index 0000000..e7e13d6
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr26580-b.c
@@ -0,0 +1,3 @@
+long long x, y, z;
+
+void foo (void) {}

diff --git a/ld/testsuite/ld-elf/pr26580-b.s b/ld/testsuite/ld-elf/pr26580-b.s
new file mode 100644 (file)
index 0000000..cb06378
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr26580-b.s
@@ -0,0 +1,10 @@
+ .text
+ .globl        _start
+ .globl        __start
+_start:
+__start:
+ .ifdef        HPUX
+one    .comm   8
+ .else
+       .comm   one, 8, 8
+ .endif

diff --git a/ld/testsuite/ld-elf/shared.exp b/ld/testsuite/ld-elf/shared.exp
index 87a40a362ed59955079fbf27e6032d910a3ae45c..6f53f9c31702223420f48195040787fac32e86a0 100644 (file)
--- a/ld/testsuite/ld-elf/shared.exp
+++ b/ld/testsuite/ld-elf/shared.exp
@@ -841,6 +841,9 @@ append build_tests {
   {"Dump pr21978.so"
    "-shared" "-fPIC -g -O2"
    {pr21978a.c pr21978b.c} {{objdump {-Sl} pr21978.od}} "pr21978.so"}
+  {"libpr26580-2.so"
+   "-shared" "-fPIC -fcommon"
+   {pr26580-b.c} {} "libpr26580-2.so"}
 }
 
 run_cc_link_tests $build_tests
@@ -1044,6 +1047,14 @@ set run_tests [list \
     [list "Run pr21964-3" \
      "-Wl,--no-as-needed,-rpath,tmpdir tmpdir/pr21964-1a.so tmpdir/pr21964-1b.so tmpdir/pr21964-3a.so" "" \
      {pr21964-3c.c} "pr21964-3" "pass.out" ] \
+    [list "pr26580-3" \
+     "" "" \
+     {pr26580-a.c} "pr26580-3" "pr26580-3.out" "-fcommon" "c" "" \
+     "-Wl,--as-needed tmpdir/libpr26580-2.so" ] \
+    [list "pr26580-4" \
+     "" "" \
+     {pr26580-a.c} "pr26580-4" "pr26580-4.out" "-fcommon" "c" "" \
+     "-Wl,--no-as-needed tmpdir/libpr26580-2.so" ] \
 ]
 
 # NetBSD ELF systems do not currently support the .*_array sections.