Re: git: c00d34566536 - main - Install unwind.h into /usr/include

From: Don Lewis <truckman_at_FreeBSD.org>
Date: Thu, 21 Apr 2022 01:35:15 UTC
On 20 Apr, Don Lewis wrote:
> On 20 Apr, Dimitry Andric wrote:
>>> On 20 Apr 2022, at 03:42, Don Lewis <truckman@FreeBSD.org> wrote:
>>> On 10 Feb, Dimitry Andric wrote:
>>>> The branch main has been updated by dim:
>>>> 
>>>> URL:
>>>> https://cgit.FreeBSD.org/src/commit/?id=c00d345665366a89aaba7244d6f078dc756f4c53
>>>> 
>>>> commit c00d345665366a89aaba7244d6f078dc756f4c53
>>>> Author:     John Baldwin <jhb@FreeBSD.org>
>>>> AuthorDate: 2022-02-10 17:57:49 +0000
>>>> Commit:     Dimitry Andric <dim@FreeBSD.org>
>>>> CommitDate: 2022-02-10 18:00:32 +0000
>>>> 
>>>>    Install unwind.h into /usr/include
>>>> 
>>>>    Install headers from LLVM's libunwind in place of the headers
>>>>    from libcxxrt and allow C applications to use the library.
>>>> 
>>>>    As part of this, remove include/unwind.h and switch libthr over
>>>>    to using the installed unwind.h.
>>>> 
>>>>    Reviewed by:    dim, emaste
>>>>    MFC after:      10 days
>>>>    Differential Revision: https://reviews.freebsd.org/D34065
>>>> ---
>>>> ObsoleteFiles.inc                        |   5 +
>>>> include/unwind.h                         | 160 -------------------------------
>>>> lib/libc++/Makefile                      |   3 -
>>>> lib/libgcc_eh/Makefile                   |   4 +
>>>> lib/libgcc_eh/Makefile.inc               |   1 -
>>>> lib/libthr/Makefile                      |   1 -
>>>> tools/build/mk/OptionalObsoleteFiles.inc |   3 -
>>>> 7 files changed, 9 insertions(+), 168 deletions(-)
>>>> 
>>>> diff --git a/ObsoleteFiles.inc b/ObsoleteFiles.inc
>>>> index 1d4c3cb4e5c2..2cac44bbd715 100644
>>>> --- a/ObsoleteFiles.inc
>>>> +++ b/ObsoleteFiles.inc
>>>> @@ -52,6 +52,11 @@
>>>> #   xargs -n1 | sort | uniq -d;
>>>> # done
>>>> 
>>>> +# 20220210: unwind.h moved to /usr/include
>>>> +OLD_FILES+=usr/include/c++/v1/unwind-arm.h
>>>> +OLD_FILES+=usr/include/c++/v1/unwind-itanium.h
>>>> +OLD_FILES+=usr/include/c++/v1/unwind.h
>>>> +
>>>> # 20220128: mips pmc events removed
>>>> OLD_FILES+=usr/share/man/man3/pmc.mips24k.3
>>>> OLD_FILES+=usr/share/man/man3/pmc.octeon.3
>>>> diff --git a/include/unwind.h b/include/unwind.h
>>>> deleted file mode 100644
>>>> index a872c0a094ba..000000000000
>>>> --- a/include/unwind.h
>>>> +++ /dev/null
>>>> @@ -1,160 +0,0 @@
>>>> -/* $FreeBSD$ */
>>>> -
>>>> -/*-
>>>> -   libunwind - a platform-independent unwind library
>>>> -
>>>> -   SPDX-License-Identifier: ISC
>>>> -
>>>> -   Copyright (C) 2003 Hewlett-Packard Co
>>>> -	Contributed by David Mosberger-Tang <davidm@hpl.hp.com>
>>>> -
>>>> -This file is part of libunwind.
>>>> -
>>>> -Permission is hereby granted, free of charge, to any person
>>>> obtaining
>>>> -a copy of this software and associated documentation files (the
>>>> -"Software"), to deal in the Software without restriction, including
>>>> -without limitation the rights to use, copy, modify, merge, publish,
>>>> -distribute, sublicense, and/or sell copies of the Software, and to
>>>> -permit persons to whom the Software is furnished to do so, subject
>>>> to
>>>> -the following conditions:
>>>> -
>>>> -The above copyright notice and this permission notice shall be
>>>> -included in all copies or substantial portions of the Software.
>>>> -
>>>> -THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>>>> -EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>>>> -MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
>>>> -NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
>>>> BE
>>>> -LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
>>>> ACTION
>>>> -OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
>>>> CONNECTION
>>>> -WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.  */
>>>> -
>>>> -#ifndef _UNWIND_H
>>>> -#define _UNWIND_H
>>>> -
>>>> -#include <sys/_types.h>
>>>> -
>>>> -#ifdef __cplusplus
>>>> -extern "C" {
>>>> -#endif
>>>> -
>>>> -/* Minimal interface as per C++ ABI draft standard:
>>>> -
>>>> -	http://www.codesourcery.com/cxx-abi/abi-eh.html */
>>>> -
>>>> -typedef enum
>>>> -  {
>>>> -    _URC_NO_REASON = 0,
>>>> -    _URC_FOREIGN_EXCEPTION_CAUGHT = 1,
>>>> -    _URC_FATAL_PHASE2_ERROR = 2,
>>>> -    _URC_FATAL_PHASE1_ERROR = 3,
>>>> -    _URC_NORMAL_STOP = 4,
>>>> -    _URC_END_OF_STACK = 5,
>>>> -    _URC_HANDLER_FOUND = 6,
>>>> -    _URC_INSTALL_CONTEXT = 7,
>>>> -    _URC_CONTINUE_UNWIND = 8
>>>> -  }
>>>> -_Unwind_Reason_Code;
>>>> -
>>>> -typedef int _Unwind_Action;
>>>> -
>>>> -#define _UA_SEARCH_PHASE	1
>>>> -#define _UA_CLEANUP_PHASE	2
>>>> -#define _UA_HANDLER_FRAME	4
>>>> -#define _UA_FORCE_UNWIND	8
>>>> -
>>>> -struct _Unwind_Context;		/* opaque data-structure */
>>>> -struct _Unwind_Exception;	/* forward-declaration */
>>>> -
>>>> -typedef void (*_Unwind_Exception_Cleanup_Fn) (_Unwind_Reason_Code,
>>>> -					      struct _Unwind_Exception *);
>>>> -
>>>> -typedef _Unwind_Reason_Code (*_Unwind_Stop_Fn) (int, _Unwind_Action,
>>>> -						__uint64_t,
>>>> -						struct _Unwind_Exception *,
>>>> -						struct _Unwind_Context *,
>>>> -						void *);
>>>> -
>>>> -/* The C++ ABI requires exception_class, private_1, and private_2
>>>> to
>>>> -   be of type uint64 and the entire structure to be
>>>> -   double-word-aligned, but that seems a bit overly IA-64-specific.
>>>> -   Using "unsigned long" instead should give us the desired effect
>>>>     on
>>>> -   IA-64, while being more general.  */
>>>> -struct _Unwind_Exception
>>>> -  {
>>>> -    __uint64_t exception_class;
>>>> -    _Unwind_Exception_Cleanup_Fn exception_cleanup;
>>>> -    unsigned long private_1;
>>>> -    unsigned long private_2;
>>>> -  };
>>>> -
>>>> -extern _Unwind_Reason_Code _Unwind_RaiseException (struct
>>>> _Unwind_Exception *);
>>>> -extern _Unwind_Reason_Code _Unwind_ForcedUnwind (struct
>>>> _Unwind_Exception *,
>>>> -						 _Unwind_Stop_Fn, void *);
>>>> -extern void _Unwind_Resume (struct _Unwind_Exception *);
>>>> -extern void _Unwind_DeleteException (struct _Unwind_Exception *);
>>>> -extern unsigned long _Unwind_GetGR (struct _Unwind_Context *, int);
>>>> -extern void _Unwind_SetGR (struct _Unwind_Context *, int, unsigned
>>>> long);
>>>> -extern unsigned long _Unwind_GetIP (struct _Unwind_Context *);
>>>> -extern unsigned long _Unwind_GetIPInfo (struct _Unwind_Context *,
>>>> int *);
>>>> -extern void _Unwind_SetIP (struct _Unwind_Context *, unsigned long);
>>>> -extern unsigned long _Unwind_GetLanguageSpecificData (struct
>>>> _Unwind_Context*);
>>>> -extern unsigned long _Unwind_GetRegionStart (struct _Unwind_Context *);
>>>> -
>>>> -#if defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
>>>> -
>>>> -/* Callback for _Unwind_Backtrace().  The backtrace stops
>>>> immediately
>>>> -   if the callback returns any value other than _URC_NO_REASON. */
>>>> -typedef _Unwind_Reason_Code (*_Unwind_Trace_Fn) (struct
>>>> _Unwind_Context *,
>>>> -						 void *);
>>>> -
>>>> -/* See http://gcc.gnu.org/ml/gcc-patches/2001-09/msg00082.html for
>>>> why
>>>> -   _UA_END_OF_STACK exists.  */
>>>> -# define _UA_END_OF_STACK	16
>>>> -
>>>> -/* If the unwind was initiated due to a forced unwind, resume that
>>>> -   operation, else re-raise the exception.  This is used by
>>>> -   __cxa_rethrow().  */
>>>> -extern _Unwind_Reason_Code
>>>> -	  _Unwind_Resume_or_Rethrow (struct _Unwind_Exception *);
>>>> -
>>>> -/* See http://gcc.gnu.org/ml/gcc-patches/2003-09/msg00154.html for
>>>> why
>>>> -   _Unwind_GetBSP() exists.  */
>>>> -extern unsigned long _Unwind_GetBSP (struct _Unwind_Context *);
>>>> -
>>>> -/* Return the "canonical frame address" for the given context.
>>>> -   This is used by NPTL... */
>>>> -extern uintptr_t _Unwind_GetCFA (struct _Unwind_Context *);
>>>> -
>>>> -/* Return the base-address for data references.  */
>>>> -extern unsigned long _Unwind_GetDataRelBase (struct _Unwind_Context *);
>>>> -
>>>> -/* Return the base-address for text references.  */
>>>> -extern unsigned long _Unwind_GetTextRelBase (struct _Unwind_Context *);
>>>> -
>>>> -/* Call _Unwind_Trace_Fn once for each stack-frame, without doing
>>>> any
>>>> -   cleanup.  The first frame for which the callback is invoked is
>>>>     the
>>>> -   one for the caller of _Unwind_Backtrace().  _Unwind_Backtrace()
>>>> -   returns _URC_END_OF_STACK when the backtrace stopped due to
>>>> -   reaching the end of the call-chain or _URC_FATAL_PHASE1_ERROR if
>>>>     it
>>>> -   stops for any other reason.  */
>>>> -extern _Unwind_Reason_Code _Unwind_Backtrace (_Unwind_Trace_Fn,
>>>> void *);
>>>> -
>>>> -/* Find the start-address of the procedure containing the specified
>>>> IP
>>>> -   or NULL if it cannot be found (e.g., because the function has no
>>>> -   unwind info).  Note: there is not necessarily a one-to-one
>>>> -   correspondence between source-level functions and procedures:
>>>>     some
>>>> -   functions don't have unwind-info and others are split into
>>>>     multiple
>>>> -   procedures.  */
>>>> -extern void *_Unwind_FindEnclosingFunction (void *);
>>>> -
>>>> -/* See also Linux Standard Base Spec:
>>>> -    http://www.linuxbase.org/spec/refspecs/LSB_1.3.0/gLSB/gLSB/libgcc-s.html */
>>>> -
>>>> -#endif /* _GNU_SOURCE || _BSD_SOURCE */
>>>> -
>>>> -#ifdef __cplusplus
>>>> -};
>>>> -#endif
>>>> -
>>>> -#endif /* _UNWIND_H */
>>>> diff --git a/lib/libc++/Makefile b/lib/libc++/Makefile
>>>> index dda8bc1772c9..fd43983481b2 100644
>>>> --- a/lib/libc++/Makefile
>>>> +++ b/lib/libc++/Makefile
>>>> @@ -239,9 +239,6 @@ STD+=		${HDRDIR}/${hdr}
>>>> STD+=		${.CURDIR}/__config_site
>>>> 
>>>> RT_HEADERS+=	cxxabi.h
>>>> -RT_HEADERS+=	unwind-arm.h
>>>> -RT_HEADERS+=	unwind-itanium.h
>>>> -RT_HEADERS+=	unwind.h
>>>> .for hdr in ${RT_HEADERS}
>>>> STD+=		${_LIBCXXRTDIR}/${hdr}
>>>> .endfor
>>>> diff --git a/lib/libgcc_eh/Makefile b/lib/libgcc_eh/Makefile
>>>> index ecffbf9cfd6a..6f2deda1adf0 100644
>>>> --- a/lib/libgcc_eh/Makefile
>>>> +++ b/lib/libgcc_eh/Makefile
>>>> @@ -11,6 +11,10 @@ WARNS?=	2
>>>> SRCS_EXC+=	int_util.c
>>>> .include "Makefile.inc"
>>>> 
>>>> +INCS+=		${UNWINDINCDIR}/__libunwind_config.h
>>>> +INCS+=		${UNWINDINCDIR}/libunwind.h
>>>> +INCS+=		${UNWINDINCDIR}/unwind.h
>>>> +
>>>> .if ${.MAKE.LEVEL} > 0
>>>> # avoid circular dependencies
>>>> GENDIRDEPS_FILTER+= Nlib/msun
>>>> diff --git a/lib/libgcc_eh/Makefile.inc b/lib/libgcc_eh/Makefile.inc
>>>> index 9e386992e78c..08e5db419840 100644
>>>> --- a/lib/libgcc_eh/Makefile.inc
>>>> +++ b/lib/libgcc_eh/Makefile.inc
>>>> @@ -34,7 +34,6 @@ CXXFLAGS.${file}+=	-fno-sanitize=address
>>>> .endfor
>>>> 
>>>> CFLAGS+=	-I${UNWINDINCDIR}
>>>> -CFLAGS+=	-I${.CURDIR}
>>>> CFLAGS+=	-D_LIBUNWIND_IS_NATIVE_ONLY
>>>> CFLAGS+=	-D_LIBUNWIND_USE_FRAME_HEADER_CACHE
>>>> CXXFLAGS+=	-fno-rtti
>>>> diff --git a/lib/libthr/Makefile b/lib/libthr/Makefile
>>>> index 038823413cb2..8fbd685e9f4b 100644
>>>> --- a/lib/libthr/Makefile
>>>> +++ b/lib/libthr/Makefile
>>>> @@ -19,7 +19,6 @@ CFLAGS+=-DPTHREAD_KERNEL
>>>> CFLAGS+=-I${SRCTOP}/lib/libc/include
>>>> CFLAGS+=-I${SRCTOP}/lib/libc/${MACHINE_CPUARCH}
>>>> CFLAGS+=-I${.CURDIR}/thread
>>>> -CFLAGS+=-I${SRCTOP}/include
>>>> CFLAGS+=-I${.CURDIR}/arch/${MACHINE_CPUARCH}/include
>>>> CFLAGS+=-I${.CURDIR}/sys
>>>> CFLAGS+=-I${SRCTOP}/libexec/rtld-elf
>>>> diff --git a/tools/build/mk/OptionalObsoleteFiles.inc
>>>> b/tools/build/mk/OptionalObsoleteFiles.inc index
>>>> ddbd303869e7..1edd4887d262 100644
>>>> --- a/tools/build/mk/OptionalObsoleteFiles.inc
>>>> +++ b/tools/build/mk/OptionalObsoleteFiles.inc
>>>> @@ -4027,9 +4027,6 @@ OLD_FILES+=usr/include/c++/v1/typeindex
>>>> OLD_FILES+=usr/include/c++/v1/typeinfo
>>>> OLD_FILES+=usr/include/c++/v1/unordered_map
>>>> OLD_FILES+=usr/include/c++/v1/unordered_set
>>>> -OLD_FILES+=usr/include/c++/v1/unwind-arm.h
>>>> -OLD_FILES+=usr/include/c++/v1/unwind-itanium.h
>>>> -OLD_FILES+=usr/include/c++/v1/unwind.h
>>>> OLD_FILES+=usr/include/c++/v1/utility
>>>> OLD_FILES+=usr/include/c++/v1/valarray
>>>> OLD_FILES+=usr/include/c++/v1/variant
>>> 
>>> This appears to be an ABI change.  I've been looking at the
>>> editors/openoffice-devel build breakage in one of it's built-in
>>> regression tests and bisected the system source down to this commit.
>> 
>> See https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=262008 and
>> https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=263370 for lots of
>> details (not all of them pertinent, unfortunately).
>> 
>> I hope that after commit b7d4192598fe and its merges to stable/13 to
>> releng/13.1, this problem will have been worked around, without having
>> to patch up all extant versions of star/open/libreoffice...
> 
> Unfortunately this does not be sufficient for openoffice, I'm still
> digging in.  One thing that bothers me is this pointer arithmetic in its
> deleteException() function:
> 
> static void deleteException( void * pExc )
> {
>     __cxa_exception const * header = ((__cxa_exception const *)pExc - 1);
> 
> That makes the code dependent on the size of the exception structure.
> That squares with this comment in cxxabi.h:
> 
> #if __LP64__
>         /**
>          * Reference count.  Used to support the C++11 exception_ptr class.  This
>          * is prepended to the structure in 64-bit mode and squeezed in to the
>          * padding left before the 64-bit aligned _Unwind_Exception at the end in
>          * 32-bit mode.
>          *
>          * Note that it is safe to extend this structure at the beginning, rather
>          * than the end, because the public API for creating it returns the address
>          * of the end (where the exception object can be stored).
>          */
>         uintptr_t referenceCount;
> #endif
> 
> but the unwind changes modify the length of the members at the end of
> the structure.  It looks like openoffice will have to do a runtime
> probe, like libreoffice has done in the past to allow a copy built with
> the older unwind to run with the newer unwind.
> 
> Unfortunately, I'm not even getting that far, the selftest code is
> failing to the build and run with the latest stable/13, where cross
> version compatibility should not be an issue.

Nevermind, I think.  I've got too many windows open, and my build on
stable/13 actually did work.

There were a couple of other sources of confusion.  The first is that
the openoffice code appears to define its own __cxa_exception, but it
the definition turns out to be protected by an ifdef and it is not
obvious that the definition is skipped.  The other is that <unwind.h>
has two radically different definitions for _Unwind_Exception, and the
ARM version, which is very unlike the older version, comes first.

My only remaining question is why the selftest fails before the recent
padding fix.  I would expect the tests to pass with the incorrectly
placed padding as long as everything was compiled with the same
structure definition.