Ignore:
Timestamp:
Dec 14, 2017, 11:11:49 AM (8 years ago)
Author:
[email protected]
Message:

JSObjects should have a mask for loading indexed properties
​https://bugs.webkit.org/show_bug.cgi?id=180768

Reviewed by Mark Lam.

JSTests:

  • stress/int16-put-by-val-in-and-out-of-bounds.js:

(test):

Source/JavaScriptCore:

This patch adds a new member to JSObject that holds an indexing
mask. The indexing mask is bitwise anded with the index used to
load a property. If for whatever reason an attacker is able to
clobber the vectorLength of our butterfly they still won't be able
to read substantially past the end of the buttefly. For
performance reasons we don't use the indexing masking for
TypedArrays. Since TypedArrays are already gigacaged the risk of
wild reads is still restricted.

This patch is a <1% regression on Speedometer and ~3% regression
on JetStream in my testing.

  • assembler/MacroAssembler.h:

(JSC::MacroAssembler::urshiftPtr):

  • bytecode/AccessCase.cpp:

(JSC::AccessCase::generateImpl):

  • dfg/DFGAbstractHeap.h:
  • dfg/DFGClobberize.h:

(JSC::DFG::clobberize):

  • dfg/DFGSpeculativeJIT.cpp:

(JSC::DFG::SpeculativeJIT::emitAllocateRawObject):
(JSC::DFG::SpeculativeJIT::compileDoublePutByVal):
(JSC::DFG::SpeculativeJIT::compileNewFunctionCommon):
(JSC::DFG::SpeculativeJIT::compileCreateActivation):
(JSC::DFG::SpeculativeJIT::compileCreateDirectArguments):
(JSC::DFG::SpeculativeJIT::compileArraySlice):
(JSC::DFG::SpeculativeJIT::compileNukeStructureAndSetButterfly):
(JSC::DFG::SpeculativeJIT::compileNewStringObject):
(JSC::DFG::SpeculativeJIT::compileNewTypedArray):

  • dfg/DFGSpeculativeJIT.h:

(JSC::DFG::SpeculativeJIT::emitAllocateJSObject):
(JSC::DFG::SpeculativeJIT::emitAllocateJSObjectWithKnownSize):

  • dfg/DFGSpeculativeJIT32_64.cpp:

(JSC::DFG::SpeculativeJIT::compile):
(JSC::DFG::SpeculativeJIT::compileAllocateNewArrayWithSize):

  • dfg/DFGSpeculativeJIT64.cpp:

(JSC::DFG::SpeculativeJIT::compile):
(JSC::DFG::SpeculativeJIT::compileAllocateNewArrayWithSize):

  • ftl/FTLAbstractHeap.cpp:

(JSC::FTL::IndexedAbstractHeap::baseIndex):

  • ftl/FTLAbstractHeap.h:
  • ftl/FTLAbstractHeapRepository.h:
  • ftl/FTLLowerDFGToB3.cpp:

(JSC::FTL::DFG::LowerDFGToB3::compileAtomicsReadModifyWrite):
(JSC::FTL::DFG::LowerDFGToB3::compileGetByVal):
(JSC::FTL::DFG::LowerDFGToB3::compileCreateActivation):
(JSC::FTL::DFG::LowerDFGToB3::compileNewFunction):
(JSC::FTL::DFG::LowerDFGToB3::compileCreateDirectArguments):
(JSC::FTL::DFG::LowerDFGToB3::compileNewStringObject):
(JSC::FTL::DFG::LowerDFGToB3::compileNewTypedArray):
(JSC::FTL::DFG::LowerDFGToB3::compileMaterializeNewObject):
(JSC::FTL::DFG::LowerDFGToB3::compileMaterializeCreateActivation):
(JSC::FTL::DFG::LowerDFGToB3::maskedIndex):
(JSC::FTL::DFG::LowerDFGToB3::computeButterflyIndexingMask):
(JSC::FTL::DFG::LowerDFGToB3::allocateObject):
(JSC::FTL::DFG::LowerDFGToB3::allocateVariableSizedObject):
(JSC::FTL::DFG::LowerDFGToB3::allocateJSArray):
(JSC::FTL::DFG::LowerDFGToB3::pointerIntoTypedArray):

  • ftl/FTLOutput.h:

(JSC::FTL::Output::baseIndex):

  • jit/AssemblyHelpers.h:

(JSC::AssemblyHelpers::emitComputeButterflyIndexingMask):
(JSC::AssemblyHelpers::nukeStructureAndStoreButterfly):
(JSC::AssemblyHelpers::emitAllocateJSObject):
(JSC::AssemblyHelpers::emitAllocateJSObjectWithKnownSize):
(JSC::AssemblyHelpers::emitAllocateVariableSizedJSObject):
(JSC::AssemblyHelpers::emitAllocateDestructibleObject):
(JSC::AssemblyHelpers::storeButterfly): Deleted.

  • jit/JITOpcodes.cpp:

