Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
function shouldBe(actual, expected, message) {
if (actual !== expected)
throw new Error(message + ": got " + actual + ", expected " + expected);
}

// indexOf and lastIndexOf use IsStrictlyEqual and includes uses SameValueZero, on Number values.
// A needle that the element type cannot represent exactly is equal to no element. The result
// must not depend on whether the engine holds the needle as an int32 or as a double.

const scratch = new Float64Array(1);
function asDouble(value)
{
scratch[0] = value;
return scratch[0];
}

function checkOne(array, needle, expected, name)
{
shouldBe(array.indexOf(needle), expected, name + " indexOf");
shouldBe(array.lastIndexOf(needle), expected, name + " lastIndexOf");
shouldBe(array.includes(needle), expected !== -1, name + " includes");
}

function check(array, needle, expected)
{
const name = array.constructor.name + " " + needle;
checkOne(array, needle, expected, name);
checkOne(array, asDouble(needle), expected, name + " as double");
}

function test()
{
{
const array = new Float32Array([16777216, 2147483648, -16777216, -2147483648, 1]);
shouldBe(array[1], 2147483648, "Float32Array holds 2^31");

check(array, 16777216, 0);
check(array, 16777217, -1); // rounds to 16777216 as a float
check(array, 16777216.5 + 0.5, -1);
check(array, parseInt("16777217"), -1);
check(array, 2147483647, -1); // rounds to 2147483648 as a float
check(array, 2147483648, 1);
check(array, -16777216, 2);
check(array, -16777217, -1);
check(array, -2147483647, -1);
check(array, -2147483648, 3);
check(array, 1, 4);
}

{
const array = new Float16Array([Infinity, 2048, 65504, -Infinity, -2048, 1]);

check(array, Infinity, 0);
check(array, 65520, -1); // rounds to Infinity as a half
check(array, 65535, -1);
check(array, 65536, -1);
check(array, 2147483647, -1);
check(array, 2048, 1);
check(array, 2049, -1); // rounds to 2048 as a half
check(array, 65504, 2);
check(array, 65505, -1); // rounds to 65504 as a half
check(array, 65519, -1);
check(array, -Infinity, 3);
check(array, -65520, -1);
check(array, -2147483648, -1);
check(array, -2048, 4);
check(array, -2049, -1);
check(array, 1, 5);
}

{
// Every int32 is a double, so a Float64Array is unaffected.
const array = new Float64Array([2147483647, -2147483648, 16777217]);
check(array, 2147483647, 0);
check(array, -2147483648, 1);
check(array, 16777217, 2);
check(array, 16777216, -1);
}

{
const array = new Int32Array([2147483647, -2147483648, 16777217]);
check(array, 2147483647, 0);
check(array, -2147483648, 1);
check(array, 16777217, 2);
check(array, 16777216, -1);
}
}

for (let i = 0; i < 1e3; ++i)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 (optional) New stress test hardcodes 1e3 iterations instead of using testLoopCount, violating JSTests/README.md rule 2 (imported by JSTests/CLAUDE.md): tests must use testLoopCount so the harness can tune iteration count per configuration for tier-up and early exit. Fix: replace for (let i = 0; i < 1e3; ++i) with for (let i = 0; i < testLoopCount; ++i).

Extended reasoning...

JSTests/CLAUDE.md @-imports JSTests/README.md, whose "Adding Tests" section states new tests are required to use testLoopCount to control iteration count because the jsc CLI sets it based on the test configuration (so tests tier up where that matters and exit early where it doesn't). Line 89 of the new test uses a literal 1e3. In eager-tier configurations this may over-iterate (risking the 200ms budget in rule 1), and in no-JIT/interpreter-only configurations it wastes time; more importantly it breaks the mandated convention that lets the harness control tier coverage. Existing stress tests (e.g. JSTests/stress/zero-to-string.js) follow the testLoopCount pattern.

Verification: nit — /home/claude/webkit/JSTests/stress/typed-array-index-of-int32-needle-not-representable.js:89 reads for (let i = 0; i < 1e3; ++i), a hardcoded iteration count. /home/claude/webkit/JSTests/CLAUDE.md line 1 @-imports README.md, and /home/claude/webkit/JSTests/README.md:17-20 states new tests are "required" to "Use testLoopCount … to control how many iterations a test runs. The jsc…

test();
12 changes: 10 additions & 2 deletions Source/JavaScriptCore/runtime/TypedArrayAdaptors.h
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,11 @@ struct FloatTypedArrayAdaptor {

static std::optional<Type> toNativeFromInt32WithoutCoercion(int32_t value)
{
return static_cast<Type>(value);
// An int32 with magnitude above 2^24 can round when converted to float.
Type valueResult = static_cast<Type>(value);
if (static_cast<double>(valueResult) != value)
return std::nullopt;
return valueResult;
}

static std::optional<Type> toNativeFromDoubleWithoutCoercion(double value)
Expand Down Expand Up @@ -400,7 +404,11 @@ struct Float16Adaptor {

static std::optional<Type> toNativeFromInt32WithoutCoercion(int32_t value)
{
return static_cast<double>(value);
// An int32 with magnitude above 2^11 can round when converted to half, and one at or above 65520 becomes an infinity.
Type valueResult { static_cast<double>(value) };
if (static_cast<double>(valueResult) != value)
return std::nullopt;
return valueResult;
}

static std::optional<Type> toNativeFromDoubleWithoutCoercion(double value)
Expand Down
Loading