This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH roland/exit-thread] Clean up __exit_thread.


On 9 May 2014 21:00, Roland McGrath <roland@hack.frob.com> wrote:
> This cleans things up in a few ways:
> * It removes a Linuxism from the pthreaddef.h files.
> * It removes duplication across all the pthreaddef.h
>   files except i386 and x86_64.
> * It avoids the useless errno setting code on non-x86.
> * It removes the undocumented and unsupported __exit_thread entry point
>   from libc.a.
>
> I verified on i386 and x86_64 builds that the code generated for
> start_thread in pthread_create.c is at least about as good as before.
>
> On x86_64, it perturbed the code generation substantially because the
> compiler knowing that the syscall sequence can never return changes how
> it organizes the blocks; probably the new code is better.
>
> On i386, the old code used 'int $0x80' and now it uses the vDSO entry point
> like regular syscall stubs do.  This should perform marginally better.  It
> also means that in a situation where wild memory clobberation has corrupted
> the TCB, an exitting thread might do more unpredictable things and possibly
> never actually exit, whereas before it was more likely to do the syscall.
> I don't think that's worth caring about.  But if it is, we can easily add
> an i386-specific exit-thread.h to do what the old code did.
>
> I didn't test any other machines, but it's hard to see how they could go
> wrong.  I'd appreciate people testing the branch on other machines.

This looks ok to me and doesn't introduce any test failures on ARM.

