Re: [PATCH] host-utils: Avoid using __builtin_subcll on buggy versions of Apple Clang

[Peter Maydell]

On Thu, 22 Jun 2023 at 15:35, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Thu, Jun 22, 2023 at 02:08:23PM +0100, Peter Maydell wrote:
> > We use __builtin_subcll() to do a 64-bit subtract with borrow-in and
> > borrow-out when the host compiler supports it.  Unfortunately some
> > versions of Apple Clang have a bug in their implementation of this
> > intrinsic which means it returns the wrong value.  The effect is that
> > a QEMU built with the affected compiler will hang when emulating x86
> > float80 division.
> >
> > The upstream LLVM issue is:
> > https://github.com/llvm/llvm-project/issues/55253
> >
> > The commit that introduced the bug apparently never made it into an
> > upstream LLVM release without the subsequent fix
> > https://github.com/llvm/llvm-project/commit/fffb6e6afdbaba563189c1f715058ed401fbc88d
> > but unfortunately it did make it into Apple Clang 14.0, as shipped
> > in Xcode 14.3 (14.2 is reported to be OK). The Apple bug number is
> > FB12210478.
> >
> > Add ifdefs to avoid use of __builtin_subcll() on Apple Clang version
> > 14 or greater.  There is not currently a version of Apple Clang which
> > has the bug fix -- when one appears we should be able to add an upper
> > bound to the ifdef condition so we can start using the builtin again.
> > We make the lower bound a conservative "any Apple clang with major
> > version 14 or greater" because the consequences of incorrectly
> > disabling the builtin when it would work are pretty small and the
> > consequences of not disabling it when we should are pretty bad.
> >
> > Many thanks to those users who both reported this bug and also
> > did a lot of work in identifying the root cause; in particular
> > to Daniel Bertalan and osy.
> >
> > Cc: qemu-stable@nongnu.org
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1631
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1659
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> > I don't have a setup to test this, so this needs testing by the
> > people who've encountered this compiler bug to confirm it does
> > the right thing...
> > ---
> >  include/qemu/compiler.h   | 13 +++++++++++++
> >  include/qemu/host-utils.h |  2 +-
> >  2 files changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
> > index c2f49df1f91..a309f90c768 100644
> > --- a/include/qemu/compiler.h
> > +++ b/include/qemu/compiler.h
> > @@ -184,4 +184,17 @@
> >  #define QEMU_DISABLE_CFI
> >  #endif
> >
> > +/*
> > + * Apple clang version 14 has a bug in its __builtin_subcll(); define
> > + * BUILTIN_SUBCLL_BROKEN for the offending versions so we can avoid it.
> > + * When a version of Apple clang which has this bug fixed is released
> > + * we can add an upper bound to this check.
> > + * See https://gitlab.com/qemu-project/qemu/-/issues/1631
> > + * and https://gitlab.com/qemu-project/qemu/-/issues/1659 for details.
> > + * The bug never made it into any upstream LLVM releases, only Apple ones.
>
> Perhaps add a reminder:
>
>  * TODO: put a max cap on __clang_major__/__clang_minor once
>  * Apple have released a version with the fix

That was what I intended the "When..." sentence to be. We
can add "TODO:" on the front to make it stand out a bit more I guess.

-- PMM