[01/10] htl: Rename _pthread_mutex_init/destroy to __pthread_mutex_init/destroy

Message ID 20200114185255.25813-3-samuel.thibault@ens-lyon.org
State New
Headers show
Series
  • Port C11 threads to GNU/Hurd
Related show

Commit Message

Samuel Thibault Jan. 14, 2020, 6:52 p.m.
---
 htl/Versions                              | 2 ++
 htl/pt-initialize.c                       | 4 ++--
 sysdeps/htl/pt-key.h                      | 2 +-
 sysdeps/htl/pt-mutex-destroy.c            | 4 ++--
 sysdeps/htl/pt-mutex-init.c               | 4 ++--
 sysdeps/htl/pthreadP.h                    | 5 +++--
 sysdeps/mach/hurd/htl/pt-mutex-destroy.c  | 4 ++--
 sysdeps/mach/hurd/htl/pt-mutex-init.c     | 6 +++---
 sysdeps/mach/hurd/i386/libpthread.abilist | 2 --
 9 files changed, 17 insertions(+), 16 deletions(-)

-- 
2.24.1

Comments

Samuel Thibault Jan. 21, 2020, 9:28 p.m. | #1
Hello,

Adhemerval Zanella, le ven. 17 janv. 2020 17:40:09 -0300, a ecrit:
> > diff --git a/sysdeps/mach/hurd/i386/libpthread.abilist b/sysdeps/mach/hurd/i386/libpthread.abilist

> > index 0ede90859c..cda8755960 100644

> > --- a/sysdeps/mach/hurd/i386/libpthread.abilist

> > +++ b/sysdeps/mach/hurd/i386/libpthread.abilist

> > @@ -14,8 +14,6 @@ GLIBC_2.12 _cthread_init_routine D 0x4

> >  GLIBC_2.12 _cthreads_flockfile F

> >  GLIBC_2.12 _cthreads_ftrylockfile F

> >  GLIBC_2.12 _cthreads_funlockfile F

> > -GLIBC_2.12 _pthread_mutex_destroy F

> > -GLIBC_2.12 _pthread_mutex_init F

> >  GLIBC_2.12 _pthread_mutex_lock F

> >  GLIBC_2.12 _pthread_mutex_trylock F

> >  GLIBC_2.12 _pthread_mutex_unlock F

> 

> I understand this change is follow Linux internal implementation

> and make mtx_init.c generic, but I don't think changing hurd 

> libpthread exported symbols is the correct solution. 

> 

> Since the symbol won't be used anymore I think we can move to

> a compat symbol, something like:

> 

> +strong_alias (__pthread_mutex_init, pthread_mutex_init);

> +hidden_def (__pthread_mutex_init)

> +#if SHLIB_COMPAT (libpthread, GLIBC_2_12, GLIBC_2_31)

> +compat_symbol (libpthread, __pthread_mutex_init, _pthread_mutex_init, GLIBC_2_12);

> +#endif


But do we need to keep the compat symbols at all? 

_pthread_mutex_lock has never been exposed in a .h file, it should have
gotten version GLIBC_PRIVATE actually since it's only used between
libc.so and libpthread.so.

Samuel
Samuel Thibault Jan. 27, 2020, 9:55 p.m. | #2
Hello,

Expanding the subject a bit to catch more opinions on this:

Samuel Thibault, le mar. 21 janv. 2020 22:28:50 +0100, a ecrit:
> Adhemerval Zanella, le ven. 17 janv. 2020 17:40:09 -0300, a ecrit:

> > > diff --git a/sysdeps/mach/hurd/i386/libpthread.abilist b/sysdeps/mach/hurd/i386/libpthread.abilist

> > > index 0ede90859c..cda8755960 100644

> > > --- a/sysdeps/mach/hurd/i386/libpthread.abilist

> > > +++ b/sysdeps/mach/hurd/i386/libpthread.abilist

> > > @@ -14,8 +14,6 @@ GLIBC_2.12 _cthread_init_routine D 0x4

