diff mbox

[COMMITTED] BZ#18383: Add test case for large alignment in TLS blocks.

Message ID CAMe9rOqBNixXBbOsuRHYTcLM_1+tp5ohJdSOmRFHK0gA-vDL3A@mail.gmail.com
State Committed
Headers

Commit Message

H.J. Lu May 7, 2015, 10:30 p.m. UTC 
  On Thu, May 7, 2015 at 2:10 PM, Roland McGrath <roland@hack.frob.com> wrote:
> The log entry needs the BZ# marker.
>
> I'm having a hard time finding the calculation in the dynamic-linking case
> that corresponds to this piece of __libc_setup_tls.  Since there are two
> places where the logic should match, we really should make it much more
> clear how they relate.  If there isn't any obvious refactoring that would
> share more of this logic between the two cases, can you at least add a
> comment in __libc_setup_tls that points to the implementation of the
> matching logic in the dynamic-linking case?
>

Here is the updated patch.

Comments

Roland McGrath June 9, 2015, 11:34 p.m. UTC | #1 
> 	[BZ #18383]
> 	* csu/libc-tls.c (__libc_setup_tls): Align TCB offset to the
> 	maximum alignment for TLS_TCB_AT_TP targets.

	* csu/libc-tls.c (__libc_setup_tls) [TLS_TCB_AT_TP]:
	Align TCB_OFFSET to MAX_ALIGN, not just TCBALIGN.  Add comment.

> @@ -138,7 +138,9 @@ __libc_setup_tls (size_t tcbsize, size_t tcbalign)
>       to request some surplus that permits dynamic loading of modules with
>       IE-model TLS.  */
>  #if TLS_TCB_AT_TP
> -  tcb_offset = roundup (memsz + GL(dl_tls_static_size), tcbalign);
> +  /* Align the TCB offset to the maximum alignment, similar to what
> +     _dl_allocate_tls_storage in elf/dl-tls.c does.  */
> +  tcb_offset = roundup (memsz + GL(dl_tls_static_size), max_align);

This would be yet more clear if it said:

  /* Align the TCB offset to the maximum alignment, as
     _dl_allocate_tls_storage (in elf/dl-tls.c) does using __libc_memalign
     and dl_tls_static_align.  */

The patch should also remove the XFAIL for tst-align-extern-static
and update the comment on the XFAILs for tst-tlsalign{,-static}
not to say that x86 is broken.

OK with those details.


Thanks,
Roland
diff mbox

Patch

From bfa0232943937b861a482aeed9cbe9f2d90e181e Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 7 May 2015 13:26:34 -0700
Subject: [PATCH] Align TCB offset to the maximum alignment

We need to align TCB offset to the maximum alignment for TLS_TCB_AT_TP
targets, similar to what _dl_allocate_tls_storage in elf/dl-tls.c does.

	[BZ #18383]
	* csu/libc-tls.c (__libc_setup_tls): Align TCB offset to the
	maximum alignment for TLS_TCB_AT_TP targets.
---
 csu/libc-tls.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/csu/libc-tls.c b/csu/libc-tls.c
index 64d1779..452c1b6 100644
--- a/csu/libc-tls.c
+++ b/csu/libc-tls.c
@@ -138,7 +138,9 @@  __libc_setup_tls (size_t tcbsize, size_t tcbalign)
      to request some surplus that permits dynamic loading of modules with
      IE-model TLS.  */
 #if TLS_TCB_AT_TP
-  tcb_offset = roundup (memsz + GL(dl_tls_static_size), tcbalign);
+  /* Align the TCB offset to the maximum alignment, similar to what
+     _dl_allocate_tls_storage in elf/dl-tls.c does.  */
+  tcb_offset = roundup (memsz + GL(dl_tls_static_size), max_align);
   tlsblock = __sbrk (tcb_offset + tcbsize + max_align);
 #elif TLS_DTV_AT_TP
   tcb_offset = roundup (tcbsize, align ?: 1);
-- 
1.9.3