Patchwork [v3] fix PR 54297 - segfaults with std::async

login
register
mail settings
Submitter Jonathan Wakely
Date Aug. 26, 2012, 2 p.m.
Message ID <CAH6eHdQkZ4KB3mqna7go5P79s4NO9EorTQMWtK=NZyDb2eFnhg@mail.gmail.com>
Download mbox | patch
Permalink /patch/180055/
State New
Headers show

Comments

Jonathan Wakely - Aug. 26, 2012, 2 p.m.
On 26 August 2012 00:02, Jonathan Wakely wrote:
> This fixes a stupid mistake I made where the functor and asynchronous
> result can go out of scope before the async thread is joined.
>
> The _Async_state_common destructor still needs to be exported from the
> library, which is what the macro hack is for.
>
>         PR libstdc++/54297
>         * include/std/future (~_Async_state_impl): Join thread before
>         derived class members are destroyed.
>         (~_Async_state_common): Only define non-trivial destructor when
>         included from future.cc for ABI compatibility reasons.
>         * src/c++11/future.cc (_GLIBCXX_ABI_COMPAT_ASYNC): Define.
>         * testsuite/30_threads/async/54297.cc: New.
>
> Tested x86_64-linux, committed to trunk and 4.7

I realised the compatibility symbols should be in one of the files
used for that purpose, so this moves them to
src/c++11/compatibility-thread-c++0x.cc

        PR libstdc++/54297
        * src/c++11/future.cc (~_Async_state_common): Move to...
        * src/c++11/compatibility-thread-c++0x.cc (~_Async_state_common):
        Here.
        (_GLIBCXX_ABI_COMPAT_ASYNC): Rename to _GLIBCXX_ASYNC_ABI_COMPAT.
        * include/std/future (_GLIBCXX_ABI_COMPAT_ASYNC): Likewise.

Tested x86_64-linux, committed to trunk and 4.7
commit 463ac67acbdada5a558dcad097e030a7479086c5
Author: Jonathan Wakely <jwakely.gcc@gmail.com>
Date:   Sun Aug 26 11:42:00 2012 +0100

    	PR libstdc++/54297
    	* src/c++11/future.cc (~_Async_state_common): Move to...
    	* src/c++11/compatibility-thread-c++0x.cc (~_Async_state_common):
    	Here.
    	(_GLIBCXX_ABI_COMPAT_ASYNC): Rename to _GLIBCXX_ASYNC_ABI_COMPAT.
    	* include/std/future (_GLIBCXX_ABI_COMPAT_ASYNC): Likewise.
H.J. Lu - Aug. 26, 2012, 2:48 p.m.
On Sun, Aug 26, 2012 at 7:00 AM, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
> On 26 August 2012 00:02, Jonathan Wakely wrote:
>> This fixes a stupid mistake I made where the functor and asynchronous
>> result can go out of scope before the async thread is joined.
>>
>> The _Async_state_common destructor still needs to be exported from the
>> library, which is what the macro hack is for.
>>
>>         PR libstdc++/54297
>>         * include/std/future (~_Async_state_impl): Join thread before
>>         derived class members are destroyed.
>>         (~_Async_state_common): Only define non-trivial destructor when
>>         included from future.cc for ABI compatibility reasons.
>>         * src/c++11/future.cc (_GLIBCXX_ABI_COMPAT_ASYNC): Define.
>>         * testsuite/30_threads/async/54297.cc: New.
>>
>> Tested x86_64-linux, committed to trunk and 4.7
>
> I realised the compatibility symbols should be in one of the files
> used for that purpose, so this moves them to
> src/c++11/compatibility-thread-c++0x.cc
>
>         PR libstdc++/54297
>         * src/c++11/future.cc (~_Async_state_common): Move to...
>         * src/c++11/compatibility-thread-c++0x.cc (~_Async_state_common):
>         Here.
>         (_GLIBCXX_ABI_COMPAT_ASYNC): Rename to _GLIBCXX_ASYNC_ABI_COMPAT.
>         * include/std/future (_GLIBCXX_ABI_COMPAT_ASYNC): Likewise.
>
> Tested x86_64-linux, committed to trunk and 4.7

