Map.forEach crashes on deleted values
authoroliver@apple.com <oliver@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Nov 2013 18:47:26 +0000 (18:47 +0000)
committeroliver@apple.com <oliver@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Nov 2013 18:47:26 +0000 (18:47 +0000)
https://bugs.webkit.org/show_bug.cgi?id=124017

Reviewed by Ryosuke Niwa.

Source/JavaScriptCore:

MapData iterator did not consider the case of the first entries
being holes.  To fix this I've refactored iteration so that we
can perform an initialisation increment on construction, whle
retaining the useful assertion in MapData::const_iterator::operator++

* runtime/MapData.h:
(JSC::MapData::const_iterator::operator++):
(JSC::MapData::const_iterator::internalIncrement):
(JSC::MapData::const_iterator::const_iterator):

LayoutTests:

Test case

* js/map-iterate-first-entry-is-a-hole-expected.txt: Added.
* js/map-iterate-first-entry-is-a-hole.html: Added.
* js/script-tests/map-iterate-first-entry-is-a-hole.js: Added.
(set map0):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@158929 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/js/map-iterate-first-entry-is-a-hole-expected.txt [new file with mode: 0644]
LayoutTests/js/map-iterate-first-entry-is-a-hole.html [new file with mode: 0644]
LayoutTests/js/script-tests/map-iterate-first-entry-is-a-hole.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/MapData.h

index 45ffc84..f824da4 100644 (file)
@@ -1,3 +1,17 @@
+2013-11-08  Oliver Hunt  <oliver@apple.com>
+
+        Map.forEach crashes on deleted values
+        https://bugs.webkit.org/show_bug.cgi?id=124017
+
+        Reviewed by Ryosuke Niwa.
+
+        Test case
+
+        * js/map-iterate-first-entry-is-a-hole-expected.txt: Added.
+        * js/map-iterate-first-entry-is-a-hole.html: Added.
+        * js/script-tests/map-iterate-first-entry-is-a-hole.js: Added.
+        (set map0):
+
 2013-10-30  Jer Noble  <jer.noble@apple.com>
 
         [MSE] Bring SourceBuffer.append up to the most recent spec.
diff --git a/LayoutTests/js/map-iterate-first-entry-is-a-hole-expected.txt b/LayoutTests/js/map-iterate-first-entry-is-a-hole-expected.txt
new file mode 100644 (file)
index 0000000..b3b1cf5
--- /dev/null
@@ -0,0 +1,9 @@
+Tests to make sure we correctly handle iterating a map when the first entry is a hole
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/js/map-iterate-first-entry-is-a-hole.html b/LayoutTests/js/map-iterate-first-entry-is-a-hole.html
new file mode 100644 (file)
index 0000000..205fd09
--- /dev/null
@@ -0,0 +1,10 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src="../resources/js-test-pre.js"></script>
+</head>
+<body>
+<script src="script-tests/map-iterate-first-entry-is-a-hole.js"></script>
+<script src="../resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/js/script-tests/map-iterate-first-entry-is-a-hole.js b/LayoutTests/js/script-tests/map-iterate-first-entry-is-a-hole.js
new file mode 100644 (file)
index 0000000..015df2d
--- /dev/null
@@ -0,0 +1,7 @@
+description("Tests to make sure we correctly handle iterating a map when the first entry is a hole");
+var map0= new Map;
+map0.set(125, {});
+map0.delete(125);
+map0.forEach(function(node) {
+    print(node);
+});
index faee97c..633da08 100644 (file)
@@ -1,3 +1,20 @@
+2013-11-08  Oliver Hunt  <oliver@apple.com>
+
+        Map.forEach crashes on deleted values
+        https://bugs.webkit.org/show_bug.cgi?id=124017
+
+        Reviewed by Ryosuke Niwa.
+
+        MapData iterator did not consider the case of the first entries
+        being holes.  To fix this I've refactored iteration so that we
+        can perform an initialisation increment on construction, whle
+        retaining the useful assertion in MapData::const_iterator::operator++
+
+        * runtime/MapData.h:
+        (JSC::MapData::const_iterator::operator++):
+        (JSC::MapData::const_iterator::internalIncrement):
+        (JSC::MapData::const_iterator::const_iterator):
+
 2013-11-08  Julien Brianceau  <jbriance@cisco.com>
 
         REGRESSION(r158883): Fix crashes for ARM architecture.
index 8b96408..4a30eaa 100644 (file)
@@ -44,7 +44,7 @@ public:
         const WTF::KeyValuePair<JSValue, JSValue> operator*() const;
         JSValue key() const { ASSERT(!atEnd()); return m_mapData->m_entries[m_index].key.get(); }
         JSValue value() const { ASSERT(!atEnd()); return m_mapData->m_entries[m_index].value.get(); }
-        void operator++();
+        void operator++() { ASSERT(!atEnd()); internalIncrement(); }
         static const_iterator end(const MapData*);
         bool operator!=(const const_iterator& other);
         bool operator==(const const_iterator& other);
@@ -56,6 +56,7 @@ public:
         // 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();
         const MapData* m_mapData;
         int32_t m_index;
     };
@@ -166,9 +167,8 @@ ALWAYS_INLINE MapData::KeyType::KeyType(JSValue v)
         value = jsNumber(i);
 }
 
-ALWAYS_INLINE void MapData::const_iterator::operator++()
+ALWAYS_INLINE void MapData::const_iterator::internalIncrement()
 {
-    ASSERT(!atEnd());
     Entry* entries = m_mapData->m_entries;
     size_t index = m_index + 1;
     size_t end = m_mapData->m_size;
@@ -179,9 +179,9 @@ ALWAYS_INLINE void MapData::const_iterator::operator++()
 
 ALWAYS_INLINE MapData::const_iterator::const_iterator(const MapData* mapData)
     : m_mapData(mapData)
-    , m_index(0)
+    , m_index(-1)
 {
-    m_mapData->m_iteratorCount++;
+    internalIncrement();
 }
 
 ALWAYS_INLINE MapData::const_iterator::~const_iterator()