bun:ffi: fix f64 argument conversion of NaN, -0.0, and BigInts

[robobun]

What

f64/double arguments are silently corrupted before the native function sees them. With a C callee that reports what it actually received:

const lib = dlopen(path, {
  isnan_f64:   { args: ["f64"], returns: "i32" },  // x != x
  signbit_f64: { args: ["f64"], returns: "i32" },  // raw sign bit of x
  echo_f64:    { args: ["f64"], returns: "f64" },
});
lib.symbols.isnan_f64(NaN);          // 0: C received 0.0
lib.symbols.signbit_f64(-0.0);       // 0: C received +0.0
lib.symbols.echo_f64(-5n);           // 5: C received the absolute value
lib.symbols.echo_f64(2n ** 1024n);   // TypeError: Cannot mix BigInt and other types
lib.symbols.echo_f64(-(2n ** 1024n)) // +Infinity: wrong sign, no error

f32 arguments and the f64 return path are correct; the bug is specific to the f64 argument wrapper. These corrupt silently, and NaN, signed zero, and negative integers are exactly the values numeric C libraries assign meaning to.

Cause

ffiWrappers[FFIType.double] in src/js/bun/ffi.ts:

if (typeof val === "bigint") {
  if (val.valueOf() < BigInt(Number.MAX_VALUE)) {
    return Math.abs(Number(val).valueOf()) + (0.00 - 0.00);
  }
}
if (!val) {
  return 0 + (0.00 - 0.00);
}
return val + (0.00 - 0.00);

• !val is true for NaN and -0.0, so both become +0.0.
• The BigInt branch looks like it meant to range-check |val| < Number.MAX_VALUE and return Number(val), but the Math.abs ended up on the return value, so every negative BigInt loses its sign.
• A BigInt at or above Number.MAX_VALUE falls through to val + (0.00 - 0.00), which throws TypeError: Cannot mix BigInt and other types; one at or below -Number.MAX_VALUE passes the < check and comes out as +Infinity.
• A string argument survives the wrapper as a string ("2.5" + 0 is "2.50"), so the compiled stub reinterprets a JSString pointer as a double.

The native decoder (JSVALUE_TO_DOUBLE in src/runtime/ffi/FFI.h) already handles every JS number, including NaN, -0.0, and int32-tagged values (covered by the existing "integral JS numbers reach C as the exact double" test). The wrapper's only job is to guarantee the stub receives a plain number.

Fix

if (typeof val === "number") {
  return val;
}
return Number(val);

Numbers pass through bit-exact. Everything else is converted with Number(): BigInt keeps its sign (out-of-range values become +/-Infinity instead of throwing, matching Number(bigint) everywhere else), strings and objects go through ToNumber, and undefined becomes NaN instead of 0.0 (matching the f32 wrapper, which is Math.fround(val)).

Test

Added to the existing double <-> JSValue conversions group in test/js/bun/ffi/cc.test.ts. The fixture tinycc-compiles C observers (x != x, the raw sign bit via a union, an echo) and calls them through CFunction, which uses the same FFIBuilder/ffiWrappers argument path as dlopen (the dlopen suite in ffi.test.js is gated on a prebuilt library that no longer gets built, and cc() symbols currently skip the argument wrappers entirely, so calling them directly would not cover this). Assertions are on the values C reports, so a JS round trip cannot mask an argument bug.

On the unfixed build, the one assertion reports every corruption:

nan_isnan:             expected 1, got 0
negative_zero_signbit: expected 1, got 0
negative_bigint:       expected "-5", got "5"
huge_bigint:           expected Infinity, got a thrown TypeError
negative_huge_bigint:  expected "-Infinity", got "Infinity"
string:                expected "2.5", got "NaN" (JSString pointer read as a double)
undefined_arg:         expected "NaN", got "0"

With the fix, bun bd test test/js/bun/ffi/ passes (18 pass, 0 fail, ASAN debug build).

Out of scope, and overlap with other open PRs

The integer argument wrappers in the same table have their own problems (saturation vs. two's-complement wrapping is inconsistent across widths, and int16_t's clamp bound of 32768 does not fit in int16). That is a behavior decision beyond this bug, and #31449 already proposes changes there.

• fix(ffi): multiple long-standing FFI correctness bugs #31449 also fixes the BigInt sign half of this wrapper, but keeps the !val branch, so NaN and -0.0 arguments are still corrupted with that patch applied.
• bun:ffi: coerce JSCallback return values to the declared return type #33095 fixes a different bug (JSCallback return values were never coerced). Callback returns reuse the ffiWrappers table, so that PR rewrites this same FFIType.double entry to an equivalent conversion. The src hunk overlaps; the subjects and most of the coverage do not (that PR asserts what C receives from callback returns, this one asserts what a symbol receives as an f64 argument, including the sign bit of -0.0 and out-of-range BigInts). Whichever lands second is a one-hunk rebase, and this test applies unchanged either way.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 2 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: eb3f9d8b-dbd4-4b66-bc69-548c73c7acc9

📥 Commits

Reviewing files that changed from the base of the PR and between 6c1f36a and 56c70c4.

📒 Files selected for processing (2)

• src/js/bun/ffi.ts
• test/js/bun/ffi/cc.test.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

^{Updated 10:00 PM PT - Jun 29th, 2026}

❌ @robobun, your commit 56c70c4 has 3 failures in Build #67064 (All Failures):

