Re: [PATCH 3/3] htl: move pthread_self into libc

[Samuel Thibault]

Hello,

Make sure to run the abi checks, you can e.g. run from the build
directory:

make -C $src_dir/elf objdir=$PWD subdir=elf ..=../ $PWD/elf/check-localplt.out
make -C $src_dir/elf objdir=$PWD subdir=elf ..=../ $PWD/elf/check-abi-libc.out
make -C $src_dir/htl objdir=$PWD subdir=htl ..=../ 
$PWD/elf/check-abi-libpthread.out

It will show this issue:

@@ -30,0 +31 @@ GLIBC_2.11 mkstemps64 F
+GLIBC_2.12 __pthread_self F
@@ -2006 +2006,0 @@ GLIBC_2.21 pthread_mutex_unlock F
-GLIBC_2.21 pthread_self F

You indeed need to tell that __pthread_self is now exported by glibc.

And your change drops the 2.21 version that was coming from forward.c,
we need to keep it. So you need to add it as a compat symbol, something
like:

+#if SHLIB_COMPAT (libc, GLIBC_2_21, GLIBC_2_37)
+compat_symbol (libc, __pthread_self, pthread_self, GLIBC_2_21);
+#endif

Concerning check-localplt.out:
Extra PLT reference: libc.so: __pthread_self

it means that some C file is not getting access to the hidden
declaration. That's because you have put the hidden declaration in
htl/pt-internal.h, while it should go to ./sysdeps/htl/pthreadP.h so
that various code get it from there like they do with NPTL.
(generally, if in doubt, look at how NPTL does things).

Samuel

