Bug 58909 - C++11's condition variables fail with static linking
Summary: C++11's condition variables fail with static linking
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: unknown
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
: 57740 66071 98098 (view as bug list)
Depends on:
Blocks:
 
Reported: 2013-10-29 02:43 UTC by joel
Modified: 2021-07-22 10:45 UTC (History)
8 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2021-01-28 00:00:00


Attachments
condition_variable::wait and pthread_cond_wait (85.95 KB, image/jpeg)
2017-03-04 07:56 UTC, abbycin
Details

Note You need to log in before you can comment on or make changes to this bug.
Description joel 2013-10-29 02:43:31 UTC
The program: 

#include <condition_variable>
int main () { std::condition_variable a; }

statically compiled as either:
g++ --std=c++0x -static -pthread aa.cc -lpthread
clang++ --std=c++11 -static -pthread aa.cc -lpthread

segfaults at the destructor's implicit call to pthread_cond_destroy(). Other uses of std::condition_variable also fail. nm shows that pthread_cond_* functions are only 'w', while programs that use other aspects of c++ threading, such as mutexes, will have 'W' pthread_mutex_* and corresponding "T" __pthread_mutex_* available.
Comment 1 Jakub Jelinek 2013-10-29 09:53:01 UTC
If this is NPTL, then the fix is to either avoid using -static (lots of reasons not to do it), or -Wl,--whole-archive -lpthread -Wl,--no-whole-archive
instead of -lpthread (in some distributions like Fedora the latter is unnecessary, as -lpthread contains a single large *.o file for these reasons).
Comment 2 Jonathan Wakely 2015-05-08 15:13:49 UTC
*** Bug 66071 has been marked as a duplicate of this bug. ***
Comment 3 Andrew Pinski 2016-08-14 03:36:54 UTC
Not a GCC issue but rather an issue with how weak functions interact with archives.
Comment 4 Andrew Pinski 2016-08-14 16:05:59 UTC
*** Bug 57740 has been marked as a duplicate of this bug. ***
Comment 5 Brooks Moses 2017-02-14 00:21:45 UTC
How is this "invalid", though?

If there is, as you said on bug 57740, an issue of "not understanding the interaction between weak links and archives" rather than a bug in how they interact, then who is it that has the misunderstanding?

joel@'s code doesn't have weak links in it.  It just defines a std::condition_variable.

Either joel@'s code is bad for some very non-obvious reason, or he needs to do special things on his link line to make it link correctly (which, at minimum, needs documentation in the libstdc++ manual), or the misunderstanding is in libstdc++ and thus libstdc++ needs to be fixed.

In either of those cases, this is a libstdc++ bug, whether of documentation or of creation of weak links.  No?
Comment 6 Jakub Jelinek 2017-02-14 06:54:45 UTC
INVALID means it is not a gcc bug, glibc has a separate bugzilla.
Comment 7 Brooks Moses 2017-02-14 08:26:39 UTC
Right, understood, but Roland seemed pretty insistent in the discussion on bug 57740 that this was a libstdc++ bug, not a glibc bug.  Your contention is that this is invalid because he's wrong about that, I take it?  And that rather than libstdc++ documenting workarounds, this should be fixed by the "libpthread.a is a single object file" fix?

