[PATCH 0/7] Multiple floating-point environment fixes

[PATCH 0/7] Multiple floating-point environment fixes

[Adhemerval Zanella]

Bruno Haible has uncovered multiple issues with floating point
environment functions on multiple platforms.  He already added
gnulib modules to overrides some functions, so some of theses fixes
arealready being used on some projects.

There are still some issues where I am not confortable to fix or
install a patch without proper hardware testing (BZ# 31023 for hppa
and BZ# 30993 for alpha), and there also one that would require a
lot of working since it requires fixing the compiler (BZ# 30973
for sh4).

Adhemerval Zanella (3):
  powerpc: Do not raise exception traps for fesetexcept/fesetexceptflag
    (BZ 30988)
  i686: Do not raise exception traps on fesetexcept (BZ 30989)
  riscv: Fix feenvupdate with FE_DFL_ENV (BZ 31022)

Bruno Haible (4):
  x86: Do not raises floating-point exception traps on fesetexceptflag
    (BZ 30990)
  manual: Clarify undefined behavior of feenableexcept (BZ 31019)
  alpha: Fix fesetexceptflag (BZ 30998)
  hppa: Fix undefined behaviour in feclearexcept (BZ 30983)

 manual/arith.texi                        |   6 ++
 math/test-fenv.c                         | 132 +++++++++++++++++++++--
 math/test-fesetexcept-traps.c            |  37 +++++--
 math/test-fexcept-traps.c                |  33 ++++--
 sysdeps/alpha/fpu/fsetexcptflg.c         |   2 +-
 sysdeps/hppa/fpu/fclrexcpt.c             |   2 +-
 sysdeps/i386/fpu/fesetexcept.c           |  46 +++++++-
 sysdeps/i386/fpu/fsetexcptflg.c          |  63 +++++++----
 sysdeps/i386/fpu/math-tests-trap-force.h |  29 +++++
 sysdeps/powerpc/fpu/fesetexcept.c        |   5 +
 sysdeps/powerpc/fpu/fsetexcptflg.c       |   9 +-
 sysdeps/riscv/rvf/fenv_private.h         |   6 +-
 sysdeps/x86/fpu/test-fenv-sse-2.c        |  23 +---
 sysdeps/x86_64/fpu/fsetexcptflg.c        |  24 +++--
 14 files changed, 332 insertions(+), 85 deletions(-)
 create mode 100644 sysdeps/i386/fpu/math-tests-trap-force.h

-- 
2.34.1

[PATCH 1/7] powerpc: Do not raise exception traps for fesetexcept/fesetexceptflag (BZ 30988)

[Adhemerval Zanella]

According to ISO C23 (7.6.4.4), fesetexcept is supposed to set
floating-point exception flags without raising a trap (unlike
feraiseexcept, which is supposed to raise a trap if feenableexcept was
called with the appropriate argument).

This is a side-effect of how we implement the GNU extension
feenableexcept, where feenableexcept/fesetenv/fesetmode/feupdateenv
might issue prctl (PR_SET_FPEXC, PR_FP_EXC_PRECISE) depending of the
argument.  And on PR_FP_EXC_PRECISE, setting a floating-point exception
flag triggers a trap.

To make the both functions follow the C23, fesetexcept and
fesetexceptflag now fail if the argument may trigger a trap.

The math tests now check for an value different than 0, instead
of bail out as unsupported for EXCEPTION_SET_FORCES_TRAP.

Checked on powerpc64le-linux-gnu.
---
 math/test-fesetexcept-traps.c      | 11 ++++-------
 math/test-fexcept-traps.c          | 11 ++++-------
 sysdeps/powerpc/fpu/fesetexcept.c  |  5 +++++
 sysdeps/powerpc/fpu/fsetexcptflg.c |  9 ++++++++-
 4 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/math/test-fesetexcept-traps.c b/math/test-fesetexcept-traps.c
index 71b6e45b33..96f6c4752f 100644
--- a/math/test-fesetexcept-traps.c
+++ b/math/test-fesetexcept-traps.c
@@ -39,16 +39,13 @@ do_test (void)
       return result;
     }
 
