mailing list of musl libc
 help / color / mirror / Atom feed
* [musl] Mutexes are not unlocking
@ 2020-11-20  5:25 a
  2020-11-20  5:58 ` Rich Felker
  0 siblings, 1 reply; 20+ messages in thread
From: a @ 2020-11-20  5:25 UTC (permalink / raw)
  To: musl

[-- Attachment #1: Type: text/plain, Size: 2005 bytes --]

Hello, all,
I am experiencing a problem with mutexes in musl-libc. The mutex is not unlocked after calling unlock(), this causes getting stuck on attempt to lock it next time. Example code (C++):

void testMutex(std::mutex &mtx, const char * name) {
fprintf(stderr, "-- testing %s --\n", name);
fprintf(stderr, "lock\n");
mtx.lock();
fprintf(stderr, "unlock\n");
mtx.unlock();
fprintf(stderr, "lock2\n");
mtx.lock();
fprintf(stderr, "unlock2\n");
mtx.unlock();
fprintf(stderr, "done\n");
} The problem can be reproduced only on musl-libc, the same binary works well on the system with glibc.
The problem does not reproduce each time, its reproducibility depends on the phase of moon.
The problem can be reproduced more often it the code calling mutex functions is located in the shared library.

Strace (when the problem is reproduced):
[pid   709] writev(2, [{iov_base="-- testing gsMutex --\n", iov_len=22}, {iov_base=NULL, iov_len=0}], 2-- testing gsMutex -- 
) = 22 
[pid   709] writev(2, [{iov_base="", iov_len=0}, {iov_base="lock\n", iov_len=5}], 2lock 
) = 5 
[pid   709] writev(2, [{iov_base="", iov_len=0}, {iov_base="unlock\n", iov_len=7}], 2unlock 
) = 7 
[pid   709] writev(2, [{iov_base="", iov_len=0}, {iov_base="lock2\n", iov_len=6}], 2lock2 
) = 6 
[pid   709] futex(0x7f3a9733e4a4, FUTEX_WAIT_PRIVATE, 2147483664, NULL

Strace (when the problem is not reproduced):
writev(2, [{iov_base="-- testing hhMutex --\n", iov_len=22}, {iov_base=NULL, iov_len=0}], 2-- testing hhMutex -- 
) = 22 
writev(2, [{iov_base="", iov_len=0}, {iov_base="lock\n", iov_len=5}], 2lock 
) = 5 
writev(2, [{iov_base="", iov_len=0}, {iov_base="unlock\n", iov_len=7}], 2unlock 
) = 7 
writev(2, [{iov_base="", iov_len=0}, {iov_base="lock2\n", iov_len=6}], 2lock2 
) = 6 
writev(2, [{iov_base="", iov_len=0}, {iov_base="unlock2\n", iov_len=8}], 2unlock2 
) = 8 
writev(2, [{iov_base="", iov_len=0}, {iov_base="done\n", iov_len=5}], 2done 
) = 5

Thanks in advance for solving the problem.

—— 
Arseniy

[-- Attachment #2: Type: text/html, Size: 3468 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [musl] Mutexes are not unlocking
  2020-11-20  5:25 [musl] Mutexes are not unlocking a
@ 2020-11-20  5:58 ` Rich Felker
  2020-11-21  6:46   ` Re[2]: " a
  0 siblings, 1 reply; 20+ messages in thread
From: Rich Felker @ 2020-11-20  5:58 UTC (permalink / raw)
  To: a; +Cc: musl

On Fri, Nov 20, 2020 at 08:25:17AM +0300, a@saur0n.science wrote:
> Hello, all,
> I am experiencing a problem with mutexes in musl-libc. The mutex is
> not unlocked after calling unlock(), this causes getting stuck on
> attempt to lock it next time. Example code (C++):
> 
> void testMutex(std::mutex &mtx, const char * name) {
> fprintf(stderr, "-- testing %s --\n", name);
> fprintf(stderr, "lock\n");
> mtx.lock();
> fprintf(stderr, "unlock\n");
> mtx.unlock();
> fprintf(stderr, "lock2\n");
> mtx.lock();
> fprintf(stderr, "unlock2\n");
> mtx.unlock();
> fprintf(stderr, "done\n");
> }
> The problem can be reproduced only on musl-libc, the same binary
> works well on the system with glibc.

The same binary? Do you mean you have a glibc-built binary you're
trying to use with musl? Or did you mean to say the same source
program?

> The problem does not reproduce each time, its reproducibility
> depends on the phase of moon.
> The problem can be reproduced more often it the code calling mutex
> functions is located in the shared library.
> 
> Strace (when the problem is reproduced):
> [pid   709] writev(2, [{iov_base="-- testing gsMutex --\n", iov_len=22}, {iov_base=NULL, iov_len=0}], 2-- testing gsMutex -- 
> ) = 22 
> [pid   709] writev(2, [{iov_base="", iov_len=0}, {iov_base="lock\n", iov_len=5}], 2lock 
> ) = 5 
> [pid   709] writev(2, [{iov_base="", iov_len=0}, {iov_base="unlock\n", iov_len=7}], 2unlock 
> ) = 7 
> [pid   709] writev(2, [{iov_base="", iov_len=0}, {iov_base="lock2\n", iov_len=6}], 2lock2 
> ) = 6 
> [pid   709] futex(0x7f3a9733e4a4, FUTEX_WAIT_PRIVATE, 2147483664, NULL
> 
> Strace (when the problem is not reproduced):
> writev(2, [{iov_base="-- testing hhMutex --\n", iov_len=22}, {iov_base=NULL, iov_len=0}], 2-- testing hhMutex -- 
> ) = 22 
> writev(2, [{iov_base="", iov_len=0}, {iov_base="lock\n", iov_len=5}], 2lock 
> ) = 5 
> writev(2, [{iov_base="", iov_len=0}, {iov_base="unlock\n", iov_len=7}], 2unlock 
> ) = 7 
> writev(2, [{iov_base="", iov_len=0}, {iov_base="lock2\n", iov_len=6}], 2lock2 
> ) = 6 
> writev(2, [{iov_base="", iov_len=0}, {iov_base="unlock2\n", iov_len=8}], 2unlock2 
> ) = 8 
> writev(2, [{iov_base="", iov_len=0}, {iov_base="done\n", iov_len=5}], 2done 
> ) = 5
> 
> Thanks in advance for solving the problem.

Can you post the full test case and information on how you built it,
including what compiler you're using (whether it's from a distro, or
one you built yourself, or what)?

Rich

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re[2]: [musl] Mutexes are not unlocking
  2020-11-20  5:58 ` Rich Felker
