|
Message-ID: <20201123114124.GP1312820@redhat.com> Date: Mon, 23 Nov 2020 11:41:24 +0000 From: Jonathan Wakely <jwakely@...hat.com> To: Rich Felker <dalias@...c.org> Cc: Florian Weimer <fw@...eb.enyo.de>, Арсений <a@...r0n.science>, musl@...ts.openwall.com Subject: Re: Mutexes are not unlocking On 22/11/20 14:28 -0500, Rich Felker wrote: >On Sun, Nov 22, 2020 at 08:23:23PM +0100, Florian Weimer wrote: >> * Rich Felker: >> >> > On Sun, Nov 22, 2020 at 09:43:35PM +0300, Арсений wrote: >> >> >> >> Hello, >> >> The problem is that mutex is not got unlocked after the first unlock(). >> >> >> >> libstdc++ uses a wrapper for pthread called gthreads. This wrapper >> >> checks for the state of the mutex system. For >> >> example, pthread_mutex_unlock() is called in a following way: >> >> >> >> static inline int >> >> __gthread_mutex_unlock (__gthread_mutex_t *__mutex) >> >> { >> >> if (__gthread_active_p ()) >> >> return __gthrw_(pthread_mutex_unlock) (__mutex); >> >> else >> >> return 0; >> >> } >> > >> > Yes. This code is invalid (it misinterprets weak symbol information to >> > draw incorrect conclusions about whether threads may be in use) and >> > thus is disabled in builds of gcc/libstdc++ targeting musl-based >> > systems. GCC and glibc-based distro folks mostly don't care because >> > it only breaks static linking, but some of them actually hack gcc's >> > libpthread.a into one giant .o file to work around the problem rather >> > than fixing this in gcc... >> >> GCC 11 has a fix (if used along with glibc 2.32), but I wonder if it's The GCC 11 change isn't used for mutexes. I use __libc_single_threaded for deciding whether to use atomics or plain non-atomic ops for ref-count updates. But for actions like mutex locks I don't use it, because it would do the wrong thing for code like: int main() { std::mutex m; std::lock_guard<std::mutex> l(m); std::thread t( []{} }; t.join(); } In this program __libc_single_threaded is true when we construct the lock_guard, but false when it is destroyed at the end of the block. If we checked __libc_single_threaded for mutex locking/unlocking then we would be inconsistent here, we would elide the lock but try to unlock a mutex that was never locked, which would be UB. Whatever condition we check to decide whether to call the pthreads function needs to give the same result for the lock and unlock. That condition is currently __gthread_active_p, which checks if a symbol is weak. >> going to run into a similar issue because inlined code from older GCC >> versions uses a diverging version of the check. No, everything uses the same check. GCC 11 didn't change anything for mutexes. >> Jonathan, more context is here: >> >> <https://www.openwall.com/lists/musl/2020/11/22/1> > >Uhg, that's a really nasty problem. Is __gthread_active_p() also >inlined? Or can it be given a definition to mitigate the problem? It's inlined, and making it a non-inline PLT call would negate some of the point of using it (it's there mostly as an optimization). But I don't think inlining it is a problem, because its definition hasn't changed. I think the right thing to do is a new configuration which completely avoids using weak symbols in gthreads, and just calls the pthread symbols directly. A future version of glibc is going to move pthreads symbols into libc anyway: https://sourceware.org/legacy-ml/libc-alpha/2019-10/msg00080.html After that, the __gthread_active_p condition is just a wasted branch, because it will be unconditionally true: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89714 If the musl libstdc++ removes the use of __gthread_active_p, but code compiled against a glibc libstdc++ inlines the __gthread_active_p check, then it seems to me that musl (or the musl libstdc++) needs to ensure that the inlined __gthread_active_p will return true. It can do that by providing a non-weak symbol called __pthread_key_create (the symbol won't be called, it just has to exist).
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.