Changeset 158583 in webkit


Ignore:
Timestamp:
Nov 4, 2013 11:52:04 AM (10 years ago)
Author:
mhahnenberg@apple.com
Message:

JSArrayBufferViews of length 0 allocate 0 CopiedSpace bytes, which is invalid
https://bugs.webkit.org/show_bug.cgi?id=123746

Reviewed by Geoffrey Garen.

Source/JavaScriptCore:

This patch disallows clients from allocating 0 bytes in CopiedSpace. We enforce this invariant
with an ASSERT in C++ code and a breakpoint in JIT code. Clients who care about 0-byte
allocations (like JSArrayBufferViews) must handle that case themselves, but we don't punish
anybody else for the rare case that somebody decides to allocate a 0-length typed array.
It also makes the allocation and copying cases consistent for CopiedSpace: no 0-byte allocations,
no 0-byte copying.

Also added a check so that JSArrayBufferViews don't try to copy their m_vector backing store when
their length is 0. Also sprinkled several ASSERTs throughout the JSArrayBufferView code to make sure that
when length is 0 m_vector is null.

  • dfg/DFGSpeculativeJIT.cpp:

(JSC::DFG::SpeculativeJIT::compileNewTypedArray):

  • dfg/DFGSpeculativeJIT.h:

(JSC::DFG::SpeculativeJIT::emitAllocateBasicStorage):

  • heap/CopiedSpaceInlines.h:

(JSC::CopiedSpace::tryAllocate):

  • runtime/ArrayBuffer.h:

(JSC::ArrayBuffer::create):

  • runtime/JSArrayBufferView.cpp:

(JSC::JSArrayBufferView::ConstructionContext::ConstructionContext):

  • runtime/JSGenericTypedArrayViewInlines.h:

(JSC::::visitChildren):
(JSC::::copyBackingStore):
(JSC::::slowDownAndWasteMemory):

LayoutTests:

Added a test to make sure that we don't crash when allocating a typed array with 0 length.

  • js/script-tests/typedarray-zero-size.js: Added.

(foo):

  • js/typedarray-zero-size-expected.txt: Added.
  • js/typedarray-zero-size.html: Added.