-  if (EXCEPTION_SET_FORCES_TRAP)
-    {
-      puts ("setting exceptions traps, cannot test on this architecture");
-      return 77;
-    }
-  /* Verify fesetexcept does not cause exception traps.  */
+  /* Verify fesetexcept does not cause exception traps.  For architectures
+     where setting the exception might result in traps the function should
+     return a nonzero value.  */
   ret = fesetexcept (FE_ALL_EXCEPT);
   if (ret == 0)
     puts ("fesetexcept (FE_ALL_EXCEPT) succeeded");
-  else
+  else if (!EXCEPTION_SET_FORCES_TRAP)
     {
       puts ("fesetexcept (FE_ALL_EXCEPT) failed");
       if (EXCEPTION_TESTS (float))
diff --git a/math/test-fexcept-traps.c b/math/test-fexcept-traps.c
index 9701c3c320..9b8f583ae6 100644
--- a/math/test-fexcept-traps.c
+++ b/math/test-fexcept-traps.c
@@ -63,14 +63,11 @@ do_test (void)
       result = 1;
     }
 
-  if (EXCEPTION_SET_FORCES_TRAP)
-    {
-      puts ("setting exceptions traps, cannot test on this architecture");
-      return 77;
-    }
-  /* The test is that this does not cause exception traps.  */
+  /* The test is that this does not cause exception traps.  For architectures
+     where setting the exception might result in traps the function should
+     return a nonzero value.  */
   ret = fesetexceptflag (&saved, FE_ALL_EXCEPT);
-  if (ret != 0)
+  if (ret != 0 && !EXCEPTION_SET_FORCES_TRAP)
     {
       puts ("fesetexceptflag failed");
       result = 1;
diff --git a/sysdeps/powerpc/fpu/fesetexcept.c b/sysdeps/powerpc/fpu/fesetexcept.c
index 609a148a95..2850156d3a 100644
--- a/sysdeps/powerpc/fpu/fesetexcept.c
+++ b/sysdeps/powerpc/fpu/fesetexcept.c
@@ -31,6 +31,11 @@ fesetexcept (int excepts)
 	    & FE_INVALID_SOFTWARE));
   if (n.l != u.l)
     {
+      if (n.l & fenv_exceptions_to_reg (excepts))
+	/* Setting the exception flags may trigger a trap.  ISO C 23 § 7.6.4.4
+	    does not allow it.   */
+	return -1;
+
       fesetenv_register (n.fenv);
 
       /* Deal with FE_INVALID_SOFTWARE not being implemented on some chips.  */
diff --git a/sysdeps/powerpc/fpu/fsetexcptflg.c b/sysdeps/powerpc/fpu/fsetexcptflg.c
index 2b22f913c0..6517e8ea03 100644
--- a/sysdeps/powerpc/fpu/fsetexcptflg.c
+++ b/sysdeps/powerpc/fpu/fsetexcptflg.c
@@ -44,7 +44,14 @@ __fesetexceptflag (const fexcept_t *flagp, int excepts)
      This may cause floating-point exceptions if the restored state
      requests it.  */
   if (n.l != u.l)
-    fesetenv_register (n.fenv);
+    {
+      if (n.l & fenv_exceptions_to_reg (excepts))
+	/* Setting the exception flags may trigger a trap.  ISO C 23 § 7.6.4.4
+	    does not allow it.   */
+	return -1;
+
+      fesetenv_register (n.fenv);
+    }
 
   /* Deal with FE_INVALID_SOFTWARE not being implemented on some chips.  */
   if (flag & FE_INVALID)
-- 
2.34.1

[PATCH 2/7] i686: Do not raise exception traps on fesetexcept (BZ 30989)

[Adhemerval Zanella]

According to ISO C23 (7.6.4.4), fesetexcept is supposed to set
floating-point exception flags without raising a trap (unlike
feraiseexcept, which is supposed to raise a trap if feenableexcept
was called with the appropriate argument).

The flags can be set in the 387 unit or in the SSE unit.  To set
a flag, it is sufficient to do it in the SSE unit, because that is
guaranteed to not trap.  However, on i386 CPUs that have only a
387 unit, set the flags in the 387, as long as this cannot trap.

Checked on i686-linux-gnu.
---
 math/test-fesetexcept-traps.c            | 28 ++++++++++++---
 sysdeps/i386/fpu/fesetexcept.c           | 46 +++++++++++++++++++++---
 sysdeps/i386/fpu/math-tests-trap-force.h | 29 +++++++++++++++
 sysdeps/x86/fpu/test-fenv-sse-2.c        | 23 +++---------
 4 files changed, 100 insertions(+), 26 deletions(-)
 create mode 100644 sysdeps/i386/fpu/math-tests-trap-force.h

diff --git a/math/test-fesetexcept-traps.c b/math/test-fesetexcept-traps.c
index 96f6c4752f..8a5c0bca80 100644
--- a/math/test-fesetexcept-traps.c
+++ b/math/test-fesetexcept-traps.c
@@ -19,6 +19,7 @@
 #include <fenv.h>
 #include <stdio.h>
 #include <math-tests.h>
+#include <math-barriers.h>
 
 static int
 do_test (void)
@@ -41,8 +42,28 @@ do_test (void)
 
   /* Verify fesetexcept does not cause exception traps.  For architectures
      where setting the exception might result in traps the function should
-     return a nonzero value.  */
-  ret = fesetexcept (FE_ALL_EXCEPT);
+     return a nonzero value.
+     Also check if the function does not alter the exception mask.  */
+  {
+    int exc_before = fegetexcept ();
+    ret = fesetexcept (FE_ALL_EXCEPT);
+    int exc_after = fegetexcept ();
+    if (exc_before != exc_after)
+      {
+	puts ("fesetexcept (FE_ALL_EXCEPT) changed the exceptions mask");
+	return 1;
+      }
+  }
+
+  /* Execute some floating-point operations, since on some CPUs exceptions
+     triggers a trap only at the next floating-point instruction.  */
+  volatile double a = 1.0;
+  volatile double b = a + a;
+  math_force_eval (b);
+  volatile long double al = 1.0L;
+  volatile long double bl = al + al;
+  math_force_eval (bl);
+
   if (ret == 0)
     puts ("fesetexcept (FE_ALL_EXCEPT) succeeded");
   else if (!EXCEPTION_SET_FORCES_TRAP)
@@ -61,5 +82,4 @@ do_test (void)
   return result;
 }
 
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
+#include <support/test-driver.c>
diff --git a/sysdeps/i386/fpu/fesetexcept.c b/sysdeps/i386/fpu/fesetexcept.c
index 18949e982a..2e9580207c 100644
--- a/sysdeps/i386/fpu/fesetexcept.c
+++ b/sysdeps/i386/fpu/fesetexcept.c
@@ -17,15 +17,53 @@
    <https://www.gnu.org/licenses/>.  */
 
 #include <fenv.h>
