Fix issues when setting public length on ArrayWithContiguous type butterflies.
https://bugs.webkit.org/show_bug.cgi?id=203211
<rdar://problem/56476097>
Reviewed by Keith Miller and Saam Barati.
For ArrayWithContiguous type butterflies, SlotVisitor scans up to the public
length of the butterfly. When setting a new public length, if the new public
length is greater than the current, we should always writeBarrier after the
setting of the new public length. Otherwise, there can be a race where the GC
scans the butterfly after new values have been written to it but before the
public length as been updated. As a result, the new values never get scanned.
For the DFG and FTL, the StoreBarrierInsertionPhase is responsible for inserting
the writeBarriers after the node. Hence, the writeBarrier is guaranteed to be
after the publicLength has been updated.
* runtime/JSArray.cpp:
(JSC::JSArray::shiftCountWithArrayStorage):
(JSC::JSArray::shiftCountWithAnyIndexingType):
* runtime/JSArrayInlines.h:
(JSC::JSArray::pushInline):
* runtime/JSObject.cpp:
(JSC::JSObject::putByIndex):
(JSC::JSObject::reallocateAndShrinkButterfly):
* runtime/JSObject.h:
(JSC::JSObject::setIndexQuickly):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@251399 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 173b6c3..b8db2f3 100644
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,33 @@
+2019-10-21 Mark Lam <mark.lam@apple.com>
+
+ Fix issues when setting public length on ArrayWithContiguous type butterflies.
+ https://bugs.webkit.org/show_bug.cgi?id=203211
+ <rdar://problem/56476097>
+
+ Reviewed by Keith Miller and Saam Barati.
+
+ For ArrayWithContiguous type butterflies, SlotVisitor scans up to the public
+ length of the butterfly. When setting a new public length, if the new public
+ length is greater than the current, we should always writeBarrier after the
+ setting of the new public length. Otherwise, there can be a race where the GC
+ scans the butterfly after new values have been written to it but before the
+ public length as been updated. As a result, the new values never get scanned.
+
+ For the DFG and FTL, the StoreBarrierInsertionPhase is responsible for inserting
+ the writeBarriers after the node. Hence, the writeBarrier is guaranteed to be
+ after the publicLength has been updated.
+
+ * runtime/JSArray.cpp:
+ (JSC::JSArray::shiftCountWithArrayStorage):
+ (JSC::JSArray::shiftCountWithAnyIndexingType):
+ * runtime/JSArrayInlines.h:
+ (JSC::JSArray::pushInline):
+ * runtime/JSObject.cpp:
+ (JSC::JSObject::putByIndex):
+ (JSC::JSObject::reallocateAndShrinkButterfly):
+ * runtime/JSObject.h:
+ (JSC::JSObject::setIndexQuickly):
+
2019-10-21 Devin Rousso <drousso@apple.com>
Web Inspector: make ObjC protocol dispatcher commands optional and add `respondsToSelector` checks to allow other inspector clients to choose what they implement
diff --git a/Source/JavaScriptCore/runtime/JSArray.cpp b/Source/JavaScriptCore/runtime/JSArray.cpp
index 9f21d8e..84ed17e 100644
--- a/Source/JavaScriptCore/runtime/JSArray.cpp
+++ b/Source/JavaScriptCore/runtime/JSArray.cpp
@@ -894,7 +894,8 @@
Butterfly* butterfly = this->butterfly();
- switch (indexingType()) {
+ auto indexingType = this->indexingType();
+ switch (indexingType) {
case ArrayClass:
return true;
@@ -934,13 +935,14 @@
for (unsigned i = end; i < oldLength; ++i)
butterfly->contiguous().at(this, i).clear();
-
+
butterfly->setPublicLength(oldLength - count);
// Our memmoving of values around in the array could have concealed some of them from
// the collector. Let's make sure that the collector scans this object again.
- vm.heap.writeBarrier(this);
-
+ if (indexingType == ArrayWithContiguous)
+ vm.heap.writeBarrier(this);
+
return true;
}
diff --git a/Source/JavaScriptCore/runtime/JSArrayInlines.h b/Source/JavaScriptCore/runtime/JSArrayInlines.h
index dbe1a4e..b5accf2 100644
--- a/Source/JavaScriptCore/runtime/JSArrayInlines.h
+++ b/Source/JavaScriptCore/runtime/JSArrayInlines.h
@@ -157,8 +157,9 @@
unsigned length = butterfly->publicLength();
ASSERT(length <= butterfly->vectorLength());
if (length < butterfly->vectorLength()) {
- butterfly->contiguous().at(this, length).set(vm, this, value);
+ butterfly->contiguous().at(this, length).setWithoutWriteBarrier(value);
butterfly->setPublicLength(length + 1);
+ vm.heap.writeBarrier(this, value);
return;
}
diff --git a/Source/JavaScriptCore/runtime/JSObject.cpp b/Source/JavaScriptCore/runtime/JSObject.cpp
index c324f72..fe8d361 100644
--- a/Source/JavaScriptCore/runtime/JSObject.cpp
+++ b/Source/JavaScriptCore/runtime/JSObject.cpp
@@ -891,9 +891,10 @@
Butterfly* butterfly = thisObject->butterfly();
if (propertyName >= butterfly->vectorLength())
break;
- butterfly->contiguous().at(thisObject, propertyName).set(vm, thisObject, value);
+ butterfly->contiguous().at(thisObject, propertyName).setWithoutWriteBarrier(value);
if (propertyName >= butterfly->publicLength())
butterfly->setPublicLength(propertyName + 1);
+ vm.heap.writeBarrier(thisObject, value);
return true;
}
@@ -3429,6 +3430,7 @@
ASSERT(length <= MAX_STORAGE_VECTOR_LENGTH);
ASSERT(hasContiguous(indexingType()) || hasInt32(indexingType()) || hasDouble(indexingType()) || hasUndecided(indexingType()));
ASSERT(m_butterfly->vectorLength() > length);
+ ASSERT(m_butterfly->publicLength() >= length);
ASSERT(!m_butterfly->indexingHeader()->preCapacity(structure(vm)));
DeferGC deferGC(vm.heap);
diff --git a/Source/JavaScriptCore/runtime/JSObject.h b/Source/JavaScriptCore/runtime/JSObject.h
index 534b7db..e148896 100644
--- a/Source/JavaScriptCore/runtime/JSObject.h
+++ b/Source/JavaScriptCore/runtime/JSObject.h
@@ -409,9 +409,10 @@
}
case ALL_CONTIGUOUS_INDEXING_TYPES: {
ASSERT(i < butterfly->vectorLength());
- butterfly->contiguous().at(this, i).set(vm, this, v);
+ butterfly->contiguous().at(this, i).setWithoutWriteBarrier(v);
if (i >= butterfly->publicLength())
butterfly->setPublicLength(i + 1);
+ vm.heap.writeBarrier(this, v);
break;
}
case ALL_DOUBLE_INDEXING_TYPES: {
