manylinux: Truly disable the effect of -Ofast/-ffast-math

Context

Commit history and issue history leading up to this PR:

• https://github.com/gevent/gevent/commit/59478ebb12384403932125d9ffefea4413840ca6 added -Ofast to CFLAGS
• #1820 added -fno-fast-math to CFLAGS in order to prevent process-wide floating-point flags from being set as an effect of -Ofast, which enables -ffast-math
• It was discovered in https://github.com/HypothesisWorks/hypothesis/issues/3092 that importing gevent still enables the floating-point flags

Minimally reproducible example

Running mxcsr_test/test.sh from my sscce-no-fast-math branch prints the status of floating-point flags after importing gevent (version 21.12.0):

$ ./test.sh
Requirement already satisfied: gevent==21.12.0 in ./env/lib/python3.10/site-packages (from -r requirements.txt (line 1)) (21.12.0)
Requirement already satisfied: zope.interface in ./env/lib/python3.10/site-packages (from gevent==21.12.0->-r requirements.txt (line 1)) (5.4.0)
Requirement already satisfied: setuptools in ./env/lib/python3.10/site-packages (from gevent==21.12.0->-r requirements.txt (line 1)) (56.0.0)
Requirement already satisfied: greenlet<2.0,>=1.1.0 in ./env/lib/python3.10/site-packages (from gevent==21.12.0->-r requirements.txt (line 1)) (1.1.2)
Requirement already satisfied: zope.event in ./env/lib/python3.10/site-packages (from gevent==21.12.0->-r requirements.txt (line 1)) (4.5.0)
running build_ext
copying build/lib.linux-x86_64-3.10/mxcsr.cpython-310-x86_64-linux-gnu.so -> 
daz_enabled=True
ftz_enabled=True
Did not expect DAZ nor FTZ to be enabled

Why does not -fno-fast-math take effect?

gcc's crtfastmath.c is the code responsible for enabling the floating-point flags, and it appears to be that it gets linked whenever -Ofast, -ffast-math, or -funsafe-math-optimizations is specified: https://github.com/gcc-mirror/gcc/blob/releases/gcc-11.2.0/gcc/config/i386/gnu-user-common.h#L50

Running cd gcc_test; make; cat result.txt in my sscce-no-fast-math branch demonstrates that -fno-fast-math cancels out -ffast-math but not -Ofast when it comes to preventing crtfastmath.c from being linked.

$ cat result.txt 
./check_ofast
DAZ enabled: yes
FTZ enabled: yes
./check_fm
DAZ enabled: yes
FTZ enabled: yes
./check_ofast_nofm
DAZ enabled: yes
FTZ enabled: yes
./check_fm_nofm
DAZ enabled: no
FTZ enabled: no
./check_ofast_fm_nofm
DAZ enabled: yes
FTZ enabled: yes

./check_ofast is an executable compiled with -Ofast, ./check_fm with -ffast-math, ./check_ofast_nofm with -Ofast -fno-fast-math, and so on.

This PR's changes

From gcc's documentation: https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html

-Ofast [..] enables optimizations that are not valid for all standard-compliant programs

The commit that introduced -Ofast, 59478ebb12384403932125d9ffefea4413840ca6, only mentions build time, and enabling this level of optimization that can break standard-compliance didn't seem to be the intention. So this PR sets the optimization level to -O3 to avoid any other unexpected behavior.

The behavior of -fno-fast-math described above may be a bug on gcc's end, but I figured we can simply go with -O3 here.

@ento we are also hitting the same assertion error while using gevent+hypothesis+locust. Do you know of any workaround until this PR is released? I tried using daz library to unset daz, ftz but that also gives the same error while running in docker.

@amitpl In my case, I'm 'working around' it by holding off on merging the feature branch that introduces gevent (as a transitive dependency of locust, as it turns out) to a codebase that uses hypothesis.

Does unsetting daz and ftz work in a non-Docker environment? I'd try adding this little C extension that reads the current state of the flags to the codebase and verify that they really are unset right before the assertion error gets thrown, patching hypothesis' internal code to call mxcsr.daz_enabled() and mxcsr.ftz_enabled() and print the results if needed.

Thanks for the PR @ento FTR, we are also running into this issue and even trying daz and ftz didn't work inside Docker container so we are fully blocked on using the Locust library (since it uses gevent internally).

Just curious, any rough timelines on when this could be merged and released?

@jamadden @denik Would you mind to have a look at this? Would be great if we could get it merged :)

And pushing out a bugfix release would also be highly appreciated.

Any update on this?

Aww!

$ echo 'int main() {}' > foo.c
$ gcc -Ofast -fno-fast-math -o foo-gcc foo.c
$ clang -Ofast -fno-fast-math -o foo-clang foo.c

and then

$ objdump -d foo-clang | grep fast
00000000004003e0 <set_fast_math>:
$ objdump -d foo-gcc | grep fast
00000000004003f0 <set_fast_math>:

clang is apparently intentionally matching gcc behavior here.

@jamadden mind if we part with -Ofast?

Congratulations, this bug made public news appearance now, since there is no one there to press the button to get this PR accepted :(

Is there any chance to get gevent into for example PSF administration, to guarantee some more maintainer attention for such occasions?

https://www.golem.de/news/fpu-tausende-python-pakete-koennten-falsche-berechnungen-liefern-2209-168134.html

Not exactly world news :)

Jason has been maintaining this for about a decade. Just because this concern is not being addressed doesn't mean that it's abandoned.

Also, does the PSF provide guaranteed maintainers for projects? Take a look at the code, gevent is pretty complex.

I'm very grateful for Jasons work and the code is surely complex. Nevertheless, it shouldn't happen that a project of that size depends on a single maintainer. At least some co-maintainer to accept occasional bug fixing PRs should be there. It's not that gevent would need new features at this point.

Thank you all for your patience and contributions! This has been merged as-of https://github.com/gevent/gevent/commit/e29bd2ee11ca5f78cc9c9bc784d875681afb8d23

How would we pick up the changes? Will there be a new tag/release any time soon?