From nobody Sun Mar 6 10:13:58 2022 X-Original-To: dev-commits-src-branches@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 9857819F84CF for ; Sun, 6 Mar 2022 10:14:06 +0000 (UTC) (envelope-from dim@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [IPv6:2610:1c1:1:606c::24b:4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4KBHW23BHqz4nnH; Sun, 6 Mar 2022 10:14:06 +0000 (UTC) (envelope-from dim@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1646561646; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=h2Ub8CoxC3Rkg8k+gxfkSI4LEsZbn9XACyWtbRCWxIA=; b=xCOJLGU0raZJ4XOSQXunzwlaYOn4V3ctXvaxft/LdoofjxrNJ3qcGm0hxf3bB0rnBaTdma o9Xl4Akj2Wlcjd/PVZ2AQlNmeBuvPRr3b7xYL8l8l1UjS0+JGocu0tixKNm6nimnZFxruf 6g0MGDAEVlgdOW+YNYUzqLRf1zTj7oFPivYm/f6+AgCpRXBgRp8k2/lfu10LL1wUuzH+cz OtnB+m42nRe+wdIOR9OVFVmq6nHKs5jDr+LQdJQSj9lq0CqYfHFNAnPFramApQHWnQztNl HGIdX0Azf1GZ3Rz5d3YstqJRYCdFXi6F3VrMhJt6Iq/hHvtvBCTH1Wz5/B/TCQ== Received: from tensor.andric.com (tensor.andric.com [87.251.56.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "tensor.andric.com", Issuer "R3" (verified OK)) (Authenticated sender: dim) by smtp.freebsd.org (Postfix) with ESMTPSA id 32C882C133; Sun, 6 Mar 2022 10:14:06 +0000 (UTC) (envelope-from dim@FreeBSD.org) Received: from smtpclient.apple (longrow.home.andric.com [192.168.0.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by tensor.andric.com (Postfix) with ESMTPSA id C48A633768; Sun, 6 Mar 2022 11:14:04 +0100 (CET) Content-Type: multipart/signed; boundary="Apple-Mail=_34223E91-A920-47C2-AA63-F723ED9EE3E7"; protocol="application/pgp-signature"; micalg=pgp-sha1 List-Id: Commits to the stable branches of the FreeBSD src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-branches List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-branches@freebsd.org X-BeenThere: dev-commits-src-branches@freebsd.org Mime-Version: 1.0 (Mac OS X Mail 15.0 \(3693.60.0.1.1\)) Subject: Re: git: b2127b6f1ae2 - stable/13 - Install unwind.h into /usr/include From: Dimitry Andric In-Reply-To: <4DF32938-153C-4682-8B54-8B9784DB25F0@FreeBSD.org> Date: Sun, 6 Mar 2022 11:13:58 +0100 Cc: Tomoaki AOKI , John Baldwin , Ed Maste , dev-commits-src-branches@freebsd.org Message-Id: References: <20220303224535.a0cca57e1f033e930a7f8f9d@dec.sakura.ne.jp> <3166E99F-1616-40D9-BD8B-D18E8104D6FF@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> To: David Chisnall X-Mailer: Apple Mail (2.3693.60.0.1.1) ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1646561646; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=h2Ub8CoxC3Rkg8k+gxfkSI4LEsZbn9XACyWtbRCWxIA=; b=lNg3Xao4Aj3Au/TEZQdW3luI9zz+IDtIgyAIaS7YwOtNx+GU0DZNxHdhy+zG/+lwSvCrws Vh3S7V8eQtD2RP/dqy40FnaUT21LsbYZg3dAMgU8uRH2Xnh1yYL6dax5w5gF/2EK5E9me+ XQ9Gh3my9h0F49gC0th5Pt+PdaM3MjgJo/Rmq2kr64gDDDE/lS27hu+m0Mh2D2pdmubHZU 9BY4O5xp5rBxSnmPv8murynEstUoOZbsMYcz93/Zou542OMMsv8345CFN/WB35NplS3luP Baacu6YFlHBav/Ez/VbYsDw8FUWhumO8XVk6Be2sby7DwaXVKBkHxVERxMu75Q== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1646561646; a=rsa-sha256; cv=none; b=GGlsmazertXJqRWTEz+4xw1GgcQS+T9XevGC87g0qhKHk3fNtRDqE55R6MCkJHNQwWUsOf aSLtTEs9zXWdBorbsMRLSNTbvWLbQq36FdH0fYUbH5hDIFObpP290e6fyL9gVGVARti2Q+ q2w9rMNAPzNZ9b4lFwfDmyBEXg6G41REeCaRkmJ2b5jY/H8txvghUGxGdlI+VwNPdbJPoW rbGLcbolYFdMhb/bhFoiiQKlRXZGHM4n6MhHLS6BhDwaDwkrHv4pe+1ogQWSD1/9ZAqI2I vbYjhcHDTgS90uNLTaLgI8GNOAfGFPPiyAvCs54E/qLWSnph/5BL4UWp4GnaAw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N --Apple-Mail=_34223E91-A920-47C2-AA63-F723ED9EE3E7 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 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. 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. 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. -Dimitry > On 6 Mar 2022, at 10:17, David Chisnall 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 = wrote: >>=20 >> On Sat, 5 Mar 2022 23:33:54 +0100 >> Dimitry Andric wrote: >>=20 >>> On 5 Mar 2022, at 22:34, Dimitry Andric wrote: >>>>=20 >>>> On 5 Mar 2022, at 03:36, Tomoaki AOKI = 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 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> = char*>(static_cast(_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(action.landing_pad)); ^ >> /usr/src/contrib/libcxxrt/exception.cc:1209:16: error: use of >> undeclared identifier 'context' _Unwind_SetIP(context, >> reinterpret_cast(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> 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 >> >=20 --Apple-Mail=_34223E91-A920-47C2-AA63-F723ED9EE3E7 Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename=signature.asc Content-Type: application/pgp-signature; name=signature.asc Content-Description: Message signed with OpenPGP -----BEGIN PGP SIGNATURE----- Version: GnuPG/MacGPG2 v2.2 iF0EARECAB0WIQR6tGLSzjX8bUI5T82wXqMKLiCWowUCYiSJZgAKCRCwXqMKLiCW o4DBAKD65ovS6HD6cUOTlNAyO09LKxGcLgCggt0pKXRudUTz1rHwdEZwgVfj7vo= =XQ/G -----END PGP SIGNATURE----- --Apple-Mail=_34223E91-A920-47C2-AA63-F723ED9EE3E7--