- Revision
- 100514
- Author
- fpi...@apple.com
- Date
- 2011-11-16 15:21:39 -0800 (Wed, 16 Nov 2011)
Log Message
DFG global variable CSE mishandles the cross-global-object inlining corner case
https://bugs.webkit.org/show_bug.cgi?id=72542
Source/_javascript_Core:
Reviewed by Geoff Garen.
Moved code to get the global object for a code origin into CodeBlock, so it is
more broadly accessible. Fixed CSE to compare both the variable number, and the
global object, before deciding to perform elimination.
* bytecode/CodeBlock.h:
(JSC::CodeBlock::globalObjectFor):
* dfg/DFGAssemblyHelpers.h:
(JSC::DFG::AssemblyHelpers::globalObjectFor):
* dfg/DFGPropagator.cpp:
(JSC::DFG::Propagator::globalVarLoadElimination):
(JSC::DFG::Propagator::performNodeCSE):
LayoutTests:
Reviewed by Geoff Garen.
* fast/js/cross-global-object-inline-global-var-expected.txt: Added.
* fast/js/cross-global-object-inline-global-var.html: Added.
* fast/js/script-tests/cross-global-object-inline-global-var.js: Added.
(foo):
(done):
(doit):
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (100513 => 100514)
--- trunk/LayoutTests/ChangeLog 2011-11-16 23:02:01 UTC (rev 100513)
+++ trunk/LayoutTests/ChangeLog 2011-11-16 23:21:39 UTC (rev 100514)
@@ -1,3 +1,17 @@
+2011-11-16 Filip Pizlo <fpi...@apple.com>
+
+ DFG global variable CSE mishandles the cross-global-object inlining corner case
+ https://bugs.webkit.org/show_bug.cgi?id=72542
+
+ Reviewed by Geoff Garen.
+
+ * fast/js/cross-global-object-inline-global-var-expected.txt: Added.
+ * fast/js/cross-global-object-inline-global-var.html: Added.
+ * fast/js/script-tests/cross-global-object-inline-global-var.js: Added.
+ (foo):
+ (done):
+ (doit):
+
2011-11-16 James Robinson <jam...@chromium.org>
[chromium] Update text baselines for mq-transform-0[23] tests
Added: trunk/LayoutTests/fast/js/cross-global-object-inline-global-var-expected.txt (0 => 100514)
--- trunk/LayoutTests/fast/js/cross-global-object-inline-global-var-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/js/cross-global-object-inline-global-var-expected.txt 2011-11-16 23:21:39 UTC (rev 100514)
@@ -0,0 +1,11 @@
+This tests that function inlining in the DFG JIT doesn't get confused by different global objects.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS foo(3) is 324
+PASS successfullyParsed is true
+
+TEST COMPLETE
+PASS done() called with 5770500
+
Added: trunk/LayoutTests/fast/js/cross-global-object-inline-global-var.html (0 => 100514)
--- trunk/LayoutTests/fast/js/cross-global-object-inline-global-var.html (rev 0)
+++ trunk/LayoutTests/fast/js/cross-global-object-inline-global-var.html 2011-11-16 23:21:39 UTC (rev 100514)
@@ -0,0 +1,11 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<div id="frameparent"></div>
+<script src=""
+<script src=""
+</body>
+</html>
Added: trunk/LayoutTests/fast/js/script-tests/cross-global-object-inline-global-var.js (0 => 100514)
--- trunk/LayoutTests/fast/js/script-tests/cross-global-object-inline-global-var.js (rev 0)
+++ trunk/LayoutTests/fast/js/script-tests/cross-global-object-inline-global-var.js 2011-11-16 23:21:39 UTC (rev 100514)
@@ -0,0 +1,58 @@
+description(
+"This tests that function inlining in the DFG JIT doesn't get confused by different global objects."
+);
+
+if (window.layoutTestController)
+ layoutTestController.waitUntilDone();
+
+var b = 321;
+
+function foo(a) {
+ return a + b;
+}
+
+shouldBe("foo(3)", "324");
+
+function done(value) {
+ var expected = 5770500;
+ if (value == expected)
+ testPassed("done() called with " + expected);
+ else
+ testFailed("done() is " + value + " and should be " + expected + ".");
+ if (window.layoutTestController)
+ layoutTestController.notifyDone();
+}
+
+function doit() {
+ document.getElementById("frameparent").innerHTML = "";
+ document.getElementById("frameparent").innerHTML = "<iframe id='testframe'>";
+ var testFrame = document.getElementById("testframe");
+ testFrame.contentDocument.open();
+
+ code = "<!DOCTYPE html>\n<head></head><body><script type=\"text/_javascript_\">\n";
+
+ // Make sure that we get as many variables as the parent.
+ for (var i = 0; i < 100; ++i)
+ code += "var b" + i + " = " +i + ";\n";
+
+ code += "result = 0;\n" +
+ "function bar(a) {\n" +
+ " var foo = window.parent.foo;\n" +
+ " return ";
+
+ for (var i = 0; i < 100; ++i)
+ code += "b" + i + " + ";
+
+ code += "foo(a);\n" +
+ "}\n" +
+ "for (var i = 0; i < 1000; ++i) {\n" +
+ " result += bar(i);\n" +
+ "}\n" +
+ "window.parent.done(result);\n" +
+ "</script></body></html>"
+
+ testFrame.contentDocument.write(code);
+}
+
+window.setTimeout(doit, 10);
+
Modified: trunk/Source/_javascript_Core/ChangeLog (100513 => 100514)
--- trunk/Source/_javascript_Core/ChangeLog 2011-11-16 23:02:01 UTC (rev 100513)
+++ trunk/Source/_javascript_Core/ChangeLog 2011-11-16 23:21:39 UTC (rev 100514)
@@ -1,3 +1,22 @@
+2011-11-16 Filip Pizlo <fpi...@apple.com>
+
+ DFG global variable CSE mishandles the cross-global-object inlining corner case
+ https://bugs.webkit.org/show_bug.cgi?id=72542
+
+ Reviewed by Geoff Garen.
+
+ Moved code to get the global object for a code origin into CodeBlock, so it is
+ more broadly accessible. Fixed CSE to compare both the variable number, and the
+ global object, before deciding to perform elimination.
+
+ * bytecode/CodeBlock.h:
+ (JSC::CodeBlock::globalObjectFor):
+ * dfg/DFGAssemblyHelpers.h:
+ (JSC::DFG::AssemblyHelpers::globalObjectFor):
+ * dfg/DFGPropagator.cpp:
+ (JSC::DFG::Propagator::globalVarLoadElimination):
+ (JSC::DFG::Propagator::performNodeCSE):
+
2011-11-16 Michael Saboff <msab...@apple.com>
Enable 8 Bit Strings in _javascript_Core
Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.h (100513 => 100514)
--- trunk/Source/_javascript_Core/bytecode/CodeBlock.h 2011-11-16 23:02:01 UTC (rev 100513)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.h 2011-11-16 23:21:39 UTC (rev 100514)
@@ -821,6 +821,14 @@
}
JSGlobalObject* globalObject() { return m_globalObject.get(); }
+
+ JSGlobalObject* globalObjectFor(CodeOrigin codeOrigin)
+ {
+ if (!codeOrigin.inlineCallFrame)
+ return globalObject();
+ // FIXME: if we ever inline based on executable not function, this code will need to change.
+ return codeOrigin.inlineCallFrame->callee->scope()->globalObject.get();
+ }
// Jump Tables
Modified: trunk/Source/_javascript_Core/dfg/DFGAssemblyHelpers.h (100513 => 100514)
--- trunk/Source/_javascript_Core/dfg/DFGAssemblyHelpers.h 2011-11-16 23:02:01 UTC (rev 100513)
+++ trunk/Source/_javascript_Core/dfg/DFGAssemblyHelpers.h 2011-11-16 23:21:39 UTC (rev 100514)
@@ -284,10 +284,7 @@
JSGlobalObject* globalObjectFor(CodeOrigin codeOrigin)
{
- if (!codeOrigin.inlineCallFrame)
- return codeBlock()->globalObject();
- // FIXME: if we ever inline based on executable not function, this code will need to change.
- return codeOrigin.inlineCallFrame->callee->scope()->globalObject.get();
+ return codeBlock()->globalObjectFor(codeOrigin);
}
bool strictModeFor(CodeOrigin codeOrigin)
Modified: trunk/Source/_javascript_Core/dfg/DFGPropagator.cpp (100513 => 100514)
--- trunk/Source/_javascript_Core/dfg/DFGPropagator.cpp 2011-11-16 23:02:01 UTC (rev 100513)
+++ trunk/Source/_javascript_Core/dfg/DFGPropagator.cpp 2011-11-16 23:21:39 UTC (rev 100514)
@@ -981,18 +981,18 @@
return NoNode;
}
- NodeIndex globalVarLoadElimination(unsigned varNumber)
+ NodeIndex globalVarLoadElimination(unsigned varNumber, JSGlobalObject* globalObject)
{
NodeIndex start = startIndexForChildren();
for (NodeIndex index = m_compileIndex; index-- > start;) {
Node& node = m_graph[index];
switch (node.op) {
case GetGlobalVar:
- if (node.varNumber() == varNumber)
+ if (node.varNumber() == varNumber && m_codeBlock->globalObjectFor(node.codeOrigin) == globalObject)
return index;
break;
case PutGlobalVar:
- if (node.varNumber() == varNumber)
+ if (node.varNumber() == varNumber && m_codeBlock->globalObjectFor(node.codeOrigin) == globalObject)
return node.child1();
break;
default:
@@ -1334,7 +1334,7 @@
// Finally handle heap accesses. These are not quite pure, but we can still
// optimize them provided that some subtle conditions are met.
case GetGlobalVar:
- setReplacement(globalVarLoadElimination(node.varNumber()));
+ setReplacement(globalVarLoadElimination(node.varNumber(), m_codeBlock->globalObjectFor(node.codeOrigin)));
break;
case GetByVal:
