Message ID | 1444276705-6797-3-git-send-email-rth@twiddle.net |
---|---|
State | New |
Headers | show |
On 10/08/2015 05:58 AM, Richard Henderson wrote: > The change in ARCH_FORK is to force conversion to generic address > space before casting to an integral type; otherwise what we get is > the unhelpful %fs-based address of tid. > > The change in THREAD_ATOMIC_CMPXCHG_VAL is to avoid __typeof copying > the address space from descr, leading to an error: automatic variable > with non-default address space. __SEG_TLS comes from the GCC patches. It needs to be documented somewhere, see my response on gcc-patches. > sysdeps/unix/sysv/linux/x86_64/arch-fork.h | 2 +- > sysdeps/x86_64/nptl/tls.h | 44 ++++++++++++++++++++++-------- > 2 files changed, 34 insertions(+), 12 deletions(-) Changelog is missing. > /* Install new dtv for current thread. */ > # define INSTALL_NEW_DTV(dtvp) \ > - ({ struct pthread *__pd; \ > + ({ pthread_self_t *__pd = THREAD_SELF; \ > THREAD_SETMEM (__pd, header.dtv, (dtvp)); }) Does this change code in the !__SEG_TLS case? (THREAD_SETMEM does not evaluate the __pd argument, which is why this worked before.) > /* Atomic compare and exchange on TLS, returning old value. */ > # define THREAD_ATOMIC_CMPXCHG_VAL(descr, member, newval, oldval) \ > - ({ __typeof (descr->member) __ret; \ > + ({ __typeof (((struct pthread *)0)->member) __ret; \ > __typeof (oldval) __old = (oldval); \ > if (sizeof (descr->member) == 4) \ > asm volatile (LOCK_PREFIX "cmpxchgl %2, %%fs:%P3" \ The sizeofs should probably refer to __ret instead of descr->member for consistency. Florian
On 10/08/2015 08:22 PM, Florian Weimer wrote: > On 10/08/2015 05:58 AM, Richard Henderson wrote: >> The change in ARCH_FORK is to force conversion to generic address >> space before casting to an integral type; otherwise what we get is >> the unhelpful %fs-based address of tid. >> >> The change in THREAD_ATOMIC_CMPXCHG_VAL is to avoid __typeof copying >> the address space from descr, leading to an error: automatic variable >> with non-default address space. > > __SEG_TLS comes from the GCC patches. It needs to be documented > somewhere, see my response on gcc-patches. You're right, there's all sorts of gcc documentation missing. >> sysdeps/unix/sysv/linux/x86_64/arch-fork.h | 2 +- >> sysdeps/x86_64/nptl/tls.h | 44 ++++++++++++++++++++++-------- >> 2 files changed, 34 insertions(+), 12 deletions(-) > > Changelog is missing. Yeah, I tend to be lazy and not write them until actually comitting. >> /* Install new dtv for current thread. */ >> # define INSTALL_NEW_DTV(dtvp) \ >> - ({ struct pthread *__pd; \ >> + ({ pthread_self_t *__pd = THREAD_SELF; \ >> THREAD_SETMEM (__pd, header.dtv, (dtvp)); }) > > Does this change code in the !__SEG_TLS case? (THREAD_SETMEM does not > evaluate the __pd argument, which is why this worked before.) No it shouldn't change code in the __SEG_TLS case. Exactly because it isn't evaluated, the initialization should be deleted. It certainly is for all of the generic uses throughout the rest of libc, so without looking I don't see why it wouldn't here. > >> /* Atomic compare and exchange on TLS, returning old value. */ >> # define THREAD_ATOMIC_CMPXCHG_VAL(descr, member, newval, oldval) \ >> - ({ __typeof (descr->member) __ret; \ >> + ({ __typeof (((struct pthread *)0)->member) __ret; \ >> __typeof (oldval) __old = (oldval); \ >> if (sizeof (descr->member) == 4) \ >> asm volatile (LOCK_PREFIX "cmpxchgl %2, %%fs:%P3" \ > > The sizeofs should probably refer to __ret instead of descr->member for > consistency. Fair enough. r~
diff --git a/sysdeps/unix/sysv/linux/x86_64/arch-fork.h b/sysdeps/unix/sysv/linux/x86_64/arch-fork.h index bb4d3ea..6698441 100644 --- a/sysdeps/unix/sysv/linux/x86_64/arch-fork.h +++ b/sysdeps/unix/sysv/linux/x86_64/arch-fork.h @@ -24,4 +24,4 @@ #define ARCH_FORK() \ INLINE_SYSCALL (clone, 4, \ CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD, 0, \ - NULL, &THREAD_SELF->tid) + NULL, (void *)&THREAD_SELF->tid) diff --git a/sysdeps/x86_64/nptl/tls.h b/sysdeps/x86_64/nptl/tls.h index bbc45f0..62634e2 100644 --- a/sysdeps/x86_64/nptl/tls.h +++ b/sysdeps/x86_64/nptl/tls.h @@ -80,7 +80,11 @@ typedef struct void *__padding[8]; } tcbhead_t; +# ifdef __SEG_TLS +typedef struct pthread __seg_tls pthread_self_t; +# else typedef struct pthread pthread_self_t; +# endif #else /* __ASSEMBLER__ */ # include <tcb-offsets.h> @@ -133,7 +137,7 @@ typedef struct pthread pthread_self_t; /* Install new dtv for current thread. */ # define INSTALL_NEW_DTV(dtvp) \ - ({ struct pthread *__pd; \ + ({ pthread_self_t *__pd = THREAD_SELF; \ THREAD_SETMEM (__pd, header.dtv, (dtvp)); }) /* Return dtv of given thread descriptor. */ @@ -182,18 +186,25 @@ typedef struct pthread pthread_self_t; assignments like pthread_descr self = thread_self(); do not get optimized away. */ -# define THREAD_SELF \ +# ifdef __SEG_TLS +# define THREAD_SELF ((pthread_self_t *)0) +# else +# define THREAD_SELF \ ({ struct pthread *__self; \ asm ("mov %%fs:%c1,%0" : "=r" (__self) \ : "i" (offsetof (struct pthread, header.self))); \ __self;}) +# endif /* Magic for libthread_db to know how to do THREAD_SELF. */ # define DB_THREAD_SELF_INCLUDE <sys/reg.h> /* For the FS constant. */ # define DB_THREAD_SELF CONST_THREAD_AREA (64, FS) /* Read member of the thread descriptor directly. */ -# define THREAD_GETMEM(descr, member) \ +# ifdef __SEG_TLS +# define THREAD_GETMEM(descr, member) ({ (descr)->member; }) +# else +# define THREAD_GETMEM(descr, member) \ ({ __typeof (descr->member) __value; \ if (sizeof (__value) == 1) \ asm volatile ("movb %%fs:%P2,%b0" \ @@ -215,10 +226,13 @@ typedef struct pthread pthread_self_t; : "i" (offsetof (struct pthread, member))); \ } \ __value; }) - +# endif /* Same as THREAD_GETMEM, but the member offset can be non-constant. */ -# define THREAD_GETMEM_NC(descr, member, idx) \ +# ifdef __SEG_TLS +# define THREAD_GETMEM_NC(descr, member, idx) ({ (descr)->member[idx]; }) +# else +# define THREAD_GETMEM_NC(descr, member, idx) \ ({ __typeof (descr->member[0]) __value; \ if (sizeof (__value) == 1) \ asm volatile ("movb %%fs:%P2(%q3),%b0" \ @@ -242,7 +256,7 @@ typedef struct pthread pthread_self_t; "r" (idx)); \ } \ __value; }) - +# endif /* Loading addresses of objects on x86-64 needs to be treated special when generating PIC code. */ @@ -254,7 +268,11 @@ typedef struct pthread pthread_self_t; /* Set member of the thread descriptor directly. */ -# define THREAD_SETMEM(descr, member, value) \ +# ifdef __SEG_TLS +# define THREAD_SETMEM(descr, member, value) \ + ({ (descr)->member = (value); (void)0; }) +# else +# define THREAD_SETMEM(descr, member, value) \ ({ if (sizeof (descr->member) == 1) \ asm volatile ("movb %b0,%%fs:%P1" : \ : "iq" (value), \ @@ -274,10 +292,14 @@ typedef struct pthread pthread_self_t; : IMM_MODE ((uint64_t) cast_to_integer (value)), \ "i" (offsetof (struct pthread, member))); \ }}) - +# endif /* Same as THREAD_SETMEM, but the member offset can be non-constant. */ -# define THREAD_SETMEM_NC(descr, member, idx, value) \ +# ifdef __SEG_TLS +# define THREAD_SETMEM_NC(descr, member, idx, value) \ + ({ (descr)->member[idx] = (value); (void)0; }) +# else +# define THREAD_SETMEM_NC(descr, member, idx, value) \ ({ if (sizeof (descr->member[0]) == 1) \ asm volatile ("movb %b0,%%fs:%P1(%q2)" : \ : "iq" (value), \ @@ -300,11 +322,11 @@ typedef struct pthread pthread_self_t; "i" (offsetof (struct pthread, member[0])), \ "r" (idx)); \ }}) - +# endif /* Atomic compare and exchange on TLS, returning old value. */ # define THREAD_ATOMIC_CMPXCHG_VAL(descr, member, newval, oldval) \ - ({ __typeof (descr->member) __ret; \ + ({ __typeof (((struct pthread *)0)->member) __ret; \ __typeof (oldval) __old = (oldval); \ if (sizeof (descr->member) == 4) \ asm volatile (LOCK_PREFIX "cmpxchgl %2, %%fs:%P3" \