Re: git: b2127b6f1ae2 - stable/13 - Install unwind.h into /usr/include

Reply: David Chisnall : "Re: git: b2127b6f1ae2 - stable/13 - Install unwind.h into /usr/include"
In reply to: Dimitry Andric : "Re: git: b2127b6f1ae2 - stable/13 - Install unwind.h into /usr/include"
Go to: [ bottom of page ] [ top of archives ] [ this month ]
From: Tomoaki AOKI <junchoon_at_dec.sakura.ne.jp>
Date: Sun, 06 Mar 2022 08:46:44 UTC
On Sat, 5 Mar 2022 23:33:54 +0100
Dimitry Andric <dim@FreeBSD.org> wrote:

> On 5 Mar 2022, at 22:34, Dimitry Andric <dim@FreeBSD.org> wrote:
> > 
> > On 5 Mar 2022, at 03:36, Tomoaki AOKI <junchoon@dec.sakura.ne.jp> wrote:
> ...
> > So according to the spec, casting the void pointer 'thrown_exception' to
> > a __cxa_exception pointer, then subtracting 1, should give you the
> > original __cxa_exception struct. In this case, it subtracts 8 bytes,
> > going from 0x87b5aff00 to 0x87b5afe88.
> 
> Ugh, actually this was 120 bytes!
> 
> 
> > Now I do exactly the same in the libreoffice frame one below, where the
> > incoming void pointer 'pExc' is the previous 'thrown_exception' value:
> > 
> > (gdb) frame 1
> > #1  gcc3::deleteException (pExc=0x87b5aff00) at bridges/source/cpp_uno/gcc3_linux_x86-64/except.cxx:139
> > 139         OUString unoName( toUNOname( header->exceptionType->name() ) );
> > (gdb) print pExc
> > $33 = (void *) 0x87b5aff00
> > (gdb) print static_cast<__cxa_exception*>(pExc)-1
> > $34 = (__cxa_exception *) 0x87b5afe80
> > 
> > So in *this* function, subtracting 1 from a __cxa_exception pointer
> > subtracts 16 bytes instead, going from 0x87b5aff00 to 0x87b5afe80!
> 
> And this was 128 bytes instead. I think I now know what's going on,
> which is that our declaration of __cxa_exception changed its size from
> 120 bytes to 128 bytes, due to the new unwind headers.
> 
> Our libcxxrt cxxabi.h header has:
> 
>   struct __cxa_exception
>   {
>   ... lots of stuff ...
>           /** The language-agnostic part of the exception header. */
>           _Unwind_Exception unwindHeader;
>   };
> 
> so the last field is a struct _Unwind_Exception. Our libcxxrt
> unwind-itanium.h header has:
> 
>   struct _Unwind_Exception
>     {
>       uint64_t exception_class;
>       _Unwind_Exception_Cleanup_Fn exception_cleanup;
>       unsigned long private_1;
>       unsigned long private_2;
>     } ;
> 
> while libunwind's version has an __aligned__ attribute at the end:
> 
>   struct _Unwind_Exception {
>     uint64_t exception_class;
>     void (*exception_cleanup)(_Unwind_Reason_Code reason,
>                               _Unwind_Exception *exc);
>   #if defined(__SEH__) && !defined(__USING_SJLJ_EXCEPTIONS__)
>     uintptr_t private_[6];
>   #else
>     uintptr_t private_1; // non-zero means forced unwind
>     uintptr_t private_2; // holds sp that phase1 found for phase2 to use
>   #endif
>   #if __SIZEOF_POINTER__ == 4
>     // The implementation of _Unwind_Exception uses an attribute mode on the
>     // above fields which has the side effect of causing this whole struct to
>     // round up to 32 bytes in size (48 with SEH). To be more explicit, we add
>     // pad fields added for binary compatibility.
>     uint32_t reserved[3];
>   #endif
>     // The Itanium ABI requires that _Unwind_Exception objects are "double-word
>     // aligned".  GCC has interpreted this to mean "use the maximum useful
>     // alignment for the target"; so do we.
>   } __attribute__((__aligned__));
> 
> (Note that upstream libcxxrt also added the reserved field and aligned
> attribute, in https://github.com/libcxxrt/libcxxrt/commit/b9616964 !)
> 
> The aligned attribute on _Unwind_Exception causes the enclosing
> __cxa_exception struct to *also* be aligned maximally, growing it from
> 120 to 128 bytes on x86_64.
> 
> So this is a bit of a fine mess we are in. There are multiple issues
> here:
> 
> 1) We broke the ABI by increasing __cxa_exception's size.
> 
> 2) We compile libcxxrt against its *own* unwind headers, so it assumes a
> 120-byte __cxa_exception size. But all other programs use the libunwind
> headers, so they assume a 128 byte __cxa_exception size.
> 
> I guess LibreOffice is just a good example which breaks because it does
> this deep poking in exception-handling land, which most programs never
> go near. That said, LibreOffice also includes the unwind.h header
> installed by the libunwind-20201110 port, so that is yet *another*
> possible incompatibility!
> 
> But I think we must do something about this. The most backward
> compatible change would be to *remove* the aligned attribute from our
> _Unwind_Exception declaration, so the old __cxa_exception size is
> restored. The problem with that is that we have to carry a patch for
> libunwind forever.
> 
> The other way would be to force libcxxrt to use the libunwind headers
> instead of its own, so that at least libcxxrt and libunwind agree on the
> size and alignment of all these structures! But that may still lead to
> crashes for older consumers.
> 
> No easy way out, in any case... :-/
> 
> -Dimitry

