| Message ID | 66f35cff.650a0220.2897d1.6678@mx.google.com |
|---|---|
| State | New |
| Headers | show |
| Series | libgcc, libstdc++: Make more entities no longer TU-local [PR115126] | expand |
On Wed, 25 Sept 2024 at 01:50, Nathaniel Shead <nathanieloshead@gmail.com> wrote: > > I found that my previous minimal change to libstdc++ was only sufficient > to pass regtest on x86_64-pc-linux-gnu; Linaro complained about ARM and > aarch64. This patch removes the rest of the internal-linkage entities I > could find exposed via libstdc++. > > The libgcc changes include some blocks specific to FreeBSD, Solaris <10, > and HP-UX; I haven't been able to test these changes at this time. > Happy to adjust or remove those hunks as needed. Apologies if I haven't > CC'd in the correct people. > > Bootstrapped and regtested on x86_64-pc-linux-gnu and > aarch64-unknown-linux-gnu, OK for trunk? > > -- >8 -- > > In C++20, modules streaming check for exposures of TU-local entities. > In general exposing internal linkage functions in a header is liable to > cause ODR violations in C++, and this is now detected in a module > context. > > This patch goes through and removes 'static' from many functions exposed > through libstdc++ to prevent code like the following from failing: > > export module M; > extern "C++" { > #include <bits/stdc++.h> > } > > Since gthreads is used from C as well, we need to choose whether to use > 'inline' or 'static inline' depending on whether we're compiling for C > or C++ (since the semantics of 'inline' are different between the > languages). Additionally we need to remove static global variables, so > we migrate these to function-local statics to avoid the ODR issues. > > There doesn't seem to be a good workaround for weakrefs, so I've left > them as-is and will work around it in the modules streaming code to > consider them as not TU-local. > > The same issue occurs in the objective-C specific parts of gthreads, but > I'm not familiar with the surrounding context and we don't currently > test modules with Objective C++ anyway so I've left it as-is. > > PR libstdc++/115126 > > libgcc/ChangeLog: > > * gthr-posix.h (__GTHREAD_INLINE): New macro. > (__gthread_active): Convert from variable to function. > (__gthread_trigger): Mark as __GTHREAD_INLINE instead of static. > (__gthread_active_p): Likewise. > (__gthread_create): Likewise. > (__gthread_join): Likewise. > (__gthread_detach): Likewise. > (__gthread_equal): Likewise. > (__gthread_self): Likewise. > (__gthread_yield): Likewise. > (__gthread_once): Likewise. > (__gthread_key_create): Likewise. > (__gthread_key_delete): Likewise. > (__gthread_getspecific): Likewise. > (__gthread_setspecific): Likewise. > (__gthread_mutex_init_function): Likewise. > (__gthread_mutex_destroy): Likewise. > (__gthread_mutex_lock): Likewise. > (__gthread_mutex_trylock): Likewise. > (__gthread_mutex_timedlock): Likewise. > (__gthread_mutex_unlock): Likewise. > (__gthread_recursive_mutex_init_function): Likewise. > (__gthread_recursive_mutex_lock): Likewise. > (__gthread_recursive_mutex_trylock): Likewise. > (__gthread_recursive_mutex_timedlock): Likewise. > (__gthread_recursive_mutex_unlock): Likewise. > (__gthread_recursive_mutex_destroy): Likewise. > (__gthread_cond_init_function): Likewise. > (__gthread_cond_broadcast): Likewise. > (__gthread_cond_signal): Likewise. > (__gthread_cond_wait): Likewise. > (__gthread_cond_timedwait): Likewise. > (__gthread_cond_wait_recursive): Likewise. > (__gthread_cond_destroy): Likewise. > (__gthread_rwlock_rdlock): Likewise. > (__gthread_rwlock_tryrdlock): Likewise. > (__gthread_rwlock_wrlock): Likewise. > (__gthread_rwlock_trywrlock): Likewise. > (__gthread_rwlock_unlock): Likewise. > * gthr-single.h (__GTHREAD_INLINE): New macro. > (__gthread_active_p): Mark as __GTHREAD_INLINE instead of static. > (__gthread_once): Likewise. > (__gthread_key_create): Likewise. > (__gthread_key_delete): Likewise. > (__gthread_getspecific): Likewise. > (__gthread_setspecific): Likewise. > (__gthread_mutex_destroy): Likewise. > (__gthread_mutex_lock): Likewise. > (__gthread_mutex_trylock): Likewise. > (__gthread_mutex_unlock): Likewise. > (__gthread_recursive_mutex_lock): Likewise. > (__gthread_recursive_mutex_trylock): Likewise. > (__gthread_recursive_mutex_unlock): Likewise. > (__gthread_recursive_mutex_destroy): Likewise. > > libstdc++-v3/ChangeLog: > > * include/bits/shared_ptr.h (std::__is_shared_ptr): Remove > unnecessary 'static'. > * include/bits/unique_ptr.h (std::__is_unique_ptr): Likewise. > * include/std/future (std::__craete_task_state): Likewise. Typo in the function name here. The libstdc++ parts are OK for trunk (and gcc-14 if that makes sense too, maybe for Clang?). If the libgcc parts take longer to get approved, please feel free to split the patch and push the libstdc++ parts separately. I can't approve the libgcc parts myself, but they look OK to me. I have one comment about them below. > * include/std/shared_mutex (_GLIBCXX_GTRHW): Likewise. > (__glibcxx_rwlock_init): Likewise. > (__glibcxx_rwlock_timedrdlock): Likewise. > (__glibcxx_rwlock_timedwrlock): Likewise. > (__glibcxx_rwlock_rdlock): Likewise. > (__glibcxx_rwlock_tryrdlock): Likewise. > (__glibcxx_rwlock_wrlock): Likewise. > (__glibcxx_rwlock_trywrlock): Likewise. > (__glibcxx_rwlock_unlock): Likewise. > (__glibcxx_rwlock_destroy): Likewise. > (__glibcxx_rwlock_init): Likewise. > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> > --- > libgcc/gthr-posix.h | 116 ++++++++++++++----------- > libgcc/gthr-single.h | 34 +++++--- > libstdc++-v3/include/bits/shared_ptr.h | 4 +- > libstdc++-v3/include/bits/unique_ptr.h | 4 +- > libstdc++-v3/include/std/future | 2 +- > libstdc++-v3/include/std/shared_mutex | 26 +++--- > 6 files changed, 104 insertions(+), 82 deletions(-) > > diff --git a/libgcc/gthr-posix.h b/libgcc/gthr-posix.h > index 82e8f9ffcf6..557bae7aaaa 100644 > --- a/libgcc/gthr-posix.h > +++ b/libgcc/gthr-posix.h > @@ -44,6 +44,12 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see > # endif > #endif > > +#ifdef __cplusplus > +# define __GTHREAD_INLINE inline > +#else > +# define __GTHREAD_INLINE static inline > +#endif > + > typedef pthread_t __gthread_t; > typedef pthread_key_t __gthread_key_t; > typedef pthread_once_t __gthread_once_t; > @@ -182,22 +188,27 @@ __gthrw(pthread_setschedparam) > > #if defined(__FreeBSD__) || (defined(__sun) && defined(__svr4__)) > > -static volatile int __gthread_active = -1; > +__GTHREAD_INLINE volatile int * > +__gthread_active (void) > +{ > + static volatile int __gthread_active_var = -1; > + return &__gthread_active_var; > +} > > -static void > +__GTHREAD_INLINE void > __gthread_trigger (void) > { > - __gthread_active = 1; > + *__gthread_active () = 1; > } > > -static inline int > +__GTHREAD_INLINE int > __gthread_active_p (void) > { > static pthread_mutex_t __gthread_active_mutex = PTHREAD_MUTEX_INITIALIZER; > static pthread_once_t __gthread_active_once = PTHREAD_ONCE_INIT; > > /* Avoid reading __gthread_active twice on the main code path. */ > - int __gthread_active_latest_value = __gthread_active; > + int __gthread_active_latest_value = *__gthread_active (); > > /* This test is not protected to avoid taking a lock on the main code > path so every update of __gthread_active in a threaded program must > @@ -214,10 +225,10 @@ __gthread_active_p (void) > } > > /* Make sure we'll never enter this block again. */ > - if (__gthread_active < 0) > - __gthread_active = 0; > + if (*__gthread_active () < 0) > + *__gthread_active () = 0; > > - __gthread_active_latest_value = __gthread_active; > + __gthread_active_latest_value = *__gthread_active (); > } > > return __gthread_active_latest_value != 0; > @@ -257,7 +268,7 @@ __gthrw2(__gthrw_(__pthread_key_create), > # define GTHR_ACTIVE_PROXY __gthrw_(pthread_cancel) > #endif > > -static inline int > +__GTHREAD_INLINE int > __gthread_active_p (void) > { > static void *const __gthread_active_ptr > @@ -288,20 +299,25 @@ __gthread_active_p (void) > > #if defined(__hppa__) && defined(__hpux__) > > -static volatile int __gthread_active = -1; > +__GTHREAD_INLINE volatile int * > +__gthread_active (void) > +{ > + static volatile int __gthread_active_var = -1; > + return &__gthread_active_var; > +} > > -static inline int > +__GTHREAD_INLINE int > __gthread_active_p (void) > { > /* Avoid reading __gthread_active twice on the main code path. */ > - int __gthread_active_latest_value = __gthread_active; > + int __gthread_active_latest_value = *__gthread_active (); > size_t __s; > > if (__builtin_expect (__gthread_active_latest_value < 0, 0)) > { > pthread_default_stacksize_np (0, &__s); > - __gthread_active = __s ? 1 : 0; > - __gthread_active_latest_value = __gthread_active; > + *__gthread_active () = __s ? 1 : 0; > + __gthread_active_latest_value = *__gthread_active (); > } > > return __gthread_active_latest_value != 0; > @@ -309,7 +325,7 @@ __gthread_active_p (void) > > #else /* not hppa-hpux */ > > -static inline int > +__GTHREAD_INLINE int > __gthread_active_p (void) > { > return 1; > @@ -669,44 +685,44 @@ __gthread_objc_condition_signal (objc_condition_t condition) > > #else /* _LIBOBJC */ > > -static inline int > +__GTHREAD_INLINE int > __gthread_create (__gthread_t *__threadid, void *(*__func) (void*), > void *__args) > { > return __gthrw_(pthread_create) (__threadid, NULL, __func, __args); > } > > -static inline int > +__GTHREAD_INLINE int > __gthread_join (__gthread_t __threadid, void **__value_ptr) > { > return __gthrw_(pthread_join) (__threadid, __value_ptr); > } > > -static inline int > +__GTHREAD_INLINE int > __gthread_detach (__gthread_t __threadid) > { > return __gthrw_(pthread_detach) (__threadid); > } > > -static inline int > +__GTHREAD_INLINE int > __gthread_equal (__gthread_t __t1, __gthread_t __t2) > { > return __gthrw_(pthread_equal) (__t1, __t2); > } > > -static inline __gthread_t > +__GTHREAD_INLINE __gthread_t > __gthread_self (void) > { > return __gthrw_(pthread_self) (); > } > > -static inline int > +__GTHREAD_INLINE int > __gthread_yield (void) > { > return __gthrw_(sched_yield) (); > } > > -static inline int > +__GTHREAD_INLINE int > __gthread_once (__gthread_once_t *__once, void (*__func) (void)) > { > if (__gthread_active_p ()) > @@ -715,38 +731,38 @@ __gthread_once (__gthread_once_t *__once, void (*__func) (void)) > return -1; > } > > -static inline int > +__GTHREAD_INLINE int > __gthread_key_create (__gthread_key_t *__key, void (*__dtor) (void *)) > { > return __gthrw_(pthread_key_create) (__key, __dtor); > } > > -static inline int > +__GTHREAD_INLINE int > __gthread_key_delete (__gthread_key_t __key) > { > return __gthrw_(pthread_key_delete) (__key); > } > > -static inline void * > +__GTHREAD_INLINE void * > __gthread_getspecific (__gthread_key_t __key) > { > return __gthrw_(pthread_getspecific) (__key); > } > > -static inline int > +__GTHREAD_INLINE int > __gthread_setspecific (__gthread_key_t __key, const void *__ptr) > { > return __gthrw_(pthread_setspecific) (__key, __ptr); > } > > -static inline void > +__GTHREAD_INLINE void > __gthread_mutex_init_function (__gthread_mutex_t *__mutex) > { > if (__gthread_active_p ()) > __gthrw_(pthread_mutex_init) (__mutex, NULL); > } > > -static inline int > +__GTHREAD_INLINE int > __gthread_mutex_destroy (__gthread_mutex_t *__mutex) > { > if (__gthread_active_p ()) > @@ -755,7 +771,7 @@ __gthread_mutex_destroy (__gthread_mutex_t *__mutex) > return 0; > } > > -static inline int > +__GTHREAD_INLINE int > __gthread_mutex_lock (__gthread_mutex_t *__mutex) > { > if (__gthread_active_p ()) > @@ -764,7 +780,7 @@ __gthread_mutex_lock (__gthread_mutex_t *__mutex) > return 0; > } > > -static inline int > +__GTHREAD_INLINE int > __gthread_mutex_trylock (__gthread_mutex_t *__mutex) > { > if (__gthread_active_p ()) > @@ -774,7 +790,7 @@ __gthread_mutex_trylock (__gthread_mutex_t *__mutex) > } > > #if _GTHREAD_USE_MUTEX_TIMEDLOCK > -static inline int > +__GTHREAD_INLINE int > __gthread_mutex_timedlock (__gthread_mutex_t *__mutex, > const __gthread_time_t *__abs_timeout) > { > @@ -785,7 +801,7 @@ __gthread_mutex_timedlock (__gthread_mutex_t *__mutex, > } > #endif > > -static inline int > +__GTHREAD_INLINE int > __gthread_mutex_unlock (__gthread_mutex_t *__mutex) > { > if (__gthread_active_p ()) > @@ -796,7 +812,7 @@ __gthread_mutex_unlock (__gthread_mutex_t *__mutex) > > #if !defined( PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP) \ > || defined(_GTHREAD_USE_RECURSIVE_MUTEX_INIT_FUNC) > -static inline int > +__GTHREAD_INLINE int > __gthread_recursive_mutex_init_function (__gthread_recursive_mutex_t *__mutex) > { > if (__gthread_active_p ()) > @@ -818,20 +834,20 @@ __gthread_recursive_mutex_init_function (__gthread_recursive_mutex_t *__mutex) > } > #endif > > -static inline int > +__GTHREAD_INLINE int > __gthread_recursive_mutex_lock (__gthread_recursive_mutex_t *__mutex) > { > return __gthread_mutex_lock (__mutex); > } > > -static inline int > +__GTHREAD_INLINE int > __gthread_recursive_mutex_trylock (__gthread_recursive_mutex_t *__mutex) > { > return __gthread_mutex_trylock (__mutex); > } > > #if _GTHREAD_USE_MUTEX_TIMEDLOCK > -static inline int > +__GTHREAD_INLINE int > __gthread_recursive_mutex_timedlock (__gthread_recursive_mutex_t *__mutex, > const __gthread_time_t *__abs_timeout) > { > @@ -839,20 +855,20 @@ __gthread_recursive_mutex_timedlock (__gthread_recursive_mutex_t *__mutex, > } > #endif > > -static inline int > +__GTHREAD_INLINE int > __gthread_recursive_mutex_unlock (__gthread_recursive_mutex_t *__mutex) > { > return __gthread_mutex_unlock (__mutex); > } > > -static inline int > +__GTHREAD_INLINE int > __gthread_recursive_mutex_destroy (__gthread_recursive_mutex_t *__mutex) > { > return __gthread_mutex_destroy (__mutex); > } > > #ifdef _GTHREAD_USE_COND_INIT_FUNC > -static inline void > +__GTHREAD_INLINE void > __gthread_cond_init_function (__gthread_cond_t *__cond) > { > if (__gthread_active_p ()) > @@ -860,46 +876,46 @@ __gthread_cond_init_function (__gthread_cond_t *__cond) > } > #endif > > -static inline int > +__GTHREAD_INLINE int > __gthread_cond_broadcast (__gthread_cond_t *__cond) > { > return __gthrw_(pthread_cond_broadcast) (__cond); > } > > -static inline int > +__GTHREAD_INLINE int > __gthread_cond_signal (__gthread_cond_t *__cond) > { > return __gthrw_(pthread_cond_signal) (__cond); > } > > -static inline int > +__GTHREAD_INLINE int > __gthread_cond_wait (__gthread_cond_t *__cond, __gthread_mutex_t *__mutex) > { > return __gthrw_(pthread_cond_wait) (__cond, __mutex); > } > > -static inline int > +__GTHREAD_INLINE int > __gthread_cond_timedwait (__gthread_cond_t *__cond, __gthread_mutex_t *__mutex, > const __gthread_time_t *__abs_timeout) > { > return __gthrw_(pthread_cond_timedwait) (__cond, __mutex, __abs_timeout); > } > > -static inline int > +__GTHREAD_INLINE int > __gthread_cond_wait_recursive (__gthread_cond_t *__cond, > __gthread_recursive_mutex_t *__mutex) > { > return __gthread_cond_wait (__cond, __mutex); > } > > -static inline int > +__GTHREAD_INLINE int > __gthread_cond_destroy (__gthread_cond_t* __cond) > { > return __gthrw_(pthread_cond_destroy) (__cond); > } > > #ifndef __cplusplus > -static inline int > +__GTHREAD_INLINE int This is a ! __cplusplus group, so these could stay as static inline. However, I don't understand why r14-6425-gb806c88fab3f9c added those for !__cplusplus instead of just adding them unconditionally and removing the equivalent code in <shared_mutex>. We should fix that, and then I suppose the functions in gthr-posix.h would want to use __GTHREAD_INLINE here. So for consistency and futureproofing, I think it makes sense to change them here. > __gthread_rwlock_rdlock (__gthread_rwlock_t *__rwlock) > { > if (__gthread_active_p ()) > @@ -908,7 +924,7 @@ __gthread_rwlock_rdlock (__gthread_rwlock_t *__rwlock) > return 0; > } > > -static inline int > +__GTHREAD_INLINE int > __gthread_rwlock_tryrdlock (__gthread_rwlock_t *__rwlock) > { > if (__gthread_active_p ()) > @@ -917,7 +933,7 @@ __gthread_rwlock_tryrdlock (__gthread_rwlock_t *__rwlock) > return 0; > } > > -static inline int > +__GTHREAD_INLINE int > __gthread_rwlock_wrlock (__gthread_rwlock_t *__rwlock) > { > if (__gthread_active_p ()) > @@ -926,7 +942,7 @@ __gthread_rwlock_wrlock (__gthread_rwlock_t *__rwlock) > return 0; > } > > -static inline int > +__GTHREAD_INLINE int > __gthread_rwlock_trywrlock (__gthread_rwlock_t *__rwlock) > { > if (__gthread_active_p ()) > @@ -935,7 +951,7 @@ __gthread_rwlock_trywrlock (__gthread_rwlock_t *__rwlock) > return 0; > } > > -static inline int > +__GTHREAD_INLINE int > __gthread_rwlock_unlock (__gthread_rwlock_t *__rwlock) > { > if (__gthread_active_p ()) > diff --git a/libgcc/gthr-single.h b/libgcc/gthr-single.h > index 8ee6b170840..0473daaf89d 100644 > --- a/libgcc/gthr-single.h > +++ b/libgcc/gthr-single.h > @@ -38,6 +38,12 @@ typedef int __gthread_recursive_mutex_t; > #define __GTHREAD_MUTEX_INIT_FUNCTION(mx) do {} while (0) > #define __GTHREAD_RECURSIVE_MUTEX_INIT 0 > > +#ifdef __cplusplus > +# define __GTHREAD_INLINE inline > +#else > +# define __GTHREAD_INLINE static inline > +#endif > + > #define UNUSED __attribute__((__unused__)) > > #ifdef _LIBOBJC > @@ -207,85 +213,85 @@ __gthread_objc_condition_signal (objc_condition_t condition UNUSED) > > #else /* _LIBOBJC */ > > -static inline int > +__GTHREAD_INLINE int > __gthread_active_p (void) > { > return 0; > } > > -static inline int > +__GTHREAD_INLINE int > __gthread_once (__gthread_once_t *__once UNUSED, void (*__func) (void) UNUSED) > { > return 0; > } > > -static inline int UNUSED > +__GTHREAD_INLINE int UNUSED > __gthread_key_create (__gthread_key_t *__key UNUSED, void (*__func) (void *) UNUSED) > { > return 0; > } > > -static int UNUSED > +__GTHREAD_INLINE int UNUSED > __gthread_key_delete (__gthread_key_t __key UNUSED) > { > return 0; > } > > -static inline void * > +__GTHREAD_INLINE void * > __gthread_getspecific (__gthread_key_t __key UNUSED) > { > return 0; > } > > -static inline int > +__GTHREAD_INLINE int > __gthread_setspecific (__gthread_key_t __key UNUSED, const void *__v UNUSED) > { > return 0; > } > > -static inline int > +__GTHREAD_INLINE int > __gthread_mutex_destroy (__gthread_mutex_t *__mutex UNUSED) > { > return 0; > } > > -static inline int > +__GTHREAD_INLINE int > __gthread_mutex_lock (__gthread_mutex_t *__mutex UNUSED) > { > return 0; > } > > -static inline int > +__GTHREAD_INLINE int > __gthread_mutex_trylock (__gthread_mutex_t *__mutex UNUSED) > { > return 0; > } > > -static inline int > +__GTHREAD_INLINE int > __gthread_mutex_unlock (__gthread_mutex_t *__mutex UNUSED) > { > return 0; > } > > -static inline int > +__GTHREAD_INLINE int > __gthread_recursive_mutex_lock (__gthread_recursive_mutex_t *__mutex) > { > return __gthread_mutex_lock (__mutex); > } > > -static inline int > +__GTHREAD_INLINE int > __gthread_recursive_mutex_trylock (__gthread_recursive_mutex_t *__mutex) > { > return __gthread_mutex_trylock (__mutex); > } > > -static inline int > +__GTHREAD_INLINE int > __gthread_recursive_mutex_unlock (__gthread_recursive_mutex_t *__mutex) > { > return __gthread_mutex_unlock (__mutex); > } > > -static inline int > +__GTHREAD_INLINE int > __gthread_recursive_mutex_destroy (__gthread_recursive_mutex_t *__mutex) > { > return __gthread_mutex_destroy (__mutex); > diff --git a/libstdc++-v3/include/bits/shared_ptr.h b/libstdc++-v3/include/bits/shared_ptr.h > index 13273afa9d2..9b1c2b4eb3d 100644 > --- a/libstdc++-v3/include/bits/shared_ptr.h > +++ b/libstdc++-v3/include/bits/shared_ptr.h > @@ -1160,9 +1160,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > #if __cpp_variable_templates > template<typename _Tp> > - static constexpr bool __is_shared_ptr = false; > + constexpr bool __is_shared_ptr = false; > template<typename _Tp> > - static constexpr bool __is_shared_ptr<shared_ptr<_Tp>> = true; > + constexpr bool __is_shared_ptr<shared_ptr<_Tp>> = true; > #endif > > /// @} relates shared_ptr > diff --git a/libstdc++-v3/include/bits/unique_ptr.h b/libstdc++-v3/include/bits/unique_ptr.h > index edcff78bff9..182173aa857 100644 > --- a/libstdc++-v3/include/bits/unique_ptr.h > +++ b/libstdc++-v3/include/bits/unique_ptr.h > @@ -1157,9 +1157,9 @@ namespace __detail > > #if __cpp_variable_templates > template<typename _Tp> > - static constexpr bool __is_unique_ptr = false; > + constexpr bool __is_unique_ptr = false; > template<typename _Tp, typename _Del> > - static constexpr bool __is_unique_ptr<unique_ptr<_Tp, _Del>> = true; > + constexpr bool __is_unique_ptr<unique_ptr<_Tp, _Del>> = true; > #endif > > /// @} group pointer_abstractions > diff --git a/libstdc++-v3/include/std/future b/libstdc++-v3/include/std/future > index 6ce7d89ca3f..437453eaa44 100644 > --- a/libstdc++-v3/include/std/future > +++ b/libstdc++-v3/include/std/future > @@ -1526,7 +1526,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > template<typename _Signature, typename _Fn, > typename _Alloc = std::allocator<int>> > - static shared_ptr<__future_base::_Task_state_base<_Signature>> > + shared_ptr<__future_base::_Task_state_base<_Signature>> > __create_task_state(_Fn&& __fn, const _Alloc& __a = _Alloc()) > { > typedef typename decay<_Fn>::type _Fn2; > diff --git a/libstdc++-v3/include/std/shared_mutex b/libstdc++-v3/include/std/shared_mutex > index 1b6478f30c3..7e7941dd7a1 100644 > --- a/libstdc++-v3/include/std/shared_mutex > +++ b/libstdc++-v3/include/std/shared_mutex > @@ -72,7 +72,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > #ifdef __gthrw > #define _GLIBCXX_GTHRW(name) \ > __gthrw(pthread_ ## name); \ > - static inline int \ > + inline int \ > __glibcxx_ ## name (pthread_rwlock_t *__rwlock) \ > { \ > if (__gthread_active_p ()) \ > @@ -88,7 +88,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > # ifndef PTHREAD_RWLOCK_INITIALIZER > _GLIBCXX_GTHRW(rwlock_destroy) > __gthrw(pthread_rwlock_init); > - static inline int > + inline int > __glibcxx_rwlock_init (pthread_rwlock_t *__rwlock) > { > if (__gthread_active_p ()) > @@ -99,7 +99,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > # endif > # if _GTHREAD_USE_MUTEX_TIMEDLOCK > __gthrw(pthread_rwlock_timedrdlock); > - static inline int > + inline int > __glibcxx_rwlock_timedrdlock (pthread_rwlock_t *__rwlock, > const timespec *__ts) > { > @@ -109,7 +109,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > return 0; > } > __gthrw(pthread_rwlock_timedwrlock); > - static inline int > + inline int > __glibcxx_rwlock_timedwrlock (pthread_rwlock_t *__rwlock, > const timespec *__ts) > { > @@ -120,33 +120,33 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > } > # endif > #else > - static inline int > + inline int > __glibcxx_rwlock_rdlock (pthread_rwlock_t *__rwlock) > { return pthread_rwlock_rdlock (__rwlock); } > - static inline int > + inline int > __glibcxx_rwlock_tryrdlock (pthread_rwlock_t *__rwlock) > { return pthread_rwlock_tryrdlock (__rwlock); } > - static inline int > + inline int > __glibcxx_rwlock_wrlock (pthread_rwlock_t *__rwlock) > { return pthread_rwlock_wrlock (__rwlock); } > - static inline int > + inline int > __glibcxx_rwlock_trywrlock (pthread_rwlock_t *__rwlock) > { return pthread_rwlock_trywrlock (__rwlock); } > - static inline int > + inline int > __glibcxx_rwlock_unlock (pthread_rwlock_t *__rwlock) > { return pthread_rwlock_unlock (__rwlock); } > - static inline int > + inline int > __glibcxx_rwlock_destroy(pthread_rwlock_t *__rwlock) > { return pthread_rwlock_destroy (__rwlock); } > - static inline int > + inline int > __glibcxx_rwlock_init(pthread_rwlock_t *__rwlock) > { return pthread_rwlock_init (__rwlock, NULL); } > # if _GTHREAD_USE_MUTEX_TIMEDLOCK > - static inline int > + inline int > __glibcxx_rwlock_timedrdlock (pthread_rwlock_t *__rwlock, > const timespec *__ts) > { return pthread_rwlock_timedrdlock (__rwlock, __ts); } > - static inline int > + inline int > __glibcxx_rwlock_timedwrlock (pthread_rwlock_t *__rwlock, > const timespec *__ts) > { return pthread_rwlock_timedwrlock (__rwlock, __ts); } > -- > 2.46.0 >
On Wed, Sep 25, 2024 at 10:43:50AM +0100, Jonathan Wakely wrote: > > libgcc/ChangeLog: > > > > * gthr-posix.h (__GTHREAD_INLINE): New macro. > > (__gthread_active): Convert from variable to function. > > (__gthread_trigger): Mark as __GTHREAD_INLINE instead of static. > > (__gthread_active_p): Likewise. > > (__gthread_create): Likewise. > > (__gthread_join): Likewise. > > (__gthread_detach): Likewise. > > (__gthread_equal): Likewise. > > (__gthread_self): Likewise. > > (__gthread_yield): Likewise. > > (__gthread_once): Likewise. > > (__gthread_key_create): Likewise. > > (__gthread_key_delete): Likewise. > > (__gthread_getspecific): Likewise. > > (__gthread_setspecific): Likewise. > > (__gthread_mutex_init_function): Likewise. > > (__gthread_mutex_destroy): Likewise. > > (__gthread_mutex_lock): Likewise. > > (__gthread_mutex_trylock): Likewise. > > (__gthread_mutex_timedlock): Likewise. > > (__gthread_mutex_unlock): Likewise. > > (__gthread_recursive_mutex_init_function): Likewise. > > (__gthread_recursive_mutex_lock): Likewise. > > (__gthread_recursive_mutex_trylock): Likewise. > > (__gthread_recursive_mutex_timedlock): Likewise. > > (__gthread_recursive_mutex_unlock): Likewise. > > (__gthread_recursive_mutex_destroy): Likewise. > > (__gthread_cond_init_function): Likewise. > > (__gthread_cond_broadcast): Likewise. > > (__gthread_cond_signal): Likewise. > > (__gthread_cond_wait): Likewise. > > (__gthread_cond_timedwait): Likewise. > > (__gthread_cond_wait_recursive): Likewise. > > (__gthread_cond_destroy): Likewise. > > (__gthread_rwlock_rdlock): Likewise. > > (__gthread_rwlock_tryrdlock): Likewise. > > (__gthread_rwlock_wrlock): Likewise. > > (__gthread_rwlock_trywrlock): Likewise. > > (__gthread_rwlock_unlock): Likewise. > > * gthr-single.h (__GTHREAD_INLINE): New macro. > > (__gthread_active_p): Mark as __GTHREAD_INLINE instead of static. > > (__gthread_once): Likewise. > > (__gthread_key_create): Likewise. > > (__gthread_key_delete): Likewise. > > (__gthread_getspecific): Likewise. > > (__gthread_setspecific): Likewise. > > (__gthread_mutex_destroy): Likewise. > > (__gthread_mutex_lock): Likewise. > > (__gthread_mutex_trylock): Likewise. > > (__gthread_mutex_unlock): Likewise. > > (__gthread_recursive_mutex_lock): Likewise. > > (__gthread_recursive_mutex_trylock): Likewise. > > (__gthread_recursive_mutex_unlock): Likewise. > > (__gthread_recursive_mutex_destroy): Likewise. I'm worried about ABI consequences of these changes. From what I can see, this doesn't change anything important for C, the inlines are still static inline and the conversion of global static vars to function-local in static inline still keeps those static. But for C++ this means significant changes. Say _ZZ16__gthread_activevE20__gthread_active_var etc. will now be STB_GNU_UNIQUE symbol, exported from whatever shared libraries and binaries are compiled with C++ and include those headers and actually use them somewhere. On one side, it has benefits (e.g. every TU won't separately test and store whether the threads are active or not), on the other side it can prevent dlclose of some shared libraries and create interdependencies between libraries. So, I wonder if we couldn't define the C++ __GTHREAD_INLINE to static inline __attribute__((__always_inline__)) (or at least when using a compiler which supports the attribute, we can now test it using preprocessor...). And whether similarly we couldn't use __attribute__((__visibility__ ("hidden"))) on the static block scope vars for C++ (again, if compiler supports that), so that the changes don't affect ABI of C++ libraries. Jakub
On Wed, 25 Sept 2024 at 11:29, Jakub Jelinek <jakub@redhat.com> wrote: > > On Wed, Sep 25, 2024 at 10:43:50AM +0100, Jonathan Wakely wrote: > > > libgcc/ChangeLog: > > > > > > * gthr-posix.h (__GTHREAD_INLINE): New macro. > > > (__gthread_active): Convert from variable to function. > > > (__gthread_trigger): Mark as __GTHREAD_INLINE instead of static. > > > (__gthread_active_p): Likewise. > > > (__gthread_create): Likewise. > > > (__gthread_join): Likewise. > > > (__gthread_detach): Likewise. > > > (__gthread_equal): Likewise. > > > (__gthread_self): Likewise. > > > (__gthread_yield): Likewise. > > > (__gthread_once): Likewise. > > > (__gthread_key_create): Likewise. > > > (__gthread_key_delete): Likewise. > > > (__gthread_getspecific): Likewise. > > > (__gthread_setspecific): Likewise. > > > (__gthread_mutex_init_function): Likewise. > > > (__gthread_mutex_destroy): Likewise. > > > (__gthread_mutex_lock): Likewise. > > > (__gthread_mutex_trylock): Likewise. > > > (__gthread_mutex_timedlock): Likewise. > > > (__gthread_mutex_unlock): Likewise. > > > (__gthread_recursive_mutex_init_function): Likewise. > > > (__gthread_recursive_mutex_lock): Likewise. > > > (__gthread_recursive_mutex_trylock): Likewise. > > > (__gthread_recursive_mutex_timedlock): Likewise. > > > (__gthread_recursive_mutex_unlock): Likewise. > > > (__gthread_recursive_mutex_destroy): Likewise. > > > (__gthread_cond_init_function): Likewise. > > > (__gthread_cond_broadcast): Likewise. > > > (__gthread_cond_signal): Likewise. > > > (__gthread_cond_wait): Likewise. > > > (__gthread_cond_timedwait): Likewise. > > > (__gthread_cond_wait_recursive): Likewise. > > > (__gthread_cond_destroy): Likewise. > > > (__gthread_rwlock_rdlock): Likewise. > > > (__gthread_rwlock_tryrdlock): Likewise. > > > (__gthread_rwlock_wrlock): Likewise. > > > (__gthread_rwlock_trywrlock): Likewise. > > > (__gthread_rwlock_unlock): Likewise. > > > * gthr-single.h (__GTHREAD_INLINE): New macro. > > > (__gthread_active_p): Mark as __GTHREAD_INLINE instead of static. > > > (__gthread_once): Likewise. > > > (__gthread_key_create): Likewise. > > > (__gthread_key_delete): Likewise. > > > (__gthread_getspecific): Likewise. > > > (__gthread_setspecific): Likewise. > > > (__gthread_mutex_destroy): Likewise. > > > (__gthread_mutex_lock): Likewise. > > > (__gthread_mutex_trylock): Likewise. > > > (__gthread_mutex_unlock): Likewise. > > > (__gthread_recursive_mutex_lock): Likewise. > > > (__gthread_recursive_mutex_trylock): Likewise. > > > (__gthread_recursive_mutex_unlock): Likewise. > > > (__gthread_recursive_mutex_destroy): Likewise. > > I'm worried about ABI consequences of these changes. > From what I can see, this doesn't change anything important for C, > the inlines are still static inline and the conversion of global static > vars to function-local in static inline still keeps those static. > > But for C++ this means significant changes. > Say > _ZZ16__gthread_activevE20__gthread_active_var > etc. will now be STB_GNU_UNIQUE symbol, exported from whatever shared > libraries and binaries are compiled with C++ and include those headers > and actually use them somewhere. I don't think that applies for HPUX or Solaris, but maybe does for FreeBSD (and it looks like those are the only targets affected by the __gthread_active_var changes). > On one side, it has benefits (e.g. every TU won't separately test and > store whether the threads are active or not), on the other side it can > prevent dlclose of some shared libraries and create interdependencies > between libraries. > > So, I wonder if we couldn't define the C++ __GTHREAD_INLINE to > static inline __attribute__((__always_inline__)) (or at least when > using a compiler which supports the attribute, we can now test it > using preprocessor...). I think that would be a nice improvement for the gthread wrapper functions even without the concern about __gthread_active_var. > And whether similarly we couldn't use > __attribute__((__visibility__ ("hidden"))) on the static block scope > vars for C++ (again, if compiler supports that), so that the changes > don't affect ABI of C++ libraries. That sounds good too.
On Wed, 25 Sept 2024 at 11:47, Jonathan Wakely <jwakely@redhat.com> wrote: > > On Wed, 25 Sept 2024 at 11:29, Jakub Jelinek <jakub@redhat.com> wrote: > > > > On Wed, Sep 25, 2024 at 10:43:50AM +0100, Jonathan Wakely wrote: > > > > libgcc/ChangeLog: > > > > > > > > * gthr-posix.h (__GTHREAD_INLINE): New macro. > > > > (__gthread_active): Convert from variable to function. > > > > (__gthread_trigger): Mark as __GTHREAD_INLINE instead of static. > > > > (__gthread_active_p): Likewise. > > > > (__gthread_create): Likewise. > > > > (__gthread_join): Likewise. > > > > (__gthread_detach): Likewise. > > > > (__gthread_equal): Likewise. > > > > (__gthread_self): Likewise. > > > > (__gthread_yield): Likewise. > > > > (__gthread_once): Likewise. > > > > (__gthread_key_create): Likewise. > > > > (__gthread_key_delete): Likewise. > > > > (__gthread_getspecific): Likewise. > > > > (__gthread_setspecific): Likewise. > > > > (__gthread_mutex_init_function): Likewise. > > > > (__gthread_mutex_destroy): Likewise. > > > > (__gthread_mutex_lock): Likewise. > > > > (__gthread_mutex_trylock): Likewise. > > > > (__gthread_mutex_timedlock): Likewise. > > > > (__gthread_mutex_unlock): Likewise. > > > > (__gthread_recursive_mutex_init_function): Likewise. > > > > (__gthread_recursive_mutex_lock): Likewise. > > > > (__gthread_recursive_mutex_trylock): Likewise. > > > > (__gthread_recursive_mutex_timedlock): Likewise. > > > > (__gthread_recursive_mutex_unlock): Likewise. > > > > (__gthread_recursive_mutex_destroy): Likewise. > > > > (__gthread_cond_init_function): Likewise. > > > > (__gthread_cond_broadcast): Likewise. > > > > (__gthread_cond_signal): Likewise. > > > > (__gthread_cond_wait): Likewise. > > > > (__gthread_cond_timedwait): Likewise. > > > > (__gthread_cond_wait_recursive): Likewise. > > > > (__gthread_cond_destroy): Likewise. > > > > (__gthread_rwlock_rdlock): Likewise. > > > > (__gthread_rwlock_tryrdlock): Likewise. > > > > (__gthread_rwlock_wrlock): Likewise. > > > > (__gthread_rwlock_trywrlock): Likewise. > > > > (__gthread_rwlock_unlock): Likewise. > > > > * gthr-single.h (__GTHREAD_INLINE): New macro. > > > > (__gthread_active_p): Mark as __GTHREAD_INLINE instead of static. > > > > (__gthread_once): Likewise. > > > > (__gthread_key_create): Likewise. > > > > (__gthread_key_delete): Likewise. > > > > (__gthread_getspecific): Likewise. > > > > (__gthread_setspecific): Likewise. > > > > (__gthread_mutex_destroy): Likewise. > > > > (__gthread_mutex_lock): Likewise. > > > > (__gthread_mutex_trylock): Likewise. > > > > (__gthread_mutex_unlock): Likewise. > > > > (__gthread_recursive_mutex_lock): Likewise. > > > > (__gthread_recursive_mutex_trylock): Likewise. > > > > (__gthread_recursive_mutex_unlock): Likewise. > > > > (__gthread_recursive_mutex_destroy): Likewise. > > > > I'm worried about ABI consequences of these changes. > > From what I can see, this doesn't change anything important for C, > > the inlines are still static inline and the conversion of global static > > vars to function-local in static inline still keeps those static. > > > > But for C++ this means significant changes. > > Say > > _ZZ16__gthread_activevE20__gthread_active_var > > etc. will now be STB_GNU_UNIQUE symbol, exported from whatever shared > > libraries and binaries are compiled with C++ and include those headers > > and actually use them somewhere. > > I don't think that applies for HPUX or Solaris, but maybe does for > FreeBSD (and it looks like those are the only targets affected by the > __gthread_active_var changes). > > > On one side, it has benefits (e.g. every TU won't separately test and > > store whether the threads are active or not), on the other side it can > > prevent dlclose of some shared libraries and create interdependencies > > between libraries. > > > > So, I wonder if we couldn't define the C++ __GTHREAD_INLINE to > > static inline __attribute__((__always_inline__)) (or at least when > > using a compiler which supports the attribute, we can now test it > > using preprocessor...). > > I think that would be a nice improvement for the gthread wrapper > functions even without the concern about __gthread_active_var. > > > And whether similarly we couldn't use > > __attribute__((__visibility__ ("hidden"))) on the static block scope > > vars for C++ (again, if compiler supports that), so that the changes > > don't affect ABI of C++ libraries. > > That sounds good too. Can you use visibility attributes on a local static? I get a warning that it's ignored.
On Wed, Sep 25, 2024 at 12:18:07PM +0100, Jonathan Wakely wrote: > > > And whether similarly we couldn't use > > > __attribute__((__visibility__ ("hidden"))) on the static block scope > > > vars for C++ (again, if compiler supports that), so that the changes > > > don't affect ABI of C++ libraries. > > > > That sounds good too. > > Can you use visibility attributes on a local static? I get a warning > that it's ignored. Indeed :( And #pragma GCC visibility push(hidden)/#pragma GCC visibility pop around just the static block scope var definition does nothing. If it is around the whole inline function though, then it seems to work. Though, unsure if we want that around the whole header; wonder what it would do with the weakrefs. Jakub
On Wed, Sep 25, 2024 at 01:30:55PM +0200, Jakub Jelinek wrote: > On Wed, Sep 25, 2024 at 12:18:07PM +0100, Jonathan Wakely wrote: > > > > And whether similarly we couldn't use > > > > __attribute__((__visibility__ ("hidden"))) on the static block scope > > > > vars for C++ (again, if compiler supports that), so that the changes > > > > don't affect ABI of C++ libraries. > > > > > > That sounds good too. > > > > Can you use visibility attributes on a local static? I get a warning > > that it's ignored. > > Indeed :( > > And #pragma GCC visibility push(hidden)/#pragma GCC visibility pop around > just the static block scope var definition does nothing. > If it is around the whole inline function though, then it seems to work. > Though, unsure if we want that around the whole header; wonder what it would > do with the weakrefs. > > Jakub > Thanks for the thoughts. WRT visibility, it looks like the main gthr.h surrounds the whole function in a #ifndef HIDE_EXPORTS #pragma GCC visibility push(default) #endif block, though I can't quite work out what the purpose of that is here (since everything is currently internal linkage to start with). But it sounds like doing something like #ifdef __has_attribute # if __has_attribute(__always_inline__) # define __GTHREAD_ALWAYS_INLINE __attribute__((__always_inline__)) # endif #endif #ifndef __GTHREAD_ALWAYS_INLINE # define __GTHREAD_ALWAYS_INLINE #endif #ifdef __cplusplus # define __GTHREAD_INLINE inline __GTHREAD_ALWAYS_INLINE #else # define __GTHREAD_INLINE static inline #endif and then marking maybe even just the new inline functions with visibility hidden should be OK? Nathaniel
diff --git a/libgcc/gthr-posix.h b/libgcc/gthr-posix.h index 82e8f9ffcf6..557bae7aaaa 100644 --- a/libgcc/gthr-posix.h +++ b/libgcc/gthr-posix.h @@ -44,6 +44,12 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see # endif #endif +#ifdef __cplusplus +# define __GTHREAD_INLINE inline +#else +# define __GTHREAD_INLINE static inline +#endif + typedef pthread_t __gthread_t; typedef pthread_key_t __gthread_key_t; typedef pthread_once_t __gthread_once_t; @@ -182,22 +188,27 @@ __gthrw(pthread_setschedparam) #if defined(__FreeBSD__) || (defined(__sun) && defined(__svr4__)) -static volatile int __gthread_active = -1; +__GTHREAD_INLINE volatile int * +__gthread_active (void) +{ + static volatile int __gthread_active_var = -1; + return &__gthread_active_var; +} -static void +__GTHREAD_INLINE void __gthread_trigger (void) { - __gthread_active = 1; + *__gthread_active () = 1; } -static inline int +__GTHREAD_INLINE int __gthread_active_p (void) { static pthread_mutex_t __gthread_active_mutex = PTHREAD_MUTEX_INITIALIZER; static pthread_once_t __gthread_active_once = PTHREAD_ONCE_INIT; /* Avoid reading __gthread_active twice on the main code path. */ - int __gthread_active_latest_value = __gthread_active; + int __gthread_active_latest_value = *__gthread_active (); /* This test is not protected to avoid taking a lock on the main code path so every update of __gthread_active in a threaded program must @@ -214,10 +225,10 @@ __gthread_active_p (void) } /* Make sure we'll never enter this block again. */ - if (__gthread_active < 0) - __gthread_active = 0; + if (*__gthread_active () < 0) + *__gthread_active () = 0; - __gthread_active_latest_value = __gthread_active; + __gthread_active_latest_value = *__gthread_active (); } return __gthread_active_latest_value != 0; @@ -257,7 +268,7 @@ __gthrw2(__gthrw_(__pthread_key_create), # define GTHR_ACTIVE_PROXY __gthrw_(pthread_cancel) #endif -static inline int +__GTHREAD_INLINE int __gthread_active_p (void) { static void *const __gthread_active_ptr @@ -288,20 +299,25 @@ __gthread_active_p (void) #if defined(__hppa__) && defined(__hpux__) -static volatile int __gthread_active = -1; +__GTHREAD_INLINE volatile int * +__gthread_active (void) +{ + static volatile int __gthread_active_var = -1; + return &__gthread_active_var; +} -static inline int +__GTHREAD_INLINE int __gthread_active_p (void) { /* Avoid reading __gthread_active twice on the main code path. */ - int __gthread_active_latest_value = __gthread_active; + int __gthread_active_latest_value = *__gthread_active (); size_t __s; if (__builtin_expect (__gthread_active_latest_value < 0, 0)) { pthread_default_stacksize_np (0, &__s); - __gthread_active = __s ? 1 : 0; - __gthread_active_latest_value = __gthread_active; + *__gthread_active () = __s ? 1 : 0; + __gthread_active_latest_value = *__gthread_active (); } return __gthread_active_latest_value != 0; @@ -309,7 +325,7 @@ __gthread_active_p (void) #else /* not hppa-hpux */ -static inline int +__GTHREAD_INLINE int __gthread_active_p (void) { return 1; @@ -669,44 +685,44 @@ __gthread_objc_condition_signal (objc_condition_t condition) #else /* _LIBOBJC */ -static inline int +__GTHREAD_INLINE int __gthread_create (__gthread_t *__threadid, void *(*__func) (void*), void *__args) { return __gthrw_(pthread_create) (__threadid, NULL, __func, __args); } -static inline int +__GTHREAD_INLINE int __gthread_join (__gthread_t __threadid, void **__value_ptr) { return __gthrw_(pthread_join) (__threadid, __value_ptr); } -static inline int +__GTHREAD_INLINE int __gthread_detach (__gthread_t __threadid) { return __gthrw_(pthread_detach) (__threadid); } -static inline int +__GTHREAD_INLINE int __gthread_equal (__gthread_t __t1, __gthread_t __t2) { return __gthrw_(pthread_equal) (__t1, __t2); } -static inline __gthread_t +__GTHREAD_INLINE __gthread_t __gthread_self (void) { return __gthrw_(pthread_self) (); } -static inline int +__GTHREAD_INLINE int __gthread_yield (void) { return __gthrw_(sched_yield) (); } -static inline int +__GTHREAD_INLINE int __gthread_once (__gthread_once_t *__once, void (*__func) (void)) { if (__gthread_active_p ()) @@ -715,38 +731,38 @@ __gthread_once (__gthread_once_t *__once, void (*__func) (void)) return -1; } -static inline int +__GTHREAD_INLINE int __gthread_key_create (__gthread_key_t *__key, void (*__dtor) (void *)) { return __gthrw_(pthread_key_create) (__key, __dtor); } -static inline int +__GTHREAD_INLINE int __gthread_key_delete (__gthread_key_t __key) { return __gthrw_(pthread_key_delete) (__key); } -static inline void * +__GTHREAD_INLINE void * __gthread_getspecific (__gthread_key_t __key) { return __gthrw_(pthread_getspecific) (__key); } -static inline int +__GTHREAD_INLINE int __gthread_setspecific (__gthread_key_t __key, const void *__ptr) { return __gthrw_(pthread_setspecific) (__key, __ptr); } -static inline void +__GTHREAD_INLINE void __gthread_mutex_init_function (__gthread_mutex_t *__mutex) { if (__gthread_active_p ()) __gthrw_(pthread_mutex_init) (__mutex, NULL); } -static inline int +__GTHREAD_INLINE int __gthread_mutex_destroy (__gthread_mutex_t *__mutex) { if (__gthread_active_p ()) @@ -755,7 +771,7 @@ __gthread_mutex_destroy (__gthread_mutex_t *__mutex) return 0; } -static inline int +__GTHREAD_INLINE int __gthread_mutex_lock (__gthread_mutex_t *__mutex) { if (__gthread_active_p ()) @@ -764,7 +780,7 @@ __gthread_mutex_lock (__gthread_mutex_t *__mutex) return 0; } -static inline int +__GTHREAD_INLINE int __gthread_mutex_trylock (__gthread_mutex_t *__mutex) { if (__gthread_active_p ()) @@ -774,7 +790,7 @@ __gthread_mutex_trylock (__gthread_mutex_t *__mutex) } #if _GTHREAD_USE_MUTEX_TIMEDLOCK -static inline int +__GTHREAD_INLINE int __gthread_mutex_timedlock (__gthread_mutex_t *__mutex, const __gthread_time_t *__abs_timeout) { @@ -785,7 +801,7 @@ __gthread_mutex_timedlock (__gthread_mutex_t *__mutex, } #endif -static inline int +__GTHREAD_INLINE int __gthread_mutex_unlock (__gthread_mutex_t *__mutex) { if (__gthread_active_p ()) @@ -796,7 +812,7 @@ __gthread_mutex_unlock (__gthread_mutex_t *__mutex) #if !defined( PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP) \ || defined(_GTHREAD_USE_RECURSIVE_MUTEX_INIT_FUNC) -static inline int +__GTHREAD_INLINE int __gthread_recursive_mutex_init_function (__gthread_recursive_mutex_t *__mutex) { if (__gthread_active_p ()) @@ -818,20 +834,20 @@ __gthread_recursive_mutex_init_function (__gthread_recursive_mutex_t *__mutex) } #endif -static inline int +__GTHREAD_INLINE int __gthread_recursive_mutex_lock (__gthread_recursive_mutex_t *__mutex) { return __gthread_mutex_lock (__mutex); } -static inline int +__GTHREAD_INLINE int __gthread_recursive_mutex_trylock (__gthread_recursive_mutex_t *__mutex) { return __gthread_mutex_trylock (__mutex); } #if _GTHREAD_USE_MUTEX_TIMEDLOCK -static inline int +__GTHREAD_INLINE int __gthread_recursive_mutex_timedlock (__gthread_recursive_mutex_t *__mutex, const __gthread_time_t *__abs_timeout) { @@ -839,20 +855,20 @@ __gthread_recursive_mutex_timedlock (__gthread_recursive_mutex_t *__mutex, } #endif -static inline int +__GTHREAD_INLINE int __gthread_recursive_mutex_unlock (__gthread_recursive_mutex_t *__mutex) { return __gthread_mutex_unlock (__mutex); } -static inline int +__GTHREAD_INLINE int __gthread_recursive_mutex_destroy (__gthread_recursive_mutex_t *__mutex) { return __gthread_mutex_destroy (__mutex); } #ifdef _GTHREAD_USE_COND_INIT_FUNC -static inline void +__GTHREAD_INLINE void __gthread_cond_init_function (__gthread_cond_t *__cond) { if (__gthread_active_p ()) @@ -860,46 +876,46 @@ __gthread_cond_init_function (__gthread_cond_t *__cond) } #endif -static inline int +__GTHREAD_INLINE int __gthread_cond_broadcast (__gthread_cond_t *__cond) { return __gthrw_(pthread_cond_broadcast) (__cond); } -static inline int +__GTHREAD_INLINE int __gthread_cond_signal (__gthread_cond_t *__cond) { return __gthrw_(pthread_cond_signal) (__cond); } -static inline int +__GTHREAD_INLINE int __gthread_cond_wait (__gthread_cond_t *__cond, __gthread_mutex_t *__mutex) { return __gthrw_(pthread_cond_wait) (__cond, __mutex); } -static inline int +__GTHREAD_INLINE int __gthread_cond_timedwait (__gthread_cond_t *__cond, __gthread_mutex_t *__mutex, const __gthread_time_t *__abs_timeout) { return __gthrw_(pthread_cond_timedwait) (__cond, __mutex, __abs_timeout); } -static inline int +__GTHREAD_INLINE int __gthread_cond_wait_recursive (__gthread_cond_t *__cond, __gthread_recursive_mutex_t *__mutex) { return __gthread_cond_wait (__cond, __mutex); } -static inline int +__GTHREAD_INLINE int __gthread_cond_destroy (__gthread_cond_t* __cond) { return __gthrw_(pthread_cond_destroy) (__cond); } #ifndef __cplusplus -static inline int +__GTHREAD_INLINE int __gthread_rwlock_rdlock (__gthread_rwlock_t *__rwlock) { if (__gthread_active_p ()) @@ -908,7 +924,7 @@ __gthread_rwlock_rdlock (__gthread_rwlock_t *__rwlock) return 0; } -static inline int +__GTHREAD_INLINE int __gthread_rwlock_tryrdlock (__gthread_rwlock_t *__rwlock) { if (__gthread_active_p ()) @@ -917,7 +933,7 @@ __gthread_rwlock_tryrdlock (__gthread_rwlock_t *__rwlock) return 0; } -static inline int +__GTHREAD_INLINE int __gthread_rwlock_wrlock (__gthread_rwlock_t *__rwlock) { if (__gthread_active_p ()) @@ -926,7 +942,7 @@ __gthread_rwlock_wrlock (__gthread_rwlock_t *__rwlock) return 0; } -static inline int +__GTHREAD_INLINE int __gthread_rwlock_trywrlock (__gthread_rwlock_t *__rwlock) { if (__gthread_active_p ()) @@ -935,7 +951,7 @@ __gthread_rwlock_trywrlock (__gthread_rwlock_t *__rwlock) return 0; } -static inline int +__GTHREAD_INLINE int __gthread_rwlock_unlock (__gthread_rwlock_t *__rwlock) { if (__gthread_active_p ()) diff --git a/libgcc/gthr-single.h b/libgcc/gthr-single.h index 8ee6b170840..0473daaf89d 100644 --- a/libgcc/gthr-single.h +++ b/libgcc/gthr-single.h @@ -38,6 +38,12 @@ typedef int __gthread_recursive_mutex_t; #define __GTHREAD_MUTEX_INIT_FUNCTION(mx) do {} while (0) #define __GTHREAD_RECURSIVE_MUTEX_INIT 0 +#ifdef __cplusplus +# define __GTHREAD_INLINE inline +#else +# define __GTHREAD_INLINE static inline +#endif + #define UNUSED __attribute__((__unused__)) #ifdef _LIBOBJC @@ -207,85 +213,85 @@ __gthread_objc_condition_signal (objc_condition_t condition UNUSED) #else /* _LIBOBJC */ -static inline int +__GTHREAD_INLINE int __gthread_active_p (void) { return 0; } -static inline int +__GTHREAD_INLINE int __gthread_once (__gthread_once_t *__once UNUSED, void (*__func) (void) UNUSED) { return 0; } -static inline int UNUSED +__GTHREAD_INLINE int UNUSED __gthread_key_create (__gthread_key_t *__key UNUSED, void (*__func) (void *) UNUSED) { return 0; } -static int UNUSED +__GTHREAD_INLINE int UNUSED __gthread_key_delete (__gthread_key_t __key UNUSED) { return 0; } -static inline void * +__GTHREAD_INLINE void * __gthread_getspecific (__gthread_key_t __key UNUSED) { return 0; } -static inline int +__GTHREAD_INLINE int __gthread_setspecific (__gthread_key_t __key UNUSED, const void *__v UNUSED) { return 0; } -static inline int +__GTHREAD_INLINE int __gthread_mutex_destroy (__gthread_mutex_t *__mutex UNUSED) { return 0; } -static inline int +__GTHREAD_INLINE int __gthread_mutex_lock (__gthread_mutex_t *__mutex UNUSED) { return 0; } -static inline int +__GTHREAD_INLINE int __gthread_mutex_trylock (__gthread_mutex_t *__mutex UNUSED) { return 0; } -static inline int +__GTHREAD_INLINE int __gthread_mutex_unlock (__gthread_mutex_t *__mutex UNUSED) { return 0; } -static inline int +__GTHREAD_INLINE int __gthread_recursive_mutex_lock (__gthread_recursive_mutex_t *__mutex) { return __gthread_mutex_lock (__mutex); } -static inline int +__GTHREAD_INLINE int __gthread_recursive_mutex_trylock (__gthread_recursive_mutex_t *__mutex) { return __gthread_mutex_trylock (__mutex); } -static inline int +__GTHREAD_INLINE int __gthread_recursive_mutex_unlock (__gthread_recursive_mutex_t *__mutex) { return __gthread_mutex_unlock (__mutex); } -static inline int +__GTHREAD_INLINE int __gthread_recursive_mutex_destroy (__gthread_recursive_mutex_t *__mutex) { return __gthread_mutex_destroy (__mutex); diff --git a/libstdc++-v3/include/bits/shared_ptr.h b/libstdc++-v3/include/bits/shared_ptr.h index 13273afa9d2..9b1c2b4eb3d 100644 --- a/libstdc++-v3/include/bits/shared_ptr.h +++ b/libstdc++-v3/include/bits/shared_ptr.h @@ -1160,9 +1160,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION #if __cpp_variable_templates template<typename _Tp> - static constexpr bool __is_shared_ptr = false; + constexpr bool __is_shared_ptr = false; template<typename _Tp> - static constexpr bool __is_shared_ptr<shared_ptr<_Tp>> = true; + constexpr bool __is_shared_ptr<shared_ptr<_Tp>> = true; #endif /// @} relates shared_ptr diff --git a/libstdc++-v3/include/bits/unique_ptr.h b/libstdc++-v3/include/bits/unique_ptr.h index edcff78bff9..182173aa857 100644 --- a/libstdc++-v3/include/bits/unique_ptr.h +++ b/libstdc++-v3/include/bits/unique_ptr.h @@ -1157,9 +1157,9 @@ namespace __detail #if __cpp_variable_templates template<typename _Tp> - static constexpr bool __is_unique_ptr = false; + constexpr bool __is_unique_ptr = false; template<typename _Tp, typename _Del> - static constexpr bool __is_unique_ptr<unique_ptr<_Tp, _Del>> = true; + constexpr bool __is_unique_ptr<unique_ptr<_Tp, _Del>> = true; #endif /// @} group pointer_abstractions diff --git a/libstdc++-v3/include/std/future b/libstdc++-v3/include/std/future index 6ce7d89ca3f..437453eaa44 100644 --- a/libstdc++-v3/include/std/future +++ b/libstdc++-v3/include/std/future @@ -1526,7 +1526,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template<typename _Signature, typename _Fn, typename _Alloc = std::allocator<int>> - static shared_ptr<__future_base::_Task_state_base<_Signature>> + shared_ptr<__future_base::_Task_state_base<_Signature>> __create_task_state(_Fn&& __fn, const _Alloc& __a = _Alloc()) { typedef typename decay<_Fn>::type _Fn2; diff --git a/libstdc++-v3/include/std/shared_mutex b/libstdc++-v3/include/std/shared_mutex index 1b6478f30c3..7e7941dd7a1 100644 --- a/libstdc++-v3/include/std/shared_mutex +++ b/libstdc++-v3/include/std/shared_mutex @@ -72,7 +72,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION #ifdef __gthrw #define _GLIBCXX_GTHRW(name) \ __gthrw(pthread_ ## name); \ - static inline int \ + inline int \ __glibcxx_ ## name (pthread_rwlock_t *__rwlock) \ { \ if (__gthread_active_p ()) \ @@ -88,7 +88,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION # ifndef PTHREAD_RWLOCK_INITIALIZER _GLIBCXX_GTHRW(rwlock_destroy) __gthrw(pthread_rwlock_init); - static inline int + inline int __glibcxx_rwlock_init (pthread_rwlock_t *__rwlock) { if (__gthread_active_p ()) @@ -99,7 +99,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION # endif # if _GTHREAD_USE_MUTEX_TIMEDLOCK __gthrw(pthread_rwlock_timedrdlock); - static inline int + inline int __glibcxx_rwlock_timedrdlock (pthread_rwlock_t *__rwlock, const timespec *__ts) { @@ -109,7 +109,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION return 0; } __gthrw(pthread_rwlock_timedwrlock); - static inline int + inline int __glibcxx_rwlock_timedwrlock (pthread_rwlock_t *__rwlock, const timespec *__ts) { @@ -120,33 +120,33 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION } # endif #else - static inline int + inline int __glibcxx_rwlock_rdlock (pthread_rwlock_t *__rwlock) { return pthread_rwlock_rdlock (__rwlock); } - static inline int + inline int __glibcxx_rwlock_tryrdlock (pthread_rwlock_t *__rwlock) { return pthread_rwlock_tryrdlock (__rwlock); } - static inline int + inline int __glibcxx_rwlock_wrlock (pthread_rwlock_t *__rwlock) { return pthread_rwlock_wrlock (__rwlock); } - static inline int + inline int __glibcxx_rwlock_trywrlock (pthread_rwlock_t *__rwlock) { return pthread_rwlock_trywrlock (__rwlock); } - static inline int + inline int __glibcxx_rwlock_unlock (pthread_rwlock_t *__rwlock) { return pthread_rwlock_unlock (__rwlock); } - static inline int + inline int __glibcxx_rwlock_destroy(pthread_rwlock_t *__rwlock) { return pthread_rwlock_destroy (__rwlock); } - static inline int + inline int __glibcxx_rwlock_init(pthread_rwlock_t *__rwlock) { return pthread_rwlock_init (__rwlock, NULL); } # if _GTHREAD_USE_MUTEX_TIMEDLOCK - static inline int + inline int __glibcxx_rwlock_timedrdlock (pthread_rwlock_t *__rwlock, const timespec *__ts) { return pthread_rwlock_timedrdlock (__rwlock, __ts); } - static inline int + inline int __glibcxx_rwlock_timedwrlock (pthread_rwlock_t *__rwlock, const timespec *__ts) { return pthread_rwlock_timedwrlock (__rwlock, __ts); }
I found that my previous minimal change to libstdc++ was only sufficient to pass regtest on x86_64-pc-linux-gnu; Linaro complained about ARM and aarch64. This patch removes the rest of the internal-linkage entities I could find exposed via libstdc++. The libgcc changes include some blocks specific to FreeBSD, Solaris <10, and HP-UX; I haven't been able to test these changes at this time. Happy to adjust or remove those hunks as needed. Apologies if I haven't CC'd in the correct people. Bootstrapped and regtested on x86_64-pc-linux-gnu and aarch64-unknown-linux-gnu, OK for trunk? -- >8 -- In C++20, modules streaming check for exposures of TU-local entities. In general exposing internal linkage functions in a header is liable to cause ODR violations in C++, and this is now detected in a module context. This patch goes through and removes 'static' from many functions exposed through libstdc++ to prevent code like the following from failing: export module M; extern "C++" { #include <bits/stdc++.h> } Since gthreads is used from C as well, we need to choose whether to use 'inline' or 'static inline' depending on whether we're compiling for C or C++ (since the semantics of 'inline' are different between the languages). Additionally we need to remove static global variables, so we migrate these to function-local statics to avoid the ODR issues. There doesn't seem to be a good workaround for weakrefs, so I've left them as-is and will work around it in the modules streaming code to consider them as not TU-local. The same issue occurs in the objective-C specific parts of gthreads, but I'm not familiar with the surrounding context and we don't currently test modules with Objective C++ anyway so I've left it as-is. PR libstdc++/115126 libgcc/ChangeLog: * gthr-posix.h (__GTHREAD_INLINE): New macro. (__gthread_active): Convert from variable to function. (__gthread_trigger): Mark as __GTHREAD_INLINE instead of static. (__gthread_active_p): Likewise. (__gthread_create): Likewise. (__gthread_join): Likewise. (__gthread_detach): Likewise. (__gthread_equal): Likewise. (__gthread_self): Likewise. (__gthread_yield): Likewise. (__gthread_once): Likewise. (__gthread_key_create): Likewise. (__gthread_key_delete): Likewise. (__gthread_getspecific): Likewise. (__gthread_setspecific): Likewise. (__gthread_mutex_init_function): Likewise. (__gthread_mutex_destroy): Likewise. (__gthread_mutex_lock): Likewise. (__gthread_mutex_trylock): Likewise. (__gthread_mutex_timedlock): Likewise. (__gthread_mutex_unlock): Likewise. (__gthread_recursive_mutex_init_function): Likewise. (__gthread_recursive_mutex_lock): Likewise. (__gthread_recursive_mutex_trylock): Likewise. (__gthread_recursive_mutex_timedlock): Likewise. (__gthread_recursive_mutex_unlock): Likewise. (__gthread_recursive_mutex_destroy): Likewise. (__gthread_cond_init_function): Likewise. (__gthread_cond_broadcast): Likewise. (__gthread_cond_signal): Likewise. (__gthread_cond_wait): Likewise. (__gthread_cond_timedwait): Likewise. (__gthread_cond_wait_recursive): Likewise. (__gthread_cond_destroy): Likewise. (__gthread_rwlock_rdlock): Likewise. (__gthread_rwlock_tryrdlock): Likewise. (__gthread_rwlock_wrlock): Likewise. (__gthread_rwlock_trywrlock): Likewise. (__gthread_rwlock_unlock): Likewise. * gthr-single.h (__GTHREAD_INLINE): New macro. (__gthread_active_p): Mark as __GTHREAD_INLINE instead of static. (__gthread_once): Likewise. (__gthread_key_create): Likewise. (__gthread_key_delete): Likewise. (__gthread_getspecific): Likewise. (__gthread_setspecific): Likewise. (__gthread_mutex_destroy): Likewise. (__gthread_mutex_lock): Likewise. (__gthread_mutex_trylock): Likewise. (__gthread_mutex_unlock): Likewise. (__gthread_recursive_mutex_lock): Likewise. (__gthread_recursive_mutex_trylock): Likewise. (__gthread_recursive_mutex_unlock): Likewise. (__gthread_recursive_mutex_destroy): Likewise. libstdc++-v3/ChangeLog: * include/bits/shared_ptr.h (std::__is_shared_ptr): Remove unnecessary 'static'. * include/bits/unique_ptr.h (std::__is_unique_ptr): Likewise. * include/std/future (std::__craete_task_state): Likewise. * include/std/shared_mutex (_GLIBCXX_GTRHW): Likewise. (__glibcxx_rwlock_init): Likewise. (__glibcxx_rwlock_timedrdlock): Likewise. (__glibcxx_rwlock_timedwrlock): Likewise. (__glibcxx_rwlock_rdlock): Likewise. (__glibcxx_rwlock_tryrdlock): Likewise. (__glibcxx_rwlock_wrlock): Likewise. (__glibcxx_rwlock_trywrlock): Likewise. (__glibcxx_rwlock_unlock): Likewise. (__glibcxx_rwlock_destroy): Likewise. (__glibcxx_rwlock_init): Likewise. Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> --- libgcc/gthr-posix.h | 116 ++++++++++++++----------- libgcc/gthr-single.h | 34 +++++--- libstdc++-v3/include/bits/shared_ptr.h | 4 +- libstdc++-v3/include/bits/unique_ptr.h | 4 +- libstdc++-v3/include/std/future | 2 +- libstdc++-v3/include/std/shared_mutex | 26 +++--- 6 files changed, 104 insertions(+), 82 deletions(-)