Attachment 342873 Details for Bug 186692 – Patch

[patch] Patch

bug-186692-20180615202429.patch (text/plain), 34.65 KB, created by Keith Miller on 2018-06-15 20:24:30 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Keith Miller

Created: 2018-06-15 20:24:30 PDT

Size: 34.65 KB

patch

obsolete

>Subversion Revision: 232801
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 206fcc33552f73f7f4e382aeb7c5c358d8a8af6b..91ba4f8c077a872bce102e1c965f8114e066743c 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,59 @@
>+2018-06-15  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
>+        proprety storage. This patch fixes that issue and gets most users
>+        of the Butterfly::createUninitialized API to pass an
>+        ObjectInitializationScope.
>+
>+        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.
>+
>+        * dfg/DFGOperations.cpp:
>+        * 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::convertFromCopyOnWrite):
>+        (JSC::JSObject::deleteProperty):
>+        (JSC::JSObject::shiftButterflyAfterFlattening):
>+        * runtime/JSObject.h:
>+        * runtime/JSObjectInlines.h:
>+        (JSC::JSObject::prepareToPutDirectWithoutTransition):
>+        * runtime/ObjectInitializationScope.cpp:
>+        (JSC::ObjectInitializationScope::verifyPropertiesAreInitialized):
>+        * 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/dfg/DFGOperations.cpp b/Source/JavaScriptCore/dfg/DFGOperations.cpp
>index 7c9e70b6bd78b9993fd6dd4ae1e00614dd2a61ae..e409efda367761026fd569bdd67d7c391040b4f7 100644
>--- a/Source/JavaScriptCore/dfg/DFGOperations.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGOperations.cpp
>@@ -1849,7 +1849,7 @@ char* JIT_OPERATION operationAllocateSimplePropertyStorageWithInitialCapacity(Ex
>     NativeCallFrameTracer tracer(&vm, exec);
> 
>     return reinterpret_cast<char*>(
>-        Butterfly::createUninitialized(vm, 0, 0, initialOutOfLineCapacity, false, 0));
>+        Butterfly::createUninitialized(vm, nullptr, nullptr, 0, initialOutOfLineCapacity, false, 0));
> }
> 
> char* JIT_OPERATION operationAllocateSimplePropertyStorage(ExecState* exec, size_t newSize)
>@@ -1858,7 +1858,7 @@ char* JIT_OPERATION operationAllocateSimplePropertyStorage(ExecState* exec, size
>     NativeCallFrameTracer tracer(&vm, exec);
> 
>     return reinterpret_cast<char*>(
>-        Butterfly::createUninitialized(vm, 0, 0, newSize, false, 0));
>+        Butterfly::createUninitialized(vm, nullptr, nullptr, 0, newSize, false, 0));
> }
> 
> char* JIT_OPERATION operationAllocateComplexPropertyStorageWithInitialCapacity(ExecState* exec, JSObject* object)
>diff --git a/Source/JavaScriptCore/runtime/Butterfly.h b/Source/JavaScriptCore/runtime/Butterfly.h
>index 6485890d9b798b302fac7a4b5c87213887b22345..41eb8433033e49caae5b5e9c7a973b4de4d22a38 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&, ObjectInitializationScope*, JSObject* intendedOwner, size_t preCapacity, size_t propertyCapacity, bool hasIndexingHeader, size_t indexingPayloadSizeInBytes, GCDeferralContext* = nullptr);
>+    static Butterfly* createUninitialized(VM&, ObjectInitializationScope*, 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..15349cbfc0eead86c130bb12c4ed4c6785a4b8e1 100644
>--- a/Source/JavaScriptCore/runtime/ButterflyInlines.h
>+++ b/Source/JavaScriptCore/runtime/ButterflyInlines.h
>@@ -28,8 +28,9 @@
> #include "ArrayStorage.h"
> #include "Butterfly.h"
> #include "JSObject.h"
>-#include "VM.h"
>+#include "ObjectInitializationScope.h"
> #include "Structure.h"
>+#include "VM.h"
> 
> namespace JSC {
> 
>@@ -74,15 +75,36 @@ 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, ObjectInitializationScope* initializationScope, JSObject* intendedOwner, 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);
>+
>+    const bool wasCreatedUnitializated = true;
>+    if (initializationScope)
>+        initializationScope->notifyAllocated(intendedOwner, wasCreatedUnitializated);
>+
>+    return result;
>+}
>+
>+inline Butterfly* Butterfly::createUninitialized(VM& vm, ObjectInitializationScope* initializationScope, JSObject* intendedOwner, 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);
>+
>+    const bool wasCreatedUnitializated = true;
>+    if (initializationScope)
>+        initializationScope->notifyAllocated(intendedOwner, wasCreatedUnitializated);
>+
>     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 +117,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 +125,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,17 +138,18 @@ 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)
>         return create(vm, intendedOwner, 0, newPropertyCapacity, false, IndexingHeader(), 0);
> 
>+    ObjectInitializationScope scope(vm);
>+
>     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, &scope, intendedOwner, preCapacity, newPropertyCapacity, hasIndexingHeader, indexingPayloadSizeInBytes);
>     memcpy(
>         result->propertyStorage() - oldPropertyCapacity,
>         oldButterfly->propertyStorage() - oldPropertyCapacity,
>@@ -139,7 +162,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 +177,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 +195,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,12 +206,14 @@ 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)
> {
>+    ObjectInitializationScope scope(vm);
>+
>     Butterfly* result = createUninitialized(
>-        vm, intendedOwner, newPreCapacity, propertyCapacity, newHasIndexingHeader,
>+        vm, &scope, intendedOwner, newPreCapacity, propertyCapacity, newHasIndexingHeader,
>         newIndexingPayloadSizeInBytes);
>     // FIXME: This could be made much more efficient if we used the property size,
>     // not the capacity.
>@@ -202,7 +227,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..0d981d90630f6abd9195c4d0a2cfa4ee1a8efcba 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,17 @@ ArrayStorage* JSObject::constructConvertedArrayStorageWithoutCopyingElements(VM&
>     Structure* structure = this->structure(vm);
>     unsigned publicLength = m_butterfly->publicLength();
>     unsigned propertyCapacity = structure->outOfLineCapacity();
>-    unsigned propertySize = structure->outOfLineSize();
>-    
>+
>+    ObjectInitializationScope scope(vm);
>+
>     Butterfly* newButterfly = Butterfly::createUninitialized(
>-        vm, this, 0, propertyCapacity, true, ArrayStorage::sizeFor(neededLength));
>+        vm, &scope, 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);
>@@ -1426,11 +1427,13 @@ void JSObject::convertFromCopyOnWrite(VM& vm)
>     ASSERT(isCopyOnWrite(indexingMode()));
>     ASSERT(structure(vm)->indexingMode() == indexingMode());
> 
>+    ObjectInitializationScope scope(vm);
>+
>     const bool hasIndexingHeader = true;
>     Butterfly* oldButterfly = butterfly();
>     size_t propertyCapacity = 0;
>     unsigned newVectorLength = Butterfly::optimalContiguousVectorLength(propertyCapacity, std::min(oldButterfly->vectorLength() * 2, MAX_STORAGE_VECTOR_LENGTH));
>-    Butterfly* newButterfly = Butterfly::createUninitialized(vm, this, 0, propertyCapacity, hasIndexingHeader, newVectorLength * sizeof(JSValue));
>+    Butterfly* newButterfly = Butterfly::createUninitialized(vm, &scope, this, 0, propertyCapacity, hasIndexingHeader, newVectorLength * sizeof(JSValue));
> 
>     memcpy(newButterfly->propertyStorage(), oldButterfly->propertyStorage(), oldButterfly->vectorLength() * sizeof(JSValue) + sizeof(IndexingHeader));
> 
>@@ -1912,7 +1915,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 +3632,8 @@ 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.
>-    
>+
>+    ObjectInitializationScope scope(vm);
>     Butterfly* oldButterfly = this->butterfly();
>     size_t preCapacity;
>     size_t indexingPayloadSizeInBytes;
>@@ -3642,7 +3646,7 @@ void JSObject::shiftButterflyAfterFlattening(const GCSafeConcurrentJSLocker&, VM
>         indexingPayloadSizeInBytes = 0;
>     }
> 
>-    Butterfly* newButterfly = Butterfly::createUninitialized(vm, this, preCapacity, outOfLineCapacityAfter, hasIndexingHeader, indexingPayloadSizeInBytes);
>+    Butterfly* newButterfly = Butterfly::createUninitialized(vm, &scope, this, preCapacity, outOfLineCapacityAfter, hasIndexingHeader, indexingPayloadSizeInBytes);
> 
>     // No need to copy the precapacity.
>     void* currentBase = oldButterfly->base(0, outOfLineCapacityAfter);
>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.h b/Source/JavaScriptCore/runtime/RegExpMatchesArray.h
>index d1a1cbe4ff0a7bb0f5db6298ef23654dec9c347e..6c01811fe713f00a391d71ab717416d8a11fbb12 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, 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,20 +80,26 @@ 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);
>-        }
>+
>+        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));
>     };
> 
>     GCDeferralContext deferralContext(vm.heap);
> 
>-    Structure* matchStructure = hasNamedCaptures ? globalObject->regExpMatchesArrayWithGroupsStructure() : globalObject->regExpMatchesArrayStructure();
>-
>     if (UNLIKELY(globalObject->isHavingABadTime())) {
>         ObjectInitializationScope scope(vm);
>         array = JSArray::tryCreateUninitializedRestricted(scope, &deferralContext, matchStructure, numSubpatterns + 1);
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index a13eb29736b08503294cfe8ce9ce6e6efed8591f..c3818706eb300bc0dc3b9d3c432ecad37eaebd43 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -1,3 +1,12 @@
>+2018-06-15  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