I mean IEEE754, of course.

I think crtfastmath.o should not be used when linking with -shared.

I'm still unconvinced that it makes sense as part of -funsafe-math-optimizations at all.  It's not an "optimization" in the sense that people usually think of when looking at compiler flags.

A recent issue triggered by this: https://bugzilla.redhat.com/show_bug.cgi?id=1127544

This issue is really, really bad, and has potential to start changing more and more run-time behavior of downstream libraries and apps the longer this bug persists.

For example, on RHEL 7, liblzma.so.5 is linked with -Ofast, which also triggers crtfastmeth.o to be used, corrupting the mxcsr register at library load time.
(The bug might be specific to RedHat, because -Ofast is added in the rpm spec.)

For some reason, Open GL libraries (libglapi) depend on libselinux which depends on liblzma.

So ANY open GL app on RHEL will now have its math/IEEE754 semantics broken.

That's terrible.

crtfastmath.o should NEVER be used when linking shared libraries, regardless of options.

(In reply to Craig Smith from comment #5)
> For example, on RHEL 7, liblzma.so.5 is linked with -Ofast, which also
> triggers crtfastmath.o to be used, corrupting the mxcsr register at library
> load time.
> (The bug might be specific to RedHat, because -Ofast is added in the rpm
> spec.)

I don't see any evidence of liblzma.so.5 being linked with -Ofast on RHEL7, can you point to that?

(In reply to Orion Poplawski from comment #6)
> (In reply to Craig Smith from comment #5)
> > For example, on RHEL 7, liblzma.so.5 is linked with -Ofast, which also
> > triggers crtfastmath.o to be used, corrupting the mxcsr register at library
> > load time.
> > (The bug might be specific to RedHat, because -Ofast is added in the rpm
> > spec.)
> 
> I don't see any evidence of liblzma.so.5 being linked with -Ofast on RHEL7,
> can you point to that?

The xz.spec file from the xz-5.1.2-0.alpha.1 source RPM includes the following:

%build
export CONFIGURE_TOP=`pwd`
mkdir objs
pushd objs
CFLAGS="%{optflags} -Ofast -funroll-loops" \
%configure2_5x
%make
popd

That version does not exist in RHEL7.  Looks like it was a Mandriva thing: https://www.rpmfind.net/linux/RPM/mandriva/devel/cooker/x86_64/media/main/release/xz-5.1.2-0.alpha.1.x86_64.html

I'm changing this from an enhancement to a bug.  It's a poor design, no one seems to think it's a good idea, and it's broken more than one thing in real life.

(In reply to Orion Poplawski from comment #8)
> That version does not exist in RHEL7.  Looks like it was a Mandriva thing:
> https://www.rpmfind.net/linux/RPM/mandriva/devel/cooker/x86_64/media/main/
> release/xz-5.1.2-0.alpha.1.x86_64.html

rpm -qi liblzma5 on my RHEL 7 system says that the source RPM is the one I mentioned, with build host 'n2.mandriva.com'.  Not sure why this is that case, if a build image got corrupted or someone manually installed an rpm form Mandriva on this system without my knowledge.

In any case, the GCC bug is causing real issues, even if possibly not on RHEL 7.  I'll find another RHEL 7 system and check.

Another manifestation of 'crtfastmath.o considered harmful':
https://github.com/gevent/gevent/pull/1820

https://github.com/llvm/llvm-project/issues/57589 was just filed, requesting to fix this behavior in Clang, as well.

Since Clang is effectively only doing it in the first place to match GCC's behavior, my feeling is that it'd be better not to make such a change in Clang only.

However, I'd just like to give my support to changing this in GCC (either to stop automatically linking against crtfastmath.o altogether, or to at least stop doing so with -shared). And, if there is a change here, would certainly then propose to match the new behavior in Clang.

Patient: Doctor it hurts when I do this.
Doctor: then don't do that and if you read the instructions I gave you I told you I would hurt this way.

Note I think it was a mistake that gcc had -ffast-math option at all now.

But that ship has sailed over 18 years ago. And people abuse optimizations that are documented this way and will still continue to abuse others similarly really.
Plus always will abuse c/c++ semantics too.

(In reply to Andrew Pinski from comment #13)
> Patient: Doctor it hurts when I do this.
> Doctor: then don't do that and if you read the instructions I gave you I
> told you I would hurt this way.
> 

This response could be applied to any bug asking for a change in behaviour of a command line option. But I accept it's been a long time.

(In reply to Andrew Pinski from comment #13)
> Patient: Doctor it hurts when I do this.
> Doctor: then don't do that and if you read the instructions I gave you I
> told you I would hurt this way.
> 

The problem I see with this analogy, is who exactly is the doctor in this situation? It seems that currently the doctor is the GCC devs, but I think the situation could be improved by having GCC itself be the doctor, by implementing a -Wfast-math warning flag or something to make the issues with -ffast-math more known. Having it documented in the instructions is just one place; it should be mentioned in all the relevant places that a user might check.

(In reply to Andrew Pinski from comment #13)
> Patient: Doctor it hurts when I do this.
> Doctor: then don't do that and if you read the instructions I gave you I
> told you I would hurt this way.

I think this analogy is off.  Maybe for most questionable optimization or codegen options it would apply, but -ffast-math is more like:

Patient: Doctor, it hurts me when my neighbor does this.
Doctor: Then tell your neighbor not to read that and to read the fine print on the box.

If nitroglycerin pills have side effects and patients need to read the label, maybe fine.  But if they start exploding and breaking other peoples' windows, that's a whole different problem.

Note that when a shared library wants -ffast-math like behavior (FTZ, etc.) _not_ linking crtfastmath.o will cause its behavior to depend on the process loading
it and whether that sets the appropriate flags or not.  That might also cause
highly surprising effects.

That said, a "true fix" would redesign how these things work and make
FTZ part of the instruction encoding.

I suppose setting/restoring the control word around all public API of a
translation unit (and with LTO a bit more optimized) would be prohibitly
expensive.

Short of dropping the feature or re-implementing it, for example in the
dynamic loader via some ELF annotations with documented behavior on
"mismatch", I don't see any fix for this bug besides asking users to
conciously not link with -ffast-math when building shared libraries - unless
that's intended.

I'll note that I can't find any documentation for this link-time behavior which
triggers for -Ofast, -ffast-math and -funsafe-math-optimizations but also
with -ffast-math -fno-unsafe-math-optimizations.

I've adjusted the list of affected targets.  What their crtfastmath.o actually
does would need to be determined, but it for sure will have the same spreading effect.

I read through the crtfastmath.c implementations for the other affected targets and confirmed that they do all set flush-to-zero in this thread:

https://threadreaderapp.com/thread/1567612053363347461.html

I agree that there should be a way for a shared library to link crtfastmath.o if it wants that behavior. But is there a reason -l:crtfastmath.o isn't sufficient in that case? Why does it need to be enabled automatically when -Ofast/-ffast-math/-funsafe-math optimizations are turned on?

The other note I would add is that in multi-threaded applications, crtfastmath.o is already not behaving as intended: FTZ/DAZ will only be set in the CPU state of the thread that loaded the shared library; it's hard to imagine a case where a user wants individual threads to have different FTZ/DAZ (unless they explicitly manage that by hand). Example:

$ cat baz.c
#include <stdio.h>
#include <unistd.h>
#include <dlfcn.h>
#include <pthread.h>

void loadlib() {
    void *handle = dlopen("./gofast.so", RTLD_LAZY);
    if (!handle) {
        fprintf(stderr, "dlopen: %s\n", dlerror());
    }
}

#define MXCSR_DAZ (1 << 6)  /* Enable denormals are zero mode */
#define MXCSR_FTZ (1 << 15) /* Enable flush to zero mode */
void printftz(int i) {
    unsigned int mxcsr = __builtin_ia32_stmxcsr ();
    printf("[%d] mxcsr.FTZ = %d, mxcsr.DAZ = %d\n", i, !!(mxcsr & MXCSR_FTZ), !!(mxcsr & MXCSR_DAZ));
    return;
}

void *thread(void *arg) {
    // Print thread id
    int i = *(int *)arg;
    if (i == 0) loadlib();
    sleep(1);
    printftz(i);
}

int main(int argc, char **argv) {
    // Create 4 threads
    pthread_t threads[4];
    int tids[4];
    for (int i = 0; i < 4; i++) {
        tids[i] = i;
        pthread_create(&threads[i], NULL, thread, &tids[i]);
    }
    // Wait for all threads to finish
    for (int i = 0; i < 4; i++) {
        pthread_join(threads[i], NULL);
    }
    return 0;
}

$ touch gofast.c
$ gcc -Ofast -fpic -shared gofast.c -o gofast.so
$ gcc -pthread baz.c -o baz -ldl

$ ./baz
[3] mxcsr.FTZ = 0, mxcsr.DAZ = 0
[0] mxcsr.FTZ = 1, mxcsr.DAZ = 1
[2] mxcsr.FTZ = 0, mxcsr.DAZ = 0
[1] mxcsr.FTZ = 0, mxcsr.DAZ = 0

(In reply to Brendan Dolan-Gavitt from comment #19)
> I read through the crtfastmath.c implementations for the other affected
> targets and confirmed that they do all set flush-to-zero in this thread:
> 
> https://threadreaderapp.com/thread/1567612053363347461.html
> 
> I agree that there should be a way for a shared library to link
> crtfastmath.o if it wants that behavior. But is there a reason
> -l:crtfastmath.o isn't sufficient in that case? Why does it need to be
> enabled automatically when -Ofast/-ffast-math/-funsafe-math optimizations
> are turned on?

The reasons for most of the "globbing" into -ffast-math/-Ofast are the
rules for SPEC CPU 2006 base flags which IIRC limited the number of flags
allowed (that's no longer a requirement for SPEC CPU 2017).  And of course
that users will not know of the flags but are likely not interested in
denormals when using -ffast-math.

> The other note I would add is that in multi-threaded applications,
> crtfastmath.o is already not behaving as intended: FTZ/DAZ will only be set
> in the CPU state of the thread that loaded the shared library; it's hard to
> imagine a case where a user wants individual threads to have different
> FTZ/DAZ (unless they explicitly manage that by hand). Example:

[...]

Yeah.  Not sure how often dynamic objects are opened from within threads
though.  That said, a possibility to enforce "consistency" at least would
be to save/restore the FP state around dlopen() so that shared objects
loaded not at program startup would not affect FP state at all?
The same could be done for shared objects loaded at program startup of
course.

The other way around would eventually be to make the CTOR __tls, that
should eventually force all threads to change their FP state(?).

(In reply to Richard Biener from comment #20)
> Yeah.  Not sure how often dynamic objects are opened from within threads
> though.

We know that some shared objects call pthread_create from ELF constructors, which is enough to potentially expose the issue if the ELF constructors are ordered in certain ways.

> That said, a possibility to enforce "consistency" at least would
> be to save/restore the FP state around dlopen() so that shared objects
> loaded not at program startup would not affect FP state at all?
> The same could be done for shared objects loaded at program startup of
> course.

I think GCC should not automatically link crtfastmath.o with -shared. That will fix this particular issue for new shared objects, where the action-at-a-distance issue is most prominent and least desirable I think.

> The other way around would eventually be to make the CTOR __tls, that
> should eventually force all threads to change their FP state(?).

No, all current TLS constructors run lazily when some language construct is used. There is no way to run TLS constructors at dlopen time. We can in theory change CPU state using glibc's SETXID broadcast, but it will impact legitimate uses of the control register (e.g., changes to the FP control word around a block of code that is executed in a non-default mode).

These days, we'd probably use GNU property notes for something like this (although we'd likely not make it a global property in the first place, not for this).

I think loading shared libraries from within threads is actually pretty common when shared libs are used as plugins for some GUI app. I made a graph of the reverse dependencies of Debian packages that contain shared libraries linking to crtfastmath.o, and you can see that most of them are related to A/V plugins (e.g. the largest cluster is from gstreamer-plugins-bad):

https://moyix.net/~moyix/ffast_debian.pdf

Granted, I think you could argue in this case that only affecting the thread that loaded the plugin is the *intended* behavior.

This bug has pernicious effects in many more places:

https://moyix.blogspot.com/2022/09/someones-been-messing-with-my-subnormals.html

Florian is right: at least, GCC should not automatically link crtfastmath.o with -shared. Better, separate fiddling with the FP control register from -ffast-math entirely.

Dropping crtfastmath.o with -shared makes sense.

(In reply to H.J. Lu from comment #24)
> Dropping crtfastmath.o with -shared makes sense.

Are you going to send a patch? I can give it a try as well (although I'm not familiar with the GCC specs language).

Created attachment 53686 [details]
A patch not to add crtfastmath.o for -shared on x86

(In reply to Florian Weimer from comment #25)
> (In reply to H.J. Lu from comment #24)
> > Dropping crtfastmath.o with -shared makes sense.
> 
> Are you going to send a patch? I can give it a try as well (although I'm not
> familiar with the GCC specs language).

Please give the patch at comment #26 a try.

(In reply to H.J. Lu from comment #27)
> (In reply to Florian Weimer from comment #25)
> > (In reply to H.J. Lu from comment #24)
> > > Dropping crtfastmath.o with -shared makes sense.
> > 
> > Are you going to send a patch? I can give it a try as well (although I'm not
> > familiar with the GCC specs language).
> 
> Please give the patch at comment #26 a try.

It may be worth expanding this patch to other architectures that currently have crtfastmath.o, since they similarly use global FPU state. The ones I have confirmed are: ia64, loongarch, aarch64, alpha, mips, arm, and sparc.

(Details in this Twitter thread, unfortunately didn't write it up elsewhere: https://threadreaderapp.com/thread/1567612053363347461.html ).

(In reply to H.J. Lu from comment #26)
> Created attachment 53686 [details]
> A patch not to add crtfastmath.o for -shared on x86

-  "%{Ofast|ffast-math|funsafe-math-optimizations:crtfastmath.o%s} \
+  "%{Ofast|ffast-math|funsafe-math-optimizations:%{!shared:crtfastmath.o%s}} \

Works for me. It's also the change I would have mde.

The master branch has been updated by hongtao Liu <liuhongt@gcc.gnu.org>:

https://gcc.gnu.org/g:88b531d0323fc1fc4173f1975d6a4f408a096f8b

commit r13-4769-g88b531d0323fc1fc4173f1975d6a4f408a096f8b
Author: liuhongt <hongtao.liu@intel.com>
Date:   Thu Dec 15 09:38:08 2022 +0800

    x86: Don't add crtfastmath.o for -shared
    
    Don't add crtfastmath.o for -shared to avoid changing the MXCSR register
    when loading a shared library.  crtfastmath.o will be used only when
    building executables.
    
            PR target/55522
            * config/i386/gnu-user-common.h (GNU_USER_TARGET_MATHFILE_SPEC):
            Don't add crtfastmath.o for -shared.
            * doc/invoke.texi (-shared): Add related documentation.

The master branch has been updated by hongtao Liu <liuhongt@gcc.gnu.org>:

https://gcc.gnu.org/g:e54375d85d4aa5889869c2672158083b2106b623

commit r13-4891-ge54375d85d4aa5889869c2672158083b2106b623
Author: liuhongt <hongtao.liu@intel.com>
Date:   Mon Dec 12 15:43:58 2022 +0800

    x86: Add a new option -mdaz-ftz to enable FTZ and DAZ flags in MXCSR.
    
    if (mdaz-ftz)
      link crtfastmath.o
    else if ((Ofast || ffast-math || funsafe-math-optimizations)
             && !shared && !mno-daz-ftz)
      link crtfastmath.o
    else
      Don't link crtfastmath.o
    
    gcc/ChangeLog:
    
            PR target/55522
            PR target/36821
            * config/i386/gnu-user-common.h (GNU_USER_TARGET_MATHFILE_SPEC):
            Link crtfastmath.o whenever -mdaz-ftz is specified. Don't link
            crtfastmath.o when -share or -mno-daz-ftz is specified.
            * config/i386/i386.opt (mdaz-ftz): New option.
            * doc/invoke.texi (x86 options): Document mftz-daz.

Do other targets want to follow suite for GCC 13 here?

(In reply to Richard Biener from comment #32)
> Do other targets want to follow suite for GCC 13 here?

I think the deviation for x86 and !x86 as-is will lead to further confusion.

I'm going to post a series of changes.

The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:0bf7131e539de4ca66b3c0eb72aa0afbd8f006dc

commit r13-5140-g0bf7131e539de4ca66b3c0eb72aa0afbd8f006dc
Author: Richard Biener <rguenther@suse.de>
Date:   Fri Jan 13 08:47:59 2023 +0100

    aarch64: Don't add crtfastmath.o for -shared
    
    Don't add crtfastmath.o for -shared to avoid altering the FP
    environment when loading a shared library.
    
            PR target/55522
            * config/aarch64/aarch64-elf-raw.h (ENDFILE_SPEC): Don't add
            crtfastmath.o for -shared.
            * config/aarch64/aarch64-freebsd.h (GNU_USER_TARGET_MATHFILE_SPEC):
            Likewise.
            * config/aarch64/aarch64-linux.h (GNU_USER_TARGET_MATHFILE_SPEC):
            Likewise.

The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:426a9f5570d74040176f81e7cf6806b36ebc907f

commit r13-5141-g426a9f5570d74040176f81e7cf6806b36ebc907f
Author: Richard Biener <rguenther@suse.de>
Date:   Fri Jan 13 08:51:10 2023 +0100

    arm: Don't add crtfastmath.o for -shared
    
    Don't add crtfastmath.o for -shared to avoid altering the FP
    environment when loading a shared library.
    
            PR target/55522
            * config/arm/linux-eabi.h (ENDFILE_SPEC): Don't add
            crtfastmath.o for -shared.
            * config/arm/unknown-elf.h (STARTFILE_SPEC): Likewise.

The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:6347bbec3efb625dfc8592fbb3099dc0364c5317

commit r13-5201-g6347bbec3efb625dfc8592fbb3099dc0364c5317
Author: Richard Biener <rguenther@suse.de>
Date:   Fri Jan 13 08:56:28 2023 +0100

    solaris2: Don't add crtfastmath.o for -shared
    
    Don't add crtfastmath.o for -shared to avoid altering the FP
    environment when loading a shared library.
    
            PR target/55522
            * config/sol2.h (ENDFILE_SPEC): Don't add crtfastmath.o for -shared.

The master branch has been updated by hongtao Liu <liuhongt@gcc.gnu.org>:

https://gcc.gnu.org/g:159b0f41adc4c8ead45e12a523470381ce716ff2

commit r13-5235-g159b0f41adc4c8ead45e12a523470381ce716ff2
Author: liuhongt <hongtao.liu@intel.com>
Date:   Fri Jan 13 16:19:47 2023 +0800

    Don't add crtfastmath.o for -shared.
    
    Patches [1] and [2] fixed PR55522 for x86-linux but left all other x86
    targets unfixed (x86-cygwin, x86-darwin and x86-mingw32).
    This patch applies a similar change to other specs using crtfastmath.o.
    
    [1] https://gcc.gnu.org/pipermail/gcc-patches/2022-December/608528.html
    [2] https://gcc.gnu.org/pipermail/gcc-patches/2022-December/608529.html
    
    gcc/ChangeLog:
    
            PR target/55522
            * config/i386/cygwin.h (ENDFILE_SPEC): Link crtfastmath.o
            whenever -mdaz-ftz is specified. Don't link crtfastmath.o when
            -share or -mno-daz-ftz is specified.
            * config/i386/darwin.h (ENDFILE_SPEC): Ditto.
            * config/i386/mingw32.h (ENDFILE_SPEC): Ditto.

The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:054e407b7f68f714c024d8a9c983fdd53ceead2b

commit r13-5305-g054e407b7f68f714c024d8a9c983fdd53ceead2b
Author: Richard Biener <rguenther@suse.de>
Date:   Fri Jan 13 08:54:33 2023 +0100

    sparc: Don't add crtfastmath.o for -shared
    
    Don't add crtfastmath.o for -shared to avoid altering the FP
    environment when loading a shared library.
    
            PR target/55522
            * config/sparc/freebsd.h (ENDFILE_SPEC): Don't add crtfastmath.o
            for -shared.
            * config/sparc/linux.h (ENDFILE_SPEC): Likewise.
            * config/sparc/linux64.h (ENDFILE_SPEC): Likewise.
            * config/sparc/sp-elf.h (ENDFILE_SPEC): Likewise.
            * config/sparc/sp64-elf.h (ENDFILE_SPEC): Likewise.

The master branch has been updated by Chenghua Xu <paulhua@gcc.gnu.org>:

https://gcc.gnu.org/g:476efe839e069e556b4b03cf6ec8c18870867960

commit r13-5424-g476efe839e069e556b4b03cf6ec8c18870867960
Author: Richard Biener <rguenther@suse.de>
Date:   Fri Jan 13 09:01:12 2023 +0100

    LoongArch: Don't add crtfastmath.o for -shared
    
    Don't add crtfastmath.o for -shared to avoid altering the FP
    environment when loading a shared library.
    
            PR target/55522
            * config/loongarch/gnu-user.h (GNU_USER_TARGET_MATHFILE_SPEC):
            Don't add crtfastmath.o for -shared.

The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:31949fba1222692548b501e4dfa4ddcf7b7fc7c6

commit r13-5450-g31949fba1222692548b501e4dfa4ddcf7b7fc7c6
Author: Richard Biener <rguenther@suse.de>
Date:   Fri Jan 13 08:50:14 2023 +0100

    alpha: Don't add crtfastmath.o for -shared
    
    Don't add crtfastmath.o for -shared to avoid altering the FP
    environment when loading a shared library.
    
            PR target/55522
            * config/alpha/linux.h (ENDFILE_SPEC): Don't add
            crtfastmath.o for -shared.

The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:ed0a765ef9b7be420fc4cf6e653fc7350f84981c

commit r13-5451-ged0a765ef9b7be420fc4cf6e653fc7350f84981c
Author: Richard Biener <rguenther@suse.de>
Date:   Fri Jan 13 08:52:07 2023 +0100

    ia64: Don't add crtfastmath.o for -shared
    
    Don't add crtfastmath.o for -shared to avoid altering the FP
    environment when loading a shared library.
    
            PR target/55522
            * config/ia64/linux.h (ENDFILE_SPEC): Don't add crtfastmath.o
            for -shared.

The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:6cdb4d4bcd52c95cb6d367b93a82eb599d4e26f0

commit r13-5452-g6cdb4d4bcd52c95cb6d367b93a82eb599d4e26f0
Author: Richard Biener <rguenther@suse.de>
Date:   Fri Jan 13 08:53:44 2023 +0100

    mips: Don't add crtfastmath.o for -shared
    
    Don't add crtfastmath.o for -shared to avoid altering the FP
    environment when loading a shared library.
    
            PR target/55522
            * config/mips/gnu-user.h (GNU_USER_TARGET_MATHFILE_SPEC):
            Don't add crtfastmath.o for -shared.

The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:b799acef461bcb78d77819dd27c8e4806177d841

commit r13-5453-gb799acef461bcb78d77819dd27c8e4806177d841
Author: Richard Biener <rguenther@suse.de>
Date:   Fri Jan 13 08:57:12 2023 +0100

    Clarify -shared effect on crtfastmath.o
    
    This rewords the note to not specifically mention crtfastmath.o
    but FP environment altering by -ffast-math or -Ofast.
    
            PR target/55522
            * doc/invoke.texi (-shared): Clarify effect on -ffast-math
            and -Ofast FP environment side-effects.

Fixed for GCC 13.

Uh, should there be some sort of -mdaz-ftz for the other arches too?

Will/Could this be back-ported to the 12.x or lower versions? (Wasn't clear to me from previous comments if this would be the case).

(In reply to OH from comment #47)
> Will/Could this be back-ported to the 12.x or lower versions? (Wasn't clear
> to me from previous comments if this would be the case).

Changing the behavior mid-release is not a good idea, so no.