Bug 55394 - Using call_once without -lpthread compiles without warning
Summary: Using call_once without -lpthread compiles without warning
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: 4.6.3
: P3 enhancement
Target Milestone: 11.0
Assignee: Jonathan Wakely
URL:
Keywords:
: 81517 97485 (view as bug list)
Depends on: 84323
Blocks:
  Show dependency treegraph
 
Reported: 2012-11-19 12:28 UTC by Geert-Jan Giezeman
Modified: 2020-11-03 18:46 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2016-11-19 00:00:00


Attachments
Source file (167 bytes, text/x-c++src)
2012-11-19 12:28 UTC, Geert-Jan Giezeman
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Geert-Jan Giezeman 2012-11-19 12:28:43 UTC
Created attachment 28731 [details]
Source file

A source file which contains call_once will compile without any errors or warnings, even when libpthread is not linked. At run time, an exception will be thrown with a very unhelpful message. The attached source file demonstrates the problem.

$ g++ -Wall -Wextra -std=c++0x callonce.cpp
$ ./a.out
terminate called after throwing an instance of 'std::system_error'
  what():  Unknown error -1
Aborted (core dumped)

Linking libpthread will remove the problem.

$ g++ -Wall -Wextra -std=c++0x callonce.cpp -lpthread
$ ./a.out
$ 

In the actual use case, call_once was called in a library that could be used in both multithreaded and singlethreaded code. That made it a hard to trace bug.

$ g++ -v
Using built-in specs.
COLLECT_GCC=g++
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/4.6/lto-wrapper
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu/Linaro 4.6.3-1ubuntu5' --with-bugurl=file:///usr/share/doc/gcc-4.6/README.Bugs --enable-languages=c,c++,fortran,objc,obj-c++ --prefix=/usr --program-suffix=-4.6 --enable-shared --enable-linker-build-id --with-system-zlib --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --with-gxx-include-dir=/usr/include/c++/4.6 --libdir=/usr/lib --enable-nls --with-sysroot=/ --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --enable-gnu-unique-object --enable-plugin --enable-objc-gc --disable-werror --with-arch-32=i686 --with-tune=generic --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
gcc version 4.6.3 (Ubuntu/Linaro 4.6.3-1ubuntu5)
Comment 1 Jonathan Wakely 2012-11-19 13:44:48 UTC
(In reply to comment #0)
> A source file which contains call_once will compile without any errors or
> warnings, even when libpthread is not linked.

Obviously this is because compiling and linking are separate steps. The compiler front-end doesn't know in advance whether the linker will be invoked correctly.

It might be possible to do something similar to PR 52681 to improve the exception's message.
Comment 2 Paolo Carlini 2012-11-19 14:55:50 UTC
Library anyway.
Comment 3 Jonathan Wakely 2014-09-19 11:04:30 UTC
related to PR 58929
Comment 4 Markus Trippelsdorf 2016-11-19 09:04:25 UTC
clang's libc++.so.1 avoids the issue because it is linked with libpthread.so.0:

 % ldd /usr/lib/libc++.so.1
        linux-vdso.so.1 (0x00007ffc1618f000)
        libpthread.so.0 => /lib/libpthread.so.0 (0x00007fe0f4fd7000)
        libc.so.6 => /lib/libc.so.6 (0x00007fe0f4c3a000)
        libm.so.6 => /lib/libm.so.6 (0x00007fe0f499f000)
        librt.so.1 => /lib/librt.so.1 (0x00007fe0f4797000)
        libgcc_s.so.1 => /lib/libgcc_s.so.1 (0x00007fe0f477e000)
        /lib64/ld-linux-x86-64.so.2 (0x00005575a195e000)

libstc++ isn't:

 % ldd /usr/lib/gcc/x86_64-pc-linux-gnu/7.0.0/libstdc++.so.6
        linux-vdso.so.1 (0x00007ffd14540000)
        libm.so.6 => /lib/libm.so.6 (0x00007f73bec87000)
        libc.so.6 => /lib/libc.so.6 (0x00007f73be8ea000)
        /lib64/ld-linux-x86-64.so.2 (0x00005594a5a06000)
        libgcc_s.so.1 => /lib/libgcc_s.so.1 (0x00007f73be8d1000)
Comment 5 Jonathan Wakely 2016-11-19 20:44:24 UTC
(In reply to Markus Trippelsdorf from comment #4)
> clang's libc++.so.1 avoids the issue because it is linked with
> libpthread.so.0:
> [...]
> libstc++ isn't:

Which is by design and a very important property to keep.

I was discussing a similar issue on IRC the other day and came to the conclusion that we should probably just call pthread_once() directly when the thread model is posix. Because that call is in a header not in libstdc++.so it won't make libstdc++.so depend on libpthread.so, but users that call std::call_once will depend on libpthread.so and users that don't call std::call_once won't depend on it. Which is what we want.
Comment 6 Jonathan Wakely 2017-07-22 20:48:38 UTC
*** Bug 81517 has been marked as a duplicate of this bug. ***
Comment 7 Andrea Bocci 2019-07-02 14:01:03 UTC
The same test program will fail in the same way, if compiled with -flto, even if -pthread is used:

$ g++-9 -Wall -Wextra -std=c++17 callonce.cpp -pthread
$ ./a.out

but

$ g++-9 -Wall -Wextra -std=c++17 callonce.cpp -pthread -flto
$ ./a.out 
terminate called after throwing an instance of 'std::system_error'
  what():  Unknown error -1
Aborted (core dumped)

Possibly because the LTO pass discards the dependency on libpthread.so:

$ ldd a.out 
        linux-vdso.so.1 (0x00007ffc09b46000)
        libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f420535e000)
        libstdc++.so.6 => /usr/lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007f4204f7e000)
        /lib64/ld-linux-x86-64.so.2 (0x00007f4205952000)
        libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f4204be0000)
        libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007f42049c8000)
