Changeset 175653 in webkit


Ignore:
Timestamp:
Nov 5, 2014 5:53:25 PM (9 years ago)
Author:
mark.lam@apple.com
Message:

PutById inline caches should have a store barrier when it triggers a structure transition.
<https://webkit.org/b/138441>

Reviewed by Geoffrey Garen.

After r174025, we no longer insert DFG store barriers when the payload of a
PutById operation is not a cell. However, this can lead to a crash when we have
PutById inline cache code transitioning the structure and re-allocating the
butterfly of an old gen object. The lack of a store barrier in that inline
cache results in the old gen object not being noticed during an eden GC scan.
As a result, its newly allocated butterfly will not be kept alive, which leads
to a stale butterfly pointer and, eventually, a crash.

It is also possible that the new structure can be collected by the eden GC if
(at GC time):

  1. It is in the eden gen.
  2. The inline cache that installed it has been evicted.
  3. There are no live eden gen objects referring to it.

The chances of this should be more rare than the butterfly re-allocation, but
it is still possible. Hence, the fix is to always add a store barrier if the
inline caches performs a structure transition.

  • jit/Repatch.cpp:

(JSC::emitPutTransitionStub):

  • Added store barrier code based on SpeculativeJIT::storeToWriteBarrierBuffer()'s implementation.
Location:
trunk/Source/JavaScriptCore
Files:
2 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/JavaScriptCore/ChangeLog

    r175651 r175653  
     12014-11-05  Mark Lam  <mark.lam@apple.com>
     2
     3        PutById inline caches should have a store barrier when it triggers a structure transition.
     4        <https://webkit.org/b/138441>
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        After r174025, we no longer insert DFG store barriers when the payload of a
     9        PutById operation is not a cell.  However, this can lead to a crash when we have
     10        PutById inline cache code transitioning the structure and re-allocating the
     11        butterfly of an old gen object.  The lack of a store barrier in that inline
     12        cache results in the old gen object not being noticed during an eden GC scan.
     13        As a result, its newly allocated butterfly will not be kept alive, which leads
     14        to a stale butterfly pointer and, eventually, a crash.
     15
     16        It is also possible that the new structure can be collected by the eden GC if
     17        (at GC time):
     18        1. It is in the eden gen.
     19        2. The inline cache that installed it has been evicted.
     20        3. There are no live eden gen objects referring to it.
     21
     22        The chances of this should be more rare than the butterfly re-allocation, but
     23        it is still possible.  Hence, the fix is to always add a store barrier if the
     24        inline caches performs a structure transition.
     25
     26        * jit/Repatch.cpp:
     27        (JSC::emitPutTransitionStub):
     28        - Added store barrier code based on SpeculativeJIT::storeToWriteBarrierBuffer()'s
     29          implementation.
     30
    1312014-11-05  Gyuyoung Kim  <gyuyoung.kim@samsung.com>
    232
  • trunk/Source/JavaScriptCore/jit/Repatch.cpp

    r175365 r175653  
    11141114#endif
    11151115   
     1116    ScratchBuffer* scratchBuffer = nullptr;
     1117
     1118#if ENABLE(GGC)
     1119    MacroAssembler::Call callFlushWriteBarrierBuffer;
     1120    MacroAssembler::Jump ownerIsRememberedOrInEden = stubJit.jumpIfIsRememberedOrInEden(baseGPR);
     1121    {
     1122        WriteBarrierBuffer* writeBarrierBuffer = &stubJit.vm()->heap.writeBarrierBuffer();
     1123        stubJit.move(MacroAssembler::TrustedImmPtr(writeBarrierBuffer), scratchGPR1);
     1124        stubJit.load32(MacroAssembler::Address(scratchGPR1, WriteBarrierBuffer::currentIndexOffset()), scratchGPR2);
     1125        MacroAssembler::Jump needToFlush =
     1126            stubJit.branch32(MacroAssembler::AboveOrEqual, scratchGPR2, MacroAssembler::Address(scratchGPR1, WriteBarrierBuffer::capacityOffset()));
     1127
     1128        stubJit.add32(MacroAssembler::TrustedImm32(1), scratchGPR2);
     1129        stubJit.store32(scratchGPR2, MacroAssembler::Address(scratchGPR1, WriteBarrierBuffer::currentIndexOffset()));
     1130
     1131        stubJit.loadPtr(MacroAssembler::Address(scratchGPR1, WriteBarrierBuffer::bufferOffset()), scratchGPR1);
     1132        // We use an offset of -sizeof(void*) because we already added 1 to scratchGPR2.
     1133        stubJit.storePtr(baseGPR, MacroAssembler::BaseIndex(scratchGPR1, scratchGPR2, MacroAssembler::ScalePtr, static_cast<int32_t>(-sizeof(void*))));
     1134
     1135        MacroAssembler::Jump doneWithBarrier = stubJit.jump();
     1136        needToFlush.link(&stubJit);
     1137
     1138        scratchBuffer = vm->scratchBufferForSize(allocator.desiredScratchBufferSizeForCall());
     1139        allocator.preserveUsedRegistersToScratchBufferForCall(stubJit, scratchBuffer, scratchGPR2);
     1140        stubJit.setupArgumentsWithExecState(baseGPR);
     1141        callFlushWriteBarrierBuffer = stubJit.call();
     1142        allocator.restoreUsedRegistersFromScratchBufferForCall(stubJit, scratchBuffer, scratchGPR2);
     1143
     1144        doneWithBarrier.link(&stubJit);
     1145    }
     1146    ownerIsRememberedOrInEden.link(&stubJit);
     1147#endif
     1148
    11161149    MacroAssembler::Jump success;
    11171150    MacroAssembler::Jump failure;
     
    11341167       
    11351168        allocator.restoreReusedRegistersByPopping(stubJit);
    1136         ScratchBuffer* scratchBuffer = vm->scratchBufferForSize(allocator.desiredScratchBufferSizeForCall());
     1169        if (!scratchBuffer)
     1170            scratchBuffer = vm->scratchBufferForSize(allocator.desiredScratchBufferSizeForCall());
    11371171        allocator.preserveUsedRegistersToScratchBufferForCall(stubJit, scratchBuffer, scratchGPR1);
    11381172#if USE(JSVALUE64)
     
    11521186    else
    11531187        patchBuffer.link(failureCases, failureLabel);
     1188#if ENABLE(GGC)
     1189    patchBuffer.link(callFlushWriteBarrierBuffer, operationFlushWriteBarrierBuffer);
     1190#endif
    11541191    if (structure->outOfLineCapacity() != oldStructure->outOfLineCapacity()) {
    11551192        patchBuffer.link(operationCall, operationReallocateStorageAndFinishPut);
Note: See TracChangeset for help on using the changeset viewer.