Date: Sun, 07 Apr 2013 10:41:29 +0800 From: David Xu <davidxu@freebsd.org> To: Bruce Evans <brde@optusnet.com.au> Cc: freebsd-gnats-submit@FreeBSD.org, freebsd-bugs@FreeBSD.org, Brian Demsky <bdemsky@uci.edu> Subject: Re: misc/177624: Swapcontext can get compiled incorrectly Message-ID: <5160DCD9.5070503@freebsd.org> In-Reply-To: <20130405044424.Y2557@besplex.bde.org> References: <201304040232.r342WFTC020054@red.freebsd.org> <20130404232206.S1025@besplex.bde.org> <20130405011027.Y1350@besplex.bde.org> <CF93B187-2005-46EA-B581-2FB10EFDD55E@uci.edu> <20130405044424.Y2557@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 2013/04/05 03:38, Bruce Evans wrote:
> On Thu, 4 Apr 2013, Brian Demsky wrote:
>
>> On Apr 4, 2013, at 8:16 AM, Bruce Evans wrote:
>>
>>> 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
>>
>> Take at setcontext:
>>
>> (gdb) disassemble setcontext
>> Dump of assembler code for function setcontext:
>> 0x00007fff90262855 <setcontext+0>: push %rbx
>> 0x00007fff90262856 <setcontext+1>: lea 0x38(%rdi),%rbx
>> 0x00007fff9026285a <setcontext+5>: cmp 0x30(%rdi),%rbx
>> 0x00007fff9026285e <setcontext+9>: je 0x7fff90262864
>> <setcontext+15>
>> 0x00007fff90262860 <setcontext+11>: mov %rbx,0x30(%rdi)
>> 0x00007fff90262864 <setcontext+15>: mov 0x4(%rdi),%edi
>> 0x00007fff90262867 <setcontext+18>: callq 0x7fff90262998
>> <sigsetmask>
>> 0x00007fff9026286c <setcontext+23>: mov %rbx,%rdi
>> 0x00007fff9026286f <setcontext+26>: pop %rbx
>> 0x00007fff90262870 <setcontext+27>: jmpq 0x7fff90262875
>> <_setcontext>
>> End of assembler dump.
>>
>> The stack from swapcontext had rbx and r14 popped after getcontext
>> stored everything. Now we push rbx and then later call sigsetmask.
>> Those two actions guarantee that the memory locations where rbx and
>> r14 were on the stack have been overwritten. When we later return to
>> the save context, it will start up swapcontext and pop the wrong
>> values off of the stack for rbx and r14.
>
> Ah, it is not really rbx and r14, but rsp and the whole stack frame of
> swapcontext() that are mishandled. Even returning from swapcontext()
> leaves the saved rsp pointing to garbage. The stack frame could have
> had almost anything on it before it became invalid, but here it has mainly
> the saved rbx and r14 (not rbp; however, when compiled by clang on FreeBSD,
> it also has the saved rbp, and when compiled with -O0 it also has the
> local variable).
>
> Now I think swapcontext() can't be written in C, for the same reason that
> setjmp() can't be written in C -- the stack frame cannot be controlled in
> C, and if it has anything at all on it (even the return address requires
> special handling), then the stack pointer saved in the context becomes
> invalid when the function returns, or even earlier for tail calls and
> other optimizations.
This reminds me that I can not override swapcontext in libthr, I had
put a wrapper for swapcontext in libthr, I am considering to remove it
now ...
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?5160DCD9.5070503>