> > >  GLIBC_2.12 _cthreads_flockfile F

> > >  GLIBC_2.12 _cthreads_ftrylockfile F

> > >  GLIBC_2.12 _cthreads_funlockfile F

> > > -GLIBC_2.12 _pthread_mutex_destroy F

> > > -GLIBC_2.12 _pthread_mutex_init F

> > >  GLIBC_2.12 _pthread_mutex_lock F

> > >  GLIBC_2.12 _pthread_mutex_trylock F

> > >  GLIBC_2.12 _pthread_mutex_unlock F

> > 

> > I understand this change is follow Linux internal implementation

> > and make mtx_init.c generic, but I don't think changing hurd 

> > libpthread exported symbols is the correct solution. 

> > 

> > Since the symbol won't be used anymore I think we can move to

> > a compat symbol, something like:

> > 

> > +strong_alias (__pthread_mutex_init, pthread_mutex_init);

> > +hidden_def (__pthread_mutex_init)

> > +#if SHLIB_COMPAT (libpthread, GLIBC_2_12, GLIBC_2_31)

> > +compat_symbol (libpthread, __pthread_mutex_init, _pthread_mutex_init, GLIBC_2_12);

> > +#endif

> 

> But do we need to keep the compat symbols at all? 

> 

> _pthread_mutex_lock has never been exposed in a .h file, it should have

> gotten version GLIBC_PRIVATE actually since it's only used between

> libc.so and libpthread.so.


Samuel
Adhemerval Zanella Jan. 28, 2020, 12:56 p.m. | #3
On 27/01/2020 18:55, Samuel Thibault wrote:
> Hello,

> 

> Expanding the subject a bit to catch more opinions on this:

> 

> Samuel Thibault, le mar. 21 janv. 2020 22:28:50 +0100, a ecrit:

>> Adhemerval Zanella, le ven. 17 janv. 2020 17:40:09 -0300, a ecrit:

>>>> diff --git a/sysdeps/mach/hurd/i386/libpthread.abilist b/sysdeps/mach/hurd/i386/libpthread.abilist

>>>> index 0ede90859c..cda8755960 100644

>>>> --- a/sysdeps/mach/hurd/i386/libpthread.abilist

>>>> +++ b/sysdeps/mach/hurd/i386/libpthread.abilist

>>>> @@ -14,8 +14,6 @@ GLIBC_2.12 _cthread_init_routine D 0x4

>>>>  GLIBC_2.12 _cthreads_flockfile F

>>>>  GLIBC_2.12 _cthreads_ftrylockfile F

>>>>  GLIBC_2.12 _cthreads_funlockfile F

>>>> -GLIBC_2.12 _pthread_mutex_destroy F

>>>> -GLIBC_2.12 _pthread_mutex_init F

>>>>  GLIBC_2.12 _pthread_mutex_lock F

>>>>  GLIBC_2.12 _pthread_mutex_trylock F

>>>>  GLIBC_2.12 _pthread_mutex_unlock F

>>>

>>> I understand this change is follow Linux internal implementation

>>> and make mtx_init.c generic, but I don't think changing hurd 

>>> libpthread exported symbols is the correct solution. 

>>>

>>> Since the symbol won't be used anymore I think we can move to

>>> a compat symbol, something like:

>>>

>>> +strong_alias (__pthread_mutex_init, pthread_mutex_init);

>>> +hidden_def (__pthread_mutex_init)

>>> +#if SHLIB_COMPAT (libpthread, GLIBC_2_12, GLIBC_2_31)

>>> +compat_symbol (libpthread, __pthread_mutex_init, _pthread_mutex_init, GLIBC_2_12);

>>> +#endif

>>

>> But do we need to keep the compat symbols at all? 

>>

>> _pthread_mutex_lock has never been exposed in a .h file, it should have

>> gotten version GLIBC_PRIVATE actually since it's only used between

>> libc.so and libpthread.so.


I think it might be fine in this case, assuming there is no external linkage
possible with header redirection. It is still a ABI break, but if an
application is using such symbol it is abusing the API.
Florian Weimer Jan. 28, 2020, 1:58 p.m. | #4
* Adhemerval Zanella:

