Re: [PATCH v2 2/3] i686: Do not raise exception traps on fesetexcept (BZ 30989)

[Bruno Haible <bruno@clisp.org>]

[-- Attachment #1: Type: text/plain, Size: 717 bytes --]

Hi Adhemerval,

In the error case, the new code mistakenly masks all floating-point
exceptions (i.e. as if someone had called fedisableexcept (FE_ALL_EXCEPT)).
This is because the FNSTENV instruction is documented as
   "Saves the current FPU operating environment at the memory
    location specified with the destination operand, and then
    masks all floating-point exceptions."

The mistake came from my initial proposed fix of BZ 30990. Sorry about that.

Here's a proposed fix, on top of your patch. I've verified that the sequence
of instructions
    __asm__ ("fnstenv %0" : "=m" (*&temp));
    __asm__ volatile ("fldcw %0" : : "m" (*&temp.__control_word));
does restore the exceptions trapping bits.

Bruno


[-- Attachment #2: BZ30989-fnstenv-fix.diff --]
[-- Type: text/x-patch, Size: 1249 bytes --]

diff --git a/sysdeps/i386/fpu/fesetexcept.c b/sysdeps/i386/fpu/fesetexcept.c
index 6eeb5ab5b0..8c93264b0c 100644
--- a/sysdeps/i386/fpu/fesetexcept.c
+++ b/sysdeps/i386/fpu/fesetexcept.c
@@ -46,15 +46,20 @@ fesetexcept (int excepts)
       fenv_t temp;
 
       __asm__ ("fnstenv %0" : "=m" (*&temp));
+      /* Note: fnstenv masks all floating-point exceptions until the fldenv
+         or fldcw below.  */
 
       /* Clear or set relevant flags.  */
       temp.__status_word |= 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.  */
-        return -1;
+        {
+          /* 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));