Comment 8 Tomasz Kłoczko 2020-01-22 12:56:43 UTC
Any progress on that issue?
Looks like I've hit this issu in protobuf https://github.com/protocolbuffers/protobuf/issues/7092
Comment 9 Sergei Trofimovich 2020-10-19 22:07:26 UTC
*** Bug 97485 has been marked as a duplicate of this bug. ***
Comment 10 Sergei Trofimovich 2020-10-20 07:52:11 UTC
How about something as simple as:

--- a/libgcc/gthr-posix.h
+++ b/libgcc/gthr-posix.h
@@ -697,7 +697,12 @@ static inline int
 __gthread_once (__gthread_once_t *__once, void (*__func) (void))
 {
   if (__gthread_active_p ())
-    return __gthrw_(pthread_once) (__once, __func);
+    /*
+     * Call non-weak form of a symbol to force linker
+     * resolve real implementation of pthread_once even
+     * for as-needed case: https://gcc.gnu.org/PR55394.
+     */
+    return pthread_once (__once, __func);
   else
     return -1;
 }

Superficial test fixes "-flto -Wl,--as-needed" case above for me.

The fix leaves '__gthread_active_p ()' to probe weak symbols but calls non-weak form of pthread_once().

If it's a plausible path I can have a pass through all static inline functions,  and convert weak calls to non-weak calls where glibc-2.32 does not define a symbol in libc.so.6.
Comment 11 Jonathan Wakely 2020-10-20 08:29:04 UTC
It's not plausible because it doesn't work for non-pthreads targets where gthr-default.h is not gthr-posix.h

We can't use pthread_once anyway, see PR 66146, so I'm rewriting it entirely in terms of either futexes or condition variables.
Comment 12 Sergei Trofimovich 2020-10-20 23:00:20 UTC
Aha, makes sense.

My hack did not survive bootstrap anyway as libgcc.a started referring pthread_once() as well.
Comment 13 GCC Commits 2020-11-03 18:45:06 UTC
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:93e79ed391b9c636f087e6eb7e70f14963cd10ad

