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.
This commit is contained in:
Alan Modra
2020-09-08 13:02:31 +09:30
parent 7ade7fba75
commit 7ba115508a
13 changed files with 160 additions and 51 deletions

View File

@ -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

View File

@ -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. */
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;
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;
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;
}

View File

@ -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.

View File

@ -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" \
] \
]

View File

@ -0,0 +1,4 @@
#...
.* 1 OBJECT GLOBAL DEFAULT .* one
#pass

View File

@ -0,0 +1,4 @@
#...
.* 8 OBJECT GLOBAL DEFAULT .* one
#pass

View File

@ -0,0 +1,2 @@
library not loaded
alignment 1

View File

@ -0,0 +1,2 @@
library loaded
alignment 8

View File

@ -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;
}

View File

@ -0,0 +1,10 @@
.text
.globl _start
.globl __start
_start:
__start:
.ifdef HPUX
one .comm 1
.else
.comm one, 1, 1
.endif

View File

@ -0,0 +1,3 @@
long long x, y, z;
void foo (void) {}

View File

@ -0,0 +1,10 @@
.text
.globl _start
.globl __start
_start:
__start:
.ifdef HPUX
one .comm 8
.else
.comm one, 8, 8
.endif

View File

@ -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.