On 24/03/14 19:19 +0000, Jonathan Wakely wrote:
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

Here's the output of GDB's pahole on __cxa_exception

(gdb) pahole struct __cxxabiv1::__cxa_exception
               struct __cxxabiv1::__cxa_exception {
 /*   0   8 */    std::type_info * exceptionType
 /*   8   8 */    void (*)(void *) exceptionDestructor
 /*  16   8 */    void (*)(void) unexpectedHandler
 /*  24   8 */    void (*)(void) terminateHandler
 /*  32   8 */    __cxxabiv1::__cxa_exception * nextException
 /*  40   4 */    int handlerCount
 /*  44   4 */    int handlerSwitchValue
 /*  48   8 */    const unsigned char * actionRecord
 /*  56   8 */    const unsigned char * languageSpecificData
 /*  64   8 */    unsigned long catchTemp
 /*  72   8 */    void * adjustedPtr
 /*  80  32 */   struct _Unwind_Exception {
 /*   0   8 */      unsigned long exception_class
 /*   8   8 */      void (*)(_Unwind_Reason_Code, _Unwind_Exception *) 
exception_cleanup
 /*  16   8 */      unsigned long private_1
 /*  24   8 */      unsigned long private_2
                 } unwindHeader
} And here's the current definition of __cxa_dependent_exception:

(gdb) pahole struct __cxxabiv1::__cxa_dependent_exception struct __cxxabiv1::__cxa_dependent_exception {
 /*   0   8 */    void * primaryException
 /*   8   8 */    void (*)(void) unexpectedHandler
 /*  16   8 */    void (*)(void) terminateHandler
 /*  24   8 */    __cxxabiv1::__cxa_exception * nextException
 /*  32   4 */    int handlerCount
 /*  36   4 */    int handlerSwitchValue
 /*  40   8 */    const unsigned char * actionRecord
 /*  48   8 */    const unsigned char * languageSpecificData
 /*  56   8 */    unsigned long catchTemp
 /*  64   8 */    void * adjustedPtr
  /* XXX 64 bit hole, try to pack */
 /*  80  32 */   struct _Unwind_Exception {
 /*   0   8 */      unsigned long exception_class
 /*   8   8 */      void (*)(_Unwind_Reason_Code, _Unwind_Exception *) 
exception_cleanup
 /*  16   8 */      unsigned long private_1
 /*  24   8 */      unsigned long private_2
                 } unwindHeader
}
Notice the hole right before the unwindHeader member which is not
there in __cxa_exception. All the member which are supposed to be at
the same offsets as in __cxa_exception are 8 bytes earlier.

The attached patch adds a __padding member near the start so changes
that to:

(gdb) pahole struct __cxxabiv1::__cxa_dependent_exception struct __cxxabiv1::__cxa_dependent_exception {
 /*   0   8 */    void * primaryException
 /*   8   8 */    void (*)(void *) __padding
 /*  16   8 */    void (*)(void) unexpectedHandler
 /*  24   8 */    void (*)(void) terminateHandler
 /*  32   8 */    __cxxabiv1::__cxa_exception * nextException
 /*  40   4 */    int handlerCount
 /*  44   4 */    int handlerSwitchValue
 /*  48   8 */    const unsigned char * actionRecord
 /*  56   8 */    const unsigned char * languageSpecificData
 /*  64   8 */    unsigned long catchTemp
 /*  72   8 */    void * adjustedPtr
 /*  80  32 */   struct _Unwind_Exception {
 /*   0   8 */      unsigned long exception_class
 /*   8   8 */      void (*)(_Unwind_Reason_Code, _Unwind_Exception *) 
exception_cleanup
 /*  16   8 */      unsigned long private_1
 /*  24   8 */      unsigned long private_2
                 } unwindHeader
}
That change allows all the pointer tricks in libsupc++/eh_*.cc to
continue working.  It's a change to the layout of a low-level type,
which is not visible to users linking to libsupc++.so, but anyone
linking statically will not be able to mix code using GCC 4.9's
libsupc++.a with the libsupc++.a from previous versions if they use
std::rethrow_exception() anywhere.

Tested x86_64-linux, I plan to commit this to trunk soon.

commit 06a845f80204947afd6866109db58cc85dc87117
Author: Jonathan Wakely <jwak...@redhat.com>
Date:   Tue Mar 25 14:42:45 2014 +0000

        PR libstdc++/60612
        * libsupc++/eh_ptr.cc: Assert __cxa_dependent_exception layout is
        compatible with __cxa_exception.
        * libsupc++/unwind-cxx.h (__cxa_dependent_exception): Add padding.
        Fix typos in comments.
        * testsuite/18_support/exception_ptr/60612-terminate.cc: New.
        * testsuite/18_support/exception_ptr/60612-unexpected.cc: New.

diff --git a/libstdc++-v3/libsupc++/eh_ptr.cc b/libstdc++-v3/libsupc++/eh_ptr.cc
index 6508749..8c25a81 100644
--- a/libstdc++-v3/libsupc++/eh_ptr.cc
+++ b/libstdc++-v3/libsupc++/eh_ptr.cc
@@ -35,6 +35,32 @@
 
 using namespace __cxxabiv1;
 
+// Verify assumptions about member layout in exception types
+namespace
+{
+template<typename Ex>
+  constexpr std::size_t unwindhdr()
+  { return offsetof(Ex, unwindHeader); }
+
+template<typename Ex>
+  constexpr std::size_t termHandler()
+  { return unwindhdr<Ex>() - offsetof(Ex, terminateHandler); }
+
+static_assert( termHandler<__cxa_exception>()
+              == termHandler<__cxa_dependent_exception>(),
+              "__cxa_dependent_exception::termHandler layout is correct" );
+
+#ifndef __ARM_EABI_UNWINDER__
+template<typename Ex>
+  constexpr std::ptrdiff_t adjptr()
+  { return unwindhdr<Ex>() - offsetof(Ex, adjustedPtr); }
+
+static_assert( adjptr<__cxa_exception>()
+              == adjptr<__cxa_dependent_exception>(),
+              "__cxa_dependent_exception::adjustedPtr layout is correct" );
+#endif
+}
+
 std::__exception_ptr::exception_ptr::exception_ptr() _GLIBCXX_USE_NOEXCEPT
 : _M_exception_object(0) { }
 
diff --git a/libstdc++-v3/libsupc++/unwind-cxx.h 
b/libstdc++-v3/libsupc++/unwind-cxx.h
index a7df603..f57c536 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
@@ -114,6 +114,11 @@ struct __cxa_dependent_exception
   // The primary exception this thing depends on.
   void *primaryException;
 
+  // Unused member to get similar layout to __cxa_exception, otherwise the
+  // alignment requirements of _Unwind_Exception would require padding bytes
+  // before the unwindHeader member.
+  void (_GLIBCXX_CDTOR_CALLABI *__padding)(void *);
+
   // The C++ standard has entertaining rules wrt calling set_terminate
   // and set_unexpected in the middle of the exception cleanup process.
   std::unexpected_handler unexpectedHandler;
@@ -130,7 +135,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();
+}

Reply via email to