Created attachment 15187 [details] 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.
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)
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);
(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. */
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)
(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.
Fixed on 2.39 (ecb1e7220ddc7a4845bbd1b6fd7fcf17aba566bd).