> On 27/01/2020 18:55, Samuel Thibault wrote:

>> Hello,

>> 

>> Expanding the subject a bit to catch more opinions on this:

>> 

>> Samuel Thibault, le mar. 21 janv. 2020 22:28:50 +0100, a ecrit:

>>> Adhemerval Zanella, le ven. 17 janv. 2020 17:40:09 -0300, a ecrit:

>>>>> diff --git a/sysdeps/mach/hurd/i386/libpthread.abilist b/sysdeps/mach/hurd/i386/libpthread.abilist

>>>>> index 0ede90859c..cda8755960 100644

>>>>> --- a/sysdeps/mach/hurd/i386/libpthread.abilist

>>>>> +++ b/sysdeps/mach/hurd/i386/libpthread.abilist

>>>>> @@ -14,8 +14,6 @@ GLIBC_2.12 _cthread_init_routine D 0x4

>>>>>  GLIBC_2.12 _cthreads_flockfile F

>>>>>  GLIBC_2.12 _cthreads_ftrylockfile F

>>>>>  GLIBC_2.12 _cthreads_funlockfile F

>>>>> -GLIBC_2.12 _pthread_mutex_destroy F

>>>>> -GLIBC_2.12 _pthread_mutex_init F

>>>>>  GLIBC_2.12 _pthread_mutex_lock F

>>>>>  GLIBC_2.12 _pthread_mutex_trylock F

>>>>>  GLIBC_2.12 _pthread_mutex_unlock F

>>>>

>>>> I understand this change is follow Linux internal implementation

>>>> and make mtx_init.c generic, but I don't think changing hurd 

>>>> libpthread exported symbols is the correct solution. 

>>>>

>>>> Since the symbol won't be used anymore I think we can move to

>>>> a compat symbol, something like:

>>>>

>>>> +strong_alias (__pthread_mutex_init, pthread_mutex_init);

>>>> +hidden_def (__pthread_mutex_init)

>>>> +#if SHLIB_COMPAT (libpthread, GLIBC_2_12, GLIBC_2_31)

>>>> +compat_symbol (libpthread, __pthread_mutex_init, _pthread_mutex_init, GLIBC_2_12);

>>>> +#endif

>>>

>>> But do we need to keep the compat symbols at all? 

>>>

>>> _pthread_mutex_lock has never been exposed in a .h file, it should have

>>> gotten version GLIBC_PRIVATE actually since it's only used between

>>> libc.so and libpthread.so.

>

> I think it might be fine in this case, assuming there is no external

> linkage possible with header redirection. It is still a ABI break, but

> if an application is using such symbol it is abusing the API.


I don't have any ABI tooling for Hurd.  But on GNU/Linux, GNU packages
often use internal APIs of other GNU packages, without much coordination
(probably assuming that's okay because it's all GNU).  For glibc, we try
to tell developers not to do that, but it's an ongoing effort and very
much incomplete at this point.

Thanks,
Florian

Patch

diff --git a/htl/Versions b/htl/Versions
index 77f7335b9c..3ae4b5c17d 100644
--- a/htl/Versions
+++ b/htl/Versions
@@ -155,6 +155,8 @@  libpthread {
     __pthread_setspecific;
     __pthread_getattr_np;
     __pthread_attr_getstack;
+    __pthread_mutex_init;
+    __pthread_mutex_destroy;
     __pthread_mutex_timedlock;
   }
 }
