[PATCH 0/3] x86_64-gnu follow-ups

[PATCH 0/3] x86_64-gnu follow-ups

[Sergey Bugaev]

* Port the TLS improvements back to i386
* Sneak in another x86_64 fix
* Attempt to make mutexes pointer-aligned

That last one is sort of ugly with how the initializers grew two pairs of
braces, but it seems to build. Maybe it would be better to try with
__attribute__ ((__aligned__ (sizeof (void *)))) instead?

Sergey Bugaev (3):
  hurd: i386 TLS tweaks
  mach: undef ENTRY2
  htl: Make pthread_mutex_t pointer-aligned

 .../htl/bits/types/struct___pthread_mutex.h   | 13 +++--
 sysdeps/mach/hurd/i386/tls.h                  | 48 ++++++++++++++-----
 sysdeps/mach/sysdep.h                         |  1 +
 3 files changed, 44 insertions(+), 18 deletions(-)

-- 
2.39.1

[PATCH 1/3] hurd: i386 TLS tweaks

[Sergey Bugaev]

* Micro-optimize TLS access using GCC's native support for gs-based
  addressing when available;
* Just use THREAD_GETMEM and THREAD_SETMEM instead of more inline
  assembly;
* Sync tcbhead_t layout with NPTL, in particular update/fix __private_ss
  offset;
* Statically assert that the two offsets that are a part of ABI are what
  we expect them to be.

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 sysdeps/mach/hurd/i386/tls.h | 48 ++++++++++++++++++++++++++----------
 1 file changed, 35 insertions(+), 13 deletions(-)

diff --git a/sysdeps/mach/hurd/i386/tls.h b/sysdeps/mach/hurd/i386/tls.h
index a6ff0107..dc43d4c7 100644
--- a/sysdeps/mach/hurd/i386/tls.h
+++ b/sysdeps/mach/hurd/i386/tls.h
@@ -38,11 +38,12 @@ typedef struct
   uintptr_t stack_guard;
   uintptr_t pointer_guard;
   int gscope_flag;
-  int private_futex;
+  unsigned int feature_1;
   /* Reservation of some values for the TM ABI.  */
-  void *__private_tm[4];
+  void *__private_tm[3];
   /* GCC split stack support.  */
   void *__private_ss;
+  void *__glibc_padding1;
 
   /* Keep these fields last, so offsets of fields above can continue being
      compatible with the i386 Linux version.  */
@@ -53,6 +54,13 @@ typedef struct
   struct rtld_catch *rtld_catch;
 } tcbhead_t;
 
