From 7ba115508aa02ffbb01a09613b5dffdd0c6563e3 Mon Sep 17 00:00:00 2001 From: Alan Modra Date: Tue, 8 Sep 2020 13:02:31 +0930 Subject: [PATCH] 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. --- bfd/ChangeLog | 12 +++++ bfd/elflink.c | 74 ++++++++++++++----------------- ld/ChangeLog | 13 ++++++ ld/testsuite/ld-elf/comm-data.exp | 44 +++++++++++++----- ld/testsuite/ld-elf/pr26580-1.sd | 4 ++ ld/testsuite/ld-elf/pr26580-2.sd | 4 ++ ld/testsuite/ld-elf/pr26580-3.out | 2 + ld/testsuite/ld-elf/pr26580-4.out | 2 + ld/testsuite/ld-elf/pr26580-a.c | 20 +++++++++ ld/testsuite/ld-elf/pr26580-a.s | 10 +++++ ld/testsuite/ld-elf/pr26580-b.c | 3 ++ ld/testsuite/ld-elf/pr26580-b.s | 10 +++++ ld/testsuite/ld-elf/shared.exp | 11 +++++ 13 files changed, 159 insertions(+), 50 deletions(-) create mode 100644 ld/testsuite/ld-elf/pr26580-1.sd create mode 100644 ld/testsuite/ld-elf/pr26580-2.sd create mode 100644 ld/testsuite/ld-elf/pr26580-3.out create mode 100644 ld/testsuite/ld-elf/pr26580-4.out create mode 100644 ld/testsuite/ld-elf/pr26580-a.c create mode 100644 ld/testsuite/ld-elf/pr26580-a.s create mode 100644 ld/testsuite/ld-elf/pr26580-b.c create mode 100644 ld/testsuite/ld-elf/pr26580-b.s diff --git a/bfd/ChangeLog b/bfd/ChangeLog index 22cee1d6642..4faa8f23511 100644 --- a/bfd/ChangeLog +++ b/bfd/ChangeLog @@ -1,3 +1,15 @@ +2020-09-08 Alan Modra + + 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 * plugin.c (bfd_plugin_canonicalize_symtab): Handle the case of an diff --git a/bfd/elflink.c b/bfd/elflink.c index 1384c1a46b8..0e339f3c1ea 100644 --- 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 0a11b79c674..4b42f05171c 100644 --- a/ld/ChangeLog +++ b/ld/ChangeLog @@ -1,3 +1,16 @@ +2020-09-08 Alan Modra + + * 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 * 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 87ae3b29912..16b8e2b8ed2 100644 --- 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 index 00000000000..b5adab054da --- /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 index 00000000000..265b880f81a --- /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 index 00000000000..566696f8b5d --- /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 index 00000000000..d91deeae3c3 --- /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 index 00000000000..5557997e706 --- /dev/null +++ b/ld/testsuite/ld-elf/pr26580-a.c @@ -0,0 +1,20 @@ +#include + +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 index 00000000000..3b70ccc2cd7 --- /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 index 00000000000..e7e13d66301 --- /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 index 00000000000..cb06378d370 --- /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 87a40a362ed..6f53f9c3170 100644 --- 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. -- 2.30.2