diff mbox

ARM: Fix handling of concurrent TLS descriptor resolution

Message ID 1400858237-4516-1-git-send-email-will.newton@linaro.org
State Accepted
Headers show

Commit Message

Will Newton May 23, 2014, 3:17 p.m. UTC 
The current code for handling concurrent resolution says that the
ABI for _dl_tlsdesc_resolve_hold is the same as that of
_dl_tlsdesc_lazy_resolver. However _dl_tlsdesc_resolve_hold is
called from the trampoline directly rather than the lazy resolver
stub so, for example, r2 has not been pushed so does not needed
to be restored.

This fixes an intermittent failure in nptl/tst-tls3 when building
glibc for arm-linux-gnueabihf with -mtls-dialect=gnu2.

ChangeLog:

2014-05-23  Will Newton  <will.newton@linaro.org>

	* sysdeps/arm/dl-tlsdesc.S (_dl_tlsdesc_resolve_hold): Save
	and restore r2 rather than just restoring.
---
 sysdeps/arm/dl-tlsdesc.S | 31 ++++++++++++-------------------
 1 file changed, 12 insertions(+), 19 deletions(-)

Comments

Joseph Myers May 23, 2014, 3:26 p.m. UTC | #1 
On Fri, 23 May 2014, Will Newton wrote:

> The current code for handling concurrent resolution says that the
> ABI for _dl_tlsdesc_resolve_hold is the same as that of
> _dl_tlsdesc_lazy_resolver. However _dl_tlsdesc_resolve_hold is
> called from the trampoline directly rather than the lazy resolver
> stub so, for example, r2 has not been pushed so does not needed
> to be restored.
> 
> This fixes an intermittent failure in nptl/tst-tls3 when building
> glibc for arm-linux-gnueabihf with -mtls-dialect=gnu2.
> 
> ChangeLog:
> 
> 2014-05-23  Will Newton  <will.newton@linaro.org>
> 
> 	* sysdeps/arm/dl-tlsdesc.S (_dl_tlsdesc_resolve_hold): Save
> 	and restore r2 rather than just restoring.

OK with a bug filed in Bugzilla (I presume this bug was user-visible in 
past releases) and the bug number then duly listed in the ChangeLog entry 
and NEWS before the bug is closed.
diff mbox

Patch

diff --git a/sysdeps/arm/dl-tlsdesc.S b/sysdeps/arm/dl-tlsdesc.S
index 1644a32..e87e7fb 100644
--- a/sysdeps/arm/dl-tlsdesc.S
+++ b/sysdeps/arm/dl-tlsdesc.S
@@ -186,7 +186,9 @@  _dl_tlsdesc_lazy_resolver:
 	.size	_dl_tlsdesc_lazy_resolver, .-_dl_tlsdesc_lazy_resolver
 
 /* Holder for lazy tls descriptors being resolve in another thread.
-   Same ABI as the lazy resolver itself.  */
+
+   Our calling convention is to clobber r0, r1 and the processor
+   flags.  All others that are modified must be saved */
 	.hidden _dl_tlsdesc_resolve_hold
 	.global	_dl_tlsdesc_resolve_hold
 	.type	_dl_tlsdesc_resolve_hold,#function
@@ -194,29 +196,20 @@  _dl_tlsdesc_lazy_resolver:
 	eabi_fnstart
 	.align 2
 _dl_tlsdesc_resolve_hold:
-	/* Tell the unwinder that r2 has already been pushed.  */
-	eabi_save ({r2})
-	cfi_adjust_cfa_offset (4)
+	eabi_save ({r2,r3,ip,lr})
+	push	{r2, r3, ip, lr}
+	cfi_adjust_cfa_offset (16)
 	cfi_rel_offset (r2, 0)
-	eabi_save ({r0,r1,r3,ip,lr})
-	push	{r0, r1, r3, ip, lr}
-	cfi_adjust_cfa_offset (20)
-	cfi_rel_offset (r0, 0)
-	cfi_rel_offset (r1, 4)
-	cfi_rel_offset (r3, 8)
-	cfi_rel_offset (ip, 12)
-	cfi_rel_offset (lr, 16)
-	adr	r2, _dl_tlsdesc_resolve_hold
+	cfi_rel_offset (r3, 4)
+	cfi_rel_offset (ip, 8)
+	cfi_rel_offset (lr, 12)
+	adr	r1, _dl_tlsdesc_resolve_hold
 	bl	_dl_tlsdesc_resolve_hold_fixup
-	pop	{r0, r1, r3, ip, lr}
-	cfi_adjust_cfa_offset (-20)
+	pop	{r2, r3, ip, lr}
+	cfi_adjust_cfa_offset (-16)
 	cfi_restore (lr)
 	cfi_restore (ip)
 	cfi_restore (r3)
-	cfi_restore (r1)
-	cfi_restore (r0)
-	pop	{r2}
-	cfi_adjust_cfa_offset (-4)
 	cfi_restore (r2)
 	sfi_breg r0, \
 	ldr     r1, [\B, #4]