Does it fix:

output is:
/export/gnu/import/git/gcc/libstdc++-v3/testsuite/30_threads/async/54297.cc:
In member function 'void Task::operator()() const':^M
/export/gnu/import/git/gcc/libstdc++-v3/testsuite/30_threads/async/54297.cc:42:5:
error: 'sleep_for' is not a member of 'std::this_thread'^M
     std::this_thread::sleep_for(std::chrono::seconds(1));^M
     ^^M

FAIL: 30_threads/async/54297.cc (test for excess errors)
Excess errors:
/export/gnu/import/git/gcc/libstdc++-v3/testsuite/30_threads/async/54297.cc:42:5:
error: 'sleep_for' is not a member of 'std::this_thread'
Jonathan Wakely - Aug. 26, 2012, 2:50 p.m.
On 26 August 2012 15:48, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Sun, Aug 26, 2012 at 7:00 AM, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
>> On 26 August 2012 00:02, Jonathan Wakely wrote:
>>> This fixes a stupid mistake I made where the functor and asynchronous
>>> result can go out of scope before the async thread is joined.
>>>
>>> The _Async_state_common destructor still needs to be exported from the
>>> library, which is what the macro hack is for.
>>>
>>>         PR libstdc++/54297
>>>         * include/std/future (~_Async_state_impl): Join thread before
>>>         derived class members are destroyed.
>>>         (~_Async_state_common): Only define non-trivial destructor when
>>>         included from future.cc for ABI compatibility reasons.
>>>         * src/c++11/future.cc (_GLIBCXX_ABI_COMPAT_ASYNC): Define.
>>>         * testsuite/30_threads/async/54297.cc: New.
>>>
>>> Tested x86_64-linux, committed to trunk and 4.7
>>
>> I realised the compatibility symbols should be in one of the files
>> used for that purpose, so this moves them to
>> src/c++11/compatibility-thread-c++0x.cc
>>
>>         PR libstdc++/54297
>>         * src/c++11/future.cc (~_Async_state_common): Move to...
>>         * src/c++11/compatibility-thread-c++0x.cc (~_Async_state_common):
>>         Here.
>>         (_GLIBCXX_ABI_COMPAT_ASYNC): Rename to _GLIBCXX_ASYNC_ABI_COMPAT.
>>         * include/std/future (_GLIBCXX_ABI_COMPAT_ASYNC): Likewise.
>>
>> Tested x86_64-linux, committed to trunk and 4.7
>
> Does it fix:

No, that needs an extra dg-requires-xxx in the test, sorry about that.

I'll fix it now.


> output is:
> /export/gnu/import/git/gcc/libstdc++-v3/testsuite/30_threads/async/54297.cc:
> In member function 'void Task::operator()() const':^M
> /export/gnu/import/git/gcc/libstdc++-v3/testsuite/30_threads/async/54297.cc:42:5:
> error: 'sleep_for' is not a member of 'std::this_thread'^M
>      std::this_thread::sleep_for(std::chrono::seconds(1));^M
>      ^^M
>
> FAIL: 30_threads/async/54297.cc (test for excess errors)
> Excess errors:
> /export/gnu/import/git/gcc/libstdc++-v3/testsuite/30_threads/async/54297.cc:42:5:
> error: 'sleep_for' is not a member of 'std::this_thread'
>
>
> --
> H.J.

Patch

