Re: gthread_once

Re: gthread_once

[Michael]

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

Essentially my plan is to compile my console only app for android but using
the musl library instead of bionic. Im compling for standard x86_64 linux
first to see if its viable. My app is plain c++ using wxwidgets (base only)
and boost threads/system. I compiled these dependencies using CC/CXX/AR etc
vars set to x86_64-linux-musl-gcc etc... and they compile fine and appear
to be only linking to musl libraries as i used -v on the compliation and
watched the directories it pulls in.

Could the pthread_once be something to do with this thread
http://sourceware-org.1504.n7.nabble.com/PATCH-Provide-pthread-atfork-in-libc-nonshared-a-and-libc-a-td246012.html#a247866


Runing file against my binary is:
./vhui: ELF 64-bit LSB  executable, x86-64, version 1 (SYSV), statically
linked, not stripped

If i run readelf -hld:

ELF Header:
  Magic:   7f 45 4c 46 02 01 01 00 00 00 00 00 00 00 00 00
  Class:                             ELF64
  Data:                              2's complement, little endian
  Version:                           1 (current)
  OS/ABI:                            UNIX - System V
  ABI Version:                       0
  Type:                              EXEC (Executable file)
  Machine:                           Advanced Micro Devices X86-64
  Version:                           0x1
  Entry point address:               0x403309
  Start of program headers:          64 (bytes into file)
  Start of section headers:          14776944 (bytes into file)
  Flags:                             0x0
  Size of this header:               64 (bytes)
  Size of program headers:           56 (bytes)
  Number of program headers:         4
  Size of section headers:           64 (bytes)
  Number of section headers:         28
  Section header string table index: 25

Program Headers:
  Type           Offset             VirtAddr           PhysAddr
                 FileSiz            MemSiz              Flags  Align
  LOAD           0x0000000000000000 0x0000000000400000 0x0000000000400000
                 0x0000000000417b4c 0x0000000000417b4c  R E    200000
  LOAD           0x0000000000417b50 0x0000000000a17b50 0x0000000000a17b50
                 0x0000000000014b98 0x0000000000037168  RW     200000
  TLS            0x0000000000417b50 0x0000000000a17b50 0x0000000000a17b50
                 0x0000000000000000 0x0000000000000018  R      8
  GNU_STACK      0x0000000000000000 0x0000000000000000 0x0000000000000000
                 0x0000000000000000 0x0000000000000000  RW     10

 Section to Segment mapping:
  Segment Sections...
   00     .init .text .fini .rodata .eh_frame .gcc_except_table
   01     .ctors .dtors .jcr .data.rel.ro .got .got.plt .data .bss
   02     .tbss
   03

There is no dynamic section in this file.

---------- Forwarded message ----------
> From: Rich Felker <dalias@libc.org>
> To: musl@lists.openwall.com
> Cc:
> Date: Fri, 17 Oct 2014 08:10:48 -0400
> Subject: Re: [musl] Re: gthread_once
> On Fri, Oct 17, 2014 at 01:42:23PM +0200, Jens Gustedt wrote:
> > Hello,
> >
> > Am Freitag, den 17.10.2014, 21:36 +1100 schrieb Michael:
> > > I'm not linking to glibc. gthread is a thin wrapper over pthread used
> > > by gcc (i.e https://gcc.gnu.org/onlinedocs/libstdc
> > > ++/manual/ext_concurrency_impl.html).
> > >
> > >
> > > It seems my problem is related to this:
> > >
> > >
> > > http://www.riscos.info/pipermail/gcc/2008-July/004525.html
> > >
> > >
> > >
> > > I have compiled g++ toolchain using musl-1.1.5
> > >
> > >
> > > Is this a bug in musl or do i need to turn off the _GTHREAD in the
> > > libstdc++ library?
> >
> > If you are really sure that your whole toolchain is built with musl,
> > things like that shouldn't happen. My guess would be that there still
> > is some inconsistency somewhere and a glibc version of pthreads is
> > loaded before musl. It could help if you'd compile your libraries with
> > debugging symbols so we could see where (which function and which
> > version) this happens.
>
> This sounds unlikely. He's static linking, so there is no separate
> loading of libraries, but even if there were, musl's dynamic linker
> refuses to load a library named libpthread.*.
>
> Still, I think it would be helpful to see some indication of whether
> the binary is actually a static binary that was created correctly. The
> output of the "file" utility run on it, and possibly the output of
> readelf -hld or even full readelf -a (although the latter might reveal
> a lot about the program and be big).
>
> > This gthread stuff doesn't seem to be too complicated. It *should*
> > just generate calls to the corresponding pthread functions, but
> > obviously here for you it doesn't. Your stack in the __gthread_once
> > function seems to be corrupted.
>
> It looks like it called a function via a function pointer that
> happened to be null.
>
> > Two fishy things:
> >
> >   - these are static inline functions, so to use this library you have
> >     to use the same pthread implementation for all compilations of
> >     application code. If anything in your tool chain goes wrong, your
> >     screwed.
> >
> >   - right at the start it seems to rely on certain features of the
> >     glibc implementation concerning weak symbols.
> >
> > This seems to be handled with the macros
> >
> > __GXX_WEAK__ && _GLIBCXX_GTHREAD_USE_WEAK
> >
> > It could be a starting point to see how they are set and to play at
> > bit with them.
>
> These sound problematic, but if there's something wrong here, I'd be
> surprised that we haven't seen failures before.
>
> Rich
>
>

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

