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: [email protected] > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1631 > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1659 > Signed-off-by: Peter Maydell <[email protected]> > --- > 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 > + */ > +#if defined(__apple_build_version__) && __clang_major__ >= 14 > +#define BUILTIN_SUBCLL_BROKEN > +#endif Reviewed-by: Daniel P. Berrangé <[email protected]> With 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 :|
