Weak references, --as-needed, and pthreads

[Cary Coutant <ccoutant at gmail dot com>]

Over on stack overflow [1], someone posted a very interesting problem
they're having with gold, g++, and pthreads. I found the problem is
caused by the combination of several factors:

(1) G++ uses weak references to call many libpthread entry points,
even though most of those entry points have stubbed alternatives in
libc.

(2) G++ (in some distros, at least) passes --as-needed to the linker
for all libraries, rather than just libgcc_s.

(3) The shared library in the sample program is built with the
-pthread option, which adds -lpthread to the linker command line in
front of -lc.

(4) The linker (neither gold nor BFD) does not add an "as-needed"
shared library to the needed list if all reference to the library are
weak.

(5) Gold does not record symbol version information gleaned from an
"as-needed" library that isn't needed. (BFD ld apparently does, or
else it picks up the version information from the same symbols in
libc.)

(6) Glibc 2.3.2 changed the size of the pthread_cond_t structure,
requiring that all APIs using that structure be versioned, including
pthread_cond_timedwait, which is called by the inlined implementation
of condition_variable::wait_for().

(7) When ld.so binds a symbol with no version information, it does not
bind to the default version; instead it binds to the older version
(not sure if that's by chance or it it deliberately picks the first or
oldest version).

Put these all together, and the sample program ends up calling the old
version of pthread_cond_timedwait() rather than the new version. If
the consuming thread reaches this point before the producing thread
reaches its call to condition_variable::notify_one(), the old version
of pthread_cond_timedwait() initializes the pthread_cond_t structure
in the condition_variable::_M_cond member as a pthread_cond_2_0_t
type, which is just a struct containing a single pointer to a
newly-allocated pthread_cond_t structure.

The inlined implementation of condition_variable::notify_one(),
however, treats the _M_cond member as a real pthread_cond_t struct,
and attempts to call pthread_mutex_lock() on the lock pointed to by
_M_cond.__data.__lock. But because _M_cond was initialized as a
pthread_cond_2_0_t, that pointer is really a pointer to the real
pthread_cond_t, not the mutex. The producing thread sees a would-be
mutex that looks like it's already locked, and blocks waiting for
another thread to unlock it. But that will never happen, because the
other thread is unlocking the real mutex.

Removing any one of the 7 factors above makes the problem go away.
Adding a strong reference to libpthread makes it a "needed" library.
Switching from g++ to clang++ removes the --as-needed option. Adding
an explicit --no-as-needed option works. Removing the -pthread option
from the command line lets gold record the version information from
the symbol definitions in libc (which is "needed" despite the weak
references to the libpthread entry points). Changing
condition_variable::wait_for() to condition_variable::wait() removes
the call to pthread_cond_timedwait(), so we're no longer likely to
bind to the wrong version.

There are reasons for all of the toolchain behaviors described here:

- Using weak references to pthread APIs means that you can build
thread-safe libraries without a forced dependency on the thread
library. Given that libc contains stubbed versions of most of those
APIs, however, I'm not convinced that the weak references are
necessary.

- Many distros seem to feel that passing --as-needed for all shared
libraries is the right thing. Given the use of weak references for
pthread APIs, however, it seems that GCC ought to ensure that
-lpthread is linked with --no-as-needed.

- The choice to not put an "as-needed" library in the dependency list
when there are only weak references to it is rooted in history (weak
references to avoid forcing libgcc_s into a program that doesn't need
EH support?).

- The choice to clear out the version information for symbols resolved
by "not-needed" libraries has something to do with preventing a
segfault in ld.so (according to an old email from Alan [2]). I'd
consider letting those symbols resolve to the definitions in libc
instead, but I'm not sure how to decide when to do that -- it's not
clear until the end whether a library ends up as "needed" or not. How
does BFD ld end up with the version info in this case?

That's a lot that needs to happen before there's actually a problem,
and it's only because this was a trivially-small test program that we
had all seven conditions trigger together. Nevertheless, this
illustrates the fragility of weak references. (It also demonstrates a
weakness of the symbol versioning mechanism, where some binding takes
place at compile time, and some takes place at link time.)

Personally, I'd like to see weak references go away. In most cases,
dlsym() is more appropriate. For libpthread, having alternate stubbed
implementations in libc is perfectly suitable for most APIs; we'd need
the one weak reference to the "canary" symbol (pthread_key_create) so
code can test for the presence of libpthread, but that could even be
replaced by a dedicated pthread_is_loaded API that returns true in the
libpthread implementation and false in the libc implementation.

Another fix would be to change the GCC driver to put "--push-state
--no-as-needed" and "--pop-state" around the -lpthread option when
linking. Given the current reliance on weak references, this would
seem fairly obvious.

Thoughts? Which remedy(ies) should I pursue?

-cary


[1] https://stackoverflow.com/questions/50751421/undefined-behavior-in-shared-lib-using-libpthread-but-not-having-it-in-elf-as-d

[2] https://sourceware.org/ml/binutils/2013-03/msg00204.html