diff mbox series

[2/2] package/mono: add libunwind optionaldependency

Message ID 20190515204721.18722-2-fontaine.fabrice@gmail.com
State Accepted
Headers show
Series [1/2] package/libunwind: add cxx exceptionssupport | expand

Commit Message

Fabrice Fontaine May 15, 2019, 8:47 p.m. UTC 
Add libunwind dependency as well as a patch to link with it

Fixes:
 - http://autobuild.buildroot.net/results/dbd64c89815d393a4e28b312d74fd80ee6de92da

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
---
 ...03-configure.ac-checks-for-libunwind.patch | 40 +++++++++++++++++++
 package/mono/mono.mk                          |  4 +-
 2 files changed, 43 insertions(+), 1 deletion(-)
 create mode 100644 package/mono/0003-configure.ac-checks-for-libunwind.patch

Comments

Thomas Petazzoni May 16, 2019, 6:29 a.m. UTC | #1 
Hello Fabrice,

On Wed, 15 May 2019 22:47:21 +0200
Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:

> +_Unwind_GetIP is used in build_stack_trace however this function can be
> +provided by libunwind so check for it to avoid the following build
> +failure:
> +
> +/home/buildroot/autobuild/run/instance-1/output/host/lib/gcc/arm-buildroot-linux-musleabihf/7.4.0/../../../../arm-buildroot-linux-musleabihf/bin/ld: ./.libs/libmini.a(libmini_la-mini-exceptions.o): in function `build_stack_trace':
> +/home/buildroot/autobuild/run/instance-1/output/build/mono-5.20.1.27/mono/mini/mini-exceptions.c:365: undefined reference to `_Unwind_GetIP'
> +collect2: error: ld returned 1 exit status

I am really not familiar with _Unwind_GetIP, and I haven't done much
research, but it seems like _Unwind_GetIP is also provided by libgcc.

Did you make some further research to understand what is
_Unwind_GetIP ? Or was your analysis just based on "mono needs
_Unwind_GetIP, it's provided by libunwind, let's link with it" ?

Thomas
Fabrice Fontaine May 16, 2019, 7:43 p.m. UTC | #2 
Hello Thomas,

Le jeu. 16 mai 2019 à 08:29, Thomas Petazzoni
<thomas.petazzoni@bootlin.com> a écrit :
>
> Hello Fabrice,
>
> On Wed, 15 May 2019 22:47:21 +0200
> Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:
>
> > +_Unwind_GetIP is used in build_stack_trace however this function can be
> > +provided by libunwind so check for it to avoid the following build
> > +failure:
> > +
> > +/home/buildroot/autobuild/run/instance-1/output/host/lib/gcc/arm-buildroot-linux-musleabihf/7.4.0/../../../../arm-buildroot-linux-musleabihf/bin/ld: ./.libs/libmini.a(libmini_la-mini-exceptions.o): in function `build_stack_trace':
> > +/home/buildroot/autobuild/run/instance-1/output/build/mono-5.20.1.27/mono/mini/mini-exceptions.c:365: undefined reference to `_Unwind_GetIP'
> > +collect2: error: ld returned 1 exit status
>
> I am really not familiar with _Unwind_GetIP, and I haven't done much
> research, but it seems like _Unwind_GetIP is also provided by libgcc.
>
> Did you make some further research to understand what is
> _Unwind_GetIP ? Or was your analysis just based on "mono needs
> _Unwind_GetIP, it's provided by libunwind, let's link with it" ?
My analysis was based on the fact that the build failures occured only
when libunwind was built before mono.
I traced back this error to the installation of unwind.h by libunwind
which contains the following definition of _Unwind_GetIP:
extern unsigned long _Unwind_GetIP (struct _Unwind_Context *);

However, libunwind implements _Unwind_GetIP in src/unwind/GetIP.c only
if --enable-cxx-exceptions is defined.

On his side, mono includes unwind.h if it's available:
#ifdef HAVE_UNWIND_H
#include <unwind.h>
#endif

unwind.h and _Unwind_GetIP can also be provided by glibc but that's
not the cause of this build failure on musl.
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Best Regards,

Fabrice
Thomas Petazzoni May 18, 2019, 8:40 p.m. UTC | #3 
Hello,

+Arnout in case he has some background knowledge on libunwind.

On Thu, 16 May 2019 21:43:20 +0200
Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:

> > Did you make some further research to understand what is
> > _Unwind_GetIP ? Or was your analysis just based on "mono needs
> > _Unwind_GetIP, it's provided by libunwind, let's link with it" ?  
> My analysis was based on the fact that the build failures occured only
> when libunwind was built before mono.
> I traced back this error to the installation of unwind.h by libunwind
> which contains the following definition of _Unwind_GetIP:
> extern unsigned long _Unwind_GetIP (struct _Unwind_Context *);
> 
> However, libunwind implements _Unwind_GetIP in src/unwind/GetIP.c only
> if --enable-cxx-exceptions is defined.
> 
> On his side, mono includes unwind.h if it's available:
> #ifdef HAVE_UNWIND_H
> #include <unwind.h>
> #endif
> 
> unwind.h and _Unwind_GetIP can also be provided by glibc but that's
> not the cause of this build failure on musl.

