Changeset 183128 in webkit

Timestamp:

Apr 22, 2015 1:44:32 PM (9 years ago)

Author:

mark.lam@apple.com

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

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• runtime/JSObject.cpp (modified) (3 diffs)
• tests/stress/sparse-array-entry-update-144067.js (added)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2015-04-22  Mark Lam  <mark.lam@apple.com>
+
+        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  <mark.lam@apple.com>
 

trunk/Source/JavaScriptCore/runtime/JSObject.cpp

@@ -585 +585 @@
         // 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);
     }
 
@@ -1718 +1718 @@
 {
     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;
@@ -1746 +1747 @@
             accessor->setSetter(vm, exec->lexicalGlobalObject(), setter);
 
-        entryInMap->set(vm, this, accessor);
+        entryInMap->set(vm, map, accessor);
         entryInMap->attributes = descriptor.attributesOverridingCurrent(oldDescriptor) & ~ReadOnly;
         return;