Date: Sun, 6 Mar 2022 19:09:11 +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: <F3B98D7D-FA04-4722-81BA-E1C6C25B21F8@FreeBSD.org> In-Reply-To: <20220306220125.3d1acd1597f07211120b6ffa@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> <4DF32938-153C-4682-8B54-8B9784DB25F0@FreeBSD.org> <D438E16C-AB7D-4759-AB97-53F8417BCB02@FreeBSD.org> <20220306220125.3d1acd1597f07211120b6ffa@dec.sakura.ne.jp>
next in thread | previous in thread | raw e-mail | index | archive | help
FWIW: In libobjc2, I=E2=80=99ve had to work around the four different = minor variants of the EH ABI that exist between different combinations = of the C++ runtime and unwind library. I=E2=80=99d recommend anyone who = actually needs to poke at these structures steals my code (which = configures a custom personality function for a function and throws a C++ = exception with known values in it through that frame and then goes to = find those values in the thrown object). Nothing else that I tried = worked reliably across *BSD, GNU/Linux, Android, and Darwin. David > On 6 Mar 2022, at 13:01, Tomoaki AOKI <junchoon@dec.sakura.ne.jp> = wrote: >=20 > +1 for fixing before 13.1. >=20 > Can .1 library as a compat library (misc/compat12 or something) an > option? >=20 > Or making an knob to choose layout (default libunwind one) and inform > users of binary-only consumers to "use the knob and rebuild base if > needed" as an errata notice? >=20 >=20 > On Sun, 6 Mar 2022 11:13:58 +0100 > Dimitry Andric <dim@FreeBSD.org> wrote: >=20 >> In an ideal world, we would just merge = https://github.com/libcxxrt/libcxxrt/commit/88bdf6b290da2f32fe212564bfad9c= 61481fb878 ("Specify double-word alignment for ARM unwind. This matches = the latest revisions from libunwind.") and the corresponding = https://github.com/libcxxrt/libcxxrt/commit/b96169641f794f7b8e28c1b78b66bb= 780445d0cd ("Updated Itanium unwind. This matches the latest revisions = from libunwind."). Then at least libcxxrt's and libunwind's idea of the = struct layouts and alignments would be the same. >>=20 >> However, that would still affect older __cxa_exception and = _Unwind_Exception consumers, at least those that rely explicitly on the = old sizes and alignments of those structs. We could bump libcxxrt.so to = .2, but then we'd still have to maintain the old .1 version for those = old consumers. >>=20 >> Btw, now that I think about it, I think we merged the unwind.h = changes for 13.1, and we should really fix them up before 13.1 goes out = the door. >>=20 >> -Dimitry >>=20 >>> On 6 Mar 2022, at 10:17, David Chisnall <theraven@FreeBSD.org> = wrote: >>>=20 >>> 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. >>>=20 >>> 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. >>>=20 >>> David >>>=20 >>>> 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 >>>> -- >>>> Tomoaki AOKI <junchoon@dec.sakura.ne.jp> >>>> <libcxxrt_errors_20220306-001.log> >>>=20 >>=20 >=20 >=20 > --=20 > Tomoaki AOKI <junchoon@dec.sakura.ne.jp>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?F3B98D7D-FA04-4722-81BA-E1C6C25B21F8>