* [PATCH] elf: Properly handle versioned symbol references in LTO IR
@ 2024-04-07 21:46 H.J. Lu
0 siblings, 0 replies; only message in thread
From: H.J. Lu @ 2024-04-07 21:46 UTC (permalink / raw)
To: binutils; +Cc: amodra
Update _bfd_elf_merge_symbol and _bfd_elf_add_default_symbol to properly
handle the versioned symbol from the first definition during IR rescan,
which should always overrides the old definition.
bfd/
PR ld/31615
* elflink.c (_bfd_elf_merge_symbol): Add an argument for the
symbol from the first definition during IR rescan, which always
overrides the old definition.
(_bfd_elf_add_default_symbol): Add an argument for the symbol
from the first definition during IR rescan. Update the default
version and its indirect symbols for symbol from the first
definition during IR rescan directly.
(elf_link_add_object_symbols): Pass true to _bfd_elf_merge_symbol
for the symbol from the first definition during IR rescan.
ld/
PR ld/31615
* testsuite/ld-plugin/lto.exp: Run ld/31615 tests.
* testsuite/ld-plugin/pr31615.ver: New file.
* testsuite/ld-plugin/pr31615a.c: Likewise.
* testsuite/ld-plugin/pr31615b.c: Likewise.
* testsuite/ld-plugin/pr31615c.c: Likewise.
* testsuite/ld-plugin/pr31615d.c: Likewise.
---
bfd/elflink.c | 133 ++++++++++++++++++++---------
ld/testsuite/ld-plugin/lto.exp | 34 ++++++++
ld/testsuite/ld-plugin/pr31615.ver | 4 +
ld/testsuite/ld-plugin/pr31615a.c | 8 ++
ld/testsuite/ld-plugin/pr31615b.c | 7 ++
ld/testsuite/ld-plugin/pr31615c.c | 8 ++
ld/testsuite/ld-plugin/pr31615d.c | 5 ++
7 files changed, 159 insertions(+), 40 deletions(-)
create mode 100644 ld/testsuite/ld-plugin/pr31615.ver
create mode 100644 ld/testsuite/ld-plugin/pr31615a.c
create mode 100644 ld/testsuite/ld-plugin/pr31615b.c
create mode 100644 ld/testsuite/ld-plugin/pr31615c.c
create mode 100644 ld/testsuite/ld-plugin/pr31615d.c
diff --git a/bfd/elflink.c b/bfd/elflink.c
index c73470276cd..58c4f88a0af 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -1087,7 +1087,8 @@ elf_merge_st_other (bfd *abfd, struct elf_link_hash_entry *h,
overriding a new definition. We set TYPE_CHANGE_OK if it is OK for
the type to change. We set SIZE_CHANGE_OK if it is OK for the size
to change. By OK to change, we mean that we shouldn't warn if the
- type or size does change. */
+ type or size does change. If RESCAN_DEFINITION is true, the new
+ symbol is from the first definition during IR rescan. */
static bool
_bfd_elf_merge_symbol (bfd *abfd,
@@ -1104,7 +1105,8 @@ _bfd_elf_merge_symbol (bfd *abfd,
bfd **override,
bool *type_change_ok,
bool *size_change_ok,
- bool *matched)
+ bool *matched,
+ bool rescan_definition)
{
asection *sec, *oldsec;
struct elf_link_hash_entry *h;
@@ -1556,6 +1558,12 @@ _bfd_elf_merge_symbol (bfd *abfd,
return true;
}
+ /* The new dynamic definition from the first definition during IR
+ rescan always overrides the old definition if it is referenced by
+ non-IR objects. */
+ if (!h->root.non_ir_ref_regular)
+ rescan_definition = false;
+
/* If a new weak symbol definition comes from a regular file and the
old symbol comes from a dynamic library, we treat the new one as
strong. Similarly, an old weak symbol definition from a regular
@@ -1572,7 +1580,9 @@ _bfd_elf_merge_symbol (bfd *abfd,
Do this before setting *type_change_ok or *size_change_ok so that
we warn properly when dynamic library symbols are overridden. */
- if (newdef && !newdyn && (olddyn || h->root.ldscript_def))
+ if (newdef
+ && (!newdyn || rescan_definition)
+ && (olddyn || h->root.ldscript_def))
newweak = false;
if (olddef && newdyn)
oldweak = false;
@@ -1705,6 +1715,7 @@ _bfd_elf_merge_symbol (bfd *abfd,
object to override a weak symbol in a shared object. */
if (newdyn
+ && !rescan_definition
&& newdef
&& (olddef
|| (h->root.type == bfd_link_hash_common
@@ -1781,7 +1792,7 @@ _bfd_elf_merge_symbol (bfd *abfd,
symbol is a function or is weak. */
flip = NULL;
- if (!newdyn
+ if ((!newdyn || rescan_definition)
&& (newdef
|| (bfd_is_com_section (sec)
&& (oldweak || oldfunc)))
@@ -1877,7 +1888,8 @@ _bfd_elf_merge_symbol (bfd *abfd,
h->root.type = bfd_link_hash_indirect;
h->root.u.i.link = (struct bfd_link_hash_entry *) flip;
(*bed->elf_backend_copy_indirect_symbol) (info, flip, h);
- if (h->def_dynamic)
+ /* Handle the new dynamic definition from the first definition. */
+ if (h->def_dynamic && !newdyn)
{
h->def_dynamic = 0;
flip->ref_dynamic = 1;
@@ -1890,7 +1902,9 @@ _bfd_elf_merge_symbol (bfd *abfd,
/* This function is called to create an indirect symbol from the
default for the symbol with the default version if needed. The
symbol is described by H, NAME, SYM, SEC, and VALUE. We
- set DYNSYM if the new indirect symbol is dynamic. */
+ set DYNSYM if the new indirect symbol is dynamic. If
+ RESCAN_DEFINITION is true, the new symbol is from the first
+ definition during IR rescan. */
static bool
_bfd_elf_add_default_symbol (bfd *abfd,
@@ -1901,7 +1915,8 @@ _bfd_elf_add_default_symbol (bfd *abfd,
asection *sec,
bfd_vma value,
bfd **poldbfd,
- bool *dynsym)
+ bool *dynsym,
+ bool rescan_definition)
{
bool type_change_ok;
bool size_change_ok;
@@ -1973,9 +1988,12 @@ _bfd_elf_add_default_symbol (bfd *abfd,
tmp_sec = sec;
if (!_bfd_elf_merge_symbol (abfd, info, shortname, sym, &tmp_sec, &value,
&hi, poldbfd, NULL, NULL, &skip, &override,
- &type_change_ok, &size_change_ok, &matched))
+ &type_change_ok, &size_change_ok, &matched,
+ rescan_definition))
return false;
+ bool rescan_indirect = false;
+
if (skip)
goto nondefault;
@@ -2020,12 +2038,30 @@ _bfd_elf_add_default_symbol (bfd *abfd,
bh->type = bfd_link_hash_undefined;
bh->u.undef.abfd = bh->u.def.section->owner;
}
- if (! (_bfd_generic_link_add_one_symbol
- (info, abfd, shortname, BSF_INDIRECT,
- bfd_ind_section_ptr,
- 0, name, false, collect, &bh)))
- return false;
- hi = (struct elf_link_hash_entry *) bh;
+ if (rescan_definition
+ && h->root.non_ir_ref_regular
+ && h->root.type == bfd_link_hash_indirect)
+ {
+ /* _bfd_elf_merge_symbol may change the default version
+ from the first definition during IR rescan to an
+ indirect symbol. Change it to the normal definition
+ and update the default symbol directly. */
+ rescan_indirect = true;
+ h->root.type = bfd_link_hash_defined;
+ h->root.u.def.section = sec;
+ h->root.u.def.value = 0;
+ hi->root.type = bfd_link_hash_indirect;
+ hi->root.u.i.link = &h->root;
+ }
+ if (hi->root.type != bfd_link_hash_indirect)
+ {
+ if (! (_bfd_generic_link_add_one_symbol
+ (info, abfd, shortname, BSF_INDIRECT,
+ bfd_ind_section_ptr,
+ 0, name, false, collect, &bh)))
+ return false;
+ hi = (struct elf_link_hash_entry *) bh;
+ }
}
}
else
@@ -2132,7 +2168,8 @@ _bfd_elf_add_default_symbol (bfd *abfd,
tmp_sec = sec;
if (!_bfd_elf_merge_symbol (abfd, info, shortname, sym, &tmp_sec, &value,
&hi, poldbfd, NULL, NULL, &skip, &override,
- &type_change_ok, &size_change_ok, &matched))
+ &type_change_ok, &size_change_ok, &matched,
+ rescan_definition))
return false;
if (skip)
@@ -2170,12 +2207,27 @@ _bfd_elf_add_default_symbol (bfd *abfd,
}
else
{
- bh = &hi->root;
- if (! (_bfd_generic_link_add_one_symbol
- (info, abfd, shortname, BSF_INDIRECT,
- bfd_ind_section_ptr, 0, name, false, collect, &bh)))
- return false;
- hi = (struct elf_link_hash_entry *) bh;
+ if (rescan_indirect)
+ {
+ /* _bfd_elf_merge_symbol may change the default version
+ from the first definition during IR rescan to an indirect
+ symbol. Change it to the normal definition and update
+ the hidden version directly. */
+ h->root.type = bfd_link_hash_defined;
+ h->root.u.def.section = sec;
+ h->root.u.def.value = 0;
+ hi->root.type = bfd_link_hash_indirect;
+ hi->root.u.i.link = &h->root;
+ }
+ if (hi->root.type != bfd_link_hash_indirect)
+ {
+ bh = &hi->root;
+ if (! (_bfd_generic_link_add_one_symbol
+ (info, abfd, shortname, BSF_INDIRECT,
+ bfd_ind_section_ptr, 0, name, false, collect, &bh)))
+ return false;
+ hi = (struct elf_link_hash_entry *) bh;
+ }
}
/* If there is a duplicate definition somewhere, then HI may not
@@ -4922,6 +4974,7 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
flagword flags;
const char *name;
bool must_copy_name = false;
+ bool rescan_definition = false;
struct elf_link_hash_entry *h;
struct elf_link_hash_entry *hi;
bool definition;
@@ -5235,11 +5288,25 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
isym->st_other = (STV_HIDDEN
| (isym->st_other & ~ELF_ST_VISIBILITY (-1)));
+ if (htab->first_hash != NULL
+ && (elf_dyn_lib_class (abfd) & DYN_AS_NEEDED) != 0)
+ {
+ /* When reloading --as-needed shared objects for new
+ symbols added from IR inputs, if this shared object
+ has the first definition, use it. */
+ struct elf_link_first_hash_entry *e
+ = ((struct elf_link_first_hash_entry *)
+ bfd_hash_lookup (htab->first_hash, name, false,
+ false));
+ if (e != NULL && e->abfd == abfd)
+ rescan_definition = true;
+ }
+
if (!_bfd_elf_merge_symbol (abfd, info, name, isym, &sec, &value,
sym_hash, &old_bfd, &old_weak,
&old_alignment, &skip, &override,
&type_change_ok, &size_change_ok,
- &matched))
+ &matched, rescan_definition))
goto error_free_vers;
if (skip)
@@ -5253,23 +5320,7 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
/* Override a definition only if the new symbol matches the
existing one. */
if (override && matched)
- {
- definition = false;
- if (htab->first_hash != NULL
- && (elf_dyn_lib_class (abfd) & DYN_AS_NEEDED) != 0
- && h->root.non_ir_ref_regular)
- {
- /* When reloading --as-needed shared objects for new
- symbols added from IR inputs, if this shared object
- has the first definition, use it. */
- struct elf_link_first_hash_entry *e
- = ((struct elf_link_first_hash_entry *)
- bfd_hash_lookup (htab->first_hash, name, false,
- false));
- if (e != NULL && e->abfd == abfd)
- definition = true;
- }
- }
+ definition = false;
if (h->versioned != unversioned
&& elf_tdata (abfd)->verdef != NULL
@@ -5418,7 +5469,9 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
&& !(hi != h
&& hi->versioned == versioned_hidden))
if (!_bfd_elf_add_default_symbol (abfd, info, h, name, isym,
- sec, value, &old_bfd, &dynsym))
+ sec, value, &old_bfd,
+ &dynsym,
+ rescan_definition))
goto error_free_vers;
/* Check the alignment when a common symbol is involved. This
diff --git a/ld/testsuite/ld-plugin/lto.exp b/ld/testsuite/ld-plugin/lto.exp
index b56d71a668b..35ce38731d0 100644
--- a/ld/testsuite/ld-plugin/lto.exp
+++ b/ld/testsuite/ld-plugin/lto.exp
@@ -555,6 +555,30 @@ set lto_link_elf_tests [list \
"" \
"pr31482c.so" \
] \
+ [list \
+ "Build pr31615b.so" \
+ "-shared -Wl,--version-script=pr31615.ver" \
+ "-fPIC" \
+ {pr31615b.c} \
+ "" \
+ "pr31615b.so" \
+ ] \
+ [list \
+ "Build pr31615c.so" \
+ "-shared -Wl,--version-script=pr31615.ver" \
+ "-fPIC" \
+ {pr31615c.c} \
+ "" \
+ "pr31615c.so" \
+ ] \
+ [list \
+ "Build pr31615d.so" \
+ "-shared -Wl,--version-script=pr31615.ver" \
+ "-fPIC" \
+ {pr31615d.c} \
+ "" \
+ "pr31615d.so" \
+ ] \
]
# PR 14918 checks that libgcc is not spuriously included in a shared link of
@@ -754,6 +778,16 @@ set lto_run_elf_shared_tests [list \
{-Wl,--as-needed,-R,tmpdir} {} \
{pr31482a.c} {pr31489b.exe} {pass1.out} {-flto} {c} {} \
{tmpdir/pr31482c.so tmpdir/pr31482b.a}] \
+ [list {pr31615a} \
+ {-Wl,-R,tmpdir} {} \
+ {pr31615a.c} {pr31615a.exe} {pass.out} {-O3 -flto} {c} {} \
+ {-Wl,--as-needed tmpdir/pr31615b.so -Wl,--no-as-needed \
+ tmpdir/pr31615d.so}] \
+ [list {pr31615b} \
+ {-Wl,-R,tmpdir} {} \
+ {pr31615a.c} {pr31615b.exe} {pass.out} {-O3 -flto} {c} {} \
+ {-Wl,--as-needed tmpdir/pr31615c.so -Wl,--no-as-needed \
+ tmpdir/pr31615d.so}] \
]
# LTO run-time tests for ELF
diff --git a/ld/testsuite/ld-plugin/pr31615.ver b/ld/testsuite/ld-plugin/pr31615.ver
new file mode 100644
index 00000000000..f1e1f9d5cd7
--- /dev/null
+++ b/ld/testsuite/ld-plugin/pr31615.ver
@@ -0,0 +1,4 @@
+BAR {
+global:
+ bar;
+};
diff --git a/ld/testsuite/ld-plugin/pr31615a.c b/ld/testsuite/ld-plugin/pr31615a.c
new file mode 100644
index 00000000000..450b2d5a5f9
--- /dev/null
+++ b/ld/testsuite/ld-plugin/pr31615a.c
@@ -0,0 +1,8 @@
+extern void bar ();
+
+int
+main()
+{
+ bar ();
+ return 0;
+}
diff --git a/ld/testsuite/ld-plugin/pr31615b.c b/ld/testsuite/ld-plugin/pr31615b.c
new file mode 100644
index 00000000000..973dc31611b
--- /dev/null
+++ b/ld/testsuite/ld-plugin/pr31615b.c
@@ -0,0 +1,7 @@
+#include <stdio.h>
+
+void
+bar (void)
+{
+ printf ("PASS\n");
+}
diff --git a/ld/testsuite/ld-plugin/pr31615c.c b/ld/testsuite/ld-plugin/pr31615c.c
new file mode 100644
index 00000000000..dbe07889760
--- /dev/null
+++ b/ld/testsuite/ld-plugin/pr31615c.c
@@ -0,0 +1,8 @@
+#include <stdio.h>
+
+__attribute__ ((weak))
+void
+bar (void)
+{
+ printf ("PASS\n");
+}
diff --git a/ld/testsuite/ld-plugin/pr31615d.c b/ld/testsuite/ld-plugin/pr31615d.c
new file mode 100644
index 00000000000..352f5987cc3
--- /dev/null
+++ b/ld/testsuite/ld-plugin/pr31615d.c
@@ -0,0 +1,5 @@
+__attribute__ ((weak))
+void
+bar (void)
+{
+}
--
2.44.0
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2024-04-07 21:46 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-07 21:46 [PATCH] elf: Properly handle versioned symbol references in LTO IR H.J. Lu
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).