* PR26580, Size and alignment of commons vs as-needed shared lib
@ 2020-09-08 13:01 Alan Modra
0 siblings, 0 replies; 2+ messages in thread
From: Alan Modra @ 2020-09-08 13:01 UTC (permalink / raw)
To: binutils
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/elflink.c b/bfd/elflink.c
index 1384c1a46b..0e339f3c1e 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 0a11b79c67..4b42f05171 100644
--- 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 87ae3b2991..16b8e2b8ed 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 0000000000..b5adab054d
--- /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 0000000000..265b880f81
--- /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 0000000000..566696f8b5
--- /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 0000000000..d91deeae3c
--- /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 0000000000..5557997e70
--- /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
index 0000000000..3b70ccc2cd
--- /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 0000000000..e7e13d6630
--- /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 0000000000..cb06378d37
--- /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 87a40a362e..6f53f9c317 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.17.1
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: PR26580, Size and alignment of commons vs as-needed shared lib
@ 2020-09-10 9:28 Alan Modra
0 siblings, 0 replies; 2+ messages in thread
From: Alan Modra @ 2020-09-10 9:28 UTC (permalink / raw)
To: binutils
Some MIPS targets, for reasons I didn't analyse, use the larger common
symbol in a shared lib rather than a smaller common in an executable.
That doesn't seem unreasonable, so allow that to pass for pr26580-2.
bfin-elf complains about not supporting copy relocs, but it's quite
silly to want a copy reloc for common symbols, so leave the fail
there. mn10300-elf and score-elf both fail the test with "PHDR
segment not covered by LOAD segment". Other tests fail similarly so
one more doesn't hurt. The failure is a consequence of supporting
dynamic objects but setting EMBEDDED in ld scripts.
PR 26580
* testsuite/ld-elf/pr26580-2.sd: Accept undefined symbol.
diff --git a/ld/testsuite/ld-elf/pr26580-2.sd b/ld/testsuite/ld-elf/pr26580-2.sd
index 265b880f81..10474ca23d 100644
--- a/ld/testsuite/ld-elf/pr26580-2.sd
+++ b/ld/testsuite/ld-elf/pr26580-2.sd
@@ -1,4 +1,4 @@
#...
-.* 8 OBJECT GLOBAL DEFAULT .* one
+.* (8 OBJECT GLOBAL DEFAULT .*|0 OBJECT GLOBAL DEFAULT UND) one
#pass
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-09-10 9:28 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-08 13:01 PR26580, Size and alignment of commons vs as-needed shared lib Alan Modra
2020-09-10 9:28 Alan Modra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).