remove remaining traces of __tls_get_new

Submitted by Szabolcs Nagy on Sept. 29, 2019, 1:05 p.m.

Details

Message ID 20190929130527.GR22009@port70.net
State New
Series "remove remaining traces of __tls_get_new"
Headers show

Commit Message

Szabolcs Nagy Sept. 29, 2019, 1:05 p.m.
reported on irc by malc_

Patch hide | download patch | download mbox

From 4a7090ab76d81b59f57a83bce9d22582e35a8b2b Mon Sep 17 00:00:00 2001
From: Szabolcs Nagy <nsz@port70.net>
Date: Sun, 29 Sep 2019 12:25:39 +0000
Subject: [PATCH] remove remaining traces of __tls_get_new

Some declarations of __tls_get_new were left in the code, even
though the definition got removed in

  commit 9d44b6460ab603487dab4d916342d9ba4467e6b9
  install dynamic tls synchronously at dlopen, streamline access

this can make the build fail with

  ld: lib/libc.so: hidden symbol `__tls_get_new' isn't defined

when libc.so is linked without --gc-sections, because a .hidden
declaration in asm code creates a reference even if the symbol
is not actually used.
---
 src/internal/pthread_impl.h | 1 -
 src/ldso/aarch64/tlsdesc.s  | 6 +-----
 src/ldso/arm/tlsdesc.S      | 2 --
 src/ldso/i386/tlsdesc.s     | 2 --
 src/ldso/x86_64/tlsdesc.s   | 2 --
 5 files changed, 1 insertion(+), 12 deletions(-)

diff --git a/src/internal/pthread_impl.h b/src/internal/pthread_impl.h
index 9b001421..5742dfc5 100644
--- a/src/internal/pthread_impl.h
+++ b/src/internal/pthread_impl.h
@@ -125,7 +125,6 @@  struct __timer {
 	 0x80000000 })
 
 void *__tls_get_addr(tls_mod_off_t *);
-hidden void *__tls_get_new(tls_mod_off_t *);
 hidden int __init_tp(void *);
 hidden void *__copy_tls(unsigned char *);
 hidden void __reset_tls();
diff --git a/src/ldso/aarch64/tlsdesc.s b/src/ldso/aarch64/tlsdesc.s
index 04d97e73..c6c685b3 100644
--- a/src/ldso/aarch64/tlsdesc.s
+++ b/src/ldso/aarch64/tlsdesc.s
@@ -9,15 +9,11 @@  __tlsdesc_static:
 	ldr x0,[x0,#8]
 	ret
 
-.hidden __tls_get_new
-
 // size_t __tlsdesc_dynamic(size_t *a)
 // {
 // 	struct {size_t modidx,off;} *p = (void*)a[1];
 // 	size_t *dtv = *(size_t**)(tp - 8);
-// 	if (p->modidx <= dtv[0])
-// 		return dtv[p->modidx] + p->off - tp;
-// 	return __tls_get_new(p) - tp;
+// 	return dtv[p->modidx] + p->off - tp;
 // }
 .global __tlsdesc_dynamic
 .hidden __tlsdesc_dynamic
diff --git a/src/ldso/arm/tlsdesc.S b/src/ldso/arm/tlsdesc.S
index 2bb75a1b..3ae133c9 100644
--- a/src/ldso/arm/tlsdesc.S
+++ b/src/ldso/arm/tlsdesc.S
@@ -8,8 +8,6 @@  __tlsdesc_static:
 	ldr r0,[r0]
 	bx lr
 
-.hidden __tls_get_new
-
 .global __tlsdesc_dynamic
 .hidden __tlsdesc_dynamic
 .type __tlsdesc_dynamic,%function
diff --git a/src/ldso/i386/tlsdesc.s b/src/ldso/i386/tlsdesc.s
index a5c0100c..32c81766 100644
--- a/src/ldso/i386/tlsdesc.s
+++ b/src/ldso/i386/tlsdesc.s
@@ -6,8 +6,6 @@  __tlsdesc_static:
 	mov 4(%eax),%eax
 	ret
 
-.hidden __tls_get_new
-
 .global __tlsdesc_dynamic
 .hidden __tlsdesc_dynamic
 .type __tlsdesc_dynamic,@function
diff --git a/src/ldso/x86_64/tlsdesc.s b/src/ldso/x86_64/tlsdesc.s
index 0151d15c..e08f1d7d 100644
--- a/src/ldso/x86_64/tlsdesc.s
+++ b/src/ldso/x86_64/tlsdesc.s
@@ -6,8 +6,6 @@  __tlsdesc_static:
 	mov 8(%rax),%rax
 	ret
 
-.hidden __tls_get_new
-
 .global __tlsdesc_dynamic
 .hidden __tlsdesc_dynamic
 .type __tlsdesc_dynamic,@function
-- 
2.21.0


Comments

Rich Felker Sept. 29, 2019, 8:56 p.m.
On Sun, Sep 29, 2019 at 03:05:27PM +0200, Szabolcs Nagy wrote:
> reported on irc by malc_

> >From 4a7090ab76d81b59f57a83bce9d22582e35a8b2b Mon Sep 17 00:00:00 2001
> From: Szabolcs Nagy <nsz@port70.net>
> Date: Sun, 29 Sep 2019 12:25:39 +0000
> Subject: [PATCH] remove remaining traces of __tls_get_new
> 
> Some declarations of __tls_get_new were left in the code, even
> though the definition got removed in
> 
>   commit 9d44b6460ab603487dab4d916342d9ba4467e6b9
>   install dynamic tls synchronously at dlopen, streamline access
> 
> this can make the build fail with
> 
>   ld: lib/libc.so: hidden symbol `__tls_get_new' isn't defined
> 
> when libc.so is linked without --gc-sections, because a .hidden
> declaration in asm code creates a reference even if the symbol
> is not actually used.