(JSC::JIT::emit_op_new_object):
(JSC::JIT::emit_op_create_this):

  • jit/JITOpcodes32_64.cpp:

(JSC::JIT::emit_op_new_object):
(JSC::JIT::emit_op_create_this):

  • jit/JITPropertyAccess.cpp:

(JSC::JIT::emitDoubleLoad):
(JSC::JIT::emitContiguousLoad):
(JSC::JIT::emitArrayStorageLoad):

  • llint/LowLevelInterpreter32_64.asm:
  • llint/LowLevelInterpreter64.asm:
  • runtime/ArrayStorage.h:

(JSC::ArrayStorage::availableVectorLength):

  • runtime/Butterfly.h:

(JSC::ContiguousData::ContiguousData):
(JSC::ContiguousData::at const):
(JSC::ContiguousData::at):
(JSC::Butterfly::publicLength const):
(JSC::Butterfly::vectorLength const):
(JSC::Butterfly::computeIndexingMaskForVectorLength):
(JSC::Butterfly::computeIndexingMask):
(JSC::Butterfly::contiguousInt32):
(JSC::ContiguousData::operator[] const): Deleted.
(JSC::ContiguousData::operator[]): Deleted.
(JSC::Butterfly::publicLength): Deleted.
(JSC::Butterfly::vectorLength): Deleted.

  • runtime/ButterflyInlines.h:

(JSC::ContiguousData<T>::at const):
(JSC::ContiguousData<T>::at):

  • runtime/ClonedArguments.cpp:

(JSC::ClonedArguments::createEmpty):

  • runtime/JSArray.cpp:

(JSC::JSArray::tryCreateUninitializedRestricted):
(JSC::JSArray::appendMemcpy):
(JSC::JSArray::setLength):
(JSC::JSArray::pop):
(JSC::JSArray::fastSlice):
(JSC::JSArray::shiftCountWithArrayStorage):
(JSC::JSArray::shiftCountWithAnyIndexingType):
(JSC::JSArray::unshiftCountWithAnyIndexingType):
(JSC::JSArray::fillArgList):
(JSC::JSArray::copyToArguments):

  • runtime/JSArrayBufferView.cpp:

(JSC::JSArrayBufferView::JSArrayBufferView):

  • runtime/JSArrayInlines.h:

(JSC::JSArray::pushInline):

  • runtime/JSFixedArray.h:

(JSC::JSFixedArray::createFromArray):

  • runtime/JSGenericTypedArrayViewInlines.h:

(JSC::JSGenericTypedArrayView<Adaptor>::slowDownAndWasteMemory):

  • runtime/JSObject.cpp:

(JSC::JSObject::getOwnPropertySlotByIndex):
(JSC::JSObject::putByIndex):
(JSC::JSObject::createInitialInt32):
(JSC::JSObject::createInitialDouble):
(JSC::JSObject::createInitialContiguous):
(JSC::JSObject::convertUndecidedToInt32):
(JSC::JSObject::convertUndecidedToDouble):
(JSC::JSObject::convertUndecidedToContiguous):
(JSC::JSObject::convertInt32ToDouble):
(JSC::JSObject::convertInt32ToArrayStorage):
(JSC::JSObject::convertDoubleToContiguous):
(JSC::JSObject::convertDoubleToArrayStorage):
(JSC::JSObject::convertContiguousToArrayStorage):
(JSC::JSObject::createInitialForValueAndSet):
(JSC::JSObject::deletePropertyByIndex):
(JSC::JSObject::getOwnPropertyNames):
(JSC::JSObject::putByIndexBeyondVectorLengthWithoutAttributes):
(JSC::JSObject::countElements):
(JSC::JSObject::ensureLengthSlow):
(JSC::JSObject::reallocateAndShrinkButterfly):
(JSC::JSObject::getEnumerableLength):

  • runtime/JSObject.h:

(JSC::JSObject::canGetIndexQuickly):
(JSC::JSObject::getIndexQuickly):
(JSC::JSObject::tryGetIndexQuickly const):
(JSC::JSObject::setIndexQuickly):
(JSC::JSObject::initializeIndex):
(JSC::JSObject::initializeIndexWithoutBarrier):
(JSC::JSObject::butterflyIndexingMaskOffset):
(JSC::JSObject::butterflyIndexingMask const):
(JSC::JSObject::setButterflyWithIndexingMask):
(JSC::JSObject::setButterfly):
(JSC::JSObject::nukeStructureAndSetButterfly):
(JSC::JSObject::JSObject):

  • runtime/RegExpMatchesArray.h:

