misc/177624: Swapcontext can get compiled incorrectly

[Bruce Evans]

On Fri, 5 Apr 2013, Bruce Evans wrote:

> On Thu, 4 Apr 2013, Brian Demsky wrote:
>
>>> Description:
>> Here is the code for swap context:
>> 
>> int
>> swapcontext(ucontext_t *oucp, const ucontext_t *ucp)
>> {
>>      int ret;
>> 
>>      if ((oucp == NULL) || (ucp == NULL)) {
>>              errno = EINVAL;
>>              return (-1);
>>      }
>>      oucp->uc_flags &= ~UCF_SWAPPED;
>>      ret = getcontext(oucp);
>>      if ((ret == 0) && !(oucp->uc_flags & UCF_SWAPPED)) {
>>              oucp->uc_flags |= UCF_SWAPPED;
>>              ret = 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 <setcontext>
>> 
>> The problem is that rbx is callee saved by compiled version of swapcontext 
>> and then reused before getcontext is called.  Getcontext then stores the 
>> wrong value for rbx and setcontext later restores the wrong value for rbx. 
>> 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 
>> call to set context trashes the copies of bx and r14 on the stack….
>
> 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