Re: [PATCH 07/18] nptl: Move __pthread_cleanup_upto into libc

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 12/03/2021 14:49, Florian Weimer via Libc-alpha wrote:
> This internal symbol is used as part of the longjmp implementation.
> Rename the file from nptl/pt-cleanup.c to nptl/pthread_cleanup_upto.c
> so that the pt-* files remain restricted to libpthread.

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  nptl/Makefile                                 | 2 +-
>  nptl/Versions                                 | 1 +
>  nptl/nptl-init.c                              | 1 -
>  nptl/pthreadP.h                               | 4 +---
>  nptl/{pt-cleanup.c => pthread_cleanup_upto.c} | 2 +-
>  sysdeps/nptl/jmp-unwind.c                     | 8 ++------
>  sysdeps/nptl/pthread-functions.h              | 1 -
>  sysdeps/unix/sysv/linux/s390/jmp-unwind.c     | 7 ++-----
>  8 files changed, 8 insertions(+), 18 deletions(-)
>  rename nptl/{pt-cleanup.c => pthread_cleanup_upto.c} (98%)
> 
> diff --git a/nptl/Makefile b/nptl/Makefile
> index 7e566675f2..8847c4dd49 100644
> --- a/nptl/Makefile
> +++ b/nptl/Makefile
> @@ -66,6 +66,7 @@ routines = \
>    pthread_attr_setscope \
>    pthread_attr_setsigmask \
>    pthread_attr_setsigmask_internal \
> +  pthread_cleanup_upto \
>    pthread_cond_destroy \
>    pthread_cond_init \
>    pthread_condattr_destroy \
> @@ -113,7 +114,6 @@ libpthread-routines = \
>    old_pthread_cond_timedwait \
>    old_pthread_cond_wait \
>    pt-allocrtsig \
> -  pt-cleanup \
>    pt-fcntl \
>    pt-interp \
>    pt-raise \

Ok.

> diff --git a/nptl/Versions b/nptl/Versions
> index cf4d5b677d..8fa0d178db 100644
> --- a/nptl/Versions
> +++ b/nptl/Versions
> @@ -109,6 +109,7 @@ libc {
>      __pthread_attr_setsigmask_internal;
>      __pthread_cleanup_pop;
>      __pthread_cleanup_push;
> +    __pthread_cleanup_upto;
>      __pthread_cond_destroy; # Used by the C11 threads.
>      __pthread_cond_init; # Used by the C11 threads.
>      __pthread_force_elision;

Ok.

> diff --git a/nptl/nptl-init.c b/nptl/nptl-init.c
> index c2b563cc68..41527dcad1 100644
> --- a/nptl/nptl-init.c
> +++ b/nptl/nptl-init.c
> @@ -88,7 +88,6 @@ static const struct pthread_functions pthread_functions =
>      .ptr_pthread_mutex_unlock = __pthread_mutex_unlock,
>      .ptr___pthread_setcancelstate = __pthread_setcancelstate,
>      .ptr_pthread_setcanceltype = __pthread_setcanceltype,
> -    .ptr___pthread_cleanup_upto = __pthread_cleanup_upto,
>      .ptr___pthread_once = __pthread_once,
>      .ptr___pthread_rwlock_rdlock = __pthread_rwlock_rdlock,
>      .ptr___pthread_rwlock_wrlock = __pthread_rwlock_wrlock,

Ok.

> diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
> index 781bc65c1c..8f3d2cc8c9 100644
> --- a/nptl/pthreadP.h
> +++ b/nptl/pthreadP.h
> @@ -332,9 +332,7 @@ extern int __make_stacks_executable (void **stack_endp) attribute_hidden;
>  
>  /* longjmp handling.  */
>  extern void __pthread_cleanup_upto (__jmp_buf target, char *targetframe);
> -#if IS_IN (libpthread)
> -hidden_proto (__pthread_cleanup_upto)
> -#endif
> +libc_hidden_proto (__pthread_cleanup_upto)
>  
>  
>  /* Functions with versioned interfaces.  */

Ok.

> diff --git a/nptl/pt-cleanup.c b/nptl/pthread_cleanup_upto.c
> similarity index 98%
> rename from nptl/pt-cleanup.c
> rename to nptl/pthread_cleanup_upto.c

Ok.

> index 541533230d..89da3e5229 100644
> --- a/nptl/pt-cleanup.c
> +++ b/nptl/pthread_cleanup_upto.c
> @@ -59,4 +59,4 @@ __pthread_cleanup_upto (__jmp_buf target, char *targetframe)
>  
>    THREAD_SETMEM (self, cleanup, cbuf);
>  }
> -hidden_def (__pthread_cleanup_upto)
> +libc_hidden_def (__pthread_cleanup_upto)

