Patchwork std::rethrow_exception is broken

login
register
mail settings
Submitter Jonathan Wakely
Date March 24, 2014, 7:19 p.m.
Message ID <CAH6eHdQG3ZqSqckxwQG=6H8qhs9q0H5L7JXxVFZMtMVkdqmM+g@mail.gmail.com>
Download mbox | patch
Permalink /patch/333121/
State New
Headers show

Comments

Jonathan Wakely - March 24, 2014, 7:19 p.m.
There is a lot of code in libsupc++/eh_* that relies on
__cxa_exception and __cxa_dependent_exception having similar layouts,
so tricks like this work:

static inline void*
__gxx_caught_object(_Unwind_Exception* eo)
{
  // Bad as it looks, this actually works for dependent exceptions too.
  __cxa_exception* header = __get_exception_header_from_ue (eo);
  return header->adjustedPtr;
}

The idea is that although the eo might be a pointer to the
unwindHeader member  of either __cxa_exception or
__cxa_dependent_exception, the adjustedPtr member will be always be at
the same location relative to the unwindHeader member, so it Just
Works.

Except it doesn't.

offsetof(__cxa_exception, unwindHeader) == 80
offsetof(__cxa_dependent_exception, unwindHeader) == 80
offsetof(__cxa_exception, adjustedPtr) == 72
offsetof(__cxa_dependent_exception, adjustedPtr) == 64

This is the cause of PR 60612, and means we need to fix a lot of code
by explicitly checking the exception class (see attached patch for a
partial fix doing that in two places) or we need to change the layout
of __cxa_dependent_exception (and then add some tests to verify the
assumptions implicit in the code!)

I don't think this is all fixable in time for 4.9.0, but the attached
patch fixes one part of the problem, so I'm probably going to commit
it and then work on a more complete fix later.
commit 0e783c3897f1d1905bd64129e6eb9902a523626a
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Mon Mar 24 12:30:20 2014 +0000

    	PR libstdc++/60612
    	* libsupc++/eh_call.cc (__cxa_call_terminate): Handle dependent
    	exceptions.
    	* libsupc++/eh_personality.cc (__cxa_call_unexpected): Likewise.
    	* libsupc++/unwind-cxx.h: Fix typos in comments.
    	* testsuite/18_support/exception_ptr/60612-terminate.cc: New.
    	* testsuite/18_support/exception_ptr/60612-unexpected.cc: New.

Patch

diff --git a/libstdc++-v3/libsupc++/eh_call.cc b/libstdc++-v3/libsupc++/eh_call.cc
index 7677692..ac75813 100644
--- a/libstdc++-v3/libsupc++/eh_call.cc
+++ b/libstdc++-v3/libsupc++/eh_call.cc
@@ -48,10 +48,12 @@  __cxa_call_terminate(_Unwind_Exception* ue_header) throw ()
       // exception.  */
       if (__is_gxx_exception_class(ue_header->exception_class))
 	{
-	  __cxa_exception* xh;
-
-	  xh = __get_exception_header_from_ue(ue_header);
-	  __terminate(xh->terminateHandler);
+	  std::terminate_handler th;
+	  if (__is_dependent_exception(ue_header->exception_class))
+	    th = __get_dependent_exception_from_ue(ue_header)->terminateHandler;
+	  else
+	    th = __get_exception_header_from_ue(ue_header)->terminateHandler;
+	  __terminate(th);
 	}
     }
   /* Call the global routine if we don't have anything better.  */
diff --git a/libstdc++-v3/libsupc++/eh_personality.cc b/libstdc++-v3/libsupc++/eh_personality.cc
index f315a83..698dc7d 100644
--- a/libstdc++-v3/libsupc++/eh_personality.cc
+++ b/libstdc++-v3/libsupc++/eh_personality.cc
@@ -737,20 +737,35 @@  __cxa_call_unexpected (void *exc_obj_in)
   } end_catch_protect_obj;
 
   lsda_header_info info;
-  __cxa_exception *xh = __get_exception_header_from_ue (exc_obj);
   const unsigned char *xh_lsda;
   _Unwind_Sword xh_switch_value;
   std::terminate_handler xh_terminate_handler;
-
+  std::unexpected_handler xh_unexpected_handler;
   // If the unexpectedHandler rethrows the exception (e.g. to categorize it),
   // it will clobber data about the current handler.  So copy the data out now.
