diff mbox

C++11: Observers for the three 'handler functions'

Message ID CAH6eHdQNz+tOsC=ARXxX48rkm_ORBMjembFJ7Kub9WJza6eAbA@mail.gmail.com
State New
Headers show

Commit Message

Jonathan Wakely April 5, 2013, 10:13 a.m. UTC 
This should fix the handlers for platforms without __atomic_exchange
for pointers by using a mutex. I used the old __mutex type not
std::mutex because it's available on more platforms and works for the
'single' thread model too.  I didn't try to optimise away the atomic
ops for accessing the handlers because throwing exceptions and
getting/setting these handlers should not be on the fast path of
performance sensitive code.

        PR libstdc++/56841
        * libsupc++/eh_ptr.cc (rethrow_exception): Use get_unexpected() and
        get_terminate() accessors.
        * libsupc++/eh_throw.cc (__cxa_throw): Likewise.
        * libsupc++/eh_terminate.cc: Use mutex when atomic builtins not
        available.
        * libsupc++/new_handler.cc: Likewise.

Tested x86_64-linux and hppa2.0-linux, committed to trunk.
commit 5a24add997a6f4052657855a4573ace82fd59ea2
Author: Jonathan Wakely <jwakely.gcc@gmail.com>
Date:   Fri Apr 5 00:39:58 2013 +0100

    	PR libstdc++/56841
    	* libsupc++/eh_ptr.cc (rethrow_exception): Use get_unexpected() and
    	get_terminate() accessors.
    	* libsupc++/eh_throw.cc (__cxa_throw): Likewise.
    	* libsupc++/eh_terminate.cc: Use mutex when atomic builtins not
    	available.
    	* libsupc++/new_handler.cc: Likewise.

Comments

Jonathan Wakely April 5, 2013, 11:59 a.m. UTC | #1 
On 5 April 2013 11:13, Jonathan Wakely wrote:
> This should fix the handlers for platforms without __atomic_exchange
> for pointers by using a mutex. I used the old __mutex type not
> std::mutex because it's available on more platforms and works for the
> 'single' thread model too.  I didn't try to optimise away the atomic
> ops for accessing the handlers because throwing exceptions and
> getting/setting these handlers should not be on the fast path of
> performance sensitive code.
>
>         PR libstdc++/56841
>         * libsupc++/eh_ptr.cc (rethrow_exception): Use get_unexpected() and
>         get_terminate() accessors.
>         * libsupc++/eh_throw.cc (__cxa_throw): Likewise.
>         * libsupc++/eh_terminate.cc: Use mutex when atomic builtins not
>         available.
>         * libsupc++/new_handler.cc: Likewise.
>
> Tested x86_64-linux and hppa2.0-linux, committed to trunk.


I did think about adding a new accessor for internal use:

std::pair<unexpected_handler, terminate_handler> __get_handlers();

which could be used in eh_throw.cc and eh_ptr.cc to avoid doing two
separate PIC calls to get_unexpected() and get_terminate(), and so
that platforms using a mutex would only lock it once.  We can
reconsider doing that later, it's only an optimisation not needed for
correctness.

I think we could also remove the extern declarations of
__unexpected_handler and __terminate_handler and make them internal to
eh_terminate.cc, as I already did for __new_handler.  Although those
names were visible to users they should not be used, are not part of
the IA-64 ABI, and are not exported from the shared lib.
diff mbox

Patch

diff --git a/libstdc++-v3/libsupc++/eh_ptr.cc b/libstdc++-v3/libsupc++/eh_ptr.cc
index f0183ce..6bc3311 100644
--- a/libstdc++-v3/libsupc++/eh_ptr.cc
+++ b/libstdc++-v3/libsupc++/eh_ptr.cc
@@ -212,8 +212,8 @@  std::rethrow_exception(std::exception_ptr ep)
   dep->primaryException = obj;
   __atomic_add_fetch (&eh->referenceCount, 1,  __ATOMIC_ACQ_REL);
 
-  dep->unexpectedHandler = __unexpected_handler;
-  dep->terminateHandler = __terminate_handler;
+  dep->unexpectedHandler = get_unexpected ();
+  dep->terminateHandler = get_terminate ();
   __GXX_INIT_DEPENDENT_EXCEPTION_CLASS(dep->unwindHeader.exception_class);
   dep->unwindHeader.exception_cleanup = __gxx_dependent_exception_cleanup;
 
diff --git a/libstdc++-v3/libsupc++/eh_terminate.cc b/libstdc++-v3/libsupc++/eh_terminate.cc
index bc38e1d..b31d2e2 100644
--- a/libstdc++-v3/libsupc++/eh_terminate.cc
+++ b/libstdc++-v3/libsupc++/eh_terminate.cc
@@ -27,6 +27,15 @@ 
 #include <cstdlib>
 #include "unwind-cxx.h"
 #include <bits/exception_defines.h>
