Title: [120121] trunk
Revision
120121
Author
[email protected]
Date
2012-06-12 14:15:43 -0700 (Tue, 12 Jun 2012)

Log Message

REGRESSION (r119779): _javascript_ TypeError: 'undefined' is not an object
https://bugs.webkit.org/show_bug.cgi?id=88783
<rdar://problem/11640299>

Source/_javascript_Core: 

Reviewed by Geoffrey Garen.
        
If you don't keep alive the base of an object access over the various checks
you do for the prototype chain, you're going to have a bad time.

* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::handleGetById):

LayoutTests: 

Reviewed by Geoffrey Garen.

* fast/js/dfg-proto-access-inline-osr-exit-expected.txt: Added.
* fast/js/dfg-proto-access-inline-osr-exit.html: Added.
* fast/js/script-tests/dfg-proto-access-inline-osr-exit.js: Added.
(foo):
(Thingy):

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (120120 => 120121)


--- trunk/LayoutTests/ChangeLog	2012-06-12 21:09:18 UTC (rev 120120)
+++ trunk/LayoutTests/ChangeLog	2012-06-12 21:15:43 UTC (rev 120121)
@@ -1,3 +1,17 @@
+2012-06-12  Filip Pizlo  <[email protected]>
+
+        REGRESSION (r119779): _javascript_ TypeError: 'undefined' is not an object
+        https://bugs.webkit.org/show_bug.cgi?id=88783
+        <rdar://problem/11640299>
+
+        Reviewed by Geoffrey Garen.
+
+        * fast/js/dfg-proto-access-inline-osr-exit-expected.txt: Added.
+        * fast/js/dfg-proto-access-inline-osr-exit.html: Added.
+        * fast/js/script-tests/dfg-proto-access-inline-osr-exit.js: Added.
+        (foo):
+        (Thingy):
+
 2012-06-12  Igor Oliveira  <[email protected]>
 
         Apply animations and transitions for first-letter element

Added: trunk/LayoutTests/fast/js/dfg-proto-access-inline-osr-exit-expected.txt (0 => 120121)


--- trunk/LayoutTests/fast/js/dfg-proto-access-inline-osr-exit-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/js/dfg-proto-access-inline-osr-exit-expected.txt	2012-06-12 21:15:43 UTC (rev 120121)
@@ -0,0 +1,209 @@
+Tests what happens when we OSR exit on an inlined prototype access due to a change in the prototype chain.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS foo({g:new Thingy()}) is 42
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/fast/js/dfg-proto-access-inline-osr-exit.html (0 => 120121)


--- trunk/LayoutTests/fast/js/dfg-proto-access-inline-osr-exit.html	                        (rev 0)
+++ trunk/LayoutTests/fast/js/dfg-proto-access-inline-osr-exit.html	2012-06-12 21:15:43 UTC (rev 120121)
@@ -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/fast/js/script-tests/dfg-proto-access-inline-osr-exit.js (0 => 120121)


--- trunk/LayoutTests/fast/js/script-tests/dfg-proto-access-inline-osr-exit.js	                        (rev 0)
+++ trunk/LayoutTests/fast/js/script-tests/dfg-proto-access-inline-osr-exit.js	2012-06-12 21:15:43 UTC (rev 120121)
@@ -0,0 +1,21 @@
+description(
+"Tests what happens when we OSR exit on an inlined prototype access due to a change in the prototype chain."
+);
+
+function foo(o) {
+    return o.g.f;
+}
+
+function Thingy() {
+}
+
+var myProto = {f:42};
+
+Thingy.prototype = myProto;
+
+for (var i = 0; i < 200; ++i) {
+    if (i == 150)
+        myProto.g = 67;
+    shouldBe("foo({g:new Thingy()})", "42");
+}
+

Modified: trunk/Source/_javascript_Core/ChangeLog (120120 => 120121)


--- trunk/Source/_javascript_Core/ChangeLog	2012-06-12 21:09:18 UTC (rev 120120)
+++ trunk/Source/_javascript_Core/ChangeLog	2012-06-12 21:15:43 UTC (rev 120121)
@@ -1,3 +1,17 @@
+2012-06-12  Filip Pizlo  <[email protected]>
+
+        REGRESSION (r119779): _javascript_ TypeError: 'undefined' is not an object
+        https://bugs.webkit.org/show_bug.cgi?id=88783
+        <rdar://problem/11640299>
+
+        Reviewed by Geoffrey Garen.
+        
+        If you don't keep alive the base of an object access over the various checks
+        you do for the prototype chain, you're going to have a bad time.
+
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::handleGetById):
+
 2012-06-12  Hojong Han  <[email protected]>
 
         Property names of the built-in object cannot be retrieved 

Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (120120 => 120121)


--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp	2012-06-12 21:09:18 UTC (rev 120120)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp	2012-06-12 21:15:43 UTC (rev 120121)
@@ -1562,6 +1562,8 @@
     // execution if it doesn't have a prediction, so we do it manually.
     if (prediction == SpecNone)
         addToGraph(ForceOSRExit);
+    
+    NodeIndex originalBaseForBaselineJIT = base;
                 
     addToGraph(CheckStructure, OpInfo(m_graph.addStructureSet(getByIdStatus.structureSet())), base);
     
@@ -1579,8 +1581,18 @@
     } else
         useInlineStorage = getByIdStatus.structureSet().allAreUsingInlinePropertyStorage();
     
+    // Unless we want bugs like https://bugs.webkit.org/show_bug.cgi?id=88783, we need to
+    // ensure that the base of the original get_by_id is kept alive until we're done with
+    // all of the speculations. We only insert the Phantom if there had been a CheckStructure
+    // on something other than the base following the CheckStructure on base, or if the
+    // access was compiled to a WeakJSConstant specific value, in which case we might not
+    // have any explicit use of the base at all.
+    if (getByIdStatus.specificValue() || originalBaseForBaselineJIT != base)
+        addToGraph(Phantom, originalBaseForBaselineJIT);
+    
     if (getByIdStatus.specificValue()) {
         ASSERT(getByIdStatus.specificValue().isCell());
+        
         set(destinationOperand,
             addToGraph(WeakJSConstant, OpInfo(getByIdStatus.specificValue().asCell())));
         return;
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to