+#include <ldsodefs.h>
 
 int
 fesetexcept (int excepts)
 {
-  fenv_t temp;
+  /* The flags can be set in the 387 unit or in the SSE unit.  To set a flag,
+     it is sufficient to do it in the SSE unit, because that is guaranteed to
+     not trap.  However, on i386 CPUs that have only a 387 unit, set the flags
+     in the 387, as long as this cannot trap.  */
 
-  __asm__ ("fnstenv %0" : "=m" (*&temp));
-  temp.__status_word |= excepts & FE_ALL_EXCEPT;
-  __asm__ ("fldenv %0" : : "m" (*&temp));
+  excepts &= FE_ALL_EXCEPT;
+
+  if (CPU_FEATURE_USABLE (SSE))
+    {
+      /* And now similarly for SSE.  */
+      unsigned int mxcsr;
+      __asm__ ("stmxcsr %0" : "=m" (*&mxcsr));
+
+      /* Set relevant flags.  */
+      mxcsr |= excepts;
+
+      /* Put the new data in effect.  */
+      __asm__ ("ldmxcsr %0" : : "m" (*&mxcsr));
+    }
+  else
+    {
+      fenv_t temp;
+
+      /* Note: fnstenv masks all floating-point exceptions until the fldenv
+	 or fldcw below.  */
+      __asm__ ("fnstenv %0" : "=m" (*&temp));
+
+      /* Set relevant flags.  */
+      temp.__status_word |= excepts;
+
+      if ((~temp.__control_word) & excepts)
+	{
+	  /* Setting the exception flags may trigger a trap (at the next
+	     floating-point instruction, but that does not matter).
+	     ISO C23 (7.6.4.4) does not allow it.  */
+	  __asm__ volatile ("fldcw %0" : : "m" (*&temp.__control_word));
+	  return -1;
+	}
+
+      /* Store the new status word (along with the rest of the environment).  */
+      __asm__ ("fldenv %0" : : "m" (*&temp));
+    }
 
   return 0;
 }
