This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] aarch64: Use explicit offsets in _dl_tlsdesc_dynamic
- From: Florian Weimer <fweimer at redhat dot com>
- To: Szabolcs Nagy <szabolcs dot nagy at arm dot com>, libc-alpha at sourceware dot org
- Cc: nd at arm dot com
- Date: Fri, 2 Dec 2016 12:52:40 +0100
- Subject: Re: [PATCH] aarch64: Use explicit offsets in _dl_tlsdesc_dynamic
- Authentication-results: sourceware.org; auth=none
- References: <20161201144952.1A8AB439942E0@oldenburg.str.redhat.com> <58403C89.1010702@arm.com>
On 12/01/2016 04:06 PM, Szabolcs Nagy wrote:
On 01/12/16 14:49, Florian Weimer wrote:
Commit 389d1f1b232b3d6b9d73ee2c50e543ace6675621 (“Partial ILP32
support for aarch64”) broke dynamic TLS support because a load
offset changed:
0000000000000030 <_dl_tlsdesc_dynamic>:
30: a9bc7bfd stp x29, x30, [sp,#-64]!
34: 910003fd mov x29, sp
38: a9020be1 stp x1, x2, [sp,#32]
3c: a90313e3 stp x3, x4, [sp,#48]
40: d53bd044 mrs x4, tpidr_el0
44: c8dffc1f ldar xzr, [x0]
48: f9400401 ldr x1, [x0,#8]
4c: f9400080 ldr x0, [x4]
50: f9400823 ldr x3, [x1,#16]
54: f9400002 ldr x2, [x0]
58: eb02007f cmp x3, x2
5c: 540001a8 b.hi 90 <_dl_tlsdesc_dynamic+0x60>
60: f9400022 ldr x2, [x1]
64: 8b021000 add x0, x0, x2, lsl #4
68: f9400000 ldr x0, [x0]
6c: b100041f cmn x0, #0x1
70: 54000100 b.eq 90 <_dl_tlsdesc_dynamic+0x60>
- 74: f9400421 ldr x1, [x1,#8]
+ 74: f9400821 ldr x1, [x1,#16]
78: 8b010000 add x0, x0, x1
…
nice catch.
It totally broke our Fedora builders because we still carry a patch
which disables static TLS allocation.
This commit introduces explicit struct offsets, generated
from the C headers, fixing the regression.
2016-12-01 Florian Weimer <fweimer@redhat.com>
* sysdeps/aarch64/tlsdesc.sym (TCBHEAD_DTV, DTV_COUNTER)
(TLS_DTV_UNALLOCATED): Add.
* sysdeps/aarch64/dl-tlsdesc.S (_dl_tlsdesc_dynamic): Use explicit
offsets.
diff --git a/sysdeps/aarch64/dl-tlsdesc.S b/sysdeps/aarch64/dl-tlsdesc.S
index 42fa943..37a0211 100644
--- a/sysdeps/aarch64/dl-tlsdesc.S
+++ b/sysdeps/aarch64/dl-tlsdesc.S
@@ -21,7 +21,7 @@
#include <sysdep.h>
#include <tls.h>
#include "tlsdesc.h"
-
+
whitespace change
Yeah, sorry about that. Will fix.
I have a test case which triggers a crash on aarch64, but I'm not yet
sure if it actually covers this bug. It fails even with the fix above.
valground still shows an OOB write in TLS data:
==16070== Invalid write of size 8
==16070== at 0x4897C9C: init_one_static_tls (allocatestack.c:1196)
==16070== by 0x4897C9C: __pthread_init_static_tls (allocatestack.c:1213)
==16070== by 0x18A1CB: _dl_try_allocate_static_tls (dl-reloc.c:106)
==16070== by 0x19383F: _dl_tlsdesc_resolve_rela_fixup (tlsdesc.c:104)
==16070== by 0x193B03: _dl_tlsdesc_resolve_rela (dl-tlsdesc.S:288)
==16070== by 0x193B03: _dl_tlsdesc_resolve_rela (dl-tlsdesc.S:288)
==16070== by 0x193B03: _dl_tlsdesc_resolve_rela (dl-tlsdesc.S:288)
==16070== by 0x193B03: _dl_tlsdesc_resolve_rela (dl-tlsdesc.S:288)
==16070== by 0x193B03: _dl_tlsdesc_resolve_rela (dl-tlsdesc.S:288)
==16070== by 0x193B03: _dl_tlsdesc_resolve_rela (dl-tlsdesc.S:288)
==16070== by 0x193B03: _dl_tlsdesc_resolve_rela (dl-tlsdesc.S:288)
==16070== by 0x193B03: _dl_tlsdesc_resolve_rela (dl-tlsdesc.S:288)
==16070== by 0x193B03: _dl_tlsdesc_resolve_rela (dl-tlsdesc.S:288)
==16070== Address 0x5240158 is 8 bytes after a block of size 272 alloc'd
==16070== at 0x4835D4C: calloc (vg_replace_malloc.c:711)
==16070== by 0x18F9CF: allocate_dtv (dl-tls.c:322)
==16070== by 0x190117: _dl_allocate_tls (dl-tls.c:570)
==16070== by 0x4898D4B: allocate_stack (allocatestack.c:578)
==16070== by 0x4898D4B: pthread_create@@GLIBC_2.17 (pthread_create.c:539)
==16070== by 0x401CCB: xpthread_create (test-skeleton.c:691)
==16070== by 0x401CCB: do_test (tst-tls-manydynamic.c:97)
==16070== by 0x4018CF: main (test-skeleton.c:539)
I need to check if this happens before the ILP32 enablement patch, too.
Florian
aarch64: Use explicit offsets in _dl_tlsdesc_dynamic
Commit 389d1f1b232b3d6b9d73ee2c50e543ace6675621 (â??Partial ILP32
support for aarch64â??) broke dynamic TLS support because a load
offset changed:
0000000000000030 <_dl_tlsdesc_dynamic>:
30: a9bc7bfd stp x29, x30, [sp,#-64]!
34: 910003fd mov x29, sp
38: a9020be1 stp x1, x2, [sp,#32]
3c: a90313e3 stp x3, x4, [sp,#48]
40: d53bd044 mrs x4, tpidr_el0
44: c8dffc1f ldar xzr, [x0]
48: f9400401 ldr x1, [x0,#8]
4c: f9400080 ldr x0, [x4]
50: f9400823 ldr x3, [x1,#16]
54: f9400002 ldr x2, [x0]
58: eb02007f cmp x3, x2
5c: 540001a8 b.hi 90 <_dl_tlsdesc_dynamic+0x60>
60: f9400022 ldr x2, [x1]
64: 8b021000 add x0, x0, x2, lsl #4
68: f9400000 ldr x0, [x0]
6c: b100041f cmn x0, #0x1
70: 54000100 b.eq 90 <_dl_tlsdesc_dynamic+0x60>
- 74: f9400421 ldr x1, [x1,#8]
+ 74: f9400821 ldr x1, [x1,#16]
78: 8b010000 add x0, x0, x1
â?¦
This commit introduces explicit struct offsets, generated
from the C headers, fixing the regression. It includes a new
test with many dynamic TLS variables, forcing full dynamic TLS
allocation, which provides test coverage for the bug.
2016-12-02 Florian Weimer <fweimer@redhat.com>
* elf/Makefile [build-shared] (tests): Add tst-latepthread.
(one-hundred, tst-tls-many-dynamic-modules): Define.
(modules-names): Add $(tst-tls-many-dynamic-modules).
(tst-tls-manydynamic%mod.os): Build with special preprocessor
macros.
(tst-tls-manydynamic): Link against libdl, libpthread.
(tst-tls-manydynamic.out): The test needs the test modules at run
time.
* elf/tst-tls-manydynamic.c: New file.
* elf/tst-tls-manydynamic.h: Likewise.
* elf/tst-tls-manydynamicmod.c: Likewise.
2016-12-01 Florian Weimer <fweimer@redhat.com>
* sysdeps/aarch64/tlsdesc.sym (TCBHEAD_DTV, DTV_COUNTER)
(TLS_DTV_UNALLOCATED): Add.
* sysdeps/aarch64/dl-tlsdesc.S (_dl_tlsdesc_dynamic): Use explicit
offsets.
diff --git a/elf/Makefile b/elf/Makefile
index 33b003b..0d6f691 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -152,7 +152,7 @@ tests += loadtest restest1 preloadtest loadfail multiload origtest resolvfail \
tst-initorder tst-initorder2 tst-relsort1 tst-null-argv \
tst-ptrguard1 tst-tlsalign tst-tlsalign-extern tst-nodelete-opened \
tst-nodelete2 tst-audit11 tst-audit12 tst-dlsym-error tst-noload \
- tst-latepthread
+ tst-latepthread tst-tls-manydynamic
# reldep9
ifeq ($(build-hardcoded-path-in-tests),yes)
tests += tst-dlopen-aout
@@ -173,6 +173,10 @@ tlsmod17a-suffixes = 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19
tlsmod18a-suffixes = 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19
tlsmod17a-modules = $(addprefix tst-tlsmod17a, $(tlsmod17a-suffixes))
tlsmod18a-modules = $(addprefix tst-tlsmod18a, $(tlsmod17a-suffixes))
+one-hundred = $(foreach x,0 1 2 3 4 5 6 7 8 9, \
+ 0$x 1$x 2$x 3$x 4$x 5$x 6$x 7$x 8$x 9$x)
+tst-tls-many-dynamic-modules := \
+ $(foreach n,$(one-hundred),tst-tls-manydynamic$(n)mod)
extra-test-objs += $(tlsmod17a-modules:=.os) $(tlsmod18a-modules:=.os) \
tst-tlsalign-vars.o
test-extras += tst-tlsmod17a tst-tlsmod18a tst-tlsalign-vars
@@ -227,7 +231,7 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \
tst-tlsalign-lib tst-nodelete-opened-lib tst-nodelete2mod \
tst-audit11mod1 tst-audit11mod2 tst-auditmod11 \
tst-audit12mod1 tst-audit12mod2 tst-audit12mod3 tst-auditmod12 \
- tst-latepthreadmod
+ tst-latepthreadmod $(tst-tls-many-dynamic-modules)
ifeq (yes,$(have-mtls-dialect-gnu2))
tests += tst-gnu2-tls1
modules-names += tst-gnu2-tls1mod
@@ -1275,6 +1279,15 @@ $(objpfx)tst-latepthreadmod.so: $(shared-thread-library)
$(objpfx)tst-latepthread: $(libdl)
$(objpfx)tst-latepthread.out: $(objpfx)tst-latepthreadmod.so
+# The test modules are parameterized by preprocessor macros.
+$(patsubst %,$(objpfx)%.os,$(tst-tls-many-dynamic-modules)): \
+ $(objpfx)tst-tls-manydynamic%mod.os : tst-tls-manydynamicmod.c
+ $(compile-command.c) \
+ -DNAME=tls_global_$* -DSETTER=set_value_$* -DGETTER=get_value_$*
+$(objpfx)tst-tls-manydynamic: $(libdl) $(shared-thread-library)
+$(objpfx)tst-tls-manydynamic.out: \
+ $(patsubst %,$(objpfx)%.so,$(tst-tls-many-dynamic-modules))
+
tst-prelink-ENV = LD_TRACE_PRELINKING=1
$(objpfx)tst-prelink-conflict.out: $(objpfx)tst-prelink.out
diff --git a/elf/tst-tls-manydynamic.c b/elf/tst-tls-manydynamic.c
new file mode 100644
index 0000000..2f0f5f7
--- /dev/null
+++ b/elf/tst-tls-manydynamic.c
@@ -0,0 +1,132 @@
+/* Test with many dynamic TLS variables.
+ Copyright (C) 2016 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+/* This test intends to exercise dynamic TLS variable allocation. It
+ achieves this by combining dlopen (to avoid static TLS allocation
+ after static TLS resizing), many DSOs with a large variable (to
+ exceed the static TLS reserve), and an already-running thread (to
+ force full dynamic TLS initialization). */
+
+#include "tst-tls-manydynamic.h"
+
+#include <dlfcn.h>
+#include <pthread.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+static int do_test (void);
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
+
+void *handles[COUNT];
+set_value_func set_value_funcs[COUNT];
+get_value_func get_value_funcs[COUNT];
+
+static void
+init_functions (void)
+{
+ for (int i = 0; i < COUNT; ++i)
+ {
+ /* Open the module. */
+ {
+ char soname[100];
+ snprintf (soname, sizeof (soname), "tst-tls-manydynamic%02dmod.so", i);
+ handles[i] = dlopen (soname, RTLD_LAZY);
+ if (handles[i] == NULL)
+ {
+ printf ("error: dlopen failed: %s\n", dlerror ());
+ exit (1);
+ }
+ }
+
+ /* Obtain the setter function. */
+ {
+ char fname[100];
+ snprintf (fname, sizeof (fname), "set_value_%02d", i);
+ void *func = dlsym (handles[i], fname);
+ if (func == NULL)
+ {
+ printf ("error: dlsym: %s\n", dlerror ());
+ exit (1);
+ }
+ set_value_funcs[i] = func;
+ }
+
+ /* Obtain the getter function. */
+ {
+ char fname[100];
+ snprintf (fname, sizeof (fname), "get_value_%02d", i);
+ void *func = dlsym (handles[i], fname);
+ if (func == NULL)
+ {
+ printf ("error: dlsym: %s\n", dlerror ());
+ exit (1);
+ }
+ get_value_funcs[i] = func;
+ }
+ }
+}
+
+/* Running thread which forces real TLS initialization. */
+static void *
+blocked_thread_func (void *closure)
+{
+ pause ();
+ return NULL;
+}
+
+static int
+do_test (void)
+{
+ pthread_t blocked_thread = xpthread_create (NULL, blocked_thread_func, NULL);
+ init_functions ();
+
+ struct value values[COUNT];
+ /* Initialze the TLS variables. */
+ for (int i = 0; i < COUNT; ++i)
+ {
+ for (int j = 0; j < PER_VALUE_COUNT; ++j)
+ values[i].num[j] = rand ();
+ set_value_funcs[i] (&values[i]);
+ }
+
+ /* Read back their values to check that they do not overlap. */
+ for (int i = 0; i < COUNT; ++i)
+ {
+ struct value actual;
+ get_value_funcs[i] (&actual);
+
+ for (int j = 0; j < PER_VALUE_COUNT; ++j)
+ if (actual.num[j] != values[i].num[j])
+ {
+ printf ("error: mismatch at variable %d/%d: %d != %d\n",
+ i, j, actual.num[j], values[i].num[j]);
+ exit (1);
+ }
+ }
+
+ pthread_cancel (blocked_thread);
+ xpthread_join (blocked_thread);
+
+ /* Close the modules. */
+ for (int i = 0; i < COUNT; ++i)
+ dlclose (handles[i]);
+
+ return 0;
+}
diff --git a/elf/tst-tls-manydynamic.h b/elf/tst-tls-manydynamic.h
new file mode 100644
index 0000000..4756ece
--- /dev/null
+++ b/elf/tst-tls-manydynamic.h
@@ -0,0 +1,44 @@
+/* Interfaces for test with many dynamic TLS variables.
+ Copyright (C) 2016 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#ifndef TST_TLS_MANYDYNAMIC_H
+#define TST_TLS_MANYDYNAMIC_H
+
+enum
+ {
+ /* This many TLS variables (and modules) are defined. */
+ COUNT = 100,
+
+ /* Number of elements in the TLS variable. */
+ PER_VALUE_COUNT = 1,
+ };
+
+/* The TLS variables are of this type. We use a larger type to ensure
+ that we can reach the static TLS limit with COUNT variables. */
+struct value
+{
+ int num[PER_VALUE_COUNT];
+};
+
+/* Set the TLS variable defined in the module. */
+typedef void (*set_value_func) (const struct value *);
+
+/* Read the TLS variable defined in the module. */
+typedef void (*get_value_func) (struct value *);
+
+#endif /* TST_TLS_MANYDYNAMICMOD_H */
diff --git a/elf/tst-tls-manydynamicmod.c b/elf/tst-tls-manydynamicmod.c
new file mode 100644
index 0000000..578f11b
--- /dev/null
+++ b/elf/tst-tls-manydynamicmod.c
@@ -0,0 +1,36 @@
+/* Module for test with many dynamic TLS variables.
+ Copyright (C) 2016 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+/* This file is parameterized by macros NAME, SETTER, GETTER, which
+ are set form the Makefile. */
+
+#include "tst-tls-manydynamic.h"
+
+__thread struct value NAME;
+
+void
+SETTER (const struct value *value)
+{
+ NAME = *value;
+}
+
+void
+GETTER (struct value *value)
+{
+ *value = NAME;
+}
diff --git a/sysdeps/aarch64/dl-tlsdesc.S b/sysdeps/aarch64/dl-tlsdesc.S
index 42fa943..9e557dd 100644
--- a/sysdeps/aarch64/dl-tlsdesc.S
+++ b/sysdeps/aarch64/dl-tlsdesc.S
@@ -154,7 +154,7 @@ _dl_tlsdesc_undefweak:
_dl_tlsdesc_dynamic (struct tlsdesc *tdp)
{
struct tlsdesc_dynamic_arg *td = tdp->arg;
- dtv_t *dtv = *(dtv_t **)((char *)__thread_pointer + DTV_OFFSET);
+ dtv_t *dtv = *(dtv_t **)((char *)__thread_pointer + TCBHEAD_DTV);
if (__builtin_expect (td->gen_count <= dtv[0].counter
&& (dtv[td->tlsinfo.ti_module].pointer.val
!= TLS_DTV_UNALLOCATED),
@@ -193,18 +193,18 @@ _dl_tlsdesc_dynamic:
td->entry in _dl_tlsdesc_resolve_rela_fixup ensuring that the load
from [x0,#PTR_SIZE] here happens after the initialization of td->arg. */
ldar PTR_REG (zr), [x0]
- ldr PTR_REG (1), [x0,#PTR_SIZE]
- ldr PTR_REG (0), [x4]
- ldr PTR_REG (3), [x1,#(PTR_SIZE * 2)]
- ldr PTR_REG (2), [x0]
+ ldr PTR_REG (1), [x0,#TLSDESC_ARG]
+ ldr PTR_REG (0), [x4,#TCBHEAD_DTV]
+ ldr PTR_REG (3), [x1,#TLSDESC_GEN_COUNT]
+ ldr PTR_REG (2), [x0,#DTV_COUNTER]
cmp PTR_REG (3), PTR_REG (2)
b.hi 2f
- ldr PTR_REG (2), [x1]
+ ldr PTR_REG (2), [x1,#TLSDESC_MODID]
add PTR_REG (0), PTR_REG (0), PTR_REG (2), lsl #(PTR_LOG_SIZE + 1)
- ldr PTR_REG (0), [x0]
- cmn x0, #0x1
+ ldr PTR_REG (0), [x0] /* Load val member of DTV entry. */
+ cmp x0, #TLS_DTV_UNALLOCATED
b.eq 2f
- ldr PTR_REG (1), [x1,#(PTR_SIZE * 2)]
+ ldr PTR_REG (1), [x1,#TLSDESC_MODOFF]
add PTR_REG (0), PTR_REG (0), PTR_REG (1)
sub PTR_REG (0), PTR_REG (0), PTR_REG (4)
1:
diff --git a/sysdeps/aarch64/tlsdesc.sym b/sysdeps/aarch64/tlsdesc.sym
index 63766af..a06a719 100644
--- a/sysdeps/aarch64/tlsdesc.sym
+++ b/sysdeps/aarch64/tlsdesc.sym
@@ -13,3 +13,6 @@ TLSDESC_ARG offsetof(struct tlsdesc, arg)
TLSDESC_GEN_COUNT offsetof(struct tlsdesc_dynamic_arg, gen_count)
TLSDESC_MODID offsetof(struct tlsdesc_dynamic_arg, tlsinfo.ti_module)
TLSDESC_MODOFF offsetof(struct tlsdesc_dynamic_arg, tlsinfo.ti_offset)
+TCBHEAD_DTV offsetof(tcbhead_t, dtv)
+DTV_COUNTER offsetof(dtv_t, counter)
+TLS_DTV_UNALLOCATED TLS_DTV_UNALLOCATED