Bug 312687 – Incorrect Math.round Result via floor(x+0.5) JIT Fast Path

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

Created attachment 479202 [details]
poc.js

## Summary

The DFG and FTL JIT compilers in JavaScriptCore contain a fast path for `Math.round()` that computes `floor(x + 0.5)` when the result is consumed as an integer and negative-zero checking is disabled. This fast path produces the wrong result for `0.49999999999999994` — the largest IEEE 754 double strictly less than `0.5` — where IEEE 754 round-to-nearest-even causes `x + 0.5` to round up to `1.0`, so `floor(1.0) = 1` instead of the correct `0`. The interpreter and the JIT's own non-fast path both return `0`; the bug is introduced exclusively by the fast path.

## Bug

### Summary

The FTL compiler's `compileArithRound()` and the DFG compiler's `compileArithRounding()` each have a fast path that uses `floor(x + 0.5)` instead of the correct `ceil`-based algorithm when `producesInteger(mode) && !shouldCheckNegativeZero(mode)`. The `floor(x + 0.5)` formula is a well-known incorrect approximation of mathematical rounding due to IEEE 754 intermediate precision loss. For `x = 0.49999999999999994`, the addition `x + 0.5` rounds to exactly `1.0` under IEEE 754 round-to-nearest-even, so `floor(1.0) = 1` instead of the correct result `0`.

### Detail

The buggy code in the FTL compiler:

```cpp
// Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:3702-3704
if (producesInteger(m_node->arithRoundingMode()) && !shouldCheckNegativeZero(m_node->arithRoundingMode())) {
    LValue value = lowDouble(m_node->child1());
    result = m_out.doubleFloor(m_out.doubleAdd(value, m_out.constDouble(0.5)));
}
```

The equivalent buggy code in the DFG compiler:

```cpp
// Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:6678-6681
if (producesInteger(node->arithRoundingMode()) && !shouldCheckNegativeZero(node->arithRoundingMode())) {
    move64ToDouble(TrustedImm64(std::bit_cast<uint64_t>(0.5)), resultFPR);
    addDouble(valueFPR, resultFPR);
    floorDouble(resultFPR, resultFPR);
}
```

Both paths compute `floor(x + 0.5)` to approximate `Math.round(x)`.

The correct implementation used in `MathCommon.cpp` and in both compilers' non-fast paths:

```cpp
// Source/JavaScriptCore/runtime/MathCommon.cpp:567-571
static ALWAYS_INLINE double roundDoubleImpl(double value)
{
    double integer = ceil(value);
    return integer - (integer - 0.5 > value);
}
```

The FTL non-fast path (the correct `else` branch at lines 3705-3722):

```cpp
// Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:3706-3722
LBasicBlock shouldRoundDown = m_out.newBlock();
LBasicBlock continuation = m_out.newBlock();

LValue value = lowDouble(m_node->child1());
LValue integerValue = m_out.doubleCeil(value);
ValueFromBlock integerValueResult = m_out.anchor(integerValue);

LValue ceilMinusHalf = m_out.doubleSub(integerValue, m_out.constDouble(0.5));
m_out.branch(m_out.doubleGreaterThanOrUnordered(ceilMinusHalf, value), unsure(shouldRoundDown), unsure(continuation));

LBasicBlock lastNext = m_out.appendTo(shouldRoundDown, continuation);
LValue integerValueRoundedDown = m_out.doubleSub(integerValue, m_out.constDouble(1));
ValueFromBlock integerValueRoundedDownResult = m_out.anchor(integerValueRoundedDown);
m_out.jump(continuation);
m_out.appendTo(continuation, lastNext);

result = m_out.phi(Double, integerValueResult, integerValueRoundedDownResult);
```

**Why the fast path fails for `x = 0.49999999999999994`:**

1. `0.49999999999999994` is the largest IEEE 754 double-precision value strictly less than `0.5` (bit pattern `3FDFFFFFFFFFFFFF`; the next representable double `3FE0000000000000` is exactly `0.5`).
2. The fast path computes `x + 0.5`. The true mathematical result `0.99999999999999994` lies between the two adjacent representable doubles `0.9999999999999999` and `1.0`. Under IEEE 754 round-to-nearest-even, this rounds to `1.0`.
3. Therefore `floor(1.0) = 1`, which is wrong.

**Why the correct (ceil-based) path works:**

1. `ceil(0.49999999999999994) = 1.0`
2. `ceilMinusHalf = 1.0 − 0.5 = 0.5` (exact, since `0.5` is representable)
3. `0.5 > 0.49999999999999994` is `true`
4. Result = `1.0 − 1 = 0.0` ✓

