Date: Sat, 13 Mar 2021 18:38:12 +0100 From: =?UTF-8?B?VMSzbA==?= Coosemans <tijl@FreeBSD.org> To: Dimitry Andric <dim@FreeBSD.org> Cc: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: Re: git: 9097e3cbcac4 - main - Partially revert libcxxrt changes to avoid _Unwind_Exception change Message-ID: <20210313183812.77b74819@FreeBSD.org> In-Reply-To: <202103131354.12DDsnKR065154@gitrepo.freebsd.org> References: <202103131354.12DDsnKR065154@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, 13 Mar 2021 13:54:49 GMT Dimitry Andric <dim@FreeBSD.org> wrote: > The branch main has been updated by dim: > > URL: https://cgit.FreeBSD.org/src/commit/?id=9097e3cbcac455eb0dedd097d8d5548c72568d0a > > commit 9097e3cbcac455eb0dedd097d8d5548c72568d0a > Author: Dimitry Andric <dim@FreeBSD.org> > AuthorDate: 2021-03-13 13:54:24 +0000 > Commit: Dimitry Andric <dim@FreeBSD.org> > CommitDate: 2021-03-13 13:54:24 +0000 > > Partially revert libcxxrt changes to avoid _Unwind_Exception change > > (Note I am also applying this to main and stable/13, to restore the old > libcxxrt ABI and to avoid having to maintain a compat library.) > > After the recent cherry-picking of libcxxrt commits 0ee0dbfb0d26 and > d2b3fadf2db5, users reported that editors/libreoffice packages from the > official package builders did not start anymore. It turns out that the > combination of these commits subtly changes the ABI, requiring all > applications that depend on internal details of struct _Unwind_Exception > (available via unwind-arm.h and unwind-itanium.h) to be recompiled. > > However, the FreeBSD package builders always use -RELEASE jails, so > these still use the old declaration of struct _Unwind_Exception, which > is not entirely compatible. In particular, LibreOffice uses this struct > in its internal "uno bridge" component, where it attempts to setup its > own exception handling mechanism. > > To fix this incompatibility, go back to the old declarations of struct > _Unwind_Exception, and restore the __LP64__ specific workaround we had > in place before (which was to cope with yet another, older ABI bug). > > Effectively, this reverts upstream libcxxrt commits 88bdf6b290da > ("Specify double-word alignment for ARM unwind") and b96169641f79 > ("Updated Itanium unwind"), and reapplies our commit 3c4fd2463bb2 > ("libcxxrt: add padding in __cxa_allocate_* to fix alignment"). > > PR: 253840 > --- > contrib/libcxxrt/exception.cc | 30 ++++++++++++++++++++++++------ > contrib/libcxxrt/unwind-arm.h | 2 +- > contrib/libcxxrt/unwind-itanium.h | 9 +++------ > 3 files changed, 28 insertions(+), 13 deletions(-) > > diff --git a/contrib/libcxxrt/exception.cc b/contrib/libcxxrt/exception.cc > index 0fb26ddb4ed2..0de878e9e6db 100644 > --- a/contrib/libcxxrt/exception.cc > +++ b/contrib/libcxxrt/exception.cc > @@ -572,6 +572,19 @@ static void free_exception(char *e) > } > } > > +#ifdef __LP64__ > +/** > + * There's an ABI bug in __cxa_exception: unwindHeader requires 16-byte > + * alignment but it was broken by the addition of the referenceCount. > + * The unwindHeader is at offset 0x58 in __cxa_exception. In order to keep > + * compatibility with consumers of the broken __cxa_exception, explicitly add > + * padding on allocation (and account for it on free). > + */ > +static const int exception_alignment_padding = 8; > +#else > +static const int exception_alignment_padding = 0; > +#endif > + > /** > * Allocates an exception structure. Returns a pointer to the space that can > * be used to store an object of thrown_size bytes. This function will use an > @@ -580,16 +593,19 @@ static void free_exception(char *e) > */ > extern "C" void *__cxa_allocate_exception(size_t thrown_size) > { > - size_t size = thrown_size + sizeof(__cxa_exception); > + size_t size = exception_alignment_padding + sizeof(__cxa_exception) + > + thrown_size; > char *buffer = alloc_or_die(size); > - return buffer+sizeof(__cxa_exception); > + return buffer + exception_alignment_padding + sizeof(__cxa_exception); > } > > extern "C" void *__cxa_allocate_dependent_exception(void) > { > - size_t size = sizeof(__cxa_dependent_exception); > + size_t size = exception_alignment_padding + > + sizeof(__cxa_dependent_exception); > char *buffer = alloc_or_die(size); > - return buffer+sizeof(__cxa_dependent_exception); > + return buffer + exception_alignment_padding + > + sizeof(__cxa_dependent_exception); > } > > /** > @@ -617,7 +633,8 @@ extern "C" void __cxa_free_exception(void *thrown_exception) > } > } > > - free_exception(reinterpret_cast<char*>(ex)); > + free_exception(reinterpret_cast<char*>(ex) - > + exception_alignment_padding); > } > > static void releaseException(__cxa_exception *exception) > @@ -644,7 +661,8 @@ void __cxa_free_dependent_exception(void *thrown_exception) > { > releaseException(realExceptionFromException(reinterpret_cast<__cxa_exception*>(ex))); > } > - free_exception(reinterpret_cast<char*>(ex)); > + free_exception(reinterpret_cast<char*>(ex) - > + exception_alignment_padding); > } > > /** > diff --git a/contrib/libcxxrt/unwind-arm.h b/contrib/libcxxrt/unwind-arm.h > index ec81237e573b..6eb9d9e45981 100644 > --- a/contrib/libcxxrt/unwind-arm.h > +++ b/contrib/libcxxrt/unwind-arm.h > @@ -97,7 +97,7 @@ struct _Unwind_Exception > } pr_cache; > /** Force alignment of next item to 8-byte boundary */ > long long int :0; > -} __attribute__((__aligned__(8))); > +}; > > /* Unwinding functions */ > _Unwind_Reason_Code _Unwind_RaiseException(struct _Unwind_Exception *ucbp); > diff --git a/contrib/libcxxrt/unwind-itanium.h b/contrib/libcxxrt/unwind-itanium.h > index 199d91de283d..1ee0cf0e81c4 100644 > --- a/contrib/libcxxrt/unwind-itanium.h > +++ b/contrib/libcxxrt/unwind-itanium.h > @@ -79,12 +79,9 @@ struct _Unwind_Exception > { > uint64_t exception_class; > _Unwind_Exception_Cleanup_Fn exception_cleanup; > - uintptr_t private_1; > - uintptr_t private_2; > -#if __SIZEOF_POINTER__ == 4 > - uint32_t reserved[3]; > -#endif > - } __attribute__((__aligned__)); > + unsigned long private_1; > + unsigned long private_2; > + } ; Shouldn't these definitions be the same as the ones in GCC?
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20210313183812.77b74819>