Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250414093715.GD2724612@port70.net>
Date: Mon, 14 Apr 2025 11:37:15 +0200
From: Szabolcs Nagy <nsz@...t70.net>
To: Alex Rønne Petersen <alex@...xrp.com>
Cc: Rich Felker <dalias@...c.org>, musl@...ts.openwall.com
Subject: Re: Re: [PATCH] configure: prevent compilers from turning a *
 b + c into fma(a, b, c)

* Alex Rønne Petersen <alex@...xrp.com> [2025-02-22 01:47:18 +0100]:
> On Sat, Feb 22, 2025 at 1:37 AM Rich Felker <dalias@...c.org> wrote:
> >
> > On Fri, Dec 06, 2024 at 01:54:45AM +0100, Alex Rønne Petersen wrote:
> > > On Wed, Aug 28, 2024 at 5:28 PM Alex Rønne Petersen <alex@...xrp.com> wrote:
> > > >
> > > > I've seen Clang do this for expressions in the fma() implementation itself,
> > > > which of course led to infinite recursion. This happened when targeting
> > > > arm-linux-musleabi with full soft float mode and -march=armv8-a. I imagine
> > > > it's possible for GCC to do similar silliness.
> > > >
> > > > Work around this by passing -ffp-contract=off for Clang and -mno-fused-madd
> > > > for GCC. This matches what glibc's configure.ac does, FWIW.
> > > > ---
> > > >  configure | 9 +++++++++
> > > >  1 file changed, 9 insertions(+)
> > > >
> > > > diff --git a/configure b/configure
> > > > index bc9fbe48..7028793f 100755
> > > > --- a/configure
> > > > +++ b/configure
> > > > @@ -355,6 +355,15 @@ tryflag CFLAGS_C99FSE -fexcess-precision=standard \
> > > >  || { test "$ARCH" = i386 && tryflag CFLAGS_C99FSE -ffloat-store ; }
> > > >  tryflag CFLAGS_C99FSE -frounding-math
> > > >
> > > > +#
> > > > +# Prevent the compiler from turning a * b + c into an fma() call.
> > > > +# Clang at least has been known to do this in the implementation of
> > > > +# fma() itself when targeting arm-linux-musleabi and armv8-a, causing
> > > > +# infinite recursion.
> > > > +#
> > > > +tryflag CFLAGS_C99FSE -mno-fused-madd
> > > > +tryflag CFLAGS_C99FSE -ffp-contract=off
> > > > +
> > > >  #
> > > >  # Semantically we want to insist that our sources follow the
> > > >  # C rules for type-based aliasing, but most if not all real-world
> > > > --
> > > > 2.40.1
> > > >
> > >
> > > Ping. Is the patch acceptable without -mno-fused-madd (for ancient
> > > GCCs)? If so, should I re-send without that line?
> >
> > If this is needed at all (IIRC I was unclear on how it was needed at
> > all with -ffreestanding) then I'd prefer to also include the option to
> > unbreak old GCC.

i think we want the -ffp-contract=off change.

that's default in gcc with -std=c99 but not
in clang, so now the clang fp behaviour is
different on targets with fma.

the recursive call to fma should not happen,
but i guess there can be such bugs in a weird
compiler like zig cc.

i'd expect musl to work fine with fp-contract=on,
i used to build and test it with that too. but
the default should be consistent across compilers.

> 
> I wrote an analysis of the problem in a reply to Alexander earlier in
> the thread. The TL;DR of that email is that compiling musl with
> vanilla clang instead of zig cc works, but it's basically by accident,
> and even then, it's brittle enough that changes in LLVM optimization
> passes or backends could break it someday even for vanilla clang.
> Passing these flags is semantically the right thing to do and
> future-proofs musl against such changes while also fixing zig cc.

i'd resend the patch with updated commit msg
and comment, something along the lines of

the fp-contract behavior is implementation defined,
so make the setting explicit. this matters on clang
where fma contraction is on by default even in
standard c mode.

the commit msg can mention that this was found by
zig cc miscompiling fma.c on arm-linux-musleabi

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.