[webkit-dev] stack alignment bug

Gavin Barraclough barraclough at apple.com
Thu Jun 4 13:58:18 PDT 2009 

On Jun 4, 2009, at 5:49 AM, Zoltan Herczeg wrote:

> 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 design the assembler is intended to be fully independent of the JS  
language JIT, and of JS specific types generally – such that it  
remains a useful general purpose code generation tool that can be used  
by other code generation engines (i.e. YARR).  Ideally code in  
'assembler' would only rely on code in WTF (presently the executable  
allocation classes are in the JS-JIT which breaks this, these should  
probably move into the assembler at some point, or access to them  
should be abstracted).

This is probably not functionality we'd want want to see in the macro  
assembler, since it is very JS-JIT specific – removing the return  
address from the x86 stack is highly unconventional, and something we  
only do in the JS JIT presently because we are effectively maintaining  
a second stack (the RegisterFile).  Presently there are already many  
examples of platform differences being abstracted in JITInlineMethods,  
this is a key part of its job.  At some point in the future it may  
become necessary to split platform specific abstractions into their  
own classes/files within the JIT.

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

When you have a patch ready for review, please attach it to the bug  
and set the review flag.

We tend to prefer to break down larger changes into smaller steps,  
ideally each of which introduces an independent, complete and coherent  
change (i.e. actually do something, and be testable, rather than just  
breaking down patches into fragments for the sake of it).  In the case  
of porting the JIT, I would suggest that a sensible first step is  
likely to submit a patch that just enables YARR_JIT (we have always  
brought up the regex engine first in the past).  This would give an  
opportunity for a reviewer to inspect the new assembler code with  
minimal changes to existing code (very little code needs to be changed  
in YARR to bring up a new platform).

cheers,
G.


> 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
>>
>

More information about the webkit-dev mailing list