• test/js/bun/http/fetch-file-upload.test.ts - timeout on 🍎 14 aarch64
• test/js/bun/http/bun-serve-file.test.ts - timeout on 🍎 14 aarch64
• test/js/node/test/parallel/test-net-connect-memleak.js - code 1 on 🐧 3.23 x64-baseline
• test/js/node/test/parallel/test-net-connect-memleak.js - code 1 on 🐧 3.23 x64

---

🧪   To try this PR locally:

bunx bun-pr 33122

That installs a local version of the PR into your bun-33122 executable, so you can run:

bun-33122 --bun

[github-actions]

This PR may be a duplicate of:

1. fix(ffi): multiple long-standing FFI correctness bugs #31449 - Also patches ffiWrappers[FFIType.double] to fix BigInt sign stripping (Math.abs) and BigInt overflow bugs in the same code block
2. bun:ffi: coerce JSCallback return values to the declared return type #33095 - Replaces the entire ffiWrappers[FFIType.double] body with an equivalent fix (typeof val === "bigint" ? Number(val) : +val), resolving all three of the same bugs (NaN, -0.0, BigInt)

🤖 Generated with Claude Code

[robobun]

Looked at both.

#31449 fixes the BigInt sign and overflow in this wrapper but keeps the if (!val) return 0 branch, so NaN and -0.0 arguments still reach C as +0.0 with that patch applied. Not a duplicate; the PR body notes how the two relate.

#33095 fixes a different bug (JSCallback return values were never coerced). Callback returns reuse the ffiWrappers table, so that PR rewrites this same FFIType.double entry to an equivalent expression and the src hunk does overlap. The coverage is different: #33095 asserts what C receives from a callback's return value (plus a few f64 argument probes), this PR asserts what a dlopen/CFunction symbol receives as an f64 argument, including the sign bit of -0.0, BigInts beyond double range in both signs, and string/undefined arguments. Whichever of the two lands second rebases a single hunk, and the tests from both apply unchanged.

Keeping this PR scoped to the f64 argument path; I updated the description to point at both.

[claude]

LGTM — the new wrapper is strictly simpler and more correct than what it replaces, and the test pins the exact edge cases (NaN, -0.0, BigInt sign, huge BigInt, string) that the old code corrupted.

Extended reasoning...

Overview

This PR replaces the ffiWrappers[FFIType.double] argument coercion in src/js/bun/ffi.ts (~10 lines of buggy logic involving !val, Math.abs, and + (0.00 - 0.00)) with a 4-line wrapper: pass numbers through untouched, otherwise Number(val). A new test in test/js/bun/ffi/cc.test.ts compiles C observers (isnan, signbit, echo) via cc() and calls them through CFunction so the FFIBuilder/ffiWrappers path is exercised, asserting on what C actually received.

Security risks

None. This is JS-side coercion of arguments before they reach the compiled FFI stub. The old code could let a JSString survive the wrapper and be reinterpreted as a double in native code; the new code closes that off by always producing a plain number. No auth, crypto, or permission surfaces are touched.

Level of scrutiny

Low-to-moderate. The functional change is a single, small wrapper string whose new behavior is trivially analyzable: typeof val === "number" ? val : Number(val). I verified that JSVALUE_TO_DOUBLE in src/runtime/ffi/FFI.h already handles both int32-tagged and double-encoded JSValues, so passing the number through unchanged is correct (NaN and -0.0 included). The only intentional behavior change beyond the bug fixes is undefined → NaN instead of 0.0, which the PR calls out explicitly and which matches the existing f32 wrapper (Math.fround(undefined)).

Other factors

• No CODEOWNERS apply to the changed paths.
• No prior human review comments to address.
• The added test follows the established subprocess-fixture pattern in the same describe block, includes positive controls (1.5 is not NaN, +0 has signbit 0), and reports thrown conversions as values so one failure cannot mask another.
• The PR description documents the interaction with #31449 and scopes out the unrelated integer-wrapper issues.

[robobun]

CI status for build 67064 (56c70c4), now finished: the change and its test are green on every lane that ran them, and each red lane is unrelated to this PR.

• darwin 26 aarch64 - test-bun (both shards): buildkite-agent artifact download timed out after 120s for step 'darwin-aarch64-build-bun'. The runner refused to continue without the binary, so no tests ran on those shards.
• darwin 14 aarch64 - test-bun (1 shard): test/js/bun/http/fetch-file-upload.test.ts > uploads roundtrip with sendfile() timed out after 10000ms on every in-job retry. The FFI test files in that same shard (ffi.test.js, addr32.test.ts, ffi-error-messages.test.ts) all passed.
• alpine 3.23 x64 - test-bun and alpine 3.23 x64-baseline - test-bun: test/js/node/test/parallel/test-net-connect-memleak.js fails assert.strictEqual(collected, true) after globalThis.gc(). The same test fails the same way on those lanes in other PRs' builds, for example 67082 and 67074, so it is independent of this change.

test/js/bun/ffi/cc.test.ts (which contains the new test) ran on the debian 13 x64-asan lane and passed (4 pass, 0 fail), and all debian, ubuntu, windows, and remaining darwin test lanes are green. Not pushing a retrigger: the alpine failure reproduces on other branches, so a re-run cannot go green until that test is fixed or quarantined. Ready for review as is.