git: 9097e3cbcac4 - main - Partially revert libcxxrt changes to avoid _Unwind_Exception change

[Tijl Coosemans]

On Sat, 13 Mar 2021 13:54:49 GMT Dimitry Andric <dim at 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 at FreeBSD.org>
> AuthorDate: 2021-03-13 13:54:24 +0000
> Commit:     Dimitry Andric <dim at 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?