This is definitely a tooling bug. There is no reference to the symbol,
only declarations of it. I think it's a good idea to cleanup the
spurious mentions of it anyway, though.

Rich
Fangrui Song Oct. 22, 2019, 12:50 a.m.
On 2019-09-29, Rich Felker wrote:
>On Sun, Sep 29, 2019 at 03:05:27PM +0200, Szabolcs Nagy wrote:
>> reported on irc by malc_
>
>> >From 4a7090ab76d81b59f57a83bce9d22582e35a8b2b Mon Sep 17 00:00:00 2001
>> From: Szabolcs Nagy <nsz@port70.net>
>> Date: Sun, 29 Sep 2019 12:25:39 +0000
>> Subject: [PATCH] remove remaining traces of __tls_get_new
>>
>> Some declarations of __tls_get_new were left in the code, even
>> though the definition got removed in
>>
>>   commit 9d44b6460ab603487dab4d916342d9ba4467e6b9
>>   install dynamic tls synchronously at dlopen, streamline access
>>
>> this can make the build fail with
>>
>>   ld: lib/libc.so: hidden symbol `__tls_get_new' isn't defined
>>
>> when libc.so is linked without --gc-sections, because a .hidden
>> declaration in asm code creates a reference even if the symbol
>> is not actually used.
>
>This is definitely a tooling bug. There is no reference to the symbol,
>only declarations of it. I think it's a good idea to cleanup the
>spurious mentions of it anyway, though.

I think it is hard to simply state that this is a tooling bug.
For the visibility attribute of a symbol, it may still be emitted into
the symbol table even if it is unused. For example:

.protected foo => STV_PROTECTED foo
.hidden foo => STV_HIDDEN foo

A relocation referencing a symbol may be dropped due to --gc-sections.
Shall we consider the symbol unused if all relocations to it are
dropped?
Rich Felker Oct. 22, 2019, 2:59 a.m.
On Mon, Oct 21, 2019 at 05:50:30PM -0700, Fangrui Song wrote:
> On 2019-09-29, Rich Felker wrote:
> >On Sun, Sep 29, 2019 at 03:05:27PM +0200, Szabolcs Nagy wrote:
> >>reported on irc by malc_
> >
> >>>From 4a7090ab76d81b59f57a83bce9d22582e35a8b2b Mon Sep 17 00:00:00 2001
> >>From: Szabolcs Nagy <nsz@port70.net>
> >>Date: Sun, 29 Sep 2019 12:25:39 +0000
> >>Subject: [PATCH] remove remaining traces of __tls_get_new
> >>
> >>Some declarations of __tls_get_new were left in the code, even
> >>though the definition got removed in
> >>
> >>  commit 9d44b6460ab603487dab4d916342d9ba4467e6b9
> >>  install dynamic tls synchronously at dlopen, streamline access
> >>
> >>this can make the build fail with
> >>
> >>  ld: lib/libc.so: hidden symbol `__tls_get_new' isn't defined
> >>
> >>when libc.so is linked without --gc-sections, because a .hidden
> >>declaration in asm code creates a reference even if the symbol
> >>is not actually used.
> >
> >This is definitely a tooling bug. There is no reference to the symbol,
> >only declarations of it. I think it's a good idea to cleanup the
> >spurious mentions of it anyway, though.
> 
> I think it is hard to simply state that this is a tooling bug.
> For the visibility attribute of a symbol, it may still be emitted into
> the symbol table even if it is unused. For example:
> 
> ..protected foo => STV_PROTECTED foo
> ..hidden foo => STV_HIDDEN foo
> 
> A relocation referencing a symbol may be dropped due to --gc-sections.
> Shall we consider the symbol unused if all relocations to it are
> dropped?

I don't think --gc-sections is relevant except that it happened to
make the problem go away. This isn't about ld spuriously thinking
there's a reference to a symbol after --gc-sections dropped the
sections containing relocations referencing it.

Rather, it looks like a bug in --no-undefined, which as documented
should only produce an error when there's a *reference* (i.e. an
outstanding relocation using) the undefined symbol, instead erroring
out just because an undefined symbol appears in the linker's working
symbol table for the purpose of tracking its declared visibility.

Rich