diff --git a/htl/pt-initialize.c b/htl/pt-initialize.c
index d5a64f7cd9..89ed742422 100644
--- a/htl/pt-initialize.c
+++ b/htl/pt-initialize.c
@@ -51,8 +51,8 @@  static const struct pthread_functions pthread_functions = {
   .ptr___pthread_exit = __pthread_exit,
   .ptr_pthread_getschedparam = __pthread_getschedparam,
   .ptr_pthread_setschedparam = __pthread_setschedparam,
-  .ptr_pthread_mutex_destroy = _pthread_mutex_destroy,
-  .ptr_pthread_mutex_init = _pthread_mutex_init,
+  .ptr_pthread_mutex_destroy = __pthread_mutex_destroy,
+  .ptr_pthread_mutex_init = __pthread_mutex_init,
   .ptr_pthread_mutex_lock = __pthread_mutex_lock,
   .ptr_pthread_mutex_trylock = __pthread_mutex_trylock,
   .ptr_pthread_mutex_unlock = __pthread_mutex_unlock,
diff --git a/sysdeps/htl/pt-key.h b/sysdeps/htl/pt-key.h
index bfaa19900a..b547e8ad29 100644
--- a/sysdeps/htl/pt-key.h
+++ b/sysdeps/htl/pt-key.h
@@ -66,7 +66,7 @@  __pthread_key_lock_ready (void)
     err = __pthread_mutexattr_settype (&attr, PTHREAD_MUTEX_RECURSIVE);
     assert_perror (err);
 
-    err = _pthread_mutex_init (&__pthread_key_lock, &attr);
+    err = __pthread_mutex_init (&__pthread_key_lock, &attr);
     assert_perror (err);
 
     err = __pthread_mutexattr_destroy (&attr);
diff --git a/sysdeps/htl/pt-mutex-destroy.c b/sysdeps/htl/pt-mutex-destroy.c
index 3824e62dd4..796fc11b8d 100644
--- a/sysdeps/htl/pt-mutex-destroy.c
+++ b/sysdeps/htl/pt-mutex-destroy.c
@@ -23,7 +23,7 @@ 
 #include <pt-internal.h>
 
 int
-_pthread_mutex_destroy (pthread_mutex_t *mutex)
+__pthread_mutex_destroy (pthread_mutex_t *mutex)
 {
   if (mutex->__attr == __PTHREAD_ERRORCHECK_MUTEXATTR
       || mutex->__attr == __PTHREAD_RECURSIVE_MUTEXATTR)
@@ -35,4 +35,4 @@  _pthread_mutex_destroy (pthread_mutex_t *mutex)
   return 0;
 }
 
-strong_alias (_pthread_mutex_destroy, pthread_mutex_destroy);
+strong_alias (__pthread_mutex_destroy, pthread_mutex_destroy);
diff --git a/sysdeps/htl/pt-mutex-init.c b/sysdeps/htl/pt-mutex-init.c
index c59bd8a3f5..77f041352e 100644
--- a/sysdeps/htl/pt-mutex-init.c
+++ b/sysdeps/htl/pt-mutex-init.c
@@ -24,7 +24,7 @@ 
 #include <pt-internal.h>
 
 int
-_pthread_mutex_init (pthread_mutex_t *mutex, const pthread_mutexattr_t *attr)
+__pthread_mutex_init (pthread_mutex_t *mutex, const pthread_mutexattr_t *attr)
 {
   *mutex = (pthread_mutex_t) __PTHREAD_MUTEX_INITIALIZER;
 
@@ -47,4 +47,4 @@  _pthread_mutex_init (pthread_mutex_t *mutex, const pthread_mutexattr_t *attr)
   return 0;
 }
 
-strong_alias (_pthread_mutex_init, pthread_mutex_init);
+strong_alias (__pthread_mutex_init, pthread_mutex_init);
diff --git a/sysdeps/htl/pthreadP.h b/sysdeps/htl/pthreadP.h
index affe7cdf53..c6ddf76fd4 100644
--- a/sysdeps/htl/pthreadP.h
+++ b/sysdeps/htl/pthreadP.h
@@ -27,7 +27,8 @@  extern pthread_t __pthread_self (void);
 extern int __pthread_kill (pthread_t threadid, int signo);
 extern struct __pthread **__pthread_threads;
 
-extern int _pthread_mutex_init (pthread_mutex_t *mutex, const pthread_mutexattr_t *attr);
+extern int __pthread_mutex_init (pthread_mutex_t *__mutex, const pthread_mutexattr_t *__attr);
+extern int __pthread_mutex_destroy (pthread_mutex_t *__mutex);
 extern int __pthread_mutex_lock (pthread_mutex_t *__mutex);
 extern int __pthread_mutex_timedlock (pthread_mutex_t *__mutex,
      const struct timespec *__abstime);
@@ -73,7 +74,7 @@  struct __pthread_cancelation_handler **___pthread_get_cleanup_stack (void) attri
 hidden_proto (__pthread_key_create)
 hidden_proto (__pthread_getspecific)
 hidden_proto (__pthread_setspecific)
-hidden_proto (_pthread_mutex_init)
+hidden_proto (__pthread_mutex_init)
 #endif
 
 #define ASSERT_TYPE_SIZE(type, size) 					\
diff --git a/sysdeps/mach/hurd/htl/pt-mutex-destroy.c b/sysdeps/mach/hurd/htl/pt-mutex-destroy.c
index afab03234e..a8a0adc03f 100644
--- a/sysdeps/mach/hurd/htl/pt-mutex-destroy.c
+++ b/sysdeps/mach/hurd/htl/pt-mutex-destroy.c
@@ -24,7 +24,7 @@ 
 #include <hurdlock.h>
 
 int
-_pthread_mutex_destroy (pthread_mutex_t *mtxp)
+__pthread_mutex_destroy (pthread_mutex_t *mtxp)
 {
   atomic_read_barrier ();
   if (*(volatile unsigned int *) &mtxp->__lock != 0)
@@ -34,4 +34,4 @@  _pthread_mutex_destroy (pthread_mutex_t *mtxp)
   return 0;
 }
 
-strong_alias (_pthread_mutex_destroy, pthread_mutex_destroy)
+strong_alias (__pthread_mutex_destroy, pthread_mutex_destroy)
diff --git a/sysdeps/mach/hurd/htl/pt-mutex-init.c b/sysdeps/mach/hurd/htl/pt-mutex-init.c
index 6b804116a5..7a2cc462f3 100644
--- a/sysdeps/mach/hurd/htl/pt-mutex-init.c
+++ b/sysdeps/mach/hurd/htl/pt-mutex-init.c
@@ -32,7 +32,7 @@  static const pthread_mutexattr_t dfl_attr = {
 };
 
 int
-_pthread_mutex_init (pthread_mutex_t *mtxp, const pthread_mutexattr_t *attrp)
+__pthread_mutex_init (pthread_mutex_t *mtxp, const pthread_mutexattr_t *attrp)
 {
   ASSERT_TYPE_SIZE (pthread_mutex_t, __SIZEOF_PTHREAD_MUTEX_T);
 
@@ -55,5 +55,5 @@  _pthread_mutex_init (pthread_mutex_t *mtxp, const pthread_mutexattr_t *attrp)
   return 0;
 }
 
-strong_alias (_pthread_mutex_init, pthread_mutex_init)
-hidden_def (_pthread_mutex_init)
+strong_alias (__pthread_mutex_init, pthread_mutex_init)
+hidden_def (__pthread_mutex_init)
diff --git a/sysdeps/mach/hurd/i386/libpthread.abilist b/sysdeps/mach/hurd/i386/libpthread.abilist
index 0ede90859c..cda8755960 100644
--- a/sysdeps/mach/hurd/i386/libpthread.abilist
+++ b/sysdeps/mach/hurd/i386/libpthread.abilist
@@ -14,8 +14,6 @@  GLIBC_2.12 _cthread_init_routine D 0x4
 GLIBC_2.12 _cthreads_flockfile F
 GLIBC_2.12 _cthreads_ftrylockfile F
 GLIBC_2.12 _cthreads_funlockfile F
-GLIBC_2.12 _pthread_mutex_destroy F
-GLIBC_2.12 _pthread_mutex_init F
 GLIBC_2.12 _pthread_mutex_lock F
 GLIBC_2.12 _pthread_mutex_trylock F
 GLIBC_2.12 _pthread_mutex_unlock F