Diff
Modified: trunk/LayoutTests/ChangeLog (158874 => 158875)
--- trunk/LayoutTests/ChangeLog 2013-11-07 22:08:21 UTC (rev 158874)
+++ trunk/LayoutTests/ChangeLog 2013-11-07 22:19:58 UTC (rev 158875)
@@ -1,3 +1,17 @@
+2013-11-07 Oliver Hunt <[email protected]>
+
+ Reproducible crash when using Map (affects Web Inspector)
+ https://bugs.webkit.org/show_bug.cgi?id=123940
+
+ Reviewed by Geoffrey Garen.
+
+ Add testcases
+
+ * js/map-grow-with-holes-expected.txt: Added.
+ * js/map-grow-with-holes.html: Added.
+ * js/script-tests/map-grow-with-holes.js: Added.
+ (get map):
+
2013-11-07 Michał Pakuła vel Rutka <[email protected]>
Unreviewed EFL gardening
Added: trunk/LayoutTests/js/map-grow-with-holes-expected.txt (0 => 158875)
--- trunk/LayoutTests/js/map-grow-with-holes-expected.txt (rev 0)
+++ trunk/LayoutTests/js/map-grow-with-holes-expected.txt 2013-11-07 22:19:58 UTC (rev 158875)
@@ -0,0 +1,9 @@
+Tests to make sure we correctly handle map compaction when growing a Map with holes
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/js/map-grow-with-holes.html (0 => 158875)
--- trunk/LayoutTests/js/map-grow-with-holes.html (rev 0)
+++ trunk/LayoutTests/js/map-grow-with-holes.html 2013-11-07 22:19:58 UTC (rev 158875)
@@ -0,0 +1,10 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script src=""
+<script src=""
+</body>
+</html>
Added: trunk/LayoutTests/js/script-tests/map-grow-with-holes.js (0 => 158875)
--- trunk/LayoutTests/js/script-tests/map-grow-with-holes.js (rev 0)
+++ trunk/LayoutTests/js/script-tests/map-grow-with-holes.js 2013-11-07 22:19:58 UTC (rev 158875)
@@ -0,0 +1,148 @@
+description("Tests to make sure we correctly handle map compaction when growing a Map with holes");
+
+/*
+ * This test case clobbers the Map with a number of new properties with periodic deletes
+ * creating holes in the backing store.
+ *
+ * When the Map requires more space we trigger an expand and compact, which then
+ * rebuilds the key->index hashmap. This means that we need to ensure that the
+ * newly added key is not in hashmap during the compaction.
+ *
+ * We duplicate the test as string keyed entries use a distinct hashmap
+ * for key->index.
+ */
+
+var map = new Map();
+var flow = {};
+
+map.clear();
+map.clear();
+map.has(81);
+map.set(81, flow);
+map.has(83);
+map.set(83, flow);
+map.has(85);
+map.set(85, flow);
+map.has(87);
+map.set(87, flow);
+map.has(89);
+map.set(89, flow);
+map.has(91);
+map.set(91, flow);
+map.has(81);
+map.get(81);
+map.delete(81);
+map.has(82);
+map.has(83);
+map.get(83);
+map.delete(83);
+map.has(84);
+map.has(85);
+map.get(85);
+map.delete(85);
+map.has(86);
+map.has(87);
+map.get(87);
+map.delete(87);
+map.has(88);
+map.has(89);
+map.get(89);
+map.delete(89);
+map.has(90);
+map.has(91);
+map.get(91);
+map.delete(91);
+map.has(92);
+map.has(93);
+map.set(93, flow);
+map.has(95);
+map.set(95, flow);
+map.has(97);
+map.set(97, flow);
+map.has(99);
+map.set(99, flow);
+map.has(101);
+map.set(101, flow);
+map.has(103);
+map.set(103, flow);
+map.has(105);
+map.set(105, flow);
+map.has(93);
+map.get(93);
+map.delete(93);
+map.has(94);
+map.has(95);
+map.get(95);
+map.delete(95);
+map.has(96);
+map.has(97);
+map.get(97);
+
+
+map = new Map();
+var flow = {};
+
+map.clear();
+map.clear();
+map.has("81");
+map.set("81", flow);
+map.has("83");
+map.set("83", flow);
+map.has("85");
+map.set("85", flow);
+map.has("87");
+map.set("87", flow);
+map.has("89");
+map.set("89", flow);
+map.has("91");
+map.set("91", flow);
+map.has("81");
+map.get("81");
+map.delete("81");
+map.has("82");
+map.has("83");
+map.get("83");
+map.delete("83");
+map.has("84");
+map.has("85");
+map.get("85");
+map.delete("85");
+map.has("86");
+map.has("87");
+map.get("87");
+map.delete("87");
+map.has("88");
+map.has("89");
+map.get("89");
+map.delete("89");
+map.has("90");
+map.has("91");
+map.get("91");
+map.delete("91");
+map.has("92");
+map.has("93");
+map.set("93", flow);
+map.has("95");
+map.set("95", flow);
+map.has("97");
+map.set("97", flow);
+map.has("99");
+map.set("99", flow);
+map.has("101");
+map.set("101", flow);
+map.has("103");
+map.set("103", flow);
+map.has("105");
+map.set("105", flow);
+map.has("93");
+map.get("93");
+map.delete("93");
+map.has("94");
+map.has("95");
+map.get("95");
+map.delete("95");
+map.has("96");
+map.has("97");
+map.get("97");
+
+
Modified: trunk/Source/_javascript_Core/ChangeLog (158874 => 158875)
--- trunk/Source/_javascript_Core/ChangeLog 2013-11-07 22:08:21 UTC (rev 158874)
+++ trunk/Source/_javascript_Core/ChangeLog 2013-11-07 22:19:58 UTC (rev 158875)
@@ -1,3 +1,19 @@
+2013-11-07 Oliver Hunt <[email protected]>
+
+ Reproducible crash when using Map (affects Web Inspector)
+ https://bugs.webkit.org/show_bug.cgi?id=123940
+
+ Reviewed by Geoffrey Garen.
+
+ Trivial fix. Once again we get bitten by attempting to be clever when
+ growing while adding entries to indexing maps.
+
+ Now we simply do a find(), and then add() _after_ we've ensured there is
+ sufficient space in the MapData list.
+
+ * runtime/MapData.cpp:
+ (JSC::MapData::add):
+
2013-11-07 Mark Lam <[email protected]>
Cosmetic: rename xxxId to xxxID for ScriptId, SourceId, and BreakpointId.
Modified: trunk/Source/_javascript_Core/runtime/MapData.cpp (158874 => 158875)
--- trunk/Source/_javascript_Core/runtime/MapData.cpp 2013-11-07 22:08:21 UTC (rev 158874)
+++ trunk/Source/_javascript_Core/runtime/MapData.cpp 2013-11-07 22:19:58 UTC (rev 158875)
@@ -80,14 +80,15 @@
template <typename Map, typename Key> MapData::Entry* MapData::add(CallFrame* callFrame, Map& map, Key key, KeyType keyValue)
{
- typename Map::AddResult result = map.add(key, m_size);
- if (!result.isNewEntry)
- return &m_entries[result.iterator->value];
- if (!ensureSpaceForAppend(callFrame)) {
- map.remove(result.iterator);
+ typename Map::iterator location = map.find(key);
+ if (location != map.end())
+ return &m_entries[location->value];
+
+ if (!ensureSpaceForAppend(callFrame))
return 0;
- }
+ auto result = map.add(key, m_size);
+ RELEASE_ASSERT(result.isNewEntry);
Entry* entry = &m_entries[m_size++];
new (entry) Entry();
entry->key.set(callFrame->vm(), this, keyValue.value);
