Changeset 189103 in webkit


Ignore:
Timestamp:
Aug 28, 2015, 10:52:35 AM (10 years ago)
Author:
mark.lam@apple.com
Message:

ScratchRegisterAllocator::preserveReusedRegistersByPushing() should allow room for C helper calls and keep sp properly aligned.
​https://bugs.webkit.org/show_bug.cgi?id=148564

Reviewed by Saam Barati.

ScratchRegisterAllocator::preserveReusedRegistersByPushing() pushes registers on
the stack in order to preserve them. But emitPutTransitionStub() (which uses
preserveReusedRegistersByPushing()) may also emit a call to a C helper function
to flush the heap write barrier buffer. The code for emitting a C helper call
expects the stack pointer (sp) to already be pointing to a location on the stack
where there's adequate space reserved for storing the arguments to the C helper,
and that space is expected to be at the top of the stack. Hence, there is a
conflict of expectations. As a result, the arguments for the C helper will
overwrite and corrupt the values that are pushed on the stack by
preserveReusedRegistersByPushing().

In addition, JIT compiled functions always position the sp such that it will be
aligned (according to platform ABI dictates) after a C call is made (i.e. after
the frame pointer and return address is pushed on to the stack).
preserveReusedRegistersByPushing()'s arbitrary pushing of a number of saved
register values may mess up this alignment.

The fix is to have preserveReusedRegistersByPushing(), after it has pushed the
saved register values, adjust the sp to reserve an additional amount of stack
space needed for C call helpers plus any padding needed to restore proper sp
alignment. The stack's ReservedZone will ensure that we have enough stack space
for this. ScratchRegisterAllocator::restoreReusedRegistersByPopping() also
needs to be updated to perform the complement of this behavior.

  • jit/Repatch.cpp:

(JSC::emitPutReplaceStub):
(JSC::emitPutTransitionStub):

  • jit/ScratchRegisterAllocator.cpp:

(JSC::ScratchRegisterAllocator::allocateScratchGPR):
(JSC::ScratchRegisterAllocator::allocateScratchFPR):
(JSC::ScratchRegisterAllocator::preserveReusedRegistersByPushing):
(JSC::ScratchRegisterAllocator::restoreReusedRegistersByPopping):

  • jit/ScratchRegisterAllocator.h:

(JSC::ScratchRegisterAllocator::numberOfReusedRegisters):

  • tests/stress/regress-148564.js: Added.

(test):
(runTest):