Re: Re: gthread_once

[Szabolcs Nagy]

* Michael <msbroadf@gmail.com> [2014-10-18 13:16:52 +1100]:

> Essentially my plan is to compile my console only app for android but using
> the musl library instead of bionic. Im compling for standard x86_64 linux
> first to see if its viable. My app is plain c++ using wxwidgets (base only)
> and boost threads/system. I compiled these dependencies using CC/CXX/AR etc
> vars set to x86_64-linux-musl-gcc etc... and they compile fine and appear
> to be only linking to musl libraries as i used -v on the compliation and
> watched the directories it pulls in.
> 
> Could the pthread_once be something to do with this thread
> http://sourceware-org.1504.n7.nabble.com/PATCH-Provide-pthread-atfork-in-libc-nonshared-a-and-libc-a-td246012.html#a247866
> 

that's a different glibc specific problem: they have separate
libpthread which makes a lot of things hard so they do various
workarounds

in your case it seems libstdc++ uses the gthr-posix.h from libgcc
that provides the __gthread_once function which is supposed to call
pthread_once through a weak reference, but the weak ref is 0
(so uninitialized, eventhough musl have the pthread_once symbol)

i'd check

 nm libstdc++.a |grep pthread_once

you should see undefined weak symbols (w) i think

and then at link time these should resolve to the pthread_once
in libc.a

Re: Re: gthread_once

[Rich Felker]

On Sat, Oct 18, 2014 at 12:18:13PM +0200, Szabolcs Nagy wrote:
> * Michael <msbroadf@gmail.com> [2014-10-18 13:16:52 +1100]:
> 
> > Essentially my plan is to compile my console only app for android but using
> > the musl library instead of bionic. Im compling for standard x86_64 linux
> > first to see if its viable. My app is plain c++ using wxwidgets (base only)
> > and boost threads/system. I compiled these dependencies using CC/CXX/AR etc
> > vars set to x86_64-linux-musl-gcc etc... and they compile fine and appear
> > to be only linking to musl libraries as i used -v on the compliation and
> > watched the directories it pulls in.
> > 
> > Could the pthread_once be something to do with this thread
> > http://sourceware-org.1504.n7.nabble.com/PATCH-Provide-pthread-atfork-in-libc-nonshared-a-and-libc-a-td246012.html#a247866
> > 
> 
> that's a different glibc specific problem: they have separate
> libpthread which makes a lot of things hard so they do various
> workarounds
> 
> in your case it seems libstdc++ uses the gthr-posix.h from libgcc
> that provides the __gthread_once function which is supposed to call
> pthread_once through a weak reference, but the weak ref is 0
> (so uninitialized, eventhough musl have the pthread_once symbol)
> 
> i'd check
> 
>  nm libstdc++.a |grep pthread_once
> 
> you should see undefined weak symbols (w) i think
> 
> and then at link time these should resolve to the pthread_once
> in libc.a