-  xh_lsda = xh->languageSpecificData;
-  xh_switch_value = xh->handlerSwitchValue;
-  xh_terminate_handler = xh->terminateHandler;
-  info.ttype_base = (_Unwind_Ptr) xh->catchTemp;
+  if (__is_dependent_exception(exc_obj->exception_class))
+    {
+      __cxa_dependent_exception *xh = __get_dependent_exception_from_ue (exc_obj);
+
+      xh_lsda = xh->languageSpecificData;
+      xh_switch_value = xh->handlerSwitchValue;
+      xh_terminate_handler = xh->terminateHandler;
+      xh_unexpected_handler = xh->unexpectedHandler;
+      info.ttype_base = (_Unwind_Ptr) xh->catchTemp;
+    }
+  else
+    {
+      __cxa_exception *xh = __get_exception_header_from_ue (exc_obj);
+
+      xh_lsda = xh->languageSpecificData;
+      xh_switch_value = xh->handlerSwitchValue;
+      xh_terminate_handler = xh->terminateHandler;
+      xh_unexpected_handler = xh->unexpectedHandler;
+      info.ttype_base = (_Unwind_Ptr) xh->catchTemp;
+    }
 
   __try 
-    { __unexpected (xh->unexpectedHandler); } 
+    { __unexpected (xh_unexpected_handler); }
   __catch(...) 
     {
       // Get the exception thrown from unexpected.
diff --git a/libstdc++-v3/libsupc++/unwind-cxx.h b/libstdc++-v3/libsupc++/unwind-cxx.h
index a7df603..1ca6d94 100644
--- a/libstdc++-v3/libsupc++/unwind-cxx.h
+++ b/libstdc++-v3/libsupc++/unwind-cxx.h
@@ -81,7 +81,7 @@  struct __cxa_exception
   // Stack of exceptions in cleanups.
   __cxa_exception* nextPropagatingException;
 
-  // The nuber of active cleanup handlers for this exception.
+  // The number of active cleanup handlers for this exception.
   int propagationCount;
 #else
   // Cache parsed handler data from the personality routine Phase 1
@@ -130,7 +130,7 @@  struct __cxa_dependent_exception
   // Stack of exceptions in cleanups.
   __cxa_exception* nextPropagatingException;
 
-  // The nuber of active cleanup handlers for this exception.
+  // The number of active cleanup handlers for this exception.
   int propagationCount;
 #else
   // Cache parsed handler data from the personality routine Phase 1
diff --git a/libstdc++-v3/testsuite/18_support/exception_ptr/60612-terminate.cc b/libstdc++-v3/testsuite/18_support/exception_ptr/60612-terminate.cc
new file mode 100644
index 0000000..ec5940d
--- /dev/null
+++ b/libstdc++-v3/testsuite/18_support/exception_ptr/60612-terminate.cc
@@ -0,0 +1,41 @@ 
+// { dg-options "-std=gnu++11" }
+// { dg-require-atomic-builtins "" }
+
+// Copyright (C) 2014 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// PR libstdc++/60612
+
+#include <exception>
+#include <stdlib.h>
+
+void terminate() { _Exit(0); }
+
+void f() noexcept
+{
+  try {
+    throw 1;
+  } catch (...) {
+    std::set_terminate(terminate);
+    std::rethrow_exception(std::current_exception());
+  }
+}
+
+int main()
+{
+  f();
+}
diff --git a/libstdc++-v3/testsuite/18_support/exception_ptr/60612-unexpected.cc b/libstdc++-v3/testsuite/18_support/exception_ptr/60612-unexpected.cc
new file mode 100644
index 0000000..3f7e2cf
--- /dev/null
+++ b/libstdc++-v3/testsuite/18_support/exception_ptr/60612-unexpected.cc
@@ -0,0 +1,41 @@ 
+// { dg-options "-std=gnu++11" }
+// { dg-require-atomic-builtins "" }
+
+// Copyright (C) 2014 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// PR libstdc++/60612
+
+#include <exception>
+#include <stdlib.h>
+
+void unexpected() { _Exit(0); }
+
+void f() throw()
+{
+  try {
+    throw 1;
+  } catch (...) {
+    std::set_unexpected(unexpected);
+    std::rethrow_exception(std::current_exception());
+  }
+}
+
+int main()
+{
+  f();
+}