> If nobody objects, I'll probably commit this on Monday.
>
>
> Thanks,
> Roland
>
>
> 2014-05-09  Roland McGrath  <roland@hack.frob.com>
>
>         * sysdeps/generic/exit-thread.h: New file.
>         * sysdeps/unix/sysv/linux/exit-thread.h: New file.
>         * include/unistd.h (__exit_thread): Remove declaration.
>         * sysdeps/unix/sysv/linux/Makefile (sysdep_routines): Drop exit-thread.
>         * sysdeps/unix/sysv/linux/exit-thread.S: File removed.
>         * csu/libc-start.c: Include <exit-thread.h>.
>         (LIBC_START_MAIN): Pass no argument to __exit_thread.
>         * nptl/pthread_create.c: Include <exit-thread.h>.
>         (start_thread): Call __exit_thread in place of __exit_thread_inline.
>         * nptl/sysdeps/i386/pthreaddef.h (__exit_thread_inline): Macro removed.
>         * nptl/sysdeps/powerpc/pthreaddef.h: Likewise.
>         * nptl/sysdeps/s390/pthreaddef.h: Likewise.
>         * nptl/sysdeps/sh/pthreaddef.h: Likewise.
>         * nptl/sysdeps/sparc/sparc32/pthreaddef.h: Likewise.
>         * nptl/sysdeps/sparc/sparc64/pthreaddef.h: Likewise.
>         * nptl/sysdeps/x86_64/pthreaddef.h: Likewise.
>         * sysdeps/aarch64/nptl/pthreaddef.h: Likewise.
>         * sysdeps/alpha/nptl/pthreaddef.h: Likewise.
>         * sysdeps/arm/nptl/pthreaddef.h: Likewise.
>         * sysdeps/hppa/nptl/pthreaddef.h: Likewise.
>         * sysdeps/ia64/nptl/pthreaddef.h: Likewise.
>         * sysdeps/m68k/nptl/pthreaddef.h: Likewise.
>         * sysdeps/microblaze/nptl/pthreaddef.h: Likewise.
>         * sysdeps/mips/nptl/pthreaddef.h: Likewise.
>         * sysdeps/tile/nptl/pthreaddef.h: Likewise.
>
> --- a/csu/libc-start.c
> +++ b/csu/libc-start.c
> @@ -20,6 +20,7 @@
>  #include <stdio.h>
>  #include <unistd.h>
>  #include <ldsodefs.h>
> +#include <exit-thread.h>
>
>  extern void __libc_init_first (int argc, char **argv, char **envp);
>  #ifndef SHARED
> @@ -312,7 +313,7 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
>
>        if (! atomic_decrement_and_test (ptr))
>         /* Not much left to do but to exit the thread, not the process.  */
> -       __exit_thread (0);
> +       __exit_thread ();
>      }
>  #else
>    /* Nothing fancy, just call the function.  */
> --- a/include/unistd.h
> +++ b/include/unistd.h
> @@ -161,9 +161,6 @@ extern int __libc_enable_secure_internal attribute_relro attribute_hidden;
>  extern void __libc_check_standard_fds (void);
>
>
> -/* Special exit function which only terminates the current thread.  */
> -extern void __exit_thread (int val) __attribute__ ((noreturn));
> -
>  /* Internal name for fork function.  */
>  extern __pid_t __libc_fork (void);
>
> --- a/nptl/pthread_create.c
> +++ b/nptl/pthread_create.c
> @@ -29,6 +29,7 @@
>  #include <libc-internal.h>
>  #include <resolv.h>
>  #include <kernel-features.h>
> +#include <exit-thread.h>
>
>  #include <shlib-compat.h>
>
> @@ -432,7 +433,7 @@ start_thread (void *arg)
>
>       The exit code is zero since in case all threads exit by calling
>       'pthread_exit' the exit status must be 0 (zero).  */
> -  __exit_thread_inline (0);
> +  __exit_thread ();
>
>    /* NOTREACHED */
>    return 0;
> --- a/nptl/sysdeps/i386/pthreaddef.h
> +++ b/nptl/sysdeps/i386/pthreaddef.h
> @@ -38,16 +38,3 @@
>
>  /* Location of current stack frame.  */
>  #define CURRENT_STACK_FRAME    __builtin_frame_address (0)
> -
> -
> -/* XXX Until we have a better place keep the definitions here.  */
> -
> -/* While there is no such syscall.  */
> -#define __exit_thread_inline(val) \
> -  while (1) {                                                                \
> -    if (__builtin_constant_p (val) && (val) == 0)                            \
> -      asm volatile ("xorl %%ebx, %%ebx; int $0x80" :: "a" (__NR_exit));              \
> -    else                                                                     \
> -      asm volatile ("movl %1, %%ebx; int $0x80"                                      \
> -                   :: "a" (__NR_exit), "r" (val));                           \
> -  }
> --- a/nptl/sysdeps/powerpc/pthreaddef.h
> +++ b/nptl/sysdeps/powerpc/pthreaddef.h
> @@ -31,10 +31,3 @@
>
>  /* Location of current stack frame.  */
>  #define CURRENT_STACK_FRAME    __builtin_frame_address (0)
> -
> -
> -/* XXX Until we have a better place keep the definitions here.  */
> -
> -/* While there is no such syscall.  */
> -#define __exit_thread_inline(val) \
> -  INLINE_SYSCALL (exit, 1, (val))
> --- a/nptl/sysdeps/s390/pthreaddef.h
> +++ b/nptl/sysdeps/s390/pthreaddef.h
> @@ -31,10 +31,3 @@
>
>  /* Location of current stack frame.  */
>  #define CURRENT_STACK_FRAME    __builtin_frame_address (0)
> -
> -
> -/* XXX Until we have a better place keep the definitions here.  */
> -
> -/* While there is no such syscall.  */
> -#define __exit_thread_inline(val) \
> -  INLINE_SYSCALL (exit, 1, (val))
> --- a/nptl/sysdeps/sh/pthreaddef.h
> +++ b/nptl/sysdeps/sh/pthreaddef.h
> @@ -32,17 +32,3 @@
>
>  /* Location of current stack frame.  */
>  #define CURRENT_STACK_FRAME    __builtin_frame_address (0)
> -
> -
> -/* XXX Until we have a better place keep the definitions here.  */
> -
> -/* While there is no such syscall.  */
> -#define __exit_thread_inline(val) \
> -  while (1) {                                                                \
> -    if (__builtin_constant_p (val) && (val) == 0)                            \
> -      asm volatile ("mov #0,r4; mov %0,r3; trapa #0x11\n\t" SYSCALL_INST_PAD  \
> -                  :: "i" (__NR_exit));  \
> -    else                                                                     \
> -      asm volatile ("mov %1,r4; mov %0,r3; trapa #0x11\n\t" SYSCALL_INST_PAD  \
> -                   :: "i" (__NR_exit), "r" (val));                           \
> -  }
> --- a/nptl/sysdeps/sparc/sparc32/pthreaddef.h
> +++ b/nptl/sysdeps/sparc/sparc32/pthreaddef.h
> @@ -31,9 +31,3 @@
>  /* Location of current stack frame.  */
>  #define CURRENT_STACK_FRAME  (stack_pointer + (2 * 64))
>  register char *stack_pointer __asm__("%sp");
> -
> -/* XXX Until we have a better place keep the definitions here.  */
> -
> -/* While there is no such syscall.  */
> -#define __exit_thread_inline(val) \
> -  INLINE_SYSCALL (exit, 1, (val))
> --- a/nptl/sysdeps/sparc/sparc64/pthreaddef.h
> +++ b/nptl/sysdeps/sparc/sparc64/pthreaddef.h
> @@ -31,9 +31,3 @@
>  /* Location of current stack frame.  */
>  #define CURRENT_STACK_FRAME  (stack_pointer + (2 * 128))
>  register char *stack_pointer __asm__("%sp");
> -
> -/* XXX Until we have a better place keep the definitions here.  */
> -
> -/* While there is no such syscall.  */
> -#define __exit_thread_inline(val) \
> -  INLINE_SYSCALL (exit, 1, (val))
> --- a/nptl/sysdeps/x86_64/pthreaddef.h
> +++ b/nptl/sysdeps/x86_64/pthreaddef.h
> @@ -42,10 +42,3 @@
>  /* Location of current stack frame.  The frame pointer is not usable.  */
>  #define CURRENT_STACK_FRAME \
>    ({ register char *frame __asm__("rsp"); frame; })
> -
> -
> -/* XXX Until we have a better place keep the definitions here.  */
> -
> -/* While there is no such syscall.  */
> -#define __exit_thread_inline(val) \
> -  asm volatile ("syscall" :: "a" (__NR_exit), "D" (val))
> --- a/sysdeps/aarch64/nptl/pthreaddef.h
> +++ b/sysdeps/aarch64/nptl/pthreaddef.h
> @@ -30,8 +30,3 @@
>
>  /* Location of current stack frame.  */
>  #define CURRENT_STACK_FRAME    __builtin_frame_address (0)
> -
> -
> -/* XXX Until we have a better place keep the definitions here.  */
> -#define __exit_thread_inline(val) \
> -  INLINE_SYSCALL (exit, 1, (val))
> --- a/sysdeps/alpha/nptl/pthreaddef.h
> +++ b/sysdeps/alpha/nptl/pthreaddef.h
> @@ -29,9 +29,3 @@
>
>  /* Location of current stack frame.  */
>  #define CURRENT_STACK_FRAME    __builtin_frame_address (0)
> -
> -/* XXX Until we have a better place keep the definitions here.  */
> -
> -/* While there is no such syscall.  */
> -#define __exit_thread_inline(val) \
> -  INLINE_SYSCALL (exit, 1, (val))
> --- a/sysdeps/arm/nptl/pthreaddef.h
> +++ b/sysdeps/arm/nptl/pthreaddef.h
> @@ -39,8 +39,3 @@
>     return the hard FP minus 12.  Of course, this makes no sense
>     without the obsolete APCS stack layout...  */
>  #define CURRENT_STACK_FRAME    (__builtin_frame_address (0) - 12)
> -
> -
> -/* XXX Until we have a better place keep the definitions here.  */
> -#define __exit_thread_inline(val) \
> -  INLINE_SYSCALL (exit, 1, (val))
> --- a/sysdeps/unix/sysv/linux/exit-thread.S
> +++ b/sysdeps/generic/exit-thread.h
> @@ -1,4 +1,5 @@
> -/* Copyright (C) 1991-2014 Free Software Foundation, Inc.
> +/* Call to terminate the current thread.  Stub version.
> +   Copyright (C) 2014 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
> @@ -15,8 +16,13 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
>
> -#include <sysdep.h>
> +/* This causes the current thread to exit, without affecting other
> +   threads in the process if there are any.  If there are no other
> +   threads left, then this has the effect of _exit (0).  */
>
> -PSEUDO (__exit_thread, exit, 1)
> -       /* Shouldn't get here.  */
> -PSEUDO_END(__exit_thread)
> +static inline void __attribute__ ((noreturn, always_inline, unused))
> +__exit_thread (void)
> +{
> +  while (1)
> +    asm ("write me!");
> +}
> --- a/sysdeps/hppa/nptl/pthreaddef.h
> +++ b/sysdeps/hppa/nptl/pthreaddef.h
> @@ -31,9 +31,3 @@
>  /* Location of current stack frame.  */
>  #define CURRENT_STACK_FRAME  stack_pointer
>  register char * stack_pointer __asm__ ("%r30");
> -
> -
> -/* XXX Until we have a better place keep the definitions here.  */
> -
> -#define __exit_thread_inline(val) \
> -  INLINE_SYSCALL (exit, 1, (val))
> --- a/sysdeps/ia64/nptl/pthreaddef.h
> +++ b/sysdeps/ia64/nptl/pthreaddef.h
> @@ -34,9 +34,3 @@
>  /* Location of current stack frame.  */
>  #define CURRENT_STACK_FRAME    __stack_pointer
>  register char *__stack_pointer __asm__ ("sp");
> -
> -/* XXX Until we have a better place keep the definitions here.  */
> -
> -/* While there is no such syscall.  */
> -#define __exit_thread_inline(val) \
> -  INLINE_SYSCALL (exit, 1, (val))
> --- a/sysdeps/m68k/nptl/pthreaddef.h
> +++ b/sysdeps/m68k/nptl/pthreaddef.h
> @@ -31,8 +31,3 @@
>
>  /* Location of current stack frame.  */
>  #define CURRENT_STACK_FRAME    __builtin_frame_address (0)
> -
> -
> -/* XXX Until we have a better place keep the definitions here.  */
> -#define __exit_thread_inline(val) \
> -  INLINE_SYSCALL (exit, 1, (val))
> --- a/sysdeps/microblaze/nptl/pthreaddef.h
> +++ b/sysdeps/microblaze/nptl/pthreaddef.h
> @@ -33,8 +33,3 @@
>
>  /* Location of current stack frame.  */
>  #define CURRENT_STACK_FRAME __builtin_frame_address (0)
> -
> -/* XXX Until we have a better place keep the definitions here.  */
> -
> -#define __exit_thread_inline(val) \
> -  INLINE_SYSCALL (exit, 1, (val))
> --- a/sysdeps/mips/nptl/pthreaddef.h
> +++ b/sysdeps/mips/nptl/pthreaddef.h
> @@ -30,9 +30,3 @@
>
>  /* Location of current stack frame.  */
>  #define CURRENT_STACK_FRAME    __builtin_frame_address (0)
> -
> -
> -/* XXX Until we have a better place keep the definitions here.  */
> -
> -#define __exit_thread_inline(val) \
> -  INLINE_SYSCALL (exit, 1, (val))
> --- a/sysdeps/tile/nptl/pthreaddef.h
> +++ b/sysdeps/tile/nptl/pthreaddef.h
> @@ -34,8 +34,3 @@
>
>  /* Location of current stack frame.  */
>  #define CURRENT_STACK_FRAME    __builtin_frame_address (0)
> -
> -/* XXX Until we have a better place keep the definitions here.  */
> -
> -#define __exit_thread_inline(val) \
> -  INLINE_SYSCALL (exit, 1, (val))
> --- a/sysdeps/unix/sysv/linux/Makefile
> +++ b/sysdeps/unix/sysv/linux/Makefile
> @@ -141,7 +141,7 @@ endif
>  ifeq ($(subdir),posix)
>  sysdep_headers += bits/initspin.h
>
> -sysdep_routines += exit-thread sched_getcpu
> +sysdep_routines += sched_getcpu
>
>  tests += tst-getcpu
>  endif
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/exit-thread.h
> @@ -0,0 +1,38 @@
> +/* Call to terminate the current thread.  Linux version.
> +   Copyright (C) 2014 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 <sysdep.h>
> +
> +/* This causes the current thread to exit, without affecting other
> +   threads in the process if there are any.  If there are no other
> +   threads left, then this has the effect of _exit (0).  */
> +
> +static inline void __attribute__ ((noreturn, always_inline, unused))
> +__exit_thread (void)
> +{
> +  /* Doing this in a loop is mostly just to satisfy the compiler that the
> +     function really qualifies as noreturn.  It also means that in some
> +     pathological situation where the system call does not get made or does
> +     not work, the thread will simply spin rather than running off the end
> +     of the caller and doing unexpectedly strange things.  */
> +  while (1)
> +    {
> +      INTERNAL_SYSCALL_DECL (err);
> +      INTERNAL_SYSCALL (exit, err, 1, 0);
> +    }
> +}



-- 
Will Newton
Toolchain Working Group, Linaro


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]