public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Bruno Haible <bruno@clisp.org>
To: libc-alpha@sourceware.org,
	Adhemerval Zanella <adhemerval.zanella@linaro.org>
Subject: Re: [PATCH v2 3/3] x86: Do not raises floating-point exception traps on fesetexceptflag (BZ 30990)
Date: Mon, 30 Oct 2023 16:22:49 +0100	[thread overview]
Message-ID: <4141177.K71DO8KEF6@nimes> (raw)
In-Reply-To: <20231024113716.3911015-4-adhemerval.zanella@linaro.org>

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

Hi Adhemerval,

Like in the PATCH v2 2/3, 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. 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: BZ30990-fnstenv-fix.diff --]
[-- Type: text/x-patch, Size: 1478 bytes --]

diff --git a/sysdeps/i386/fpu/fsetexcptflg.c b/sysdeps/i386/fpu/fsetexcptflg.c
index ccbcf35e8e..8bff9026a1 100644
--- a/sysdeps/i386/fpu/fsetexcptflg.c
+++ b/sysdeps/i386/fpu/fsetexcptflg.c
@@ -36,6 +36,8 @@ __fesetexceptflag (const fexcept_t *flagp, int excepts)
   /* Get the current x87 FPU environment.  We have to do this since we
      cannot separately set the status word.  */
   __asm__ ("fnstenv %0" : "=m" (*&temp));
+  /* Note: fnstenv masks all floating-point exceptions until the fldenv
+     or fldcw below.  */
 
   if (CPU_FEATURE_USABLE (SSE))
     {
@@ -62,10 +64,13 @@ __fesetexceptflag (const fexcept_t *flagp, int excepts)
       temp.__status_word ^= (temp.__status_word ^ *flagp) & excepts;
 
       if ((~temp.__control_word) & temp.__status_word & excepts)
-        /* Setting the exception flags may trigger a trap (at the next
-           floating-point instruction, but that does not matter).
-           ISO C 23 § 7.6.4.5 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 C 23 § 7.6.4.5 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));

      reply	other threads:[~2023-10-30 15:22 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-24 11:37 [PATCH v2 0/3] Fix fesetexcept/fesetexceptflag on powerpc and x86 Adhemerval Zanella
2023-10-24 11:37 ` [PATCH v2 1/3] powerpc: Do not raise exception traps for fesetexcept/fesetexceptflag (BZ 30988) Adhemerval Zanella
2023-10-24 11:37 ` [PATCH v2 2/3] i686: Do not raise exception traps on fesetexcept (BZ 30989) Adhemerval Zanella
2023-10-24 13:43   ` Bruno Haible
2023-10-26 18:39     ` Adhemerval Zanella Netto
2023-10-30 15:21   ` Bruno Haible
2023-10-30 16:05     ` Adhemerval Zanella Netto
2023-10-24 11:37 ` [PATCH v2 3/3] x86: Do not raises floating-point exception traps on fesetexceptflag (BZ 30990) Adhemerval Zanella
2023-10-30 15:22   ` Bruno Haible [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4141177.K71DO8KEF6@nimes \
    --to=bruno@clisp.org \
    --cc=adhemerval.zanella@linaro.org \
    --cc=libc-alpha@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).