- Revision
- 184318
- Author
- [email protected]
- Date
- 2015-05-13 16:57:17 -0700 (Wed, 13 May 2015)
Log Message
Creating a new blank document in icloud pages causes an AI error: Abstract value (CellBytecodedoubleBoolOther, TOP, TOP) for double node has type outside SpecFullDouble.
https://bugs.webkit.org/show_bug.cgi?id=144856
Reviewed by Benjamin Poulain.
First I made fixTypeForRepresentation() print out better diagnostics when it dies.
Then I fixed the bug: Node::convertToIdentityOn(Node*) needs to make sure that when it
converts to a representation-changing node, it needs to use one of the UseKinds that such
a node expects. For example, DoubleRep(UntypedUse:) doesn't make sense; it needs to be
something like DoubleRep(NumberUse:) since it will speculate that the input is a number.
* dfg/DFGAbstractInterpreter.h:
(JSC::DFG::AbstractInterpreter::setBuiltInConstant):
* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* dfg/DFGAbstractValue.cpp:
(JSC::DFG::AbstractValue::fixTypeForRepresentation):
* dfg/DFGAbstractValue.h:
* dfg/DFGInPlaceAbstractState.cpp:
(JSC::DFG::InPlaceAbstractState::initialize):
* dfg/DFGNode.cpp:
(JSC::DFG::Node::convertToIdentityOn):
* tests/stress/cloned-arguments-get-by-val-double-array.js: Added.
(foo):
Modified Paths
Added Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (184317 => 184318)
--- trunk/Source/_javascript_Core/ChangeLog 2015-05-13 23:33:10 UTC (rev 184317)
+++ trunk/Source/_javascript_Core/ChangeLog 2015-05-13 23:57:17 UTC (rev 184318)
@@ -1,3 +1,31 @@
+2015-05-13 Filip Pizlo <[email protected]>
+
+ Creating a new blank document in icloud pages causes an AI error: Abstract value (CellBytecodedoubleBoolOther, TOP, TOP) for double node has type outside SpecFullDouble.
+ https://bugs.webkit.org/show_bug.cgi?id=144856
+
+ Reviewed by Benjamin Poulain.
+
+ First I made fixTypeForRepresentation() print out better diagnostics when it dies.
+
+ Then I fixed the bug: Node::convertToIdentityOn(Node*) needs to make sure that when it
+ converts to a representation-changing node, it needs to use one of the UseKinds that such
+ a node expects. For example, DoubleRep(UntypedUse:) doesn't make sense; it needs to be
+ something like DoubleRep(NumberUse:) since it will speculate that the input is a number.
+
+ * dfg/DFGAbstractInterpreter.h:
+ (JSC::DFG::AbstractInterpreter::setBuiltInConstant):
+ * dfg/DFGAbstractInterpreterInlines.h:
+ (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+ * dfg/DFGAbstractValue.cpp:
+ (JSC::DFG::AbstractValue::fixTypeForRepresentation):
+ * dfg/DFGAbstractValue.h:
+ * dfg/DFGInPlaceAbstractState.cpp:
+ (JSC::DFG::InPlaceAbstractState::initialize):
+ * dfg/DFGNode.cpp:
+ (JSC::DFG::Node::convertToIdentityOn):
+ * tests/stress/cloned-arguments-get-by-val-double-array.js: Added.
+ (foo):
+
2015-05-13 Commit Queue <[email protected]>
Unreviewed, rolling out r184313.
Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreter.h (184317 => 184318)
--- trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreter.h 2015-05-13 23:33:10 UTC (rev 184317)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreter.h 2015-05-13 23:57:17 UTC (rev 184318)
@@ -168,7 +168,7 @@
{
AbstractValue& abstractValue = forNode(node);
abstractValue.set(m_graph, value, m_state.structureClobberState());
- abstractValue.fixTypeForRepresentation(node);
+ abstractValue.fixTypeForRepresentation(m_graph, node);
}
void setConstant(Node* node, FrozenValue value)
Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h (184317 => 184318)
--- trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2015-05-13 23:33:10 UTC (rev 184317)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2015-05-13 23:57:17 UTC (rev 184318)
@@ -348,7 +348,7 @@
break;
}
forNode(node).setType(m_graph, forNode(node->child1()).m_type);
- forNode(node).fixTypeForRepresentation(node);
+ forNode(node).fixTypeForRepresentation(m_graph, node);
break;
}
@@ -371,7 +371,7 @@
}
forNode(node).setType(m_graph, forNode(node->child1()).m_type & ~SpecDoubleImpureNaN);
- forNode(node).fixTypeForRepresentation(node);
+ forNode(node).fixTypeForRepresentation(m_graph, node);
break;
}
Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractValue.cpp (184317 => 184318)
--- trunk/Source/_javascript_Core/dfg/DFGAbstractValue.cpp 2015-05-13 23:33:10 UTC (rev 184317)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractValue.cpp 2015-05-13 23:57:17 UTC (rev 184318)
@@ -136,7 +136,7 @@
checkConsistency();
}
-void AbstractValue::fixTypeForRepresentation(NodeFlags representation)
+void AbstractValue::fixTypeForRepresentation(Graph& graph, NodeFlags representation, Node* node)
{
if (representation == NodeResultDouble) {
if (m_value) {
@@ -148,39 +148,30 @@
m_type &= ~SpecMachineInt;
m_type |= SpecInt52AsDouble;
}
- if (m_type & ~SpecFullDouble) {
- startCrashing();
- dataLog("Abstract value ", *this, " for double node has type outside SpecFullDouble.\n");
- CRASH();
- }
+ if (m_type & ~SpecFullDouble)
+ DFG_CRASH(graph, node, toCString("Abstract value ", *this, " for double node has type outside SpecFullDouble.\n").data());
} else if (representation == NodeResultInt52) {
if (m_type & SpecInt52AsDouble) {
m_type &= ~SpecInt52AsDouble;
m_type |= SpecInt52;
}
- if (m_type & ~SpecMachineInt) {
- startCrashing();
- dataLog("Abstract value ", *this, " for int52 node has type outside SpecMachineInt.\n");
- CRASH();
- }
+ if (m_type & ~SpecMachineInt)
+ DFG_CRASH(graph, node, toCString("Abstract value ", *this, " for int52 node has type outside SpecMachineInt.\n").data());
} else {
if (m_type & SpecInt52) {
m_type &= ~SpecInt52;
m_type |= SpecInt52AsDouble;
}
- if (m_type & ~SpecBytecodeTop) {
- startCrashing();
- dataLog("Abstract value ", *this, " for value node has type outside SpecBytecodeTop.\n");
- CRASH();
- }
+ if (m_type & ~SpecBytecodeTop)
+ DFG_CRASH(graph, node, toCString("Abstract value ", *this, " for value node has type outside SpecBytecodeTop.\n").data());
}
checkConsistency();
}
-void AbstractValue::fixTypeForRepresentation(Node* node)
+void AbstractValue::fixTypeForRepresentation(Graph& graph, Node* node)
{
- fixTypeForRepresentation(node->result());
+ fixTypeForRepresentation(graph, node->result(), node);
}
FiltrationResult AbstractValue::filter(Graph& graph, const StructureSet& other)
Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractValue.h (184317 => 184318)
--- trunk/Source/_javascript_Core/dfg/DFGAbstractValue.h 2015-05-13 23:33:10 UTC (rev 184317)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractValue.h 2015-05-13 23:57:17 UTC (rev 184318)
@@ -213,8 +213,8 @@
checkConsistency();
}
- void fixTypeForRepresentation(NodeFlags representation);
- void fixTypeForRepresentation(Node*);
+ void fixTypeForRepresentation(Graph&, NodeFlags representation, Node* = nullptr);
+ void fixTypeForRepresentation(Graph&, Node*);
bool operator==(const AbstractValue& other) const
{
Modified: trunk/Source/_javascript_Core/dfg/DFGInPlaceAbstractState.cpp (184317 => 184318)
--- trunk/Source/_javascript_Core/dfg/DFGInPlaceAbstractState.cpp 2015-05-13 23:33:10 UTC (rev 184317)
+++ trunk/Source/_javascript_Core/dfg/DFGInPlaceAbstractState.cpp 2015-05-13 23:57:17 UTC (rev 184318)
@@ -167,7 +167,7 @@
VariableAccessData* variable = node->variableAccessData();
FlushFormat format = variable->flushFormat();
target.merge(source);
- target.fixTypeForRepresentation(resultFor(format));
+ target.fixTypeForRepresentation(m_graph, resultFor(format));
}
block->cfaShouldRevisit = true;
}
Modified: trunk/Source/_javascript_Core/dfg/DFGNode.cpp (184317 => 184318)
--- trunk/Source/_javascript_Core/dfg/DFGNode.cpp 2015-05-13 23:33:10 UTC (rev 184317)
+++ trunk/Source/_javascript_Core/dfg/DFGNode.cpp 2015-05-13 23:57:17 UTC (rev 184318)
@@ -116,17 +116,44 @@
}
switch (output) {
case NodeResultDouble:
- RELEASE_ASSERT(input == NodeResultInt52 || input == NodeResultJS);
setOpAndDefaultFlags(DoubleRep);
- return;
+ switch (input) {
+ case NodeResultInt52:
+ child1().setUseKind(Int52RepUse);
+ return;
+ case NodeResultJS:
+ child1().setUseKind(NumberUse);
+ return;
+ default:
+ RELEASE_ASSERT_NOT_REACHED();
+ return;
+ }
case NodeResultInt52:
- RELEASE_ASSERT(input == NodeResultDouble || input == NodeResultJS);
setOpAndDefaultFlags(Int52Rep);
- return;
+ switch (input) {
+ case NodeResultDouble:
+ child1().setUseKind(DoubleRepMachineIntUse);
+ return;
+ case NodeResultJS:
+ child1().setUseKind(MachineIntUse);
+ return;
+ default:
+ RELEASE_ASSERT_NOT_REACHED();
+ return;
+ }
case NodeResultJS:
- RELEASE_ASSERT(input == NodeResultDouble || input == NodeResultInt52);
setOpAndDefaultFlags(ValueRep);
- return;
+ switch (input) {
+ case NodeResultDouble:
+ child1().setUseKind(DoubleRepUse);
+ return;
+ case NodeResultInt52:
+ child1().setUseKind(Int52RepUse);
+ return;
+ default:
+ RELEASE_ASSERT_NOT_REACHED();
+ return;
+ }
default:
RELEASE_ASSERT_NOT_REACHED();
return;
Added: trunk/Source/_javascript_Core/tests/stress/cloned-arguments-get-by-val-double-array.js (0 => 184318)
--- trunk/Source/_javascript_Core/tests/stress/cloned-arguments-get-by-val-double-array.js (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/cloned-arguments-get-by-val-double-array.js 2015-05-13 23:57:17 UTC (rev 184318)
@@ -0,0 +1,13 @@
+function foo() {
+ "use strict";
+ return arguments[0] + 1.5;
+}
+
+noInline(foo);
+
+for (var i = 0; i < 10000; ++i) {
+ var result = foo(4.2);
+ if (result != 5.7)
+ throw "Error: bad result: " + result;
+}
+
