[Bug math/16064] New: x86 fenv_t doesn't include SSE state

[Bug math/16064] New: x86 fenv_t doesn't include SSE state

[jsm28 at gcc dot gnu.org]

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

            Bug ID: 16064
           Summary: x86 fenv_t doesn't include SSE state
           Product: glibc
           Version: 2.18
            Status: NEW
          Severity: normal
          Priority: P2
         Component: math
          Assignee: unassigned at sourceware dot org
          Reporter: jsm28 at gcc dot gnu.org
              Host: i?86-*-*

The fenv_t type in sysdeps/x86/fpu/bits/fenv.h only includes the SSE MXCSR for
x86_64, not for 32-bit x86.  Since various fenv.h functions *do* use SSE,
runtime-conditionally on it being supported, this means peculiarities such as
feholdexcept/feupdateenv pairs clearing SSE exception state and trap enablement
in feholdexcept, but not restoring it in feupdateenv because the old MXCSR
state wasn't saved.

Fixing this, thereby increasing the size of fenv_t, will require new symbol
versions for fegetenv / fesetenv / feholdexcept / feupdateenv on x86 (with the
compat versions of the functions continuing to save/restore only the x87 parts
of the state).

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

[Bug math/16064] x86 fenv_t doesn't include SSE state

[bugdal at aerifal dot cx]

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

Rich Felker <bugdal at aerifal dot cx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bugdal at aerifal dot cx

--- Comment #1 from Rich Felker <bugdal at aerifal dot cx> ---
No, fixing it is much easier. The best way, in my opinion, is simply to merge
the SSE exception flags into the x87 registers when saving the context, and
clear them when restoring. However, if I'm not mistaken glibc already solves
this problem in a different way, reusing one of the "unimportant" fields in the
x87 structure (EIP of last fault, I believe) to store the SSE state. This of
course works too.

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

[Bug math/16064] x86 fenv_t doesn't include SSE state

[joseph at codesourcery dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=16064

--- Comment #2 from joseph at codesourcery dot com <joseph at codesourcery dot com> ---
The EIP hack is only used in the internal fenv inlines used within libm 
functions that need to change and restore state themselves, not in the 
public interface implementation.

The EIP hack would seem reasonable for avoiding new symbol versions.  
Merging SSE state into x87 state seems a less good idea; as a 
quality-of-implementation matter, fenv_t should include 
architecture-specific floating-point status flags and control modes, and 
at least the FZ and DAZ bits of MXCSR don't correspond to x87 state.

(I've filed bug 16068 for failure of fesetenv to handle some of the 
architecture-specific state as it should.)

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

[Bug math/16064] x86 fenv_t doesn't include SSE state

[bugdal at aerifal dot cx]

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

--- Comment #3 from Rich Felker <bugdal at aerifal dot cx> ---
I'm fairly indifferent on which solution is used (among the two options that
don't require a new symbol version), but I disagree that merging the states is
bad from a QoI standpoint. The saved floating point environment needs only
preserve the state visible to a correct application, and separate x87 and SSE
states are not visible (the functions to set the modes set both, and the
functions to read exception flags already read and merge both). In my mind, the
only reason the x87 hardware structure is mimicked in fenv is for the sake of
simplifying/optimizing the code to save/restore state. Otherwise, fenv_t could
just be an arch-independent structure containing the rounding mode and
exception flags.

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

[Bug math/16064] [i386] x86 fenv_t doesn't include SSE state

[jsm28 at gcc dot gnu.org]

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

Joseph Myers <jsm28 at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|x86 fenv_t doesn't include  |[i386] x86 fenv_t doesn't
                   |SSE state                   |include SSE state

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

[Bug math/16064] [i386] x86 fenv_t doesn't include SSE state

[jsm28 at gcc dot gnu.org]

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

Joseph Myers <jsm28 at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #5 from Joseph Myers <jsm28 at gcc dot gnu.org> ---
Fixed for 2.20.

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

[Bug math/16064] [i386] x86 fenv_t doesn't include SSE state

[cvs-commit at gcc dot gnu.org]

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

--- Comment #4 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, master has been updated
       via  77d08acaadfb197cd03c56ac8f72baabeb305f25 (commit)
      from  28162f4dac274a4b635d95fc2442bef888df1d57 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=77d08acaadfb197cd03c56ac8f72baabeb305f25

commit 77d08acaadfb197cd03c56ac8f72baabeb305f25
Author: Joseph Myers <joseph@codesourcery.com>
Date:   Fri May 9 16:59:56 2014 +0000

    Include SSE state in i386 fenv_t (bug 16064).

    This patch fixes bug 16064, i386 fenv_t not including SSE state, using
    the technique suggested there of storing the state in the existing
    __eip field of fenv_t to avoid needing to increase the size of fenv_t
    and add new symbol versions.  The included testcase, which previously
    failed for i386 (but passed for x86_64), illustrates how the previous
    state was buggy.

    This patch causes the SSE state to be included *to the extent it is on
    x86_64*.  Where some state should logically be included but isn't for
    x86_64 (see bug 16068), this patch does not cause it to be included
    for i386 either.  The idea is that any patch fixing that bug should
    fix it for both x86_64 and i386 at once.

    Tested i386 and x86_64.  (I haven't tested the case of a CPU without
    SSE2 disabling the test.)

        [BZ #16064]
        * sysdeps/i386/fpu/fegetenv.c: Include <unistd.h>, <ldsodefs.h>
        and <dl-procinfo.h>.
        (__fegetenv): Save SSE state in envp->__eip if supported.
        * sysdeps/i386/fpu/feholdexcpt.c (feholdexcept): Save SSE state in
        envp->__eip if supported.
        * sysdeps/i386/fpu/fesetenv.c: Include <unistd.h>, <ldsodefs.h>
        and <dl-procinfo.h>.
        (__fesetenv): Always set __eip, __cs_selector, __opcode,
        __data_offset and __data_selector in environment to 0.  Set SSE
        state if supported.
        * sysdeps/x86/fpu/Makefile [$(subdir) = math] (tests): Add
        test-fenv-sse.
        [$(subdir) = math] (CFLAGS-test-fenv-sse.c): Add -msse2
        -mfpmath=sse.
        * sysdeps/x86/fpu/test-fenv-sse.c: New file.

-----------------------------------------------------------------------

Summary of changes:
 ChangeLog                       |   19 +++++
 NEWS                            |    4 +-
 sysdeps/i386/fpu/fegetenv.c     |    6 ++
 sysdeps/i386/fpu/feholdexcpt.c  |    4 +-
 sysdeps/i386/fpu/fesetenv.c     |   50 ++++++++++----
 sysdeps/x86/fpu/Makefile        |    2 +
 sysdeps/x86/fpu/test-fenv-sse.c |  138 +++++++++++++++++++++++++++++++++++++++
 7 files changed, 204 insertions(+), 19 deletions(-)
 create mode 100644 sysdeps/x86/fpu/test-fenv-sse.c

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

[Bug math/16064] [i386] x86 fenv_t doesn't include SSE state

[fweimer at redhat dot com]

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

Florian Weimer <fweimer at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |security-

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

[Bug math/16064] [i386] x86 fenv_t doesn't include SSE state

[jsm28 at gcc dot gnu.org]

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

Joseph Myers <jsm28 at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |simonbyrne at gmail dot com

--- Comment #7 from Joseph Myers <jsm28 at gcc dot gnu.org> ---
*** Bug 17907 has been marked as a duplicate of this bug. ***

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