Date: Sun, 6 Mar 2022 09:17:40 +0000 From: David Chisnall <theraven@FreeBSD.org> To: Tomoaki AOKI <junchoon@dec.sakura.ne.jp> Cc: Dimitry Andric <dim@FreeBSD.org>, John Baldwin <jhb@freebsd.org>, Ed Maste <emaste@freebsd.org>, dev-commits-src-branches@freebsd.org Subject: Re: git: b2127b6f1ae2 - stable/13 - Install unwind.h into /usr/include Message-ID: <4DF32938-153C-4682-8B54-8B9784DB25F0@FreeBSD.org> In-Reply-To: <20220306174644.dca71df7cf1400ea48cafaca@dec.sakura.ne.jp> References: <20220303224535.a0cca57e1f033e930a7f8f9d@dec.sakura.ne.jp> <3166E99F-1616-40D9-BD8B-D18E8104D6FF@FreeBSD.org> <a9863238-04e5-381d-105d-311a2b0c3f56@FreeBSD.org> <6E5AD771-3140-480A-BF20-95B8E8A27189@FreeBSD.org> <20220305113613.7760bb3845cda1c04707fb34@dec.sakura.ne.jp> <7482C54A-1128-4AFF-A74E-53D88334C1D9@FreeBSD.org> <682D0394-CDC8-44AB-B9FF-2B969AE2D39A@FreeBSD.org> <20220306174644.dca71df7cf1400ea48cafaca@dec.sakura.ne.jp>
next in thread | previous in thread | raw e-mail | index | archive | help
libcxxrt provides its own unwind.h that includes an abstraction layer =
over the differences between the Arm and Itanium ABIs. It provides all =
of the standard definitions in addition to this, so you should be able =
to build other things against it, but you won=E2=80=99t be able to build =
it with a different unwind.h.
You could potentially split the unwind.h that it provides into two =
headers, one with only the standard definitions and one with the others, =
I=E2=80=99d be happy to review a PR to do this.
David
> On 6 Mar 2022, at 08:46, Tomoaki AOKI <junchoon@dec.sakura.ne.jp> =
wrote:
>=20
> On Sat, 5 Mar 2022 23:33:54 +0100
> Dimitry Andric <dim@FreeBSD.org> wrote:
>=20
>> On 5 Mar 2022, at 22:34, Dimitry Andric <dim@FreeBSD.org> wrote:
>>>=20
>>> 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.
>>=20
>> Ugh, actually this was 120 bytes!
>>=20
>>=20
>>> Now I do exactly the same in the libreoffice frame one below, where =
the
>>> incoming void pointer 'pExc' is the previous 'thrown_exception' =
value:
>>>=20
>>> (gdb) frame 1
>>> #1 gcc3::deleteException (pExc=3D0x87b5aff00) at =
bridges/source/cpp_uno/gcc3_linux_x86-64/except.cxx:139
>>> 139 OUString unoName( toUNOname( =
header->exceptionType->name() ) );
>>> (gdb) print pExc
>>> $33 =3D (void *) 0x87b5aff00
>>> (gdb) print static_cast<__cxa_exception*>(pExc)-1
>>> $34 =3D (__cxa_exception *) 0x87b5afe80
>>>=20
>>> So in *this* function, subtracting 1 from a __cxa_exception pointer
>>> subtracts 16 bytes instead, going from 0x87b5aff00 to 0x87b5afe80!
>>=20
>> 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.
>>=20
>> Our libcxxrt cxxabi.h header has:
>>=20
>> struct __cxa_exception
>> {
>> ... lots of stuff ...
>> /** The language-agnostic part of the exception header. */
>> _Unwind_Exception unwindHeader;
>> };
>>=20
>> so the last field is a struct _Unwind_Exception. Our libcxxrt
>> unwind-itanium.h header has:
>>=20
>> struct _Unwind_Exception
>> {
>> uint64_t exception_class;
>> _Unwind_Exception_Cleanup_Fn exception_cleanup;
>> unsigned long private_1;
>> unsigned long private_2;
>> } ;
>>=20
>> while libunwind's version has an __aligned__ attribute at the end:
>>=20
>> 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__ =3D=3D 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__));
>>=20
>> (Note that upstream libcxxrt also added the reserved field and =
aligned
>> attribute, in https://github.com/libcxxrt/libcxxrt/commit/b9616964 !)
>>=20
>> 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.
>>=20
>> So this is a bit of a fine mess we are in. There are multiple issues
>> here:
>>=20
>> 1) We broke the ABI by increasing __cxa_exception's size.
>>=20
>> 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.
>>=20
>> 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!
>>=20
>> 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.
>>=20
>> 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.
>>=20
>> No easy way out, in any case... :-/
>>=20
>> -Dimitry
>=20
> Did a quick stupid test by
>=20
> *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.
>=20
> *Rename unsind*.h to something else just to be sure.
>=20
> and got errors by missing macros. At least BEGIN_PERSONALITY_FUNCTION.
>=20
> Insufficient part would be needed to be extracted to additional =
header.
>=20
> Emitted errors are as follows:
>=20
> /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 =3D !isCXXException(exceptionClass); =
^~~~~~~~~~~~~~
> exception_class
> /usr/src/contrib/libcxxrt/exception.cc:242:23: note: 'exception_class'
> declared here static const uint64_t exception_class =3D
> ^
> /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 =3D=3D 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 =3D=3D 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=3D]
> 20 errors generated.
> *** Error code 1
>=20
> Stop.
> make: stopped in /usr/src/lib/libcxxrt
> .ERROR_TARGET=3D'exception.o'
> =
.ERROR_META_FILE=3D'/usr/obj/usr/src/amd64.amd64/lib/libcxxrt/exception.o.=
meta'
> .MAKE.LEVEL=3D'0'
> MAKEFILE=3D''
> .MAKE.MODE=3D'meta missing-filemon=3Dyes missing-meta=3Dyes silent=3Dyes=
> verbose' _ERROR_CMD=3D'c++ -O2 -pipe -fno-common
> -isystem /usr/src/contrib/libcxxrt -nostdinc++ -march=3Dhaswell
> -mretpoline -Wno-format-zero-length -fstack-protector-strong
> -Wno-empty-body -Wno-string-plus-int -Wno-unused-const-variable
> -Wno-error=3Dunused-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=3Dc++14 -Wno-c++11-extensions
> -c /usr/src/contrib/libcxxrt/exception.cc -o
> exception.o;' .CURDIR=3D'/usr/src/lib/libcxxrt' .MAKE=3D'make' =
.OBJDIR=3D'/usr/obj/usr/src/amd64.amd64/lib/libcxxrt' .TARGETS=3D'
> all' DESTDIR=3D'' LD_LIBRARY_PATH=3D'' MACHINE=3D'amd64' =
MACHINE_ARCH=3D'amd64'
> MAKEOBJDIRPREFIX=3D'' MAKESYSPATH=3D'/usr/src/share/mk'
> MAKE_VERSION=3D'20220208'
> =
PATH=3D'/sbin:/bin:/usr/sbin:/usr/bin:/usr/games:/usr/local/sbin:/usr/loca=
l/bin:/root/bin'
> SRCTOP=3D'/usr/src' OBJTOP=3D'/usr/obj/usr/src/amd64.amd64'
>=20
>=20
> --=20
> Tomoaki AOKI <junchoon@dec.sakura.ne.jp>
> <libcxxrt_errors_20220306-001.log>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4DF32938-153C-4682-8B54-8B9784DB25F0>