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

[Joelle van Dyne]

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