Location:
trunk
Files:
3 added
8 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r158582 r158583  
     12013-11-04  Mark Hahnenberg  <mhahnenberg@apple.com>
     2
     3        JSArrayBufferViews of length 0 allocate 0 CopiedSpace bytes, which is invalid
     4        https://bugs.webkit.org/show_bug.cgi?id=123746
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        Added a test to make sure that we don't crash when allocating a typed array with 0 length.
     9
     10        * js/script-tests/typedarray-zero-size.js: Added.
     11        (foo):
     12        * js/typedarray-zero-size-expected.txt: Added.
     13        * js/typedarray-zero-size.html: Added.
     14
    1152013-11-04  Alexey Proskuryakov  <ap@apple.com>
    216
  • trunk/Source/JavaScriptCore/ChangeLog

    r158580 r158583  
     12013-11-04  Mark Hahnenberg  <mhahnenberg@apple.com>
     2
     3        JSArrayBufferViews of length 0 allocate 0 CopiedSpace bytes, which is invalid
     4        https://bugs.webkit.org/show_bug.cgi?id=123746
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        This patch disallows clients from allocating 0 bytes in CopiedSpace. We enforce this invariant
     9        with an ASSERT in C++ code and a breakpoint in JIT code. Clients who care about 0-byte
     10        allocations (like JSArrayBufferViews) must handle that case themselves, but we don't punish
     11        anybody else for the rare case that somebody decides to allocate a 0-length typed array.
     12        It also makes the allocation and copying cases consistent for CopiedSpace: no 0-byte allocations,
     13        no 0-byte copying.
     14 
     15        Also added a check so that JSArrayBufferViews don't try to copy their m_vector backing store when
     16        their length is 0. Also sprinkled several ASSERTs throughout the JSArrayBufferView code to make sure that
     17        when length is 0 m_vector is null.
     18
     19        * dfg/DFGSpeculativeJIT.cpp:
     20        (JSC::DFG::SpeculativeJIT::compileNewTypedArray):
     21        * dfg/DFGSpeculativeJIT.h:
     22        (JSC::DFG::SpeculativeJIT::emitAllocateBasicStorage):
     23        * heap/CopiedSpaceInlines.h:
     24        (JSC::CopiedSpace::tryAllocate):
     25        * runtime/ArrayBuffer.h:
     26        (JSC::ArrayBuffer::create):
     27        * runtime/JSArrayBufferView.cpp:
     28        (JSC::JSArrayBufferView::ConstructionContext::ConstructionContext):
     29        * runtime/JSGenericTypedArrayViewInlines.h:
     30        (JSC::::visitChildren):
     31        (JSC::::copyBackingStore):
     32        (JSC::::slowDownAndWasteMemory):
     33
    1342013-11-04  Julien Brianceau  <jbriance@cisco.com>
    235
  • trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

    r158304 r158583  
    47104710    slowCases.append(m_jit.branch32(
    47114711        MacroAssembler::Above, sizeGPR, TrustedImm32(JSArrayBufferView::fastSizeLimit)));
     4712    slowCases.append(m_jit.branchTest32(MacroAssembler::Zero, sizeGPR));
    47124713   
    47134714    m_jit.move(sizeGPR, scratchGPR);
  • trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h

    r158384 r158583  
    20532053    {
    20542054        CopiedAllocator* copiedAllocator = &m_jit.vm()->heap.storageAllocator();
    2055        
     2055
     2056        // It's invalid to allocate zero bytes in CopiedSpace.
     2057#ifndef NDEBUG
     2058        m_jit.move(size, resultGPR);
     2059        MacroAssembler::Jump nonZeroSize = m_jit.branchTest32(MacroAssembler::NonZero, resultGPR);
     2060        m_jit.breakpoint();
     2061        nonZeroSize.link(&m_jit);
     2062#endif
     2063
    20562064        m_jit.loadPtr(&copiedAllocator->m_currentRemaining, resultGPR);
    20572065        MacroAssembler::Jump slowPath = m_jit.branchSubPtr(JITCompiler::Signed, size, resultGPR);
  • trunk/Source/JavaScriptCore/heap/CopiedSpaceInlines.h

    r153169 r158583  
    151151{
    152152    ASSERT(!m_heap->vm()->isInitializingObject());
     153    ASSERT(bytes);
    153154
    154155    if (!m_allocator.tryAllocate(bytes, outPtr))
  • trunk/Source/JavaScriptCore/runtime/ArrayBuffer.h

    r154305 r158583  
    161161        return 0;
    162162    RefPtr<ArrayBuffer> buffer = adoptRef(new ArrayBuffer(contents));
     163    ASSERT(!byteLength || source);
    163164    memcpy(buffer->data(), source, byteLength);
    164165    return buffer.release();
  • trunk/Source/JavaScriptCore/runtime/JSArrayBufferView.cpp

    r154422 r158583  
    4646    if (length <= fastSizeLimit) {
    4747        // Attempt GC allocation.
    48         void* temp;
     48        void* temp = 0;
    4949        size_t size = sizeOf(length, elementSize);
    50         if (!vm.heap.tryAllocateStorage(0, size, &temp))
     50        // CopiedSpace only allows non-zero size allocations.
     51        if (size && !vm.heap.tryAllocateStorage(0, size, &temp))
    5152            return;
    5253
  • trunk/Source/JavaScriptCore/runtime/JSGenericTypedArrayViewInlines.h

    r156964 r158583  
    442442    switch (thisObject->m_mode) {
    443443    case FastTypedArray: {
    444         visitor.copyLater(thisObject, TypedArrayVectorCopyToken, thisObject->m_vector, thisObject->byteSize());
     444        if (thisObject->m_vector)
     445            visitor.copyLater(thisObject, TypedArrayVectorCopyToken, thisObject->m_vector, thisObject->byteSize());
    445446        break;
    446447    }
     
    470471    if (token == TypedArrayVectorCopyToken
    471472        && visitor.checkIfShouldCopy(thisObject->m_vector)) {
     473        ASSERT(thisObject->m_vector);
    472474        void* oldVector = thisObject->m_vector;
    473475        void* newVector = visitor.allocateNewSpace(thisObject->byteSize());
     
    506508    if (thisObject->m_mode == FastTypedArray
    507509        && !thisObject->m_butterfly && size >= sizeof(IndexingHeader)) {
     510        ASSERT(thisObject->m_vector);
    508511        // Reuse already allocated memory if at all possible.
    509512        thisObject->m_butterfly =
Note: See TracChangeset for help on using the changeset viewer.