Modified: trunk/JSTests/ChangeLog (233120 => 233121)
--- trunk/JSTests/ChangeLog 2018-06-23 04:18:27 UTC (rev 233120)
+++ trunk/JSTests/ChangeLog 2018-06-23 05:27:44 UTC (rev 233121)
@@ -1,3 +1,14 @@
+2018-06-22 Keith Miller <[email protected]>
+
+ unshift should zero unused property storage
+ https://bugs.webkit.org/show_bug.cgi?id=186960
+
+ Reviewed by Saam Barati.
+
+ * stress/array-unshift-zero-property-storage.js: Added.
+ (run):
+ (test):
+
2018-06-22 Mark Lam <[email protected]>
PropertyCondition::isValidValueForAttributes() should also consider deleted values.
Added: trunk/JSTests/stress/array-unshift-zero-property-storage.js (0 => 233121)
--- trunk/JSTests/stress/array-unshift-zero-property-storage.js (rev 0)
+++ trunk/JSTests/stress/array-unshift-zero-property-storage.js 2018-06-23 05:27:44 UTC (rev 233121)
@@ -0,0 +1,34 @@
+let foo = "get some property storage";
+let first = "new first element";
+let bar = "ensure property storage is zeroed";
+
+function run(array) {
+ array.foo = foo;
+ array.unshift(first, ...new Array(100));
+ array.bar = bar;
+ return array;
+}
+noInline(run);
+
+function test() {
+ let array = run([]);
+ if (array.foo !== foo)
+ throw new Error();
+ if (array.bar !== bar)
+ throw new Error();
+ if (array[0] !== first)
+ throw new Error();
+
+ array = [];
+ array.unshift(1);
+ array = run(array);
+ if (array.foo !== foo)
+ throw new Error();
+ if (array.bar !== bar)
+ throw new Error();
+ if (array[0] !== first)
+ throw new Error();
+}
+
+for (let i = 0; i < 1; i++)
+ test();
Modified: trunk/Source/_javascript_Core/ChangeLog (233120 => 233121)
--- trunk/Source/_javascript_Core/ChangeLog 2018-06-23 04:18:27 UTC (rev 233120)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-06-23 05:27:44 UTC (rev 233121)
@@ -1,3 +1,18 @@
+2018-06-22 Keith Miller <[email protected]>
+
+ unshift should zero unused property storage
+ https://bugs.webkit.org/show_bug.cgi?id=186960
+
+ Reviewed by Saam Barati.
+
+ Also, this patch adds the zeroed unused property storage assertion
+ to one more place it was missing.
+
+ * runtime/JSArray.cpp:
+ (JSC::JSArray::unshiftCountSlowCase):
+ * runtime/JSObjectInlines.h:
+ (JSC::JSObject::putDirectInternal):
+
2018-06-22 Mark Lam <[email protected]>
PropertyCondition::isValidValueForAttributes() should also consider deleted values.
Modified: trunk/Source/_javascript_Core/runtime/JSArray.cpp (233120 => 233121)
--- trunk/Source/_javascript_Core/runtime/JSArray.cpp 2018-06-23 04:18:27 UTC (rev 233120)
+++ trunk/Source/_javascript_Core/runtime/JSArray.cpp 2018-06-23 05:27:44 UTC (rev 233121)
@@ -347,7 +347,7 @@
// Step 2:
// We're either going to choose to allocate a new ArrayStorage, or we're going to reuse the existing one.
- void* newAllocBase = 0;
+ void* newAllocBase = nullptr;
unsigned newStorageCapacity;
bool allocatedNewStorage;
// If the current storage array is sufficiently large (but not too large!) then just keep using it.
@@ -356,11 +356,11 @@
newStorageCapacity = currentCapacity;
allocatedNewStorage = false;
} else {
- size_t newSize = Butterfly::totalSize(0, propertyCapacity, true, ArrayStorage::sizeFor(desiredCapacity));
- newAllocBase = vm.jsValueGigacageAuxiliarySpace.allocateNonVirtual(
- vm, newSize, nullptr, AllocationFailureMode::ReturnNull);
- if (!newAllocBase)
+ const unsigned preCapacity = 0;
+ Butterfly* newButterfly = Butterfly::tryCreateUninitialized(vm, this, preCapacity, propertyCapacity, true, ArrayStorage::sizeFor(desiredCapacity));
+ if (!newButterfly)
return false;
+ newAllocBase = newButterfly->base(preCapacity, propertyCapacity);
newStorageCapacity = desiredCapacity;
allocatedNewStorage = true;
}
@@ -385,23 +385,25 @@
unsigned newVectorLength = requiredVectorLength + postCapacity;
RELEASE_ASSERT(newVectorLength <= MAX_STORAGE_VECTOR_LENGTH);
- unsigned newIndexBias = newStorageCapacity - newVectorLength;
+ unsigned preCapacity = newStorageCapacity - newVectorLength;
- Butterfly* newButterfly = Butterfly::fromBase(newAllocBase, newIndexBias, propertyCapacity);
+ Butterfly* newButterfly = Butterfly::fromBase(newAllocBase, preCapacity, propertyCapacity);
if (addToFront) {
ASSERT(count + usedVectorLength <= newVectorLength);
memmove(newButterfly->arrayStorage()->m_vector + count, storage->m_vector, sizeof(JSValue) * usedVectorLength);
memmove(newButterfly->propertyStorage() - propertySize, butterfly->propertyStorage() - propertySize, sizeof(JSValue) * propertySize + sizeof(IndexingHeader) + ArrayStorage::sizeFor(0));
-
+
if (allocatedNewStorage) {
// We will set the vectorLength to newVectorLength. We populated requiredVectorLength
// (usedVectorLength + count), which is less. Clear the difference.
for (unsigned i = requiredVectorLength; i < newVectorLength; ++i)
newButterfly->arrayStorage()->m_vector[i].clear();
+ // We don't need to zero the pre-capacity because it is not available to use as property storage.
+ memset(newButterfly->base(0, propertyCapacity), 0, (propertyCapacity - propertySize) * sizeof(JSValue));
}
- } else if ((newAllocBase != butterfly->base(structure)) || (newIndexBias != storage->m_indexBias)) {
- memmove(newButterfly->propertyStorage() - propertySize, butterfly->propertyStorage() - propertySize, sizeof(JSValue) * propertySize + sizeof(IndexingHeader) + ArrayStorage::sizeFor(0));
+ } else if ((newAllocBase != butterfly->base(structure)) || (preCapacity != storage->m_indexBias)) {
+ memmove(newButterfly->propertyStorage() - propertyCapacity, butterfly->propertyStorage() - propertyCapacity, sizeof(JSValue) * propertyCapacity + sizeof(IndexingHeader) + ArrayStorage::sizeFor(0));
memmove(newButterfly->arrayStorage()->m_vector, storage->m_vector, sizeof(JSValue) * usedVectorLength);
for (unsigned i = requiredVectorLength; i < newVectorLength; i++)
@@ -409,7 +411,7 @@
}
newButterfly->arrayStorage()->setVectorLength(newVectorLength);
- newButterfly->arrayStorage()->m_indexBias = newIndexBias;
+ newButterfly->arrayStorage()->m_indexBias = preCapacity;
setButterfly(vm, newButterfly);
Modified: trunk/Source/_javascript_Core/runtime/JSObjectInlines.h (233120 => 233121)
--- trunk/Source/_javascript_Core/runtime/JSObjectInlines.h 2018-06-23 04:18:27 UTC (rev 233120)
+++ trunk/Source/_javascript_Core/runtime/JSObjectInlines.h 2018-06-23 05:27:44 UTC (rev 233121)
@@ -329,6 +329,10 @@
validateOffset(offset);
ASSERT(newStructure->isValidOffset(offset));
+
+ // This assertion verifies that the concurrent GC won't read garbage if the concurrentGC
+ // is running at the same time we put without transitioning.
+ ASSERT(!getDirect(offset) || !JSValue::encode(getDirect(offset)));
putDirect(vm, offset, value);
setStructure(vm, newStructure);
slot.setNewProperty(this, offset);
