Hi Jonathan, Just wanted to gently ping to see if you've had a chance to take a look.
Thanks, Keno On Thu, Oct 16, 2025 at 3:15 PM Jonathan Wakely <[email protected]> wrote: > > I went travelling for a month on the day you sent the first patch and so I > missed it. I'll take a look ASAP. > > > > On Thu, 16 Oct 2025, 19:39 Keno Fischer, <[email protected]> wrote: >> >> Hello all, >> >> My apologies for not following up on the below earlier, but we're getting >> additional user complaints about this ABI break, so it's back on my radar. >> Were there any comments or concerns on my RFC patch below? >> >> Keno >> >> On Tue, Jul 15, 2025 at 1:37 AM Keno Fischer <[email protected]> wrote: >> > >> > The _GLIBCXX_HAVE_TLS flag is an ABI-breaking flag - setting it removes the >> > definitions for `set_lock_ptr` (as well as the comptability symbols for >> > even older ABIs). Try to improve this situation by changing the flag to >> > retain the old symbols, falling back to the previous behavior if >> > __once_call >> > is not set. These symbols are used to forward closure data for the >> > initialization >> > from whichever thread ends up winning the pthread_once. The __once_call >> > is set immediately before the pthread_once and unset (in a destructor) >> > immediately after. To avoid the case where there may be a nested >> > __once_proxy >> > call that uses the old ABI, we also unset __once_call in the success path >> > after retrieving the pointer. >> > >> > Signed-off-by: Keno Fischer <[email protected]> >> > --- >> > >> > Greetings from the Julia Language infrastructure team. For background >> > behind this patch, we maintain a large number of pre-built binaries for >> > multiple operating systems, including windows. On windows, we generally >> > follow the MSYS2-provided mingw ABI for our binaries (even though we >> > maintain our own toolchains to actually generate the binaries). >> > At some point recently, msys2 switched to building gcc with --enable-tls. >> > This caused an ABI break for us, making our old binaries no longer >> > work in newer msys2 environments because. I have two distinct questions >> > here: >> > >> > 1. Is there something obviously wrong with this approach that I'm >> > missing? >> > >> > 2. Is this appropriate for upstream inclusion? >> > >> > We're fine if the answer to #2 is no - in which case we'd ship a patched >> > libstdc++ version with this patch for a while until all binaries that >> > have shipped with the old ABI have filtered out as part of our usual >> > replacement cycle. Of course, if this approach doesn't work, we'd >> > appreciate any thoughts on how to maintain ABI compatibility here, even >> > temporarily. >> > >> > (I think the diffstat looks messier than it is because of the indentation >> > changes - the basic changes is just to merge the two __once_proxy >> > calls together, checking if __once_call is set, using it if so and falling >> > back to the old ABI if not). >> > >> > libstdc++-v3/src/c++11/mutex.cc | 111 +++++++++++++++++--------------- >> > 1 file changed, 59 insertions(+), 52 deletions(-) >> > >> > diff --git a/libstdc++-v3/src/c++11/mutex.cc >> > b/libstdc++-v3/src/c++11/mutex.cc >> > index d5da5c66ae9..4be64633315 100644 >> > --- a/libstdc++-v3/src/c++11/mutex.cc >> > +++ b/libstdc++-v3/src/c++11/mutex.cc >> > @@ -23,6 +23,7 @@ >> > // <http://www.gnu.org/licenses/>. >> > >> > #include <mutex> >> > +#include <bits/std_function.h> // std::function >> > >> > #ifdef _GLIBCXX_HAS_GTHREADS >> > >> > @@ -30,30 +31,17 @@ namespace std _GLIBCXX_VISIBILITY(default) >> > { >> > _GLIBCXX_BEGIN_NAMESPACE_VERSION >> > >> > -#ifdef _GLIBCXX_HAVE_TLS >> > - __thread void* __once_callable; >> > - __thread void (*__once_call)(); >> > +// Explicit instantiation due to -fno-implicit-instantiation. >> > +template class function<void()>; >> > >> > - extern "C" void __once_proxy() >> > - { >> > - // The caller stored a function pointer in __once_call. If it requires >> > - // any state, it gets it from __once_callable. >> > - __once_call(); >> > - } >> > +function<void()> __once_functor; >> > >> > -#else // ! TLS >> > - >> > - // Explicit instantiation due to -fno-implicit-instantiation. >> > - template class function<void()>; >> > - >> > - function<void()> __once_functor; >> > - >> > - mutex& >> > - __get_once_mutex() >> > - { >> > - static mutex once_mutex; >> > - return once_mutex; >> > - } >> > +mutex& >> > +__get_once_mutex() >> > +{ >> > + static mutex once_mutex; >> > + return once_mutex; >> > +} >> > >> > namespace >> > { >> > @@ -66,41 +54,60 @@ namespace >> > } >> > } >> > >> > - // code linked against ABI 3.4.12 and later uses this >> > - void >> > - __set_once_functor_lock_ptr(unique_lock<mutex>* __ptr) >> > - { >> > - (void) set_lock_ptr(__ptr); >> > - } >> > +// code linked against ABI 3.4.12 and later uses this >> > +void >> > +__set_once_functor_lock_ptr(unique_lock<mutex>* __ptr) >> > +{ >> > + (void) set_lock_ptr(__ptr); >> > +} >> > >> > - // unsafe - retained for compatibility with ABI 3.4.11 >> > - unique_lock<mutex>& >> > - __get_once_functor_lock() >> > - { >> > - static unique_lock<mutex> once_functor_lock(__get_once_mutex(), >> > defer_lock); >> > - return once_functor_lock; >> > - } >> > +// unsafe - retained for compatibility with ABI 3.4.11 >> > +unique_lock<mutex>& >> > +__get_once_functor_lock() >> > +{ >> > + static unique_lock<mutex> once_functor_lock(__get_once_mutex(), >> > defer_lock); >> > + return once_functor_lock; >> > +} >> > >> > - // This is called via pthread_once while __get_once_mutex() is locked. >> > - extern "C" void >> > - __once_proxy() >> > - { >> > - // Get the callable out of the global functor. >> > - function<void()> callable = std::move(__once_functor); >> > - >> > - // Then unlock the global mutex >> > - if (unique_lock<mutex>* lock = set_lock_ptr(nullptr)) >> > - { >> > - // Caller is using the new ABI and provided a pointer to its lock. >> > - lock->unlock(); >> > +#ifdef _GLIBCXX_HAVE_TLS >> > + __thread void* __once_callable; >> > + __thread void (*__once_call)(); >> > +#endif >> > + >> > +extern "C" void >> > +__once_proxy() >> > +{ >> > +#ifdef _GLIBCXX_HAVE_TLS >> > + if (__once_call) { >> > + void (*this_once_call)() = __once_call; >> > + // Reset __once_call early in case of a nested call to __once_proxy >> > + // with the old ABI. >> > + __once_call = NULL; >> > + return this_once_call(); >> > } >> > - else >> > - __get_once_functor_lock().unlock(); // global lock >> > >> > - // Finally, invoke the callable. >> > - callable(); >> > + // For compatibility with callers linked against >> > non-_GLIBCXX_HAVE_TLS ABI, >> > + // we fall through here to the old behavior. >> > +#endif >> > + >> > + // If the caller is not using _GLIBCXX_HAVE_TLS, this was called >> > + // via pthread_once while __get_once_mutex() is locked. >> > + >> > + // Get the callable out of the global functor. >> > + function<void()> callable = std::move(__once_functor); >> > + >> > + // Then unlock the global mutex >> > + if (unique_lock<mutex>* lock = set_lock_ptr(nullptr)) >> > + { >> > + // Caller is using the new ABI and provided a pointer to its lock. >> > + lock->unlock(); >> > } >> > -#endif // ! TLS >> > + else >> > + __get_once_functor_lock().unlock(); // global lock >> > + >> > + // Finally, invoke the callable. >> > + callable(); >> > +} >> > >> > _GLIBCXX_END_NAMESPACE_VERSION >> > } // namespace std >> > -- >> > 2.43.0
