REGRESSION (r48582): Crash in StructureStubInfo::initPutByIdTransition when reloading...
authoroliver@apple.com <oliver@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 21 Sep 2009 18:59:56 +0000 (18:59 +0000)
committeroliver@apple.com <oliver@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 21 Sep 2009 18:59:56 +0000 (18:59 +0000)
https://bugs.webkit.org/show_bug.cgi?id=29599

Reviewed by Geoff Garen

It is unsafe to attempt to cache new property transitions on
dictionaries of any type.

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

JavaScriptCore/ChangeLog
JavaScriptCore/interpreter/Interpreter.cpp
JavaScriptCore/jit/JITStubs.cpp
LayoutTests/ChangeLog
LayoutTests/fast/js/resources/transition-cache-dictionary-crash.js [new file with mode: 0644]
LayoutTests/fast/js/transition-cache-dictionary-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/js/transition-cache-dictionary-crash.html [new file with mode: 0644]

index 2f30dec..1766555 100644 (file)
@@ -1,5 +1,20 @@
 2009-09-21  Oliver Hunt  <oliver@apple.com>
 
+        Reviewed by Geoff Garen.
+
+        REGRESSION (r48582): Crash in StructureStubInfo::initPutByIdTransition when reloading trac.webkit.org
+        https://bugs.webkit.org/show_bug.cgi?id=29599
+
+        It is unsafe to attempt to cache new property transitions on
+        dictionaries of any type.
+
+        * interpreter/Interpreter.cpp:
+        (JSC::Interpreter::tryCachePutByID):
+        * jit/JITStubs.cpp:
+        (JSC::JITThunks::tryCachePutByID):
+
+2009-09-21  Oliver Hunt  <oliver@apple.com>
+
         RS=Maciej Stachowiak.
 
         Re-land SNES fix with corrected assertion.
index 624832c..8a8fb3c 100644 (file)
@@ -988,6 +988,10 @@ NEVER_INLINE void Interpreter::tryCachePutByID(CallFrame* callFrame, CodeBlock*
 
     // Structure transition, cache transition info
     if (slot.type() == PutPropertySlot::NewProperty) {
+        if (structure->isDictionary()) {
+            vPC[0] = getOpcode(op_put_by_id_generic);
+            return;
+        }
         vPC[0] = getOpcode(op_put_by_id_transition);
         vPC[4] = structure->previousID();
         vPC[5] = structure;
index f197526..055a536 100644 (file)
@@ -695,7 +695,7 @@ NEVER_INLINE void JITThunks::tryCachePutByID(CallFrame* callFrame, CodeBlock* co
     // Structure transition, cache transition info
     if (slot.type() == PutPropertySlot::NewProperty) {
         StructureChain* prototypeChain = structure->prototypeChain(callFrame);
-        if (!prototypeChain->isCacheable()) {
+        if (!prototypeChain->isCacheable() || structure->isDictionary()) {
             ctiPatchCallByReturnAddress(codeBlock, returnAddress, FunctionPtr(cti_op_put_by_id_generic));
             return;
         }
index 107f00b..7c09293 100644 (file)
@@ -1,3 +1,17 @@
+2009-09-21  Oliver Hunt  <oliver@apple.com>
+
+        Reviewed by Geoff Garen.
+
+        REGRESSION (r48582): Crash in StructureStubInfo::initPutByIdTransition when reloading trac.webkit.org
+        https://bugs.webkit.org/show_bug.cgi?id=29599
+
+        Add test case for preventing property addition transition caching on dictionaries.
+
+        * fast/js/resources/transition-cache-dictionary-crash.js: Added.
+        (f):
+        * fast/js/transition-cache-dictionary-crash-expected.txt: Added.
+        * fast/js/transition-cache-dictionary-crash.html: Added.
+
 2009-09-20  Adam Barth  <abarth@webkit.org>
 
         Reviewed by Maciej Stachowiak.
diff --git a/LayoutTests/fast/js/resources/transition-cache-dictionary-crash.js b/LayoutTests/fast/js/resources/transition-cache-dictionary-crash.js
new file mode 100644 (file)
index 0000000..e83bce8
--- /dev/null
@@ -0,0 +1,19 @@
+description("Test to ensure we don't attempt to cache new property transitions on dictionary.  Passes if you don't crash.");
+
+var cacheableDictionary = {};
+for (var i = 0; i < 500; i++)
+    cacheableDictionary["a" + i] = i;
+
+function f(o) {
+    o.crash = "doom!";
+}
+f({});
+f(cacheableDictionary);
+f(cacheableDictionary);
+f(cacheableDictionary);
+f(cacheableDictionary);
+f(cacheableDictionary);
+f(cacheableDictionary);
+f(cacheableDictionary);
+f(cacheableDictionary);
+successfullyParsed = true;
diff --git a/LayoutTests/fast/js/transition-cache-dictionary-crash-expected.txt b/LayoutTests/fast/js/transition-cache-dictionary-crash-expected.txt
new file mode 100644 (file)
index 0000000..3f65917
--- /dev/null
@@ -0,0 +1,9 @@
+Test to ensure we don't attempt to cache new property transitions on dictionary. Passes if you don't crash.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/js/transition-cache-dictionary-crash.html b/LayoutTests/fast/js/transition-cache-dictionary-crash.html
new file mode 100644 (file)
index 0000000..d2293fa
--- /dev/null
@@ -0,0 +1,13 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<link rel="stylesheet" href="resources/js-test-style.css">
+<script src="resources/js-test-pre.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script src="resources/transition-cache-dictionary-crash.js"></script>
+<script src="resources/js-test-post.js"></script>
+</body>
+</html>