The offsets into ucontext_t for fpregs, sigmask, fpregsmem, and mxcsr (as
specified in sysdeps/unix/sysv/linux/x86_64/ucontext_i.h) are wrong.

Fixing this could be a potential ABI change, although I don't see how anything
that ever used these fields could have possibly worked in the first place.

------- Additional Comment #1 From Nicholas Miell 2005-10-12 05:21 -------

Created an attachment (id=698)
correct the ucontext offsets

------- Additional Comment #2 From Nicholas Miell 2005-10-12 05:59 -------

Even with this patch, the the context functions still use the wrong offsets.

The problem is that FNSTENV and FLDENV operate on memory with the following layout:

struct fpenv
{
  uint16_t cwd;
  uint16_t __pad0;
  uint16_t swd;
  uint16_t __pad1;
  uint16_t ftw;
  uint16_t __pad2;
  uint32_t eip;
  uint16_t cs;
  uint16_t opcode;
  uint32_t edi;
  uint16_t ds;
  uint16_t __pad3
}; 

while struct _libc_fpstate uses the 64-bit FXSAVE/FXRSTOR format, which starts
like this:

struct _libc_fpstate
{
  __uint16_t cwd;
  __uint16_t swd;
  __uint16_t ftw;
  __uint16_t fop;
  __uint64_t rip;
  __uint64_t rdp;
  __uint32_t mxcsr;
  /* ... */
}

Which means that the existing code which does FNSTENV to uc.__fpregs_mem will
store members in the wrong locations and will corrupt the saved %mxcsr (assuming
the struct offsets in ucontext_i.h are corrected without any other updates to
the context functions).

------- Additional Comment #3 From Ulrich Drepper 2005-10-14 16:31 -------

Fix in the CVS trunk version.

------- Additional Comment #4 From Nicholas Miell 2005-10-15 03:27 -------

Sorry, I should have marked my patch as obsolete when I added comment #2.

Fortunately, I was wrong and the MXCSR register isn't actually corrupted --
STMXCSR occurs after FNSTENV, so the saved %ds and padding are overwritten by
the saved %mxcsr, but they're purely informational and don't effect execution
when they're loaded by FLDENV (afaik). (Meaning no new bugs were introduced, I
think.)

I was just going to submit a patch to setcontext, getcontext and swapcontext
that makes them use FXSAVE and FXRSTOR instead of FNSTENV/STMXCSR and
FLDENV/LDMXCSR --  it turns out that saving and restoring the entire FPU state
with these intructions is faster than just saving/restoring the x87 environment
and MXCSR register and FXSAVE/FXRSTOR would naturally put things in the right
place in struct _libc_fpstate.

However, I ran into a problem that I don't know how to solve:

The FXSAVE area needs to be 16-byte aligned, however the natural alignment of
struct _libc_fpstate is 8. As such, the __fpregs_mem member of struct ucontext
isn't aligned sufficiently for use with the FXSAVE and FXRSTOR instructions.

_libc_fpstate should probably get an __attribute__((aligned(16))), but this
would change the layout of struct ucontext and definately break the ABI.

------- Additional Comment #5 From Ulrich Drepper 2005-10-16 08:23 -------

There is no reason to reopen the bug.  It's fixed, and no changes to the data
types can be made.