This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] PR ld/18720: Properly merge hidden versioned symbol
On Mon, Nov 21, 2016 at 4:14 PM, Alan Modra <amodra@gmail.com> wrote:
> On Mon, Nov 21, 2016 at 01:09:20PM -0800, H.J. Lu wrote:
>> On Sun, Nov 20, 2016 at 7:09 PM, Alan Modra <amodra@gmail.com> wrote:
>> > On Wed, Aug 05, 2015 at 07:43:21PM -0700, H.J. Lu wrote:
>> >> I will check it in this Friday unless there is an objection.
>> >
>> > HJ, git 6e33951edc change to elf_link_output_extsym where you
>> > introduce local_bind, is broken. You can't change global syms to
>> > STB_LOCAL that late, after _bfd_elf_link_renumber_dynsyms, as otherwise
>> > you run the risk of ordering STB_LOCAL symbols after global symbols.
>> > The ELF gABI is clear that "all symbols with STB_LOCAL binding precede
>> > the weak and global symbols". (It's also wrong to test flags on a
>> > bfd_link_hash_warning symbol.)
>> >
>> > I don't have a testcase for you. I noticed the bug when looking at
>> > PR20828. Please revert the elf_link_output_extsym change and
>> > implement by setting forced_local before the last run of
>> > _bfd_elf_link_renumber_dynsyms.
>> >
>>
>> How about this patch?
>
> OK thanks, it looks almost the same as the one I threw together.
>
> You could also move the code hiding weak undefined non-default
> visibility symbols.
>
> /* If a weak undefined symbol has non-default visibility, we also
> hide it from the dynamic linker. */
> if (ELF_ST_VISIBILITY (h->other) != STV_DEFAULT
> && h->root.type == bfd_link_hash_undefweak)
> (*bed->elf_backend_hide_symbol) (eif->info, h, TRUE);
>
> /* A hidden versioned symbol in executable should be forced local if
> it is is locally defined, not referenced by shared library and not
> exported. */
> else if (bfd_link_executable (eif->info)
> etc.
>
This is what I checked in.
Thanks.
--
H.J.
From 5fcacf7f68b34b275af586e2e72f29aef5522b05 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Mon, 21 Nov 2016 13:05:44 -0800
Subject: [PATCH] Properly hide hidden versioned symbol in executable
A hidden versioned symbol in executable should be forced local if it is
locally defined, not referenced by shared library and not exported. We
must do it before _bfd_elf_link_renumber_dynsyms.
bfd/
* elflink.c (_bfd_elf_fix_symbol_flags): Hide hidden versioned
symbol in executable.
(elf_link_output_extsym): Don't change bind from global to
local when linking executable.
ld/
* testsuite/ld-elf/indirect.exp: Add a test for PR 18720.
* testsuite/ld-elf/pr18720.rd: New file.
---
bfd/elflink.c | 55 ++++++++++++++++++++--------------------
ld/testsuite/ld-elf/indirect.exp | 3 +++
ld/testsuite/ld-elf/pr18720.rd | 4 +++
3 files changed, 34 insertions(+), 28 deletions(-)
create mode 100644 ld/testsuite/ld-elf/pr18720.rd
diff --git a/bfd/elflink.c b/bfd/elflink.c
index 76f91e9..5abac8d 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -2655,18 +2655,35 @@ _bfd_elf_fix_symbol_flags (struct elf_link_hash_entry *h,
&& (h->root.u.def.section->owner->flags & (DYNAMIC | BFD_PLUGIN)) == 0)
h->def_regular = 1;
+ /* If a weak undefined symbol has non-default visibility, we also
+ hide it from the dynamic linker. */
+ if (ELF_ST_VISIBILITY (h->other) != STV_DEFAULT
+ && h->root.type == bfd_link_hash_undefweak)
+ (*bed->elf_backend_hide_symbol) (eif->info, h, TRUE);
+
+ /* A hidden versioned symbol in executable should be forced local if
+ it is is locally defined, not referenced by shared library and not
+ exported. */
+ else if (bfd_link_executable (eif->info)
+ && h->versioned == versioned_hidden
+ && !eif->info->export_dynamic
+ && !h->dynamic
+ && !h->ref_dynamic
+ && h->def_regular)
+ (*bed->elf_backend_hide_symbol) (eif->info, h, TRUE);
+
/* If -Bsymbolic was used (which means to bind references to global
symbols to the definition within the shared object), and this
symbol was defined in a regular object, then it actually doesn't
need a PLT entry. Likewise, if the symbol has non-default
visibility. If the symbol has hidden or internal visibility, we
will force it local. */
- if (h->needs_plt
- && bfd_link_pic (eif->info)
- && is_elf_hash_table (eif->info->hash)
- && (SYMBOLIC_BIND (eif->info, h)
- || ELF_ST_VISIBILITY (h->other) != STV_DEFAULT)
- && h->def_regular)
+ else if (h->needs_plt
+ && bfd_link_pic (eif->info)
+ && is_elf_hash_table (eif->info->hash)
+ && (SYMBOLIC_BIND (eif->info, h)
+ || ELF_ST_VISIBILITY (h->other) != STV_DEFAULT)
+ && h->def_regular)
{
bfd_boolean force_local;
@@ -2675,12 +2692,6 @@ _bfd_elf_fix_symbol_flags (struct elf_link_hash_entry *h,
(*bed->elf_backend_hide_symbol) (eif->info, h, force_local);
}
- /* If a weak undefined symbol has non-default visibility, we also
- hide it from the dynamic linker. */
- if (ELF_ST_VISIBILITY (h->other) != STV_DEFAULT
- && h->root.type == bfd_link_hash_undefweak)
- (*bed->elf_backend_hide_symbol) (eif->info, h, TRUE);
-
/* If this is a weak defined symbol in a dynamic object, and we know
the real definition in the dynamic object, copy interesting flags
over to the real definition. */
@@ -9250,16 +9261,6 @@ elf_link_output_extsym (struct bfd_hash_entry *bh, void *data)
long indx;
int ret;
unsigned int type;
- /* A symbol is bound locally if it is forced local or it is locally
- defined, hidden versioned, not referenced by shared library and
- not exported when linking executable. */
- bfd_boolean local_bind = (h->forced_local
- || (bfd_link_executable (flinfo->info)
- && !flinfo->info->export_dynamic
- && !h->dynamic
- && !h->ref_dynamic
- && h->def_regular
- && h->versioned == versioned_hidden));
if (h->root.type == bfd_link_hash_warning)
{
@@ -9271,12 +9272,12 @@ elf_link_output_extsym (struct bfd_hash_entry *bh, void *data)
/* Decide whether to output this symbol in this pass. */
if (eoinfo->localsyms)
{
- if (!local_bind)
+ if (!h->forced_local)
return TRUE;
}
else
{
- if (local_bind)
+ if (h->forced_local)
return TRUE;
}
@@ -9493,7 +9494,7 @@ elf_link_output_extsym (struct bfd_hash_entry *bh, void *data)
abort ();
}
- if (local_bind)
+ if (h->forced_local)
{
sym.st_info = ELF_ST_INFO (STB_LOCAL, type);
/* Turn off visibility on local symbol. */
@@ -9604,10 +9605,8 @@ elf_link_output_extsym (struct bfd_hash_entry *bh, void *data)
/* Since there is no version information in the dynamic string,
if there is no version info in symbol version section, we will
have a run-time problem if not linking executable, referenced
- by shared library, not locally defined, or not bound locally.
- */
+ by shared library, or not bound locally. */
if (h->verinfo.verdef == NULL
- && !local_bind
&& (!bfd_link_executable (flinfo->info)
|| h->ref_dynamic
|| !h->def_regular))
diff --git a/ld/testsuite/ld-elf/indirect.exp b/ld/testsuite/ld-elf/indirect.exp
index b4766fd..3176210 100644
--- a/ld/testsuite/ld-elf/indirect.exp
+++ b/ld/testsuite/ld-elf/indirect.exp
@@ -91,6 +91,9 @@ set build_tests {
{"Build pr18720b1.o"
"-r -nostdlib tmpdir/pr18720b.o" ""
{dummy.c} {} "pr18720b1.o"}
+ {"Build pr18720a"
+ "tmpdir/pr18720a.o tmpdir/pr18720b.o tmpdir/libpr18720c.so" ""
+ {check-ptr-eq.c} {{readelf {--dyn-syms} pr18720.rd}} "pr18720a"}
{"Build libpr19553b.so"
"-shared -Wl,--version-script=pr19553.map" "-fPIC"
{pr19553b.c} {} "libpr19553b.so"}
diff --git a/ld/testsuite/ld-elf/pr18720.rd b/ld/testsuite/ld-elf/pr18720.rd
new file mode 100644
index 0000000..b5e848f
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr18720.rd
@@ -0,0 +1,4 @@
+#failif
+#...
+.* found at index >= .dynsym's sh_info value .*
+#...
--
2.7.4