[Bug math/30988] New: fesetexcept raises floating-point exception traps on ppc, ppc64, ppc64le

[Bug math/30988] New: fesetexcept raises floating-point exception traps on ppc, ppc64, ppc64le

[bruno at clisp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30988

            Bug ID: 30988
           Summary: fesetexcept raises floating-point exception traps on
                    ppc, ppc64, ppc64le
           Product: glibc
           Version: 2.37
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: math
          Assignee: unassigned at sourceware dot org
          Reporter: bruno at clisp dot org
  Target Milestone: ---

Created attachment 15187
  --> https://sourceware.org/bugzilla/attachment.cgi?id=15187&action=edit
test case foo.c

According to ISO C 23 § 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 function changes the state of the floating-point exception flags, but
does not cause any other side effects that might be associated with raising
floating-point exceptions. 267)
  Footnote 267) Implementation extensions like traps for floating-point
exceptions and IEC 60559 exception handling do not occur."

That's not how glibc's implementation does it on powerpc, powerpc64, and
ppc64le.

How to reproduce:
1. Compile the attached program foo.c.
$ gcc -Wall foo.c -lm
or for the 32-bit powerpc architecture:
$ gcc -m32 -Wall foo.c -lm

2. Run it.
$ ./a.out
Floating point exception

3. Run it under gdb.
$ gdb a.out 
(gdb) run
Starting program: /home/haible/a.out 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/powerpc64le-linux-gnu/libthread_db.so.1".

Program received signal SIGFPE, Arithmetic exception.
fesetexcept (excepts=536870912) at ../sysdeps/powerpc/fpu/fesetexcept.c:34
34      ../sysdeps/powerpc/fpu/fesetexcept.c: No such file or directory.
(gdb) x/i $pc
=> 0x7ffff7e21070 <fesetexcept+160>:    mtfsf   255,f0,1


I think it is a hardware limitation that on PowerPC CPUs, setting a
floating-point exception flag triggers a trap, when traps are enabled
for the particular exception and globally for the thread (via
prctl (PR_SET_FPEXC, PR_FP_EXC_PRECISE)).
See "PowerPC User Instruction Set Architecture" Book I Version 2.01
<https://math-atlas.sourceforge.net/devel/assembly/ppc_isa.pdf>, p. 90:
  "In this architecture, if software is to be notified that a
   given kind of exception has occurred, the corre-
   sponding FPSCR exception enable bit must be set to 1
   and a mode other than Ignore Exceptions Mode must
   be used.    In this case the system floating-point
   enabled exception error handler is invoked if an
   enabled floating-point exception occurs. The system
   floating-point enabled exception error handler is also
   invoked if a Move To FPSCR instruction causes an
   exception bit and the corresponding enable bit both to
   be 1; the Move To FPSCR instruction is considered to
   cause the enabled exception."

In this situation, I would expect the fesetexcept() function to return
a non-zero value (as error indicator) and not modify the FPSCR register.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug math/30988] fesetexcept raises floating-point exception traps on ppc, ppc64, ppc64le

[bruno at clisp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30988

Bruno Haible <bruno at clisp dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
               Host|                            |powerpc64-unknown-linux-gnu

--- Comment #1 from Bruno Haible <bruno at clisp dot org> ---
Seen on
- glibc 2.37 (gcc203.fsffrance.org, POWER8 CPU)
- glibc 2.36 (cfarm29.cfarm.net, POWER9 CPU)
- glibc 2.34 (gcc120.fsffrance.org, POWER10 CPU)

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug math/30988] fesetexcept raises floating-point exception traps on ppc, ppc64, ppc64le

[adhemerval.zanella at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30988

Adhemerval Zanella <adhemerval.zanella at linaro dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |adhemerval.zanella at linaro dot o
                   |                            |rg

--- Comment #2 from Adhemerval Zanella <adhemerval.zanella at linaro dot org> ---
Indeed, this seems to be an unexpected side-effect of how we implement the GNU
extension feenableexcept and returning an error seems the best option indeed.

Maybe something like:

diff --git a/sysdeps/powerpc/fpu/fesetexcept.c
b/sysdeps/powerpc/fpu/fesetexcept.c
index 609a148a95..e5396daf44 100644
--- a/sysdeps/powerpc/fpu/fesetexcept.c
+++ b/sysdeps/powerpc/fpu/fesetexcept.c
@@ -29,6 +29,10 @@ fesetexcept (int excepts)
         /* Turn FE_INVALID into FE_INVALID_SOFTWARE.  */
         | (excepts >> ((31 - FPSCR_VX) - (31 - FPSCR_VXSOFT))
            & FE_INVALID_SOFTWARE));