If I understand the explanation of what's going on correctly, it's a
huge bug in libstdc++, and exactly the same as the one that's known
(and that we've discussed on IRC, etc.) many times in libxml2:

https://bugzilla.gnome.org/show_bug.cgi?id=704904

It looks to me like gcc is expecting dynamic-linking-like and/or
bloatware-libpthread-like behavior, where if any one of the pthread
symbols has a definition, they all do. This is definitely NOT the case
with static linking, especially with musl. It's also not the case with
glibc, though they have more indirect dependencies that cause a large
portion (but not all) of libpthread to be pulled in when you use any
one part, and some distros (at least Redhat ones, IIRC) link their
libpthread.a with a hack to merge all the modules into one giant .o
file inside the .a precisely to work around bugs like this.

If I'm correct, just removing all the weak attributes from the
gthr-posix.h stuff should fix the bug.

Rich

Re: Re: gthread_once

[Szabolcs Nagy]

* Rich Felker <dalias@libc.org> [2014-10-18 11:45:18 -0400]:
> 
> If I understand the explanation of what's going on correctly, it's a
> huge bug in libstdc++, and exactly the same as the one that's known
> (and that we've discussed on IRC, etc.) many times in libxml2:
> 
> https://bugzilla.gnome.org/show_bug.cgi?id=704904
> 
> It looks to me like gcc is expecting dynamic-linking-like and/or
> bloatware-libpthread-like behavior, where if any one of the pthread
> symbols has a definition, they all do. This is definitely NOT the case
> with static linking, especially with musl. It's also not the case with
> glibc, though they have more indirect dependencies that cause a large
> portion (but not all) of libpthread to be pulled in when you use any
> one part, and some distros (at least Redhat ones, IIRC) link their
> libpthread.a with a hack to merge all the modules into one giant .o
> file inside the .a precisely to work around bugs like this.
> 
> If I'm correct, just removing all the weak attributes from the
> gthr-posix.h stuff should fix the bug.

yes this is the bug

it turns out gthr-posix.h decides if there are threads
if pthread_cancel is present (weak ref is non-null)
(on glibc it checks pthread_key_create and on bionic it
checks pthread_create)

if there is no pthread_cancel then libstdc++ will assume
no threads and __gthread calls fail (eg iostream locks
are skipped, in call_once a system_error is thrown)

if there is pthread_cancel then a __gthread_something call
will call pthread_something and if that is otherwise
unreferenced it will crash (in case of static linking)

example that crases here whit -static:

#include <iostream>
#include <pthread.h>
int (*f)(pthread_t) = pthread_cancel;
int main()
{
	std::cout << "";
}

possible workaround: create a .o that references all
pthread_* functions and link it into your app
(this is the bloatware-libpthread solution until libstdc++ is fixed)

Re: Re: gthread_once

[Rich Felker]

On Sat, Oct 18, 2014 at 08:14:30PM +0200, Szabolcs Nagy wrote:
> * Rich Felker <dalias@libc.org> [2014-10-18 11:45:18 -0400]:
> > 
> > If I understand the explanation of what's going on correctly, it's a
> > huge bug in libstdc++, and exactly the same as the one that's known
> > (and that we've discussed on IRC, etc.) many times in libxml2:
> > 
> > https://bugzilla.gnome.org/show_bug.cgi?id=704904
> > 
> > It looks to me like gcc is expecting dynamic-linking-like and/or
> > bloatware-libpthread-like behavior, where if any one of the pthread
> > symbols has a definition, they all do. This is definitely NOT the case
> > with static linking, especially with musl. It's also not the case with
> > glibc, though they have more indirect dependencies that cause a large
> > portion (but not all) of libpthread to be pulled in when you use any
> > one part, and some distros (at least Redhat ones, IIRC) link their
> > libpthread.a with a hack to merge all the modules into one giant .o
> > file inside the .a precisely to work around bugs like this.
> > 
> > If I'm correct, just removing all the weak attributes from the
> > gthr-posix.h stuff should fix the bug.
> 
> yes this is the bug
> 
> it turns out gthr-posix.h decides if there are threads
> if pthread_cancel is present (weak ref is non-null)
> (on glibc it checks pthread_key_create and on bionic it
> checks pthread_create)
> 
> if there is no pthread_cancel then libstdc++ will assume
> no threads and __gthread calls fail (eg iostream locks
> are skipped, in call_once a system_error is thrown)
> 
> if there is pthread_cancel then a __gthread_something call
> will call pthread_something and if that is otherwise
> unreferenced it will crash (in case of static linking)

There seem to be two flaws involved here:

First is the use of weak references to determine if a program is
"potentially multithreaded" in order to optimize out synchronization
when it's not. This is why nsz had to work to find a trick to
reproduce the bug. There is no reason to think that pthread_key_create
or pthread_cancel is necessarily linked in a multithreaded program, or
that they won't be linked in non-multithreaded programs. Testing for
pthread_create is actually close to valid -- if they don't care about
the case where pthread_create is not present in the main program but
loaded dynamically via dlopen. (glibc explicitly does not support such
usage, and it can't happen in musl since musl does not support dlopen
in static programs and always has pthread_create present in
dynamic-linked ones.) To make it more correct, the condition checked
should be pthread_create||thrd_create, since a program could become
multithreaded via C11 threads rather than POSIX threads.

The second issue is the use of weak references to functions which are
actually needed. This is identical to the libxml2 bug I cited before:

https://bugzilla.gnome.org/show_bug.cgi?id=704904

Using a weak reference is only valid for purposes like "if anything
else in the program uses function FOO, we need to use function FOO
here too". It's not valid for the seemingly similar purpose of "if
anything else in the program uses the library containing FOO, we need
to use FOO here". As an implementation detail, this latter pattern
happens to work for current implementations of dynamic linking, but
it's completely incorrect for static linking and could theoretically
be wrong in the future for dynamic linking too.

> example that crases here whit -static:
> 
> #include <iostream>
> #include <pthread.h>
> int (*f)(pthread_t) = pthread_cancel;
> int main()
> {
> 	std::cout << "";
> }
> 
> possible workaround: create a .o that references all
> pthread_* functions and link it into your app
> (this is the bloatware-libpthread solution until libstdc++ is fixed)

Can't it be fixed just by changing gthr-posix.h not to use weak
references ever, and rebuilding gcc's target libs (or all of gcc)?

Rich

Re: Re: gthread_once

[Jens Gustedt]

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

Am Samstag, den 18.10.2014, 15:02 -0400 schrieb Rich Felker:
> Can't it be fixed just by changing gthr-posix.h not to use weak
> references ever, and rebuilding gcc's target libs (or all of gcc)?

Looking how this done for that gthread thing, __GXX_WEAK__ could be a
good candidate to tweak. Hopefully this used consistently in other
parts of the library.

Jens

-- 
:: INRIA Nancy Grand Est ::: AlGorille ::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536   ::
:: :::::::::::::::::::::: gsm France : +33 651400183   ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::





[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: Re: gthread_once

[Michael]

[-- Attachment #1.1: Type: text/plain, Size: 891 bytes --]

Thanks guys, i added my patch (attached) to musl-cross build system
(patches directory) and that fixed the problem.


On Sun, Oct 19, 2014 at 6:25 AM, Jens Gustedt <jens.gustedt@inria.fr> wrote:

> Am Samstag, den 18.10.2014, 15:02 -0400 schrieb Rich Felker:
> > Can't it be fixed just by changing gthr-posix.h not to use weak
> > references ever, and rebuilding gcc's target libs (or all of gcc)?
>
> Looking how this done for that gthread thing, __GXX_WEAK__ could be a
> good candidate to tweak. Hopefully this used consistently in other
> parts of the library.
>
> Jens
>
> --
> :: INRIA Nancy Grand Est ::: AlGorille ::: ICube/ICPS :::
> :: ::::::::::::::: office Strasbourg : +33 368854536   ::
> :: :::::::::::::::::::::: gsm France : +33 651400183   ::
> :: ::::::::::::::: gsm international : +49 15737185122 ::
> :: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::
>
>
>
>
>

[-- Attachment #1.2: Type: text/html, Size: 1436 bytes --]

[-- Attachment #2: gcc-4.8.3-gthread.diff --]
[-- Type: text/plain, Size: 473 bytes --]

--- a/libgcc/gthr.h	2014-10-19 09:38:42.878238566 +1100
+++ b/libgcc/gthr.h	2014-10-19 09:50:42.893740340 +1100
@@ -136,10 +136,8 @@
 /* The pe-coff weak support isn't fully compatible to ELF's weak.
    For static libraries it might would work, but as we need to deal
    with shared versions too, we disable it for mingw-targets.  */
-#ifdef __MINGW32__
 #undef GTHREAD_USE_WEAK
 #define GTHREAD_USE_WEAK 0
-#endif
 
 #ifndef GTHREAD_USE_WEAK
 #define GTHREAD_USE_WEAK 1

Re: Re: gthread_once

[Rich Felker]

On Fri, Oct 17, 2014 at 01:42:23PM +0200, Jens Gustedt wrote:
> Hello,
> 
> Am Freitag, den 17.10.2014, 21:36 +1100 schrieb Michael:
> > I'm not linking to glibc. gthread is a thin wrapper over pthread used
> > by gcc (i.e https://gcc.gnu.org/onlinedocs/libstdc
> > ++/manual/ext_concurrency_impl.html).
> > 
> > 
> > It seems my problem is related to this:
> > 
> > 
> > http://www.riscos.info/pipermail/gcc/2008-July/004525.html
> > 
> > 
> > 
> > I have compiled g++ toolchain using musl-1.1.5
> > 
> > 
> > Is this a bug in musl or do i need to turn off the _GTHREAD in the
> > libstdc++ library?
> 
> If you are really sure that your whole toolchain is built with musl,
> things like that shouldn't happen. My guess would be that there still
> is some inconsistency somewhere and a glibc version of pthreads is
> loaded before musl. It could help if you'd compile your libraries with
> debugging symbols so we could see where (which function and which
> version) this happens.

This sounds unlikely. He's static linking, so there is no separate
loading of libraries, but even if there were, musl's dynamic linker
refuses to load a library named libpthread.*.

Still, I think it would be helpful to see some indication of whether
the binary is actually a static binary that was created correctly. The
output of the "file" utility run on it, and possibly the output of
readelf -hld or even full readelf -a (although the latter might reveal
a lot about the program and be big).

> This gthread stuff doesn't seem to be too complicated. It *should*
> just generate calls to the corresponding pthread functions, but
> obviously here for you it doesn't. Your stack in the __gthread_once
> function seems to be corrupted.

It looks like it called a function via a function pointer that
happened to be null.

> Two fishy things:
> 
>   - these are static inline functions, so to use this library you have
>     to use the same pthread implementation for all compilations of
>     application code. If anything in your tool chain goes wrong, your
>     screwed.
> 
>   - right at the start it seems to rely on certain features of the
>     glibc implementation concerning weak symbols.
> 
> This seems to be handled with the macros
> 
> __GXX_WEAK__ && _GLIBCXX_GTHREAD_USE_WEAK
> 
> It could be a starting point to see how they are set and to play at
> bit with them.

These sound problematic, but if there's something wrong here, I'd be
surprised that we haven't seen failures before.

Rich

Re: Re: gthread_once

[Jens Gustedt]

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

Hello,

Am Freitag, den 17.10.2014, 21:36 +1100 schrieb Michael:
> I'm not linking to glibc. gthread is a thin wrapper over pthread used
> by gcc (i.e https://gcc.gnu.org/onlinedocs/libstdc
> ++/manual/ext_concurrency_impl.html).
> 
> 
> It seems my problem is related to this:
> 
> 
> http://www.riscos.info/pipermail/gcc/2008-July/004525.html
> 
> 
> 
> I have compiled g++ toolchain using musl-1.1.5
> 
> 
> Is this a bug in musl or do i need to turn off the _GTHREAD in the
> libstdc++ library?

If you are really sure that your whole toolchain is built with musl,
things like that shouldn't happen. My guess would be that there still
is some inconsistency somewhere and a glibc version of pthreads is
loaded before musl. It could help if you'd compile your libraries with
debugging symbols so we could see where (which function and which
version) this happens.

This gthread stuff doesn't seem to be too complicated. It *should*
just generate calls to the corresponding pthread functions, but
obviously here for you it doesn't. Your stack in the __gthread_once
function seems to be corrupted.

Two fishy things:

  - these are static inline functions, so to use this library you have
    to use the same pthread implementation for all compilations of
    application code. If anything in your tool chain goes wrong, your
    screwed.

  - right at the start it seems to rely on certain features of the
    glibc implementation concerning weak symbols.

This seems to be handled with the macros

__GXX_WEAK__ && _GLIBCXX_GTHREAD_USE_WEAK

It could be a starting point to see how they are set and to play at
bit with them.

Jens

-- 
:: INRIA Nancy Grand Est ::: AlGorille ::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536   ::
:: :::::::::::::::::::::: gsm France : +33 651400183   ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]