Skip to content

Commit

Permalink
Array.prototype.indexOf constant-folding should account for non-numer…
Browse files Browse the repository at this point in the history
…ic index

https://bugs.webkit.org/show_bug.cgi?id=246275
<rdar://problem/101242631>

Reviewed by Yusuke Suzuki.

With this change, a Check DFG node is emitted when Array.prototype.indexOf is
constant-folded to ensure an OSR exit for non-numeric index, whose coercion
might be observable.

* JSTests/stress/array-indexof-constant-folding-index-coercion.js: Added.
* Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupArrayIndexOf):

Canonical link: https://commits.webkit.org/256590@main
  • Loading branch information
Alexey Shvayka committed Nov 11, 2022
1 parent 90805ce commit 77b468c
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 7 deletions.
50 changes: 50 additions & 0 deletions JSTests/stress/array-indexof-constant-folding-index-coercion.js
@@ -0,0 +1,50 @@
function shouldBe(actual, expected)
{
if (actual !== expected)
throw new Error('bad value: ' + actual);
}

(function() {
const int32Array = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9];
const doubleArray = [0.5, 1.5, 2.5, 3.5, 4.5, 5.5, 6.5, 7.5, 8.5, 9.5];

const indexOfInt32Cell = index => int32Array.indexOf("test", index);
const indexOfInt32Other = index => int32Array.indexOf(null, index);
const indexOfInt32Boolean = index => int32Array.indexOf(true, index);
const indexOfDoubleCell = index => doubleArray.indexOf(Symbol(), index);
const indexOfDoubleOther = index => doubleArray.indexOf(undefined, index);
const indexOfDoubleBoolean = index => doubleArray.indexOf(false, index);

noInline(indexOfInt32Cell);
noInline(indexOfInt32Other);
noInline(indexOfInt32Boolean);
noInline(indexOfDoubleCell);
noInline(indexOfDoubleOther);
noInline(indexOfDoubleBoolean);

for (var i = 0; i < 1e6; ++i) {
shouldBe(indexOfInt32Cell(0), -1);
shouldBe(indexOfInt32Other(0), -1);
shouldBe(indexOfInt32Boolean(0), -1);
shouldBe(indexOfDoubleCell(0), -1);
shouldBe(indexOfDoubleOther(0), -1);
shouldBe(indexOfDoubleBoolean(0), -1);
}

let coercibleIndexCalls = 0;
const coercibleIndex = {
valueOf: () => {
coercibleIndexCalls++;
return 0;
},
};

shouldBe(indexOfInt32Cell(coercibleIndex), -1);
shouldBe(indexOfInt32Other(coercibleIndex), -1);
shouldBe(indexOfInt32Boolean(coercibleIndex), -1);
shouldBe(indexOfDoubleCell(coercibleIndex), -1);
shouldBe(indexOfDoubleOther(coercibleIndex), -1);
shouldBe(indexOfDoubleBoolean(coercibleIndex), -1);

shouldBe(coercibleIndexCalls, 6);
})();
17 changes: 10 additions & 7 deletions Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
Expand Up @@ -4357,26 +4357,33 @@ class FixupPhase : public Phase {

Edge& searchElement = m_graph.varArgChild(node, 1);

fixEdge<KnownCellUse>(array);
if (node->numChildren() == 4)
fixEdge<Int32Use>(m_graph.varArgChild(node, 2));

// Constant folding.
switch (node->arrayMode().type()) {
case Array::Double:
case Array::Int32: {
if (searchElement->shouldSpeculateCell()) {
m_insertionSet.insertNode(m_indexInBlock, SpecNone, Check, node->origin, Edge(searchElement.node(), CellUse));
fixEdge<CellUse>(searchElement);
m_insertionSet.insertCheck(m_graph, m_indexInBlock, node);
m_graph.convertToConstant(node, jsNumber(-1));
observeUseKindOnNode<CellUse>(searchElement.node());
return;
}

if (searchElement->shouldSpeculateOther()) {
m_insertionSet.insertNode(m_indexInBlock, SpecNone, Check, node->origin, Edge(searchElement.node(), OtherUse));
fixEdge<OtherUse>(searchElement);
m_insertionSet.insertCheck(m_graph, m_indexInBlock, node);
m_graph.convertToConstant(node, jsNumber(-1));
observeUseKindOnNode<OtherUse>(searchElement.node());
return;
}

if (searchElement->shouldSpeculateBoolean()) {
m_insertionSet.insertNode(m_indexInBlock, SpecNone, Check, node->origin, Edge(searchElement.node(), BooleanUse));
fixEdge<BooleanUse>(searchElement);
m_insertionSet.insertCheck(m_graph, m_indexInBlock, node);
m_graph.convertToConstant(node, jsNumber(-1));
observeUseKindOnNode<BooleanUse>(searchElement.node());
return;
Expand All @@ -4387,10 +4394,6 @@ class FixupPhase : public Phase {
break;
}

fixEdge<KnownCellUse>(array);
if (node->numChildren() == 4)
fixEdge<Int32Use>(m_graph.varArgChild(node, 2));

switch (node->arrayMode().type()) {
case Array::Double: {
if (searchElement->shouldSpeculateNumber())
Expand Down

0 comments on commit 77b468c

Please sign in to comment.