misc/177624: Swapcontext can get compiled incorrectly

[Bruce Evans]

The following reply was made to PR misc/177624; it has been noted by GNATS.

From: Bruce Evans <brde at optusnet.com.au>
To: Bruce Evans <brde at optusnet.com.au>
Cc: Brian Demsky <bdemsky at uci.edu>, freebsd-bugs at freebsd.org,
        freebsd-gnats-submit at freebsd.org
Subject: Re: misc/177624: Swapcontext can get compiled incorrectly
Date: Fri, 5 Apr 2013 02:16:02 +1100 (EST)

   This message is in MIME format.  The first part should be readable text,
   while the remaining parts are likely unreadable without MIME-aware tools.
 
 --0-1588918552-1365088562=:1350
 Content-Type: TEXT/PLAIN; charset=X-UNKNOWN; format=flowed
 Content-Transfer-Encoding: QUOTED-PRINTABLE
 
 On Fri, 5 Apr 2013, Bruce Evans wrote:
 
 > On Thu, 4 Apr 2013, Brian Demsky wrote:
 >
 >>> Description:
 >> Here is the code for swap context:
 >>=20
 >> int
 >> swapcontext(ucontext_t *oucp, const ucontext_t *ucp)
 >> {
 >>      int ret;
 >>=20
 >>      if ((oucp =3D=3D NULL) || (ucp =3D=3D NULL)) {
 >>              errno =3D EINVAL;
 >>              return (-1);
 >>      }
 >>      oucp->uc_flags &=3D ~UCF_SWAPPED;
 >>      ret =3D getcontext(oucp);
 >>      if ((ret =3D=3D 0) && !(oucp->uc_flags & UCF_SWAPPED)) {
 >>              oucp->uc_flags |=3D UCF_SWAPPED;
 >>              ret =3D setcontext(ucp);
 >>      }
 >>      return (ret);
 >> }
 >
 >> On the OS X port of libc in Mac OSX 10.7.5, this gets compiled as:
 >
 >> ...
 >> 0x00007fff901e870b <swapcontext+89>:    pop    %rbx
 >> 0x00007fff901e870c <swapcontext+90>:    pop    %r14
 >> 0x00007fff901e870e <swapcontext+92>:    jmpq   0x7fff90262855 <setcontex=
 t>
 >>=20
 >> The problem is that rbx is callee saved by compiled version of swapconte=
 xt=20
 >> and then reused before getcontext is called.  Getcontext then stores the=
 =20
 >> wrong value for rbx and setcontext later restores the wrong value for rb=
 x.=20
 >> If the caller had any value in rbx, it has been trashed at this point.
 >
 > Later you wrote:
 >
 >> The analysis is a little wrong about the problem.  Ultimately, the tail=
 =20
 >> call to set context trashes the copies of bx and r14 on the stack=85.
 >
 > The bug seems to be in setcontext().  It must preserve the callee-saved
 > registers, not restore them.  This would happen automatically if more
 > were written in C.  But setcontext() can't be written entirely in C,
 > since it must save all callee-saved registers including ones not used
 > and therefore not normally saved by any C function that it might be in,
 > and possibly also including callee-saved registers for nonstandard or
 > non-C ABIs.  In FreeBSD, it is apparently always a syscall.
 
 This is more than a little wrong.  When setcontext() succeeds, it
 doesn't return here.  Then it acts like longjmp() and must restore all
 the callee-saved to whatever they were when getcontext() was called.
 Otherwise, it must not clobber any callee-saved registers (then it
 differs from longjmp().  longjmp() just can't fail).
 
 Now I don't see any bug here.  If the saved state is returned to, then
 it is as if getcontext() returned, and the intermediately-saved %rbx
 is correct (we will restore the orginal %rbx if we return).  If
 setcontext() fails, then it should preserve all callee-saved registers.
 In the tail-call case, we have already restored the orginal %rbx and
 the failing setcontext() should preserve that.
 
 Bruce
 --0-1588918552-1365088562=:1350--