Subject: rare Emacs screwups on x86 due to GCC bug 58416
From: Paul Eggert <[email-masked]>
To: Emacs bug reports and feature requests <[email-masked]>
Date: Tue, 16 Jul 2024 16:26:33 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

While testing GNU Emacs built on Fedora 40 with gcc (GCC) 14.1.1 
20240607 (Red Hat 14.1.1-5) with -m32 for x86 and configured 
--with-wide-int, I discovered that Emacs misbehaved in a hard-to-debug 
way due to GCC bug 58416. This bug causes GCC to generate wrong x86 
machine instructions when a C program accesses a union containing a 
'double'.

The bug I observed is that if you have something like this:

    union u { double d; long long int i; } u;

then GCC sometimes generates x86 instructions that copy u.i by using 
fldl/fstpl instruction pairs to push the 64-bit quantity onto the 387 
floating point stack, and then pop the stack into another memory 
location. Unfortunately the fldl/fstpl trick fails in the unusual case 
when the bit pattern of u.i, when interpreted as a double, is a NaN, as 
that can cause the fldl/fstpl pair to store a different NaN with a 
different bit pattern, which means the destination integer disagrees 
with u.i.

The bug is obscure, since the bug's presence depends on the GCC version, 
on the optimization options used, on the exact source code, and on the 
exact integer value at runtime (the value is typically copied correctly 
even when GCC has generated the incorrect machine code, since most long 
long int values don't alias with NaNs).

In short the bug appears to be rare.

Here are some possible courses of action:

* Do nothing and hope x86 users won't run into this rare bug.

* Have the GCC folks fix the bug. However, given that the bug has been 
reported for over a decade multiple times without a fix, it seems that 
fixing it is too difficult and/or too low priority for this aging 
platform. Also, even if the bug is fixed in future GCC the bug will 
still be present with people using older GCC.

* Build with Clang or some other compiler instead. We should be 
encouraging GCC, though.

* Rewrite Emacs to never use 'double' (or 'float' or 'long double') 
inside a union. This could be painful and hardly seems worthwhile.

* When using GCC to build Emacs on x86, compile with safer options that 
make the bug impossible. The attached proposed patch does that, by 
telling GCC not to use the 387 stack. (This patch fixed the Emacs 
misbehavior in my experimental build.) The downside is that the 
resulting Emacs executables need SSE2, introduced for the Pentium 4 in 
2000 <https://en.wikipedia.org/wiki/SSE2>. Nowadays few users need to 
run Emacs on non-SSE2 x86, so this may be good enough. Also, the 
proposed patch gives the builder an option to compile Emacs without the 
safer options, for people who want to build for older Intel-compatible 
platforms and who don't mind an occasional wrong answer or crash.
From b6c6c0545607687e36fc47e5d5079aec4f58d591 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Mon, 15 Jul 2024 10:26:47 -0700
Subject: [PATCH] Work around GCC bug 58416 when building for x86

* configure.ac (C_SWITCH_MACHINE): Add -mfpmath=sse and perhaps
-msse2 to work around GCC bug 58416.
---
 configure.ac | 43 +++++++++++++++++++++++++++++++++++++++++++
 etc/NEWS     |  9 +++++++++
 2 files changed, 52 insertions(+)

diff --git a/configure.ac b/configure.ac
index e2b6dc2fc4d..4e74d66c65f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2325,6 +2325,49 @@ AC_DEFUN
     fi
   ;;
 esac
+
+AC_CACHE_CHECK([for flags to work around GCC bug 58416],
+  [emacs_cv_gcc_bug_58416_CFLAGS],
+  [emacs_cv_gcc_bug_58416_CFLAGS='none needed'
+   AS_CASE([$canonical],
+     [[i[3456]86-* | x86_64-*]],
+	[AS_IF([test "$GCC" = yes],
+	   [old_CFLAGS=$CFLAGS
+	    for emacs_cv_gcc_bug_58416_CFLAGS in \
+		'none needed' '-mfpmath=sse' '-msse2 -mfpmath=sse' 'none work'
+	    do
+	      AS_CASE([$emacs_cv_gcc_bug_58416_CFLAGS],
+	        ['none work'], [break],
+		['none needed'], [],
+		[CFLAGS="$old_CFLAGS $emacs_cv_gcc_bug_58416_CFLAGS"])
+	      AC_COMPILE_IFELSE(
+		[AC_LANG_DEFINES_PROVIDED
+		 [/* Work around GCC bug with double in unions on x86,
+		     where the generated insns copy non-floating-point data
+		     via fldl/fstpl instruction pairs.  This can misbehave
+		     the data's bit pattern looks like a NaN.  See, e.g.:
+			https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58416
+			https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93271
+			https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114659
+		     Problem observed with 'gcc -m32' with GCC 14.1.1
+		     20240607 (Red Hat 14.1.1-5) on x86-64.  */
+		  #include <float.h>
+		  #if \
+		      ((defined __GNUC__ && !defined __clang__) \
+		       && (defined __i386__ || defined __x86_64__) \
+		       && ! (defined FLT_EVAL_METHOD \
+			     && 0 <= FLT_EVAL_METHOD \
+			     && FLT_EVAL_METHOD <= 1))
+		  # error "GCC bug 58416 is possibly present"
+		  #endif
+		]],
+		[break])
+	    done
+	    CFLAGS=$old_CFLAGS])])])
+AS_CASE([$emacs_cv_gcc_bug_58416_CFLAGS],
+  [-*],
+    [C_SWITCH_MACHINE="$C_SWITCH_MACHINE $emacs_cv_gcc_bug_58416_CFLAGS"])
+
 AC_SUBST([C_SWITCH_MACHINE])
 
 C_SWITCH_SYSTEM=
diff --git a/etc/NEWS b/etc/NEWS
index f10f9ae4d65..f3852750b9a 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -24,6 +24,15 @@ applies, and please also update docstrings as needed.
 
 * Installation Changes in Emacs 31.1
 
+** When using GCC to build Emacs on 32-bit x86 systems, 'configure' now
+defaults to specifying the GCC options -msse2 and -mfpmath=sse to work
+around GCC bug 58416.  As a result, the resulting Emacs executable now
+requires support for SSE2, introduced by Intel in 2000 for the Pentium 4
+and by AMD in 2003 for the Opteron and Athlon 64.  To build Emacs with
+GCC for older x86 processors, pass 'emacs_cv_gcc_bug_58416_CFLAGS=no' to
+'configure'; although the resulting Emacs may generate incorrect results
+or dump core, any such misbehavior should be rare.
+
 
 * Startup Changes in Emacs 31.1
 
-- 
2.45.2