LoongArch: fix missing trap for enabled exceptions on narrowing operation
Checks
| Context |
Check |
Description |
| redhat-pt-bot/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
| redhat-pt-bot/TryBot-32bit |
success
|
Build for i686
|
| linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_glibc_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_glibc_check--master-arm |
fail
|
Test failed
|
| linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 |
fail
|
Test failed
|
Commit Message
The libc_feupdateenv_test macro is supposed to trap when the trap for a
previously held exception is enabled. But
libc_feupdateenv_test_loongarch wasn't doing it properly: the comment
claims "setting of the cause bits" would cause "the hardware to generate
the exception" but that's simply not true for the LoongArch movgr2fcsr
instruction.
To fix the issue, we need to call __feraiseexcept in case a held exception
is enabled to trap.
Signed-off-by: Xi Ruoyao <xry111@xry111.site>
---
math/Makefile | 3 ++
math/test-narrowing-trap.c | 90 ++++++++++++++++++++++++++++++++
sysdeps/loongarch/fenv_private.h | 22 ++++----
sysdeps/loongarch/fpu_control.h | 3 ++
4 files changed, 107 insertions(+), 11 deletions(-)
create mode 100644 math/test-narrowing-trap.c
Comments
On Thu, 2026-03-19 at 16:31 +0800, Xi Ruoyao wrote:
/* snip */
> +#include <errno.h>
> +#include <fenv.h>
> +#include <math.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <sys/wait.h>
> +#include <sys/resource.h>
> +#include <unistd.h>
> +
> +#define EXCEPTION_ENABLE_SUPPORTED(...) 1
> +#define math_force_eval(...) ((void)0)
Oops, those two lines are left overs from the draft version. Please
disregard them.
I'll send v2 after hearing reviews from others.
* Xi Ruoyao:
> diff --git a/math/test-narrowing-trap.c b/math/test-narrowing-trap.c
> new file mode 100644
> index 0000000000..110ac15f62
> + if (!EXCEPTION_ENABLE_SUPPORTED (FE_INVALID))
> + {
> + puts ("feenableexcept (FE_INVALID) not supported, cannot test.");
> + return 77;
> + }
You can use FAIL_UNSUPPORTED from <support/check.h>.
> + pid = fork ();
> + if (pid == 0)
> + {
> +#ifdef RLIMIT_CORE
> + /* Try to avoid dumping core. */
> + struct rlimit core_limit = {};
> + setrlimit (RLIMIT_CORE, &core_limit);
> +#endif
The test framework (<support/test-driver.c>) deals with that.
We also have error-checking variants xfork, xwaitpid, and TEST_VERIFY
and TEST_COMPARE for checking specific conditions.
Thanks,
Florian
On Thu, 2026-03-19 at 10:14 +0100, Florian Weimer wrote:
> * Xi Ruoyao:
>
> > diff --git a/math/test-narrowing-trap.c b/math/test-narrowing-trap.c
> > new file mode 100644
> > index 0000000000..110ac15f62
>
> > + if (!EXCEPTION_ENABLE_SUPPORTED (FE_INVALID))
> > + {
> > + puts ("feenableexcept (FE_INVALID) not supported, cannot test.");
> > + return 77;
> > + }
>
> You can use FAIL_UNSUPPORTED from <support/check.h>.
>
> > + pid = fork ();
> > + if (pid == 0)
> > + {
> > +#ifdef RLIMIT_CORE
> > + /* Try to avoid dumping core. */
> > + struct rlimit core_limit = {};
> > + setrlimit (RLIMIT_CORE, &core_limit);
> > +#endif
>
> The test framework (<support/test-driver.c>) deals with that.
>
> We also have error-checking variants xfork, xwaitpid, and TEST_VERIFY
> and TEST_COMPARE for checking specific conditions.
test-fenv.c uses fork() and check if it returns ENOSYS (if so it skips
the test). Is doing so really necessary or may I just use xfork()
instead?
* Xi Ruoyao:
> On Thu, 2026-03-19 at 10:14 +0100, Florian Weimer wrote:
>> * Xi Ruoyao:
>>
>> > diff --git a/math/test-narrowing-trap.c b/math/test-narrowing-trap.c
>> > new file mode 100644
>> > index 0000000000..110ac15f62
>>
>> > + if (!EXCEPTION_ENABLE_SUPPORTED (FE_INVALID))
>> > + {
>> > + puts ("feenableexcept (FE_INVALID) not supported, cannot test.");
>> > + return 77;
>> > + }
>>
>> You can use FAIL_UNSUPPORTED from <support/check.h>.
>>
>> > + pid = fork ();
>> > + if (pid == 0)
>> > + {
>> > +#ifdef RLIMIT_CORE
>> > + /* Try to avoid dumping core. */
>> > + struct rlimit core_limit = {};
>> > + setrlimit (RLIMIT_CORE, &core_limit);
>> > +#endif
>>
>> The test framework (<support/test-driver.c>) deals with that.
>>
>> We also have error-checking variants xfork, xwaitpid, and TEST_VERIFY
>> and TEST_COMPARE for checking specific conditions.
>
> test-fenv.c uses fork() and check if it returns ENOSYS (if so it skips
> the test). Is doing so really necessary or may I just use xfork()
> instead?
No, this is just a very, very old test. We don't support testing glibc
without a working fork.
Thanks,
Florian
在 2026/3/19 下午4:31, Xi Ruoyao 写道:
> The libc_feupdateenv_test macro is supposed to trap when the trap for a
> previously held exception is enabled. But
> libc_feupdateenv_test_loongarch wasn't doing it properly: the comment
> claims "setting of the cause bits" would cause "the hardware to generate
> the exception" but that's simply not true for the LoongArch movgr2fcsr
> instruction.
>
> To fix the issue, we need to call __feraiseexcept in case a held exception
> is enabled to trap.
>
> Signed-off-by: Xi Ruoyao <xry111@xry111.site>
> ---
> math/Makefile | 3 ++
> math/test-narrowing-trap.c | 90 ++++++++++++++++++++++++++++++++
> sysdeps/loongarch/fenv_private.h | 22 ++++----
> sysdeps/loongarch/fpu_control.h | 3 ++
> 4 files changed, 107 insertions(+), 11 deletions(-)
> create mode 100644 math/test-narrowing-trap.c
>
> diff --git a/math/Makefile b/math/Makefile
> index 2eb0085de9..7a9352c2cd 100644
> --- a/math/Makefile
> +++ b/math/Makefile
> @@ -563,6 +563,7 @@ tests = \
> test-nan-overflow \
> test-nan-payload \
> test-narrow-macros \
> + test-narrowing-trap \
> test-nearbyint-except \
> test-nearbyint-except-2 \
> test-powl \
> @@ -1172,6 +1173,8 @@ CFLAGS-test-ceil-except-2.c += -fno-builtin
> CFLAGS-test-floor-except-2.c += -fno-builtin
> CFLAGS-test-trunc-except-2.c += -fno-builtin
>
> +CFLAGS-test-narrowing-trap.c += -fno-builtin
> +
> include ../Rules
>
> gen-all-calls = $(gen-libm-calls) $(gen-calls)
> diff --git a/math/test-narrowing-trap.c b/math/test-narrowing-trap.c
> new file mode 100644
> index 0000000000..110ac15f62
> --- /dev/null
> +++ b/math/test-narrowing-trap.c
HI, this test failed on loongarch64-linux-gnusf.
> @@ -0,0 +1,90 @@
> +/* Copyright (C) 2026 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 _GNU_SOURCE
> +# define _GNU_SOURCE
> +#endif
> +
> +#include <errno.h>
> +#include <fenv.h>
> +#include <math.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <sys/wait.h>
> +#include <sys/resource.h>
> +#include <unistd.h>
> +
> +#define EXCEPTION_ENABLE_SUPPORTED(...) 1
> +#define math_force_eval(...) ((void)0)
> +
> +int
> +main (void)
> +{
> + pid_t pid;
> +
> + if (!EXCEPTION_ENABLE_SUPPORTED (FE_INVALID))
> + {
> + puts ("feenableexcept (FE_INVALID) not supported, cannot test.");
> + return 77;
> + }
> +
> + pid = fork ();
> + if (pid == 0)
> + {
> +#ifdef RLIMIT_CORE
> + /* Try to avoid dumping core. */
> + struct rlimit core_limit = {};
> + setrlimit (RLIMIT_CORE, &core_limit);
> +#endif
> +
> + int r = feenableexcept (FE_INVALID);
> + if (r == -1)
> + exit (2);
> +
> + fdiv (0.0, 0.0);
> + exit (0);
> + }
> + else if (pid < 0)
> + {
> + if (errno == ENOSYS)
> + {
> + puts ("fork not implemented, test skipped");
> + exit (77);
> + }
> +
> + puts ("fail: fork failed");
> + exit (1);
> + }
> + else
> + {
> + int status;
> + if (waitpid (pid, &status, 0) != pid)
> + {
> + puts ("fail: waitpid failed");
> + exit (1);
> + }
> +
> + if (WIFSIGNALED (status) && WTERMSIG (status) == SIGFPE)
> + {
> + puts ("pass: process received SIGFPE");
> + exit (0);
> + }
> +
> + printf ("fail: process exited with status %d\n", status);
> + exit (1);
> + }
> +}
> diff --git a/sysdeps/loongarch/fenv_private.h b/sysdeps/loongarch/fenv_private.h
> index 7b49d82527..f782447fa2 100644
> --- a/sysdeps/loongarch/fenv_private.h
> +++ b/sysdeps/loongarch/fenv_private.h
> @@ -115,20 +115,20 @@ static __always_inline int
> libc_feupdateenv_test_loongarch (fenv_t *envp, int excepts)
> {
> /* int ret = fetestexcept (excepts); feupdateenv (envp); return ret; */
> - int cw, temp;
> + int held_ex, cw = envp->__fp_control_register;
>
> - /* Get current control word. */
> - _FPU_GETCW (cw);
> + /* Get the current flag, i.e. all exceptions raised since we started
> + to hold the exceptions. We don't care CAUSE. */
> + _FPU_GET_FLAGS_CAUSE (held_ex);
>
> - /* Set flag bits (which are accumulative), and *also* set the
> - cause bits. The setting of the cause bits is what actually causes
> - the hardware to generate the exception, if the corresponding enable
> - bit is set as well. */
> - temp = cw & FE_ALL_EXCEPT;
> - temp |= envp->__fp_control_register | (temp << CAUSE_SHIFT);
> + /* Set flag bits (which are accumulative). */
> + cw |= held_ex;
> + _FPU_SETCW (cw);
>
> - /* Set new state. */
> - _FPU_SETCW (temp);
> + /* Raise SIGFPE for any new exceptions since the hold, in case any is
> + enabled. */
> + if (__glibc_unlikely (((cw & ENABLE_MASK) << ENABLE_SHIFT) & held_ex))
> + __feraiseexcept (held_ex);
>
> return cw & excepts & FE_ALL_EXCEPT;
> }
> diff --git a/sysdeps/loongarch/fpu_control.h b/sysdeps/loongarch/fpu_control.h
> index 95976fa192..4d9dd52fca 100644
> --- a/sysdeps/loongarch/fpu_control.h
> +++ b/sysdeps/loongarch/fpu_control.h
> @@ -97,6 +97,9 @@ extern void __loongarch_fpu_setcw (fpu_control_t) __THROW;
> #define _FPU_GET_ENABLES(cw) __asm__ volatile ("movfcsr2gr %0,$fcsr1" : "=r"(cw))
> #define _FPU_SET_ENABLES(cw) __asm__ volatile ("movgr2fcsr $fcsr1,%0" : : "r"(cw))
>
> +#define _FPU_GET_FLAGS_CAUSE(cw) __asm__ volatile ("movfcsr2gr %0,$fcsr2" : "=r"(cw))
> +#define _FPU_SET_FLAGS_CAUSE(cw) __asm__ volatile ("movgr2fcsr $fcsr2,%0" : : "r"(cw))
> +
> #define _FPU_GET_RM(cw) __asm__ volatile ("movfcsr2gr %0,$fcsr3" : "=r"(cw))
> #define _FPU_SET_RM(cw) __asm__ volatile ("movgr2fcsr $fcsr3,%0" : : "r"(cw))
>
On Fri, 2026-03-20 at 09:25 +0800, caiyinyu wrote:
> > diff --git a/math/test-narrowing-trap.c b/math/test-narrowing-trap.c
> > new file mode 100644
> > index 0000000000..110ac15f62
> > --- /dev/null
> > +++ b/math/test-narrowing-trap.c
>
>
> HI, this test failed on loongarch64-linux-gnusf.
It's because the "#define EXCEPTION_ENABLE_SUPPORTED(...) 1" line that I
should have removed before submission. I'll remove it in v2 as I've
mentioned.
That line also causes failures on Linaro CI (ARM and AArch64).
@@ -563,6 +563,7 @@ tests = \
test-nan-overflow \
test-nan-payload \
test-narrow-macros \
+ test-narrowing-trap \
test-nearbyint-except \
test-nearbyint-except-2 \
test-powl \
@@ -1172,6 +1173,8 @@ CFLAGS-test-ceil-except-2.c += -fno-builtin
CFLAGS-test-floor-except-2.c += -fno-builtin
CFLAGS-test-trunc-except-2.c += -fno-builtin
+CFLAGS-test-narrowing-trap.c += -fno-builtin
+
include ../Rules
gen-all-calls = $(gen-libm-calls) $(gen-calls)
new file mode 100644
@@ -0,0 +1,90 @@
+/* Copyright (C) 2026 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 _GNU_SOURCE
+# define _GNU_SOURCE
+#endif
+
+#include <errno.h>
+#include <fenv.h>
+#include <math.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/wait.h>
+#include <sys/resource.h>
+#include <unistd.h>
+
+#define EXCEPTION_ENABLE_SUPPORTED(...) 1
+#define math_force_eval(...) ((void)0)
+
+int
+main (void)
+{
+ pid_t pid;
+
+ if (!EXCEPTION_ENABLE_SUPPORTED (FE_INVALID))
+ {
+ puts ("feenableexcept (FE_INVALID) not supported, cannot test.");
+ return 77;
+ }
+
+ pid = fork ();
+ if (pid == 0)
+ {
+#ifdef RLIMIT_CORE
+ /* Try to avoid dumping core. */
+ struct rlimit core_limit = {};
+ setrlimit (RLIMIT_CORE, &core_limit);
+#endif
+
+ int r = feenableexcept (FE_INVALID);
+ if (r == -1)
+ exit (2);
+
+ fdiv (0.0, 0.0);
+ exit (0);
+ }
+ else if (pid < 0)
+ {
+ if (errno == ENOSYS)
+ {
+ puts ("fork not implemented, test skipped");
+ exit (77);
+ }
+
+ puts ("fail: fork failed");
+ exit (1);
+ }
+ else
+ {
+ int status;
+ if (waitpid (pid, &status, 0) != pid)
+ {
+ puts ("fail: waitpid failed");
+ exit (1);
+ }
+
+ if (WIFSIGNALED (status) && WTERMSIG (status) == SIGFPE)
+ {
+ puts ("pass: process received SIGFPE");
+ exit (0);
+ }
+
+ printf ("fail: process exited with status %d\n", status);
+ exit (1);
+ }
+}
@@ -115,20 +115,20 @@ static __always_inline int
libc_feupdateenv_test_loongarch (fenv_t *envp, int excepts)
{
/* int ret = fetestexcept (excepts); feupdateenv (envp); return ret; */
- int cw, temp;
+ int held_ex, cw = envp->__fp_control_register;
- /* Get current control word. */
- _FPU_GETCW (cw);
+ /* Get the current flag, i.e. all exceptions raised since we started
+ to hold the exceptions. We don't care CAUSE. */
+ _FPU_GET_FLAGS_CAUSE (held_ex);
- /* Set flag bits (which are accumulative), and *also* set the
- cause bits. The setting of the cause bits is what actually causes
- the hardware to generate the exception, if the corresponding enable
- bit is set as well. */
- temp = cw & FE_ALL_EXCEPT;
- temp |= envp->__fp_control_register | (temp << CAUSE_SHIFT);
+ /* Set flag bits (which are accumulative). */
+ cw |= held_ex;
+ _FPU_SETCW (cw);
- /* Set new state. */
- _FPU_SETCW (temp);
+ /* Raise SIGFPE for any new exceptions since the hold, in case any is
+ enabled. */
+ if (__glibc_unlikely (((cw & ENABLE_MASK) << ENABLE_SHIFT) & held_ex))
+ __feraiseexcept (held_ex);
return cw & excepts & FE_ALL_EXCEPT;
}
@@ -97,6 +97,9 @@ extern void __loongarch_fpu_setcw (fpu_control_t) __THROW;
#define _FPU_GET_ENABLES(cw) __asm__ volatile ("movfcsr2gr %0,$fcsr1" : "=r"(cw))
#define _FPU_SET_ENABLES(cw) __asm__ volatile ("movgr2fcsr $fcsr1,%0" : : "r"(cw))
+#define _FPU_GET_FLAGS_CAUSE(cw) __asm__ volatile ("movfcsr2gr %0,$fcsr2" : "=r"(cw))
+#define _FPU_SET_FLAGS_CAUSE(cw) __asm__ volatile ("movgr2fcsr $fcsr2,%0" : : "r"(cw))
+
#define _FPU_GET_RM(cw) __asm__ volatile ("movfcsr2gr %0,$fcsr3" : "=r"(cw))
#define _FPU_SET_RM(cw) __asm__ volatile ("movgr2fcsr $fcsr3,%0" : : "r"(cw))