- Revision
- 139136
- Author
- [email protected]
- Date
- 2013-01-08 17:11:32 -0800 (Tue, 08 Jan 2013)
Log Message
DFG shouldn't treat the 'this' argument as being captured if a code block uses arguments
https://bugs.webkit.org/show_bug.cgi?id=106398
<rdar://problem/12439776>
Source/_javascript_Core:
Reviewed by Mark Hahnenberg.
This is a possible optimization for inlined calls, and fixes crashes for inlined constructors, in the case
that the inlined code used arguments. The problem was that assuming that 'this' was captured implies the
assumption that it was initialized by the caller, which is wrong for constructors and this.
Also added a pretty essential DFG IR validation rule: we shouldn't have any live locals at the top of the
root block. This helps to catch this bug: our assumption that 'this' was captured in an inlined constructor
that used arguments led to liveness for the temporary that would have held 'this' in the caller being
propagated all the way up to the entrypoint of the function.
* bytecode/CodeBlock.h:
(JSC::CodeBlock::isCaptured):
* dfg/DFGValidate.cpp:
(JSC::DFG::Validate::validate):
(JSC::DFG::Validate::reportValidationContext):
(Validate):
(JSC::DFG::Validate::dumpGraphIfAppropriate):
LayoutTests:
Reviewed by Mark Hahnenberg.
* fast/js/dfg-inline-constructor-that-uses-arguments-expected.txt: Added.
* fast/js/dfg-inline-constructor-that-uses-arguments.html: Added.
* fast/js/jsc-test-list:
* fast/js/script-tests/dfg-inline-constructor-that-uses-arguments.js: Added.
(Foo):
(bar):
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (139135 => 139136)
--- trunk/LayoutTests/ChangeLog 2013-01-09 00:59:43 UTC (rev 139135)
+++ trunk/LayoutTests/ChangeLog 2013-01-09 01:11:32 UTC (rev 139136)
@@ -1,3 +1,18 @@
+2013-01-08 Filip Pizlo <[email protected]>
+
+ DFG shouldn't treat the 'this' argument as being captured if a code block uses arguments
+ https://bugs.webkit.org/show_bug.cgi?id=106398
+ <rdar://problem/12439776>
+
+ Reviewed by Mark Hahnenberg.
+
+ * fast/js/dfg-inline-constructor-that-uses-arguments-expected.txt: Added.
+ * fast/js/dfg-inline-constructor-that-uses-arguments.html: Added.
+ * fast/js/jsc-test-list:
+ * fast/js/script-tests/dfg-inline-constructor-that-uses-arguments.js: Added.
+ (Foo):
+ (bar):
+
2013-01-08 Viatcheslav Ostapenko <[email protected]>
[EFL][WK2] WebGL test cases are sometimes crashing
Added: trunk/LayoutTests/fast/js/dfg-inline-constructor-that-uses-arguments-expected.txt (0 => 139136)
--- trunk/LayoutTests/fast/js/dfg-inline-constructor-that-uses-arguments-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/js/dfg-inline-constructor-that-uses-arguments-expected.txt 2013-01-09 01:11:32 UTC (rev 139136)
@@ -0,0 +1,209 @@
+Tests that we can inline a constructor that uses arguments without failing DFG validation.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS bar().x is 42
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/js/dfg-inline-constructor-that-uses-arguments.html (0 => 139136)
--- trunk/LayoutTests/fast/js/dfg-inline-constructor-that-uses-arguments.html (rev 0)
+++ trunk/LayoutTests/fast/js/dfg-inline-constructor-that-uses-arguments.html 2013-01-09 01:11:32 UTC (rev 139136)
@@ -0,0 +1,10 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script src=""
+<script src=""
+</body>
+</html>
Modified: trunk/LayoutTests/fast/js/jsc-test-list (139135 => 139136)
--- trunk/LayoutTests/fast/js/jsc-test-list 2013-01-09 00:59:43 UTC (rev 139135)
+++ trunk/LayoutTests/fast/js/jsc-test-list 2013-01-09 01:11:32 UTC (rev 139136)
@@ -131,6 +131,7 @@
fast/js/dfg-inline-arguments-use-from-getter
fast/js/dfg-inline-arguments-use-from-uninlined-code
fast/js/dfg-inline-constant
+fast/js/dfg-inline-constructor-that-uses-arguments
fast/js/dfg-inline-early-return
fast/js/dfg-inline-function-dot-caller
fast/js/dfg-inline-many-blocks
Added: trunk/LayoutTests/fast/js/script-tests/dfg-inline-constructor-that-uses-arguments.js (0 => 139136)
--- trunk/LayoutTests/fast/js/script-tests/dfg-inline-constructor-that-uses-arguments.js (rev 0)
+++ trunk/LayoutTests/fast/js/script-tests/dfg-inline-constructor-that-uses-arguments.js 2013-01-09 01:11:32 UTC (rev 139136)
@@ -0,0 +1,14 @@
+description(
+"Tests that we can inline a constructor that uses arguments without failing DFG validation."
+);
+
+function Foo() {
+ this.x = arguments[0];
+}
+
+function bar() {
+ return new Foo(42);
+}
+
+for (var i = 0; i < 200; ++i)
+ shouldBe("bar().x", "42");
Modified: trunk/Source/_javascript_Core/ChangeLog (139135 => 139136)
--- trunk/Source/_javascript_Core/ChangeLog 2013-01-09 00:59:43 UTC (rev 139135)
+++ trunk/Source/_javascript_Core/ChangeLog 2013-01-09 01:11:32 UTC (rev 139136)
@@ -1,5 +1,30 @@
2013-01-08 Filip Pizlo <[email protected]>
+ DFG shouldn't treat the 'this' argument as being captured if a code block uses arguments
+ https://bugs.webkit.org/show_bug.cgi?id=106398
+ <rdar://problem/12439776>
+
+ Reviewed by Mark Hahnenberg.
+
+ This is a possible optimization for inlined calls, and fixes crashes for inlined constructors, in the case
+ that the inlined code used arguments. The problem was that assuming that 'this' was captured implies the
+ assumption that it was initialized by the caller, which is wrong for constructors and this.
+
+ Also added a pretty essential DFG IR validation rule: we shouldn't have any live locals at the top of the
+ root block. This helps to catch this bug: our assumption that 'this' was captured in an inlined constructor
+ that used arguments led to liveness for the temporary that would have held 'this' in the caller being
+ propagated all the way up to the entrypoint of the function.
+
+ * bytecode/CodeBlock.h:
+ (JSC::CodeBlock::isCaptured):
+ * dfg/DFGValidate.cpp:
+ (JSC::DFG::Validate::validate):
+ (JSC::DFG::Validate::reportValidationContext):
+ (Validate):
+ (JSC::DFG::Validate::dumpGraphIfAppropriate):
+
+2013-01-08 Filip Pizlo <[email protected]>
+
REGRESSION (r138921): Crash in JSC::Arguments::create
https://bugs.webkit.org/show_bug.cgi?id=106329
<rdar://problem/12974196>
Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.h (139135 => 139136)
--- trunk/Source/_javascript_Core/bytecode/CodeBlock.h 2013-01-09 00:59:43 UTC (rev 139135)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.h 2013-01-09 01:11:32 UTC (rev 139136)
@@ -594,12 +594,12 @@
bool isCaptured(int operand, InlineCallFrame* inlineCallFrame = 0) const
{
- if (inlineCallFrame && !operandIsArgument(operand))
+ if (operandIsArgument(operand))
+ return operandToArgument(operand) && usesArguments();
+
+ if (inlineCallFrame)
return inlineCallFrame->capturedVars.get(operand);
- if (operandIsArgument(operand))
- return usesArguments();
-
// The activation object isn't in the captured region, but it's "captured"
// in the sense that stores to its location can be observed indirectly.
if (needsActivation() && operand == activationRegister())
Modified: trunk/Source/_javascript_Core/dfg/DFGValidate.cpp (139135 => 139136)
--- trunk/Source/_javascript_Core/dfg/DFGValidate.cpp 2013-01-09 00:59:43 UTC (rev 139135)
+++ trunk/Source/_javascript_Core/dfg/DFGValidate.cpp 2013-01-09 01:11:32 UTC (rev 139136)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2012 Apple Inc. All rights reserved.
+ * Copyright (C) 2012, 2013 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -28,6 +28,7 @@
#if ENABLE(DFG_JIT)
+#include "CodeBlockWithJITType.h"
#include <wtf/Assertions.h>
#include <wtf/BitVector.h>
@@ -76,6 +77,11 @@
// NB. This code is not written for performance, since it is not intended to run
// in release builds.
+ // Validate that all local variables at the head of the root block are dead.
+ BasicBlock* root = m_graph.m_blocks[0].get();
+ for (unsigned i = 0; i < root->variablesAtHead.numberOfLocals(); ++i)
+ V_EQUAL((static_cast<VirtualRegister>(i), 0), NoNode, root->variablesAtHead.local(i));
+
// Validate ref counts and uses.
Vector<unsigned> myRefCounts;
myRefCounts.fill(0, m_graph.size());
@@ -304,6 +310,11 @@
dataLogF("@%u -> %s@%u", nodeIndex, useKindToString(edge.useKind()), edge.index());
}
+ void reportValidationContext(VirtualRegister local, BlockIndex blockIndex)
+ {
+ dataLogF("r%d in Block #%u", local, blockIndex);
+ }
+
void reportValidationContext(
VirtualRegister local, BlockIndex sourceBlockIndex, BlockTag, BlockIndex destinationBlockIndex)
{
@@ -343,7 +354,7 @@
{
if (m_graphDumpMode == DontDumpGraph)
return;
- dataLogF("Graph at time of failure:\n");
+ dataLog("Graph of ", CodeBlockWithJITType(m_graph.m_codeBlock, JITCode::DFGJIT), " at time of failure:\n");
m_graph.dump();
}
};
