Title: [174856] trunk/Source/_javascript_Core
- Revision
- 174856
- Author
- [email protected]
- Date
- 2014-10-17 18:12:46 -0700 (Fri, 17 Oct 2014)
Log Message
Web Process crash when starting the web inspector after r174025.
<https://webkit.org/b/137340>
Reviewed by Filip Pizlo.
After r174025, we can generate a bad graph in the DFG fixup phase like so:
102:<!0:-> StoreBarrier(Check:KnownCell:@19, ..., bc#44)
60:<!0:-> PutStructure(Check:KnownCell:@19, ..., bc#44)
103:<!0:-> Check(Check:NotCell:@54, ..., bc#44)
// ^-- PutByOffset's StoreBarrier has been elided and replaced
// with a speculation check which can OSR exit.
61:<!0:-> PutByOffset(Check:KnownCell:@19, ..., bc#44)
As a result, the structure change will get executed even if we end up OSR
exiting before the PutByOffset. In the baseline JIT code, the structure now
erroneously tells the put operation that there is a value in that property
slot when it is actually uninitialized (hence, the crash).
The fix is to insert the Check at the earliest point possible:
1. If the checked node is in the same bytecode as the PutByOffset, then
the earliest point where we can insert the Check is right after the
checked node.
2. If the checked node is from a preceding bytecode (before the PutByOffset),
then the earliest point where we can insert the Check is at the start
of the current bytecode.
Also reverted the workaround from r174749: https://webkit.org/b/137758.
Benchmark results appear to be a wash on aggregate.
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::indexOfNode):
(JSC::DFG::FixupPhase::indexOfFirstNodeOfExitOrigin):
(JSC::DFG::FixupPhase::fixupNode):
(JSC::DFG::FixupPhase::insertCheck):
* dfg/DFGInsertionSet.h:
(JSC::DFG::InsertionSet::insertOutOfOrder):
(JSC::DFG::InsertionSet::insertOutOfOrderNode):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (174855 => 174856)
--- trunk/Source/_javascript_Core/ChangeLog 2014-10-18 00:25:08 UTC (rev 174855)
+++ trunk/Source/_javascript_Core/ChangeLog 2014-10-18 01:12:46 UTC (rev 174856)
@@ -1,3 +1,47 @@
+2014-10-17 Mark Lam <[email protected]>
+
+ Web Process crash when starting the web inspector after r174025.
+ <https://webkit.org/b/137340>
+
+ Reviewed by Filip Pizlo.
+
+ After r174025, we can generate a bad graph in the DFG fixup phase like so:
+
+ 102:<!0:-> StoreBarrier(Check:KnownCell:@19, ..., bc#44)
+ 60:<!0:-> PutStructure(Check:KnownCell:@19, ..., bc#44)
+ 103:<!0:-> Check(Check:NotCell:@54, ..., bc#44)
+ // ^-- PutByOffset's StoreBarrier has been elided and replaced
+ // with a speculation check which can OSR exit.
+ 61:<!0:-> PutByOffset(Check:KnownCell:@19, ..., bc#44)
+
+ As a result, the structure change will get executed even if we end up OSR
+ exiting before the PutByOffset. In the baseline JIT code, the structure now
+ erroneously tells the put operation that there is a value in that property
+ slot when it is actually uninitialized (hence, the crash).
+
+ The fix is to insert the Check at the earliest point possible:
+
+ 1. If the checked node is in the same bytecode as the PutByOffset, then
+ the earliest point where we can insert the Check is right after the
+ checked node.
+
+ 2. If the checked node is from a preceding bytecode (before the PutByOffset),
+ then the earliest point where we can insert the Check is at the start
+ of the current bytecode.
+
+ Also reverted the workaround from r174749: https://webkit.org/b/137758.
+
+ Benchmark results appear to be a wash on aggregate.
+
+ * dfg/DFGFixupPhase.cpp:
+ (JSC::DFG::FixupPhase::indexOfNode):
+ (JSC::DFG::FixupPhase::indexOfFirstNodeOfExitOrigin):
+ (JSC::DFG::FixupPhase::fixupNode):
+ (JSC::DFG::FixupPhase::insertCheck):
+ * dfg/DFGInsertionSet.h:
+ (JSC::DFG::InsertionSet::insertOutOfOrder):
+ (JSC::DFG::InsertionSet::insertOutOfOrderNode):
+
2014-10-10 Oliver Hunt <[email protected]>
Various arguments optimisations in codegen fail to account for arguments being in lexical record
Modified: trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (174855 => 174856)
--- trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2014-10-18 00:25:08 UTC (rev 174855)
+++ trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2014-10-18 01:12:46 UTC (rev 174856)
@@ -87,6 +87,33 @@
m_insertionSet.execute(block);
}
+ inline unsigned indexOfNode(Node* node, unsigned indexToSearchFrom)
+ {
+ unsigned index = indexToSearchFrom;
+ while (index) {
+ if (m_block->at(index) == node)
+ break;
+ index--;
+ }
+ ASSERT(m_block->at(index) == node);
+ return index;
+ }
+
+ inline unsigned indexOfFirstNodeOfExitOrigin(CodeOrigin& originForExit, unsigned indexToSearchFrom)
+ {
+ unsigned index = indexToSearchFrom;
+ ASSERT(m_block->at(index)->origin.forExit == originForExit);
+ while (index) {
+ index--;
+ if (m_block->at(index)->origin.forExit != originForExit) {
+ index++;
+ break;
+ }
+ }
+ ASSERT(m_block->at(index)->origin.forExit == originForExit);
+ return index;
+ }
+
void fixupNode(Node* node)
{
NodeType op = node->op();
@@ -943,7 +970,7 @@
if (!node->child1()->hasStorageResult())
fixEdge<KnownCellUse>(node->child1());
fixEdge<KnownCellUse>(node->child2());
- insertStoreBarrier(m_indexInBlock, node->child2());
+ insertStoreBarrier(m_indexInBlock, node->child2(), node->child3());
break;
}
@@ -1710,7 +1737,20 @@
void insertCheck(unsigned indexInBlock, Node* node)
{
observeUseKindOnNode<useKind>(node);
- m_insertionSet.insertNode(
+ CodeOrigin& checkedNodeOrigin = node->origin.forExit;
+ CodeOrigin& currentNodeOrigin = m_currentNode->origin.forExit;
+ if (currentNodeOrigin == checkedNodeOrigin) {
+ // The checked node is within the same bytecode. Hence, the earliest
+ // position we can insert the check is right after the checked node.
+ indexInBlock = indexOfNode(node, indexInBlock);
+ indexInBlock++;
+ } else {
+ // The checked node is from a preceding bytecode. Hence, the earliest
+ // position we can insert the check is at the start of the current
+ // bytecode.
+ indexInBlock = indexOfFirstNodeOfExitOrigin(currentNodeOrigin, indexInBlock);
+ }
+ m_insertionSet.insertOutOfOrderNode(
indexInBlock, SpecNone, Check, m_currentNode->origin, Edge(node, useKind));
}
Modified: trunk/Source/_javascript_Core/dfg/DFGInsertionSet.h (174855 => 174856)
--- trunk/Source/_javascript_Core/dfg/DFGInsertionSet.h 2014-10-18 00:25:08 UTC (rev 174855)
+++ trunk/Source/_javascript_Core/dfg/DFGInsertionSet.h 2014-10-18 01:12:46 UTC (rev 174856)
@@ -116,6 +116,34 @@
return insertConstantForUse(index, NodeOrigin(origin), value, useKind);
}
+ Node* insertOutOfOrder(const Insertion& insertion)
+ {
+ size_t targetIndex = insertion.index();
+ size_t entry = m_insertions.size();
+ if (entry) {
+ do {
+ entry--;
+ if (m_insertions[entry].index() <= targetIndex) {
+ entry++;
+ break;
+ }
+ } while (entry);
+ }
+ m_insertions.insert(entry, insertion);
+ return insertion.element();
+ }
+
+ Node* insertOutOfOrder(size_t index, Node* element)
+ {
+ return insertOutOfOrder(Insertion(index, element));
+ }
+
+ template<typename... Params>
+ Node* insertOutOfOrderNode(size_t index, SpeculatedType type, Params... params)
+ {
+ return insertOutOfOrder(index, m_graph.addNode(type, params...));
+ }
+
void execute(BasicBlock* block)
{
executeInsertions(*block, m_insertions);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