@ 2020-11-21  6:46   ` a
  2020-11-21 15:51     ` Rich Felker
  0 siblings, 1 reply; 20+ messages in thread
From: a @ 2020-11-21  6:46 UTC (permalink / raw)
  To: Rich Felker; +Cc: musl

[-- Attachment #1: Type: text/plain, Size: 4523 bytes --]

Hello,
Thanks for the fast response.

I compiled binary under OpenSUSE towards glibc and started on Alpine Linux. I think it is not a problem because mutex is aggregated by a pointer.

I am using gcc 10.2.1 20201028 [revision a78cd759754c92cecbf235ac9b447dcdff6c6e2f] installed from OpenSUSE reporitories.

Unfortunately, I cannot post a "minimal working example", because the bug does not reproduce on a small programs.

This is the internal state of the mutex which caused problems at the moment of second lock operation:
(gdb) print gsMutex 
$1 = {<std::__mutex_base> = {_M_mutex = {__data = {__lock = 0, __count = 2147483664, __owner = 1, __nusers = 0, __kind = 0, __spins = 0, __elision = 0, __list = { 
         __prev =  0x0 , __next =  0x0 }}, __size = "\000\000\000\000\020\000\000\200\001", '\000' <repeats 30 times>, __align = -9223371968135299072}}, <No data fields>}

Backtrace:
#0   __syscall_cp_c ( nr =<optimized out>,  u =140737352115364,  v =128,  w =-2147483632,  x =0,  y =0,  z =0) at  ./arch/x86_64/syscall_arch.h :61 
#1   0x00007ffff7fbcacf in  __timedwait_cp ( addr=addr@entry =0x7ffff7e124a4 <gsMutex+4>,  val=val@entry =-2147483632,  clk=clk@entry =0,  at=at@entry =0x0,  
    priv =<optimized out>,  priv@entry =128) at  src/thread/__timedwait.c :31 
#2   0x00007ffff7fbcb69 in  __timedwait ( addr=addr@entry =0x7ffff7e124a4 <gsMutex+4>,  val =-2147483632,  clk=clk@entry =0,  at=at@entry =0x0,  priv=priv@entry =128) 
   at  src/thread/__timedwait.c :48 