The fast path is activated when the DFG `FixupPhase` determines:
- The result is predicted to be Int32 or Boolean (from `isInt32OrBooleanSpeculation(node->getHeapPrediction())`)
- `roundShouldSpeculateInt32` returns `true`
- `bytecodeCanIgnoreNegativeZero` returns `true` (when `Math.round(x)` feeds into `| 0` or any integer context)

This sets `ArithRoundingMode::Int32`, making `producesInteger()` return `true` and `shouldCheckNegativeZero()` return `false`, entering the fast path in both compilers.

### Trigger Conditions

1. A function calls `Math.round(x)` with the result consumed by a bitwise operation (e.g., `| 0`), setting `bytecodeCanIgnoreNegativeZero` and causing `ArithRoundingMode::Int32`
2. The function is JIT-compiled by DFG or FTL (requires ~50,000+ iterations with double inputs)
3. The input `0.49999999999999994` (the largest IEEE 754 double less than `0.5`) is passed to the compiled function
4. No special flags or builtins are required — pure JavaScript triggers this

## Version

### Reproduced Version

- `main` branch HEAD (2026-04-17): `a4390137a4038c07661f58e45ee205b5a623f9dd`

## Reproduction Case

A single PoC demonstrates the bug by JIT-compiling a function using `Math.round(x) | 0` and then passing the critical value.

### Release Build

```bash
WebKitBuild/JSCOnly/Release/bin/jsc poc.js
```

Result:

```
0 1
```

Debug build produces the same output; no assertions fire in either build.

The first value (`0`) is the unoptimized (interpreter) result; the second (`1`) is the FTL-optimized result.

### PoC Code

```js
function f(x) { return Math.round(x) | 0; }

const unopt = f(0.49999999999999994);
for (let i = 0; i < 200000; i++) f(i * 0.1);
const opt = f(0.49999999999999994);

print(unopt, opt); // expected: 0 0 — actual: 0 1
```

## Suggested Patch

The fix removes the incorrect `floor(x + 0.5)` fast path in both compilers, letting the existing `ceil`-based algorithm — already present in the `else` branch — handle all inputs unconditionally. No new code is needed; the correct path already exists. Tested: patched binary outputs `0 0`; reverted binary outputs `0 1`.

```diff
diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
index 29003297b1dd..7ffccf7bbec1 100644
--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
@@ -6675,11 +6675,7 @@ void SpeculativeJIT::compileArithRounding(Node* node)
             case ArithRound: {
                 FPRTemporary result(this);
                 FPRReg resultFPR = result.fpr();
-                if (producesInteger(node->arithRoundingMode()) && !shouldCheckNegativeZero(node->arithRoundingMode())) {
-                    move64ToDouble(TrustedImm64(std::bit_cast<uint64_t>(0.5)), resultFPR);
-                    addDouble(valueFPR, resultFPR);
-                    floorDouble(resultFPR, resultFPR);
-                } else {
+                {
                     ceilDouble(valueFPR, resultFPR);
 
                     FPRTemporary scratch(this);
diff --git a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
index a11da9de3b88..153826f5876a 100644
--- a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
+++ b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
@@ -3699,10 +3699,7 @@ private:
         JSGlobalObject* globalObject = m_graph.globalObjectFor(m_origin.semantic);
         if (m_node->child1().useKind() == DoubleRepUse) {
             LValue result = nullptr;
-            if (producesInteger(m_node->arithRoundingMode()) && !shouldCheckNegativeZero(m_node->arithRoundingMode())) {
-                LValue value = lowDouble(m_node->child1());
-                result = m_out.doubleFloor(m_out.doubleAdd(value, m_out.constDouble(0.5)));
-            } else {
+            {
                 LBasicBlock shouldRoundDown = m_out.newBlock();
                 LBasicBlock continuation = m_out.newBlock();
```

Both diffs simply remove the incorrect fast path and promote the existing `else` branch to unconditional execution. The `ceil`-based algorithm already handles the `producesInteger && !shouldCheckNegativeZero` case correctly — the difference is one additional `ceilDouble` instruction plus a branch (well-predicted as not-taken for typical integer values), which is negligible overhead. This matches the algorithm in `MathCommon.cpp::roundDoubleImpl()` and the interpreter.

### Credit Information

Reporter credit: Junyoung Park (@candymate) of KAIST Hacking Lab

[Radar WebKit Bug Importer]

<rdar://problem/175122231>

[Kai Tamkun]

Pull request: https://github.com/WebKit/WebKit/pull/63337

[EWS]

Committed 313090@main (a64b9e0912a4): <https://commits.webkit.org/313090@main>

Reviewed commits have been landed. Closing PR #63337 and removing active labels.

[EWS]

Committed 305413.889@safari-7624-branch (6852c1905a40): <https://commits.webkit.org/305413.889@safari-7624-branch>

Reviewed commits have been landed. Closing PR #5247 and removing active labels.