+
+  if (u.l & FPSCR_ENABLES_MASK)
+    return -1;
+
   if (n.l != u.l)
     {
       fesetenv_register (n.fenv);

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug math/30988] fesetexcept raises floating-point exception traps on ppc, ppc64, ppc64le

[bruno at clisp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30988

--- Comment #3 from Bruno Haible <bruno at clisp dot org> ---
(In reply to Adhemerval Zanella from comment #2)
> Indeed, this seems to be an unexpected side-effect of how we implement the
> GNU extension feenableexcept

There's no better way, IMO, to implement feenableexcept. The problem
really comes from the hardware.

> Maybe something like:
> 
> diff --git a/sysdeps/powerpc/fpu/fesetexcept.c
> b/sysdeps/powerpc/fpu/fesetexcept.c
> index 609a148a95..e5396daf44 100644
> --- a/sysdeps/powerpc/fpu/fesetexcept.c
> +++ b/sysdeps/powerpc/fpu/fesetexcept.c
> @@ -29,6 +29,10 @@ fesetexcept (int excepts)
>          /* Turn FE_INVALID into FE_INVALID_SOFTWARE.  */
>          | (excepts >> ((31 - FPSCR_VX) - (31 - FPSCR_VXSOFT))
>             & FE_INVALID_SOFTWARE));
> +
> +  if (u.l & FPSCR_ENABLES_MASK)
> +    return -1;
> +
>    if (n.l != u.l)
>      {
>        fesetenv_register (n.fenv);

This patch has the drawback of failing even in some situations that it could
handle. For example, if trapping on FE_DIVBYZERO is enabled and someone calls
fesetexcept (FE_INVALID), there is no reason to fail. I would therefore suggest
this patch instead:

diff --git a/sysdeps/powerpc/fpu/fesetexcept.c
b/sysdeps/powerpc/fpu/fesetexcept.c
index 609a148a95..5137df7ba1 100644
--- a/sysdeps/powerpc/fpu/fesetexcept.c
+++ b/sysdeps/powerpc/fpu/fesetexcept.c
@@ -31,6 +31,13 @@ fesetexcept (int excepts)
            & FE_INVALID_SOFTWARE));
   if (n.l != u.l)
     {
+      if (n.l & (exceptions >> FPSCR_EXCEPT_TO_ENABLE_SHIFT))
+        {
+          /* 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. 
*/

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug math/30988] fesetexcept raises floating-point exception traps on ppc, ppc64, ppc64le

[adhemerval.zanella at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30988

--- Comment #4 from Adhemerval Zanella <adhemerval.zanella at linaro dot org> ---
In fact, we are already aware of this limitation and the tests are masking off
this issue with the internal EXCEPTION_SET_FORCES_TRAP flag (set only for
powerpc). I am unsure why Joseph did not consider this a bug when he added the
tests. The fesetexceptflag has the same issue, so I think we will need
something like:

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)

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug math/30988] fesetexcept raises floating-point exception traps on ppc, ppc64, ppc64le

[bruno at clisp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30988

--- Comment #5 from Bruno Haible <bruno at clisp dot org> ---
(In reply to Adhemerval Zanella from comment #4)
> The fesetexceptflag has the same issue

Indeed, for fesetexceptflag ISO C § 7.6.4.5 has the same requirement.

Your patches look good.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug math/30988] fesetexcept raises floating-point exception traps on ppc, ppc64, ppc64le

[adhemerval.zanella at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30988

Adhemerval Zanella <adhemerval.zanella at linaro dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at sourceware dot org   |adhemerval.zanella at linaro dot o
                   |                            |rg
   Target Milestone|---                         |2.39
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |FIXED

--- Comment #6 from Adhemerval Zanella <adhemerval.zanella at linaro dot org> ---
Fixed on 2.39 (ecb1e7220ddc7a4845bbd1b6fd7fcf17aba566bd).

-- 
You are receiving this mail because:
You are on the CC list for the bug.