Did a quick stupid test by

 *Replacing #include "unwind.h" by #include <unwind.h> in
  contrib/libcxxrt/cxxabi.h, the only file including unwind.h on
  contrib/cxxrt directory to pick it from standard place.

 *Rename unsind*.h to something else just to be sure.

and got errors by missing macros. At least BEGIN_PERSONALITY_FUNCTION.

Insufficient part would be needed to be extracted to additional header.

Emitted errors are as follows:

/usr/src/lib/libcxxrt# make
Building /usr/obj/usr/src/amd64.amd64/lib/libcxxrt/auxhelper.o
Building /usr/obj/usr/src/amd64.amd64/lib/libcxxrt/dynamic_cast.o
Building /usr/obj/usr/src/amd64.amd64/lib/libcxxrt/exception.o
/usr/src/contrib/libcxxrt/exception.cc:1084:1: error: unknown type name
'BEGIN_PERSONALITY_FUNCTION'
BEGIN_PERSONALITY_FUNCTION(__gxx_personality_v0) ^
/usr/src/contrib/libcxxrt/exception.cc:1084:49: error: expected ';'
after top level declarator
BEGIN_PERSONALITY_FUNCTION(__gxx_personality_v0) ^
                                                ;
/usr/src/contrib/libcxxrt/exception.cc:1098:42: error: use of
undeclared identifier 'exceptionClass'; did you mean 'exception_class'?
bool foreignException = !isCXXException(exceptionClass); ^~~~~~~~~~~~~~
                                                exception_class
/usr/src/contrib/libcxxrt/exception.cc:242:23: note: 'exception_class'
declared here static const uint64_t exception_class =
                      ^
/usr/src/contrib/libcxxrt/exception.cc:1101:2: error: expected
unqualified-id if (!foreignException)
        ^
/usr/src/contrib/libcxxrt/exception.cc:1112:91: error: use of
undeclared identifier 'context' reinterpret_cast<unsigned
char*>(static_cast<uintptr_t>(_Unwind_GetLanguageSpecificData(context)));
^ /usr/src/contrib/libcxxrt/exception.cc:1116:2: error: expected
unqualified-id if (0 == lsda_addr) { return
continueUnwinding(exceptionObject, context); } ^
/usr/src/contrib/libcxxrt/exception.cc:1137:2: error: expected
unqualified-id if (actions & _UA_SEARCH_PHASE)
        ^
/usr/src/contrib/libcxxrt/exception.cc:1176:2: error: expected
unqualified-id if (!(actions & _UA_HANDLER_FRAME))
        ^