+#include <bits/atomic_lockfree_defines.h>
+
+#if ATOMIC_POINTER_LOCK_FREE < 2
+#include <ext/concurrence.h>
+namespace
+{
+  __gnu_cxx::__mutex mx;
+}
+#endif
 
 using namespace __cxxabiv1;
 
@@ -65,7 +74,13 @@  std::terminate_handler
 std::set_terminate (std::terminate_handler func) throw()
 {
   std::terminate_handler old;
+#if ATOMIC_POINTER_LOCK_FREE > 1
   __atomic_exchange (&__terminate_handler, &func, &old, __ATOMIC_ACQ_REL);
+#else
+  __gnu_cxx::__scoped_lock l(mx);
+  old = __terminate_handler;
+  __terminate_handler = func;
+#endif
   return old;
 }
 
@@ -73,7 +88,12 @@  std::terminate_handler
 std::get_terminate () noexcept
 {
   std::terminate_handler func;
+#if ATOMIC_POINTER_LOCK_FREE > 1
   __atomic_load (&__terminate_handler, &func, __ATOMIC_ACQUIRE);
+#else
+  __gnu_cxx::__scoped_lock l(mx);
+  func = __terminate_handler;
+#endif
   return func;
 }
 
@@ -81,7 +101,13 @@  std::unexpected_handler
 std::set_unexpected (std::unexpected_handler func) throw()
 {
   std::unexpected_handler old;
+#if ATOMIC_POINTER_LOCK_FREE > 1
   __atomic_exchange (&__unexpected_handler, &func, &old, __ATOMIC_ACQ_REL);
+#else
+  __gnu_cxx::__scoped_lock l(mx);
+  old = __unexpected_handler;
+  __unexpected_handler = func;
+#endif
   return old;
 }
 
@@ -89,6 +115,11 @@  std::unexpected_handler
 std::get_unexpected () noexcept
 {
   std::unexpected_handler func;
+#if ATOMIC_POINTER_LOCK_FREE > 1
   __atomic_load (&__unexpected_handler, &func, __ATOMIC_ACQUIRE);
+#else
+  __gnu_cxx::__scoped_lock l(mx);
+  func = __unexpected_handler;
+#endif
   return func;
 }
diff --git a/libstdc++-v3/libsupc++/eh_throw.cc b/libstdc++-v3/libsupc++/eh_throw.cc
index a79a025..5d37698 100644
--- a/libstdc++-v3/libsupc++/eh_throw.cc
+++ b/libstdc++-v3/libsupc++/eh_throw.cc
@@ -68,8 +68,8 @@  __cxxabiv1::__cxa_throw (void *obj, std::type_info *tinfo,
   header->referenceCount = 1;
   header->exc.exceptionType = tinfo;
   header->exc.exceptionDestructor = dest;
-  header->exc.unexpectedHandler = __unexpected_handler;
-  header->exc.terminateHandler = __terminate_handler;
+  header->exc.unexpectedHandler = std::get_unexpected ();
+  header->exc.terminateHandler = std::get_terminate ();
   __GXX_INIT_PRIMARY_EXCEPTION_CLASS(header->exc.unwindHeader.exception_class);
   header->exc.unwindHeader.exception_cleanup = __gxx_exception_cleanup;
 
diff --git a/libstdc++-v3/libsupc++/new_handler.cc b/libstdc++-v3/libsupc++/new_handler.cc
index 2f6bb5e..5253cfd 100644
--- a/libstdc++-v3/libsupc++/new_handler.cc
+++ b/libstdc++-v3/libsupc++/new_handler.cc
@@ -24,6 +24,15 @@ 
 // <http://www.gnu.org/licenses/>.
 
 #include "new"
+#include <bits/atomic_lockfree_defines.h>
+
+#if ATOMIC_POINTER_LOCK_FREE < 2
+#include <ext/concurrence.h>
+namespace
+{
+  __gnu_cxx::__mutex mx;
+}
+#endif
 
 const std::nothrow_t std::nothrow = { };
 
@@ -37,8 +46,14 @@  new_handler
 std::set_new_handler (new_handler handler) throw()
 {
   new_handler prev_handler;
+#if ATOMIC_POINTER_LOCK_FREE > 1
   __atomic_exchange (&__new_handler, &handler, &prev_handler,
 		     __ATOMIC_ACQ_REL);
+#else
+  __gnu_cxx::__scoped_lock l(mx);
+  prev_handler = __new_handler;
+  __new_handler = handler;
+#endif
   return prev_handler;
 }
 
@@ -46,6 +61,11 @@  new_handler
 std::get_new_handler () noexcept
 {
   new_handler handler;
+#if ATOMIC_POINTER_LOCK_FREE > 1
   __atomic_load (&__new_handler, &handler, __ATOMIC_ACQUIRE);
+#else
+  __gnu_cxx::__scoped_lock l(mx);
+  handler = __new_handler;
+#endif
   return handler;
 }