commit r11-4691-g93e79ed391b9c636f087e6eb7e70f14963cd10ad
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Nov 3 18:44:32 2020 +0000

    libstdc++: Rewrite std::call_once to use futexes [PR 66146]
    
    The current implementation of std::call_once uses pthread_once, which
    only meets the C++ requirements when compiled with support for
    exceptions. For most glibc targets and all non-glibc targets,
    pthread_once does not work correctly if the init_routine exits via an
    exception. The pthread_once_t object is left in the "active" state, and
    any later attempts to run another init_routine will block forever.
    
    This change makes std::call_once work correctly for Linux targets, by
    replacing the use of pthread_once with a futex, based on the code from
    __cxa_guard_acquire. For both glibc and musl, the Linux implementation
    of pthread_once is already based on futexes, and pthread_once_t is just
    a typedef for int, so this change does not alter the layout of
    std::once_flag. By choosing the values for the int appropriately, the
    new code is even ABI compatible. Code that calls the old implementation
    of std::call_once will use pthread_once to manipulate the int, while new
    code will use the new std::once_flag members to manipulate it, but they
    should interoperate correctly. In both cases, the int is initially zero,
    has the lowest bit set when there is an active execution, and equals 2
    after a successful returning execution. The difference with the new code
    is that exceptional exceptions are correctly detected and the int is
    reset to zero.
    
    The __cxa_guard_acquire code (and musl's pthread_once) use an additional
    state to say there are other threads waiting. This allows the futex wake
    syscall to be skipped if there is no contention. Glibc doesn't use a
    waiter bit, so we have to unconditionally issue the wake in order to be
    compatible with code calling the old std::call_once that uses Glibc's
    pthread_once. If we know that we're using musl (and musl's pthread_once
    doesn't change) it would be possible to set a waiting state and check
    for it in std::once_flag::_M_finish(bool), but this patch doesn't do
    that.
    
    This doesn't fix the bug for non-linux targets. A similar approach could
    be used for targets where we know the definition of pthread_once_t is a
    mutex and an integer. We could make once_flag._M_activate() use
    pthread_mutex_lock on the mutex member within the pthread_once_t, and
    then only set the integer if the execution finishes, and then unlock the
    mutex. That would require careful study of each target's pthread_once
    implementation and that work is left for a later date.
    
    This also fixes PR 55394 because pthread_once is no longer needed, and
    PR 84323 because the fast path is now just an atomic load.
    
    As a consequence of the new implementation that doesn't use
    pthread_once, we can also make std::call_once work for targets with no
    gthreads support. The code for the single-threaded implementation
    follows the same methods as on Linux, but with no need for atomics or
    futexes.
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/55394
            PR libstdc++/66146
            PR libstdc++/84323
            * config/abi/pre/gnu.ver (GLIBCXX_3.4.29): Add new symbols.
            * include/std/mutex [!_GLIBCXX_HAS_GTHREADS] (once_flag): Define
            even when gthreads is not supported.
            (once_flag::_M_once) [_GLIBCXX_HAVE_LINUX_FUTEX]: Change type
            from __gthread_once_t to int.
            (once_flag::_M_passive(), once_flag::_M_activate())
            (once_flag::_M_finish(bool), once_flag::_Active_execution):
            Define new members for futex and non-threaded implementation.
            [_GLIBCXX_HAS_GTHREADS] (once_flag::_Prepare_execution): New
            RAII helper type.
            (call_once): Use new members of once_flag.
            * src/c++11/mutex.cc (std::once_flag::_M_activate): Define.
            (std::once_flag::_M_finish): Define.
            * testsuite/30_threads/call_once/39909.cc: Do not require
            gthreads.
            * testsuite/30_threads/call_once/49668.cc: Likewise.
            * testsuite/30_threads/call_once/60497.cc: Likewise.
            * testsuite/30_threads/call_once/call_once1.cc: Likewise.
            * testsuite/30_threads/call_once/dr2442.cc: Likewise.
            * testsuite/30_threads/call_once/once_flag.cc: Add test for
            constexpr constructor.
            * testsuite/30_threads/call_once/66146.cc: New test.
            * testsuite/30_threads/call_once/constexpr.cc: Removed.
            * testsuite/30_threads/once_flag/cons/constexpr.cc: Removed.
Comment 14 Jonathan Wakely 2020-11-03 18:46:35 UTC
Fixed for GCC 11.