Hm, did you notice that mono has its own libunwind library, which
provides _Unwind_GetIP ?

See:

external/corert/src/Native/libunwind/src/UnwindLevel1-gcc-ext.c:_LIBUNWIND_EXPORT uintptr_t _Unwind_GetIPInfo(struct _Unwind_Context *context,
external/corert/src/Native/libunwind/src/UnwindLevel1-gcc-ext.c:  _LIBUNWIND_TRACE_API("_Unwind_GetIPInfo(context=%p)", (void *)context);
external/corert/src/Native/libunwind/src/UnwindLevel1-gcc-ext.c:  return _Unwind_GetIP(context);
external/corert/src/Native/libunwind/src/Unwind-EHABI.cpp:    uintptr_t pc = _Unwind_GetIP(context);
external/corert/src/Native/libunwind/src/Unwind-sjlj.c:_LIBUNWIND_EXPORT uintptr_t _Unwind_GetIP(struct _Unwind_Context *context) {
external/corert/src/Native/libunwind/src/Unwind-sjlj.c:  _LIBUNWIND_TRACE_API("_Unwind_GetIP(context=%p) => 0x%lX", context,
external/corert/src/Native/libunwind/src/Unwind-sjlj.c:_LIBUNWIND_EXPORT uintptr_t _Unwind_GetIPInfo(struct _Unwind_Context *context,
external/corert/src/Native/libunwind/src/Unwind-sjlj.c:  _LIBUNWIND_TRACE_API("_Unwind_GetIPInfo(context=%p, %p) => 0x%lX",
external/corert/src/Native/libunwind/src/UnwindCursor.hpp:  // This matches the behaviour of _Unwind_GetIP on arm.
external/corert/src/Native/libunwind/src/UnwindLevel1.c:_LIBUNWIND_EXPORT uintptr_t _Unwind_GetIP(struct _Unwind_Context *context) {
external/corert/src/Native/libunwind/src/UnwindLevel1.c:  _LIBUNWIND_TRACE_API("_Unwind_GetIP(context=%p) => 0x%" PRIx64,
external/corert/src/Native/libunwind/src/Unwind_AppleExtras.cpp:NOT_HERE_BEFORE_10_6(_Unwind_GetIP)
external/corert/src/Native/libunwind/src/Unwind_AppleExtras.cpp:NOT_HERE_BEFORE_10_6(_Unwind_GetIPInfo)
external/corert/src/Native/libunwind/src/Unwind_AppleExtras.cpp:NOT_HERE_BEFORE_5_0(_Unwind_GetIP)
external/corert/src/Native/libunwind/src/Unwind_AppleExtras.cpp:NOT_HERE_BEFORE_5_0(_Unwind_GetIPInfo)
external/corert/src/Native/libunwind/include/unwind.h:extern uintptr_t _Unwind_GetIP(struct _Unwind_Context *context);
external/corert/src/Native/libunwind/include/unwind.h:uintptr_t _Unwind_GetIP(struct _Unwind_Context *context) {
external/corert/src/Native/libunwind/include/unwind.h:// _Unwind_GetIPInfo is a gcc extension that can be called from within a
external/corert/src/Native/libunwind/include/unwind.h:// personality handler.  Similar to _Unwind_GetIP() but also returns in
external/corert/src/Native/libunwind/include/unwind.h:extern uintptr_t _Unwind_GetIPInfo(struct _Unwind_Context *context,

Also
http://refspecs.linuxfoundation.org/LSB_4.1.0/LSB-Core-generic/LSB-Core-generic.html#LIBGCC-SMAN
says _Unwind_GetIP() is supposed to be provided by libgcc_s.

On the other hand, glibc and uclibc provide a _Unwind_GetIP macro, but
only for ARM. uClibc libubacktrace also contains this:

static void backtrace_init (void)
{
        void *handle = dlopen (LIBGCC_S_SO, RTLD_LAZY);

        if (handle == NULL
                || ((unwind_backtrace = dlsym (handle, "_Unwind_Backtrace")) == NULL)
                || ((unwind_getip = dlsym (handle, "_Unwind_GetIP")) == NULL)) {
                printf(LIBGCC_S_SO " must be installed for backtrace to work\n");
                abort();
        }
}

So it looks for _Unwind_GetIP in libgcc.

So, I'm really confused by what is supposed to provide _Unwind_GetIP(),
and I don't feel comfortable with just saying "libunwind has it, let's
use it".

Thomas
Arnout Vandecappelle May 25, 2019, 5:03 p.m. UTC | #4 
On 18/05/2019 22:40, Thomas Petazzoni wrote:
> Hello,
> 
> +Arnout in case he has some background knowledge on libunwind.
> 
> On Thu, 16 May 2019 21:43:20 +0200
> Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:
> 
>>> Did you make some further research to understand what is
>>> _Unwind_GetIP ? Or was your analysis just based on "mono needs
>>> _Unwind_GetIP, it's provided by libunwind, let's link with it" ?  
>> My analysis was based on the fact that the build failures occured only
>> when libunwind was built before mono.
>> I traced back this error to the installation of unwind.h by libunwind
>> which contains the following definition of _Unwind_GetIP:
>> extern unsigned long _Unwind_GetIP (struct _Unwind_Context *);
>>
>> However, libunwind implements _Unwind_GetIP in src/unwind/GetIP.c only
>> if --enable-cxx-exceptions is defined.
>>
>> On his side, mono includes unwind.h if it's available:
>> #ifdef HAVE_UNWIND_H
>> #include <unwind.h>
>> #endif

 But mini-exceptions.c calls _Unwind_GetIP independently of that define. It's
called depending on  MONO_ARCH_HAVE_UNWIND_BACKTRACE, which is set for linux on
arm and amd64 (but strangely enough not on arm64).

 Also, AFAICS gcc will always install unwind.h. So HAVE_UNWIND_H should be set
even if libunwind is not built. The real problem is that libunwind and libgcc
have different definitions of _Unwind_GetIP() on arm: libgcc defines it as a
macro instead of a function.

 So I would say that libunwind is the real problem: it replaces gcc's unwind.h
with its own version, which is not compatible. (It doesn't really replace it; it
just puts another unwind.h earlier in the include path.) Thus, it forces any
package that expects to link with libgcc's unwind to instead link with libunwind.

>> unwind.h and _Unwind_GetIP can also be provided by glibc but that's
>> not the cause of this build failure on musl.
> 
> Hm, did you notice that mono has its own libunwind library, which
> provides _Unwind_GetIP ?
> 
> See:
> 
> external/corert/src/Native/libunwind/src/UnwindLevel1-gcc-ext.c:_LIBUNWIND_EXPORT uintptr_t _Unwind_GetIPInfo(struct _Unwind_Context *context,
> external/corert/src/Native/libunwind/src/UnwindLevel1-gcc-ext.c:  _LIBUNWIND_TRACE_API("_Unwind_GetIPInfo(context=%p)", (void *)context);
> external/corert/src/Native/libunwind/src/UnwindLevel1-gcc-ext.c:  return _Unwind_GetIP(context);
> external/corert/src/Native/libunwind/src/Unwind-EHABI.cpp:    uintptr_t pc = _Unwind_GetIP(context);
> external/corert/src/Native/libunwind/src/Unwind-sjlj.c:_LIBUNWIND_EXPORT uintptr_t _Unwind_GetIP(struct _Unwind_Context *context) {
> external/corert/src/Native/libunwind/src/Unwind-sjlj.c:  _LIBUNWIND_TRACE_API("_Unwind_GetIP(context=%p) => 0x%lX", context,
> external/corert/src/Native/libunwind/src/Unwind-sjlj.c:_LIBUNWIND_EXPORT uintptr_t _Unwind_GetIPInfo(struct _Unwind_Context *context,
> external/corert/src/Native/libunwind/src/Unwind-sjlj.c:  _LIBUNWIND_TRACE_API("_Unwind_GetIPInfo(context=%p, %p) => 0x%lX",
> external/corert/src/Native/libunwind/src/UnwindCursor.hpp:  // This matches the behaviour of _Unwind_GetIP on arm.
> external/corert/src/Native/libunwind/src/UnwindLevel1.c:_LIBUNWIND_EXPORT uintptr_t _Unwind_GetIP(struct _Unwind_Context *context) {
> external/corert/src/Native/libunwind/src/UnwindLevel1.c:  _LIBUNWIND_TRACE_API("_Unwind_GetIP(context=%p) => 0x%" PRIx64,
> external/corert/src/Native/libunwind/src/Unwind_AppleExtras.cpp:NOT_HERE_BEFORE_10_6(_Unwind_GetIP)
> external/corert/src/Native/libunwind/src/Unwind_AppleExtras.cpp:NOT_HERE_BEFORE_10_6(_Unwind_GetIPInfo)
> external/corert/src/Native/libunwind/src/Unwind_AppleExtras.cpp:NOT_HERE_BEFORE_5_0(_Unwind_GetIP)
> external/corert/src/Native/libunwind/src/Unwind_AppleExtras.cpp:NOT_HERE_BEFORE_5_0(_Unwind_GetIPInfo)

 This is a (partial?) copy of LLVM, but it is not actually built. The only files
that are built are Unwind-EHABI.cpp and libunwind.cpp. And AppleExtras on Darwin.

> external/corert/src/Native/libunwind/include/unwind.h:extern uintptr_t _Unwind_GetIP(struct _Unwind_Context *context);
> external/corert/src/Native/libunwind/include/unwind.h:uintptr_t _Unwind_GetIP(struct _Unwind_Context *context) {
> external/corert/src/Native/libunwind/include/unwind.h:// _Unwind_GetIPInfo is a gcc extension that can be called from within a
> external/corert/src/Native/libunwind/include/unwind.h:// personality handler.  Similar to _Unwind_GetIP() but also returns in
> external/corert/src/Native/libunwind/include/unwind.h:extern uintptr_t _Unwind_GetIPInfo(struct _Unwind_Context *context,

 The headers, on the other hand, do get added to the include path.

 Always a good idea to include a header file but not build all of the
corresponding sources...

> 
> Also
> http://refspecs.linuxfoundation.org/LSB_4.1.0/LSB-Core-generic/LSB-Core-generic.html#LIBGCC-SMAN
> says _Unwind_GetIP() is supposed to be provided by libgcc_s.
> 
> On the other hand, glibc and uclibc provide a _Unwind_GetIP macro, but

 It's a mystery why they do if gcc already provides the same...

> only for ARM. uClibc libubacktrace also contains this:
> 
> static void backtrace_init (void)
> {
>         void *handle = dlopen (LIBGCC_S_SO, RTLD_LAZY);
> 
>         if (handle == NULL
>                 || ((unwind_backtrace = dlsym (handle, "_Unwind_Backtrace")) == NULL)
>                 || ((unwind_getip = dlsym (handle, "_Unwind_GetIP")) == NULL)) {
>                 printf(LIBGCC_S_SO " must be installed for backtrace to work\n");
>                 abort();
>         }
> }
> 
> So it looks for _Unwind_GetIP in libgcc.
> 
> So, I'm really confused by what is supposed to provide _Unwind_GetIP(),
> and I don't feel comfortable with just saying "libunwind has it, let's
> use it".

 To me it feels similar to -latomic (which I also don't completely understand):
stuff that gets included through some headers that may come from various
sources, and you may or may not need to link with some library depending on what
happens the be the actual source.

 Regards,
 Arnout
Angelo Compagnucci Aug. 16, 2019, 7:22 a.m. UTC | #5 
Hi All,

On Sat, May 25, 2019 at 7:03 PM Arnout Vandecappelle <arnout@mind.be> wrote:
>
>
>
> On 18/05/2019 22:40, Thomas Petazzoni wrote:
> > Hello,
> >
> > +Arnout in case he has some background knowledge on libunwind.
> >
> > On Thu, 16 May 2019 21:43:20 +0200
> > Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:
> >
> >>> Did you make some further research to understand what is
> >>> _Unwind_GetIP ? Or was your analysis just based on "mono needs
> >>> _Unwind_GetIP, it's provided by libunwind, let's link with it" ?
> >> My analysis was based on the fact that the build failures occured only
> >> when libunwind was built before mono.
> >> I traced back this error to the installation of unwind.h by libunwind
> >> which contains the following definition of _Unwind_GetIP:
> >> extern unsigned long _Unwind_GetIP (struct _Unwind_Context *);
> >>
> >> However, libunwind implements _Unwind_GetIP in src/unwind/GetIP.c only
> >> if --enable-cxx-exceptions is defined.
> >>
> >> On his side, mono includes unwind.h if it's available:
> >> #ifdef HAVE_UNWIND_H
> >> #include <unwind.h>
> >> #endif
>
>  But mini-exceptions.c calls _Unwind_GetIP independently of that define. It's
> called depending on  MONO_ARCH_HAVE_UNWIND_BACKTRACE, which is set for linux on
> arm and amd64 (but strangely enough not on arm64).
>
>  Also, AFAICS gcc will always install unwind.h. So HAVE_UNWIND_H should be set
> even if libunwind is not built. The real problem is that libunwind and libgcc
> have different definitions of _Unwind_GetIP() on arm: libgcc defines it as a
> macro instead of a function.
>
>  So I would say that libunwind is the real problem: it replaces gcc's unwind.h
> with its own version, which is not compatible. (It doesn't really replace it; it
> just puts another unwind.h earlier in the include path.) Thus, it forces any
> package that expects to link with libgcc's unwind to instead link with libunwind.

This patch looks right to me. If BR2_PACKAGE_LIBUNWIND is somewhat
selected, it should be built before mono else mono will fail to
compile.

@Fabrice Fontaine : have you tried to upstream this patch?

>
> >> unwind.h and _Unwind_GetIP can also be provided by glibc but that's
> >> not the cause of this build failure on musl.
> >
> > Hm, did you notice that mono has its own libunwind library, which
> > provides _Unwind_GetIP ?
> >
> > See:
> >
> > external/corert/src/Native/libunwind/src/UnwindLevel1-gcc-ext.c:_LIBUNWIND_EXPORT uintptr_t _Unwind_GetIPInfo(struct _Unwind_Context *context,
> > external/corert/src/Native/libunwind/src/UnwindLevel1-gcc-ext.c:  _LIBUNWIND_TRACE_API("_Unwind_GetIPInfo(context=%p)", (void *)context);
> > external/corert/src/Native/libunwind/src/UnwindLevel1-gcc-ext.c:  return _Unwind_GetIP(context);
> > external/corert/src/Native/libunwind/src/Unwind-EHABI.cpp:    uintptr_t pc = _Unwind_GetIP(context);
> > external/corert/src/Native/libunwind/src/Unwind-sjlj.c:_LIBUNWIND_EXPORT uintptr_t _Unwind_GetIP(struct _Unwind_Context *context) {
> > external/corert/src/Native/libunwind/src/Unwind-sjlj.c:  _LIBUNWIND_TRACE_API("_Unwind_GetIP(context=%p) => 0x%lX", context,
> > external/corert/src/Native/libunwind/src/Unwind-sjlj.c:_LIBUNWIND_EXPORT uintptr_t _Unwind_GetIPInfo(struct _Unwind_Context *context,
> > external/corert/src/Native/libunwind/src/Unwind-sjlj.c:  _LIBUNWIND_TRACE_API("_Unwind_GetIPInfo(context=%p, %p) => 0x%lX",
> > external/corert/src/Native/libunwind/src/UnwindCursor.hpp:  // This matches the behaviour of _Unwind_GetIP on arm.
> > external/corert/src/Native/libunwind/src/UnwindLevel1.c:_LIBUNWIND_EXPORT uintptr_t _Unwind_GetIP(struct _Unwind_Context *context) {
> > external/corert/src/Native/libunwind/src/UnwindLevel1.c:  _LIBUNWIND_TRACE_API("_Unwind_GetIP(context=%p) => 0x%" PRIx64,
> > external/corert/src/Native/libunwind/src/Unwind_AppleExtras.cpp:NOT_HERE_BEFORE_10_6(_Unwind_GetIP)
> > external/corert/src/Native/libunwind/src/Unwind_AppleExtras.cpp:NOT_HERE_BEFORE_10_6(_Unwind_GetIPInfo)
> > external/corert/src/Native/libunwind/src/Unwind_AppleExtras.cpp:NOT_HERE_BEFORE_5_0(_Unwind_GetIP)
> > external/corert/src/Native/libunwind/src/Unwind_AppleExtras.cpp:NOT_HERE_BEFORE_5_0(_Unwind_GetIPInfo)
>
>  This is a (partial?) copy of LLVM, but it is not actually built. The only files
> that are built are Unwind-EHABI.cpp and libunwind.cpp. And AppleExtras on Darwin.
>
> > external/corert/src/Native/libunwind/include/unwind.h:extern uintptr_t _Unwind_GetIP(struct _Unwind_Context *context);
> > external/corert/src/Native/libunwind/include/unwind.h:uintptr_t _Unwind_GetIP(struct _Unwind_Context *context) {
> > external/corert/src/Native/libunwind/include/unwind.h:// _Unwind_GetIPInfo is a gcc extension that can be called from within a
> > external/corert/src/Native/libunwind/include/unwind.h:// personality handler.  Similar to _Unwind_GetIP() but also returns in
> > external/corert/src/Native/libunwind/include/unwind.h:extern uintptr_t _Unwind_GetIPInfo(struct _Unwind_Context *context,
>
>  The headers, on the other hand, do get added to the include path.
>
>  Always a good idea to include a header file but not build all of the
> corresponding sources...
>
> >
> > Also
> > http://refspecs.linuxfoundation.org/LSB_4.1.0/LSB-Core-generic/LSB-Core-generic.html#LIBGCC-SMAN
> > says _Unwind_GetIP() is supposed to be provided by libgcc_s.
> >
> > On the other hand, glibc and uclibc provide a _Unwind_GetIP macro, but
>
>  It's a mystery why they do if gcc already provides the same...
>
> > only for ARM. uClibc libubacktrace also contains this:
> >
> > static void backtrace_init (void)
> > {
> >         void *handle = dlopen (LIBGCC_S_SO, RTLD_LAZY);
> >
> >         if (handle == NULL
> >                 || ((unwind_backtrace = dlsym (handle, "_Unwind_Backtrace")) == NULL)
> >                 || ((unwind_getip = dlsym (handle, "_Unwind_GetIP")) == NULL)) {
> >                 printf(LIBGCC_S_SO " must be installed for backtrace to work\n");
> >                 abort();
> >         }
> > }
> >
> > So it looks for _Unwind_GetIP in libgcc.
> >
> > So, I'm really confused by what is supposed to provide _Unwind_GetIP(),
> > and I don't feel comfortable with just saying "libunwind has it, let's
> > use it".
>
>  To me it feels similar to -latomic (which I also don't completely understand):
> stuff that gets included through some headers that may come from various
> sources, and you may or may not need to link with some library depending on what
> happens the be the actual source.
>
>  Regards,
>  Arnout
>
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
Fabrice Fontaine Aug. 16, 2019, 8:34 a.m. UTC | #6 
Dear Angelo,

Le ven. 16 août 2019 à 09:22, Angelo Compagnucci
<angelo@amarulasolutions.com> a écrit :
>
> Hi All,
>
> On Sat, May 25, 2019 at 7:03 PM Arnout Vandecappelle <arnout@mind.be> wrote:
> >
> >
> >
> > On 18/05/2019 22:40, Thomas Petazzoni wrote:
> > > Hello,
> > >
> > > +Arnout in case he has some background knowledge on libunwind.
> > >
> > > On Thu, 16 May 2019 21:43:20 +0200
> > > Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:
> > >
> > >>> Did you make some further research to understand what is
> > >>> _Unwind_GetIP ? Or was your analysis just based on "mono needs
> > >>> _Unwind_GetIP, it's provided by libunwind, let's link with it" ?
> > >> My analysis was based on the fact that the build failures occured only
> > >> when libunwind was built before mono.
> > >> I traced back this error to the installation of unwind.h by libunwind
> > >> which contains the following definition of _Unwind_GetIP:
> > >> extern unsigned long _Unwind_GetIP (struct _Unwind_Context *);
> > >>
> > >> However, libunwind implements _Unwind_GetIP in src/unwind/GetIP.c only
> > >> if --enable-cxx-exceptions is defined.
> > >>
> > >> On his side, mono includes unwind.h if it's available:
> > >> #ifdef HAVE_UNWIND_H
> > >> #include <unwind.h>
> > >> #endif
> >
> >  But mini-exceptions.c calls _Unwind_GetIP independently of that define. It's
> > called depending on  MONO_ARCH_HAVE_UNWIND_BACKTRACE, which is set for linux on
> > arm and amd64 (but strangely enough not on arm64).
> >
> >  Also, AFAICS gcc will always install unwind.h. So HAVE_UNWIND_H should be set
> > even if libunwind is not built. The real problem is that libunwind and libgcc
> > have different definitions of _Unwind_GetIP() on arm: libgcc defines it as a
> > macro instead of a function.
> >
> >  So I would say that libunwind is the real problem: it replaces gcc's unwind.h
> > with its own version, which is not compatible. (It doesn't really replace it; it
> > just puts another unwind.h earlier in the include path.) Thus, it forces any
> > package that expects to link with libgcc's unwind to instead link with libunwind.
>
> This patch looks right to me. If BR2_PACKAGE_LIBUNWIND is somewhat
> selected, it should be built before mono else mono will fail to
> compile.
>
> @Fabrice Fontaine : have you tried to upstream this patch?
Nope, following Arnout 's review I didn't try to upstream it.
>
> >
> > >> unwind.h and _Unwind_GetIP can also be provided by glibc but that's
> > >> not the cause of this build failure on musl.
> > >
> > > Hm, did you notice that mono has its own libunwind library, which
> > > provides _Unwind_GetIP ?
> > >
> > > See:
> > >
> > > external/corert/src/Native/libunwind/src/UnwindLevel1-gcc-ext.c:_LIBUNWIND_EXPORT uintptr_t _Unwind_GetIPInfo(struct _Unwind_Context *context,
> > > external/corert/src/Native/libunwind/src/UnwindLevel1-gcc-ext.c:  _LIBUNWIND_TRACE_API("_Unwind_GetIPInfo(context=%p)", (void *)context);
> > > external/corert/src/Native/libunwind/src/UnwindLevel1-gcc-ext.c:  return _Unwind_GetIP(context);
> > > external/corert/src/Native/libunwind/src/Unwind-EHABI.cpp:    uintptr_t pc = _Unwind_GetIP(context);
> > > external/corert/src/Native/libunwind/src/Unwind-sjlj.c:_LIBUNWIND_EXPORT uintptr_t _Unwind_GetIP(struct _Unwind_Context *context) {
> > > external/corert/src/Native/libunwind/src/Unwind-sjlj.c:  _LIBUNWIND_TRACE_API("_Unwind_GetIP(context=%p) => 0x%lX", context,
> > > external/corert/src/Native/libunwind/src/Unwind-sjlj.c:_LIBUNWIND_EXPORT uintptr_t _Unwind_GetIPInfo(struct _Unwind_Context *context,
> > > external/corert/src/Native/libunwind/src/Unwind-sjlj.c:  _LIBUNWIND_TRACE_API("_Unwind_GetIPInfo(context=%p, %p) => 0x%lX",
> > > external/corert/src/Native/libunwind/src/UnwindCursor.hpp:  // This matches the behaviour of _Unwind_GetIP on arm.
> > > external/corert/src/Native/libunwind/src/UnwindLevel1.c:_LIBUNWIND_EXPORT uintptr_t _Unwind_GetIP(struct _Unwind_Context *context) {
> > > external/corert/src/Native/libunwind/src/UnwindLevel1.c:  _LIBUNWIND_TRACE_API("_Unwind_GetIP(context=%p) => 0x%" PRIx64,
> > > external/corert/src/Native/libunwind/src/Unwind_AppleExtras.cpp:NOT_HERE_BEFORE_10_6(_Unwind_GetIP)
> > > external/corert/src/Native/libunwind/src/Unwind_AppleExtras.cpp:NOT_HERE_BEFORE_10_6(_Unwind_GetIPInfo)
> > > external/corert/src/Native/libunwind/src/Unwind_AppleExtras.cpp:NOT_HERE_BEFORE_5_0(_Unwind_GetIP)
> > > external/corert/src/Native/libunwind/src/Unwind_AppleExtras.cpp:NOT_HERE_BEFORE_5_0(_Unwind_GetIPInfo)
> >
> >  This is a (partial?) copy of LLVM, but it is not actually built. The only files
> > that are built are Unwind-EHABI.cpp and libunwind.cpp. And AppleExtras on Darwin.
> >
> > > external/corert/src/Native/libunwind/include/unwind.h:extern uintptr_t _Unwind_GetIP(struct _Unwind_Context *context);
> > > external/corert/src/Native/libunwind/include/unwind.h:uintptr_t _Unwind_GetIP(struct _Unwind_Context *context) {
> > > external/corert/src/Native/libunwind/include/unwind.h:// _Unwind_GetIPInfo is a gcc extension that can be called from within a
> > > external/corert/src/Native/libunwind/include/unwind.h:// personality handler.  Similar to _Unwind_GetIP() but also returns in
> > > external/corert/src/Native/libunwind/include/unwind.h:extern uintptr_t _Unwind_GetIPInfo(struct _Unwind_Context *context,
> >
> >  The headers, on the other hand, do get added to the include path.
> >
> >  Always a good idea to include a header file but not build all of the
> > corresponding sources...
> >
> > >
> > > Also
> > > http://refspecs.linuxfoundation.org/LSB_4.1.0/LSB-Core-generic/LSB-Core-generic.html#LIBGCC-SMAN
> > > says _Unwind_GetIP() is supposed to be provided by libgcc_s.
> > >
> > > On the other hand, glibc and uclibc provide a _Unwind_GetIP macro, but
> >
> >  It's a mystery why they do if gcc already provides the same...
> >
> > > only for ARM. uClibc libubacktrace also contains this:
> > >
> > > static void backtrace_init (void)
> > > {
> > >         void *handle = dlopen (LIBGCC_S_SO, RTLD_LAZY);
> > >
> > >         if (handle == NULL
> > >                 || ((unwind_backtrace = dlsym (handle, "_Unwind_Backtrace")) == NULL)
> > >                 || ((unwind_getip = dlsym (handle, "_Unwind_GetIP")) == NULL)) {
> > >                 printf(LIBGCC_S_SO " must be installed for backtrace to work\n");
> > >                 abort();
> > >         }
> > > }
> > >
> > > So it looks for _Unwind_GetIP in libgcc.
> > >
> > > So, I'm really confused by what is supposed to provide _Unwind_GetIP(),
> > > and I don't feel comfortable with just saying "libunwind has it, let's
> > > use it".
> >
> >  To me it feels similar to -latomic (which I also don't completely understand):
> > stuff that gets included through some headers that may come from various
> > sources, and you may or may not need to link with some library depending on what
> > happens the be the actual source.
> >
> >  Regards,
> >  Arnout
> >
> > _______________________________________________
> > buildroot mailing list
> > buildroot@busybox.net
> > http://lists.busybox.net/mailman/listinfo/buildroot
Best Regards,

Fabrice
Arnout Vandecappelle Oct. 27, 2019, 6:26 p.m. UTC | #7 
Hi all,

 Thomas and I spent a couple of more hours to get to the bottom of this, and it
gets even weirder...

On 16/08/2019 10:34, Fabrice Fontaine wrote:
> Dear Angelo,
> 
> Le ven. 16 août 2019 à 09:22, Angelo Compagnucci
> <angelo@amarulasolutions.com> a écrit :
>>
>> Hi All,
>>
>> On Sat, May 25, 2019 at 7:03 PM Arnout Vandecappelle <arnout@mind.be> wrote:
>>>
>>>
>>>
>>> On 18/05/2019 22:40, Thomas Petazzoni wrote:
>>>> Hello,
>>>>
>>>> +Arnout in case he has some background knowledge on libunwind.
>>>>
>>>> On Thu, 16 May 2019 21:43:20 +0200
>>>> Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:
>>>>
>>>>>> Did you make some further research to understand what is
>>>>>> _Unwind_GetIP ? Or was your analysis just based on "mono needs
>>>>>> _Unwind_GetIP, it's provided by libunwind, let's link with it" ?
>>>>> My analysis was based on the fact that the build failures occured only
>>>>> when libunwind was built before mono.
>>>>> I traced back this error to the installation of unwind.h by libunwind
>>>>> which contains the following definition of _Unwind_GetIP:
>>>>> extern unsigned long _Unwind_GetIP (struct _Unwind_Context *);
>>>>>
>>>>> However, libunwind implements _Unwind_GetIP in src/unwind/GetIP.c only
>>>>> if --enable-cxx-exceptions is defined.
>>>>>
>>>>> On his side, mono includes unwind.h if it's available:
>>>>> #ifdef HAVE_UNWIND_H
>>>>> #include <unwind.h>
>>>>> #endif
>>>
>>>  But mini-exceptions.c calls _Unwind_GetIP independently of that define. It's
>>> called depending on  MONO_ARCH_HAVE_UNWIND_BACKTRACE, which is set for linux on
>>> arm and amd64 (but strangely enough not on arm64).
>>>
>>>  Also, AFAICS gcc will always install unwind.h. So HAVE_UNWIND_H should be set
>>> even if libunwind is not built. The real problem is that libunwind and libgcc
>>> have different definitions of _Unwind_GetIP() on arm: libgcc defines it as a
>>> macro instead of a function.

 Indeed, gcc installs a copy of unwind.h, but in a different location. What
happens is that normally the unwind.h of gcc is used, because the default search
path is

GCCDIR/include
GCCDIR/../../../../arm-buildroot-linux-gnueabihf/include
STAGING_DIR/usr/include

(GCCDIR is where cc1 etc. are installed; the second include is where the
libstdc++ headers get installed). Because of this order, the unwind.h from gcc
is used, and all is well.

 *However*, in the specific case of a musl toolchain, it is

GCCDIR/../../../../arm-buildroot-linux-gnueabihf/include
STAGING_DIR/usr/include
GCCDIR/include


 And because this is not yet weird enough: when you move the toolchain to a
different location (e.g. when you use it as an external toolchain), the search
path of a musl toolchain becomes

GCCDIR/../../../../arm-buildroot-linux-gnueabihf/include
GCCDIR/include
STAGING_DIR/usr/include


 Therefore, the problem with mono libunwind only occurs for an internal musl
toolchain, not for an external one...


 This is where we gave up...


>>>  So I would say that libunwind is the real problem: it replaces gcc's unwind.h
>>> with its own version, which is not compatible. (It doesn't really replace it; it
>>> just puts another unwind.h earlier in the include path.) Thus, it forces any
>>> package that expects to link with libgcc's unwind to instead link with libunwind.
>>
>> This patch looks right to me. If BR2_PACKAGE_LIBUNWIND is somewhat
>> selected, it should be built before mono else mono will fail to
>> compile.
[snip]
>>>  To me it feels similar to -latomic (which I also don't completely understand):
>>> stuff that gets included through some headers that may come from various
>>> sources, and you may or may not need to link with some library depending on what
>>> happens the be the actual source.

 In conclusion, the most practical solution is to do this, because it is
basically sane: there is an unwind header installed by libunwind, so it makes
sense to also link with it.

 Therefore, I've applied both patches, thanks.

 Regards,
 Arnout
diff mbox series

Patch

diff --git a/package/mono/0003-configure.ac-checks-for-libunwind.patch b/package/mono/0003-configure.ac-checks-for-libunwind.patch
new file mode 100644
index 0000000000..8ff6b2f5c7
--- /dev/null
+++ b/package/mono/0003-configure.ac-checks-for-libunwind.patch
@@ -0,0 +1,40 @@ 
+From 4dc1d1fe553f3a8ad00919324419aba54675239e Mon Sep 17 00:00:00 2001
+From: Fabrice Fontaine <fontaine.fabrice@gmail.com>
+Date: Wed, 15 May 2019 22:30:01 +0200
+Subject: [PATCH] configure.ac: checks for libunwind
+
+_Unwind_GetIP is used in build_stack_trace however this function can be
+provided by libunwind so check for it to avoid the following build
+failure:
+
+/home/buildroot/autobuild/run/instance-1/output/host/lib/gcc/arm-buildroot-linux-musleabihf/7.4.0/../../../../arm-buildroot-linux-musleabihf/bin/ld: ./.libs/libmini.a(libmini_la-mini-exceptions.o): in function `build_stack_trace':
+/home/buildroot/autobuild/run/instance-1/output/build/mono-5.20.1.27/mono/mini/mini-exceptions.c:365: undefined reference to `_Unwind_GetIP'
+collect2: error: ld returned 1 exit status
+
+Fixes:
+ - http://autobuild.buildroot.net/results/dbd64c89815d393a4e28b312d74fd80ee6de92da
+
+Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
+---
+ configure.ac | 5 +++++
+ 1 file changed, 5 insertions(+)
+
+diff --git a/configure.ac b/configure.ac
+index d724f9e2d27..c3067246f08 100644
+--- a/configure.ac
++++ b/configure.ac
+@@ -2206,6 +2206,11 @@ if test x$host_win32 = xno; then
+ 	dnl *****************************
+ 	AC_CHECK_LIB(socket, socket, LIBS="$LIBS -lsocket")
+ 
++	dnl *****************************
++	dnl *** Checks for libunwind ***
++	dnl ****************************
++	AC_CHECK_LIB(unwind, _Unwind_GetIP, LIBS="$LIBS -lunwind")
++
+ 	case "$host" in
+ 		*-*-*freebsd*)
+ 			dnl *****************************
+-- 
+2.20.1
+
diff --git a/package/mono/mono.mk b/package/mono/mono.mk
index bb482a6d5f..53589f4071 100644
--- a/package/mono/mono.mk
+++ b/package/mono/mono.mk
@@ -40,7 +40,9 @@  ifeq ($(BR2_PACKAGE_LIBICONV),y)
 MONO_DEPENDENCIES += libiconv
 endif
 
-MONO_DEPENDENCIES += host-mono
+MONO_DEPENDENCIES += \
+	host-mono \
+	$(if $(BR2_PACKAGE_LIBUNWIND),libunwind)
 
 ## Mono managed