This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] x86: Check the versioned __tls_get_addr symbol
- From: "H.J. Lu" <hjl dot tools at gmail dot com>
- To: Alan Modra <amodra at gmail dot com>
- Cc: Binutils <binutils at sourceware dot org>
- Date: Sat, 20 Jan 2018 14:37:42 -0800
- Subject: Re: [PATCH] x86: Check the versioned __tls_get_addr symbol
- Authentication-results: sourceware.org; auth=none
- References: <20180119185737.GA22094@gmail.com> <20180120002033.GI20622@bubble.grove.modra.org> <CAMe9rOrETYQh=pJyC2SS+jf9XcZJM_a9mrsttJzLPuXxEcWfPg@mail.gmail.com>
On Fri, Jan 19, 2018 at 6:00 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, Jan 19, 2018 at 4:20 PM, Alan Modra <amodra@gmail.com> wrote:
>> On Fri, Jan 19, 2018 at 10:57:37AM -0800, H.J. Lu wrote:
>>> --- a/bfd/elfxx-x86.c
>>> +++ b/bfd/elfxx-x86.c
>>> @@ -856,7 +856,16 @@ _bfd_x86_elf_link_check_relocs (bfd *abfd, struct bfd_link_info *info)
>>> htab->tls_get_addr,
>>> FALSE, FALSE, FALSE);
>>> if (h != NULL)
>>> - elf_x86_hash_entry (h)->tls_get_addr = 1;
>>> + {
>>> + elf_x86_hash_entry (h)->tls_get_addr = 1;
>>> +
>>> + /* Check the versioned __tls_get_addr symbol. */
>>> + while (h->root.type == bfd_link_hash_indirect)
>>> + {
>>> + h = (struct elf_link_hash_entry *) h->root.u.i.link;
>>> + elf_x86_hash_entry (h)->tls_get_addr = 1;
>>> + }
>>> + }
>>
>> I think it would be a little more bomb proof to pass follow=TRUE on
>> the elf_link_hash_lookup call, and then always test the flag on the
>> real symbol (which would mean a patch to elf_i386_check_tls_transition
>> and elf_x86_64_check_tls_transition). We have some really odd cases
>
> I'd like to keep the change as small as possible for backporting to
> 2.30.
>
>> in _bfd_elf_merge_symbol where a versioned symbol can end up pointing
>> at the undecorated symbol. So it might be possible to have
>> __tls_get_addr@VER -> __tls_get_addr -> __tls_get_addr@@VER
>> in which case you wouldn't set the flag on __tls_get_addr@VER.
>>
>
> But I do want to cover all bases. Do you have a testcase to show
> such a scenario so that I can verify that it is covered?
This is what I checked into master and 2.30 branch. We will
revisit it in the future if there is a testcase.
--
H.J.
From b7d19c903e8768cc48b7d3984b2b923a0288b176 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Fri, 19 Jan 2018 10:53:36 -0800
Subject: [PATCH] x86: Check the versioned __tls_get_addr symbol
We need to check the versioned __tls_get_addr symbol when looking up
"__tls_get_addr".
bfd/
PR ld/22721
* elfxx-x86.c (_bfd_x86_elf_link_check_relocs): Check the
versioned __tls_get_addr symbol.
ld/
PR ld/22721
* testsuite/ld-plugin/lto.exp: Run PR ld/22721 tests.
* testsuite/ld-plugin/pr22721.t: New file.
* testsuite/ld-plugin/pr22721a.s: Likewise.
* testsuite/ld-plugin/pr22721b.c: Likewise.
---
bfd/elfxx-x86.c | 11 ++++++++++-
ld/testsuite/ld-plugin/lto.exp | 26 ++++++++++++++++++++++++++
ld/testsuite/ld-plugin/pr22721.t | 7 +++++++
ld/testsuite/ld-plugin/pr22721a.s | 8 ++++++++
ld/testsuite/ld-plugin/pr22721b.c | 7 +++++++
5 files changed, 58 insertions(+), 1 deletion(-)
create mode 100644 ld/testsuite/ld-plugin/pr22721.t
create mode 100644 ld/testsuite/ld-plugin/pr22721a.s
create mode 100644 ld/testsuite/ld-plugin/pr22721b.c
diff --git a/bfd/elfxx-x86.c b/bfd/elfxx-x86.c
index a9ee4ba387..a7db5d9dfe 100644
--- a/bfd/elfxx-x86.c
+++ b/bfd/elfxx-x86.c
@@ -856,7 +856,16 @@ _bfd_x86_elf_link_check_relocs (bfd *abfd, struct bfd_link_info *info)
htab->tls_get_addr,
FALSE, FALSE, FALSE);
if (h != NULL)
- elf_x86_hash_entry (h)->tls_get_addr = 1;
+ {
+ elf_x86_hash_entry (h)->tls_get_addr = 1;
+
+ /* Check the versioned __tls_get_addr symbol. */
+ while (h->root.type == bfd_link_hash_indirect)
+ {
+ h = (struct elf_link_hash_entry *) h->root.u.i.link;
+ elf_x86_hash_entry (h)->tls_get_addr = 1;
+ }
+ }
/* "__ehdr_start" will be defined by linker as a hidden symbol
later if it is referenced and not defined. */
diff --git a/ld/testsuite/ld-plugin/lto.exp b/ld/testsuite/ld-plugin/lto.exp
index 93f964921a..896f453def 100644
--- a/ld/testsuite/ld-plugin/lto.exp
+++ b/ld/testsuite/ld-plugin/lto.exp
@@ -510,6 +510,32 @@ if { [at_least_gcc_version 4 7] } {
} {
fail $testname
}
+
+ run_cc_link_tests [list \
+ [list \
+ "Build pr22721a.so" \
+ "-shared -nostdlib -nostartfiles -Wl,-version-script,pr22721.t" \
+ "" \
+ {pr22721a.s} \
+ {} \
+ "pr22721a.so" \
+ ] \
+ [list \
+ "Build pr22721b.o" \
+ "$plug_opt" \
+ "-O2 -fPIC -flto $lto_no_fat" \
+ {pr22721b.c} \
+ ] \
+ [list \
+ "Build PR ld/pr22721" \
+ "-O2 -flto -fuse-linker-plugin -nostdlib -nostartfiles \
+ -Wl,-e,_start tmpdir/pr22721b.o tmpdir/pr22721a.so" \
+ "" \
+ {dummy.c} \
+ {} \
+ "pr22721.exe"
+ ] \
+ ]
}
set testname "PR ld/12942 (3)"
set exec_output [run_host_cmd "$CXX" "-O2 -flto -fuse-linker-plugin tmpdir/pr12942b.o tmpdir/pr12942a.o"]
diff --git a/ld/testsuite/ld-plugin/pr22721.t b/ld/testsuite/ld-plugin/pr22721.t
new file mode 100644
index 0000000000..92695296ba
--- /dev/null
+++ b/ld/testsuite/ld-plugin/pr22721.t
@@ -0,0 +1,7 @@
+SUNWprivate_1.1 {
+global:
+ __tls_get_addr;
+ ___tls_get_addr;
+local:
+ *;
+};
diff --git a/ld/testsuite/ld-plugin/pr22721a.s b/ld/testsuite/ld-plugin/pr22721a.s
new file mode 100644
index 0000000000..e06079ee9b
--- /dev/null
+++ b/ld/testsuite/ld-plugin/pr22721a.s
@@ -0,0 +1,8 @@
+ .text
+ .globl __tls_get_addr
+ .globl ___tls_get_addr
+ .type __tls_get_addr,%function
+ .set ___tls_get_addr, __tls_get_addr
+__tls_get_addr:
+ .byte 0
+ .size __tls_get_addr, .-__tls_get_addr
diff --git a/ld/testsuite/ld-plugin/pr22721b.c b/ld/testsuite/ld-plugin/pr22721b.c
new file mode 100644
index 0000000000..ec42cdc3bd
--- /dev/null
+++ b/ld/testsuite/ld-plugin/pr22721b.c
@@ -0,0 +1,7 @@
+__thread int foo_var = 1;
+
+int
+_start (void)
+{
+ return foo_var;
+}
--
2.14.3