Re: [PATCH] coroutine: add libucontext as external library

[Daniel P . Berrangé]

On Tue, Mar 09, 2021 at 01:21:29PM -0800, Joelle van Dyne wrote:
> On Tue, Mar 9, 2021 at 10:24 AM Joelle van Dyne <j@getutm.app> wrote:
> >
> > On Tue, Mar 9, 2021 at 7:38 AM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > >
> > > On Mon, Mar 08, 2021 at 07:26:36PM -0800, Joelle van Dyne wrote:
> > > > iOS does not support ucontext natively for aarch64 and the sigaltstack 
> > > > is
> > > > also unsupported (even worse, it fails silently, see:
> > > > https://openradar.appspot.com/13002712 )
> > > >
> > > > As a workaround we include a library implementation of ucontext and add 
> > > > it
> > > > as a build option.
> > > >
> > > > Signed-off-by: Joelle van Dyne <j@getutm.app>
> > > > ---
> > > >  configure                 | 21 ++++++++++++++++++---
> > > >  meson.build               | 12 +++++++++++-
> > > >  util/coroutine-ucontext.c |  9 +++++++++
> > > >  .gitmodules               |  3 +++
> > > >  MAINTAINERS               |  6 ++++++
> > > >  meson_options.txt         |  2 ++
> > > >  subprojects/libucontext   |  1 +
> > > >  7 files changed, 50 insertions(+), 4 deletions(-)
> > > >  create mode 160000 subprojects/libucontext
> > > >
> > > > diff --git a/configure b/configure
> > > > index 34fccaa2ba..5f225894a9 100755
> > > > --- a/configure
> > > > +++ b/configure
> > > > @@ -1773,7 +1773,7 @@ Advanced options (experts only):
> > > >    --oss-lib                path to OSS library
> > > >    --cpu=CPU                Build for host CPU [$cpu]
> > > >    --with-coroutine=BACKEND coroutine backend. Supported options:
> > > > -                           ucontext, sigaltstack, windows
> > > > +                           ucontext, libucontext, sigaltstack, windows
> > >
> > > This approach mixes the concept of the coroutine backend (ucontext,
> > > sigaltstack, etc) with the optional libucontext library dependency.
> > >
> > > libucontext is not a coroutine backend. The patch had to introduce
> > > $coroutine_impl in addition to $coroutine in order to work around this.
> > > Let's avoid combining these two independent concepts into
> > > --with-coroutine=.
> > >
> > > I suggest treating libucontext as an optional library dependency in
> > > ./configure with explicit --enable-libucontext/--disable-libucontext
> > > options. Most of the time neither option will be provided by the user
> > > and ./configure should automatically decide whether libucontext is
> > > needed or not.
> > >
> > > > +case $coroutine in
> > > > +libucontext)
> > > > +  git_submodules="${git_submodules} subprojects/libucontext"
> > > > +  mkdir -p libucontext
> > >
> > > Why is this mkdir necessary?
> >
> > That is a typo, will fix.
> >
> > Thanks to all the feedback in this thread. I will shelve this patchset
> > for now and see if it's possible to fix ucontext on Darwin. Or if we
> > go with gcoroutine that would work as well. Either way it seems like
> > this isn't ready yet.
> >
> > -j
> 
> The following is enough to get ucontext working on macOS 11 (Apple
> seems to have fixed it when they added ARM64 support for M1 Macs).
> However, ucontext still does not work (no symbols) on iOS so there's
> not much point in switching from sigaltstack.
> 
> -j
> 
> diff --git a/configure b/configure
> index a2736ecf16..042f4e87a5 100755
> --- a/configure
> +++ b/configure
> @@ -774,7 +774,8 @@ Darwin)
>    audio_possible_drivers="coreaudio sdl"
>    # Disable attempts to use ObjectiveC features in os/object.h since they
>    # won't work when we're compiling with gcc as a C compiler.
> -  QEMU_CFLAGS="-DOS_OBJECT_USE_OBJC=0 $QEMU_CFLAGS"
> +  # _XOPEN_SOURCE and _DARWIN_C_SOURCE needed for ucontext
> +  QEMU_CFLAGS="-D_XOPEN_SOURCE=500 -D_DARWIN_C_SOURCE
> -DOS_OBJECT_USE_OBJC=0 $QEMU_CFLAGS"
>  ;;
>  SunOS)
>    solaris="yes"
> @@ -4486,17 +4487,15 @@ fi
>  # specific one.
> 
>  ucontext_works=no
> -if test "$darwin" != "yes"; then
> -  cat > $TMPC << EOF
> +cat > $TMPC << EOF
>  #include <ucontext.h>
>  #ifdef __stub_makecontext
>  #error Ignoring glibc stub makecontext which will always fail
>  #endif
>  int main(void) { makecontext(0, 0, 0); return 0; }
>  EOF
> -  if compile_prog "" "" ; then
> -    ucontext_works=yes
> -  fi
> +if compile_prog "" "" ; then
> +  ucontext_works=yes
>  fi
> 
>  if test "$coroutine" = ""; then

I have tried doing this before, and while it was enough for the compile
to succeed, I found that tests failed / hung when running the macOS CI
jobs.  Did you actually try running tests with this change directly,
and/or under Cirrus CI ?


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|