REGRESSION (r181458): Heap use-after-free in JSSetIterator destructor
https://bugs.webkit.org/show_bug.cgi?id=142696
Reviewed and tweaked by Geoffrey Garen.
Source/JavaScriptCore:
Before r142556, JSSetIterator::destroy was not defined.
So accidentally MapData::const_iterator in JSSet was never destroyed.
But it had non trivial destructor, decrementing MapData->m_iteratorCount.
After r142556, JSSetIterator::destroy works.
It correctly destruct MapData::const_iterator and m_iteratorCount partially works.
But JSSetIterator::~JSSetIterator requires owned JSSet since it mutates MapData->m_iteratorCount.
It is guaranteed that JSSet is live since JSSetIterator has a reference to JSSet
and marks it in visitChildren (WriteBarrier<Unknown>).
However, the order of destructions is not guaranteed in GC-ed system.
Consider the following case,
allocate JSSet and subsequently allocate JSSetIterator.
And they resides in the separated MarkedBlock, <1> and <2>.
JSSet<1> <- JSSetIterator<2>
And after that, when performing GC, Marker decides that the above 2 objects are not marked.
And Marker also decides MarkedBlocks <1> and <2> can be sweeped.
First Sweeper sweep <1>, destruct JSSet<1> and free MarkedBlock<1>.
Second Sweeper sweep <2>, attempt to destruct JSSetIterator<2>.
However, JSSetIterator<2>'s destructor,
JSSetIterator::~JSSetIterator requires live JSSet<1>, it causes use-after-free.
In this patch, we introduce WeakGCMap into JSMap/JSSet to track live iterators.
When packing the removed elements in JSSet/JSMap, we apply the change to all live
iterators tracked by WeakGCMap.
WeakGCMap can only track JSCell since they are managed by GC.
So we drop JSSet/JSMap C++ style iterators. Instead of C++ style iterator, this patch
introduces JS style iterator signatures into C++ class IteratorData.
If we need to iterate over JSMap/JSSet, use JSSetIterator/JSMapIterator instead of using
IteratorData directly.
Patch by Yusuke Suzuki <utatane.tea@gmail.com> on 2015-03-24
* runtime/JSMap.cpp:
(JSC::JSMap::destroy):
* runtime/JSMap.h:
(JSC::JSMap::JSMap):
(JSC::JSMap::begin): Deleted.
(JSC::JSMap::end): Deleted.
* runtime/JSMapIterator.cpp:
(JSC::JSMapIterator::destroy):
* runtime/JSMapIterator.h:
(JSC::JSMapIterator::next):
(JSC::JSMapIterator::nextKeyValue):
(JSC::JSMapIterator::iteratorData):
(JSC::JSMapIterator::JSMapIterator):
* runtime/JSSet.cpp:
(JSC::JSSet::destroy):
* runtime/JSSet.h:
(JSC::JSSet::JSSet):
(JSC::JSSet::begin): Deleted.
(JSC::JSSet::end): Deleted.
* runtime/JSSetIterator.cpp:
(JSC::JSSetIterator::destroy):
* runtime/JSSetIterator.h:
(JSC::JSSetIterator::next):
(JSC::JSSetIterator::iteratorData):
(JSC::JSSetIterator::JSSetIterator):
* runtime/MapData.h:
(JSC::MapDataImpl::IteratorData::finish):
(JSC::MapDataImpl::IteratorData::isFinished):
(JSC::MapDataImpl::shouldPack):
(JSC::JSIterator>::MapDataImpl):
(JSC::JSIterator>::KeyType::KeyType):
(JSC::JSIterator>::IteratorData::IteratorData):
(JSC::JSIterator>::IteratorData::next):
(JSC::JSIterator>::IteratorData::ensureSlot):
(JSC::JSIterator>::IteratorData::applyMapDataPatch):
(JSC::JSIterator>::IteratorData::refreshCursor):
(JSC::MapDataImpl::const_iterator::key): Deleted.
(JSC::MapDataImpl::const_iterator::value): Deleted.
(JSC::MapDataImpl::const_iterator::operator++): Deleted.
(JSC::MapDataImpl::const_iterator::finish): Deleted.
(JSC::MapDataImpl::const_iterator::atEnd): Deleted.
(JSC::MapDataImpl::begin): Deleted.
(JSC::MapDataImpl::end): Deleted.
(JSC::MapDataImpl<Entry>::MapDataImpl): Deleted.
(JSC::MapDataImpl<Entry>::clear): Deleted.
(JSC::MapDataImpl<Entry>::KeyType::KeyType): Deleted.
(JSC::MapDataImpl<Entry>::const_iterator::internalIncrement): Deleted.
(JSC::MapDataImpl<Entry>::const_iterator::ensureSlot): Deleted.
(JSC::MapDataImpl<Entry>::const_iterator::const_iterator): Deleted.
(JSC::MapDataImpl<Entry>::const_iterator::~const_iterator): Deleted.
(JSC::MapDataImpl<Entry>::const_iterator::operator): Deleted.
(JSC::=): Deleted.
* runtime/MapDataInlines.h:
(JSC::JSIterator>::clear):
(JSC::JSIterator>::find):
(JSC::JSIterator>::contains):
(JSC::JSIterator>::add):
(JSC::JSIterator>::set):
(JSC::JSIterator>::get):
(JSC::JSIterator>::remove):
(JSC::JSIterator>::replaceAndPackBackingStore):
(JSC::JSIterator>::replaceBackingStore):
(JSC::JSIterator>::ensureSpaceForAppend):
(JSC::JSIterator>::visitChildren):
(JSC::JSIterator>::copyBackingStore):
(JSC::JSIterator>::applyMapDataPatch):
(JSC::MapDataImpl<Entry>::find): Deleted.
(JSC::MapDataImpl<Entry>::contains): Deleted.
(JSC::MapDataImpl<Entry>::add): Deleted.
(JSC::MapDataImpl<Entry>::set): Deleted.
(JSC::MapDataImpl<Entry>::get): Deleted.
(JSC::MapDataImpl<Entry>::remove): Deleted.
(JSC::MapDataImpl<Entry>::replaceAndPackBackingStore): Deleted.
(JSC::MapDataImpl<Entry>::replaceBackingStore): Deleted.
(JSC::MapDataImpl<Entry>::ensureSpaceForAppend): Deleted.
(JSC::MapDataImpl<Entry>::visitChildren): Deleted.
(JSC::MapDataImpl<Entry>::copyBackingStore): Deleted.
* runtime/MapPrototype.cpp:
(JSC::mapProtoFuncForEach):
* runtime/SetPrototype.cpp:
(JSC::setProtoFuncForEach):
* runtime/WeakGCMap.h:
(JSC::WeakGCMap::forEach):
* tests/stress/modify-map-during-iteration.js: Added.
(testValue):
(identityPairs):
(.set if):
(var):
(set map):
* tests/stress/modify-set-during-iteration.js: Added.
(testValue):
(set forEach):
(set delete):
Source/WebCore:
Use JSSetIterator/JSMapIterator to iterate over JSSet and JSMap.
Patch by Yusuke Suzuki <utatane.tea@gmail.com> on 2015-03-24
* ForwardingHeaders/runtime/JSMapIterator.h: Added.
* ForwardingHeaders/runtime/JSSetIterator.h: Added.
* bindings/js/SerializedScriptValue.cpp:
(WebCore::CloneSerializer::serialize):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@181922 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 30f18d3..dc756c8 100644
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,140 @@
+2015-03-24 Yusuke Suzuki <utatane.tea@gmail.com>
+
+ REGRESSION (r181458): Heap use-after-free in JSSetIterator destructor
+ https://bugs.webkit.org/show_bug.cgi?id=142696
+
+ Reviewed and tweaked by Geoffrey Garen.
+
+ Before r142556, JSSetIterator::destroy was not defined.
+ So accidentally MapData::const_iterator in JSSet was never destroyed.
+ But it had non trivial destructor, decrementing MapData->m_iteratorCount.
+
+ After r142556, JSSetIterator::destroy works.
+ It correctly destruct MapData::const_iterator and m_iteratorCount partially works.
+ But JSSetIterator::~JSSetIterator requires owned JSSet since it mutates MapData->m_iteratorCount.
+
+ It is guaranteed that JSSet is live since JSSetIterator has a reference to JSSet
+ and marks it in visitChildren (WriteBarrier<Unknown>).
+ However, the order of destructions is not guaranteed in GC-ed system.
+
+ Consider the following case,
+ allocate JSSet and subsequently allocate JSSetIterator.
+ And they resides in the separated MarkedBlock, <1> and <2>.
+
+ JSSet<1> <- JSSetIterator<2>
+
+ And after that, when performing GC, Marker decides that the above 2 objects are not marked.
+ And Marker also decides MarkedBlocks <1> and <2> can be sweeped.
+
+ First Sweeper sweep <1>, destruct JSSet<1> and free MarkedBlock<1>.
+ Second Sweeper sweep <2>, attempt to destruct JSSetIterator<2>.
+ However, JSSetIterator<2>'s destructor,
+ JSSetIterator::~JSSetIterator requires live JSSet<1>, it causes use-after-free.
+
+ In this patch, we introduce WeakGCMap into JSMap/JSSet to track live iterators.
+ When packing the removed elements in JSSet/JSMap, we apply the change to all live
+ iterators tracked by WeakGCMap.
+
+ WeakGCMap can only track JSCell since they are managed by GC.
+ So we drop JSSet/JSMap C++ style iterators. Instead of C++ style iterator, this patch
+ introduces JS style iterator signatures into C++ class IteratorData.
+ If we need to iterate over JSMap/JSSet, use JSSetIterator/JSMapIterator instead of using
+ IteratorData directly.
+
+ * runtime/JSMap.cpp:
+ (JSC::JSMap::destroy):
+ * runtime/JSMap.h:
+ (JSC::JSMap::JSMap):
+ (JSC::JSMap::begin): Deleted.
+ (JSC::JSMap::end): Deleted.
+ * runtime/JSMapIterator.cpp:
+ (JSC::JSMapIterator::destroy):
+ * runtime/JSMapIterator.h:
+ (JSC::JSMapIterator::next):
+ (JSC::JSMapIterator::nextKeyValue):
+ (JSC::JSMapIterator::iteratorData):
+ (JSC::JSMapIterator::JSMapIterator):
+ * runtime/JSSet.cpp:
+ (JSC::JSSet::destroy):
+ * runtime/JSSet.h:
+ (JSC::JSSet::JSSet):
+ (JSC::JSSet::begin): Deleted.
+ (JSC::JSSet::end): Deleted.
+ * runtime/JSSetIterator.cpp:
+ (JSC::JSSetIterator::destroy):
+ * runtime/JSSetIterator.h:
+ (JSC::JSSetIterator::next):
+ (JSC::JSSetIterator::iteratorData):
+ (JSC::JSSetIterator::JSSetIterator):
+ * runtime/MapData.h:
+ (JSC::MapDataImpl::IteratorData::finish):
+ (JSC::MapDataImpl::IteratorData::isFinished):
+ (JSC::MapDataImpl::shouldPack):
+ (JSC::JSIterator>::MapDataImpl):
+ (JSC::JSIterator>::KeyType::KeyType):
+ (JSC::JSIterator>::IteratorData::IteratorData):
+ (JSC::JSIterator>::IteratorData::next):
+ (JSC::JSIterator>::IteratorData::ensureSlot):
+ (JSC::JSIterator>::IteratorData::applyMapDataPatch):
+ (JSC::JSIterator>::IteratorData::refreshCursor):
+ (JSC::MapDataImpl::const_iterator::key): Deleted.
+ (JSC::MapDataImpl::const_iterator::value): Deleted.
+ (JSC::MapDataImpl::const_iterator::operator++): Deleted.
+ (JSC::MapDataImpl::const_iterator::finish): Deleted.
+ (JSC::MapDataImpl::const_iterator::atEnd): Deleted.
+ (JSC::MapDataImpl::begin): Deleted.
+ (JSC::MapDataImpl::end): Deleted.
+ (JSC::MapDataImpl<Entry>::MapDataImpl): Deleted.
+ (JSC::MapDataImpl<Entry>::clear): Deleted.
+ (JSC::MapDataImpl<Entry>::KeyType::KeyType): Deleted.
+ (JSC::MapDataImpl<Entry>::const_iterator::internalIncrement): Deleted.
+ (JSC::MapDataImpl<Entry>::const_iterator::ensureSlot): Deleted.
+ (JSC::MapDataImpl<Entry>::const_iterator::const_iterator): Deleted.
+ (JSC::MapDataImpl<Entry>::const_iterator::~const_iterator): Deleted.
+ (JSC::MapDataImpl<Entry>::const_iterator::operator): Deleted.
+ (JSC::=): Deleted.
+ * runtime/MapDataInlines.h:
+ (JSC::JSIterator>::clear):
+ (JSC::JSIterator>::find):
+ (JSC::JSIterator>::contains):
+ (JSC::JSIterator>::add):
+ (JSC::JSIterator>::set):
+ (JSC::JSIterator>::get):
+ (JSC::JSIterator>::remove):
+ (JSC::JSIterator>::replaceAndPackBackingStore):
+ (JSC::JSIterator>::replaceBackingStore):
+ (JSC::JSIterator>::ensureSpaceForAppend):
+ (JSC::JSIterator>::visitChildren):
+ (JSC::JSIterator>::copyBackingStore):
+ (JSC::JSIterator>::applyMapDataPatch):
+ (JSC::MapDataImpl<Entry>::find): Deleted.
+ (JSC::MapDataImpl<Entry>::contains): Deleted.
+ (JSC::MapDataImpl<Entry>::add): Deleted.
+ (JSC::MapDataImpl<Entry>::set): Deleted.
+ (JSC::MapDataImpl<Entry>::get): Deleted.
+ (JSC::MapDataImpl<Entry>::remove): Deleted.
+ (JSC::MapDataImpl<Entry>::replaceAndPackBackingStore): Deleted.
+ (JSC::MapDataImpl<Entry>::replaceBackingStore): Deleted.
+ (JSC::MapDataImpl<Entry>::ensureSpaceForAppend): Deleted.
+ (JSC::MapDataImpl<Entry>::visitChildren): Deleted.
+ (JSC::MapDataImpl<Entry>::copyBackingStore): Deleted.
+ * runtime/MapPrototype.cpp:
+ (JSC::mapProtoFuncForEach):
+ * runtime/SetPrototype.cpp:
+ (JSC::setProtoFuncForEach):
+ * runtime/WeakGCMap.h:
+ (JSC::WeakGCMap::forEach):
+ * tests/stress/modify-map-during-iteration.js: Added.
+ (testValue):
+ (identityPairs):
+ (.set if):
+ (var):
+ (set map):
+ * tests/stress/modify-set-during-iteration.js: Added.
+ (testValue):
+ (set forEach):
+ (set delete):
+
2015-03-24 Mark Lam <mark.lam@apple.com>
The ExecutionTimeLimit test should use its own JSGlobalContextRef.
diff --git a/Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj b/Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj
index c23122d..0a77bea 100644
--- a/Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj
+++ b/Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj
@@ -1305,7 +1305,7 @@
A74DEF93182D991400522C22 /* MapIteratorPrototype.cpp in Sources */ = {isa = PBXBuildFile; fileRef = A74DEF8D182D991400522C22 /* MapIteratorPrototype.cpp */; };
A74DEF94182D991400522C22 /* MapIteratorPrototype.h in Headers */ = {isa = PBXBuildFile; fileRef = A74DEF8E182D991400522C22 /* MapIteratorPrototype.h */; };
A74DEF95182D991400522C22 /* JSMapIterator.cpp in Sources */ = {isa = PBXBuildFile; fileRef = A74DEF8F182D991400522C22 /* JSMapIterator.cpp */; };
- A74DEF96182D991400522C22 /* JSMapIterator.h in Headers */ = {isa = PBXBuildFile; fileRef = A74DEF90182D991400522C22 /* JSMapIterator.h */; };
+ A74DEF96182D991400522C22 /* JSMapIterator.h in Headers */ = {isa = PBXBuildFile; fileRef = A74DEF90182D991400522C22 /* JSMapIterator.h */; settings = {ATTRIBUTES = (Private, ); }; };
A75706DE118A2BCF0057F88F /* JITArithmetic32_64.cpp in Sources */ = {isa = PBXBuildFile; fileRef = A75706DD118A2BCF0057F88F /* JITArithmetic32_64.cpp */; };
A75EE9B218AAB7E200AAD043 /* BuiltinNames.h in Headers */ = {isa = PBXBuildFile; fileRef = A75EE9B018AAB7E200AAD043 /* BuiltinNames.h */; };
A76140CD182982CB00750624 /* ArgumentsIteratorConstructor.cpp in Sources */ = {isa = PBXBuildFile; fileRef = A76140C7182982CB00750624 /* ArgumentsIteratorConstructor.cpp */; };
@@ -1352,7 +1352,7 @@
A790DD6D182F499700588807 /* SetIteratorPrototype.cpp in Sources */ = {isa = PBXBuildFile; fileRef = A790DD67182F499700588807 /* SetIteratorPrototype.cpp */; };
A790DD6E182F499700588807 /* SetIteratorPrototype.h in Headers */ = {isa = PBXBuildFile; fileRef = A790DD68182F499700588807 /* SetIteratorPrototype.h */; };
A790DD6F182F499700588807 /* JSSetIterator.cpp in Sources */ = {isa = PBXBuildFile; fileRef = A790DD69182F499700588807 /* JSSetIterator.cpp */; };
- A790DD70182F499700588807 /* JSSetIterator.h in Headers */ = {isa = PBXBuildFile; fileRef = A790DD6A182F499700588807 /* JSSetIterator.h */; };
+ A790DD70182F499700588807 /* JSSetIterator.h in Headers */ = {isa = PBXBuildFile; fileRef = A790DD6A182F499700588807 /* JSSetIterator.h */; settings = {ATTRIBUTES = (Private, ); }; };
A7986D5717A0BB1E00A95DD0 /* DFGEdgeUsesStructure.h in Headers */ = {isa = PBXBuildFile; fileRef = A7986D5617A0BB1E00A95DD0 /* DFGEdgeUsesStructure.h */; settings = {ATTRIBUTES = (Private, ); }; };
A7A4AE0817973B26005612B1 /* MacroAssemblerX86Common.cpp in Sources */ = {isa = PBXBuildFile; fileRef = A7A4AE0717973B26005612B1 /* MacroAssemblerX86Common.cpp */; };
A7A4AE1017973B4D005612B1 /* JITStubsX86Common.h in Headers */ = {isa = PBXBuildFile; fileRef = A7A4AE0C17973B4D005612B1 /* JITStubsX86Common.h */; };
diff --git a/Source/JavaScriptCore/runtime/JSMap.cpp b/Source/JavaScriptCore/runtime/JSMap.cpp
index 441006d..352648b 100644
--- a/Source/JavaScriptCore/runtime/JSMap.cpp
+++ b/Source/JavaScriptCore/runtime/JSMap.cpp
@@ -27,6 +27,7 @@
#include "JSMap.h"
#include "JSCJSValueInlines.h"
+#include "JSMapIterator.h"
#include "MapDataInlines.h"
#include "SlotVisitorInlines.h"
#include "StructureInlines.h"
@@ -37,7 +38,8 @@
void JSMap::destroy(JSCell* cell)
{
- jsCast<JSMap*>(cell)->~JSMap();
+ JSMap* thisObject = jsCast<JSMap*>(cell);
+ thisObject->JSMap::~JSMap();
}
void JSMap::visitChildren(JSCell* cell, SlotVisitor& visitor)
diff --git a/Source/JavaScriptCore/runtime/JSMap.h b/Source/JavaScriptCore/runtime/JSMap.h
index 4afd0bb..837f7e3 100644
--- a/Source/JavaScriptCore/runtime/JSMap.h
+++ b/Source/JavaScriptCore/runtime/JSMap.h
@@ -32,10 +32,14 @@
namespace JSC {
+class JSMapIterator;
+
class JSMap : public JSDestructibleObject {
public:
typedef JSDestructibleObject Base;
+ friend class JSMapIterator;
+
// Our marking functions expect Entry to maintain this layout, and have all
// fields be WriteBarrier<Unknown>
class Entry {
@@ -82,7 +86,7 @@
}
};
- typedef MapDataImpl<Entry> MapData;
+ typedef MapDataImpl<Entry, JSMapIterator> MapData;
DECLARE_EXPORT_INFO;
@@ -103,16 +107,6 @@
return create(exec->vm(), structure);
}
- typedef MapData::const_iterator const_iterator;
-
- const_iterator begin() const
- {
- return m_mapData.begin();
- }
- const_iterator end() const
- {
- return m_mapData.end();
- }
bool has(ExecState*, JSValue);
size_t size(ExecState*);
JSValue get(ExecState*, JSValue);
@@ -123,7 +117,7 @@
private:
JSMap(VM& vm, Structure* structure)
: Base(vm, structure)
- , m_mapData()
+ , m_mapData(vm)
{
}
diff --git a/Source/JavaScriptCore/runtime/JSMapIterator.cpp b/Source/JavaScriptCore/runtime/JSMapIterator.cpp
index e9d7d42..885362b 100644
--- a/Source/JavaScriptCore/runtime/JSMapIterator.cpp
+++ b/Source/JavaScriptCore/runtime/JSMapIterator.cpp
@@ -29,6 +29,7 @@
#include "JSCJSValueInlines.h"
#include "JSCellInlines.h"
#include "JSMap.h"
+#include "MapDataInlines.h"
#include "SlotVisitorInlines.h"
#include "StructureInlines.h"
@@ -44,7 +45,8 @@
void JSMapIterator::destroy(JSCell* cell)
{
- jsCast<JSMapIterator*>(cell)->~JSMapIterator();
+ JSMapIterator* thisObject = jsCast<JSMapIterator*>(cell);
+ thisObject->JSMapIterator::~JSMapIterator();
}
void JSMapIterator::visitChildren(JSCell* cell, SlotVisitor& visitor)
diff --git a/Source/JavaScriptCore/runtime/JSMapIterator.h b/Source/JavaScriptCore/runtime/JSMapIterator.h
index ab87b3a..cc2ae86 100644
--- a/Source/JavaScriptCore/runtime/JSMapIterator.h
+++ b/Source/JavaScriptCore/runtime/JSMapIterator.h
@@ -57,16 +57,27 @@
bool next(CallFrame* callFrame, JSValue& value)
{
- if (!m_iterator.ensureSlot())
+ WTF::KeyValuePair<JSValue, JSValue> pair;
+ if (!m_iterator.next(pair))
return false;
if (m_kind == MapIterateValue)
- value = m_iterator.value();
+ value = pair.value;
else if (m_kind == MapIterateKey)
- value = m_iterator.key();
+ value = pair.key;
else
- value = createPair(callFrame, m_iterator.key(), m_iterator.value());
- ++m_iterator;
+ value = createPair(callFrame, pair.key, pair.value);
+ return true;
+ }
+
+ bool nextKeyValue(JSValue& key, JSValue& value)
+ {
+ WTF::KeyValuePair<JSValue, JSValue> pair;
+ if (!m_iterator.next(pair))
+ return false;
+
+ key = pair.key;
+ value = pair.value;
return true;
}
@@ -79,21 +90,26 @@
JSValue iteratedValue() const { return m_map.get(); }
JSMapIterator* clone(ExecState*);
+ JSMap::MapData::IteratorData* iteratorData()
+ {
+ return &m_iterator;
+ }
+
private:
JSMapIterator(VM& vm, Structure* structure, JSMap* iteratedObject, MapIterationKind kind)
: Base(vm, structure)
- , m_iterator(iteratedObject->begin())
+ , m_iterator(iteratedObject->m_mapData.createIteratorData(this))
, m_kind(kind)
{
}
static void destroy(JSCell*);
- void finishCreation(VM&, JSMap*);
+ JS_EXPORT_PRIVATE void finishCreation(VM&, JSMap*);
JSValue createPair(CallFrame*, JSValue, JSValue);
static void visitChildren(JSCell*, SlotVisitor&);
WriteBarrier<JSMap> m_map;
- JSMap::const_iterator m_iterator;
+ JSMap::MapData::IteratorData m_iterator;
MapIterationKind m_kind;
};
diff --git a/Source/JavaScriptCore/runtime/JSSet.cpp b/Source/JavaScriptCore/runtime/JSSet.cpp
index 8fc5637..d875345 100644
--- a/Source/JavaScriptCore/runtime/JSSet.cpp
+++ b/Source/JavaScriptCore/runtime/JSSet.cpp
@@ -27,6 +27,7 @@
#include "JSSet.h"
#include "JSCJSValueInlines.h"
+#include "JSSetIterator.h"
#include "MapDataInlines.h"
#include "SlotVisitorInlines.h"
#include "StructureInlines.h"
@@ -37,7 +38,8 @@
void JSSet::destroy(JSCell* cell)
{
- jsCast<JSSet*>(cell)->~JSSet();
+ JSSet* thisObject = jsCast<JSSet*>(cell);
+ thisObject->JSSet::~JSSet();
}
void JSSet::visitChildren(JSCell* cell, SlotVisitor& visitor)
diff --git a/Source/JavaScriptCore/runtime/JSSet.h b/Source/JavaScriptCore/runtime/JSSet.h
index f052abe..e55362e 100644
--- a/Source/JavaScriptCore/runtime/JSSet.h
+++ b/Source/JavaScriptCore/runtime/JSSet.h
@@ -32,10 +32,14 @@
namespace JSC {
+class JSSetIterator;
+
class JSSet : public JSDestructibleObject {
public:
typedef JSDestructibleObject Base;
+ friend class JSSetIterator;
+
// Our marking functions expect Entry to maintain this layout, and have all
// fields be WriteBarrier<Unknown>
class Entry {
@@ -78,7 +82,7 @@
}
};
- typedef MapDataImpl<Entry> SetData;
+ typedef MapDataImpl<Entry, JSSetIterator> SetData;
DECLARE_EXPORT_INFO;
@@ -99,16 +103,6 @@
return create(exec->vm(), structure);
}
- typedef SetData::const_iterator const_iterator;
-
- const_iterator begin() const
- {
- return m_setData.begin();
- }
- const_iterator end() const
- {
- return m_setData.end();
- }
bool has(ExecState*, JSValue);
size_t size(ExecState*);
JS_EXPORT_PRIVATE void add(ExecState*, JSValue);
@@ -118,7 +112,7 @@
private:
JSSet(VM& vm, Structure* structure)
: Base(vm, structure)
- , m_setData()
+ , m_setData(vm)
{
}
diff --git a/Source/JavaScriptCore/runtime/JSSetIterator.cpp b/Source/JavaScriptCore/runtime/JSSetIterator.cpp
index 5ecac12..3a0ddbe 100644
--- a/Source/JavaScriptCore/runtime/JSSetIterator.cpp
+++ b/Source/JavaScriptCore/runtime/JSSetIterator.cpp
@@ -29,6 +29,7 @@
#include "JSCJSValueInlines.h"
#include "JSCellInlines.h"
#include "JSSet.h"
+#include "MapDataInlines.h"
#include "SlotVisitorInlines.h"
#include "StructureInlines.h"
@@ -52,7 +53,8 @@
void JSSetIterator::destroy(JSCell* cell)
{
- jsCast<JSSetIterator*>(cell)->~JSSetIterator();
+ JSSetIterator* thisObject = jsCast<JSSetIterator*>(cell);
+ thisObject->JSSetIterator::~JSSetIterator();
}
JSValue JSSetIterator::createPair(CallFrame* callFrame, JSValue key, JSValue value)
diff --git a/Source/JavaScriptCore/runtime/JSSetIterator.h b/Source/JavaScriptCore/runtime/JSSetIterator.h
index b063c51..6dad6b1 100644
--- a/Source/JavaScriptCore/runtime/JSSetIterator.h
+++ b/Source/JavaScriptCore/runtime/JSSetIterator.h
@@ -58,13 +58,13 @@
bool next(CallFrame* callFrame, JSValue& value)
{
- if (!m_iterator.ensureSlot())
+ WTF::KeyValuePair<JSValue, JSValue> pair;
+ if (!m_iterator.next(pair))
return false;
if (m_kind == SetIterateValue || m_kind == SetIterateKey)
- value = m_iterator.key();
+ value = pair.key;
else
- value = createPair(callFrame, m_iterator.key(), m_iterator.key());
- ++m_iterator;
+ value = createPair(callFrame, pair.key, pair.key);
return true;
}
@@ -77,21 +77,26 @@
JSValue iteratedValue() const { return m_set.get(); }
JSSetIterator* clone(ExecState*);
+ JSSet::SetData::IteratorData* iteratorData()
+ {
+ return &m_iterator;
+ }
+
private:
JSSetIterator(VM& vm, Structure* structure, JSSet* iteratedObject, SetIterationKind kind)
: Base(vm, structure)
- , m_iterator(iteratedObject->begin())
+ , m_iterator(iteratedObject->m_setData.createIteratorData(this))
, m_kind(kind)
{
}
static void destroy(JSCell*);
- void finishCreation(VM&, JSSet*);
- JSValue createPair(CallFrame*, JSValue, JSValue);
+ JS_EXPORT_PRIVATE void finishCreation(VM&, JSSet*);
+ JS_EXPORT_PRIVATE JSValue createPair(CallFrame*, JSValue, JSValue);
static void visitChildren(JSCell*, SlotVisitor&);
WriteBarrier<JSSet> m_set;
- JSSet::const_iterator m_iterator;
+ JSSet::SetData::IteratorData m_iterator;
SetIterationKind m_kind;
};
diff --git a/Source/JavaScriptCore/runtime/MapData.h b/Source/JavaScriptCore/runtime/MapData.h
index 6f3bac2..a8af1ab 100644
--- a/Source/JavaScriptCore/runtime/MapData.h
+++ b/Source/JavaScriptCore/runtime/MapData.h
@@ -27,45 +27,57 @@
#define MapData_h
#include "JSCell.h"
+#include "WeakGCMapInlines.h"
#include <wtf/HashFunctions.h>
#include <wtf/HashMap.h>
#include <wtf/MathExtras.h>
+#include <wtf/RefCounted.h>
+#include <wtf/RefPtr.h>
+#include <wtf/Vector.h>
namespace JSC {
class ExecState;
+class VM;
-template<typename Entry>
+template<typename Entry, typename JSIterator>
class MapDataImpl {
public:
enum : int32_t {
minimumMapSize = 8
};
- struct const_iterator {
- const_iterator(const MapDataImpl*);
- ~const_iterator();
- const WTF::KeyValuePair<JSValue, JSValue> operator*() const;
- JSValue key() const { RELEASE_ASSERT(!atEnd()); return m_mapData->m_entries[m_index].key().get(); }
- JSValue value() const { RELEASE_ASSERT(!atEnd()); return m_mapData->m_entries[m_index].value().get(); }
- void operator++() { ASSERT(!atEnd()); internalIncrement(); }
- static const_iterator end(const MapDataImpl*);
- bool operator!=(const const_iterator& other);
- bool operator==(const const_iterator& other);
- void finish() { m_index = std::numeric_limits<int32_t>::max(); }
+ class IteratorData {
+ public:
+ friend class MapDataImpl;
- bool ensureSlot();
+ IteratorData(const MapDataImpl*);
+ bool next(WTF::KeyValuePair<JSValue, JSValue>&);
+
+ void didRemoveEntry(int32_t index)
+ {
+ if (m_index <= index)
+ return;
+ --m_index;
+ }
+
+ void didRemoveAllEntries()
+ {
+ m_index = 0;
+ }
+
+ void finish()
+ {
+ m_index = -1;
+ }
private:
- // This is a bit gnarly. We use an index of -1 to indicate the
- // "end()" iterator. By casting to unsigned we can immediately
- // test if both iterators are at the end of their iteration.
- // We need this in order to keep the common case (eg. iter != end())
- // fast.
- bool atEnd() const { return static_cast<size_t>(m_index) >= static_cast<size_t>(m_mapData->m_size); }
- void internalIncrement();
+ bool ensureSlot() const;
+ bool isFinished() const { return m_index == -1; }
+ int32_t refreshCursor() const;
+
const MapDataImpl* m_mapData;
- int32_t m_index;
+ mutable int32_t m_index;
};
struct KeyType {
@@ -74,7 +86,7 @@
JSValue value;
};
- MapDataImpl();
+ MapDataImpl(VM&);
void set(ExecState*, JSCell* owner, KeyType, JSValue);
JSValue get(ExecState*, KeyType);
@@ -82,8 +94,7 @@
bool contains(ExecState*, KeyType);
size_t size(ExecState*) const { return m_size - m_deletedCount; }
- const_iterator begin() const { return const_iterator(this); }
- const_iterator end() const { return const_iterator::end(this); }
+ IteratorData createIteratorData(JSIterator*);
void clear();
@@ -104,7 +115,7 @@
ALWAYS_INLINE Entry* add(ExecState*, JSCell* owner, KeyType);
template <typename Map, typename Key> ALWAYS_INLINE Entry* add(ExecState*, JSCell* owner, Map&, Key, KeyType);
- ALWAYS_INLINE bool shouldPack() const { return m_deletedCount && !m_iteratorCount; }
+ ALWAYS_INLINE bool shouldPack() const { return m_deletedCount; }
CheckedBoolean ensureSpaceForAppend(ExecState*, JSCell* owner);
ALWAYS_INLINE void replaceAndPackBackingStore(Entry* destination, int32_t newSize);
@@ -117,35 +128,22 @@
int32_t m_capacity;
int32_t m_size;
int32_t m_deletedCount;
- mutable int32_t m_iteratorCount;
Entry* m_entries;
+ WeakGCMap<JSIterator*, JSIterator> m_iterators;
};
-template<typename Entry>
-ALWAYS_INLINE MapDataImpl<Entry>::MapDataImpl()
+template<typename Entry, typename JSIterator>
+ALWAYS_INLINE MapDataImpl<Entry, JSIterator>::MapDataImpl(VM& vm)
: m_capacity(0)
, m_size(0)
, m_deletedCount(0)
- , m_iteratorCount(0)
, m_entries(nullptr)
+ , m_iterators(vm)
{
}
-template<typename Entry>
-ALWAYS_INLINE void MapDataImpl<Entry>::clear()
-{
- m_cellKeyedTable.clear();
- m_valueKeyedTable.clear();
- m_stringKeyedTable.clear();
- m_symbolKeyedTable.clear();
- m_capacity = 0;
- m_size = 0;
- m_deletedCount = 0;
- m_entries = nullptr;
-}
-
-template<typename Entry>
-ALWAYS_INLINE MapDataImpl<Entry>::KeyType::KeyType(JSValue v)
+template<typename Entry, typename JSIterator>
+ALWAYS_INLINE MapDataImpl<Entry, JSIterator>::KeyType::KeyType(JSValue v)
{
if (!v.isDouble()) {
value = v;
@@ -164,73 +162,45 @@
value = jsNumber(i);
}
-template<typename Entry>
-ALWAYS_INLINE void MapDataImpl<Entry>::const_iterator::internalIncrement()
-{
- Entry* entries = m_mapData->m_entries;
- size_t index = m_index + 1;
- size_t end = m_mapData->m_size;
- while (index < end && !entries[index].key())
- index++;
- m_index = index;
-}
-
-template<typename Entry>
-ALWAYS_INLINE bool MapDataImpl<Entry>::const_iterator::ensureSlot()
-{
- // When an iterator exists outside of host cost it is possible for
- // the containing map to be modified
- Entry* entries = m_mapData->m_entries;
- size_t index = m_index;
- size_t end = m_mapData->m_size;
- if (index < end && entries[index].key())
- return true;
- internalIncrement();
- return static_cast<size_t>(m_index) < end;
-}
-
-template<typename Entry>
-ALWAYS_INLINE MapDataImpl<Entry>::const_iterator::const_iterator(const MapDataImpl<Entry>* mapData)
+template<typename Entry, typename JSIterator>
+ALWAYS_INLINE MapDataImpl<Entry, JSIterator>::IteratorData::IteratorData(const MapDataImpl<Entry, JSIterator>* mapData)
: m_mapData(mapData)
- , m_index(-1)
+ , m_index(0)
{
- internalIncrement();
}
-template<typename Entry>
-ALWAYS_INLINE MapDataImpl<Entry>::const_iterator::~const_iterator()
+template<typename Entry, typename JSIterator>
+ALWAYS_INLINE bool MapDataImpl<Entry, JSIterator>::IteratorData::next(WTF::KeyValuePair<JSValue, JSValue>& pair)
{
- m_mapData->m_iteratorCount--;
-}
-
-template<typename Entry>
-ALWAYS_INLINE const WTF::KeyValuePair<JSValue, JSValue> MapDataImpl<Entry>::const_iterator::operator*() const
-{
- Entry* entry = &m_mapData->m_entries[m_index];
- return WTF::KeyValuePair<JSValue, JSValue>(entry->key().get(), entry->value().get());
-}
-
-template<typename Entry>
-ALWAYS_INLINE auto MapDataImpl<Entry>::const_iterator::end(const MapDataImpl<Entry>* mapData) -> const_iterator
-{
- const_iterator result(mapData);
- result.m_index = -1;
- return result;
-}
-
-template<typename Entry>
-ALWAYS_INLINE bool MapDataImpl<Entry>::const_iterator::operator!=(const const_iterator& other)
-{
- ASSERT(other.m_mapData == m_mapData);
- if (atEnd() && other.atEnd())
+ if (!ensureSlot())
return false;
- return m_index != other.m_index;
+ Entry* entry = &m_mapData->m_entries[m_index];
+ pair = WTF::KeyValuePair<JSValue, JSValue>(entry->key().get(), entry->value().get());
+ m_index += 1;
+ return true;
}
-template<typename Entry>
-ALWAYS_INLINE bool MapDataImpl<Entry>::const_iterator::operator==(const const_iterator& other)
+// This is a bit gnarly. We use an index of -1 to indicate the
+// finished state. By casting to unsigned we can immediately
+// test if both iterators are at the end of their iteration.
+template<typename Entry, typename JSIterator>
+ALWAYS_INLINE bool MapDataImpl<Entry, JSIterator>::IteratorData::ensureSlot() const
{
- return !(*this != other);
+ int32_t index = refreshCursor();
+ return static_cast<size_t>(index) < static_cast<size_t>(m_mapData->m_size);
+}
+
+template<typename Entry, typename JSIterator>
+ALWAYS_INLINE int32_t MapDataImpl<Entry, JSIterator>::IteratorData::refreshCursor() const
+{
+ if (isFinished())
+ return m_index;
+
+ Entry* entries = m_mapData->m_entries;
+ size_t end = m_mapData->m_size;
+ while (static_cast<size_t>(m_index) < end && !entries[m_index].key())
+ m_index++;
+ return m_index;
}
}
diff --git a/Source/JavaScriptCore/runtime/MapDataInlines.h b/Source/JavaScriptCore/runtime/MapDataInlines.h
index fb2c53a..478308992 100644
--- a/Source/JavaScriptCore/runtime/MapDataInlines.h
+++ b/Source/JavaScriptCore/runtime/MapDataInlines.h
@@ -37,8 +37,24 @@
namespace JSC {
-template<typename Entry>
-inline Entry* MapDataImpl<Entry>::find(ExecState* exec, KeyType key)
+template<typename Entry, typename JSIterator>
+inline void MapDataImpl<Entry, JSIterator>::clear()
+{
+ m_cellKeyedTable.clear();
+ m_valueKeyedTable.clear();
+ m_stringKeyedTable.clear();
+ m_symbolKeyedTable.clear();
+ m_capacity = 0;
+ m_size = 0;
+ m_deletedCount = 0;
+ m_entries = nullptr;
+ m_iterators.forEach([](JSIterator* iterator, JSIterator*) {
+ iterator->iteratorData()->didRemoveAllEntries();
+ });
+}
+
+template<typename Entry, typename JSIterator>
+inline Entry* MapDataImpl<Entry, JSIterator>::find(ExecState* exec, KeyType key)
{
if (key.value.isString()) {
auto iter = m_stringKeyedTable.find(asString(key.value)->value(exec).impl());
@@ -65,15 +81,15 @@
return &m_entries[iter->value];
}
-template<typename Entry>
-inline bool MapDataImpl<Entry>::contains(ExecState* exec, KeyType key)
+template<typename Entry, typename JSIterator>
+inline bool MapDataImpl<Entry, JSIterator>::contains(ExecState* exec, KeyType key)
{
return find(exec, key);
}
-template<typename Entry>
+template<typename Entry, typename JSIterator>
template <typename Map, typename Key>
-inline Entry* MapDataImpl<Entry>::add(ExecState* exec, JSCell* owner, Map& map, Key key, KeyType keyValue)
+inline Entry* MapDataImpl<Entry, JSIterator>::add(ExecState* exec, JSCell* owner, Map& map, Key key, KeyType keyValue)
{
typename Map::iterator location = map.find(key);
if (location != map.end())
@@ -90,8 +106,8 @@
return entry;
}
-template<typename Entry>
-inline void MapDataImpl<Entry>::set(ExecState* exec, JSCell* owner, KeyType key, JSValue value)
+template<typename Entry, typename JSIterator>
+inline void MapDataImpl<Entry, JSIterator>::set(ExecState* exec, JSCell* owner, KeyType key, JSValue value)
{
Entry* location = add(exec, owner, key);
if (!location)
@@ -99,8 +115,8 @@
location->setValue(exec->vm(), owner, value);
}
-template<typename Entry>
-inline Entry* MapDataImpl<Entry>::add(ExecState* exec, JSCell* owner, KeyType key)
+template<typename Entry, typename JSIterator>
+inline Entry* MapDataImpl<Entry, JSIterator>::add(ExecState* exec, JSCell* owner, KeyType key)
{
if (key.value.isString())
return add(exec, owner, m_stringKeyedTable, asString(key.value)->value(exec).impl(), key);
@@ -111,16 +127,16 @@
return add(exec, owner, m_valueKeyedTable, JSValue::encode(key.value), key);
}
-template<typename Entry>
-inline JSValue MapDataImpl<Entry>::get(ExecState* exec, KeyType key)
+template<typename Entry, typename JSIterator>
+inline JSValue MapDataImpl<Entry, JSIterator>::get(ExecState* exec, KeyType key)
{
if (Entry* entry = find(exec, key))
return entry->value().get();
return JSValue();
}
-template<typename Entry>
-inline bool MapDataImpl<Entry>::remove(ExecState* exec, KeyType key)
+template<typename Entry, typename JSIterator>
+inline bool MapDataImpl<Entry, JSIterator>::remove(ExecState* exec, KeyType key)
{
int32_t location;
if (key.value.isString()) {
@@ -153,16 +169,20 @@
return true;
}
-template<typename Entry>
-inline void MapDataImpl<Entry>::replaceAndPackBackingStore(Entry* destination, int32_t newCapacity)
+template<typename Entry, typename JSIterator>
+inline void MapDataImpl<Entry, JSIterator>::replaceAndPackBackingStore(Entry* destination, int32_t newCapacity)
{
ASSERT(shouldPack());
int32_t newEnd = 0;
RELEASE_ASSERT(newCapacity > 0);
for (int32_t i = 0; i < m_size; i++) {
Entry& entry = m_entries[i];
- if (!entry.key())
+ if (!entry.key()) {
+ m_iterators.forEach([i](JSIterator* iterator, JSIterator*) {
+ iterator->iteratorData()->didRemoveEntry(i);
+ });
continue;
+ }
ASSERT(newEnd < newCapacity);
destination[newEnd] = entry;
@@ -191,8 +211,8 @@
m_entries = destination;
}
-template<typename Entry>
-inline void MapDataImpl<Entry>::replaceBackingStore(Entry* destination, int32_t newCapacity)
+template<typename Entry, typename JSIterator>
+inline void MapDataImpl<Entry, JSIterator>::replaceBackingStore(Entry* destination, int32_t newCapacity)
{
ASSERT(!shouldPack());
RELEASE_ASSERT(newCapacity > 0);
@@ -202,8 +222,8 @@
m_entries = destination;
}
-template<typename Entry>
-inline CheckedBoolean MapDataImpl<Entry>::ensureSpaceForAppend(ExecState* exec, JSCell* owner)
+template<typename Entry, typename JSIterator>
+inline CheckedBoolean MapDataImpl<Entry, JSIterator>::ensureSpaceForAppend(ExecState* exec, JSCell* owner)
{
if (m_capacity > m_size)
return true;
@@ -224,8 +244,8 @@
return true;
}
-template<typename Entry>
-inline void MapDataImpl<Entry>::visitChildren(JSCell* owner, SlotVisitor& visitor)
+template<typename Entry, typename JSIterator>
+inline void MapDataImpl<Entry, JSIterator>::visitChildren(JSCell* owner, SlotVisitor& visitor)
{
Entry* entries = m_entries;
if (!entries)
@@ -244,8 +264,8 @@
visitor.copyLater(owner, MapBackingStoreCopyToken, entries, capacityInBytes());
}
-template<typename Entry>
-inline void MapDataImpl<Entry>::copyBackingStore(CopyVisitor& visitor, CopyToken token)
+template<typename Entry, typename JSIterator>
+inline void MapDataImpl<Entry, JSIterator>::copyBackingStore(CopyVisitor& visitor, CopyToken token)
{
if (token == MapBackingStoreCopyToken && visitor.checkIfShouldCopy(m_entries)) {
Entry* oldEntries = m_entries;
@@ -258,4 +278,11 @@
}
}
+template<typename Entry, typename JSIterator>
+inline auto MapDataImpl<Entry, JSIterator>::createIteratorData(JSIterator* iterator) -> IteratorData
+{
+ m_iterators.set(iterator, iterator);
+ return IteratorData(this);
+}
+
}
diff --git a/Source/JavaScriptCore/runtime/MapPrototype.cpp b/Source/JavaScriptCore/runtime/MapPrototype.cpp
index 6d7a11d..88df583 100644
--- a/Source/JavaScriptCore/runtime/MapPrototype.cpp
+++ b/Source/JavaScriptCore/runtime/MapPrototype.cpp
@@ -119,22 +119,26 @@
return JSValue::encode(throwTypeError(callFrame, WTF::ASCIILiteral("Map.prototype.forEach called without callback")));
JSValue thisValue = callFrame->argument(1);
VM* vm = &callFrame->vm();
+ JSMapIterator* iterator = JSMapIterator::create(*vm, callFrame->callee()->globalObject()->mapIteratorStructure(), map, MapIterateKeyValue);
+ JSValue key, value;
if (callType == CallTypeJS) {
JSFunction* function = jsCast<JSFunction*>(callBack);
CachedCall cachedCall(callFrame, function, 2);
- for (auto ptr = map->begin(), end = map->end(); ptr != end && !vm->exception(); ++ptr) {
+ while (iterator->nextKeyValue(key, value) && !vm->exception()) {
cachedCall.setThis(thisValue);
- cachedCall.setArgument(0, ptr.value());
- cachedCall.setArgument(1, ptr.key());
+ cachedCall.setArgument(0, value);
+ cachedCall.setArgument(1, key);
cachedCall.call();
}
+ iterator->finish();
} else {
- for (auto ptr = map->begin(), end = map->end(); ptr != end && !vm->exception(); ++ptr) {
+ while (iterator->nextKeyValue(key, value) && !vm->exception()) {
MarkedArgumentBuffer args;
- args.append(ptr.value());
- args.append(ptr.key());
+ args.append(value);
+ args.append(key);
JSC::call(callFrame, callBack, callType, callData, thisValue, args);
}
+ iterator->finish();
}
return JSValue::encode(jsUndefined());
}
diff --git a/Source/JavaScriptCore/runtime/SetPrototype.cpp b/Source/JavaScriptCore/runtime/SetPrototype.cpp
index 6a6c49b..40066ae 100644
--- a/Source/JavaScriptCore/runtime/SetPrototype.cpp
+++ b/Source/JavaScriptCore/runtime/SetPrototype.cpp
@@ -128,20 +128,24 @@
return JSValue::encode(throwTypeError(callFrame, WTF::ASCIILiteral("Set.prototype.forEach called without callback")));
JSValue thisValue = callFrame->argument(1);
VM* vm = &callFrame->vm();
+ JSSetIterator* iterator = JSSetIterator::create(*vm, callFrame->callee()->globalObject()->setIteratorStructure(), set, SetIterateKey);
+ JSValue key;
if (callType == CallTypeJS) {
JSFunction* function = jsCast<JSFunction*>(callBack);
CachedCall cachedCall(callFrame, function, 1);
- for (auto ptr = set->begin(), end = set->end(); ptr != end && !vm->exception(); ++ptr) {
+ while (iterator->next(callFrame, key) && !vm->exception()) {
cachedCall.setThis(thisValue);
- cachedCall.setArgument(0, ptr.key());
+ cachedCall.setArgument(0, key);
cachedCall.call();
}
+ iterator->finish();
} else {
- for (auto ptr = set->begin(), end = set->end(); ptr != end && !vm->exception(); ++ptr) {
+ while (iterator->next(callFrame, key) && !vm->exception()) {
MarkedArgumentBuffer args;
- args.append(ptr.key());
+ args.append(key);
JSC::call(callFrame, callBack, callType, callData, thisValue, args);
}
+ iterator->finish();
}
return JSValue::encode(jsUndefined());
}
diff --git a/Source/JavaScriptCore/runtime/WeakGCMap.h b/Source/JavaScriptCore/runtime/WeakGCMap.h
index fd1fa6e..16d3d44 100644
--- a/Source/JavaScriptCore/runtime/WeakGCMap.h
+++ b/Source/JavaScriptCore/runtime/WeakGCMap.h
@@ -70,6 +70,17 @@
m_map.clear();
}
+ bool isEmpty() const
+ {
+ const_iterator it = m_map.begin();
+ const_iterator end = m_map.end();
+ while (it != end) {
+ if (it->value)
+ return true;
+ }
+ return false;
+ }
+
iterator find(const KeyType& key)
{
iterator it = m_map.find(key);
@@ -84,6 +95,15 @@
return const_cast<WeakGCMap*>(this)->find(key);
}
+ template<typename Functor>
+ void forEach(Functor functor)
+ {
+ for (auto& pair : m_map) {
+ if (pair.value)
+ functor(pair.key, pair.value.get());
+ }
+ }
+
bool contains(const KeyType& key) const
{
return find(key) != m_map.end();
diff --git a/Source/JavaScriptCore/tests/stress/modify-map-during-iteration.js b/Source/JavaScriptCore/tests/stress/modify-map-during-iteration.js
new file mode 100644
index 0000000..78e438e
--- /dev/null
+++ b/Source/JavaScriptCore/tests/stress/modify-map-during-iteration.js
@@ -0,0 +1,76 @@
+
+function testValue(value, expected) {
+ if (value !== expected)
+ throw new Error("bad value: expected:(" + expected + "),actual:(" + value +").");
+}
+
+function identityPairs(array) {
+ return array.map(function (i) { return [i, i]; });
+}
+
+var map = new Map(identityPairs([0]));
+var counter = 0;
+for (var [elm, _] of map) {
+ testValue(elm, counter);
+ map.set(elm + 1, elm + 1);
+ if (elm > 10000) {
+ map.clear();
+ }
+ ++counter;
+}
+testValue(counter, 10002);
+
+var map = new Map(identityPairs([0, 1, 2, 3]));
+var counter = 0;
+for (var [elm, _] of map) {
+ testValue(elm, counter);
+ map.clear();
+ ++counter;
+}
+testValue(counter, 1);
+
+var map = new Map(identityPairs([0, 1, 2, 3]));
+var exp = [0, 2, 3];
+var counter = 0;
+for (var [elm, _] of map) {
+ testValue(elm, exp[counter]);
+ map.delete(counter + 1);
+ ++counter;
+}
+testValue(counter, 3);
+
+var map = new Map(identityPairs([0, 1, 2, 3]));
+var iter = map.keys();
+var iter2 = map.keys();
+testValue(iter2.next().value, 0);
+
+// Consume all output of iter.
+for (var elm of iter);
+
+testValue(iter.next().done, true);
+testValue(iter.next().value, undefined);
+
+map.clear();
+map.set(1, 1).set(2, 2).set(3, 3);
+
+testValue(iter.next().done, true);
+testValue(iter.next().value, undefined);
+testValue(iter2.next().value, 1);
+testValue(iter2.next().value, 2);
+testValue(iter2.next().value, 3);
+
+var map = new Map();
+map.set(1, 1);
+map.delete(1);
+map.forEach(function (i) {
+ throw new Error("unreeachable.");
+});
+
+var map = new Map();
+var iter = map[Symbol.iterator]();
+map.set(1, 1);
+map.delete(1);
+for (var [elm, _] of map) {
+ throw new Error("unreeachable.");
+}
+
diff --git a/Source/JavaScriptCore/tests/stress/modify-set-during-iteration.js b/Source/JavaScriptCore/tests/stress/modify-set-during-iteration.js
new file mode 100644
index 0000000..da4d20a
--- /dev/null
+++ b/Source/JavaScriptCore/tests/stress/modify-set-during-iteration.js
@@ -0,0 +1,72 @@
+
+function testValue(value, expected) {
+ if (value !== expected)
+ throw new Error("bad value: expected:(" + expected + "),actual:(" + value +").");
+}
+
+var set = new Set([0]);
+var counter = 0;
+for (var elm of set) {
+ testValue(elm, counter);
+ set.add(elm + 1);
+ if (elm > 10000) {
+ set.clear();
+ }
+ ++counter;
+}
+testValue(counter, 10002);
+
+var set = new Set([0, 1, 2, 3]);
+var counter = 0;
+for (var elm of set) {
+ testValue(elm, counter);
+ set.clear();
+ ++counter;
+}
+testValue(counter, 1);
+
+var set = new Set([0, 1, 2, 3]);
+var exp = [0, 2, 3];
+var counter = 0;
+for (var elm of set) {
+ testValue(elm, exp[counter]);
+ set.delete(counter + 1);
+ ++counter;
+}
+testValue(counter, 3);
+
+var set = new Set([0, 1, 2, 3]);
+var iter = set[Symbol.iterator]();
+var iter2 = set[Symbol.iterator]();
+testValue(iter2.next().value, 0);
+
+// Consume all output of iter.
+for (var elm of iter);
+
+testValue(iter.next().done, true);
+testValue(iter.next().value, undefined);
+
+set.clear();
+set.add(1).add(2).add(3);
+
+testValue(iter.next().done, true);
+testValue(iter.next().value, undefined);
+testValue(iter2.next().value, 1);
+testValue(iter2.next().value, 2);
+testValue(iter2.next().value, 3);
+
+var set = new Set();
+set.add(1);
+set.delete(1);
+set.forEach(function (i) {
+ throw new Error("unreeachable.");
+});
+
+var set = new Set();
+var iter = set[Symbol.iterator]();
+set.add(1);
+set.delete(1);
+for (var elm of iter) {
+ throw new Error("unreeachable.");
+}
+
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 2d7bc56..e74e20b 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,17 @@
+2015-03-24 Yusuke Suzuki <utatane.tea@gmail.com>
+
+ REGRESSION (r181458): Heap use-after-free in JSSetIterator destructor
+ https://bugs.webkit.org/show_bug.cgi?id=142696
+
+ Reviewed and tweaked by Geoffrey Garen.
+
+ Use JSSetIterator/JSMapIterator to iterate over JSSet and JSMap.
+
+ * ForwardingHeaders/runtime/JSMapIterator.h: Added.
+ * ForwardingHeaders/runtime/JSSetIterator.h: Added.
+ * bindings/js/SerializedScriptValue.cpp:
+ (WebCore::CloneSerializer::serialize):
+
2015-03-24 Dan Bernstein <mitz@apple.com>
Tried to fix the iOS Simulator build.
diff --git a/Source/WebCore/ForwardingHeaders/runtime/JSMapIterator.h b/Source/WebCore/ForwardingHeaders/runtime/JSMapIterator.h
new file mode 100644
index 0000000..735ccbd
--- /dev/null
+++ b/Source/WebCore/ForwardingHeaders/runtime/JSMapIterator.h
@@ -0,0 +1,4 @@
+#ifndef WebCore_FWD_JSMapIterator_h
+#define WebCore_FWD_JSMapIterator_h
+#include <JavaScriptCore/JSMapIterator.h>
+#endif
diff --git a/Source/WebCore/ForwardingHeaders/runtime/JSSetIterator.h b/Source/WebCore/ForwardingHeaders/runtime/JSSetIterator.h
new file mode 100644
index 0000000..0cb43ee
--- /dev/null
+++ b/Source/WebCore/ForwardingHeaders/runtime/JSSetIterator.h
@@ -0,0 +1,4 @@
+#ifndef WebCore_FWD_JSSetIterator_h
+#define WebCore_FWD_JSSetIterator_h
+#include <JavaScriptCore/JSSetIterator.h>
+#endif
diff --git a/Source/WebCore/bindings/js/SerializedScriptValue.cpp b/Source/WebCore/bindings/js/SerializedScriptValue.cpp
index 757ecd9..ba2603c 100644
--- a/Source/WebCore/bindings/js/SerializedScriptValue.cpp
+++ b/Source/WebCore/bindings/js/SerializedScriptValue.cpp
@@ -61,7 +61,9 @@
#include <runtime/JSCInlines.h>
#include <runtime/JSDataView.h>
#include <runtime/JSMap.h>
+#include <runtime/JSMapIterator.h>
#include <runtime/JSSet.h>
+#include <runtime/JSSetIterator.h>
#include <runtime/JSTypedArrays.h>
#include <runtime/MapData.h>
#include <runtime/MapDataInlines.h>
@@ -1220,10 +1222,8 @@
Vector<uint32_t, 16> lengthStack;
Vector<PropertyNameArray, 16> propertyStack;
Vector<JSObject*, 32> inputObjectStack;
- Vector<JSMap*, 4> mapStack;
- Vector<JSMap::const_iterator, 4> mapIteratorStack;
- Vector<JSSet*, 4> setStack;
- Vector<JSSet::const_iterator, 4> setIteratorStack;
+ Vector<JSMapIterator*, 4> mapIteratorStack;
+ Vector<JSSetIterator*, 4> setIteratorStack;
Vector<JSValue, 4> mapIteratorValueStack;
Vector<WalkerState, 16> stateStack;
WalkerState state = StateUnknown;
@@ -1356,19 +1356,19 @@
JSMap* inMap = jsCast<JSMap*>(inValue);
if (!startMap(inMap))
break;
+ JSMapIterator* iterator = JSMapIterator::create(m_exec->vm(), m_exec->lexicalGlobalObject()->mapIteratorStructure(), inMap, MapIterateKeyValue);
m_gcBuffer.append(inMap);
- mapStack.append(inMap);
- mapIteratorStack.append(inMap->begin());
+ m_gcBuffer.append(iterator);
+ mapIteratorStack.append(iterator);
inputObjectStack.append(inMap);
goto mapDataStartVisitEntry;
}
mapDataStartVisitEntry:
case MapDataStartVisitEntry: {
- JSMap::const_iterator& ptr = mapIteratorStack.last();
- JSMap* map = mapStack.last();
- if (ptr == map->end()) {
+ JSMapIterator* iterator = mapIteratorStack.last();
+ JSValue key, value;
+ if (!iterator->nextKeyValue(key, value)) {
mapIteratorStack.removeLast();
- mapStack.removeLast();
JSObject* object = inputObjectStack.last();
ASSERT(jsDynamicCast<JSMap*>(object));
propertyStack.append(PropertyNameArray(m_exec));
@@ -1377,9 +1377,9 @@
indexStack.append(0);
goto objectStartVisitMember;
}
- inValue = ptr.key();
- m_gcBuffer.append(ptr.value());
- mapIteratorValueStack.append(ptr.value());
+ inValue = key;
+ m_gcBuffer.append(value);
+ mapIteratorValueStack.append(value);
stateStack.append(MapDataEndVisitKey);
goto stateUnknown;
}
@@ -1390,8 +1390,6 @@
goto stateUnknown;
}
case MapDataEndVisitValue: {
- if (mapIteratorStack.last() != mapStack.last()->end())
- ++mapIteratorStack.last();
goto mapDataStartVisitEntry;
}
@@ -1402,19 +1400,19 @@
JSSet* inSet = jsCast<JSSet*>(inValue);
if (!startSet(inSet))
break;
+ JSSetIterator* iterator = JSSetIterator::create(m_exec->vm(), m_exec->lexicalGlobalObject()->setIteratorStructure(), inSet, SetIterateKey);
m_gcBuffer.append(inSet);
- setStack.append(inSet);
- setIteratorStack.append(inSet->begin());
+ m_gcBuffer.append(iterator);
+ setIteratorStack.append(iterator);
inputObjectStack.append(inSet);
goto setDataStartVisitEntry;
}
setDataStartVisitEntry:
case SetDataStartVisitEntry: {
- JSSet::const_iterator& ptr = setIteratorStack.last();
- JSSet* set = setStack.last();
- if (ptr == set->end()) {
+ JSSetIterator* iterator = setIteratorStack.last();
+ JSValue key;
+ if (!iterator->next(m_exec, key)) {
setIteratorStack.removeLast();
- setStack.removeLast();
JSObject* object = inputObjectStack.last();
ASSERT(jsDynamicCast<JSSet*>(object));
propertyStack.append(PropertyNameArray(m_exec));
@@ -1423,13 +1421,11 @@
indexStack.append(0);
goto objectStartVisitMember;
}
- inValue = ptr.key();
+ inValue = key;
stateStack.append(SetDataEndVisitKey);
goto stateUnknown;
}
case SetDataEndVisitKey: {
- if (setIteratorStack.last() != setStack.last()->end())
- ++setIteratorStack.last();
goto setDataStartVisitEntry;
}