Ok.

> diff --git a/sysdeps/nptl/jmp-unwind.c b/sysdeps/nptl/jmp-unwind.c
> index a79c792e82..96c4862c74 100644
> --- a/sysdeps/nptl/jmp-unwind.c
> +++ b/sysdeps/nptl/jmp-unwind.c
> @@ -19,14 +19,10 @@
>  #include <setjmp.h>
>  #include <stddef.h>
>  #include <libc-lock.h>
> -
> -extern void __pthread_cleanup_upto (__jmp_buf env, char *targetframe);
> -#pragma weak __pthread_cleanup_upto
> -
> +#include <nptl/pthreadP.h>
>  
>  void
>  _longjmp_unwind (jmp_buf env, int val)
>  {
> -  __libc_ptf_call (__pthread_cleanup_upto, (env->__jmpbuf,
> -					    CURRENT_STACK_FRAME), 0);
> +  __pthread_cleanup_upto (env->__jmpbuf, CURRENT_STACK_FRAME);
>  }

Ok.

> diff --git a/sysdeps/nptl/pthread-functions.h b/sysdeps/nptl/pthread-functions.h
> index 4268084b66..1bbe609598 100644
> --- a/sysdeps/nptl/pthread-functions.h
> +++ b/sysdeps/nptl/pthread-functions.h
> @@ -49,7 +49,6 @@ struct pthread_functions
>    int (*ptr_pthread_mutex_unlock) (pthread_mutex_t *);
>    int (*ptr___pthread_setcancelstate) (int, int *);
>    int (*ptr_pthread_setcanceltype) (int, int *);
> -  void (*ptr___pthread_cleanup_upto) (__jmp_buf, char *);
>    int (*ptr___pthread_once) (pthread_once_t *, void (*) (void));
>    int (*ptr___pthread_rwlock_rdlock) (pthread_rwlock_t *);
>    int (*ptr___pthread_rwlock_wrlock) (pthread_rwlock_t *);

Ok.

> diff --git a/sysdeps/unix/sysv/linux/s390/jmp-unwind.c b/sysdeps/unix/sysv/linux/s390/jmp-unwind.c
> index 2bdde57ec7..8e0399a3e9 100644
> --- a/sysdeps/unix/sysv/linux/s390/jmp-unwind.c
> +++ b/sysdeps/unix/sysv/linux/s390/jmp-unwind.c
> @@ -19,15 +19,12 @@
>  #include <setjmp.h>
>  #include <stddef.h>
>  #include <libc-lock.h>
> -
> -extern void __pthread_cleanup_upto (__jmp_buf env, char *targetframe);
> -#pragma weak __pthread_cleanup_upto
> -
> +#include <nptl/pthreadP.h>
>  
>  void
>  _longjmp_unwind (jmp_buf env, int val)
>  {
>    char local_var;
>  
> -  __libc_ptf_call (__pthread_cleanup_upto, (env->__jmpbuf, &local_var), 0);
> +  __pthread_cleanup_upto (env->__jmpbuf, &local_var);
>  }
> 


Ok.