[v2] LoongArch: fix missing trap for enabled exceptions on narrowing operation

  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>
---

Change from v1:
- Fix a leftover from earlier debugging in the test case, which caused
  test failures on 32-bit LoongArch and ARM.
- Simplify the test using glibc test framework.

 math/Makefile                    |  3 ++
 math/test-narrowing-trap.c       | 60 ++++++++++++++++++++++++++++++++
 sysdeps/loongarch/fenv_private.h | 22 ++++++------
 sysdeps/loongarch/fpu_control.h  |  3 ++
 4 files changed, 77 insertions(+), 11 deletions(-)
 create mode 100644 math/test-narrowing-trap.c
  

LGTM

在 2026/3/20 下午5:12, 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>
> ---
>
> Change from v1:
> - Fix a leftover from earlier debugging in the test case, which caused
>    test failures on 32-bit LoongArch and ARM.
> - Simplify the test using glibc test framework.
>
>   math/Makefile                    |  3 ++
>   math/test-narrowing-trap.c       | 60 ++++++++++++++++++++++++++++++++
>   sysdeps/loongarch/fenv_private.h | 22 ++++++------
>   sysdeps/loongarch/fpu_control.h  |  3 ++
>   4 files changed, 77 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..27c31245c1
> --- /dev/null
> +++ b/math/test-narrowing-trap.c
> @@ -0,0 +1,60 @@
> +/* 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 <fenv.h>
> +#include <math.h>
> +#include <math-tests.h>
> +#include <signal.h>
> +#include <stdlib.h>
> +#include <support/check.h>
> +#include <support/xunistd.h>
> +
> +static int
> +do_test (void)
> +{
> +  pid_t pid;
> +
> +  if (!EXCEPTION_ENABLE_SUPPORTED (FE_INVALID))
> +    FAIL_UNSUPPORTED ("feenableexcept (FE_INVALID) not supported");
> +
> +  pid = xfork ();
> +  if (pid == 0)
> +    {
> +      int r = feenableexcept (FE_INVALID);
> +      if (r == -1)
> +	exit (2);
> +
> +      fdiv (0.0, 0.0);
> +      exit (0);
> +    }
> +  else
> +    {
> +      int status;
> +      xwaitpid (pid, &status, 0);
> +
> +      TEST_VERIFY (WIFSIGNALED (status));
> +      TEST_COMPARE (WTERMSIG (status), SIGFPE);
> +    }
> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
> 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))
>
  

* Xi Ruoyao:

> diff --git a/math/test-narrowing-trap.c b/math/test-narrowing-trap.c
> new file mode 100644
> index 0000000000..27c31245c1
> --- /dev/null
> +++ b/math/test-narrowing-trap.c
> @@ -0,0 +1,60 @@
> +/* 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

We automatically define _GNU_SOURCE for tests, so that's unnecessary.

> +#include <fenv.h>
> +#include <math.h>
> +#include <math-tests.h>
> +#include <signal.h>
> +#include <stdlib.h>
> +#include <support/check.h>
> +#include <support/xunistd.h>
> +
> +static int
> +do_test (void)
> +{
> +  pid_t pid;
> +
> +  if (!EXCEPTION_ENABLE_SUPPORTED (FE_INVALID))
> +    FAIL_UNSUPPORTED ("feenableexcept (FE_INVALID) not supported");
> +
> +  pid = xfork ();
> +  if (pid == 0)
> +    {
> +      int r = feenableexcept (FE_INVALID);
> +      if (r == -1)
> +	exit (2);

Tabs vs spaces inconsistency, I think.  And you could use
TEST_VERIFY (r != -1).  The test framework uses a shared memory
segment, so that subtest failures after fork are communicated
outwards even if the exit status is zero.

> +      fdiv (0.0, 0.0);
> +      exit (0);

This should be _exit (0), so that atexit handlers are not run twice.
(We have protection against this internally, but I think test
code should show how to use fork correctly if possible.)

Sorry, should have mentioned that before.

Thanks,
Florian
  

在 2026/3/20 下午6:50, Florian Weimer 写道:
> * Xi Ruoyao:
>
>> diff --git a/math/test-narrowing-trap.c b/math/test-narrowing-trap.c
>> new file mode 100644
>> index 0000000000..27c31245c1
>> --- /dev/null
>> +++ b/math/test-narrowing-trap.c
>> @@ -0,0 +1,60 @@
>> +/* 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
> We automatically define _GNU_SOURCE for tests, so that's unnecessary.
>
>> +#include <fenv.h>
>> +#include <math.h>
>> +#include <math-tests.h>
>> +#include <signal.h>
>> +#include <stdlib.h>
>> +#include <support/check.h>
>> +#include <support/xunistd.h>
>> +
>> +static int
>> +do_test (void)
>> +{
>> +  pid_t pid;
>> +
>> +  if (!EXCEPTION_ENABLE_SUPPORTED (FE_INVALID))
>> +    FAIL_UNSUPPORTED ("feenableexcept (FE_INVALID) not supported");
>> +
>> +  pid = xfork ();
>> +  if (pid == 0)
>> +    {
>> +      int r = feenableexcept (FE_INVALID);
>> +      if (r == -1)
>> +	exit (2);
> Tabs vs spaces inconsistency, I think.  And you could use
> TEST_VERIFY (r != -1).  The test framework uses a shared memory
> segment, so that subtest failures after fork are communicated
> outwards even if the exit status is zero.
>
>> +      fdiv (0.0, 0.0);
>> +      exit (0);
> This should be _exit (0), so that atexit handlers are not run twice.
> (We have protection against this internally, but I think test
> code should show how to use fork correctly if possible.)
>
> Sorry, should have mentioned that before.

Gentle ping.


>
> Thanks,
> Florian