(JSC::tryCreateUninitializedRegExpMatchesArray):

  • runtime/Structure.cpp:

(JSC::Structure::flattenDictionaryStructure):

Source/WebCore:

  • bindings/js/JSDOMConvertSequences.h:

(WebCore::Detail::NumericSequenceConverter::convertArray):
(WebCore::Detail::SequenceConverter::convertArray):

Source/WTF:

Add a clz that wraps the builtin clz intrinisics provided by
various compilers. The clz function by default assumes that
the input may be zero. On X86 this makes a difference because not
all CPUs have LZCNT and BSR has undefined behavior on zero. On ARM,
the zero check gets optimized away, regardless.

  • wtf/StdLibExtras.h:

(std::clz):

File:
1 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/runtime/JSObject.h

    r224564 r225913  
    263263        case ALL_INT32_INDEXING_TYPES:
    264264        case ALL_CONTIGUOUS_INDEXING_TYPES:
    265             return i < butterfly->vectorLength() && butterfly->contiguous()[i];
     265            return i < butterfly->vectorLength() && butterfly->contiguous().at(this, i);
    266266        case ALL_DOUBLE_INDEXING_TYPES: {
    267267            if (i >= butterfly->vectorLength())
    268268                return false;
    269             double value = butterfly->contiguousDouble()[i];
     269            double value = butterfly->contiguousDouble().at(this, i);
    270270            if (value != value)
    271271                return false;
    … …  
    285285        switch (indexingType()) {
    286286        case ALL_INT32_INDEXING_TYPES:
    287             return jsNumber(butterfly->contiguous()[i].get().asInt32());
     287            return jsNumber(butterfly->contiguous().at(this, i).get().asInt32());
    288288        case ALL_CONTIGUOUS_INDEXING_TYPES:
    289             return butterfly->contiguous()[i].get();
     289            return butterfly->contiguous().at(this, i).get();
    290290        case ALL_DOUBLE_INDEXING_TYPES:
    291             return JSValue(JSValue::EncodeAsDouble, butterfly->contiguousDouble()[i]);
     291            return JSValue(JSValue::EncodeAsDouble, butterfly->contiguousDouble().at(this, i));
    292292        case ALL_ARRAY_STORAGE_INDEXING_TYPES:
    293293            return butterfly->arrayStorage()->m_vector[i].get();
    … …  
    307307        case ALL_INT32_INDEXING_TYPES:
    308308            if (i < butterfly->publicLength()) {
    309                 JSValue result = butterfly->contiguous()[i].get();
     309                JSValue result = butterfly->contiguous().at(this, i).get();
    310310                ASSERT(result.isInt32() || !result);
    311311                return result;
    … …  
    314314        case ALL_CONTIGUOUS_INDEXING_TYPES:
    315315            if (i < butterfly->publicLength())
    316                 return butterfly->contiguous()[i].get();
     316                return butterfly->contiguous().at(this, i).get();
    317317            break;
    318318        case ALL_DOUBLE_INDEXING_TYPES: {
    319319            if (i >= butterfly->publicLength())
    320320                break;
    321             double result = butterfly->contiguousDouble()[i];
     321            double result = butterfly->contiguousDouble().at(this, i);
    322322            if (result != result)
    323323                break;
    … …  
    389389        case ALL_CONTIGUOUS_INDEXING_TYPES: {
    390390            ASSERT(i < butterfly->vectorLength());
    391             butterfly->contiguous()[i].set(vm, this, v);
     391            butterfly->contiguous().at(this, i).set(vm, this, v);
    392392            if (i >= butterfly->publicLength())
    393393                butterfly->setPublicLength(i + 1);
    … …  
    405405                return;
    406406            }
    407             butterfly->contiguousDouble()[i] = value;
     407            butterfly->contiguousDouble().at(this, i) = value;
    408408            if (i >= butterfly->publicLength())
    409409                butterfly->setPublicLength(i + 1);
    … …  
    455455            ASSERT(i < butterfly->publicLength());
    456456            ASSERT(i < butterfly->vectorLength());
    457             butterfly->contiguous()[i].set(vm, this, v);
     457            butterfly->contiguous().at(this, i).set(vm, this, v);
    458458            break;
    459459        }
    … …  
    470470                return;
    471471            }
    472             butterfly->contiguousDouble()[i] = value;
     472            butterfly->contiguousDouble().at(this, i) = value;
    473473            break;
    474474        }
    … …  
    509509            ASSERT(i < butterfly->publicLength());
    510510            ASSERT(i < butterfly->vectorLength());
    511             butterfly->contiguous()[i].setWithoutWriteBarrier(v);
     511            butterfly->contiguous().at(this, i).setWithoutWriteBarrier(v);
    512512            break;
    513513        }
    … …  
    518518            double value = v.asNumber();
    519519            RELEASE_ASSERT(value == value);
    520             butterfly->contiguousDouble()[i] = value;
     520            butterfly->contiguousDouble().at(this, i) = value;
    521521            break;
    522522        }
    … …  
    771771    // in-place.
    772772    void nukeStructureAndSetButterfly(VM&, StructureID, Butterfly*);
     773
     774    // Call this only if you are a JSGenericTypedArrayView or are clearing the butterfly.
     775    void setButterflyWithIndexingMask(VM&, Butterfly*, uint32_t indexingMask);
    773776   
    774777    void setStructure(VM&, Structure*);
    … …  
    859862        return OBJECT_OFFSETOF(JSObject, m_butterfly);
    860863    }
     864    static ptrdiff_t butterflyIndexingMaskOffset() { return OBJECT_OFFSETOF(JSObject, m_butterflyIndexingMask); }
     865    uintptr_t butterflyIndexingMask() const { return m_butterflyIndexingMask; }
    861866       
    862867    void* butterflyAddress()
    … …  
    886891    // To instantiate objects you likely want JSFinalObject, below.
    887892    // To create derived types you likely want JSNonFinalObject, below.
    888     JSObject(VM&, Structure*, Butterfly* = 0);
     893    JSObject(VM&, Structure*, Butterfly* = nullptr);
    889894   
    890895    // Visits the butterfly unless there is a race. Returns the structure if there was no race.
    … …  
    10521057    PropertyOffset prepareToPutDirectWithoutTransition(VM&, PropertyName, unsigned attributes, StructureID, Structure*);
    10531058
    1054 protected:
    10551059    AuxiliaryBarrier<Butterfly*> m_butterfly;
    1056 #if USE(JSVALUE32_64)
    1057 private:
    1058     uint32_t m_padding;
    1059 #endif
     1060    uint32_t m_butterflyIndexingMask { 0 };
    10601061};
    10611062
    … …  
    12481249}
    12491250
    1250 inline void JSObject::setButterfly(VM& vm, Butterfly* butterfly)
    1251 {
     1251inline void JSObject::setButterflyWithIndexingMask(VM& vm, Butterfly* butterfly, uint32_t indexingMask)
     1252{
     1253    // These are the only two current use cases for this.
     1254    ASSERT(structure()->hijacksIndexingHeader() || !butterfly);
     1255    m_butterflyIndexingMask = indexingMask;
    12521256    if (isX86() || vm.heap.mutatorShouldBeFenced()) {
    12531257        WTF::storeStoreFence();
    … …  
    12561260        return;
    12571261    }
    1258    
     1262
    12591263    m_butterfly.set(vm, this, butterfly);
    12601264}
    12611265
     1266inline void JSObject::setButterfly(VM& vm, Butterfly* butterfly)
     1267{
     1268    ASSERT(!structure()->hijacksIndexingHeader());
     1269    m_butterflyIndexingMask = butterfly->computeIndexingMask();
     1270    ASSERT(m_butterflyIndexingMask >= butterfly->vectorLength());
     1271    if (isX86() || vm.heap.mutatorShouldBeFenced()) {
     1272        WTF::storeStoreFence();
     1273        m_butterfly.set(vm, this, butterfly);
     1274        WTF::storeStoreFence();
     1275        return;
     1276    }
     1277
     1278    m_butterfly.set(vm, this, butterfly);
     1279}
     1280
    12621281inline void JSObject::nukeStructureAndSetButterfly(VM& vm, StructureID oldStructureID, Butterfly* butterfly)
    12631282{
     1283    ASSERT(!vm.getStructure(oldStructureID)->hijacksIndexingHeader());
     1284    m_butterflyIndexingMask = butterfly->computeIndexingMask();
     1285    ASSERT(m_butterflyIndexingMask >= butterfly->vectorLength());
    12641286    if (isX86() || vm.heap.mutatorShouldBeFenced()) {
    12651287        setStructureIDDirectly(nuke(oldStructureID));
    … …  
    12691291        return;
    12701292    }
    1271    
     1293
    12721294    m_butterfly.set(vm, this, butterfly);
    12731295}
    … …  
    13021324    , m_butterfly(vm, this, butterfly)
    13031325{
     1326    if (butterfly)
     1327        m_butterflyIndexingMask = butterfly->computeIndexingMask();
    13041328}
    13051329
Note: See TracChangeset for help on using the changeset viewer.