std::rethrow_exception is broken

[François Dumont]

Shouldn't static assertion messages say that layout is incorrect ? It is 
strange to have compilation error with a message saying that it is 
correct, isn't it ?

François


On 25/03/2014 18:25, Jonathan Wakely wrote:
> 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.
>