Guy-Fleury Iteriteka via Libc-alpha, le mar. 03 janv. 2023 12:45:15 +0200, a 
ecrit:
> ---
>  htl/Makefile                              |  3 +--
>  htl/Versions                              | 13 ++++++++++---
>  htl/forward.c                             |  4 ----
>  htl/pt-initialize.c                       |  1 -
>  htl/pt-internal.h                         |  1 +
>  htl/pt-self.c                             |  8 ++++++--
>  sysdeps/htl/pthread-functions.h           |  2 --
>  sysdeps/mach/hurd/i386/libc.abilist       |  2 ++
>  sysdeps/mach/hurd/i386/libpthread.abilist |  2 --
>  9 files changed, 20 insertions(+), 16 deletions(-)
> 
> diff --git a/htl/Makefile b/htl/Makefile
> index b569cfcd..c75855ad 100644
> --- a/htl/Makefile
> +++ b/htl/Makefile
> @@ -52,7 +52,6 @@ libpthread-routines := pt-attr pt-attr-destroy 
> pt-attr-getdetachstate           \
>       pt-exit                                                             \
>       pt-initialize                                                       \
>       pt-join                                                             \
> -     pt-self                                                             \
>       pt-sigmask                                                          \
>       pt-spin-inlines                                                     \
>       pt-cleanup                                                          \
> @@ -164,7 +163,7 @@ headers :=                                \
>  
>  distribute :=
>  
> -routines := forward libc_pthread_init alloca_cutoff htlfreeres pt-nthreads 
> pt-pthread_self
> +routines := forward libc_pthread_init alloca_cutoff htlfreeres pt-nthreads 
> pt-pthread_self pt-self
>  shared-only-routines = forward
>  
>  extra-libs := libpthread
> diff --git a/htl/Versions b/htl/Versions
> index 9ec84811..e1cb362c 100644
> --- a/htl/Versions
> +++ b/htl/Versions
> @@ -1,4 +1,9 @@
>  libc {
> +  GLIBC_2.12 {
> +     pthread_self;
> +    __pthread_self;
> +  }
> +
>    GLIBC_2.21 {
>      pthread_attr_destroy; pthread_attr_getdetachstate;
>      pthread_attr_getinheritsched; pthread_attr_getschedparam;
> @@ -26,6 +31,11 @@ libc {
>      thrd_current; thrd_equal; thrd_sleep; thrd_yield;
>    }
>  
> +  GLIBC_2.37 {
> +    pthread_self;
> +    __pthread_self;
> +  }
> +
>    GLIBC_PRIVATE {
>      __libc_alloca_cutoff;
>      __libc_pthread_init;
> @@ -119,9 +129,6 @@ libpthread {
>      pthread_rwlockattr_destroy; pthread_rwlockattr_getpshared;
>      pthread_rwlockattr_init; pthread_rwlockattr_setpshared;
>  
> -    pthread_self;
> -    __pthread_self;
> -
>      pthread_setcancelstate; pthread_setcanceltype;
>      pthread_setconcurrency; pthread_setschedparam;
>      pthread_setschedprio; pthread_setspecific;
> diff --git a/htl/forward.c b/htl/forward.c
> index 00527348..57b0b66c 100644
> --- a/htl/forward.c
> +++ b/htl/forward.c
> @@ -130,10 +130,6 @@ FORWARD (pthread_mutex_lock, (pthread_mutex_t *mutex), 
> (mutex), 0)
>  
>  FORWARD (pthread_mutex_unlock, (pthread_mutex_t *mutex), (mutex), 0)
>  
> -
> -FORWARD2 (pthread_self, pthread_t, (void), (), return 0)
> -
> -
>  FORWARD (__pthread_setcancelstate, (int state, int *oldstate),
>        (state, oldstate), 0)
>  strong_alias (__pthread_setcancelstate, pthread_setcancelstate);
> diff --git a/htl/pt-initialize.c b/htl/pt-initialize.c
> index 02e6ad6b..fcad3b13 100644
> --- a/htl/pt-initialize.c
> +++ b/htl/pt-initialize.c
> @@ -56,7 +56,6 @@ static const struct pthread_functions pthread_functions = {
>    .ptr_pthread_mutex_lock = __pthread_mutex_lock,
>    .ptr_pthread_mutex_trylock = __pthread_mutex_trylock,
>    .ptr_pthread_mutex_unlock = __pthread_mutex_unlock,
> -  .ptr_pthread_self = __pthread_self,
>    .ptr___pthread_setcancelstate = __pthread_setcancelstate,
>    .ptr_pthread_setcanceltype = __pthread_setcanceltype,
>    .ptr___pthread_get_cleanup_stack = __pthread_get_cleanup_stack,
> diff --git a/htl/pt-internal.h b/htl/pt-internal.h
> index b787acf8..ade71b24 100644
> --- a/htl/pt-internal.h
> +++ b/htl/pt-internal.h
> @@ -192,6 +192,7 @@ extern int __pthread_max_threads;
>  extern struct __pthread *_pthread_self (void);
>  #endif
>  
> +libc_hidden_proto (__pthread_self)
>  /* Stores the stack of cleanup handlers for the thread.  */
>  extern __thread struct __pthread_cancelation_handler 
> *__pthread_cleanup_stack;
>  
> diff --git a/htl/pt-self.c b/htl/pt-self.c
> index e05ec69b..dc37d5dc 100644
> --- a/htl/pt-self.c
> +++ b/htl/pt-self.c
> @@ -17,7 +17,7 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #include <pthread.h>
> -
> +#include <shlib-compat.h>
>  #include <pt-internal.h>
>  
>  /* Return the thread ID of the calling thread.  */
> @@ -36,4 +36,8 @@ __pthread_self (void)
>    return self->thread;
>  }
>  
> -weak_alias (__pthread_self, pthread_self);
> +libc_hidden_def (__pthread_self)
> +versioned_symbol (libc, __pthread_self, pthread_self, GLIBC_2_37);
> +#if OTHER_SHLIB_COMPAT (libpthread, GLIBC_2_12, GLIBC_2_37)
> +compat_symbol (libc, __pthread_self, pthread_self, GLIBC_2_12);
> +#endif
> \ No newline at end of file
> diff --git a/sysdeps/htl/pthread-functions.h b/sysdeps/htl/pthread-functions.h
> index ccccc8e5..2f0e3df0 100644
> --- a/sysdeps/htl/pthread-functions.h
> +++ b/sysdeps/htl/pthread-functions.h
> @@ -56,7 +56,6 @@ int _pthread_mutex_init (pthread_mutex_t *,
>  int __pthread_mutex_lock (pthread_mutex_t *);
>  int __pthread_mutex_trylock (pthread_mutex_t *);
>  int __pthread_mutex_unlock (pthread_mutex_t *);
> -pthread_t __pthread_self (void);
>  int __pthread_setcancelstate (int, int *);
>  int __pthread_setcanceltype (int, int *);
>  struct __pthread_cancelation_handler **__pthread_get_cleanup_stack (void);
> @@ -112,7 +111,6 @@ struct pthread_functions
>    int (*ptr_pthread_mutex_lock) (pthread_mutex_t *);
>    int (*ptr_pthread_mutex_trylock) (pthread_mutex_t *);
>    int (*ptr_pthread_mutex_unlock) (pthread_mutex_t *);
> -  pthread_t (*ptr_pthread_self) (void);
>    int (*ptr___pthread_setcancelstate) (int, int *);
>    int (*ptr_pthread_setcanceltype) (int, int *);
>    struct __pthread_cancelation_handler **(*ptr___pthread_get_cleanup_stack) 
> (void);
> diff --git a/sysdeps/mach/hurd/i386/libc.abilist 
> b/sysdeps/mach/hurd/i386/libc.abilist
> index 4e3200ef..29b08e73 100644
> --- a/sysdeps/mach/hurd/i386/libc.abilist
> +++ b/sysdeps/mach/hurd/i386/libc.abilist
> @@ -28,6 +28,7 @@ GLIBC_2.11 mkostemps F
>  GLIBC_2.11 mkostemps64 F
>  GLIBC_2.11 mkstemps F
>  GLIBC_2.11 mkstemps64 F
> +GLIBC_2.12 pthread_self F
>  GLIBC_2.13 __fentry__ F
>  GLIBC_2.14 syncfs F
>  GLIBC_2.15 __fdelt_chk F
> @@ -2294,6 +2295,7 @@ GLIBC_2.36 arc4random_buf F
>  GLIBC_2.36 arc4random_uniform F
>  GLIBC_2.36 c8rtomb F
>  GLIBC_2.36 mbrtoc8 F
> +GLIBC_2.37 pthread_self F
>  GLIBC_2.4 __confstr_chk F
>  GLIBC_2.4 __fgets_chk F
>  GLIBC_2.4 __fgets_unlocked_chk F
> diff --git a/sysdeps/mach/hurd/i386/libpthread.abilist 
> b/sysdeps/mach/hurd/i386/libpthread.abilist
> index b9c9b75c..2ef0f670 100644
> --- a/sysdeps/mach/hurd/i386/libpthread.abilist
> +++ b/sysdeps/mach/hurd/i386/libpthread.abilist
> @@ -4,7 +4,6 @@ GLIBC_2.12 __pthread_get_cleanup_stack F
>  GLIBC_2.12 __pthread_key_create F
>  GLIBC_2.12 __pthread_kill F
>  GLIBC_2.12 __pthread_mutex_transfer_np F
> -GLIBC_2.12 __pthread_self F
>  GLIBC_2.12 __pthread_spin_destroy F
>  GLIBC_2.12 __pthread_spin_init F
>  GLIBC_2.12 __pthread_spin_lock F
> @@ -109,7 +108,6 @@ GLIBC_2.12 pthread_rwlockattr_destroy F
>  GLIBC_2.12 pthread_rwlockattr_getpshared F
>  GLIBC_2.12 pthread_rwlockattr_init F
>  GLIBC_2.12 pthread_rwlockattr_setpshared F
> -GLIBC_2.12 pthread_self F
>  GLIBC_2.12 pthread_setcancelstate F
>  GLIBC_2.12 pthread_setcanceltype F
>  GLIBC_2.12 pthread_setconcurrency F
> -- 
> 2.38.1
> 

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