Attachment 342981 Details for Bug 186692 – Patch

[patch] Patch

bug-186692-20180618153748.patch (text/plain), 36.06 KB, created by Keith Miller on 2018-06-18 15:37:49 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Keith Miller

Created: 2018-06-18 15:37:49 PDT

Size: 36.06 KB

patch

obsolete

>Subversion Revision: 232801
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 206fcc33552f73f7f4e382aeb7c5c358d8a8af6b..30dbcabf609dab39b27623002a602834a9bd635a 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,88 @@
>+2018-06-18  Keith Miller  <keith_miller@apple.com>
>+
>+        Properly zero unused property storage offsets
>+        https://bugs.webkit.org/show_bug.cgi?id=186692
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Since the concurrent GC might see a property slot before the mutator has actually
>+        stored the value there, we need to ensure that slot doesn't have garbage in it.
>+
>+        Right now when calling constructConvertedArrayStorageWithoutCopyingElements
>+        or creating a RegExp matches array we never cleared the unused
>+        property storage. ObjectIntializationScope has also been upgraded
>+        to look for our invariants around property storage. Additionally,
>+        a new assertion has been added to check for JSValue() when adding
>+        a new property.
>+
>+        Lastly, we used to put undefined into deleted property offsets. To
>+        make things simpler, this patch causes us to store JSValue() there
>+        instead.
>+
>+        * runtime/Butterfly.h:
>+        (JSC::Butterfly::offsetOfVectorLength):
>+        * runtime/ButterflyInlines.h:
>+        (JSC::Butterfly::tryCreateUninitialized):
>+        (JSC::Butterfly::createUninitialized):
>+        (JSC::Butterfly::tryCreate):
>+        (JSC::Butterfly::create):
>+        (JSC::Butterfly::createOrGrowPropertyStorage):
>+        (JSC::Butterfly::createOrGrowArrayRight):
>+        (JSC::Butterfly::growArrayRight):
>+        (JSC::Butterfly::resizeArray):
>+        * runtime/JSArray.cpp:
>+        (JSC::JSArray::tryCreateUninitializedRestricted):
>+        (JSC::createArrayButterflyInDictionaryIndexingMode): Deleted.
>+        * runtime/JSArray.h:
>+        (JSC::tryCreateArrayButterfly):
>+        * runtime/JSObject.cpp:
>+        (JSC::JSObject::createArrayStorageButterfly):
>+        (JSC::JSObject::constructConvertedArrayStorageWithoutCopyingElements):
>+        (JSC::JSObject::deleteProperty):
>+        (JSC::JSObject::shiftButterflyAfterFlattening):
>+        * runtime/JSObject.h:
>+        * runtime/JSObjectInlines.h:
>+        (JSC::JSObject::prepareToPutDirectWithoutTransition):
>+        * runtime/ObjectInitializationScope.cpp:
>+        (JSC::ObjectInitializationScope::verifyPropertiesAreInitialized):
>+        * runtime/ObjectInitializationScope.h:
>+        (JSC::ObjectInitializationScope::release):
>+        * runtime/RegExpMatchesArray.h:
>+        (JSC::tryCreateUninitializedRegExpMatchesArray):
>+        (JSC::createRegExpMatchesArray):
>+
>+        * runtime/Butterfly.h:
>+        (JSC::Butterfly::offsetOfVectorLength):
>+        * runtime/ButterflyInlines.h:
>+        (JSC::Butterfly::tryCreateUninitialized):
>+        (JSC::Butterfly::createUninitialized):
>+        (JSC::Butterfly::tryCreate):
>+        (JSC::Butterfly::create):
>+        (JSC::Butterfly::createOrGrowPropertyStorage):
>+        (JSC::Butterfly::createOrGrowArrayRight):
>+        (JSC::Butterfly::growArrayRight):
>+        (JSC::Butterfly::resizeArray):
>+        * runtime/JSArray.cpp:
>+        (JSC::JSArray::tryCreateUninitializedRestricted):
>+        (JSC::createArrayButterflyInDictionaryIndexingMode): Deleted.
>+        * runtime/JSArray.h:
>+        (JSC::tryCreateArrayButterfly):
>+        * runtime/JSObject.cpp:
>+        (JSC::JSObject::createArrayStorageButterfly):
>+        (JSC::JSObject::constructConvertedArrayStorageWithoutCopyingElements):
>+        (JSC::JSObject::deleteProperty):
>+        (JSC::JSObject::shiftButterflyAfterFlattening):
>+        * runtime/JSObject.h:
>+        * runtime/JSObjectInlines.h:
>+        (JSC::JSObject::prepareToPutDirectWithoutTransition):
>+        * runtime/ObjectInitializationScope.cpp:
>+        (JSC::ObjectInitializationScope::verifyPropertiesAreInitialized):
>+        * runtime/RegExpMatchesArray.cpp:
>+        (JSC::createEmptyRegExpMatchesArray):
>+        * runtime/RegExpMatchesArray.h:
>+        (JSC::tryCreateUninitializedRegExpMatchesArray):
>+        (JSC::createRegExpMatchesArray):
>+
> 2018-06-13  Keith Miller  <keith_miller@apple.com>
> 
>         AutomaticThread should have a way to provide a thread name
>diff --git a/Source/JavaScriptCore/runtime/Butterfly.h b/Source/JavaScriptCore/runtime/Butterfly.h
>index 6485890d9b798b302fac7a4b5c87213887b22345..52f602606cf7488ebbd7a917f2d142ddcd8633be 100644
>--- a/Source/JavaScriptCore/runtime/Butterfly.h
>+++ b/Source/JavaScriptCore/runtime/Butterfly.h
>@@ -35,6 +35,8 @@ namespace JSC {
> 
> class VM;
> class CopyVisitor;
>+class ObjectInitializationScope;
>+class GCDeferralContext;
> struct ArrayStorage;
> 
> template <typename T>
>@@ -159,12 +161,13 @@ public:
>     static ptrdiff_t offsetOfArrayBuffer() { return offsetOfIndexingHeader() + IndexingHeader::offsetOfArrayBuffer(); }
>     static ptrdiff_t offsetOfPublicLength() { return offsetOfIndexingHeader() + IndexingHeader::offsetOfPublicLength(); }
>     static ptrdiff_t offsetOfVectorLength() { return offsetOfIndexingHeader() + IndexingHeader::offsetOfVectorLength(); }
>-    
>-    static Butterfly* createUninitialized(VM&, JSCell* intendedOwner, size_t preCapacity, size_t propertyCapacity, bool hasIndexingHeader, size_t indexingPayloadSizeInBytes);
> 
>-    static Butterfly* tryCreate(VM& vm, JSCell*, size_t preCapacity, size_t propertyCapacity, bool hasIndexingHeader, const IndexingHeader& indexingHeader, size_t indexingPayloadSizeInBytes);
>-    static Butterfly* create(VM&, JSCell* intendedOwner, size_t preCapacity, size_t propertyCapacity, bool hasIndexingHeader, const IndexingHeader&, size_t indexingPayloadSizeInBytes);
>-    static Butterfly* create(VM&, JSCell* intendedOwner, Structure*);
>+    static Butterfly* tryCreateUninitialized(VM&, JSObject* intendedOwner, size_t preCapacity, size_t propertyCapacity, bool hasIndexingHeader, size_t indexingPayloadSizeInBytes, GCDeferralContext* = nullptr);
>+    static Butterfly* createUninitialized(VM&, JSObject* intendedOwner, size_t preCapacity, size_t propertyCapacity, bool hasIndexingHeader, size_t indexingPayloadSizeInBytes);
>+
>+    static Butterfly* tryCreate(VM& vm, JSObject*, size_t preCapacity, size_t propertyCapacity, bool hasIndexingHeader, const IndexingHeader& indexingHeader, size_t indexingPayloadSizeInBytes);
>+    static Butterfly* create(VM&, JSObject* intendedOwner, size_t preCapacity, size_t propertyCapacity, bool hasIndexingHeader, const IndexingHeader&, size_t indexingPayloadSizeInBytes);
>+    static Butterfly* create(VM&, JSObject* intendedOwner, Structure*);
>     
>     IndexingHeader* indexingHeader() { return IndexingHeader::from(this); }
>     const IndexingHeader* indexingHeader() const { return IndexingHeader::from(this); }
>@@ -203,7 +206,7 @@ public:
>     void* base(Structure*);
> 
>     static Butterfly* createOrGrowArrayRight(
>-        Butterfly*, VM&, JSCell* intendedOwner, Structure* oldStructure,
>+        Butterfly*, VM&, JSObject* intendedOwner, Structure* oldStructure,
>         size_t propertyCapacity, bool hadIndexingHeader,
>         size_t oldIndexingPayloadSizeInBytes, size_t newIndexingPayloadSizeInBytes); 
> 
>@@ -212,11 +215,11 @@ public:
>     // methods is not exhaustive and is not intended to encapsulate all possible allocation
>     // modes of butterflies - there are code paths that allocate butterflies by calling
>     // directly into Heap::tryAllocateStorage.
>-    static Butterfly* createOrGrowPropertyStorage(Butterfly*, VM&, JSCell* intendedOwner, Structure*, size_t oldPropertyCapacity, size_t newPropertyCapacity);
>-    Butterfly* growArrayRight(VM&, JSCell* intendedOwner, Structure* oldStructure, size_t propertyCapacity, bool hadIndexingHeader, size_t oldIndexingPayloadSizeInBytes, size_t newIndexingPayloadSizeInBytes); // Assumes that preCapacity is zero, and asserts as much.
>-    Butterfly* growArrayRight(VM&, JSCell* intendedOwner, Structure*, size_t newIndexingPayloadSizeInBytes);
>-    Butterfly* resizeArray(VM&, JSCell* intendedOwner, size_t propertyCapacity, bool oldHasIndexingHeader, size_t oldIndexingPayloadSizeInBytes, size_t newPreCapacity, bool newHasIndexingHeader, size_t newIndexingPayloadSizeInBytes);
>-    Butterfly* resizeArray(VM&, JSCell* intendedOwner, Structure*, size_t newPreCapacity, size_t newIndexingPayloadSizeInBytes); // Assumes that you're not changing whether or not the object has an indexing header.
>+    static Butterfly* createOrGrowPropertyStorage(Butterfly*, VM&, JSObject* intendedOwner, Structure*, size_t oldPropertyCapacity, size_t newPropertyCapacity);
>+    Butterfly* growArrayRight(VM&, JSObject* intendedOwner, Structure* oldStructure, size_t propertyCapacity, bool hadIndexingHeader, size_t oldIndexingPayloadSizeInBytes, size_t newIndexingPayloadSizeInBytes); // Assumes that preCapacity is zero, and asserts as much.
>+    Butterfly* growArrayRight(VM&, JSObject* intendedOwner, Structure*, size_t newIndexingPayloadSizeInBytes);
>+    Butterfly* resizeArray(VM&, JSObject* intendedOwner, size_t propertyCapacity, bool oldHasIndexingHeader, size_t oldIndexingPayloadSizeInBytes, size_t newPreCapacity, bool newHasIndexingHeader, size_t newIndexingPayloadSizeInBytes);
>+    Butterfly* resizeArray(VM&, JSObject* intendedOwner, Structure*, size_t newPreCapacity, size_t newIndexingPayloadSizeInBytes); // Assumes that you're not changing whether or not the object has an indexing header.
>     Butterfly* unshift(Structure*, size_t numberOfSlots);
>     Butterfly* shift(Structure*, size_t numberOfSlots);
> };
>diff --git a/Source/JavaScriptCore/runtime/ButterflyInlines.h b/Source/JavaScriptCore/runtime/ButterflyInlines.h
>index 6a89a0d384f1e1c76a0486dc9c9707bf14d930e6..03a5118b5d3766e2129788b3c1b25a67ceff3a8a 100644
>--- a/Source/JavaScriptCore/runtime/ButterflyInlines.h
>+++ b/Source/JavaScriptCore/runtime/ButterflyInlines.h
>@@ -28,8 +28,8 @@
> #include "ArrayStorage.h"
> #include "Butterfly.h"
> #include "JSObject.h"
>-#include "VM.h"
> #include "Structure.h"
>+#include "VM.h"
> 
> namespace JSC {
> 
>@@ -74,15 +74,28 @@ ALWAYS_INLINE unsigned Butterfly::optimalContiguousVectorLength(Structure* struc
>     return optimalContiguousVectorLength(structure ? structure->outOfLineCapacity() : 0, vectorLength);
> }
> 
>-inline Butterfly* Butterfly::createUninitialized(VM& vm, JSCell*, size_t preCapacity, size_t propertyCapacity, bool hasIndexingHeader, size_t indexingPayloadSizeInBytes)
>+inline Butterfly* Butterfly::tryCreateUninitialized(VM& vm, JSObject*, size_t preCapacity, size_t propertyCapacity, bool hasIndexingHeader, size_t indexingPayloadSizeInBytes, GCDeferralContext* deferralContext)
>+{
>+    size_t size = totalSize(preCapacity, propertyCapacity, hasIndexingHeader, indexingPayloadSizeInBytes);
>+    void* base = vm.jsValueGigacageAuxiliarySpace.allocateNonVirtual(vm, size, deferralContext, AllocationFailureMode::ReturnNull);
>+    if (UNLIKELY(!base))
>+        return nullptr;
>+
>+    Butterfly* result = fromBase(base, preCapacity, propertyCapacity);
>+
>+    return result;
>+}
>+
>+inline Butterfly* Butterfly::createUninitialized(VM& vm, JSObject*, size_t preCapacity, size_t propertyCapacity, bool hasIndexingHeader, size_t indexingPayloadSizeInBytes)
> {
>     size_t size = totalSize(preCapacity, propertyCapacity, hasIndexingHeader, indexingPayloadSizeInBytes);
>     void* base = vm.jsValueGigacageAuxiliarySpace.allocateNonVirtual(vm, size, nullptr, AllocationFailureMode::Assert);
>     Butterfly* result = fromBase(base, preCapacity, propertyCapacity);
>+
>     return result;
> }
> 
>-inline Butterfly* Butterfly::tryCreate(VM& vm, JSCell*, size_t preCapacity, size_t propertyCapacity, bool hasIndexingHeader, const IndexingHeader& indexingHeader, size_t indexingPayloadSizeInBytes)
>+inline Butterfly* Butterfly::tryCreate(VM& vm, JSObject*, size_t preCapacity, size_t propertyCapacity, bool hasIndexingHeader, const IndexingHeader& indexingHeader, size_t indexingPayloadSizeInBytes)
> {
>     size_t size = totalSize(preCapacity, propertyCapacity, hasIndexingHeader, indexingPayloadSizeInBytes);
>     void* base = vm.jsValueGigacageAuxiliarySpace.allocateNonVirtual(vm, size, nullptr, AllocationFailureMode::ReturnNull);
>@@ -95,7 +108,7 @@ inline Butterfly* Butterfly::tryCreate(VM& vm, JSCell*, size_t preCapacity, size
>     return result;
> }
> 
>-inline Butterfly* Butterfly::create(VM& vm, JSCell* intendedOwner, size_t preCapacity, size_t propertyCapacity, bool hasIndexingHeader, const IndexingHeader& indexingHeader, size_t indexingPayloadSizeInBytes)
>+inline Butterfly* Butterfly::create(VM& vm, JSObject* intendedOwner, size_t preCapacity, size_t propertyCapacity, bool hasIndexingHeader, const IndexingHeader& indexingHeader, size_t indexingPayloadSizeInBytes)
> {
>     Butterfly* result = tryCreate(vm, intendedOwner, preCapacity, propertyCapacity, hasIndexingHeader, indexingHeader, indexingPayloadSizeInBytes);
> 
>@@ -103,7 +116,7 @@ inline Butterfly* Butterfly::create(VM& vm, JSCell* intendedOwner, size_t preCap
>     return result;
> }
> 
>-inline Butterfly* Butterfly::create(VM& vm, JSCell* intendedOwner, Structure* structure)
>+inline Butterfly* Butterfly::create(VM& vm, JSObject* intendedOwner, Structure* structure)
> {
>     return create(
>         vm, intendedOwner, 0, structure->outOfLineCapacity(),
>@@ -116,7 +129,7 @@ inline void* Butterfly::base(Structure* structure)
> }
> 
> inline Butterfly* Butterfly::createOrGrowPropertyStorage(
>-    Butterfly* oldButterfly, VM& vm, JSCell* intendedOwner, Structure* structure, size_t oldPropertyCapacity, size_t newPropertyCapacity)
>+    Butterfly* oldButterfly, VM& vm, JSObject* intendedOwner, Structure* structure, size_t oldPropertyCapacity, size_t newPropertyCapacity)
> {
>     RELEASE_ASSERT(newPropertyCapacity > oldPropertyCapacity);
>     if (!oldButterfly)
>@@ -125,8 +138,7 @@ inline Butterfly* Butterfly::createOrGrowPropertyStorage(
>     size_t preCapacity = oldButterfly->indexingHeader()->preCapacity(structure);
>     size_t indexingPayloadSizeInBytes = oldButterfly->indexingHeader()->indexingPayloadSizeInBytes(structure);
>     bool hasIndexingHeader = structure->hasIndexingHeader(intendedOwner);
>-    Butterfly* result = createUninitialized(
>-        vm, intendedOwner, preCapacity, newPropertyCapacity, hasIndexingHeader, indexingPayloadSizeInBytes);
>+    Butterfly* result = createUninitialized(vm, intendedOwner, preCapacity, newPropertyCapacity, hasIndexingHeader, indexingPayloadSizeInBytes);
>     memcpy(
>         result->propertyStorage() - oldPropertyCapacity,
>         oldButterfly->propertyStorage() - oldPropertyCapacity,
>@@ -139,7 +151,7 @@ inline Butterfly* Butterfly::createOrGrowPropertyStorage(
> }
> 
> inline Butterfly* Butterfly::createOrGrowArrayRight(
>-    Butterfly* oldButterfly, VM& vm, JSCell* intendedOwner, Structure* oldStructure,
>+    Butterfly* oldButterfly, VM& vm, JSObject* intendedOwner, Structure* oldStructure,
>     size_t propertyCapacity, bool hadIndexingHeader, size_t oldIndexingPayloadSizeInBytes,
>     size_t newIndexingPayloadSizeInBytes)
> {
>@@ -154,7 +166,7 @@ inline Butterfly* Butterfly::createOrGrowArrayRight(
> }
> 
> inline Butterfly* Butterfly::growArrayRight(
>-    VM& vm, JSCell* intendedOwner, Structure* oldStructure, size_t propertyCapacity,
>+    VM& vm, JSObject* intendedOwner, Structure* oldStructure, size_t propertyCapacity,
>     bool hadIndexingHeader, size_t oldIndexingPayloadSizeInBytes,
>     size_t newIndexingPayloadSizeInBytes)
> {
>@@ -172,7 +184,7 @@ inline Butterfly* Butterfly::growArrayRight(
> }
> 
> inline Butterfly* Butterfly::growArrayRight(
>-    VM& vm, JSCell* intendedOwner, Structure* oldStructure,
>+    VM& vm, JSObject* intendedOwner, Structure* oldStructure,
>     size_t newIndexingPayloadSizeInBytes)
> {
>     return growArrayRight(
>@@ -183,13 +195,11 @@ inline Butterfly* Butterfly::growArrayRight(
> }
> 
> inline Butterfly* Butterfly::resizeArray(
>-    VM& vm, JSCell* intendedOwner, size_t propertyCapacity, bool oldHasIndexingHeader,
>+    VM& vm, JSObject* intendedOwner, size_t propertyCapacity, bool oldHasIndexingHeader,
>     size_t oldIndexingPayloadSizeInBytes, size_t newPreCapacity, bool newHasIndexingHeader,
>     size_t newIndexingPayloadSizeInBytes)
> {
>-    Butterfly* result = createUninitialized(
>-        vm, intendedOwner, newPreCapacity, propertyCapacity, newHasIndexingHeader,
>-        newIndexingPayloadSizeInBytes);
>+    Butterfly* result = createUninitialized(vm, intendedOwner, newPreCapacity, propertyCapacity, newHasIndexingHeader, newIndexingPayloadSizeInBytes);
>     // FIXME: This could be made much more efficient if we used the property size,
>     // not the capacity.
>     void* to = result->propertyStorage() - propertyCapacity;
>@@ -202,7 +212,7 @@ inline Butterfly* Butterfly::resizeArray(
> }
> 
> inline Butterfly* Butterfly::resizeArray(
>-    VM& vm, JSCell* intendedOwner, Structure* structure, size_t newPreCapacity,
>+    VM& vm, JSObject* intendedOwner, Structure* structure, size_t newPreCapacity,
>     size_t newIndexingPayloadSizeInBytes)
> {
>     bool hasIndexingHeader = structure->hasIndexingHeader(intendedOwner);
>diff --git a/Source/JavaScriptCore/runtime/JSArray.cpp b/Source/JavaScriptCore/runtime/JSArray.cpp
>index 1ad6eb3edf2182d29b074a333a06f7cf8ce212ae..4a6011d40f73e5123225403bd22e3143c3389740 100644
>--- a/Source/JavaScriptCore/runtime/JSArray.cpp
>+++ b/Source/JavaScriptCore/runtime/JSArray.cpp
>@@ -43,20 +43,6 @@ STATIC_ASSERT_IS_TRIVIALLY_DESTRUCTIBLE(JSArray);
> 
> const ClassInfo JSArray::s_info = {"Array", &JSNonFinalObject::s_info, nullptr, nullptr, CREATE_METHOD_TABLE(JSArray)};
> 
>-Butterfly* createArrayButterflyInDictionaryIndexingMode(
>-    VM& vm, JSCell* intendedOwner, unsigned initialLength)
>-{
>-    Butterfly* butterfly = Butterfly::create(
>-        vm, intendedOwner, 0, 0, true, IndexingHeader(), ArrayStorage::sizeFor(0));
>-    ArrayStorage* storage = butterfly->arrayStorage();
>-    storage->setLength(initialLength);
>-    storage->setVectorLength(0);
>-    storage->m_indexBias = 0;
>-    storage->m_sparseMap.clear();
>-    storage->m_numValuesInVector = 0;
>-    return butterfly;
>-}
>-
> JSArray* JSArray::tryCreateUninitializedRestricted(ObjectInitializationScope& scope, GCDeferralContext* deferralContext, Structure* structure, unsigned initialLength)
> {
>     VM& vm = scope.vm();
>@@ -64,9 +50,9 @@ JSArray* JSArray::tryCreateUninitializedRestricted(ObjectInitializationScope& sc
>     if (UNLIKELY(initialLength > MAX_STORAGE_VECTOR_LENGTH))
>         return 0;
> 
>-    JSGlobalObject* globalObject = structure->globalObject();
>-    bool createUninitialized = globalObject->isOriginalArrayStructure(structure);
>     unsigned outOfLineStorage = structure->outOfLineCapacity();
>+    JSGlobalObject* globalObject = structure->globalObject();
>+    ASSERT_UNUSED(globalObject, globalObject->isOriginalArrayStructure(structure) || structure == globalObject->regExpMatchesArrayStructure() || structure == globalObject->regExpMatchesArrayWithGroupsStructure());
> 
>     Butterfly* butterfly;
>     IndexingType indexingType = structure->indexingType();
>@@ -87,12 +73,11 @@ JSArray* JSArray::tryCreateUninitializedRestricted(ObjectInitializationScope& sc
>         butterfly = Butterfly::fromBase(temp, 0, outOfLineStorage);
>         butterfly->setVectorLength(vectorLength);
>         butterfly->setPublicLength(initialLength);
>-        unsigned i = (createUninitialized ? initialLength : 0);
>         if (hasDouble(indexingType)) {
>-            for (; i < vectorLength; ++i)
>+            for (unsigned i = initialLength; i < vectorLength; ++i)
>                 butterfly->contiguousDouble().atUnsafe(i) = PNaN;
>         } else {
>-            for (; i < vectorLength; ++i)
>+            for (unsigned i = initialLength; i < vectorLength; ++i)
>                 butterfly->contiguous().atUnsafe(i).clear();
>         }
>     } else {
>@@ -110,12 +95,13 @@ JSArray* JSArray::tryCreateUninitializedRestricted(ObjectInitializationScope& sc
>         storage->m_indexBias = indexBias;
>         storage->m_sparseMap.clear();
>         storage->m_numValuesInVector = initialLength;
>-        unsigned i = (createUninitialized ? initialLength : 0);
>-        for (; i < vectorLength; ++i)
>+        for (unsigned i = initialLength; i < vectorLength; ++i)
>             storage->m_vector[i].clear();
>     }
> 
>     JSArray* result = createWithButterfly(vm, deferralContext, structure, butterfly);
>+
>+    const bool createUninitialized = true;
>     scope.notifyAllocated(result, createUninitialized);
>     return result;
> }
>diff --git a/Source/JavaScriptCore/runtime/JSArray.h b/Source/JavaScriptCore/runtime/JSArray.h
>index 7150bbfc450f0a1d824aa65f9919ee67223ab10e..602f2a0a0729a1d1a5d00bd962bb3ca2c4e2959c 100644
>--- a/Source/JavaScriptCore/runtime/JSArray.h
>+++ b/Source/JavaScriptCore/runtime/JSArray.h
>@@ -203,7 +203,7 @@ private:
>     void setLengthWritable(ExecState*, bool writable);
> };
> 
>-inline Butterfly* tryCreateArrayButterfly(VM& vm, JSCell* intendedOwner, unsigned initialLength)
>+inline Butterfly* tryCreateArrayButterfly(VM& vm, JSObject* intendedOwner, unsigned initialLength)
> {
>     Butterfly* butterfly = Butterfly::tryCreate(
>         vm, intendedOwner, 0, 0, true, baseIndexingHeaderForArrayStorage(initialLength),
>@@ -217,9 +217,6 @@ inline Butterfly* tryCreateArrayButterfly(VM& vm, JSCell* intendedOwner, unsigne
>     return butterfly;
> }
> 
>-Butterfly* createArrayButterflyInDictionaryIndexingMode(
>-    VM&, JSCell* intendedOwner, unsigned initialLength);
>-
> inline JSArray* JSArray::tryCreate(VM& vm, Structure* structure, unsigned initialLength, unsigned vectorLengthHint)
> {
>     ASSERT(vectorLengthHint >= initialLength);
>diff --git a/Source/JavaScriptCore/runtime/JSObject.cpp b/Source/JavaScriptCore/runtime/JSObject.cpp
>index 696a146ecb5c9d29dda63e730c24a4eef1a242bc..71598eb41a07679d96f94ac15cf0022324b0ec24 100644
>--- a/Source/JavaScriptCore/runtime/JSObject.cpp
>+++ b/Source/JavaScriptCore/runtime/JSObject.cpp
>@@ -1089,7 +1089,7 @@ ContiguousJSValues JSObject::createInitialContiguous(VM& vm, unsigned length)
>     return newButterfly->contiguous();
> }
> 
>-Butterfly* JSObject::createArrayStorageButterfly(VM& vm, JSCell* intendedOwner, Structure* structure, unsigned length, unsigned vectorLength, Butterfly* oldButterfly)
>+Butterfly* JSObject::createArrayStorageButterfly(VM& vm, JSObject* intendedOwner, Structure* structure, unsigned length, unsigned vectorLength, Butterfly* oldButterfly)
> {
>     Butterfly* newButterfly = Butterfly::createOrGrowArrayRight(
>         oldButterfly, vm, intendedOwner, structure, structure->outOfLineCapacity(), false, 0,
>@@ -1172,16 +1172,14 @@ ArrayStorage* JSObject::constructConvertedArrayStorageWithoutCopyingElements(VM&
>     Structure* structure = this->structure(vm);
>     unsigned publicLength = m_butterfly->publicLength();
>     unsigned propertyCapacity = structure->outOfLineCapacity();
>-    unsigned propertySize = structure->outOfLineSize();
>-    
>-    Butterfly* newButterfly = Butterfly::createUninitialized(
>-        vm, this, 0, propertyCapacity, true, ArrayStorage::sizeFor(neededLength));
>+
>+    Butterfly* newButterfly = Butterfly::createUninitialized(vm, this, 0, propertyCapacity, true, ArrayStorage::sizeFor(neededLength));
>     
>     memcpy(
>-        newButterfly->propertyStorage() - propertySize,
>-        m_butterfly->propertyStorage() - propertySize,
>-        propertySize * sizeof(EncodedJSValue));
>-    
>+        newButterfly->base(0, propertyCapacity),
>+        m_butterfly->base(0, propertyCapacity),
>+        propertyCapacity * sizeof(EncodedJSValue));
>+
>     ArrayStorage* newStorage = newButterfly->arrayStorage();
>     newStorage->setVectorLength(neededLength);
>     newStorage->setLength(publicLength);
>@@ -1912,7 +1910,7 @@ bool JSObject::deleteProperty(JSCell* cell, ExecState* exec, PropertyName proper
>             thisObject->setStructure(vm, Structure::removePropertyTransition(vm, structure, propertyName, offset));
> 
>         if (offset != invalidOffset)
>-            thisObject->putDirectUndefined(offset);
>+            thisObject->locationForOffset(offset)->clear();
>     }
> 
>     return true;
>@@ -3629,7 +3627,7 @@ void JSObject::shiftButterflyAfterFlattening(const GCSafeConcurrentJSLocker&, VM
>     // This could interleave visitChildren because some old structure could have been a non
>     // dictionary structure. We have to be crazy careful. But, we are guaranteed to be holding
>     // the structure's lock right now, and that helps a bit.
>-    
>+
>     Butterfly* oldButterfly = this->butterfly();
>     size_t preCapacity;
>     size_t indexingPayloadSizeInBytes;
>diff --git a/Source/JavaScriptCore/runtime/JSObject.h b/Source/JavaScriptCore/runtime/JSObject.h
>index faabc5d6c6e414e68d56c5adcbd5d637da5b8c77..88bb6e0b3efd2f5652b8d605c0559b9c4294adb7 100644
>--- a/Source/JavaScriptCore/runtime/JSObject.h
>+++ b/Source/JavaScriptCore/runtime/JSObject.h
>@@ -943,7 +943,7 @@ protected:
>     void convertDoubleForValue(VM&, JSValue);
>     void convertFromCopyOnWrite(VM&);
> 
>-    static Butterfly* createArrayStorageButterfly(VM&, JSCell* intendedOwner, Structure*, unsigned length, unsigned vectorLength, Butterfly* oldButterfly = nullptr);
>+    static Butterfly* createArrayStorageButterfly(VM&, JSObject* intendedOwner, Structure*, unsigned length, unsigned vectorLength, Butterfly* oldButterfly = nullptr);
>     ArrayStorage* createArrayStorage(VM&, unsigned length, unsigned vectorLength);
>     ArrayStorage* createInitialArrayStorage(VM&);
>         
>diff --git a/Source/JavaScriptCore/runtime/JSObjectInlines.h b/Source/JavaScriptCore/runtime/JSObjectInlines.h
>index efb0f53534624bffef984c4139f999b27823fbb6..ba480e39edc25a5be13aed280dd6ef1dc42aa790 100644
>--- a/Source/JavaScriptCore/runtime/JSObjectInlines.h
>+++ b/Source/JavaScriptCore/runtime/JSObjectInlines.h
>@@ -203,6 +203,7 @@ ALWAYS_INLINE PropertyOffset JSObject::prepareToPutDirectWithoutTransition(VM& v
>                 setStructureIDDirectly(structureID);
>             } else
>                 structure->setLastOffset(newLastOffset);
>+            ASSERT(!getDirect(offset));
>             result = offset;
>         });
>     return result;
>diff --git a/Source/JavaScriptCore/runtime/ObjectInitializationScope.cpp b/Source/JavaScriptCore/runtime/ObjectInitializationScope.cpp
>index 9818a4cfba7eb72a21e1bcdb4bfe25d50952829f..dd348a71f1a315ebbde65350a42122b6724ca053 100644
>--- a/Source/JavaScriptCore/runtime/ObjectInitializationScope.cpp
>+++ b/Source/JavaScriptCore/runtime/ObjectInitializationScope.cpp
>@@ -60,15 +60,16 @@ void ObjectInitializationScope::notifyAllocated(JSObject* object, bool wasCreate
> void ObjectInitializationScope::verifyPropertiesAreInitialized(JSObject* object)
> {
>     Butterfly* butterfly = object->butterfly();
>-    IndexingType indexingType = object->structure(m_vm)->indexingType();
>+    Structure* structure = object->structure(m_vm);
>+    IndexingType indexingType = structure->indexingType();
>     unsigned vectorLength = butterfly->vectorLength();
>-    if (UNLIKELY(hasUndecided(indexingType))) {
>+    if (UNLIKELY(hasUndecided(indexingType)) || !hasIndexedProperties(indexingType)) {
>         // Nothing to verify.
>     } else if (LIKELY(!hasAnyArrayStorage(indexingType))) {
>         auto data = butterfly->contiguous().data();
>         for (unsigned i = 0; i < vectorLength; ++i) {
>             if (isScribbledValue(data[i].get())) {
>-                dataLog("Found scribbled value at i = ", i, "\n");
>+                dataLogLn("Found scribbled value at i = ", i);
>                 ASSERT_NOT_REACHED();
>             }
>         }
>@@ -76,11 +77,21 @@ void ObjectInitializationScope::verifyPropertiesAreInitialized(JSObject* object)
>         ArrayStorage* storage = butterfly->arrayStorage();
>         for (unsigned i = 0; i < vectorLength; ++i) {
>             if (isScribbledValue(storage->m_vector[i].get())) {
>-                dataLog("Found scribbled value at i = ", i, "\n");
>+                dataLogLn("Found scribbled value at i = ", i);
>                 ASSERT_NOT_REACHED();
>             }
>         }
>     }
>+
>+    for (int64_t i = 0; i < static_cast<int64_t>(structure->outOfLineCapacity()); i++) {
>+        // We rely on properties past the last offset be zero for concurrent GC.
>+        if (i + firstOutOfLineOffset > structure->lastOffset())
>+            ASSERT(!butterfly->propertyStorage()[-i - 1].get());
>+        else if (isScribbledValue(butterfly->propertyStorage()[-i - 1].get())) {
>+            dataLogLn("Found scribbled property at i = ", -i - 1);
>+            ASSERT_NOT_REACHED();
>+        }
>+    }
> }
> #endif
> 
>diff --git a/Source/JavaScriptCore/runtime/RegExpMatchesArray.cpp b/Source/JavaScriptCore/runtime/RegExpMatchesArray.cpp
>index e6de25410aba3d1b90c0fffcab9970c08a0d5d1c..412e5f7ffba27ce5ea4693e46705704d072419f0 100644
>--- a/Source/JavaScriptCore/runtime/RegExpMatchesArray.cpp
>+++ b/Source/JavaScriptCore/runtime/RegExpMatchesArray.cpp
>@@ -37,9 +37,9 @@ JSArray* createEmptyRegExpMatchesArray(JSGlobalObject* globalObject, JSString* i
>     // https://bugs.webkit.org/show_bug.cgi?id=155144
>     
>     GCDeferralContext deferralContext(vm.heap);
>-    
>+    ObjectInitializationScope scope(vm);
>+
>     if (UNLIKELY(globalObject->isHavingABadTime())) {
>-        ObjectInitializationScope scope(vm);
>         array = JSArray::tryCreateUninitializedRestricted(scope, &deferralContext, globalObject->regExpMatchesArrayStructure(), regExp->numSubpatterns() + 1);
>         // FIXME: we should probably throw an out of memory error here, but
>         // when making this change we should check that all clients of this
>diff --git a/Source/JavaScriptCore/runtime/RegExpMatchesArray.h b/Source/JavaScriptCore/runtime/RegExpMatchesArray.h
>index d1a1cbe4ff0a7bb0f5db6298ef23654dec9c347e..0303d58a165607639937588f8eff2dc5ad17492a 100644
>--- a/Source/JavaScriptCore/runtime/RegExpMatchesArray.h
>+++ b/Source/JavaScriptCore/runtime/RegExpMatchesArray.h
>@@ -40,20 +40,20 @@ ALWAYS_INLINE JSArray* tryCreateUninitializedRegExpMatchesArray(ObjectInitializa
>     if (vectorLength > MAX_STORAGE_VECTOR_LENGTH)
>         return 0;
> 
>-    JSGlobalObject* globalObject = structure->globalObject();
>-    bool createUninitialized = globalObject->isOriginalArrayStructure(structure);
>-    void* temp = vm.jsValueGigacageAuxiliarySpace.allocateNonVirtual(vm, Butterfly::totalSize(0, structure->outOfLineCapacity(), true, vectorLength * sizeof(EncodedJSValue)), deferralContext, AllocationFailureMode::ReturnNull);
>-    if (UNLIKELY(!temp))
>+    const bool hasIndexingHeader = true;
>+    Butterfly* butterfly = Butterfly::tryCreateUninitialized(vm, nullptr, 0, structure->outOfLineCapacity(), hasIndexingHeader, vectorLength * sizeof(EncodedJSValue), deferralContext);
>+    if (UNLIKELY(!butterfly))
>         return nullptr;
>-    Butterfly* butterfly = Butterfly::fromBase(temp, 0, structure->outOfLineCapacity());
>+
>     butterfly->setVectorLength(vectorLength);
>     butterfly->setPublicLength(initialLength);
> 
>-    unsigned i = (createUninitialized ? initialLength : 0);
>-    for (; i < vectorLength; ++i)
>+    for (unsigned i = initialLength; i < vectorLength; ++i)
>         butterfly->contiguous().atUnsafe(i).clear();
> 
>     JSArray* result = JSArray::createWithButterfly(vm, deferralContext, structure, butterfly);
>+
>+    const bool createUninitialized = true;
>     scope.notifyAllocated(result, createUninitialized);
>     return result;
> }
>@@ -80,23 +80,29 @@ ALWAYS_INLINE JSArray* createRegExpMatchesArray(
>     unsigned numSubpatterns = regExp->numSubpatterns();
>     bool hasNamedCaptures = regExp->hasNamedCaptures();
>     JSObject* groups = nullptr;
>+    Structure* matchStructure = globalObject->regExpMatchesArrayStructure();
>+    if (hasNamedCaptures) {
>+        groups = JSFinalObject::create(vm, JSFinalObject::createStructure(vm, globalObject, globalObject->objectPrototype(), 0));
>+        matchStructure = globalObject->regExpMatchesArrayWithGroupsStructure();
>+    }
> 
>     auto setProperties = [&] () {
>         array->putDirect(vm, RegExpMatchesArrayIndexPropertyOffset, jsNumber(result.start));
>         array->putDirect(vm, RegExpMatchesArrayInputPropertyOffset, input);
>-        if (hasNamedCaptures) {
>-            groups = JSFinalObject::create(vm, JSFinalObject::createStructure(vm, globalObject, globalObject->objectPrototype(), 0));
>+        if (hasNamedCaptures)
>             array->putDirect(vm, RegExpMatchesArrayGroupsPropertyOffset, groups);
>-        }
>-    };
> 
>-    GCDeferralContext deferralContext(vm.heap);
>-
>-    Structure* matchStructure = hasNamedCaptures ? globalObject->regExpMatchesArrayWithGroupsStructure() : globalObject->regExpMatchesArrayStructure();
>+        ASSERT(!array->butterfly()->indexingHeader()->preCapacity(matchStructure));
>+        auto capacity = matchStructure->outOfLineCapacity();
>+        auto size = matchStructure->outOfLineSize();
>+        memset(array->butterfly()->base(0, capacity), 0, (capacity - size) * sizeof(JSValue));
>+    };
> 
>     if (UNLIKELY(globalObject->isHavingABadTime())) {
>+        GCDeferralContext deferralContext(vm.heap);
>         ObjectInitializationScope scope(vm);
>         array = JSArray::tryCreateUninitializedRestricted(scope, &deferralContext, matchStructure, numSubpatterns + 1);
>+
>         // FIXME: we should probably throw an out of memory error here, but
>         // when making this change we should check that all clients of this
>         // function will correctly handle an exception being thrown from here.
>@@ -115,21 +121,20 @@ ALWAYS_INLINE JSArray* createRegExpMatchesArray(
>             else
>                 value = jsUndefined();
>             array->initializeIndexWithoutBarrier(scope, i, value);
>-            if (groups) {
>-                String groupName = regExp->getCaptureGroupName(i);
>-                if (!groupName.isEmpty())
>-                    groups->putDirect(vm, Identifier::fromString(&vm, groupName), value);
>-            }
>         }
>     } else {
>+        GCDeferralContext deferralContext(vm.heap);
>         ObjectInitializationScope scope(vm);
>         array = tryCreateUninitializedRegExpMatchesArray(scope, &deferralContext, matchStructure, numSubpatterns + 1);
>+
>+        // FIXME: we should probably throw an out of memory error here, but
>+        // when making this change we should check that all clients of this
>+        // function will correctly handle an exception being thrown from here.
>+        // https://bugs.webkit.org/show_bug.cgi?id=169786
>         RELEASE_ASSERT(array);
>         
>         setProperties();
>         
>-        // Now the object is safe to scan by GC.
>-
>         array->initializeIndexWithoutBarrier(scope, 0, jsSubstringOfResolved(vm, &deferralContext, input, result.start, result.end - result.start), ArrayWithContiguous);
>         
>         for (unsigned i = 1; i <= numSubpatterns; ++i) {
>@@ -140,11 +145,16 @@ ALWAYS_INLINE JSArray* createRegExpMatchesArray(
>             else
>                 value = jsUndefined();
>             array->initializeIndexWithoutBarrier(scope, i, value, ArrayWithContiguous);
>-            if (groups) {
>-                String groupName = regExp->getCaptureGroupName(i);
>-                if (!groupName.isEmpty())
>-                    groups->putDirect(vm, Identifier::fromString(&vm, groupName), value);
>-            }
>+        }
>+    }
>+
>+    // Now the object is safe to scan by GC.
>+
>+    if (groups) {
>+        for (unsigned i = 1; i <= numSubpatterns; ++i) {
>+            String groupName = regExp->getCaptureGroupName(i);
>+            if (!groupName.isEmpty())
>+                groups->putDirect(vm, Identifier::fromString(&vm, groupName), array->getIndexQuickly(i));
>         }
>     }
>     return array;
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index a13eb29736b08503294cfe8ce9ce6e6efed8591f..1bcfc7a6c72af08bfd2107cc6b84d8396c166558 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -1,3 +1,12 @@
>+2018-06-18  Keith Miller  <keith_miller@apple.com>
>+
>+        Properly zero unused property storage offsets
>+        https://bugs.webkit.org/show_bug.cgi?id=186692
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * stress/butterfly-zero-unused-butterfly-properties.js: Added.
>+
> 2018-06-12  Yusuke Suzuki  <utatane.tea@gmail.com>
> 
>         Update test262 for Array#sort
>diff --git a/JSTests/stress/butterfly-zero-unused-butterfly-properties.js b/JSTests/stress/butterfly-zero-unused-butterfly-properties.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..da3faa65a1b1dd4fc5f121cea4a16d5478458148
>--- /dev/null
>+++ b/JSTests/stress/butterfly-zero-unused-butterfly-properties.js
>@@ -0,0 +1,9 @@
>+//@ runDefault("--jitPolicyScale=0", "--gcMaxHeapSize=2000")
>+
>+// This test happened to catch a case where we failed to zero the space in the butterfly before m_lastOffset when reallocating.
>+
>+var array = Array(1000);
>+for (var i = 0; i < 100000; ++i) {
>+    array[i - array.length] = '';
>+    array[i ^ array.length] = '';
>+}

Actions: View | Formatted Diff | Diff

Attachments on bug 186692: 342843 | 342851 | 342854 | 342859 | 342865 | 342872 | 342873 | 342875 | 342876 | 342959 | 342967 | 342972 | 342981 | 342983