/usr/src/contrib/libcxxrt/exception.cc:1188:2: error: expected
unqualified-id else if (foreignException)
        ^
/usr/src/contrib/libcxxrt/exception.cc:1195:2: error: expected
unqualified-id else if (ex->catchTemp == 0)
        ^
/usr/src/contrib/libcxxrt/exception.cc:1200:2: error: expected
unqualified-id else
        ^
/usr/src/contrib/libcxxrt/exception.cc:1209:2: error: C++ requires a
type specifier for all declarations _Unwind_SetIP(context,
reinterpret_cast<unsigned long>(action.landing_pad)); ^
/usr/src/contrib/libcxxrt/exception.cc:1209:16: error: use of
undeclared identifier 'context' _Unwind_SetIP(context,
reinterpret_cast<unsigned long>(action.landing_pad)); ^
/usr/src/contrib/libcxxrt/exception.cc:1210:2: error: C++ requires a
type specifier for all declarations _Unwind_SetGR(context,
__builtin_eh_return_data_regno(0), ^
/usr/src/contrib/libcxxrt/exception.cc:1210:16: error: use of
undeclared identifier 'context' _Unwind_SetGR(context,
__builtin_eh_return_data_regno(0), ^
/usr/src/contrib/libcxxrt/exception.cc:1211:48: error: use of
undeclared identifier 'exceptionObject' reinterpret_cast<unsigned
long>(exceptionObject)); ^
/usr/src/contrib/libcxxrt/exception.cc:1212:2: error: C++ requires a
type specifier for all declarations _Unwind_SetGR(context,
__builtin_eh_return_data_regno(1), selector); ^
/usr/src/contrib/libcxxrt/exception.cc:1212:16: error: use of
undeclared identifier 'context' _Unwind_SetGR(context,
__builtin_eh_return_data_regno(1), selector); ^
/usr/src/contrib/libcxxrt/exception.cc:1214:2: error: expected
unqualified-id return _URC_INSTALL_CONTEXT;
        ^
fatal error: too many errors emitted, stopping now [-ferror-limit=]
20 errors generated.
*** Error code 1

Stop.
make: stopped in /usr/src/lib/libcxxrt
.ERROR_TARGET='exception.o'
.ERROR_META_FILE='/usr/obj/usr/src/amd64.amd64/lib/libcxxrt/exception.o.meta'
.MAKE.LEVEL='0'
MAKEFILE=''
.MAKE.MODE='meta missing-filemon=yes missing-meta=yes silent=yes
verbose' _ERROR_CMD='c++  -O2 -pipe -fno-common
-isystem /usr/src/contrib/libcxxrt -nostdinc++ -march=haswell
-mretpoline -Wno-format-zero-length -fstack-protector-strong
-Wno-empty-body -Wno-string-plus-int -Wno-unused-const-variable
-Wno-error=unused-but-set-variable -Wno-tautological-compare
-Wno-unused-value -Wno-parentheses-equality -Wno-unused-function
-Wno-enum-conversion -Wno-unused-local-typedef
-Wno-address-of-packed-member -Wno-switch -Wno-switch-enum
-Wno-knr-promoted-parameter -Wno-parentheses -Qunused-arguments
-mretpoline -std=c++14    -Wno-c++11-extensions
-c /usr/src/contrib/libcxxrt/exception.cc -o
exception.o;' .CURDIR='/usr/src/lib/libcxxrt' .MAKE='make' .OBJDIR='/usr/obj/usr/src/amd64.amd64/lib/libcxxrt' .TARGETS='
all' DESTDIR='' LD_LIBRARY_PATH='' MACHINE='amd64' MACHINE_ARCH='amd64'
MAKEOBJDIRPREFIX='' MAKESYSPATH='/usr/src/share/mk'
MAKE_VERSION='20220208'
PATH='/sbin:/bin:/usr/sbin:/usr/bin:/usr/games:/usr/local/sbin:/usr/local/bin:/root/bin'
SRCTOP='/usr/src' OBJTOP='/usr/obj/usr/src/amd64.amd64'


-- 
Tomoaki AOKI    <junchoon@dec.sakura.ne.jp>