* [PATCH 1/7] powerpc: Do not raise exception traps for fesetexcept/fesetexceptflag (BZ 30988)
2023-11-03 12:24 [PATCH 0/7] Multiple floating-point environment fixes Adhemerval Zanella
@ 2023-11-03 12:24 ` Adhemerval Zanella
2023-11-03 12:24 ` [PATCH 2/7] i686: Do not raise exception traps on fesetexcept (BZ 30989) Adhemerval Zanella
` (6 subsequent siblings)
7 siblings, 0 replies; 24+ messages in thread
From: Adhemerval Zanella @ 2023-11-03 12:24 UTC (permalink / raw)
To: libc-alpha, Bruno Haible
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
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH 2/7] i686: Do not raise exception traps on fesetexcept (BZ 30989)
2023-11-03 12:24 [PATCH 0/7] Multiple floating-point environment fixes Adhemerval Zanella
2023-11-03 12:24 ` [PATCH 1/7] powerpc: Do not raise exception traps for fesetexcept/fesetexceptflag (BZ 30988) Adhemerval Zanella
@ 2023-11-03 12:24 ` Adhemerval Zanella
2023-11-06 0:15 ` Bruno Haible
2023-11-03 12:24 ` [PATCH 3/7] x86: Do not raises floating-point exception traps on fesetexceptflag (BZ 30990) Adhemerval Zanella
` (5 subsequent siblings)
7 siblings, 1 reply; 24+ messages in thread
From: Adhemerval Zanella @ 2023-11-03 12:24 UTC (permalink / raw)
To: libc-alpha, Bruno Haible
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
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH 3/7] x86: Do not raises floating-point exception traps on fesetexceptflag (BZ 30990)
2023-11-03 12:24 [PATCH 0/7] Multiple floating-point environment fixes Adhemerval Zanella
2023-11-03 12:24 ` [PATCH 1/7] powerpc: Do not raise exception traps for fesetexcept/fesetexceptflag (BZ 30988) Adhemerval Zanella
2023-11-03 12:24 ` [PATCH 2/7] i686: Do not raise exception traps on fesetexcept (BZ 30989) Adhemerval Zanella
@ 2023-11-03 12:24 ` Adhemerval Zanella
2023-11-06 0:17 ` Bruno Haible
2023-11-03 12:24 ` [PATCH 4/7] manual: Clarify undefined behavior of feenableexcept (BZ 31019) Adhemerval Zanella
` (4 subsequent siblings)
7 siblings, 1 reply; 24+ messages in thread
From: Adhemerval Zanella @ 2023-11-03 12:24 UTC (permalink / raw)
To: libc-alpha, Bruno Haible
From: Bruno Haible <bruno@clisp.org>
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. When we need
to clear a flag, we need to do so in both units, due to the way
fetestexcept is implemented.
When we need 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.
Co-authored-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
math/test-fexcept-traps.c | 22 ++++++++++-
sysdeps/i386/fpu/fsetexcptflg.c | 63 ++++++++++++++++++++-----------
sysdeps/x86_64/fpu/fsetexcptflg.c | 24 +++++++-----
3 files changed, 77 insertions(+), 32 deletions(-)
diff --git a/math/test-fexcept-traps.c b/math/test-fexcept-traps.c
index 9b8f583ae6..13aff229c3 100644
--- a/math/test-fexcept-traps.c
+++ b/math/test-fexcept-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)
@@ -66,7 +67,26 @@ do_test (void)
/* 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);
+ {
+ int exc_before = fegetexcept ();
+ ret = fesetexceptflag (&saved, FE_ALL_EXCEPT);
+ int exc_after = fegetexcept ();
+ if (exc_before != exc_after)
+ {
+ puts ("fesetexceptflag (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 && !EXCEPTION_SET_FORCES_TRAP)
{
puts ("fesetexceptflag failed");
diff --git a/sysdeps/i386/fpu/fsetexcptflg.c b/sysdeps/i386/fpu/fsetexcptflg.c
index e724b7d6fd..480165cff9 100644
--- a/sysdeps/i386/fpu/fsetexcptflg.c
+++ b/sysdeps/i386/fpu/fsetexcptflg.c
@@ -17,42 +17,63 @@
<https://www.gnu.org/licenses/>. */
#include <fenv.h>
-#include <math.h>
-#include <unistd.h>
#include <ldsodefs.h>
-#include <dl-procinfo.h>
int
__fesetexceptflag (const fexcept_t *flagp, int excepts)
{
- fenv_t temp;
+ /* The flags can be set in the 387 unit or in the SSE unit. When we need to
+ clear a flag, we need to do so in both units, due to the way fetestexcept
+ is implemented.
+ When we need 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. */
- /* Get the current environment. We have to do this since we cannot
- separately set the status word. */
- __asm__ ("fnstenv %0" : "=m" (*&temp));
+ fenv_t temp;
- temp.__status_word &= ~(excepts & FE_ALL_EXCEPT);
- temp.__status_word |= *flagp & excepts & FE_ALL_EXCEPT;
+ excepts &= FE_ALL_EXCEPT;
- /* Store the new status word (along with the rest of the environment.
- Possibly new exceptions are set but they won't get executed unless
- the next floating-point instruction. */
- __asm__ ("fldenv %0" : : "m" (*&temp));
+ /* Get the current x87 FPU environment. We have to do this since we
+ cannot separately set the status word.
+ Note: fnstenv masks all floating-point exceptions until the fldenv
+ or fldcw below. */
+ __asm__ ("fnstenv %0" : "=m" (*&temp));
- /* If the CPU supports SSE, we set the MXCSR as well. */
if (CPU_FEATURE_USABLE (SSE))
{
- unsigned int xnew_exc;
+ unsigned int mxcsr;
+
+ /* Clear relevant flags. */
+ temp.__status_word &= ~(excepts & ~ *flagp);
- /* Get the current MXCSR. */
- __asm__ ("stmxcsr %0" : "=m" (*&xnew_exc));
+ /* Store the new status word (along with the rest of the environment). */
+ __asm__ ("fldenv %0" : : "m" (*&temp));
- /* Set the relevant bits. */
- xnew_exc &= ~(excepts & FE_ALL_EXCEPT);
- xnew_exc |= *flagp & excepts & FE_ALL_EXCEPT;
+ /* And now similarly for SSE. */
+ __asm__ ("stmxcsr %0" : "=m" (*&mxcsr));
+
+ /* Clear or set relevant flags. */
+ mxcsr ^= (mxcsr ^ *flagp) & excepts;
/* Put the new data in effect. */
- __asm__ ("ldmxcsr %0" : : "m" (*&xnew_exc));
+ __asm__ ("ldmxcsr %0" : : "m" (*&mxcsr));
+ }
+ else
+ {
+ /* Clear or set relevant flags. */
+ temp.__status_word ^= (temp.__status_word ^ *flagp) & excepts;
+
+ if ((~temp.__control_word) & temp.__status_word & excepts)
+ {
+ /* Setting the exception flags may trigger a trap (at the next
+ floating-point instruction, but that does not matter).
+ ISO C 23 § 7.6.4.5 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));
}
/* Success. */
diff --git a/sysdeps/x86_64/fpu/fsetexcptflg.c b/sysdeps/x86_64/fpu/fsetexcptflg.c
index a3ac1dea01..2ce2b509f2 100644
--- a/sysdeps/x86_64/fpu/fsetexcptflg.c
+++ b/sysdeps/x86_64/fpu/fsetexcptflg.c
@@ -22,30 +22,34 @@
int
fesetexceptflag (const fexcept_t *flagp, int excepts)
{
+ /* The flags can be set in the 387 unit or in the SSE unit.
+ When we need to clear a flag, we need to do so in both units,
+ due to the way fetestexcept() is implemented.
+ When we need to set a flag, it is sufficient to do it in the SSE unit,
+ because that is guaranteed to not trap. */
+
fenv_t temp;
unsigned int mxcsr;
- /* XXX: Do we really need to set both the exception in both units?
- Shouldn't it be enough to set only the SSE unit? */
+ excepts &= FE_ALL_EXCEPT;
/* Get the current x87 FPU environment. We have to do this since we
cannot separately set the status word. */
__asm__ ("fnstenv %0" : "=m" (*&temp));
- temp.__status_word &= ~(excepts & FE_ALL_EXCEPT);
- temp.__status_word |= *flagp & excepts & FE_ALL_EXCEPT;
+ /* Clear relevant flags. */
+ temp.__status_word &= ~(excepts & ~ *flagp);
- /* Store the new status word (along with the rest of the environment.
- Possibly new exceptions are set but they won't get executed unless
- the next floating-point instruction. */
+ /* Store the new status word (along with the rest of the environment). */
__asm__ ("fldenv %0" : : "m" (*&temp));
- /* And now the same for SSE. */
+ /* And now similarly for SSE. */
__asm__ ("stmxcsr %0" : "=m" (*&mxcsr));
- mxcsr &= ~(excepts & FE_ALL_EXCEPT);
- mxcsr |= *flagp & excepts & FE_ALL_EXCEPT;
+ /* Clear or set relevant flags. */
+ mxcsr ^= (mxcsr ^ *flagp) & excepts;
+ /* Put the new data in effect. */
__asm__ ("ldmxcsr %0" : : "m" (*&mxcsr));
/* Success. */
--
2.34.1
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH 4/7] manual: Clarify undefined behavior of feenableexcept (BZ 31019)
2023-11-03 12:24 [PATCH 0/7] Multiple floating-point environment fixes Adhemerval Zanella
` (2 preceding siblings ...)
2023-11-03 12:24 ` [PATCH 3/7] x86: Do not raises floating-point exception traps on fesetexceptflag (BZ 30990) Adhemerval Zanella
@ 2023-11-03 12:24 ` Adhemerval Zanella
2023-11-03 16:20 ` Joseph Myers
2023-11-03 12:24 ` [PATCH 5/7] riscv: Fix feenvupdate with FE_DFL_ENV (BZ 31022) Adhemerval Zanella
` (3 subsequent siblings)
7 siblings, 1 reply; 24+ messages in thread
From: Adhemerval Zanella @ 2023-11-03 12:24 UTC (permalink / raw)
To: libc-alpha, Bruno Haible
From: Bruno Haible <bruno@clisp.org>
* manual/arith.texi (Control Functions): Explain undefined behavior
of feenableexcept in a special case.
---
manual/arith.texi | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/manual/arith.texi b/manual/arith.texi
index fa7110e992..68c6446a5b 100644
--- a/manual/arith.texi
+++ b/manual/arith.texi
@@ -1176,6 +1176,12 @@ enabled, the status of the other exceptions is not changed.
The function returns the previous enabled exceptions in case the
operation was successful, @code{-1} otherwise.
+
+Note: Enabling traps for an exception for which the exception flag is
+currently already set (@pxref{Status bit operations}) has undefined
+consequences: it may or may not trigger a trap immediately.
+@c It triggers a trap immediately on powerpc*, at the next floating-
+@c instruction on i386, and not at all on the other CPUs.
@end deftypefun
@deftypefun int fedisableexcept (int @var{excepts})
--
2.34.1
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 4/7] manual: Clarify undefined behavior of feenableexcept (BZ 31019)
2023-11-03 12:24 ` [PATCH 4/7] manual: Clarify undefined behavior of feenableexcept (BZ 31019) Adhemerval Zanella
@ 2023-11-03 16:20 ` Joseph Myers
2023-11-06 0:17 ` Bruno Haible
0 siblings, 1 reply; 24+ messages in thread
From: Joseph Myers @ 2023-11-03 16:20 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha, Bruno Haible
On Fri, 3 Nov 2023, Adhemerval Zanella wrote:
> +Note: Enabling traps for an exception for which the exception flag is
> +currently already set (@pxref{Status bit operations}) has undefined
> +consequences: it may or may not trigger a trap immediately.
I think we should say "unspecified", not "undefined" (in the manual and
commit message). It might trap immediately, on a future floating-point
instruction or not at all, but there shouldn't be any other possibilities;
it's not undefined behavior.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 5/7] riscv: Fix feenvupdate with FE_DFL_ENV (BZ 31022)
2023-11-03 12:24 [PATCH 0/7] Multiple floating-point environment fixes Adhemerval Zanella
` (3 preceding siblings ...)
2023-11-03 12:24 ` [PATCH 4/7] manual: Clarify undefined behavior of feenableexcept (BZ 31019) Adhemerval Zanella
@ 2023-11-03 12:24 ` Adhemerval Zanella
2023-11-06 0:21 ` Bruno Haible
` (2 more replies)
2023-11-03 12:24 ` [PATCH 6/7] alpha: Fix fesetexceptflag (BZ 30998) Adhemerval Zanella
` (2 subsequent siblings)
7 siblings, 3 replies; 24+ messages in thread
From: Adhemerval Zanella @ 2023-11-03 12:24 UTC (permalink / raw)
To: libc-alpha, Bruno Haible
libc_feupdateenv_riscv should check for FE_DFL_ENV, similar to
libc_fesetenv_riscv.
Also extend the test-fenv.c to test fenvupdate.
Checked on riscv under qemu-system.
---
math/test-fenv.c | 132 ++++++++++++++++++++++++++++---
sysdeps/riscv/rvf/fenv_private.h | 6 +-
2 files changed, 127 insertions(+), 11 deletions(-)
diff --git a/math/test-fenv.c b/math/test-fenv.c
index 0af7141ba7..734e0a219d 100644
--- a/math/test-fenv.c
+++ b/math/test-fenv.c
@@ -196,6 +196,30 @@ set_single_exc (const char *test_name, int fe_exc, fexcept_t exception)
feclearexcept (exception);
test_exceptions (str, ALL_EXC ^ fe_exc, 0);
}
+
+static void
+update_single_exc (const char *test_name, const fenv_t *envp, int fe_exc,
+ int fe_exc_clear, fexcept_t exception)
+{
+ char str[200];
+ /* The standard allows the inexact exception to be set together with the
+ underflow and overflow exceptions. So ignore the inexact flag if the
+ others are raised. */
+ int ignore_inexact = (fe_exc & (UNDERFLOW_EXC | OVERFLOW_EXC)) != 0;
+
+ strcpy (str, test_name);
+ strcat (str, ": set flag, with rest not set");
+ feclearexcept (FE_ALL_EXCEPT);
+ feraiseexcept (exception);
+ feupdateenv (envp);
+ test_exceptions (str, fe_exc, ignore_inexact);
+
+ strcpy (str, test_name);
+ strcat (str, ": clear flag, rest also unset");
+ feclearexcept (exception);
+ feupdateenv (envp);
+ test_exceptions (str, fe_exc_clear, ignore_inexact);
+}
#endif
static void
@@ -233,22 +257,32 @@ fe_tests (void)
}
#if FE_ALL_EXCEPT
+static const char *
+funcname (int (*func)(const fenv_t *))
+{
+ if (func == fesetenv)
+ return "fesetenv";
+ else if (func == feupdateenv)
+ return "feupdateenv";
+ __builtin_unreachable ();
+}
+
/* Test that program aborts with no masked interrupts */
static void
-feenv_nomask_test (const char *flag_name, int fe_exc)
+feenv_nomask_test (const char *flag_name, int fe_exc, int (*func)(const fenv_t *))
{
# if defined FE_NOMASK_ENV
int status;
pid_t pid;
if (!EXCEPTION_ENABLE_SUPPORTED (FE_ALL_EXCEPT)
- && fesetenv (FE_NOMASK_ENV) != 0)
+ && func (FE_NOMASK_ENV) != 0)
{
printf ("Test: not testing FE_NOMASK_ENV, it isn't implemented.\n");
return;
}
- printf ("Test: after fesetenv (FE_NOMASK_ENV) processes will abort\n");
+ printf ("Test: after %s (FE_NOMASK_ENV) processes will abort\n", funcname (func));
printf (" when feraiseexcept (%s) is called.\n", flag_name);
pid = fork ();
if (pid == 0)
@@ -295,12 +329,12 @@ feenv_nomask_test (const char *flag_name, int fe_exc)
/* Test that program doesn't abort with default environment */
static void
-feenv_mask_test (const char *flag_name, int fe_exc)
+feenv_mask_test (const char *flag_name, int fe_exc, int (*func)(const fenv_t *))
{
int status;
pid_t pid;
- printf ("Test: after fesetenv (FE_DFL_ENV) processes will not abort\n");
+ printf ("Test: after %s (FE_DFL_ENV) processes will not abort\n", funcname (func));
printf (" when feraiseexcept (%s) is called.\n", flag_name);
pid = fork ();
if (pid == 0)
@@ -313,7 +347,7 @@ feenv_mask_test (const char *flag_name, int fe_exc)
setrlimit (RLIMIT_CORE, &core_limit);
#endif
- fesetenv (FE_DFL_ENV);
+ func (FE_DFL_ENV);
feraiseexcept (fe_exc);
exit (2);
}
@@ -615,10 +649,18 @@ feenable_test (const char *flag_name, int fe_exc)
static void
fe_single_test (const char *flag_name, int fe_exc)
{
- feenv_nomask_test (flag_name, fe_exc);
- feenv_mask_test (flag_name, fe_exc);
+ feenv_nomask_test (flag_name, fe_exc, fesetenv);
+ feenv_mask_test (flag_name, fe_exc, fesetenv);
feenable_test (flag_name, fe_exc);
}
+
+
+static void
+feupdate_single_test (const char *flag_name, int fe_exc)
+{
+ feenv_nomask_test (flag_name, fe_exc, feupdateenv);
+ feenv_mask_test (flag_name, fe_exc, feupdateenv);
+}
#endif
@@ -646,6 +688,73 @@ feenv_tests (void)
fesetenv (FE_DFL_ENV);
}
+#if FE_ALL_EXCEPT
+static void
+feupdateenv_single_test (const char *test_name, int fe_exc,
+ fexcept_t exception)
+{
+ char str[100];
+ fenv_t env;
+ int res;
+
+ snprintf (str, sizeof str, "feupdateenv %s and FL_DFL_ENV", test_name);
+ update_single_exc (str, FE_DFL_ENV, fe_exc, NO_EXC, exception);
+
+ feraiseexcept (FE_ALL_EXCEPT);
+ res = fegetenv (&env);
+ if (res != 0)
+ {
+ printf ("fegetenv failed: %d\n", res);
+ ++count_errors;
+ return;
+ }
+
+ snprintf (str, sizeof str, "feupdateenv %s and FE_ALL_EXCEPT", test_name);
+ update_single_exc (str, &env, ALL_EXC, ALL_EXC, exception);
+}
+#endif
+
+static void
+feupdateenv_tests (void)
+{
+ /* We might have some exceptions still set. */
+ feclearexcept (FE_ALL_EXCEPT);
+
+#ifdef FE_DIVBYZERO
+ feupdate_single_test ("FE_DIVBYZERO", FE_DIVBYZERO);
+#endif
+#ifdef FE_INVALID
+ feupdate_single_test ("FE_INVALID", FE_INVALID);
+#endif
+#ifdef FE_INEXACT
+ feupdate_single_test ("FE_INEXACT", FE_INEXACT);
+#endif
+#ifdef FE_UNDERFLOW
+ feupdate_single_test ("FE_UNDERFLOW", FE_UNDERFLOW);
+#endif
+#ifdef FE_OVERFLOW
+ feupdate_single_test ("FE_OVERFLOW", FE_OVERFLOW);
+#endif
+
+#ifdef FE_DIVBYZERO
+ feupdateenv_single_test ("DIVBYZERO", DIVBYZERO_EXC, FE_DIVBYZERO);
+#endif
+#ifdef FE_INVALID
+ feupdateenv_single_test ("INVALID", INVALID_EXC, FE_INVALID);
+#endif
+#ifdef FE_INEXACT
+ feupdateenv_single_test ("INEXACT", INEXACT_EXC, FE_INEXACT);
+#endif
+#ifdef FE_UNDERFLOW
+ feupdateenv_single_test ("UNDERFLOW", UNDERFLOW_EXC, FE_UNDERFLOW);
+#endif
+#ifdef FE_OVERFLOW
+ feupdateenv_single_test ("OVERFLOW", OVERFLOW_EXC, FE_OVERFLOW);
+#endif
+
+ feupdateenv (FE_DFL_ENV);
+}
+
static void
feholdexcept_tests (void)
@@ -766,13 +875,14 @@ initial_tests (void)
#endif
}
-int
-main (void)
+static int
+do_test (void)
{
initial_tests ();
fe_tests ();
feenv_tests ();
feholdexcept_tests ();
+ feupdateenv_tests ();
if (count_errors)
{
@@ -782,3 +892,5 @@ main (void)
printf ("\n All tests passed successfully.\n");
return 0;
}
+
+#include <support/test-driver.c>
diff --git a/sysdeps/riscv/rvf/fenv_private.h b/sysdeps/riscv/rvf/fenv_private.h
index 40e23661b7..c91d871160 100644
--- a/sysdeps/riscv/rvf/fenv_private.h
+++ b/sysdeps/riscv/rvf/fenv_private.h
@@ -123,7 +123,11 @@ libc_feupdateenv_test_riscv (const fenv_t *envp, int ex)
static __always_inline void
libc_feupdateenv_riscv (const fenv_t *envp)
{
- _FPU_SETCW (*envp | riscv_getflags ());
+ long int env = (long int) envp - (long int) FE_DFL_ENV;
+ if (env != 0)
+ env = *envp;
+
+ _FPU_SETCW (env | riscv_getflags ());
}
#define libc_feupdateenv libc_feupdateenv_riscv
--
2.34.1
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 5/7] riscv: Fix feenvupdate with FE_DFL_ENV (BZ 31022)
2023-11-03 12:24 ` [PATCH 5/7] riscv: Fix feenvupdate with FE_DFL_ENV (BZ 31022) Adhemerval Zanella
@ 2023-11-06 0:21 ` Bruno Haible
2023-11-06 11:28 ` Adhemerval Zanella Netto
2023-11-06 0:24 ` Bruno Haible
2024-01-09 13:44 ` Szabolcs Nagy
2 siblings, 1 reply; 24+ messages in thread
From: Bruno Haible @ 2023-11-06 0:21 UTC (permalink / raw)
To: libc-alpha, Adhemerval Zanella
[-- Attachment #1: Type: text/plain, Size: 1062 bytes --]
This code gave me puzzles:
long int env = (long int) envp - (long int) FE_DFL_ENV;
if (env != 0)
env = *envp;
* What is the intent of the code?
* Can signed integer overflow happen in the first line? (It cannot, due to
the value of FE_DFL_ENV being -1 and envp being otherwise aligned mod 4.
But one should not have to think that far.)
* It assumes that intptr_t is the same as 'long int'. Which is not
universally true.
I would suggest to replace the function with this code:
libc_feupdateenv_riscv (const fenv_t *envp)
{
long int env = (envp != FE_DFL_ENV ? *envp : 0);
_FPU_SETCW (env | riscv_getflags ());
}
* It is clearer.
* It does not make assumptions regarding FE_DFL_ENV or alignment.
* It produces the exact same code; see attached foo.c and foo.s,
generated by "riscv64-linux-gnu-gcc -O2 -S foo.c".
Probably the idiom that you copied was meant to allow the use of a
conditional load instruction. But I would avoid such micro-optimizations
and instead rely on the compiler to choose the best instructions for a
task.
Bruno
[-- Attachment #2: foo.c --]
[-- Type: text/x-csrc, Size: 632 bytes --]
typedef unsigned int fenv_t;
#define FE_DFL_ENV ((__const fenv_t *) -1)
# define _FPU_SETCW(cw) __asm__ volatile ("fssr %z0" : : "rJ" (cw))
static __attribute__ ((__always_inline__)) int
riscv_getflags (void)
{
int flags;
asm volatile ("frflags %0" : "=r" (flags));
return flags;
}
void
libc_feupdateenv_riscv_az (const fenv_t *envp)
{
long int env = (long int) envp - (long int) FE_DFL_ENV;
if (env != 0)
env = *envp;
_FPU_SETCW (env | riscv_getflags ());
}
void
libc_feupdateenv_riscv_bh (const fenv_t *envp)
{
long int env = (envp != FE_DFL_ENV ? *envp : 0);
_FPU_SETCW (env | riscv_getflags ());
}
[-- Attachment #3: foo.s --]
[-- Type: text/plain, Size: 839 bytes --]
.file "foo.c"
.option pic
.text
.align 1
.globl libc_feupdateenv_riscv_az
.type libc_feupdateenv_riscv_az, @function
libc_feupdateenv_riscv_az:
li a5,-1
li a4,0
beq a0,a5,.L2
lwu a4,0(a0)
.L2:
#APP
# 9 "foo.c" 1
frflags a5
# 0 "" 2
#NO_APP
sext.w a5,a5
or a5,a5,a4
#APP
# 20 "foo.c" 1
fssr a5
# 0 "" 2
#NO_APP
ret
.size libc_feupdateenv_riscv_az, .-libc_feupdateenv_riscv_az
.align 1
.globl libc_feupdateenv_riscv_bh
.type libc_feupdateenv_riscv_bh, @function
libc_feupdateenv_riscv_bh:
li a5,-1
li a4,0
beq a0,a5,.L6
lwu a4,0(a0)
.L6:
#APP
# 9 "foo.c" 1
frflags a5
# 0 "" 2
#NO_APP
sext.w a5,a5
or a5,a5,a4
#APP
# 28 "foo.c" 1
fssr a5
# 0 "" 2
#NO_APP
ret
.size libc_feupdateenv_riscv_bh, .-libc_feupdateenv_riscv_bh
.ident "GCC: (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0"
.section .note.GNU-stack,"",@progbits
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 5/7] riscv: Fix feenvupdate with FE_DFL_ENV (BZ 31022)
2023-11-06 0:21 ` Bruno Haible
@ 2023-11-06 11:28 ` Adhemerval Zanella Netto
0 siblings, 0 replies; 24+ messages in thread
From: Adhemerval Zanella Netto @ 2023-11-06 11:28 UTC (permalink / raw)
To: Bruno Haible, libc-alpha
On 05/11/23 21:21, Bruno Haible wrote:
> This code gave me puzzles:
>
> long int env = (long int) envp - (long int) FE_DFL_ENV;
> if (env != 0)
> env = *envp;
>
> * What is the intent of the code?
> * Can signed integer overflow happen in the first line? (It cannot, due to
> the value of FE_DFL_ENV being -1 and envp being otherwise aligned mod 4.
> But one should not have to think that far.)
> * It assumes that intptr_t is the same as 'long int'. Which is not
> universally true.
>
> I would suggest to replace the function with this code:
>
> libc_feupdateenv_riscv (const fenv_t *envp)
> {
> long int env = (envp != FE_DFL_ENV ? *envp : 0);
>
> _FPU_SETCW (env | riscv_getflags ());
> }
>
> * It is clearer.
> * It does not make assumptions regarding FE_DFL_ENV or alignment.
> * It produces the exact same code; see attached foo.c and foo.s,
> generated by "riscv64-linux-gnu-gcc -O2 -S foo.c".
Indeed I was not sure why libc_fesetenv_riscv used this specific idiom
to check whether the input is FE_DFL_ENV. I will change to this, it
is clear.
>
> Probably the idiom that you copied was meant to allow the use of a
> conditional load instruction. But I would avoid such micro-optimizations
> and instead rely on the compiler to choose the best instructions for a
> task.
>
> Bruno
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 5/7] riscv: Fix feenvupdate with FE_DFL_ENV (BZ 31022)
2023-11-03 12:24 ` [PATCH 5/7] riscv: Fix feenvupdate with FE_DFL_ENV (BZ 31022) Adhemerval Zanella
2023-11-06 0:21 ` Bruno Haible
@ 2023-11-06 0:24 ` Bruno Haible
2023-11-06 11:32 ` Adhemerval Zanella Netto
2024-01-09 13:44 ` Szabolcs Nagy
2 siblings, 1 reply; 24+ messages in thread
From: Bruno Haible @ 2023-11-06 0:24 UTC (permalink / raw)
To: libc-alpha, Adhemerval Zanella
> +feupdateenv_single_test (const char *test_name, int fe_exc,
> + fexcept_t exception)
The 3rd argument type should be 'int', not 'fexcept_t'.
The type fexcept_t is only for use with the functions fegetexceptflag,
fesetexceptflag, fetestexceptflag. It would be perfectly reasonable for
a platform to define fexcept_t to 'unsigned char', regardless of the
values of the FE_* constants.
> +update_single_exc (const char *test_name, const fenv_t *envp, int fe_exc,
> + int fe_exc_clear, fexcept_t exception)
Likewise.
Bruno
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 5/7] riscv: Fix feenvupdate with FE_DFL_ENV (BZ 31022)
2023-11-06 0:24 ` Bruno Haible
@ 2023-11-06 11:32 ` Adhemerval Zanella Netto
0 siblings, 0 replies; 24+ messages in thread
From: Adhemerval Zanella Netto @ 2023-11-06 11:32 UTC (permalink / raw)
To: Bruno Haible, libc-alpha
On 05/11/23 21:24, Bruno Haible wrote:
>> +feupdateenv_single_test (const char *test_name, int fe_exc,
>> + fexcept_t exception)
>
> The 3rd argument type should be 'int', not 'fexcept_t'.
>
> The type fexcept_t is only for use with the functions fegetexceptflag,
> fesetexceptflag, fetestexceptflag. It would be perfectly reasonable for
> a platform to define fexcept_t to 'unsigned char', regardless of the
> values of the FE_* constants.
Ok.
>
>> +update_single_exc (const char *test_name, const fenv_t *envp, int fe_exc,
>> + int fe_exc_clear, fexcept_t exception)
>
> Likewise.
Ok.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 5/7] riscv: Fix feenvupdate with FE_DFL_ENV (BZ 31022)
2023-11-03 12:24 ` [PATCH 5/7] riscv: Fix feenvupdate with FE_DFL_ENV (BZ 31022) Adhemerval Zanella
2023-11-06 0:21 ` Bruno Haible
2023-11-06 0:24 ` Bruno Haible
@ 2024-01-09 13:44 ` Szabolcs Nagy
2024-01-09 18:16 ` Adhemerval Zanella Netto
2 siblings, 1 reply; 24+ messages in thread
From: Szabolcs Nagy @ 2024-01-09 13:44 UTC (permalink / raw)
To: Adhemerval Zanella, libc-alpha, Bruno Haible
The 11/03/2023 09:24, Adhemerval Zanella wrote:
> libc_feupdateenv_riscv should check for FE_DFL_ENV, similar to
> libc_fesetenv_riscv.
>
> Also extend the test-fenv.c to test fenvupdate.
>
> Checked on riscv under qemu-system.
the test fails on aarch64 when trapping is supported.
> +++ b/math/test-fenv.c
> @@ -196,6 +196,30 @@ set_single_exc (const char *test_name, int fe_exc, fexcept_t exception)
> feclearexcept (exception);
> test_exceptions (str, ALL_EXC ^ fe_exc, 0);
> }
> +
> +static void
> +update_single_exc (const char *test_name, const fenv_t *envp, int fe_exc,
> + int fe_exc_clear, fexcept_t exception)
> +{
> + char str[200];
> + /* The standard allows the inexact exception to be set together with the
> + underflow and overflow exceptions. So ignore the inexact flag if the
> + others are raised. */
> + int ignore_inexact = (fe_exc & (UNDERFLOW_EXC | OVERFLOW_EXC)) != 0;
> +
> + strcpy (str, test_name);
> + strcat (str, ": set flag, with rest not set");
> + feclearexcept (FE_ALL_EXCEPT);
> + feraiseexcept (exception);
this relies on disabled trapping.
> + feupdateenv (envp);
> + test_exceptions (str, fe_exc, ignore_inexact);
> +
> + strcpy (str, test_name);
> + strcat (str, ": clear flag, rest also unset");
> + feclearexcept (exception);
> + feupdateenv (envp);
> + test_exceptions (str, fe_exc_clear, ignore_inexact);
> +}
...
> /* Test that program aborts with no masked interrupts */
> static void
> -feenv_nomask_test (const char *flag_name, int fe_exc)
> +feenv_nomask_test (const char *flag_name, int fe_exc, int (*func)(const fenv_t *))
> {
> # if defined FE_NOMASK_ENV
> int status;
> pid_t pid;
>
> if (!EXCEPTION_ENABLE_SUPPORTED (FE_ALL_EXCEPT)
> - && fesetenv (FE_NOMASK_ENV) != 0)
> + && func (FE_NOMASK_ENV) != 0)
trapping is enabled here on targets where trapping is optional.
> +static void
> +feupdate_single_test (const char *flag_name, int fe_exc)
> +{
> + feenv_nomask_test (flag_name, fe_exc, feupdateenv);
> + feenv_mask_test (flag_name, fe_exc, feupdateenv);
> +}
trapping is still on after this function.
> +static void
> +feupdateenv_single_test (const char *test_name, int fe_exc,
> + fexcept_t exception)
> +{
> + char str[100];
> + fenv_t env;
> + int res;
> +
> + snprintf (str, sizeof str, "feupdateenv %s and FL_DFL_ENV", test_name);
> + update_single_exc (str, FE_DFL_ENV, fe_exc, NO_EXC, exception);
this relies on disabled trapping.
> +static void
> +feupdateenv_tests (void)
> +{
...
> +#ifdef FE_OVERFLOW
> + feupdate_single_test ("FE_OVERFLOW", FE_OVERFLOW);
> +#endif
> +
> +#ifdef FE_DIVBYZERO
> + feupdateenv_single_test ("DIVBYZERO", DIVBYZERO_EXC, FE_DIVBYZERO);
> +#endif
this test fails because previous enables trapping mode.
fwiw a feupdateenv (FE_DFL_ENV); in between the tests
makes math/test-fenv pass.
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 5/7] riscv: Fix feenvupdate with FE_DFL_ENV (BZ 31022)
2024-01-09 13:44 ` Szabolcs Nagy
@ 2024-01-09 18:16 ` Adhemerval Zanella Netto
0 siblings, 0 replies; 24+ messages in thread
From: Adhemerval Zanella Netto @ 2024-01-09 18:16 UTC (permalink / raw)
To: Szabolcs Nagy, libc-alpha, Bruno Haible
On 09/01/24 10:44, Szabolcs Nagy wrote:
> The 11/03/2023 09:24, Adhemerval Zanella wrote:
>> libc_feupdateenv_riscv should check for FE_DFL_ENV, similar to
>> libc_fesetenv_riscv.
>>
>> Also extend the test-fenv.c to test fenvupdate.
>>
>> Checked on riscv under qemu-system.
>
> the test fails on aarch64 when trapping is supported.
>
>> +++ b/math/test-fenv.c
>> @@ -196,6 +196,30 @@ set_single_exc (const char *test_name, int fe_exc, fexcept_t exception)
>> feclearexcept (exception);
>> test_exceptions (str, ALL_EXC ^ fe_exc, 0);
>> }
>> +
>> +static void
>> +update_single_exc (const char *test_name, const fenv_t *envp, int fe_exc,
>> + int fe_exc_clear, fexcept_t exception)
>> +{
>> + char str[200];
>> + /* The standard allows the inexact exception to be set together with the
>> + underflow and overflow exceptions. So ignore the inexact flag if the
>> + others are raised. */
>> + int ignore_inexact = (fe_exc & (UNDERFLOW_EXC | OVERFLOW_EXC)) != 0;
>> +
>> + strcpy (str, test_name);
>> + strcat (str, ": set flag, with rest not set");
>> + feclearexcept (FE_ALL_EXCEPT);
>> + feraiseexcept (exception);
>
> this relies on disabled trapping.
>
>> + feupdateenv (envp);
>> + test_exceptions (str, fe_exc, ignore_inexact);
>> +
>> + strcpy (str, test_name);
>> + strcat (str, ": clear flag, rest also unset");
>> + feclearexcept (exception);
>> + feupdateenv (envp);
>> + test_exceptions (str, fe_exc_clear, ignore_inexact);
>> +}
> ...
>> /* Test that program aborts with no masked interrupts */
>> static void
>> -feenv_nomask_test (const char *flag_name, int fe_exc)
>> +feenv_nomask_test (const char *flag_name, int fe_exc, int (*func)(const fenv_t *))
>> {
>> # if defined FE_NOMASK_ENV
>> int status;
>> pid_t pid;
>>
>> if (!EXCEPTION_ENABLE_SUPPORTED (FE_ALL_EXCEPT)
>> - && fesetenv (FE_NOMASK_ENV) != 0)
>> + && func (FE_NOMASK_ENV) != 0)
>
> trapping is enabled here on targets where trapping is optional.
>
>> +static void
>> +feupdate_single_test (const char *flag_name, int fe_exc)
>> +{
>> + feenv_nomask_test (flag_name, fe_exc, feupdateenv);
>> + feenv_mask_test (flag_name, fe_exc, feupdateenv);
>> +}
>
> trapping is still on after this function.
>
>> +static void
>> +feupdateenv_single_test (const char *test_name, int fe_exc,
>> + fexcept_t exception)
>> +{
>> + char str[100];
>> + fenv_t env;
>> + int res;
>> +
>> + snprintf (str, sizeof str, "feupdateenv %s and FL_DFL_ENV", test_name);
>> + update_single_exc (str, FE_DFL_ENV, fe_exc, NO_EXC, exception);
>
> this relies on disabled trapping.
>
>> +static void
>> +feupdateenv_tests (void)
>> +{
> ...
>> +#ifdef FE_OVERFLOW
>> + feupdate_single_test ("FE_OVERFLOW", FE_OVERFLOW);
>> +#endif
>> +
>> +#ifdef FE_DIVBYZERO
>> + feupdateenv_single_test ("DIVBYZERO", DIVBYZERO_EXC, FE_DIVBYZERO);
>> +#endif
>
> this test fails because previous enables trapping mode.
>
> fwiw a feupdateenv (FE_DFL_ENV); in between the tests
> makes math/test-fenv pass.
I will take a look, thanks for the heads up.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 6/7] alpha: Fix fesetexceptflag (BZ 30998)
2023-11-03 12:24 [PATCH 0/7] Multiple floating-point environment fixes Adhemerval Zanella
` (4 preceding siblings ...)
2023-11-03 12:24 ` [PATCH 5/7] riscv: Fix feenvupdate with FE_DFL_ENV (BZ 31022) Adhemerval Zanella
@ 2023-11-03 12:24 ` Adhemerval Zanella
2023-11-03 12:24 ` [PATCH 7/7] hppa: Fix undefined behaviour in feclearexcept (BZ 30983) Adhemerval Zanella
2023-11-06 0:25 ` [PATCH 0/7] Multiple floating-point environment fixes Bruno Haible
7 siblings, 0 replies; 24+ messages in thread
From: Adhemerval Zanella @ 2023-11-03 12:24 UTC (permalink / raw)
To: libc-alpha, Bruno Haible
From: Bruno Haible <bruno@clisp.org>
It clears some exception flags that are outside the EXCEPTS argument.
It fixes math/test-fexcept on qemu-user.
---
sysdeps/alpha/fpu/fsetexcptflg.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sysdeps/alpha/fpu/fsetexcptflg.c b/sysdeps/alpha/fpu/fsetexcptflg.c
index 70f3666a6e..63eb06845d 100644
--- a/sysdeps/alpha/fpu/fsetexcptflg.c
+++ b/sysdeps/alpha/fpu/fsetexcptflg.c
@@ -27,7 +27,7 @@ __fesetexceptflag (const fexcept_t *flagp, int excepts)
tmp = __ieee_get_fp_control ();
/* Set all the bits that were called for. */
- tmp = (tmp & ~SWCR_STATUS_MASK) | (*flagp & excepts & SWCR_STATUS_MASK);
+ tmp ^= (tmp ^ *flagp) & excepts & SWCR_STATUS_MASK;
/* And store it back. */
__ieee_set_fp_control (tmp);
--
2.34.1
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH 7/7] hppa: Fix undefined behaviour in feclearexcept (BZ 30983)
2023-11-03 12:24 [PATCH 0/7] Multiple floating-point environment fixes Adhemerval Zanella
` (5 preceding siblings ...)
2023-11-03 12:24 ` [PATCH 6/7] alpha: Fix fesetexceptflag (BZ 30998) Adhemerval Zanella
@ 2023-11-03 12:24 ` Adhemerval Zanella
2023-11-06 0:25 ` [PATCH 0/7] Multiple floating-point environment fixes Bruno Haible
7 siblings, 0 replies; 24+ messages in thread
From: Adhemerval Zanella @ 2023-11-03 12:24 UTC (permalink / raw)
To: libc-alpha, Bruno Haible
From: Bruno Haible <bruno@clisp.org>
The expression
(excepts & FE_ALL_EXCEPT) << 27
produces a signed integer overflow when 'excepts' is specified as
FE_INVALID (= 0x10), because
- excepts is of type 'int',
- FE_ALL_EXCEPT is of type 'int',
- thus (excepts & FE_ALL_EXCEPT) is (int) 0x10,
- 'int' is 32 bits wide.
The patched code produces the same instruction sequence as
previosuly.
---
sysdeps/hppa/fpu/fclrexcpt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sysdeps/hppa/fpu/fclrexcpt.c b/sysdeps/hppa/fpu/fclrexcpt.c
index 055fb04ccc..46caf39ec1 100644
--- a/sysdeps/hppa/fpu/fclrexcpt.c
+++ b/sysdeps/hppa/fpu/fclrexcpt.c
@@ -26,7 +26,7 @@ feclearexcept (int excepts)
/* Get the current status word. */
__asm__ ("fstd %%fr0,0(%1)" : "=m" (s.l) : "r" (&s.l) : "%r0");
/* Clear all the relevant bits. */
- s.sw[0] &= ~((excepts & FE_ALL_EXCEPT) << 27);
+ s.sw[0] &= ~(((unsigned int) excepts & FE_ALL_EXCEPT) << 27);
__asm__ ("fldd 0(%0),%%fr0" : : "r" (&s.l), "m" (s.l) : "%r0");
/* Success. */
--
2.34.1
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 0/7] Multiple floating-point environment fixes
2023-11-03 12:24 [PATCH 0/7] Multiple floating-point environment fixes Adhemerval Zanella
` (6 preceding siblings ...)
2023-11-03 12:24 ` [PATCH 7/7] hppa: Fix undefined behaviour in feclearexcept (BZ 30983) Adhemerval Zanella
@ 2023-11-06 0:25 ` Bruno Haible
7 siblings, 0 replies; 24+ messages in thread
From: Bruno Haible @ 2023-11-06 0:25 UTC (permalink / raw)
To: libc-alpha, Adhemerval Zanella
Patches 1/7, 6/7, 7/7 look OK to me. Specific remarks on the other ones.
Bruno
^ permalink raw reply [flat|nested] 24+ messages in thread