[Bug libc/1468] New: AMD64 {get,set,swap,make}context use wrong offsets into ucontext_t

[Bug libc/1468] New: AMD64 {get,set,swap,make}context use wrong offsets into ucontext_t

[nmiell at comcast dot net]

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.

-- 
           Summary: AMD64 {get,set,swap,make}context use wrong offsets into
                    ucontext_t
           Product: glibc
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: libc
        AssignedTo: gotom at debian dot or dot jp
        ReportedBy: nmiell at comcast dot net
                CC: glibc-bugs at sources dot redhat dot com


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

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/1468] AMD64 {get,set,swap,make}context use wrong offsets into ucontext_t

[nmiell at comcast dot net]

------- Additional Comments From nmiell at comcast dot net  2005-10-12 05:21 -------
Created an attachment (id=698)
 --> (http://sourceware.org/bugzilla/attachment.cgi?id=698&action=view)
correct the ucontext offsets


-- 


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

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/1468] AMD64 {get,set,swap,make}context use wrong offsets into ucontext_t

[nmiell at comcast dot net]

------- Additional Comments From nmiell at comcast dot net  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).

-- 


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

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/1468] AMD64 {get,set,swap,make}context use wrong offsets into ucontext_t

[drepper at redhat dot com]

------- Additional Comments From drepper at redhat dot com  2005-10-14 16:31 -------
Fix in the CVS trunk version.

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


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

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/1468] AMD64 {get,set,swap,make}context use wrong offsets into ucontext_t

[nmiell at comcast dot net]

------- Additional Comments From nmiell at comcast dot net  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.

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


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

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/1468] AMD64 {get,set,swap,make}context use wrong offsets into ucontext_t

[drepper at redhat dot com]

------- Additional Comments From drepper at redhat dot com  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.

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


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

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.