[webkit-dev] stack alignment bug

[Zoltan Herczeg]

Hi Gavin,

the alignment error was not your fault. When you start porting the JIT,
you need to keep many things in your head, and I totally forgot about
stack alignment. The entry and exit functions are not portable, and you
have to arrange the stack frame by yourself for your architecture.

I am happy that pushes/pops are removed entirely from the code, but
perhaps the new inline functions should be moved to macro assembler level.
Pushes and pops are x86 helpers instructions, since x86-32 has only 8
general purpose registers. We have no idea when and how they are used
(especially not in the future), that is why I came up with the fake stack.
Of course it would be better to remove them. (And use the link register on
non-x86 machines)

By the way, could you take a look at our macro-assembler based ARM JIT
port (bug #24986)

Regards
Zoltan

> Hi Zoltan,
>
> I'm a little confused – maybe I'm misunderstanding you, but the JIT
> does just subtract a fixed amount from the stack pointer on entry (28
> on x86, for a total frame size including return address, caller frame
> pointer and callee preserved registers of 48, a multiple of 16 to
> preserve stack alignment).  All JIT code then runs at the same stack
> depth.  The only pops in the JIT are simply removing the the return
> address implicitly pushed on x86, and the only pushes (bar a function
> call in put property access transition realloc) are restoring the
> return address prior to a return (or a tail call).  It is not clear to
> me what you're envisaging 'fake_sp' would be used for.
>
> I've just landed a patch to move the pushes & pops in wrapper
> functions, and to switch put transition realloc to use a regular
> function call, hopefully this tidies things up a little.
>
> cheers,
> G.
>
>
> On Jun 4, 2009, at 1:19 AM, Zoltan Herczeg wrote:
>
>> Hi,
>>
>> actually there was a bug which took me a day to find out what
>> happened. It
>> was somewhere deep in libc, called by a function in DateMath.cpp. It
>> seemed that the stack was overwritten. By libc??? I can't belive it.
>> Finally I realized that gcc's alloca realigned the stack (to 8 bytes
>> on
>> ARM), so everything was in a wrong place (looked overwritten at first
>> sight).
>>
>> My fake stack pointer idea:
>> fake_sp: any non-volatile general purpose register
>>
>> JIT_entry:
>>  mov fake_sp, sp
>>  sub sp, sp, 32   ; I belive this is enough for the JIT,
>>                   ; correct me if I am wrong
>>  ; use fake_sp instead of sp for push/pops
>>
>> JIT_leava:
>>  add sp, sp, 32
>>
>> I hope this even works for PPC (if someone ever wants to port the
>> JIT to
>> old macs).
>>
>> Zoltan
>>
>>> Zoltan,
>>> I filed a bug here: https://bugs.webkit.org/show_bug.cgi?id=26164
>>> Stack is originally aligned then jit code destroys it; and, some data
>>> structure or point to double is not aligned and I'm still trying to
>>> find
>>> where they are.
>>> I'm not sure how the fake stack would be, would you mind explains a
>>> bit
>>> more?
>>> Did you face same problem?
>>> Thanks also for your articles that gives new ideas.
>>> rgds
>>> joe
>>>
>>> --- On Wed, 6/3/09, Zoltan Herczeg <zherczeg at inf.u-szeged.hu> wrote:
>>>
>>>> From: Zoltan Herczeg <zherczeg at inf.u-szeged.hu>
>>>> Subject: Re: [webkit-dev] stack alignment bug
>>>> To: "x yz" <lastguy at yahoo.com>
>>>> Cc: webkit-dev at lists.webkit.org
>>>> Date: Wednesday, June 3, 2009, 7:35 PM
>>>> Hi,
>>>>
>>>> true, some architectures have strict policies for stack
>>>> handling. Perhaps
>>>> the worst one is PowerPC with its organized stack frame
>>>> (back chains,
>>>> pre-defined register save areas, etc). I think a fake stack
>>>> pointer for
>>>> JIT can solve the x86 compatibility problem.
>>>>
>>>> 1) allocate enough aligned stack space for the worst case
>>>> when you enter
>>>> to JIT
>>>> 2) the fake stack pointer should use this pre-allocated
>>>> stack frame.
>>>>
>>>> Zoltan
>>>>
>>>>> I don't know how to file bug so I posted here.
>>>>> In privateCompileCTIMachineTrampolines() there are
>>>> multiple align() to
>>>>> align code on 16byte margin, yet, the stack can be put
>>>> on 32bit margin
>>>>> that causes crush.
>>>>> Suppose original stack is aligned to 8/16bytes, the
>>>> above function
>>>>> frequently pop/push regT3 that makes stack
>>>> mis-aligned. Then int to double
>>>>> conversion uses stack that will fail.
>>>>> rgds
>>>>> joe
>>
>>
>> _______________________________________________
>> webkit-dev mailing list
>> webkit-dev at lists.webkit.org
>> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>