Unfortunately, it seems fairly clear that filing this as a glibc bug would have Roland closing it as invalid there, and this will simply not get fixed.  :(
Comment 8 Jonathan Wakely 2017-02-14 10:51:23 UTC
Well Roland's fix doesn't work, and his claims about libstdc++ problems are based on ancient history, so I don't find his arguments very persuasive.

I'm happy to document the requirements in the libstdc++ manual, there's no downside to that.

We can also avoid weak symbols for some functions that are only referenced from headers, e.g. pthread_call_once. We only need the weak symbols in definitions that end up in libstdc++.so and libstdc++.a, so that those libraries don't unconditionally depend on libphtread. But if user code instantiates the std::call_once function template then they should get a dependency on pthread_call_once, because they're actually using it. Going via the __gthread_call_once weak symbol has no benefit in that case. But the std::condition_variable definitions are in the library, and so we do need to use weak symbols for the cases involved in this PR.
Comment 9 joel 2017-02-16 04:28:49 UTC
Wow, I had completely forgotten about the time I spent puzzled over this behaviour in 2013.

I'm guessing that just the existence of this as a bug report for 'condition variables static linking' is sufficient documentation for the rare time it happens for people. As such I'll note that my unpleasant fix was to include this in a header:

void pthread_cond_bug() {
        pthread_cond_signal((pthread_cond_t *) nullptr);
        pthread_cond_init((pthread_cond_t *) nullptr,
                          (const pthread_condattr_t *) nullptr);
        pthread_cond_destroy((pthread_cond_t *) nullptr);
        pthread_cond_timedwait((pthread_cond_t *) nullptr, (pthread_mutex_t *)
                               nullptr, (const struct timespec *) nullptr);
        pthread_cond_wait((pthread_cond_t *) nullptr, (pthread_mutex_t *) (nullptr);
}

without actually ever calling it; iirc it links in the functions as a result.
Comment 10 abbycin 2017-03-04 07:56:00 UTC
Created attachment 40881 [details]
condition_variable::wait and pthread_cond_wait

test platform
OS openSUSE Tumbleweed 64bit
kernel version 4.10.1-1-default 
gcc version 6.3.1 20170202 [gcc-6-branch revision 245119] (SUSE Linux)
Comment 11 Jonathan Wakely 2020-12-02 16:36:18 UTC
*** Bug 98098 has been marked as a duplicate of this bug. ***
Comment 12 Keegan Saunders 2021-01-28 16:48:39 UTC
This behaviour does not occur with clang when statically linking libc++. It is, however, still an issue on clang and gcc when using libstdc++.

It is still possible to reproduce this issue with:

clang version 11.0.0 (Fedora 11.0.0-2.fc33)

and

g++ (GCC) 10.2.1 20201125 (Red Hat 10.2.1-9)

As libc++ links against glibc, this appears to be strictly an issue with libstdc++ rather than glibc.
Comment 13 Jakub Jelinek 2021-01-28 16:58:31 UTC
Maybe libc++ doesn't bother with supporting not linking against -lpthread.
Comment 14 Jonathan Wakely 2021-01-28 17:32:17 UTC
(In reply to Jakub Jelinek from comment #13)
> Maybe libc++ doesn't bother with supporting not linking against -lpthread.

libc++ is linked to libpthread.so unconditionally.
Comment 15 Jonathan Wakely 2021-01-28 17:42:23 UTC
And the static libc++.a doesn't use weak symbols, so you need to link to libpthread youself, and the necessary symbols get pulled in correctly.

The problem for libstdc++.a is that the weak symbols do not cause the libpthread.a symbols to get linked into the binary.
Comment 16 Jakub Jelinek 2021-01-28 17:49:10 UTC
Are those weak refs from libstdc++.a objects or from the user *.o files?
If the former, perhaps we could declare some libstdc++ APIs (related to threading) as requiring linking of -lpthread and made them non-weak in libstdc++.a.
I don't really see how can one reproduce this on Fedora/RHEL/CentOS where libpthread.a
contains a single libpthread.o and therefore it is either you link no thread support at all, or link it completely.
Comment 17 Jonathan Wakely 2021-01-28 18:01:08 UTC
(In reply to Jakub Jelinek from comment #16)
> Are those weak refs from libstdc++.a objects or from the user *.o files?

From libstdc++.a

> If the former, perhaps we could declare some libstdc++ APIs (related to
> threading) as requiring linking of -lpthread and made them non-weak in
> libstdc++.a.

Yes, I think we should just use the non-weak pthread_xxx symbols directly when
!defined(_GLIBCXX_SHARED) and rely on those sections from libstdc++.a not being used unless they're needed.

> I don't really see how can one reproduce this on Fedora/RHEL/CentOS where
> libpthread.a
> contains a single libpthread.o and therefore it is either you link no thread
> support at all, or link it completely.

The example in comment 0 fails on Fedora. Nothing in libstdc++.a causes anything from libpthread.o to be used.
Comment 18 Jonathan Wakely 2021-01-28 18:07:30 UTC
(In reply to Jonathan Wakely from comment #17)
> (In reply to Jakub Jelinek from comment #16)
> The example in comment 0 fails on Fedora. Nothing in libstdc++.a causes
> anything from libpthread.o to be used.

However, any real program that uses std::condition_variable is likely to also use std::thread, and that will cause a dependency on symbols in libpthread.o
Comment 19 Jakub Jelinek 2021-01-28 18:17:08 UTC
Is that because some functions are also in libc.a and not just in libpthread.a?
Comment 20 Jonathan Wakely 2021-01-28 18:39:45 UTC
I don't think so. The linker just doesn't resolve the undefined weak symbol for pthread_cond_destroy is left equal to zero, and so there's a segfault when it gets called.

$ g++ cv.C -static -g -Wl,--trace-symbol=pthread_cond_destroy -pthread
/usr/bin/ld: /usr/lib/gcc/x86_64-redhat-linux/10/libstdc++.a(condition_variable.o): reference to pthread_cond_destroy


IIUC it's not finding the definition in libc.a, or any definition at all.
Comment 21 Jakub Jelinek 2021-01-28 18:48:33 UTC
So how does that work for dynamic linking when -lpthread is not linked in?
Does it crash too?
Comment 22 Jonathan Wakely 2021-01-28 19:00:19 UTC
In that case it finds the no-op symbol in libc.so.6 and doesn't crash.

$ g++ cv.C   -g -Wl,--trace-symbol=pthread_cond_destroy 
/usr/bin/ld: /lib64/libc.so.6: definition of pthread_cond_destroy


This fixes the static linking case:

--- a/libstdc++-v3/src/c++11/condition_variable.cc
+++ b/libstdc++-v3/src/c++11/condition_variable.cc
@@ -31,9 +31,19 @@ namespace std _GLIBCXX_VISIBILITY(default)
 {
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
-  condition_variable::condition_variable() noexcept = default;
+  condition_variable::condition_variable() noexcept
+  {
+#if defined __GLIBC__ && ! defined _GLIBCXX_SHARED
+    asm("extern pthread_cond_init");
+#endif
+  }
 
-  condition_variable::~condition_variable() noexcept = default;
+  condition_variable::~condition_variable() noexcept
+  {
+#if defined __GLIBC__ && ! defined _GLIBCXX_SHARED
+    asm("extern pthread_cond_destroy");
+#endif
+  }
 
   void
   condition_variable::wait(unique_lock<mutex>& __lock) noexcept
Comment 23 Jakub Jelinek 2021-01-28 19:12:26 UTC
Unaware of extern directive in gas.
I'd think (but it would need to be surrounded by configury checks) that something like:
.section .gnu.need.pthread_cond, "e"
.8byte pthread_cond_init
...
.previous
where you'd list all the symbols you want to force in should work though, and wouldn't actually waste .data or .rodata.
Comment 24 Jakub Jelinek 2021-01-28 19:24:47 UTC
Actually asm (".global pthread_cond_init"); (etc.) is probably better, doesn't need relocations, just adds the symbol to the symtab as non-weak.
Comment 25 Marc Glisse 2021-07-22 08:10:27 UTC
Note that this also affects dynamic linking with -Wl,--as-needed (which some platforms use by default).

#include <mutex>
int main(){
  std::once_flag o;
  std::call_once(o, [](){});
}

$ g++ b.cc -lpthread && ldd ./a.out            
	linux-vdso.so.1 (0x00007ffca7b60000)
	libstdc++.so.6 => /lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007f9c809ac000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f9c807e7000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f9c806a3000)
	/lib64/ld-linux-x86-64.so.2 (0x00007f9c80bd4000)
	libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007f9c80689000)

No libpthread there :-(

(using -pthread instead of -lpthread works, but some build systems like cmake use -lpthread by default)
Comment 26 Jonathan Wakely 2021-07-22 08:55:39 UTC
If you create a new thread of execution then you'll get a non-weak reference to pthread_create, which should cause libpthread.so to be linked even with -Wl,--as-needed (and for static linking it will work if libpthread.a has a single .o with all symbols).

If you don't actually have multiple threads in your program, then things like condition_variable and once_flag can end up using the stubs in libc.so.6 which are no-ops. But since you don't have multiple threads, it's probably not a major problem. Most uses of std::once_flag would be better done with a local static variable anyway (the exception being non-static data members of classes).

With glibc 2.34 the problem goes away, so I'm not sure it's worth investing much effort in libstdc++ trying to work around the problems with weak symbols.
Comment 27 Marc Glisse 2021-07-22 10:45:48 UTC
(In reply to Jonathan Wakely from comment #26)
> If you create a new thread of execution then you'll get a non-weak reference
> to pthread_create, which should cause libpthread.so to be linked even with
> -Wl,--as-needed (and for static linking it will work if libpthread.a has a
> single .o with all symbols).
> 
> If you don't actually have multiple threads in your program, then things
> like condition_variable and once_flag can end up using the stubs in
> libc.so.6 which are no-ops. But since you don't have multiple threads, it's
> probably not a major problem.

For call_once, it throws an exception whether there are other threads or not, it isn't a no-op.
(as you might guess, this code is in a library, I don't control if threads are used elsewhere)

> Most uses of std::once_flag would be better
> done with a local static variable anyway (the exception being non-static
> data members of classes).

I build trees with a once_flag in each node, there is no way I can do that with static variables.

> With glibc 2.34 the problem goes away, so I'm not sure it's worth investing
> much effort in libstdc++ trying to work around the problems with weak
> symbols.

Ok. I just wanted to advertise that the issue is not limited to static linking.

(too bad you had to revert the new call_once implementation)