Location:
trunk/Source/JavaScriptCore
Files:
1 added
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r189091 r189103  
     12015-08-28  Mark Lam  <mark.lam@apple.com>
     2
     3        ScratchRegisterAllocator::preserveReusedRegistersByPushing() should allow room for C helper calls and keep sp properly aligned.
     4        https://bugs.webkit.org/show_bug.cgi?id=148564
     5
     6        Reviewed by Saam Barati.
     7
     8        ScratchRegisterAllocator::preserveReusedRegistersByPushing() pushes registers on
     9        the stack in order to preserve them.  But emitPutTransitionStub() (which uses
     10        preserveReusedRegistersByPushing()) may also emit a call to a C helper function
     11        to flush the heap write barrier buffer.  The code for emitting a C helper call
     12        expects the stack pointer (sp) to already be pointing to a location on the stack
     13        where there's adequate space reserved for storing the arguments to the C helper,
     14        and that space is expected to be at the top of the stack.  Hence, there is a
     15        conflict of expectations.  As a result, the arguments for the C helper will
     16        overwrite and corrupt the values that are pushed on the stack by
     17        preserveReusedRegistersByPushing().
     18
     19        In addition, JIT compiled functions always position the sp such that it will be
     20        aligned (according to platform ABI dictates) after a C call is made (i.e. after
     21        the frame pointer and return address is pushed on to the stack).
     22        preserveReusedRegistersByPushing()'s arbitrary pushing of a number of saved
     23        register values may mess up this alignment.
     24
     25        The fix is to have preserveReusedRegistersByPushing(), after it has pushed the
     26        saved register values, adjust the sp to reserve an additional amount of stack
     27        space needed for C call helpers plus any padding needed to restore proper sp
     28        alignment.  The stack's ReservedZone will ensure that we have enough stack space
     29        for this.  ScratchRegisterAllocator::restoreReusedRegistersByPopping() also
     30        needs to be updated to perform the complement of this behavior.
     31
     32        * jit/Repatch.cpp:
     33        (JSC::emitPutReplaceStub):
     34        (JSC::emitPutTransitionStub):
     35        * jit/ScratchRegisterAllocator.cpp:
     36        (JSC::ScratchRegisterAllocator::allocateScratchGPR):
     37        (JSC::ScratchRegisterAllocator::allocateScratchFPR):
     38        (JSC::ScratchRegisterAllocator::preserveReusedRegistersByPushing):
     39        (JSC::ScratchRegisterAllocator::restoreReusedRegistersByPopping):
     40        * jit/ScratchRegisterAllocator.h:
     41        (JSC::ScratchRegisterAllocator::numberOfReusedRegisters):
     42        * tests/stress/regress-148564.js: Added.
     43        (test):
     44        (runTest):
     45
    1462015-08-28  Csaba Osztrogonác  <ossy@webkit.org>
    247

  • trunk/Source/JavaScriptCore/jit/Repatch.cpp

    r189082 r189103  
    917917    CCallHelpers stubJit(vm, exec->codeBlock());
    918918
    919     allocator.preserveReusedRegistersByPushing(stubJit);
     919    size_t numberOfPaddingBytes = allocator.preserveReusedRegistersByPushing(stubJit);
    920920
    921921    MacroAssembler::Jump badStructure = branchStructure(stubJit,
    … …  
    946946   
    947947    if (allocator.didReuseRegisters()) {
    948         allocator.restoreReusedRegistersByPopping(stubJit);
     948        allocator.restoreReusedRegistersByPopping(stubJit, numberOfPaddingBytes);
    949949        success = stubJit.jump();
    950950       
    951951        badStructure.link(&stubJit);
    952         allocator.restoreReusedRegistersByPopping(stubJit);
     952        allocator.restoreReusedRegistersByPopping(stubJit, numberOfPaddingBytes);
    953953        failure = stubJit.jump();
    954954    } else {
    … …  
    10541054        scratchGPR3 = InvalidGPRReg;
    10551055   
    1056     allocator.preserveReusedRegistersByPushing(stubJit);
     1056    size_t numberOfPaddingBytes = allocator.preserveReusedRegistersByPushing(stubJit);
    10571057
    10581058    MacroAssembler::JumpList failureCases;
    … …  
    11701170           
    11711171    if (allocator.didReuseRegisters()) {
    1172         allocator.restoreReusedRegistersByPopping(stubJit);
     1172        allocator.restoreReusedRegistersByPopping(stubJit, numberOfPaddingBytes);
    11731173        success = stubJit.jump();
    11741174
    11751175        failureCases.link(&stubJit);
    1176         allocator.restoreReusedRegistersByPopping(stubJit);
     1176        allocator.restoreReusedRegistersByPopping(stubJit, numberOfPaddingBytes);
    11771177        failure = stubJit.jump();
    11781178    } else
    … …  
    11851185        slowPath.link(&stubJit);
    11861186       
    1187         allocator.restoreReusedRegistersByPopping(stubJit);
     1187        allocator.restoreReusedRegistersByPopping(stubJit, numberOfPaddingBytes);
    11881188        if (!scratchBuffer)
    11891189            scratchBuffer = vm->scratchBufferForSize(allocator.desiredScratchBufferSizeForCall());

  • trunk/Source/JavaScriptCore/jit/ScratchRegisterAllocator.cpp

    r167187 r189103  
    3030
    3131#include "JSCInlines.h"
     32#include "MaxFrameExtentForSlowPathCall.h"
    3233#include "VM.h"
    3334
    … …  
    9293FPRReg ScratchRegisterAllocator::allocateScratchFPR() { return allocateScratch<FPRInfo>(); }
    9394
    94 void ScratchRegisterAllocator::preserveReusedRegistersByPushing(MacroAssembler& jit)
     95size_t ScratchRegisterAllocator::preserveReusedRegistersByPushing(MacroAssembler& jit)
    9596{
    9697    if (!didReuseRegisters())
    97         return;
    98        
     98        return 0;
     99
     100    size_t numberOfBytesPushed = 0;
     101
    99102    for (unsigned i = 0; i < FPRInfo::numberOfRegisters; ++i) {
    100103        FPRReg reg = FPRInfo::toRegister(i);
    101         if (m_scratchRegisters.getFPRByIndex(i) && m_usedRegisters.get(reg))
     104        if (m_scratchRegisters.getFPRByIndex(i) && m_usedRegisters.get(reg)) {
    102105            jit.pushToSave(reg);
     106            numberOfBytesPushed += sizeof(double);
     107        }
    103108    }
    104109    for (unsigned i = 0; i < GPRInfo::numberOfRegisters; ++i) {
    105110        GPRReg reg = GPRInfo::toRegister(i);
    106         if (m_scratchRegisters.getGPRByIndex(i) && m_usedRegisters.get(reg))
     111        if (m_scratchRegisters.getGPRByIndex(i) && m_usedRegisters.get(reg)) {
    107112            jit.pushToSave(reg);
    108     }
    109 }
    110 
    111 void ScratchRegisterAllocator::restoreReusedRegistersByPopping(MacroAssembler& jit)
     113            numberOfBytesPushed += sizeof(uintptr_t);
     114        }
     115    }
     116
     117    size_t totalStackAdjustmentBytes = numberOfBytesPushed + maxFrameExtentForSlowPathCall;
     118    totalStackAdjustmentBytes = WTF::roundUpToMultipleOf(stackAlignmentBytes(), totalStackAdjustmentBytes);
     119
     120    size_t numberOfPaddingBytes = totalStackAdjustmentBytes - numberOfBytesPushed;
     121    jit.subPtr(MacroAssembler::TrustedImm32(numberOfPaddingBytes), MacroAssembler::stackPointerRegister);
     122
     123    return numberOfPaddingBytes;
     124}
     125
     126void ScratchRegisterAllocator::restoreReusedRegistersByPopping(MacroAssembler& jit, size_t numberOfPaddingBytes)
    112127{
    113128    if (!didReuseRegisters())
    114129        return;
    115        
     130
     131    jit.addPtr(MacroAssembler::TrustedImm32(numberOfPaddingBytes), MacroAssembler::stackPointerRegister);
     132
    116133    for (unsigned i = GPRInfo::numberOfRegisters; i--;) {
    117134        GPRReg reg = GPRInfo::toRegister(i);

  • trunk/Source/JavaScriptCore/jit/ScratchRegisterAllocator.h

    r165414 r189103  
    6363    }
    6464   
    65     void preserveReusedRegistersByPushing(MacroAssembler& jit);
    66     void restoreReusedRegistersByPopping(MacroAssembler& jit);
     65    // preserveReusedRegistersByPushing() returns the number of padding bytes used to keep the stack
     66    // pointer properly aligned and to reserve room for calling a C helper. This number of padding
     67    // bytes must be provided to restoreReusedRegistersByPopping() in order to reverse the work done
     68    // by preserveReusedRegistersByPushing().
     69    size_t preserveReusedRegistersByPushing(MacroAssembler& jit);
     70    void restoreReusedRegistersByPopping(MacroAssembler& jit, size_t numberOfPaddingBytes);
    6771   
    6872    RegisterSet usedRegistersForCall() const;
Note: See TracChangeset for help on using the changeset viewer.