diff mbox

libstdc++/61841 odr-use pthread_create in std::thread constructor

Message ID 20140813184057.GF6927@redhat.com
State New
Headers show

Commit Message

Jonathan Wakely Aug. 13, 2014, 6:40 p.m. UTC
This patch causes the std::thread constructor that launches a new
thread to odr-use pthread_create on glibc-based targets, so that the
program depends directly on pthread_create (and must be linked to
libpthread) not just on the weak symbol in gthr-posix.h

This should fix the Hurd bug in PR61841 as well as the problem
described in https://gcc.gnu.org/ml/gcc/2013-09/msg00196.html

Tested x86_64-linux, committed to trunk.

This is not suitable for the branches as it requires a new exported
symbol. We could maybe just add some other odr-use of pthread_create,
but it would probably be optimized away anyway.

Comments

Jakub Jelinek Aug. 13, 2014, 6:47 p.m. UTC | #1
On Wed, Aug 13, 2014 at 07:40:57PM +0100, Jonathan Wakely wrote:
> This should fix the Hurd bug in PR61841 as well as the problem
> described in https://gcc.gnu.org/ml/gcc/2013-09/msg00196.html
> 
> Tested x86_64-linux, committed to trunk.
> 
> This is not suitable for the branches as it requires a new exported
> symbol. We could maybe just add some other odr-use of pthread_create,
> but it would probably be optimized away anyway.

E.g. __asm ("" : : "r" (&pthread_create)); would not be optimized away.

> commit 79c721b72b7f9288256cc4bf388cf6b60b65e838
> Author: Jonathan Wakely <jwakely@redhat.com>
> Date:   Fri Jul 18 15:35:49 2014 +0100
> 
>     	PR libstdc++/61841
>     	* include/std/thread (thread::_M_start_thread): Declare new overload.
>     	(thread::thread<_Callable, _Args...>): Call new overload with an
>     	explicit reference to pthread_create.
>     	* src/c++11/thread.cc (thread::_M_start_thread): Add new overload.
>     	* config/abi/pre/gnu.ver: Export new function.

	Jakub
Jonathan Wakely Aug. 13, 2014, 7:02 p.m. UTC | #2
On 13/08/14 20:47 +0200, Jakub Jelinek wrote:
>On Wed, Aug 13, 2014 at 07:40:57PM +0100, Jonathan Wakely wrote:
>> This should fix the Hurd bug in PR61841 as well as the problem
>> described in https://gcc.gnu.org/ml/gcc/2013-09/msg00196.html
>>
>> Tested x86_64-linux, committed to trunk.
>>
>> This is not suitable for the branches as it requires a new exported
>> symbol. We could maybe just add some other odr-use of pthread_create,
>> but it would probably be optimized away anyway.
>
>E.g. __asm ("" : : "r" (&pthread_create)); would not be optimized away.

Good idea - thanks. Do we want to do that instead of adding the new
exported function I've added on trunk?
Jakub Jelinek Aug. 13, 2014, 7:06 p.m. UTC | #3
On Wed, Aug 13, 2014 at 08:02:22PM +0100, Jonathan Wakely wrote:
> On 13/08/14 20:47 +0200, Jakub Jelinek wrote:
> >On Wed, Aug 13, 2014 at 07:40:57PM +0100, Jonathan Wakely wrote:
> >>This should fix the Hurd bug in PR61841 as well as the problem
> >>described in https://gcc.gnu.org/ml/gcc/2013-09/msg00196.html
> >>
> >>Tested x86_64-linux, committed to trunk.
> >>
> >>This is not suitable for the branches as it requires a new exported
> >>symbol. We could maybe just add some other odr-use of pthread_create,
> >>but it would probably be optimized away anyway.
> >
> >E.g. __asm ("" : : "r" (&pthread_create)); would not be optimized away.
> 
> Good idea - thanks. Do we want to do that instead of adding the new
> exported function I've added on trunk?

Dunno, you're the maintainer ;)  On one side, asm has the disadvantage of
being a scheduling barrier (because it is implicitly volatile), on the other
side, we are talking about pthread_create call, so perhaps a few lost ticks
is nothing compared to that.

	Jakub
diff mbox

Patch

commit 79c721b72b7f9288256cc4bf388cf6b60b65e838
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Jul 18 15:35:49 2014 +0100

    	PR libstdc++/61841
    	* include/std/thread (thread::_M_start_thread): Declare new overload.
    	(thread::thread<_Callable, _Args...>): Call new overload with an
    	explicit reference to pthread_create.
    	* src/c++11/thread.cc (thread::_M_start_thread): Add new overload.
    	* config/abi/pre/gnu.ver: Export new function.

diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver
index ed7a93f..41fac71 100644
--- a/libstdc++-v3/config/abi/pre/gnu.ver
+++ b/libstdc++-v3/config/abi/pre/gnu.ver
@@ -1372,6 +1372,9 @@  GLIBCXX_3.4.21 {
     # std::regex_error::regex_error(std::regex_constants::error_type)
     _ZNSt11regex_errorC2ENSt15regex_constants10error_typeE;
 
+    # void std::thread::_M_start_thread(__shared_base_type, void(*)())
+    _ZNSt6thread15_M_start_threadESt10shared_ptrINS_10_Impl_baseEEPFvvE;
+
 } GLIBCXX_3.4.20;
 
 
diff --git a/libstdc++-v3/include/std/thread b/libstdc++-v3/include/std/thread
index efcb101..0576347 100644
--- a/libstdc++-v3/include/std/thread
+++ b/libstdc++-v3/include/std/thread
@@ -132,9 +132,17 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       explicit 
       thread(_Callable&& __f, _Args&&... __args)
       {
+#ifdef GTHR_ACTIVE_PROXY
+	// Create a reference to pthread_create, not just the gthr weak symbol
+        _M_start_thread(_M_make_routine(std::__bind_simple(
+                std::forward<_Callable>(__f),
+                std::forward<_Args>(__args)...)),
+	    reinterpret_cast<void(*)()>(&pthread_create));
+#else
         _M_start_thread(_M_make_routine(std::__bind_simple(
                 std::forward<_Callable>(__f),
                 std::forward<_Args>(__args)...)));
+#endif
       }
 
     ~thread()
@@ -183,6 +191,9 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   private:
     void
+    _M_start_thread(__shared_base_type, void (*)());
+
+    void
     _M_start_thread(__shared_base_type);
 
     template<typename _Callable>
diff --git a/libstdc++-v3/src/c++11/thread.cc b/libstdc++-v3/src/c++11/thread.cc
index 49aacb5..bbcc99c 100644
--- a/libstdc++-v3/src/c++11/thread.cc
+++ b/libstdc++-v3/src/c++11/thread.cc
@@ -137,6 +137,12 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       __throw_system_error(int(errc::operation_not_permitted));
 #endif
 
+    _M_start_thread(__b, nullptr);
+  }
+
+  void
+  thread::_M_start_thread(__shared_base_type __b, void (*)())
+  {
     __b->_M_this_ptr = __b;
     int __e = __gthread_create(&_M_id._M_thread,
 			       &execute_native_thread_routine, __b.get());