From f8dd0d79123927f57a3982601ce5cb98f743d438 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 10 Oct 2026 15:55:53 +0000 Subject: [PATCH 1/2] [JSC] PutCellButterflySlot forbids the double format for the variable it stores The DFG and the FTL inline Array.prototype.sort for at most 16 elements (ByteCodeParser::handleArraySort). The insertion sort keeps its pivot in a variable, passes it to the inlined comparator as the first argument, and stores it back into the scratch with PutCellButterflySlot. That store writes the value as is, the loop reads the scratch of an Int32 array back as Int32s with no check, and ArraySortCommit copies the scratch as is into the array. A comparator that does double arithmetic on its first parameter makes double voting give that variable the double format. The pivot of an Int32 array then goes back into the scratch as a boxed double: function f(v, w, x, y) { return [v, w, x, y].sort((a, b) => (a + 0.5) - (b + 0.5)); } Once the DFG has compiled f, f(17, 8, 1, 1) returns the elements out of order, as boxed doubles in an ArrayWithInt32. Element reads give 8,1,1,17, join() gives "0,0,0,17" and indexOf(8) gives -1. After the first such store the comparator also gets numbers that are not in the array. FTL code for f aborts under --validateAbstractInterpreterState=1. Double voting now merges CantUseDoubleFormat into the variable that a PutCellButterflySlot stores. Only the double format changes the bits that go back: a variable that holds an Int32 or a JSValue gives back the value that came out of the array. The pivot of such a sort is now a JSValue, and the comparator converts it where it uses it. A sort whose pivot was not in the double format gets the code it got before. The inlined sort is upstream's (WebKit/WebKit#63908), and upstream main returns the same arrays. * JSTests/stress/array-sort-inline-int32-double-comparator.js: Added. * Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp: --- ...ray-sort-inline-int32-double-comparator.js | 49 +++++++++++++++++++ .../dfg/DFGPredictionPropagationPhase.cpp | 14 ++++++ 2 files changed, 63 insertions(+) create mode 100644 JSTests/stress/array-sort-inline-int32-double-comparator.js diff --git a/JSTests/stress/array-sort-inline-int32-double-comparator.js b/JSTests/stress/array-sort-inline-int32-double-comparator.js new file mode 100644 index 000000000000..452afc34448b --- /dev/null +++ b/JSTests/stress/array-sort-inline-int32-double-comparator.js @@ -0,0 +1,49 @@ +//@ defaultRun +//@ runNoCJIT("--validateGraphAtEachPhase=true") + +// The inlined sort keeps the pivot of its insertion sort in a DFG variable, stores it back into the scratch +// as is, and reads the scratch of an Int32 array back with no type check. A comparator that does double +// arithmetic on its first parameter must not make that variable a double. The sort would then store boxed +// doubles into an ArrayWithInt32, compare other numbers than the elements, and return them out of order. + +function shouldBe(actual, expected) { + if (actual !== expected) + throw new Error("bad value: " + actual + " expected: " + expected); +} + +var float64 = new Float64Array([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17]); + +function make(name, comparator) { + return new Function("float64", `return function ${name}(i) { return [17, 8, i % 9 + 1, i * 7 % 9 + 1].sort(${comparator}); };`)(float64); +} + +function test(optimized, expected, expectedIndexOf8) { + for (var i = 0; i < testLoopCount; ++i) { + var actual = optimized(i); + // join() and indexOf() read the elements of an ArrayWithInt32 as Int32s. + shouldBe(actual.join(), expected[i % 9]); + shouldBe(actual.indexOf(8), expectedIndexOf8[i % 9]); + } +} +noInline(test); + +var comparators = [ + "(a, b) => (a + 0.5) - (b + 0.5)", + "(a, b) => a - float64[b]", +]; + +for (var comparator of comparators) { + var optimized = make("optimized", comparator); + var reference = make("reference", comparator); + noInline(optimized); + noDFG(reference); + + var expected = []; + var expectedIndexOf8 = []; + for (var i = 0; i < 9; ++i) { + var sorted = reference(i); + expected.push(sorted.join()); + expectedIndexOf8.push(sorted.indexOf(8)); + } + test(optimized, expected, expectedIndexOf8); +} diff --git a/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp b/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp index 48fd3625c796..98e956591dd5 100644 --- a/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp +++ b/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp @@ -932,6 +932,20 @@ class PredictionPropagationPhase : public Phase { break; } + case PutCellButterflySlot: { + // The inlined sort stores this value as is, and ArraySortCommit copies the scratch as is into + // the array. For an ArrayWithInt32 the sort also reads the scratch slots back as Int32s with + // no check. A variable in the double format gives a boxed double here, also for an element + // that came out of the array as an Int32. The sort would then compare other numbers than the + // elements and leave boxed doubles in an Int32 array. If this node gets an array mode, only + // Array::Int32 needs this. + m_graph.voteChildren(node, VoteValue, weight); + Node* value = node->child3().node(); + if (value->op() == GetLocal) + value->variableAccessData()->mergeDoubleFormatState(CantUseDoubleFormat); + break; + } + case DataViewSet: { DataViewData data = node->dataViewData(); if (data.isFloatingPoint) From 252ca22859778a31fcf44829d4dc03a3370db097 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Fri, 16 Oct 2026 18:01:50 +0000 Subject: [PATCH 2/2] [JSC] Shorten the test and the comment for the inlined sort's pivot format --- ...ray-sort-inline-int32-double-comparator.js | 47 +++---------------- .../dfg/DFGPredictionPropagationPhase.cpp | 8 +--- 2 files changed, 8 insertions(+), 47 deletions(-) diff --git a/JSTests/stress/array-sort-inline-int32-double-comparator.js b/JSTests/stress/array-sort-inline-int32-double-comparator.js index 452afc34448b..583f227c9871 100644 --- a/JSTests/stress/array-sort-inline-int32-double-comparator.js +++ b/JSTests/stress/array-sort-inline-int32-double-comparator.js @@ -1,49 +1,14 @@ -//@ defaultRun -//@ runNoCJIT("--validateGraphAtEachPhase=true") - -// The inlined sort keeps the pivot of its insertion sort in a DFG variable, stores it back into the scratch -// as is, and reads the scratch of an Int32 array back with no type check. A comparator that does double -// arithmetic on its first parameter must not make that variable a double. The sort would then store boxed -// doubles into an ArrayWithInt32, compare other numbers than the elements, and return them out of order. - function shouldBe(actual, expected) { if (actual !== expected) throw new Error("bad value: " + actual + " expected: " + expected); } -var float64 = new Float64Array([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17]); - -function make(name, comparator) { - return new Function("float64", `return function ${name}(i) { return [17, 8, i % 9 + 1, i * 7 % 9 + 1].sort(${comparator}); };`)(float64); -} - -function test(optimized, expected, expectedIndexOf8) { - for (var i = 0; i < testLoopCount; ++i) { - var actual = optimized(i); - // join() and indexOf() read the elements of an ArrayWithInt32 as Int32s. - shouldBe(actual.join(), expected[i % 9]); - shouldBe(actual.indexOf(8), expectedIndexOf8[i % 9]); - } +// Double arithmetic on the comparator's first parameter must not make the inlined sort store its pivot +// back into the Int32 array as a double. +function test(a, b, c, d) { + return [a, b, c, d].sort((x, y) => (x + 0.5) - (y + 0.5)); } noInline(test); -var comparators = [ - "(a, b) => (a + 0.5) - (b + 0.5)", - "(a, b) => a - float64[b]", -]; - -for (var comparator of comparators) { - var optimized = make("optimized", comparator); - var reference = make("reference", comparator); - noInline(optimized); - noDFG(reference); - - var expected = []; - var expectedIndexOf8 = []; - for (var i = 0; i < 9; ++i) { - var sorted = reference(i); - expected.push(sorted.join()); - expectedIndexOf8.push(sorted.indexOf(8)); - } - test(optimized, expected, expectedIndexOf8); -} +for (var i = 0; i < testLoopCount; ++i) + shouldBe(test(17, 8, 1, 1).join(), "1,1,8,17"); diff --git a/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp b/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp index 770959b050b2..95fde70338e5 100644 --- a/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp +++ b/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp @@ -933,12 +933,8 @@ class PredictionPropagationPhase : public Phase { } case PutCellButterflySlot: { - // The inlined sort stores this value as is, and ArraySortCommit copies the scratch as is into - // the array. For an ArrayWithInt32 the sort also reads the scratch slots back as Int32s with - // no check. A variable in the double format gives a boxed double here, also for an element - // that came out of the array as an Int32. The sort would then compare other numbers than the - // elements and leave boxed doubles in an Int32 array. If this node gets an array mode, only - // Array::Int32 needs this. + // The inlined sort reads the scratch of an Int32 array back with no check, and ArraySortCommit + // copies it into the array as is. A pivot in the double format would be stored as a boxed double. m_graph.voteChildren(node, VoteValue, weight); Node* value = node->child3().node(); if (value->op() == GetLocal)