#3   0x00007ffff7fbeb4b in  __pthread_mutex_timedlock ( m =0x7ffff7e124a0 <gsMutex>,  at =0x0) at  src/thread/pthread_mutex_timedlock.c :67 
#4   0x00007ffff7eb6de4 in  std::mutex::lock() () from /usr/lib/libstdc++.so.6 
#5   0x00007ffff7e0e363 in  testMutex ( mtx =...,  name=name@entry =0x7ffff7e0f1fa "gsMutex") at  Session.cpp :27
(

>Пятница, 20 ноября 2020, 8:58 +03:00 от Rich Felker <dalias@libc.org>:
>
>On Fri, Nov 20, 2020 at 08:25:17AM +0300,  a@saur0n.science wrote:
>> Hello, all,
>> I am experiencing a problem with mutexes in musl-libc. The mutex is
>> not unlocked after calling unlock(), this causes getting stuck on
>> attempt to lock it next time. Example code (C++):
>> 
>> void testMutex(std::mutex &mtx, const char * name) {
>> fprintf(stderr, "-- testing %s --\n", name);
>> fprintf(stderr, "lock\n");
>> mtx.lock();
>> fprintf(stderr, "unlock\n");
>> mtx.unlock();
>> fprintf(stderr, "lock2\n");
>> mtx.lock();
>> fprintf(stderr, "unlock2\n");
>> mtx.unlock();
>> fprintf(stderr, "done\n");
>> }
>> The problem can be reproduced only on musl-libc, the same binary
>> works well on the system with glibc.
>The same binary? Do you mean you have a glibc-built binary you're
>trying to use with musl? Or did you mean to say the same source
>program?
>
>> The problem does not reproduce each time, its reproducibility
>> depends on the phase of moon.
>> The problem can be reproduced more often it the code calling mutex
>> functions is located in the shared library.
>> 
>> Strace (when the problem is reproduced):
>> [pid   709] writev(2, [{iov_base="-- testing gsMutex --\n", iov_len=22}, {iov_base=NULL, iov_len=0}], 2-- testing gsMutex -- 
>> ) = 22 
>> [pid   709] writev(2, [{iov_base="", iov_len=0}, {iov_base="lock\n", iov_len=5}], 2lock 
>> ) = 5 
>> [pid   709] writev(2, [{iov_base="", iov_len=0}, {iov_base="unlock\n", iov_len=7}], 2unlock 
>> ) = 7 
>> [pid   709] writev(2, [{iov_base="", iov_len=0}, {iov_base="lock2\n", iov_len=6}], 2lock2 
>> ) = 6 
>> [pid   709] futex(0x7f3a9733e4a4, FUTEX_WAIT_PRIVATE, 2147483664, NULL
>> 
>> Strace (when the problem is not reproduced):
>> writev(2, [{iov_base="-- testing hhMutex --\n", iov_len=22}, {iov_base=NULL, iov_len=0}], 2-- testing hhMutex -- 
>> ) = 22 
>> writev(2, [{iov_base="", iov_len=0}, {iov_base="lock\n", iov_len=5}], 2lock 
>> ) = 5 
>> writev(2, [{iov_base="", iov_len=0}, {iov_base="unlock\n", iov_len=7}], 2unlock 
>> ) = 7 
>> writev(2, [{iov_base="", iov_len=0}, {iov_base="lock2\n", iov_len=6}], 2lock2 
>> ) = 6 
>> writev(2, [{iov_base="", iov_len=0}, {iov_base="unlock2\n", iov_len=8}], 2unlock2 
>> ) = 8 
>> writev(2, [{iov_base="", iov_len=0}, {iov_base="done\n", iov_len=5}], 2done 
>> ) = 5
>> 
>> Thanks in advance for solving the problem.
>
>Can you post the full test case and information on how you built it,
>including what compiler you're using (whether it's from a distro, or
>one you built yourself, or what)?
>
>Rich


—— 
Арсений
minimal   working   example

[-- Attachment #2: Type: text/html, Size: 18572 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [musl] Mutexes are not unlocking
  2020-11-21  6:46   ` Re[2]: " a
@ 2020-11-21 15:51     ` Rich Felker
  2020-11-22 18:43       ` Re[2]: " Арсений
  0 siblings, 1 reply; 20+ messages in thread
From: Rich Felker @ 2020-11-21 15:51 UTC (permalink / raw)
  To: a; +Cc: musl

On Sat, Nov 21, 2020 at 09:46:57AM +0300, a@saur0n.science wrote:
> Hello,
> Thanks for the fast response.
> 
> I compiled binary under OpenSUSE towards glibc and started on Alpine
> Linux.

I think you mean just compiled normally on OpenSUSE without doing
anything related to musl at this point, but could you please conform
that that's the case?

Also, did you link libstdc++ statically or dynamically? Are you moving
any additional shared libraries over from the glibc system, or just
the application binary? Does it have any other libraries static linked
into it?

This is why I asked for full information on how you built it. The
answers to these questions determine what direction you should look
in.

musl provides some minimal level of glibc-ABI-compat for attempting to
get binaryware that you can't rebuild against musl to work. This
doesn't mean building binaries for glibc and attempting to use them
with musl is necessarily supported usage. It's possible that you're
doing something that just can't work, but it's also possible that
there are simple mistakes you could correct and get this working.

> I think it is not a problem because mutex is aggregated by a
> pointer.

I'm also not clear exactly what you mean by that.

> I am using gcc 10.2.1 20201028 [revision
> a78cd759754c92cecbf235ac9b447dcdff6c6e2f] installed from OpenSUSE
> reporitories.
> 
> Unfortunately, I cannot post a "minimal working example", because
> the bug does not reproduce on a small programs.

This means the problem is in something you haven't shown or described.

> This is the internal state of the mutex which caused problems at the
> moment of second lock operation:

> (gdb) print gsMutex 
> $1 = {<std::__mutex_base> = {_M_mutex = {__data = {__lock = 0, __count = 2147483664, __owner = 1, __nusers = 0, __kind = 0, __spins = 0, __elision = 0, __list = { 
>          __prev =  0x0 , __next =  0x0 }}, __size = "\000\000\000\000\020\000\000\200\001", '\000' <repeats 30 times>, __align = -9223371968135299072}}, <No data fields>}

This is printing out a glibc data structure, which isn't going to be
meaningful.

Rich

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re[2]: [musl] Mutexes are not unlocking
  2020-11-21 15:51     ` Rich Felker
@ 2020-11-22 18:43       ` Арсений
  2020-11-22 19:11         ` Rich Felker
  0 siblings, 1 reply; 20+ messages in thread
From: Арсений @ 2020-11-22 18:43 UTC (permalink / raw)
  To: Rich Felker; +Cc: musl

[-- Attachment #1: Type: text/plain, Size: 4092 bytes --]


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;
}
 
The function __gthread_active_p() is an inline function which returns non-nullptr value is pthreads is available.
 
It seems that std::mutex::lock() in libstdc++ from Alpine Linux repos does not use ghtreads:
 
=> 0x00007ffff7eb6dde <+0>:     push   %rdx
   0x00007ffff7eb6ddf <+1>:     callq  0x7ffff7eaf550 <pthread_mutex_lock@plt>
   0x00007ffff7eb6de4 <+6>:     test   %eax,%eax
   0x00007ffff7eb6de6 <+8>:     je     0x7ffff7eb6def <_ZNSt5mutex4lockEv+17>
   0x00007ffff7eb6de8 <+10>:    mov    %eax,%edi
   0x00007ffff7eb6dea <+12>:    callq  0x7ffff7eb3f50 <_ZSt20__throw_system_errori@plt>
   0x00007ffff7eb6def <+17>:    pop    %rax
   0x00007ffff7eb6df0 <+18>:    retq 
 
std::mutex::lock() from glibc-compatible libstdc++ is not inlined by compiler during build, because it contains check for errors:
    void
    lock()
    {
      int __e = __gthread_mutex_lock(&_M_mutex);

      // EINVAL, EAGAIN, EBUSY, EINVAL, EDEADLK(may)
      if (__e)
    __throw_system_error(__e);
    }
 
std::mutex::unlock() from glibc-compatible libstdc++ is inlined, because it does not contain anything but call to __gthread_mutex_lock():
    void
    unlock()
    {
      // XXX EINVAL, EAGAIN, EPERM
      __gthread_mutex_unlock(&_M_mutex);
    }
This is why pthread_mutex_lock() from musl is called, but pthread_mutex_unlock() is not.
  
>Суббота, 21 ноября 2020, 18:51 +03:00 от Rich Felker <dalias@libc.org>:
> 
>On Sat, Nov 21, 2020 at 09:46:57AM +0300,  a@saur0n.science wrote:
>> Hello,
>> Thanks for the fast response.
>>
>> I compiled binary under OpenSUSE towards glibc and started on Alpine
>> Linux.
>
>I think you mean just compiled normally on OpenSUSE without doing
>anything related to musl at this point, but could you please conform
>that that's the case?
>
>Also, did you link libstdc++ statically or dynamically? Are you moving
>any additional shared libraries over from the glibc system, or just
>the application binary? Does it have any other libraries static linked
>into it?
>
>This is why I asked for full information on how you built it. The
>answers to these questions determine what direction you should look
>in.
>
>musl provides some minimal level of glibc-ABI-compat for attempting to
>get binaryware that you can't rebuild against musl to work. This
>doesn't mean building binaries for glibc and attempting to use them
>with musl is necessarily supported usage. It's possible that you're
>doing something that just can't work, but it's also possible that
>there are simple mistakes you could correct and get this working.
>
>> I think it is not a problem because mutex is aggregated by a
>> pointer.
>
>I'm also not clear exactly what you mean by that.
>
>> I am using gcc 10.2.1 20201028 [revision
>> a78cd759754c92cecbf235ac9b447dcdff6c6e2f] installed from OpenSUSE
>> reporitories.
>>
>> Unfortunately, I cannot post a "minimal working example", because
>> the bug does not reproduce on a small programs.
>
>This means the problem is in something you haven't shown or described.
>
>> This is the internal state of the mutex which caused problems at the
>> moment of second lock operation:
>
>> (gdb) print gsMutex
>> $1 = {<std::__mutex_base> = {_M_mutex = {__data = {__lock = 0, __count = 2147483664, __owner = 1, __nusers = 0, __kind = 0, __spins = 0, __elision = 0, __list = {
>>          __prev = 0x0 , __next = 0x0 }}, __size = "\000\000\000\000\020\000\000\200\001", '\000' <repeats 30 times>, __align = -9223371968135299072}}, <No data fields>}
>
>This is printing out a glibc data structure, which isn't going to be
>meaningful.
>
>Rich
 
 
——
Арсений
 

[-- Attachment #2: Type: text/html, Size: 6595 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [musl] Mutexes are not unlocking
  2020-11-22 18:43       ` Re[2]: " Арсений
@ 2020-11-22 19:11         ` Rich Felker
  2020-11-22 19:23           ` Florian Weimer
  0 siblings, 1 reply; 20+ messages in thread
From: Rich Felker @ 2020-11-22 19:11 UTC (permalink / raw)
  To: Арсений; +Cc: musl

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...

> The function __gthread_active_p() is an inline function which
> returns non-nullptr value is pthreads is available.
> 
> It seems that std::mutex::lock() in libstdc++ from Alpine Linux
> repos does not use ghtreads:

Yes. That's a standard requirement for building a musl-targeting gcc.

>  
> => 0x00007ffff7eb6dde <+0>:     push   %rdx
>    0x00007ffff7eb6ddf <+1>:     callq  0x7ffff7eaf550 <pthread_mutex_lock@plt>
>    0x00007ffff7eb6de4 <+6>:     test   %eax,%eax
>    0x00007ffff7eb6de6 <+8>:     je     0x7ffff7eb6def <_ZNSt5mutex4lockEv+17>
>    0x00007ffff7eb6de8 <+10>:    mov    %eax,%edi
>    0x00007ffff7eb6dea <+12>:    callq  0x7ffff7eb3f50 <_ZSt20__throw_system_errori@plt>
>    0x00007ffff7eb6def <+17>:    pop    %rax
>    0x00007ffff7eb6df0 <+18>:    retq 
>  
> std::mutex::lock() from glibc-compatible libstdc++ is not inlined by
> compiler during build, because it contains check for errors:
>     void
>     lock()
>     {
>       int __e = __gthread_mutex_lock(&_M_mutex);
> 
>       // EINVAL, EAGAIN, EBUSY, EINVAL, EDEADLK(may)
>       if (__e)
>     __throw_system_error(__e);
>     }
>  
> std::mutex::unlock() from glibc-compatible libstdc++ is inlined,
> because it does not contain anything but call to
> __gthread_mutex_lock():
>     void
>     unlock()
>     {
>       // XXX EINVAL, EAGAIN, EPERM
>       __gthread_mutex_unlock(&_M_mutex);
>     }
> This is why pthread_mutex_lock() from musl is called, but pthread_mutex_unlock() is not.

Thanks for doing the root cause analysis here. It will be interesting
(and probably infuriating) to the gcompat folks working on making
glibc-linked binaries work on musl, but maybe they already have a
workaround for this.

Anyway the answer to your original question is probably just "don't do
that" unless you want to get into fixing GCC...

Rich

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [musl] Mutexes are not unlocking
  2020-11-22 19:11         ` Rich Felker
@ 2020-11-22 19:23           ` Florian Weimer
  2020-11-22 19:28             ` Rich Felker
  0 siblings, 1 reply; 20+ messages in thread
From: Florian Weimer @ 2020-11-22 19:23 UTC (permalink / raw)
  To: Rich Felker
  Cc: Арсений, musl, jwakely

* 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
going to run into a similar issue because inlined code from older GCC
versions uses a diverging version of the check.

Jonathan, more context is here:

  <https://www.openwall.com/lists/musl/2020/11/22/1>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [musl] Mutexes are not unlocking
  2020-11-22 19:23           ` Florian Weimer
@ 2020-11-22 19:28             ` Rich Felker
  2020-11-22 19:45               ` Re[2]: " Арсений
                                 ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Rich Felker @ 2020-11-22 19:28 UTC (permalink / raw)
  To: Florian Weimer
  Cc: Арсений, musl, jwakely

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
> going to run into a similar issue because inlined code from older GCC
> versions uses a diverging version of the check.
> 
> 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?

Rich

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re[2]: [musl] Mutexes are not unlocking
  2020-11-22 19:28             ` Rich Felker
@ 2020-11-22 19:45               ` Арсений
  2020-11-22 20:05               ` Арсений
  2020-11-23 11:41               ` Jonathan Wakely
  2 siblings, 0 replies; 20+ messages in thread
From: Арсений @ 2020-11-22 19:45 UTC (permalink / raw)
  To: Rich Felker; +Cc: musl, jwakely, Florian Weimer

[-- Attachment #1: Type: text/plain, Size: 2269 bytes --]


Hello,
 
Yes, it is inlined:
  0x00007ffff7e0e3e9 <+88>:    mov    0x3b98(%rip),%r12        # 0x7ffff7e11f88 
  0x00007ffff7e0e3f0 <+95>:    test   %r12,%r12 
  0x00007ffff7e0e3f3 <+98>:    je     0x7ffff7e0e3fd <testMutexPP(std::mutex&, char const*)+108> 
  0x00007ffff7e0e3f5 <+100>:   mov    %rbp,%rdi 
  0x00007ffff7e0e3f8 <+103>:   callq  0x7ffff7e0d270 <pthread_mutex_unlock@plt>
Weak reference to pthread_key_create() is stored at 0x7ffff7e11f88. On musl, this memory location contains 0x0. 
>Воскресенье, 22 ноября 2020, 22:28 +03:00 от Rich Felker <dalias@libc.org>:
> 
>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
>> going to run into a similar issue because inlined code from older GCC
>> versions uses a diverging version of the check.
>>
>> 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?
>
>Rich 
 
 
——
Арсений
 

[-- Attachment #2: Type: text/html, Size: 4457 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re[2]: [musl] Mutexes are not unlocking
  2020-11-22 19:28             ` Rich Felker
  2020-11-22 19:45               ` Re[2]: " Арсений
@ 2020-11-22 20:05               ` Арсений
  2020-11-23 12:24                 ` Jonathan Wakely
  2020-11-23 11:41               ` Jonathan Wakely
  2 siblings, 1 reply; 20+ messages in thread
From: Арсений @ 2020-11-22 20:05 UTC (permalink / raw)
  To: Rich Felker; +Cc: musl, jwakely, Florian Weimer

[-- Attachment #1: Type: text/plain, Size: 1918 bytes --]


Hello,
 
I fixed the problem by making a workaround. Specifying -D_GLIBCXX_GTHREAD_USE_WEAK=0 forces libstdc++ headers do not use weak symbols. Mutexes are correctly locked and unlocked now. 
>Воскресенье, 22 ноября 2020, 22:28 +03:00 от Rich Felker <dalias@libc.org>:
> 
>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
>> going to run into a similar issue because inlined code from older GCC
>> versions uses a diverging version of the check.
>>
>> 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?
>
>Rich 
 
 
——
Арсений
 

[-- Attachment #2: Type: text/html, Size: 2782 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [musl] Mutexes are not unlocking
  2020-11-22 19:28             ` Rich Felker
  2020-11-22 19:45               ` Re[2]: " Арсений
  2020-11-22 20:05               ` Арсений
@ 2020-11-23 11:41               ` Jonathan Wakely
  2020-11-23 14:53                 ` Rich Felker
  2 siblings, 1 reply; 20+ messages in thread
From: Jonathan Wakely @ 2020-11-23 11:41 UTC (permalink / raw)
  To: Rich Felker
  Cc: Florian Weimer, Арсений, musl

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).




^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [musl] Mutexes are not unlocking
  2020-11-22 20:05               ` Арсений
@ 2020-11-23 12:24                 ` Jonathan Wakely
  2020-11-23 14:56                   ` Rich Felker
  0 siblings, 1 reply; 20+ messages in thread
From: Jonathan Wakely @ 2020-11-23 12:24 UTC (permalink / raw)
  To: Арсений
  Cc: Rich Felker, musl, Florian Weimer

On 22/11/20 23:05 +0300, Арсений wrote:
>
>Hello,
> 
>I fixed the problem by making a workaround. Specifying -D_GLIBCXX_GTHREAD_USE_WEAK=0 forces libstdc++ headers do not use weak symbols. Mutexes are correctly locked and unlocked now.

That might "work" but is unsupported, because that macro is for
libstdc++'s internal use, not for users to define/undefine.

But then libstdc++ doesn't support being compiled/linked against a
glibc libstdc++ and then running against musl at runtime (I didn't
even know that was an option until today) so one more unsupported
thing probably won't hurt :-)

Maybe we should just bless the use of that macro as supported, which
would solve https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89714



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [musl] Mutexes are not unlocking
  2020-11-23 11:41               ` Jonathan Wakely
@ 2020-11-23 14:53                 ` Rich Felker
  2020-11-23 16:19                   ` Jonathan Wakely
  0 siblings, 1 reply; 20+ messages in thread
From: Rich Felker @ 2020-11-23 14:53 UTC (permalink / raw)
  To: Jonathan Wakely
  Cc: Florian Weimer, Арсений, musl

On Mon, Nov 23, 2020 at 11:41:24AM +0000, Jonathan Wakely wrote:
> 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

The potential problem is that removing the condition makes an
inconsistency with binaries where the condition was already inlined.

> 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).

The target files for -musl tuples disable the hack entirely by adding
t-gthr-noweak and -DGTHREAD_USE_WEAK=0 where appropriate. So there's
no issue here. OP's problem was trying to make glibc-linked binaries
and use them with musl's very limited glibc-ABI-compat capabilities,
which is not recommended; it's only intended as an aid in running
binaryware (esp shared libraries) you can't build/get native versions
of.

Rich

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [musl] Mutexes are not unlocking
  2020-11-23 12:24                 ` Jonathan Wakely
@ 2020-11-23 14:56                   ` Rich Felker
  2020-11-23 16:58                     ` Jonathan Wakely
  0 siblings, 1 reply; 20+ messages in thread
From: Rich Felker @ 2020-11-23 14:56 UTC (permalink / raw)
  To: Jonathan Wakely
  Cc: Арсений, musl, Florian Weimer

On Mon, Nov 23, 2020 at 12:24:28PM +0000, Jonathan Wakely wrote:
> On 22/11/20 23:05 +0300, Арсений wrote:
> >
> >Hello,
> > 
> >I fixed the problem by making a workaround. Specifying -D_GLIBCXX_GTHREAD_USE_WEAK=0 forces libstdc++ headers do not use weak symbols. Mutexes are correctly locked and unlocked now.
> 
> That might "work" but is unsupported, because that macro is for
> libstdc++'s internal use, not for users to define/undefine.
> 
> But then libstdc++ doesn't support being compiled/linked against a
> glibc libstdc++ and then running against musl at runtime (I didn't
> even know that was an option until today) so one more unsupported
> thing probably won't hurt :-)
> 
> Maybe we should just bless the use of that macro as supported, which
> would solve https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89714

As you noted in comment 2, that won't fix the uses internal to
libstdc++.{so,a}, only the inlined ones. I think this could give the
wrong behavior in the opposite direction -- calling unlock without
lock, thereby causing an error (for error-checking mutexes) or trap
(if UB catching traps are in place for other types).

Rich

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [musl] Mutexes are not unlocking
  2020-11-23 14:53                 ` Rich Felker
@ 2020-11-23 16:19                   ` Jonathan Wakely
  2020-11-23 16:51                     ` Rich Felker
  2020-11-23 16:59                     ` Florian Weimer
  0 siblings, 2 replies; 20+ messages in thread
From: Jonathan Wakely @ 2020-11-23 16:19 UTC (permalink / raw)
  To: Rich Felker
  Cc: Florian Weimer, Арсений, musl

On 23/11/20 09:53 -0500, Rich Felker wrote:
>On Mon, Nov 23, 2020 at 11:41:24AM +0000, Jonathan Wakely wrote:
>> 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
>
>The potential problem is that removing the condition makes an
>inconsistency with binaries where the condition was already inlined.

But if the result of the condition is always true, it doesn't matter.
The code that doesn't check the condition assumes it's true, and the
code that already inlined the condition does a runtime check which
will be true. The end result is the same.

>> 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).
>
>The target files for -musl tuples disable the hack entirely by adding
>t-gthr-noweak and -DGTHREAD_USE_WEAK=0 where appropriate. So there's
>no issue here. OP's problem was trying to make glibc-linked binaries
>and use them with musl's very limited glibc-ABI-compat capabilities,
>which is not recommended; it's only intended as an aid in running
>binaryware (esp shared libraries) you can't build/get native versions
>of.

OK. If supporting that not recommended case does become desirable,
defining a __pthread_key_create symbol somewhere should work.

Or users might be able to solve it for themselves by creating a tiny
shared library that contains nothing but a symbol called
__pthread_key_create and LD_PRELOADing that, so that the checks in the
binaryware looking for that symbol become true.



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [musl] Mutexes are not unlocking
  2020-11-23 16:19                   ` Jonathan Wakely
@ 2020-11-23 16:51                     ` Rich Felker
  2020-11-23 17:10                       ` Jonathan Wakely
  2020-11-23 16:59                     ` Florian Weimer
  1 sibling, 1 reply; 20+ messages in thread
From: Rich Felker @ 2020-11-23 16:51 UTC (permalink / raw)
  To: Jonathan Wakely
  Cc: Florian Weimer, Арсений, musl

On Mon, Nov 23, 2020 at 04:19:07PM +0000, Jonathan Wakely wrote:
> On 23/11/20 09:53 -0500, Rich Felker wrote:
> >On Mon, Nov 23, 2020 at 11:41:24AM +0000, Jonathan Wakely wrote:
> >>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
> >
> >The potential problem is that removing the condition makes an
> >inconsistency with binaries where the condition was already inlined.
> 
> But if the result of the condition is always true, it doesn't matter.
> The code that doesn't check the condition assumes it's true, and the
> code that already inlined the condition does a runtime check which
> will be true. The end result is the same.

The condition is false if you static link and don't use
pthread_key_create in your program. I think the problem would only
arise if you have old object files with new libstdc++.a or vice versa
though, but this could reasonably happen with binaryware libraries.

> >>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).
> >
> >The target files for -musl tuples disable the hack entirely by adding
> >t-gthr-noweak and -DGTHREAD_USE_WEAK=0 where appropriate. So there's
> >no issue here. OP's problem was trying to make glibc-linked binaries
> >and use them with musl's very limited glibc-ABI-compat capabilities,
> >which is not recommended; it's only intended as an aid in running
> >binaryware (esp shared libraries) you can't build/get native versions
> >of.
> 
> OK. If supporting that not recommended case does become desirable,
> defining a __pthread_key_create symbol somewhere should work.
> 
> Or users might be able to solve it for themselves by creating a tiny
> shared library that contains nothing but a symbol called
> __pthread_key_create and LD_PRELOADing that, so that the checks in the
> binaryware looking for that symbol become true.

My understanding was that only static linking was broken by the weak
ref hack, so I'm not sure why it's happening to begin with. Is
libstdc++ actually looking for __pthread_key_create rather than
pthread_key_create? That could explain it since we don't provide the
former (and as responsibility for making this stuff work is shifted to
gcompat, gcompat could provide it).

Rich

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [musl] Mutexes are not unlocking
  2020-11-23 14:56                   ` Rich Felker
@ 2020-11-23 16:58                     ` Jonathan Wakely
  0 siblings, 0 replies; 20+ messages in thread
From: Jonathan Wakely @ 2020-11-23 16:58 UTC (permalink / raw)
  To: Rich Felker
  Cc: Арсений, musl, Florian Weimer

On 23/11/20 09:56 -0500, Rich Felker wrote:
>On Mon, Nov 23, 2020 at 12:24:28PM +0000, Jonathan Wakely wrote:
>> On 22/11/20 23:05 +0300, Арсений wrote:
>> >
>> >Hello,
>> > 
>> >I fixed the problem by making a workaround. Specifying -D_GLIBCXX_GTHREAD_USE_WEAK=0 forces libstdc++ headers do not use weak symbols. Mutexes are correctly locked and unlocked now.
>>
>> That might "work" but is unsupported, because that macro is for
>> libstdc++'s internal use, not for users to define/undefine.
>>
>> But then libstdc++ doesn't support being compiled/linked against a
>> glibc libstdc++ and then running against musl at runtime (I didn't
>> even know that was an option until today) so one more unsupported
>> thing probably won't hurt :-)
>>
>> Maybe we should just bless the use of that macro as supported, which
>> would solve https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89714
>
>As you noted in comment 2, that won't fix the uses internal to
>libstdc++.{so,a}, only the inlined ones. I think this could give the
>wrong behavior in the opposite direction -- calling unlock without
>lock, thereby causing an error (for error-checking mutexes) or trap
>(if UB catching traps are in place for other types).

I've just had a quick look, and (for linux targets) the only case I
found where there's a lock outside the library (in user code) and a
corresponding unlock inside the library, or vice versa, is in
std::notify_all_at_thread_exit. (I only looked very quickly, so there
might be other cases I missed).

It does look like it would be possible for user code to lock the mutex
(via a direct all to the non-weak pthread_mutex_lock) and then call
std::notify_all_at_thread_exit() which unlock the mutex, which would
test __gthread_active_p, which would incorrectly think the program is
single-threaded, and not call pthread_mutex_unlock.

So recommending -D_GLIBCXX_GTHREAD_USE_WEAK=0 is a bad idea. We would
need a better way for users to override the checks.



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [musl] Mutexes are not unlocking
  2020-11-23 16:19                   ` Jonathan Wakely
  2020-11-23 16:51                     ` Rich Felker
@ 2020-11-23 16:59                     ` Florian Weimer
  1 sibling, 0 replies; 20+ messages in thread
From: Florian Weimer @ 2020-11-23 16:59 UTC (permalink / raw)
  To: Jonathan Wakely
  Cc: Rich Felker, Арсений, musl

* Jonathan Wakely:

> Or users might be able to solve it for themselves by creating a tiny
> shared library that contains nothing but a symbol called
> __pthread_key_create and LD_PRELOADing that, so that the checks in the
> binaryware looking for that symbol become true.

Typically, that doesn't work for main programs because the link editor
will resolve weak unexported symbols to zero.  There might not be a
dynamic symbol entry for __pthread_key_create in this case.  Shared
objects are different in this regard.

I think interoperability for these mismatches has to come directly
from the mutex implementation.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [musl] Mutexes are not unlocking
  2020-11-23 16:51                     ` Rich Felker
@ 2020-11-23 17:10                       ` Jonathan Wakely
  2020-11-23 17:18                         ` Florian Weimer
  0 siblings, 1 reply; 20+ messages in thread
From: Jonathan Wakely @ 2020-11-23 17:10 UTC (permalink / raw)
  To: Rich Felker
  Cc: Florian Weimer, Арсений, musl

On 23/11/20 11:51 -0500, Rich Felker wrote:
>On Mon, Nov 23, 2020 at 04:19:07PM +0000, Jonathan Wakely wrote:
>> On 23/11/20 09:53 -0500, Rich Felker wrote:
>> >On Mon, Nov 23, 2020 at 11:41:24AM +0000, Jonathan Wakely wrote:
>> >>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).
>> >
>> >The target files for -musl tuples disable the hack entirely by adding
>> >t-gthr-noweak and -DGTHREAD_USE_WEAK=0 where appropriate. So there's
>> >no issue here. OP's problem was trying to make glibc-linked binaries
>> >and use them with musl's very limited glibc-ABI-compat capabilities,
>> >which is not recommended; it's only intended as an aid in running
>> >binaryware (esp shared libraries) you can't build/get native versions
>> >of.
>>
>> OK. If supporting that not recommended case does become desirable,
>> defining a __pthread_key_create symbol somewhere should work.
>>
>> Or users might be able to solve it for themselves by creating a tiny
>> shared library that contains nothing but a symbol called
>> __pthread_key_create and LD_PRELOADing that, so that the checks in the
>> binaryware looking for that symbol become true.
>
>My understanding was that only static linking was broken by the weak
>ref hack, so I'm not sure why it's happening to begin with. Is
>libstdc++ actually looking for __pthread_key_create rather than
>pthread_key_create?

Yes. I don't remember why but maybe pthread_key_create exists as a
stub in glibc's libc.so and so __pthread_key_create is used to
**really** detect that libpthread.so is linked in.

>That could explain it since we don't provide the
>former (and as responsibility for making this stuff work is shifted to
>gcompat, gcompat could provide it).

Ah, if you already have a library for glibc-compatibility then yes,
adding __pthread_key_create there would seem to make sense.



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [musl] Mutexes are not unlocking
  2020-11-23 17:10                       ` Jonathan Wakely
@ 2020-11-23 17:18                         ` Florian Weimer
  0 siblings, 0 replies; 20+ messages in thread
From: Florian Weimer @ 2020-11-23 17:18 UTC (permalink / raw)
  To: Jonathan Wakely
  Cc: Rich Felker, Арсений, musl

* Jonathan Wakely:

> Yes. I don't remember why but maybe pthread_key_create exists as a
> stub in glibc's libc.so and so __pthread_key_create is used to
> **really** detect that libpthread.so is linked in.

There's no pthread_key_create in libc.  Roland actually added a
comment why __pthread_key_create is used:

+/* For a program to be multi-threaded the only thing that it certainly must
+   be using is pthread_create.  However, there may be other libraries that
+   intercept pthread_create with their own definitions to wrap pthreads
+   functionality for some purpose.  In those cases, pthread_create being
+   defined might not necessarily mean that libpthread is actually linked
+   in.

I believe the Boehm/Demers/Weiser garbage collector used to be one of
these libraries.

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2020-11-23 17:19 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-20  5:25 [musl] Mutexes are not unlocking a
2020-11-20  5:58 ` Rich Felker
2020-11-21  6:46   ` Re[2]: " a
2020-11-21 15:51     ` Rich Felker
2020-11-22 18:43       ` Re[2]: " Арсений
2020-11-22 19:11         ` Rich Felker
2020-11-22 19:23           ` Florian Weimer
2020-11-22 19:28             ` Rich Felker
2020-11-22 19:45               ` Re[2]: " Арсений
2020-11-22 20:05               ` Арсений
2020-11-23 12:24                 ` Jonathan Wakely
2020-11-23 14:56                   ` Rich Felker
2020-11-23 16:58                     ` Jonathan Wakely
2020-11-23 11:41               ` Jonathan Wakely
2020-11-23 14:53                 ` Rich Felker
2020-11-23 16:19                   ` Jonathan Wakely
2020-11-23 16:51                     ` Rich Felker
2020-11-23 17:10                       ` Jonathan Wakely
2020-11-23 17:18                         ` Florian Weimer
2020-11-23 16:59                     ` Florian Weimer

mailing list of musl libc

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.vuxu.org/musl

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 musl musl/ http://inbox.vuxu.org/musl \
		musl@inbox.vuxu.org
	public-inbox-index musl

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.vuxu.org/vuxu.archive.musl


code repositories for the project(s) associated with this inbox:

	https://git.vuxu.org/mirror/musl/

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git