diff --git a/sysdeps/i386/fpu/math-tests-trap-force.h b/sysdeps/i386/fpu/math-tests-trap-force.h
new file mode 100644
index 0000000000..f41e1ffc2d
--- /dev/null
+++ b/sysdeps/i386/fpu/math-tests-trap-force.h
@@ -0,0 +1,29 @@
+/* Configuration for math tests: support for setting exception flags
+   without causing enabled traps.  i686 version.
+   Copyright (C) 2023 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef I386_FPU_MATH_TESTS_TRAP_FORCE_H
+#define I386_FPU_MATH_TESTS_TRAP_FORCE_H 1
+
+#include <cpu-features.h>
+
+/* Setting exception flags in FPU Status Register results in enabled traps for
+   those exceptions being taken.  */
+#define EXCEPTION_SET_FORCES_TRAP !CPU_FEATURE_USABLE (SSE)
+
+#endif /* math-tests-trap-force.h.  */
diff --git a/sysdeps/x86/fpu/test-fenv-sse-2.c b/sysdeps/x86/fpu/test-fenv-sse-2.c
index f3e820b6ed..7a0503790f 100644
--- a/sysdeps/x86/fpu/test-fenv-sse-2.c
+++ b/sysdeps/x86/fpu/test-fenv-sse-2.c
@@ -22,17 +22,8 @@
 #include <stdbool.h>
 #include <stdint.h>
 #include <stdio.h>
-
-static bool
-have_sse2 (void)
-{
-  unsigned int eax, ebx, ecx, edx;
-
-  if (!__get_cpuid (1, &eax, &ebx, &ecx, &edx))
-    return false;
-
-  return (edx & bit_SSE2) != 0;
-}
+#include <cpu-features.h>
+#include <support/check.h>
 
 static uint32_t
 get_sse_mxcsr (void)
@@ -164,13 +155,9 @@ sse_tests (void)
 static int
 do_test (void)
 {
-  if (!have_sse2 ())
-    {
-      puts ("CPU does not support SSE2, cannot test");
-      return 0;
-    }
+  if (!CPU_FEATURE_USABLE (SSE2))
+    FAIL_UNSUPPORTED ("CPU does not support SSE2");
   return sse_tests ();
 }
 
-#define TEST_FUNCTION do_test ()
-#include <test-skeleton.c>
+#include <support/test-driver.c>
-- 
2.34.1

Re: [PATCH 0/7] Multiple floating-point environment fixes

[Adhemerval Zanella Netto]

On 02/11/23 16:55, Adhemerval Zanella wrote:
> Bruno Haible has uncovered multiple issues with floating point
> environment functions on multiple platforms.  He already added
> gnulib modules to overrides some functions, so some of theses fixes
> arealready being used on some projects.
> 
> There are still some issues where I am not confortable to fix or
> install a patch without proper hardware testing (BZ# 31023 for hppa
> and BZ# 30993 for alpha), and there also one that would require a
> lot of working since it requires fixing the compiler (BZ# 30973
> for sh4).

I will resend, there were a problem on my side where the thread
was truncated. 

[PATCH 1/7] powerpc: Do not raise exception traps for fesetexcept/fesetexceptflag (BZ 30988)

[Adhemerval Zanella]

According to ISO C23 (7.6.4.4), fesetexcept is supposed to set
floating-point exception flags without raising a trap (unlike
feraiseexcept, which is supposed to raise a trap if feenableexcept was
called with the appropriate argument).

This is a side-effect of how we implement the GNU extension
feenableexcept, where feenableexcept/fesetenv/fesetmode/feupdateenv
might issue prctl (PR_SET_FPEXC, PR_FP_EXC_PRECISE) depending of the
argument.  And on PR_FP_EXC_PRECISE, setting a floating-point exception
flag triggers a trap.

To make the both functions follow the C23, fesetexcept and
fesetexceptflag now fail if the argument may trigger a trap.

The math tests now check for an value different than 0, instead
of bail out as unsupported for EXCEPTION_SET_FORCES_TRAP.

Checked on powerpc64le-linux-gnu.
---
 math/test-fesetexcept-traps.c      | 11 ++++-------
 math/test-fexcept-traps.c          | 11 ++++-------
 sysdeps/powerpc/fpu/fesetexcept.c  |  5 +++++
 sysdeps/powerpc/fpu/fsetexcptflg.c |  9 ++++++++-
 4 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/math/test-fesetexcept-traps.c b/math/test-fesetexcept-traps.c
index 71b6e45b33..96f6c4752f 100644
--- a/math/test-fesetexcept-traps.c
+++ b/math/test-fesetexcept-traps.c
@@ -39,16 +39,13 @@ do_test (void)
       return result;
     }
 
