- Revision
- 183128
- Author
- [email protected]
- Date
- 2015-04-22 13:44:32 -0700 (Wed, 22 Apr 2015)
Log Message
SparseArrayEntry's write barrier owner should be the SparseArrayValueMap.
https://bugs.webkit.org/show_bug.cgi?id=144067
Reviewed by Michael Saboff.
Currently, there are a few places where the JSObject that owns the
SparseArrayValueMap is designated as the owner of the SparseArrayEntry
write barrier. This is a bug and can result in the GC collecting the
SparseArrayEntry even though it is being referenced by the
SparseArrayValueMap. This patch fixes the bug.
* runtime/JSObject.cpp:
(JSC::JSObject::enterDictionaryIndexingModeWhenArrayStorageAlreadyExists):
(JSC::JSObject::putIndexedDescriptor):
* tests/stress/sparse-array-entry-update-144067.js: Added.
(useMemoryToTriggerGCs):
(foo):
Modified Paths
Added Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (183127 => 183128)
--- trunk/Source/_javascript_Core/ChangeLog 2015-04-22 20:39:46 UTC (rev 183127)
+++ trunk/Source/_javascript_Core/ChangeLog 2015-04-22 20:44:32 UTC (rev 183128)
@@ -1,5 +1,25 @@
2015-04-22 Mark Lam <[email protected]>
+ SparseArrayEntry's write barrier owner should be the SparseArrayValueMap.
+ https://bugs.webkit.org/show_bug.cgi?id=144067
+
+ Reviewed by Michael Saboff.
+
+ Currently, there are a few places where the JSObject that owns the
+ SparseArrayValueMap is designated as the owner of the SparseArrayEntry
+ write barrier. This is a bug and can result in the GC collecting the
+ SparseArrayEntry even though it is being referenced by the
+ SparseArrayValueMap. This patch fixes the bug.
+
+ * runtime/JSObject.cpp:
+ (JSC::JSObject::enterDictionaryIndexingModeWhenArrayStorageAlreadyExists):
+ (JSC::JSObject::putIndexedDescriptor):
+ * tests/stress/sparse-array-entry-update-144067.js: Added.
+ (useMemoryToTriggerGCs):
+ (foo):
+
+2015-04-22 Mark Lam <[email protected]>
+
Give the heap object iterators the ability to return early.
https://bugs.webkit.org/show_bug.cgi?id=144011
Modified: trunk/Source/_javascript_Core/runtime/JSObject.cpp (183127 => 183128)
--- trunk/Source/_javascript_Core/runtime/JSObject.cpp 2015-04-22 20:39:46 UTC (rev 183127)
+++ trunk/Source/_javascript_Core/runtime/JSObject.cpp 2015-04-22 20:44:32 UTC (rev 183128)
@@ -584,7 +584,7 @@
// This will always be a new entry in the map, so no need to check we can write,
// and attributes are default so no need to set them.
if (value)
- map->add(this, i).iterator->value.set(vm, this, value);
+ map->add(this, i).iterator->value.set(vm, map, value);
}
DeferGC deferGC(vm.heap);
@@ -1717,12 +1717,13 @@
void JSObject::putIndexedDescriptor(ExecState* exec, SparseArrayEntry* entryInMap, const PropertyDescriptor& descriptor, PropertyDescriptor& oldDescriptor)
{
VM& vm = exec->vm();
+ auto map = m_butterfly->arrayStorage()->m_sparseMap.get();
if (descriptor.isDataDescriptor()) {
if (descriptor.value())
- entryInMap->set(vm, this, descriptor.value());
+ entryInMap->set(vm, map, descriptor.value());
else if (oldDescriptor.isAccessorDescriptor())
- entryInMap->set(vm, this, jsUndefined());
+ entryInMap->set(vm, map, jsUndefined());
entryInMap->attributes = descriptor.attributesOverridingCurrent(oldDescriptor) & ~Accessor;
return;
}
@@ -1745,7 +1746,7 @@
if (setter)
accessor->setSetter(vm, exec->lexicalGlobalObject(), setter);
- entryInMap->set(vm, this, accessor);
+ entryInMap->set(vm, map, accessor);
entryInMap->attributes = descriptor.attributesOverridingCurrent(oldDescriptor) & ~ReadOnly;
return;
}
Added: trunk/Source/_javascript_Core/tests/stress/sparse-array-entry-update-144067.js (0 => 183128)
--- trunk/Source/_javascript_Core/tests/stress/sparse-array-entry-update-144067.js (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/sparse-array-entry-update-144067.js 2015-04-22 20:44:32 UTC (rev 183128)
@@ -0,0 +1,56 @@
+/*
+ * Copyright (C) 2015 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+// Regression test for https://bugs.webkit.org/show_bug.cgi?id=144067.
+// This test aims to continually override the setter in a sparse array object, and
+// trigger GCs to give it a chance to collect the newly set entry value if the bug exists.
+// With the bug fixed, this test should not crash.
+
+var data = ""
+var sparseObj = {};
+
+for (var i = 0; i < 5; i++)
+ sparseObj[i] = i;
+
+function useMemoryToTriggerGCs() {
+ var arr = [];
+ var limit = DFGTrue() ? 10000 : 100;
+ for (var i = 0; i < limit; i++)
+ arr[i] = { a: "using" + i, b: "up" + i, c: "memory" + i };
+ return arr;
+}
+
+function foo(x) {
+ if (!x)
+ return;
+ data.textContent = sparseObj.__defineSetter__("16384", foo);
+ for (var i = 0; i < 10; i++)
+ sparseObj.__defineSetter__("" + (16384 + i), foo);
+ useMemoryToTriggerGCs();
+ sparseObj[16384] = x - 1;
+}
+
+var recursionDepthNeededToTriggerTheFailure = 100;
+foo(recursionDepthNeededToTriggerTheFailure);
