Fix alignment of TLS variables for tls variant TLS_TCB_AT_TP [BZ #23403]
diff mbox series

Message ID de2c631c-19ed-46e9-9ca9-02be132240f7@linux.ibm.com
State New
Headers show
Series
  • Fix alignment of TLS variables for tls variant TLS_TCB_AT_TP [BZ #23403]
Related show

Commit Message

Stefan Liebler Feb. 5, 2019, 8:29 a.m. UTC
Hi,

the alignment of TLS variables is wrong if accessed from within a thread 
for architectures with tls variant TLS_TCB_AT_TP. For the main thread 
the static tls data is properly aligned. For other threads the alignment 
depends on the alignment of the thread pointer as the static tls data is 
located relative to this pointer.

This patch adds this alignment for TLS_TCB_AT_TP variants in the same 
way as it is already done for TLS_DTV_AT_TP. The thread pointer is also 
already properly aligned if the user provides its own stack for the new 
thread.

This patch extends the testcase nptl/tst-tls1.c in order to check the 
alignment of the tls variables and it adds a pthread_create invocation 
with a user provided stack. The test itself is migrated from 
test-skeleton.c to test-driver.c and the missing support functions 
xpthread_attr_setstack and xposix_memalign are added.

Okay to commit?

Bye
Stefan

ChangeLog:

	[BZ #23403]
	* nptl/allocatestack.c (allocate_stack): Align pointer pd for
	TLS_TCB_AT_TP tls variant.
	* nptl/tst-tls1.c: Migrate to support/test-driver.c.
	Add alignment checks.
	* support/Makefile (libsupport-routines): Add xposix_memalign
	and xpthread_setstack.
	* support/support.h: Add xposix_memalign.
	* support/xthread.h: Add xpthread_attr_setstack.
	* support/xposix_memalign.c: New File.
	* support/xpthread_attr_setstack.c: Likewise.

Comments

Andreas Schwab Feb. 5, 2019, 9:07 a.m. UTC | #1
On Feb 05 2019, Stefan Liebler <stli@linux.ibm.com> wrote:

> 	[BZ #23403]
> 	* nptl/allocatestack.c (allocate_stack): Align pointer pd for
> 	TLS_TCB_AT_TP tls variant.
> 	* nptl/tst-tls1.c: Migrate to support/test-driver.c.
> 	Add alignment checks.
> 	* support/Makefile (libsupport-routines): Add xposix_memalign
> 	and xpthread_setstack.
> 	* support/support.h: Add xposix_memalign.
> 	* support/xthread.h: Add xpthread_attr_setstack.
> 	* support/xposix_memalign.c: New File.
> 	* support/xpthread_attr_setstack.c: Likewise.

Ok.

Andreas.
Stefan Liebler Feb. 6, 2019, 8:26 a.m. UTC | #2
On 02/05/2019 10:07 AM, Andreas Schwab wrote:
> On Feb 05 2019, Stefan Liebler <stli@linux.ibm.com> wrote:
> 
>> 	[BZ #23403]
>> 	* nptl/allocatestack.c (allocate_stack): Align pointer pd for
>> 	TLS_TCB_AT_TP tls variant.
>> 	* nptl/tst-tls1.c: Migrate to support/test-driver.c.
>> 	Add alignment checks.
>> 	* support/Makefile (libsupport-routines): Add xposix_memalign
>> 	and xpthread_setstack.
>> 	* support/support.h: Add xposix_memalign.
>> 	* support/xthread.h: Add xpthread_attr_setstack.
>> 	* support/xposix_memalign.c: New File.
>> 	* support/xpthread_attr_setstack.c: Likewise.
> 
> Ok.
> 
> Andreas.
> 
Committed.
Thanks.

Patch
diff mbox series

commit 6c3eaae64ebbcdbb8d1c3e938b384f4bdd7b9b74
Author: Stefan Liebler <stli@linux.ibm.com>
Date:   Mon Feb 4 17:22:24 2019 +0100

    Fix alignment of TLS variables for tls variant TLS_TCB_AT_TP [BZ #23403]
    
    The alignment of TLS variables is wrong if accessed from within a thread
    for architectures with tls variant TLS_TCB_AT_TP.
    For the main thread the static tls data is properly aligned.
    For other threads the alignment depends on the alignment of the thread
    pointer as the static tls data is located relative to this pointer.
    
    This patch adds this alignment for TLS_TCB_AT_TP variants in the same way
    as it is already done for TLS_DTV_AT_TP. The thread pointer is also already
    properly aligned if the user provides its own stack for the new thread.
    
    This patch extends the testcase nptl/tst-tls1.c in order to check the
    alignment of the tls variables and it adds a pthread_create invocation
    with a user provided stack.
    The test itself is migrated from test-skeleton.c to test-driver.c
    and the missing support functions xpthread_attr_setstack and xposix_memalign
    are added.
    
    ChangeLog:
    
            [BZ #23403]
            * nptl/allocatestack.c (allocate_stack): Align pointer pd for
            TLS_TCB_AT_TP tls variant.
            * nptl/tst-tls1.c: Migrate to support/test-driver.c.
            Add alignment checks.
            * support/Makefile (libsupport-routines): Add xposix_memalign and
            xpthread_setstack.
            * support/support.h: Add xposix_memalign.
            * support/xthread.h: Add xpthread_attr_setstack.
            * support/xposix_memalign.c: New File.
            * support/xpthread_attr_setstack.c: Likewise.

diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c
index 670cb8ffe6..590350647b 100644
--- a/nptl/allocatestack.c
+++ b/nptl/allocatestack.c
@@ -572,7 +572,9 @@  allocate_stack (const struct pthread_attr *attr, struct pthread **pdp,
 
 	  /* Place the thread descriptor at the end of the stack.  */
 #if TLS_TCB_AT_TP
-	  pd = (struct pthread *) ((char *) mem + size) - 1;
+	  pd = (struct pthread *) ((((uintptr_t) mem + size)
+				    - TLS_TCB_SIZE)
+				   & ~__static_tls_align_m1);
 #elif TLS_DTV_AT_TP
 	  pd = (struct pthread *) ((((uintptr_t) mem + size
 				    - __static_tls_size)
diff --git a/nptl/tst-tls1.c b/nptl/tst-tls1.c
index 00489e23e9..1a915224a7 100644
--- a/nptl/tst-tls1.c
+++ b/nptl/tst-tls1.c
@@ -19,12 +19,16 @@ 
 #include <pthread.h>
 #include <stdio.h>
 #include <stdlib.h>
-
+#include <stdint.h>
+#include <inttypes.h>
+#include <support/support.h>
+#include <support/check.h>
+#include <support/xthread.h>
 
 struct test_s
 {
-  int a;
-  int b;
+  __attribute__ ((aligned(0x20))) int a;
+  __attribute__ ((aligned(0x200))) int b;
 };
 
 #define INIT_A 1
@@ -36,15 +40,34 @@  __thread struct test_s s __attribute__ ((tls_model ("initial-exec"))) =
   .b = INIT_B
 };
 
+/* Use noinline in combination with not static to ensure that the
+   alignment check is really done.  Otherwise it was optimized out!  */
+__attribute__ ((noinline)) void
+check_alignment (const char *thr_name, const char *ptr_name,
+		 int *ptr, int alignment)
+{
+  uintptr_t offset_aligment = ((uintptr_t) ptr) & (alignment - 1);
+  if (offset_aligment)
+    {
+      FAIL_EXIT1 ("%s (%p) is not 0x%x-byte aligned in %s thread\n",
+		  ptr_name, ptr, alignment, thr_name);
+    }
+}
+
+static void
+check_s (const char *thr_name)
+{
+  if (s.a != INIT_A || s.b != INIT_B)
+    FAIL_EXIT1 ("initial value of s in %s thread wrong\n", thr_name);
+
+  check_alignment (thr_name, "s.a", &s.a, 0x20);
+  check_alignment (thr_name, "s.b", &s.b, 0x200);
+}
 
 static void *
 tf (void *arg)
 {
-  if (s.a != INIT_A || s.b != INIT_B)
-    {
-      puts ("initial value of s in child thread wrong");
-      exit (1);
-    }
+  check_s ("child");
 
   ++s.a;
 
@@ -55,25 +78,14 @@  tf (void *arg)
 int
 do_test (void)
 {
-  if (s.a != INIT_A || s.b != INIT_B)
-    {
-      puts ("initial value of s in main thread wrong");
-      exit (1);
-    }
+  check_s ("main");
 
   pthread_attr_t a;
 
-  if (pthread_attr_init (&a) != 0)
-    {
-      puts ("attr_init failed");
-      exit (1);
-    }
+  xpthread_attr_init (&a);
 
-  if (pthread_attr_setstacksize (&a, 1 * 1024 * 1024) != 0)
-    {
-      puts ("attr_setstacksize failed");
-      return 1;
-    }
+#define STACK_SIZE (1 * 1024 * 1024)
+  xpthread_attr_setstacksize (&a, STACK_SIZE);
 
 #define N 10
   int i;
@@ -83,29 +95,25 @@  do_test (void)
       pthread_t th[M];
       int j;
       for (j = 0; j < M; ++j, ++s.a)
-	if (pthread_create (&th[j], &a, tf, NULL) != 0)
-	  {
-	    puts ("pthread_create failed");
-	    exit (1);
-	  }
+	th[j] = xpthread_create (&a, tf, NULL);
 
       for (j = 0; j < M; ++j)
-	if (pthread_join (th[j], NULL) != 0)
-	  {
-	    puts ("pthread_join failed");
-	    exit (1);
-	  }
+	xpthread_join (th[j]);
     }
 
-  if (pthread_attr_destroy (&a) != 0)
-    {
-      puts ("attr_destroy failed");
-      exit (1);
-    }
+  /* Also check the alignment of the tls variables if a misaligned stack is
+     specified.  */
+  pthread_t th;
+  void *thr_stack = NULL;
+  thr_stack = xposix_memalign (0x200, STACK_SIZE + 1);
+  xpthread_attr_setstack (&a, thr_stack + 1, STACK_SIZE);
+  th = xpthread_create (&a, tf, NULL);
+  xpthread_join (th);
+  free (thr_stack);
+
+  xpthread_attr_destroy (&a);
 
   return 0;
 }
 
-
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
+#include <support/test-driver.c>
diff --git a/support/Makefile b/support/Makefile
index c15b93647c..9ff0ec3fff 100644
--- a/support/Makefile
+++ b/support/Makefile
@@ -99,10 +99,12 @@  libsupport-routines = \
   xopen \
   xpipe \
   xpoll \
+  xposix_memalign \
   xpthread_attr_destroy \
   xpthread_attr_init \
   xpthread_attr_setdetachstate \
   xpthread_attr_setguardsize \
+  xpthread_attr_setstack \
   xpthread_attr_setstacksize \
   xpthread_barrier_destroy \
   xpthread_barrier_init \
diff --git a/support/support.h b/support/support.h
index 119495e5a9..97fef2cd23 100644
--- a/support/support.h
+++ b/support/support.h
@@ -86,6 +86,7 @@  int support_descriptor_supports_holes (int fd);
 void *xmalloc (size_t) __attribute__ ((malloc));
 void *xcalloc (size_t n, size_t s) __attribute__ ((malloc));
 void *xrealloc (void *p, size_t n);
+void *xposix_memalign (size_t alignment, size_t n);
 char *xasprintf (const char *format, ...)
   __attribute__ ((format (printf, 1, 2), malloc));
 char *xstrdup (const char *);
diff --git a/support/xposix_memalign.c b/support/xposix_memalign.c
new file mode 100644
index 0000000000..5501a0846a
--- /dev/null
+++ b/support/xposix_memalign.c
@@ -0,0 +1,35 @@ 
+/* Error-checking wrapper for posix_memalign.
+   Copyright (C) 2019 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/>.  */
+
+#include <support/support.h>
+#include <stdlib.h>
+#include <errno.h>
+
+void *
+xposix_memalign (size_t alignment, size_t n)
+{
+  void *p = NULL;
+
+  int ret = posix_memalign (&p, alignment, n);
+  if (ret)
+    {
+      errno = ret;
+      oom_error ("posix_memalign", n);
+    }
+  return p;
+}
diff --git a/support/xpthread_attr_setstack.c b/support/xpthread_attr_setstack.c
new file mode 100644
index 0000000000..c3772e240b
--- /dev/null
+++ b/support/xpthread_attr_setstack.c
@@ -0,0 +1,26 @@ 
+/* pthread_attr_setstack with error checking.
+   Copyright (C) 2019 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/>.  */
+
+#include <support/xthread.h>
+
+void
+xpthread_attr_setstack (pthread_attr_t *attr, void *stackaddr, size_t stacksize)
+{
+  xpthread_check_return ("pthread_attr_setstack",
+			 pthread_attr_setstack (attr, stackaddr, stacksize));
+}
diff --git a/support/xthread.h b/support/xthread.h
index 9fe1f68b3b..5204f78ed2 100644
--- a/support/xthread.h
+++ b/support/xthread.h
@@ -68,6 +68,8 @@  void xpthread_attr_destroy (pthread_attr_t *attr);
 void xpthread_attr_init (pthread_attr_t *attr);
 void xpthread_attr_setdetachstate (pthread_attr_t *attr,
 				   int detachstate);
+void xpthread_attr_setstack (pthread_attr_t *attr, void *stackaddr,
+			     size_t stacksize);
 void xpthread_attr_setstacksize (pthread_attr_t *attr,
 				 size_t stacksize);
 void xpthread_attr_setguardsize (pthread_attr_t *attr,