-  if (EXCEPTION_SET_FORCES_TRAP)
-    {
-      puts ("setting exceptions traps, cannot test on this architecture");
-      return 77;
-    }
-  /* Verify fesetexcept does not cause exception traps.  */
+  /* Verify fesetexcept does not cause exception traps.  For architectures
+     where setting the exception might result in traps the function should
+     return a nonzero value.  */
   ret = fesetexcept (FE_ALL_EXCEPT);
   if (ret == 0)
     puts ("fesetexcept (FE_ALL_EXCEPT) succeeded");
-  else
+  else if (!EXCEPTION_SET_FORCES_TRAP)
     {
       puts ("fesetexcept (FE_ALL_EXCEPT) failed");
       if (EXCEPTION_TESTS (float))
diff --git a/math/test-fexcept-traps.c b/math/test-fexcept-traps.c
index 9701c3c320..9b8f583ae6 100644
--- a/math/test-fexcept-traps.c
+++ b/math/test-fexcept-traps.c
@@ -63,14 +63,11 @@ do_test (void)
       result = 1;
     }
 
-  if (EXCEPTION_SET_FORCES_TRAP)
-    {
-      puts ("setting exceptions traps, cannot test on this architecture");
-      return 77;
-    }
-  /* The test is that this does not cause exception traps.  */
+  /* The test is that this does not cause exception traps.  For architectures
+     where setting the exception might result in traps the function should
+     return a nonzero value.  */
   ret = fesetexceptflag (&saved, FE_ALL_EXCEPT);
-  if (ret != 0)
+  if (ret != 0 && !EXCEPTION_SET_FORCES_TRAP)
     {
       puts ("fesetexceptflag failed");
       result = 1;
diff --git a/sysdeps/powerpc/fpu/fesetexcept.c b/sysdeps/powerpc/fpu/fesetexcept.c
index 609a148a95..2850156d3a 100644
--- a/sysdeps/powerpc/fpu/fesetexcept.c
+++ b/sysdeps/powerpc/fpu/fesetexcept.c
@@ -31,6 +31,11 @@ fesetexcept (int excepts)
 	    & FE_INVALID_SOFTWARE));
   if (n.l != u.l)
     {
+      if (n.l & fenv_exceptions_to_reg (excepts))
+	/* Setting the exception flags may trigger a trap.  ISO C 23 § 7.6.4.4
+	    does not allow it.   */
+	return -1;
+
       fesetenv_register (n.fenv);
 
       /* Deal with FE_INVALID_SOFTWARE not being implemented on some chips.  */
diff --git a/sysdeps/powerpc/fpu/fsetexcptflg.c b/sysdeps/powerpc/fpu/fsetexcptflg.c
index 2b22f913c0..6517e8ea03 100644
--- a/sysdeps/powerpc/fpu/fsetexcptflg.c
+++ b/sysdeps/powerpc/fpu/fsetexcptflg.c
@@ -44,7 +44,14 @@ __fesetexceptflag (const fexcept_t *flagp, int excepts)
      This may cause floating-point exceptions if the restored state
      requests it.  */
   if (n.l != u.l)
-    fesetenv_register (n.fenv);
+    {
+      if (n.l & fenv_exceptions_to_reg (excepts))
+	/* Setting the exception flags may trigger a trap.  ISO C 23 § 7.6.4.4
+	    does not allow it.   */
+	return -1;
+
+      fesetenv_register (n.fenv);
+    }
 
   /* Deal with FE_INVALID_SOFTWARE not being implemented on some chips.  */
   if (flag & FE_INVALID)
-- 
2.34.1