Properly zero unused property storage offsets
authorkeith_miller@apple.com <keith_miller@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 18 Jun 2018 23:53:27 +0000 (23:53 +0000)
committerkeith_miller@apple.com <keith_miller@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 18 Jun 2018 23:53:27 +0000 (23:53 +0000)
https://bugs.webkit.org/show_bug.cgi?id=186692

Reviewed by Filip Pizlo.

JSTests:

* stress/butterfly-zero-unused-butterfly-properties.js: Added.

Source/JavaScriptCore:

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.

We used to put undefined into deleted property offsets. To
make things simpler, this patch causes us to store JSValue() there
instead.

Lastly, this patch fixes an issue where we would initialize the
array storage of RegExpMatchesArray twice. First with 0 and
secondly with the actual result. Now we only zero memory between
vector length and public length.

* 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):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@232951 268f45cc-cd09-0410-ab3c-d52691b4dbfc

13 files changed:
JSTests/ChangeLog
JSTests/stress/butterfly-zero-unused-butterfly-properties.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/Butterfly.h
Source/JavaScriptCore/runtime/ButterflyInlines.h
Source/JavaScriptCore/runtime/JSArray.cpp
Source/JavaScriptCore/runtime/JSArray.h
Source/JavaScriptCore/runtime/JSObject.cpp
Source/JavaScriptCore/runtime/JSObject.h
Source/JavaScriptCore/runtime/JSObjectInlines.h
Source/JavaScriptCore/runtime/ObjectInitializationScope.cpp
Source/JavaScriptCore/runtime/RegExpMatchesArray.cpp
Source/JavaScriptCore/runtime/RegExpMatchesArray.h

index 2c8f268..3672b57 100644 (file)
@@ -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 Filip Pizlo.
+
+        * stress/butterfly-zero-unused-butterfly-properties.js: Added.
+
 2018-06-18  Michael Saboff  <msaboff@apple.com>
 
         Support Unicode 11 in RegExp
diff --git a/JSTests/stress/butterfly-zero-unused-butterfly-properties.js b/JSTests/stress/butterfly-zero-unused-butterfly-properties.js
new file mode 100644 (file)
index 0000000..da3faa6
--- /dev/null
@@ -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] = '';
+}
index 8e98973..dedc9fa 100644 (file)
@@ -1,3 +1,93 @@
+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 Filip Pizlo.
+
+        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.
+
+        We used to put undefined into deleted property offsets. To
+        make things simpler, this patch causes us to store JSValue() there
+        instead.
+
+        Lastly, this patch fixes an issue where we would initialize the
+        array storage of RegExpMatchesArray twice. First with 0 and
+        secondly with the actual result. Now we only zero memory between
+        vector length and public length.
+
+        * 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-18  Tadeu Zagallo  <tzagallo@apple.com>
 
         Share structure across instances of classes exported through the ObjC API
index 6485890..43a9be4 100644 (file)
@@ -35,6 +35,7 @@ namespace JSC {
 
 class VM;
 class CopyVisitor;
+class GCDeferralContext;
 struct ArrayStorage;
 
 template <typename T>
@@ -159,12 +160,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 +205,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 +214,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);
 };
index 6a89a0d..03a5118 100644 (file)
@@ -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);
index 1ad6eb3..4a6011d 100644 (file)
@@ -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;
 }
index 7150bbf..602f2a0 100644 (file)
@@ -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);
index 696a146..71598eb 100644 (file)
@@ -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;
index faabc5d..88bb6e0 100644 (file)
@@ -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&);
         
index efb0f53..ba480e3 100644 (file)
@@ -203,6 +203,7 @@ ALWAYS_INLINE PropertyOffset JSObject::prepareToPutDirectWithoutTransition(VM& v
                 setStructureIDDirectly(structureID);
             } else
                 structure->setLastOffset(newLastOffset);
+            ASSERT(!getDirect(offset));
             result = offset;
         });
     return result;
index 9818a4c..dd348a7 100644 (file)
@@ -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
 
index e6de254..412e5f7 100644 (file)
@@ -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
index d1a1cbe..e957bf9 100644 (file)
@@ -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,18 @@ 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.
+
+    // We initialize the groups object late as it could allocate, which with the current API could cause
+    // allocations.
+    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;