diff --git a/libstdc++-v3/include/std/future b/libstdc++-v3/include/std/future
index feae382..9568192 100644
--- a/libstdc++-v3/include/std/future
+++ b/libstdc++-v3/include/std/future
@@ -1427,7 +1427,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
   class __future_base::_Async_state_common : public __future_base::_State_base
   {
   protected:
-#if defined(_GLIBCXX_HAVE_TLS) && defined(_GLIBCXX_ABI_COMPAT_ASYNC)
+#ifdef _GLIBCXX_ASYNC_ABI_COMPAT
     ~_Async_state_common();
 #else
     ~_Async_state_common() = default;
diff --git a/libstdc++-v3/src/c++11/compatibility-thread-c++0x.cc b/libstdc++-v3/src/c++11/compatibility-thread-c++0x.cc
index cc8761e..e5c7eec 100644
--- a/libstdc++-v3/src/c++11/compatibility-thread-c++0x.cc
+++ b/libstdc++-v3/src/c++11/compatibility-thread-c++0x.cc
@@ -22,11 +22,16 @@ 
 // see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 // <http://www.gnu.org/licenses/>.
 
+#include <bits/c++config.h>
+#if defined(_GLIBCXX_HAVE_TLS) && defined(PIC)
+#define _GLIBCXX_ASYNC_ABI_COMPAT
+#endif
+
 #include <future>
 #include <mutex>
 
 #ifndef __GXX_EXPERIMENTAL_CXX0X__
-# error "compatibility-c++0x.cc must be compiled with -std=gnu++0x"
+# error "compatibility-thread-c++0x.cc must be compiled with -std=gnu++0x"
 #endif
 
 #define _GLIBCXX_ASM_SYMVER(cur, old, version) \
@@ -70,3 +75,23 @@  _GLIBCXX_ASM_SYMVER(_ZN9__gnu_cxx11try_to_lockE, _ZSt11try_to_lock, GLIBCXX_3.4.
 
 #endif
 #endif // _GLIBCXX_HAS_GTHREADS && _GLIBCXX_USE_C99_STDINT_TR1
+
+
+// XXX GLIBCXX_ABI Deprecated
+// gcc-4.7.0
+// <future> export changes
+#if defined(_GLIBCXX_HAS_GTHREADS) && defined(_GLIBCXX_USE_C99_STDINT_TR1) \
+  && (ATOMIC_INT_LOCK_FREE > 1)
+#if defined(_GLIBCXX_HAVE_TLS) && defined(PIC)
+namespace std _GLIBCXX_VISIBILITY(default)
+{
+_GLIBCXX_BEGIN_NAMESPACE_VERSION
+  __future_base::_Async_state_common::~_Async_state_common() { _M_join(); }
+
+  // Explicit instantiation due to -fno-implicit-instantiation.
+  template void call_once(once_flag&, void (thread::*&&)(), reference_wrapper<thread>&&);
+  template _Bind_simple_helper<void (thread::*)(), reference_wrapper<thread>>::__type __bind_simple(void (thread::*&&)(), reference_wrapper<thread>&&);
+_GLIBCXX_END_NAMESPACE_VERSION
+} // namespace std
+#endif
+#endif // _GLIBCXX_HAS_GTHREADS && _GLIBCXX_USE_C99_STDINT_TR1
diff --git a/libstdc++-v3/src/c++11/future.cc b/libstdc++-v3/src/c++11/future.cc
index eaea393..0552537 100644
--- a/libstdc++-v3/src/c++11/future.cc
+++ b/libstdc++-v3/src/c++11/future.cc
@@ -22,8 +22,6 @@ 
 // see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 // <http://www.gnu.org/licenses/>.
 
-#define _GLIBCXX_ABI_COMPAT_ASYNC
-
 #include <future>
 
 namespace
@@ -86,15 +84,6 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
   __future_base::_Result_base::~_Result_base() = default;
 
   __future_base::_State_base::~_State_base() = default;
-
-#ifdef _GLIBCXX_HAVE_TLS
-  // Exported for compatibility with ABI version 3.4.17
-  __future_base::_Async_state_common::~_Async_state_common() { _M_join(); }
-
-  // Explicit instantiation due to -fno-implicit-instantiation.
-  template void call_once(once_flag&, void (thread::*&&)(), reference_wrapper<thread>&&);
-  template _Bind_simple_helper<void (thread::*)(), reference_wrapper<thread>>::__type __bind_simple(void (thread::*&&)(), reference_wrapper<thread>&&);
-#endif
 #endif
 
 _GLIBCXX_END_NAMESPACE_VERSION