+/* GCC generates %gs:0x14 to access the stack guard.  */
+_Static_assert (offsetof (tcbhead_t, stack_guard) == 0x14,
+                "stack guard offset");
+/* libgcc uses %gs:0x30 to access the split stack pointer.  */
+_Static_assert (offsetof (tcbhead_t, __private_ss) == 0x30,
+                "split stack pointer offset");
+
 /* Return tcbhead_t from a TLS segment descriptor.  */
 # define HURD_DESC_TLS(desc)						      \
   ({									      \
@@ -166,8 +174,25 @@ out:
 # define TLS_INIT_TP(descr) \
     _hurd_tls_init ((tcbhead_t *) (descr))
 
+# if __GNUC_PREREQ (6, 0)
+
+#  define THREAD_SELF							      \
+  (*(tcbhead_t * __seg_gs *) offsetof (tcbhead_t, tcb))
+#  define THREAD_GETMEM(descr, member)					      \
+  (*(__typeof (descr->member) __seg_gs *) offsetof (tcbhead_t, member))
+#  define THREAD_GETMEM_NC(descr, member, idx)				      \
+  (*(__typeof (descr->member[0]) __seg_gs *)				      \
+   (offsetof (tcbhead_t, member) + (idx) * sizeof (descr->member[0])))
+#  define THREAD_SETMEM(descr, member, value)				      \
+  (*(__typeof (descr->member) __seg_gs *) offsetof (tcbhead_t, member) = value)
+#  define THREAD_SETMEM_NC(descr, member, index, value)			      \
+  (*(__typeof (descr->member[0]) __seg_gs *)				      \
+   (offsetof (tcbhead_t, member) + (idx) * sizeof (descr->member[0])))
+
+# else
+
 /* Return the TCB address of the current thread.  */
-# define THREAD_SELF							      \
+#  define THREAD_SELF							      \
   ({ tcbhead_t *__tcb;							      \
      __asm__ ("movl %%gs:%c1,%0" : "=r" (__tcb)				      \
 	      : "i" (offsetof (tcbhead_t, tcb)));			      \
@@ -200,7 +225,7 @@ out:
 
 
 /* Same as THREAD_GETMEM, but the member offset can be non-constant.  */
-# define THREAD_GETMEM_NC(descr, member, idx) \
+#  define THREAD_GETMEM_NC(descr, member, idx) \
   ({ __typeof (descr->member[0]) __value;				      \
      _Static_assert (sizeof (__value) == 1				      \
 		     || sizeof (__value) == 4				      \
@@ -229,7 +254,7 @@ out:
 
 
 /* Set member of the thread descriptor directly.  */
-# define THREAD_SETMEM(descr, member, value) \
+#  define THREAD_SETMEM(descr, member, value) \
   ({									      \
      _Static_assert (sizeof (descr->member) == 1			      \
 		     || sizeof (descr->member) == 4			      \
@@ -254,7 +279,7 @@ out:
 
 
 /* Same as THREAD_SETMEM, but the member offset can be non-constant.  */
-# define THREAD_SETMEM_NC(descr, member, idx, value) \
+#  define THREAD_SETMEM_NC(descr, member, idx, value) \
   ({									      \
      _Static_assert (sizeof (descr->member[0]) == 1			      \
 		     || sizeof (descr->member[0]) == 4			      \
@@ -279,6 +304,8 @@ out:
 			 "r" (idx));					      \
        }})
 
+# endif /* __GNUC_PREREQ (6, 0) */
+
 /* Return the TCB address of a thread given its state.
    Note: this is expensive.  */
 # define THREAD_TCB(thread, thread_state)				      \
@@ -295,15 +322,10 @@ out:
      HURD_DESC_TLS (___desc);})
 
 /* Install new dtv for current thread.  */
-# define INSTALL_NEW_DTV(dtvp)						      \
-  ({ asm volatile ("movl %0,%%gs:%P1"					      \
-		   : : "ir" (dtvp), "i" (offsetof (tcbhead_t, dtv))); })
+# define INSTALL_NEW_DTV(dtvp) THREAD_SETMEM (THREAD_SELF, dtv, dtvp)
 
 /* Return the address of the dtv for the current thread.  */
-# define THREAD_DTV()							      \
-  ({ dtv_t *_dtv;							      \
-     asm ("movl %%gs:%P1,%0" : "=q" (_dtv) : "i" (offsetof (tcbhead_t, dtv)));\
-     _dtv; })
+# define THREAD_DTV() THREAD_GETMEM (THREAD_SELF, dtv)
 
 
 /* Set the stack guard field in TCB head.  */
-- 
2.39.1

Re: [PATCH 1/3] hurd: i386 TLS tweaks

[Samuel Thibault]

Applied, thanks!

Sergey Bugaev via Libc-alpha, le mar. 14 févr. 2023 20:37:20 +0300, a ecrit:
> * Micro-optimize TLS access using GCC's native support for gs-based
>   addressing when available;
> * Just use THREAD_GETMEM and THREAD_SETMEM instead of more inline
>   assembly;
> * Sync tcbhead_t layout with NPTL, in particular update/fix __private_ss
>   offset;
> * Statically assert that the two offsets that are a part of ABI are what
>   we expect them to be.
> 
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
>  sysdeps/mach/hurd/i386/tls.h | 48 ++++++++++++++++++++++++++----------
>  1 file changed, 35 insertions(+), 13 deletions(-)
> 
> diff --git a/sysdeps/mach/hurd/i386/tls.h b/sysdeps/mach/hurd/i386/tls.h
> index a6ff0107..dc43d4c7 100644
> --- a/sysdeps/mach/hurd/i386/tls.h
> +++ b/sysdeps/mach/hurd/i386/tls.h
> @@ -38,11 +38,12 @@ typedef struct
>    uintptr_t stack_guard;
>    uintptr_t pointer_guard;
>    int gscope_flag;
> -  int private_futex;
> +  unsigned int feature_1;
>    /* Reservation of some values for the TM ABI.  */
> -  void *__private_tm[4];
> +  void *__private_tm[3];
>    /* GCC split stack support.  */
>    void *__private_ss;
> +  void *__glibc_padding1;
>  
>    /* Keep these fields last, so offsets of fields above can continue being
>       compatible with the i386 Linux version.  */
> @@ -53,6 +54,13 @@ typedef struct
>    struct rtld_catch *rtld_catch;
>  } tcbhead_t;
>  
> +/* GCC generates %gs:0x14 to access the stack guard.  */
> +_Static_assert (offsetof (tcbhead_t, stack_guard) == 0x14,
> +                "stack guard offset");
> +/* libgcc uses %gs:0x30 to access the split stack pointer.  */
> +_Static_assert (offsetof (tcbhead_t, __private_ss) == 0x30,
> +                "split stack pointer offset");
> +
>  /* Return tcbhead_t from a TLS segment descriptor.  */
>  # define HURD_DESC_TLS(desc)						      \
>    ({									      \
> @@ -166,8 +174,25 @@ out:
>  # define TLS_INIT_TP(descr) \
>      _hurd_tls_init ((tcbhead_t *) (descr))
>  
> +# if __GNUC_PREREQ (6, 0)
> +
> +#  define THREAD_SELF							      \
> +  (*(tcbhead_t * __seg_gs *) offsetof (tcbhead_t, tcb))
> +#  define THREAD_GETMEM(descr, member)					      \
> +  (*(__typeof (descr->member) __seg_gs *) offsetof (tcbhead_t, member))
> +#  define THREAD_GETMEM_NC(descr, member, idx)				      \
> +  (*(__typeof (descr->member[0]) __seg_gs *)				      \
> +   (offsetof (tcbhead_t, member) + (idx) * sizeof (descr->member[0])))
> +#  define THREAD_SETMEM(descr, member, value)				      \
> +  (*(__typeof (descr->member) __seg_gs *) offsetof (tcbhead_t, member) = value)
> +#  define THREAD_SETMEM_NC(descr, member, index, value)			      \
> +  (*(__typeof (descr->member[0]) __seg_gs *)				      \
> +   (offsetof (tcbhead_t, member) + (idx) * sizeof (descr->member[0])))
> +
> +# else
> +
>  /* Return the TCB address of the current thread.  */
> -# define THREAD_SELF							      \
> +#  define THREAD_SELF							      \
>    ({ tcbhead_t *__tcb;							      \
>       __asm__ ("movl %%gs:%c1,%0" : "=r" (__tcb)				      \
>  	      : "i" (offsetof (tcbhead_t, tcb)));			      \
> @@ -200,7 +225,7 @@ out:
>  
>  
>  /* Same as THREAD_GETMEM, but the member offset can be non-constant.  */
> -# define THREAD_GETMEM_NC(descr, member, idx) \
> +#  define THREAD_GETMEM_NC(descr, member, idx) \
>    ({ __typeof (descr->member[0]) __value;				      \
>       _Static_assert (sizeof (__value) == 1				      \
>  		     || sizeof (__value) == 4				      \
> @@ -229,7 +254,7 @@ out:
>  
>  
>  /* Set member of the thread descriptor directly.  */
> -# define THREAD_SETMEM(descr, member, value) \
> +#  define THREAD_SETMEM(descr, member, value) \
>    ({									      \
>       _Static_assert (sizeof (descr->member) == 1			      \
>  		     || sizeof (descr->member) == 4			      \
> @@ -254,7 +279,7 @@ out:
>  
>  
>  /* Same as THREAD_SETMEM, but the member offset can be non-constant.  */
> -# define THREAD_SETMEM_NC(descr, member, idx, value) \
> +#  define THREAD_SETMEM_NC(descr, member, idx, value) \
>    ({									      \
>       _Static_assert (sizeof (descr->member[0]) == 1			      \
>  		     || sizeof (descr->member[0]) == 4			      \
> @@ -279,6 +304,8 @@ out:
>  			 "r" (idx));					      \
>         }})
>  
> +# endif /* __GNUC_PREREQ (6, 0) */
> +
>  /* Return the TCB address of a thread given its state.
>     Note: this is expensive.  */
>  # define THREAD_TCB(thread, thread_state)				      \
> @@ -295,15 +322,10 @@ out:
>       HURD_DESC_TLS (___desc);})
>  
>  /* Install new dtv for current thread.  */
> -# define INSTALL_NEW_DTV(dtvp)						      \
> -  ({ asm volatile ("movl %0,%%gs:%P1"					      \
> -		   : : "ir" (dtvp), "i" (offsetof (tcbhead_t, dtv))); })
> +# define INSTALL_NEW_DTV(dtvp) THREAD_SETMEM (THREAD_SELF, dtv, dtvp)
>  
>  /* Return the address of the dtv for the current thread.  */
> -# define THREAD_DTV()							      \
> -  ({ dtv_t *_dtv;							      \
> -     asm ("movl %%gs:%P1,%0" : "=q" (_dtv) : "i" (offsetof (tcbhead_t, dtv)));\
> -     _dtv; })
> +# define THREAD_DTV() THREAD_GETMEM (THREAD_SELF, dtv)
>  
>  
>  /* Set the stack guard field in TCB head.  */
> -- 
> 2.39.1
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.

[PATCH 2/3] mach: undef ENTRY2

[Sergey Bugaev]

This macro from Mach headers conflicts with how
sysdeps/x86_64/multiarch/strcmp-sse2.S expects it to be defined.

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 sysdeps/mach/sysdep.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sysdeps/mach/sysdep.h b/sysdeps/mach/sysdep.h
index fe26bcb3..1c869f5c 100644
--- a/sysdeps/mach/sysdep.h
+++ b/sysdeps/mach/sysdep.h
@@ -30,6 +30,7 @@
 /* For ELF we need to add the `.type' directive to make shared libraries
    work right.  */
 #undef ENTRY
+#undef ENTRY2
 #define ENTRY(name) \
   .globl name; \
   .align ALIGN; \
-- 
2.39.1

Re: [PATCH 2/3] mach: undef ENTRY2

[Samuel Thibault]

Applied, thanks!

Sergey Bugaev via Libc-alpha, le mar. 14 févr. 2023 20:37:21 +0300, a ecrit:
> This macro from Mach headers conflicts with how
> sysdeps/x86_64/multiarch/strcmp-sse2.S expects it to be defined.
> 
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
>  sysdeps/mach/sysdep.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/sysdeps/mach/sysdep.h b/sysdeps/mach/sysdep.h
> index fe26bcb3..1c869f5c 100644
> --- a/sysdeps/mach/sysdep.h
> +++ b/sysdeps/mach/sysdep.h
> @@ -30,6 +30,7 @@
>  /* For ELF we need to add the `.type' directive to make shared libraries
>     work right.  */
>  #undef ENTRY
> +#undef ENTRY2
>  #define ENTRY(name) \
>    .globl name; \
>    .align ALIGN; \
> -- 
> 2.39.1
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.

[PATCH 3/3] htl: Make pthread_mutex_t pointer-aligned

[Sergey Bugaev]

This is for future-proofing. On i386, it is 4-byte aligned anyway, but
on x86_64, we want it 8-byte aligned, not 4-byte aligned.

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 .../hurd/htl/bits/types/struct___pthread_mutex.h    | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/sysdeps/mach/hurd/htl/bits/types/struct___pthread_mutex.h b/sysdeps/mach/hurd/htl/bits/types/struct___pthread_mutex.h
index d7b239f4..11caa870 100644
--- a/sysdeps/mach/hurd/htl/bits/types/struct___pthread_mutex.h
+++ b/sysdeps/mach/hurd/htl/bits/types/struct___pthread_mutex.h
@@ -30,20 +30,23 @@ struct __pthread_mutex
   int __shpid;
   int __type;
   int __flags;
-  unsigned int __reserved1;
-  unsigned int __reserved2;
+  union
+  {
+    unsigned int __reserved[2];
+    void *__pointer_aligned;
+  };
 };
 
 /* Static mutex initializers. */
 #define __PTHREAD_MUTEX_INITIALIZER   \
-  { 0, 0, 0, 0, __PTHREAD_MUTEX_TIMED, 0, 0, 0 }
+  { 0, 0, 0, 0, __PTHREAD_MUTEX_TIMED, 0, { { 0, 0 } } }
 
 /* The +1 is to mantain binary compatibility with the old
  * libpthread implementation. */
 #define __PTHREAD_ERRORCHECK_MUTEX_INITIALIZER   \
-  { 0, 0, 0, 0, __PTHREAD_MUTEX_ERRORCHECK + 1, 0, 0, 0 }
+  { 0, 0, 0, 0, __PTHREAD_MUTEX_ERRORCHECK + 1, 0, { { 0, 0 } } }
 
 #define __PTHREAD_RECURSIVE_MUTEX_INITIALIZER   \
-  { 0, 0, 0, 0, __PTHREAD_MUTEX_RECURSIVE + 1, 0, 0, 0 }
+  { 0, 0, 0, 0, __PTHREAD_MUTEX_RECURSIVE + 1, 0, { { 0, 0 } } }
 
 #endif /* bits/types/struct___pthread_mutex.h */
-- 
2.39.1

Re: [PATCH 3/3] htl: Make pthread_mutex_t pointer-aligned

[Samuel Thibault]

Applied, thanks!

Sergey Bugaev via Libc-alpha, le mar. 14 févr. 2023 20:37:22 +0300, a ecrit:
> This is for future-proofing. On i386, it is 4-byte aligned anyway, but
> on x86_64, we want it 8-byte aligned, not 4-byte aligned.
> 
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
>  .../hurd/htl/bits/types/struct___pthread_mutex.h    | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/sysdeps/mach/hurd/htl/bits/types/struct___pthread_mutex.h b/sysdeps/mach/hurd/htl/bits/types/struct___pthread_mutex.h
> index d7b239f4..11caa870 100644
> --- a/sysdeps/mach/hurd/htl/bits/types/struct___pthread_mutex.h
> +++ b/sysdeps/mach/hurd/htl/bits/types/struct___pthread_mutex.h
> @@ -30,20 +30,23 @@ struct __pthread_mutex
>    int __shpid;
>    int __type;
>    int __flags;
> -  unsigned int __reserved1;
> -  unsigned int __reserved2;
> +  union
> +  {
> +    unsigned int __reserved[2];
> +    void *__pointer_aligned;
> +  };
>  };
>  
>  /* Static mutex initializers. */
>  #define __PTHREAD_MUTEX_INITIALIZER   \
> -  { 0, 0, 0, 0, __PTHREAD_MUTEX_TIMED, 0, 0, 0 }
> +  { 0, 0, 0, 0, __PTHREAD_MUTEX_TIMED, 0, { { 0, 0 } } }
>  
>  /* The +1 is to mantain binary compatibility with the old
>   * libpthread implementation. */
>  #define __PTHREAD_ERRORCHECK_MUTEX_INITIALIZER   \
> -  { 0, 0, 0, 0, __PTHREAD_MUTEX_ERRORCHECK + 1, 0, 0, 0 }
> +  { 0, 0, 0, 0, __PTHREAD_MUTEX_ERRORCHECK + 1, 0, { { 0, 0 } } }
>  
>  #define __PTHREAD_RECURSIVE_MUTEX_INITIALIZER   \
> -  { 0, 0, 0, 0, __PTHREAD_MUTEX_RECURSIVE + 1, 0, 0, 0 }
> +  { 0, 0, 0, 0, __PTHREAD_MUTEX_RECURSIVE + 1, 0, { { 0, 0 } } }
>  
>  #endif /* bits/types/struct___pthread_mutex.h */
> -- 
> 2.39.1
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.