Engine-native FFI (bun:ffi) under USE(BUN_JSC_ADDITIONS) - #319
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughChangesFFI integration
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@JSTests/stress/ffi-arity.js`:
- Around line 75-76: Update the comment immediately above the addF32 check to
describe the actual missing-argument behavior as undefined/NaN, matching the
asserted NaN result and the corresponding addF64 case; leave the assertion
unchanged.
In `@JSTests/stress/ffi-raw-read.js`:
- Around line 1-12: Add the top-level $vm.useJIT() guard used by
ffi-typedarray-storage-modes.js before the ffiFunction setup in this test, so
the entire test is skipped when JIT is disabled while retaining the existing
behavior when JIT is available.
In `@Source/JavaScriptCore/dfg/DFGMayExit.cpp`:
- Around line 225-230: Move the USE(BUN_JSC_ADDITIONS)-guarded FFIRawRead case
out of the preceding fall-through chain and place it after the
ExitsForExceptions group has terminated. Preserve FFIRawRead’s result = Exits
behavior while ensuring the empty cases before it continue reaching result =
ExitsForExceptions.
In `@Source/JavaScriptCore/dfg/DFGSafeToExecute.h`:
- Around line 349-351: Remove FFIRawRead from the safe-to-hoist switch cases in
safeToExecute() and route it through the return-false path. Preserve safe
handling for the other node types, ensuring FFIRawRead cannot be code-motion
optimized without proven pointer validity and memory ordering.
In `@Source/JavaScriptCore/ffi/FFIDFG.cpp`:
- Around line 219-224: Update the Type::Pointer, Type::CString, Type::Function,
and Type::Buffer branch in the CallFFI return-type speculation logic to include
SpecHeapBigInt alongside SpecBytecodeNumber and SpecOther. Preserve the existing
null-to-jsNull and numeric representation behavior while ensuring the abstract
interpreter accounts for exact JSBigInt results.
In `@Source/JavaScriptCore/ffi/FFIRawMemory.h`:
- Around line 49-55: Update the documentation above createReadObject to state
that raw read address inputs are limited to numeric values and BigInt, and that
views, ArrayBuffers, null, and undefined are not accepted; callers must convert
supported pointer-like inputs first. Remove the inaccurate claim that raw
readers accept every ptr argument type while preserving the existing
no-bounds-checking and reader behavior descriptions.
In `@Source/JavaScriptCore/tools/JSDollarVM.cpp`:
- Around line 4545-4569: Update dollarVMParseFFIType to delegate FFI type
conversion to the exported FFI::typeFromJS helper instead of duplicating numeric
and string parsing. Preserve the existing exception-scope handling and confirm
the $vm API does not require its current TypeError wording before removing the
local validation and parse logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0b0b2b2d-f911-490f-a8a9-ef9439a7274a
📒 Files selected for processing (80)
JSTests/stress/ffi-align.jsJSTests/stress/ffi-arity-ladders.jsJSTests/stress/ffi-arity.jsJSTests/stress/ffi-callbacks.jsJSTests/stress/ffi-callffi-was-compiled.jsJSTests/stress/ffi-canary.jsJSTests/stress/ffi-conversion-errors-host.jsJSTests/stress/ffi-conversion-errors.jsJSTests/stress/ffi-fuzz-signatures.jsJSTests/stress/ffi-host-path.jsJSTests/stress/ffi-napi.jsJSTests/stress/ffi-no-jit.jsJSTests/stress/ffi-osr-and-exceptions.jsJSTests/stress/ffi-pointers-and-buffers.jsJSTests/stress/ffi-raw-read.jsJSTests/stress/ffi-signature-errors.jsJSTests/stress/ffi-subword-and-returns.jsJSTests/stress/ffi-tier-differential.jsJSTests/stress/ffi-typedarray-storage-modes.jsJSTests/stress/ffi-types-echo.jsSource/JavaScriptCore/CMakeLists.txtSource/JavaScriptCore/Sources.txtSource/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.hSource/JavaScriptCore/dfg/DFGByteCodeParser.cppSource/JavaScriptCore/dfg/DFGClobberize.hSource/JavaScriptCore/dfg/DFGDataViewData.hSource/JavaScriptCore/dfg/DFGDoesGC.cppSource/JavaScriptCore/dfg/DFGFixupPhase.cppSource/JavaScriptCore/dfg/DFGMayExit.cppSource/JavaScriptCore/dfg/DFGNode.cppSource/JavaScriptCore/dfg/DFGNode.hSource/JavaScriptCore/dfg/DFGNodeType.hSource/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cppSource/JavaScriptCore/dfg/DFGSafeToExecute.hSource/JavaScriptCore/dfg/DFGSpeculativeJIT.hSource/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cppSource/JavaScriptCore/dfg/DFGSpeculativeJIT64.cppSource/JavaScriptCore/dfg/DFGStrengthReductionPhase.cppSource/JavaScriptCore/ffi/BunFFI.cppSource/JavaScriptCore/ffi/BunFFI.hSource/JavaScriptCore/ffi/FFICallHost.cppSource/JavaScriptCore/ffi/FFICallHost.hSource/JavaScriptCore/ffi/FFICallbackThunk.cppSource/JavaScriptCore/ffi/FFICallbackThunk.hSource/JavaScriptCore/ffi/FFICallingConvention.cppSource/JavaScriptCore/ffi/FFICallingConvention.hSource/JavaScriptCore/ffi/FFIContext.cppSource/JavaScriptCore/ffi/FFIContext.hSource/JavaScriptCore/ffi/FFIConversions.cppSource/JavaScriptCore/ffi/FFIConversions.hSource/JavaScriptCore/ffi/FFIDFG.cppSource/JavaScriptCore/ffi/FFIDFG.hSource/JavaScriptCore/ffi/FFIDFGCodegen.cppSource/JavaScriptCore/ffi/FFIICStub.cppSource/JavaScriptCore/ffi/FFIICStub.hSource/JavaScriptCore/ffi/FFIInvokeThunk.cppSource/JavaScriptCore/ffi/FFIInvokeThunk.hSource/JavaScriptCore/ffi/FFIRawMemory.cppSource/JavaScriptCore/ffi/FFIRawMemory.hSource/JavaScriptCore/ffi/FFISignature.cppSource/JavaScriptCore/ffi/FFISignature.hSource/JavaScriptCore/ffi/FFIType.hSource/JavaScriptCore/ffi/JSFFICallback.cppSource/JavaScriptCore/ffi/JSFFICallback.hSource/JavaScriptCore/ffi/JSFFIFunction.cppSource/JavaScriptCore/ffi/JSFFIFunction.hSource/JavaScriptCore/ffi/tests/FFITestFixtures.cppSource/JavaScriptCore/ffi/tests/FFITestFixtures.hSource/JavaScriptCore/ffi/tests/testFFI.cppSource/JavaScriptCore/ftl/FTLCapabilities.cppSource/JavaScriptCore/ftl/FTLLowerDFGToB3.cppSource/JavaScriptCore/heap/Heap.cppSource/JavaScriptCore/heap/Heap.hSource/JavaScriptCore/runtime/Intrinsic.hSource/JavaScriptCore/runtime/JSGlobalObject.cppSource/JavaScriptCore/runtime/JSGlobalObject.hSource/JavaScriptCore/runtime/OptionsList.hSource/JavaScriptCore/runtime/VM.hSource/JavaScriptCore/shell/CMakeLists.txtSource/JavaScriptCore/tools/JSDollarVM.cpp
Preview Builds
|
Implements the machinery bun:ffi needs directly in JavaScriptCore, so Bun can create FFI functions and callbacks without generating and JIT-compiling a C trampoline per symbol with TinyCC. Core (Source/JavaScriptCore/ffi/): - FFI::Type / FFI::Signature: interned signatures, wire-compatible with Bun's existing FFIType tags. - FFICallingConvention: SysV x86-64, AAPCS64 (incl. Apple sub-word stack packing) and Win64 argument/return classification. - FFIInvokeThunk: one JIT'd, signature-pure invoke thunk per signature; the only code that emits the native callee ABI. Every tier funnels through a canonical uint64_t slot buffer. - JSFFIFunction: a JSFunction subclass. Calls run through a C++ host path or, when useFFIICStub is on, a per-function JIT'd entry stub installed as the executable's call code, with a shared C++ slow path for coercion misses. - JSFFICallback + FFICallbackThunk: C-callable JIT'd trampolines that box native arguments and call back into JS, leaving exceptions pending for the outer FFI call site. FFI::CallbackEntryScope suspends the exception-check verifier's outstanding obligation across the foreign-frame re-entry. - FFIConversions: the JS<->native conversion rules, shared by every tier. Numeric params accept numbers, booleans, null/undefined and BigInts and wrap to width (never clamp); strings/Symbols throw. Pointers above 2^53 are surfaced as exact BigInts and BigInt addresses are accepted. - FFIRawMemory: the bun:ffi `read` singleton (u8..f64/ptr/intptr) with unaligned-safe host paths, plus a shared FFIRawReadIntrinsic. JIT integration: - CallFFI DFG/FTL node: created by strength reduction from a Call whose callee is a constant JSFFIFunction (mirroring CallWasm). Arguments are speculated to their declared types and written unboxed into a stack slot buffer; no CallFrame is built for the native call. - FFIRawRead DFG/FTL node: the DataView typed-load machinery with the base taken from the (Int52 / Int32 / truncated-double) address argument; no bounds check by design. DataViewData is split out into DFGDataViewData.h. Tests: an ffi/tests C++ harness (testFFI: calling-convention goldens, the conversion matrix, and a differential of the invoke thunk against clang calling the same fixtures natively) plus JSTests/stress/ffi-*.js covering types, arity, sub-word/return normalization, pointers/buffers/typed-array storage modes, callbacks (marshaling, exceptions, GC, re-entrancy), callee-saved canaries, stack alignment, OSR exits, tier differentials, the raw-memory readers, and a seeded conversion fuzzer.
Address defects found in code review of the engine-native FFI change: - DFGMayExit.cpp: the FFIRawRead case had been spliced into a fall-through case group, so under USE(BUN_JSC_ADDITIONS) ~30 node types (Call/New*/ RegExpExec*/CallWasm/...) fell into `result = Exits` and never reached their intended ExitsForExceptions. Move the case after the group's body. - All FFIRawRead `case` labels are now unconditional with a guarded body (DFG_CRASH under !USE(BUN_JSC_ADDITIONS)), matching the CallFFI convention; the guarded labels broke -Wswitch in every non-Bun port. - DFG CallFFI: add the missing exceptionCheck() after operationFFIBoxSlot (which allocates BigInts and can throw OOM); the FTL and IC stub twins already checked. - read.ptr / read.intptr now surface a plain double at every tier (Bun's reader contract), rather than the host path reusing the FFI ptr-return rule (null / BigInt) and diverging from the DFG/FTL lowering. - SafeToExecute: FFIRawRead returns false. A raw dereference must never be speculatively hoisted (e.g. by LICM) above its guarding null check. - The CallFFI pointer-family AI type now includes the BigInt speculations (an address above 2^53 boxes to a HeapBigInt), fixing an unsound type. - JSFFICallback::create eagerly materializes the FFIContext on the mutator, as JSFFIFunction::create already did. - Signature::invokeThunk(): lock-free published pointer for the per-call fast path; the lock is now taken only for one-time generation. - Reader-table signedness metadata and an Intrinsic.h comment corrected.
737093a to
a3a2455
Compare
Points WEBKIT_VERSION at the autobuild-preview-pr-319-737093a0 prebuilt so CI can build and test this branch without a local WebKit. Repin to the merged commit sha once the WebKit PR lands.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Source/JavaScriptCore/ffi/FFIDFG.cpp`:
- Around line 109-120: Update the narrow integer cases in the type switch around
operationFFIWriteSlot to guard Int32Use insertion with the argument’s
shouldSpeculateInt32() result; when speculation is not appropriate, use
UntypedUse so runtime conversion handles boxed or non-Int32 values. First verify
that operationFFIWriteSlot supports UntypedUse for Char, Int8, Uint8, Int16,
Uint16, Int32, and Uint32, preserving the existing fast path when speculation
succeeds.
In `@Source/JavaScriptCore/ffi/JSFFICallback.cpp`:
- Line 148: Update the callback pointer assignment in the JSFFICallback
construction path to preserve exact nativeEntrypoint() values: use a numeric
result for safely representable pointers and a BigInt result when the pointer
exceeds Number.MAX_SAFE_INTEGER, matching the existing FFI pointer conversion
contract.
In `@Source/JavaScriptCore/ftl/FTLCapabilities.cpp`:
- Around line 214-215: Guard all Bun-specific FFI opcode handling with
USE(BUN_JSC_ADDITIONS), matching Intrinsic.h: wrap the FFIRawRead and CallFFI
cases in FTLCapabilities.cpp, the unsupported-backend FFI cases in
DFGSpeculativeJIT32_64.cpp, and the CallFFI heap-prediction and cell-operand
cases in DFGNode.h at the specified ranges. Ensure these cases are excluded when
Bun additions are disabled.
In `@Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp`:
- Around line 1575-1586: Guard the entire CallFFI switch case in the FTL
lowering dispatch with USE(BUN_JSC_ADDITIONS), matching the existing FFIRawRead
guard. Keep compileCallFFI() and its case-specific handling within that
conditional so non-Bun builds do not reference the unavailable CallFFI enum.
In `@Source/JavaScriptCore/heap/Heap.cpp`:
- Around line 120-121: Wrap the JSFFICallback.h and JSFFIFunction.h includes in
`#if` USE(BUN_JSC_ADDITIONS) / `#endif` guards, matching the existing conditional
boundary used by the related initializer below.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1507f51c-dc1a-4ffb-840a-0b47507f15ac
📒 Files selected for processing (80)
JSTests/stress/ffi-align.jsJSTests/stress/ffi-arity-ladders.jsJSTests/stress/ffi-arity.jsJSTests/stress/ffi-callbacks.jsJSTests/stress/ffi-callffi-was-compiled.jsJSTests/stress/ffi-canary.jsJSTests/stress/ffi-conversion-errors-host.jsJSTests/stress/ffi-conversion-errors.jsJSTests/stress/ffi-fuzz-signatures.jsJSTests/stress/ffi-host-path.jsJSTests/stress/ffi-napi.jsJSTests/stress/ffi-no-jit.jsJSTests/stress/ffi-osr-and-exceptions.jsJSTests/stress/ffi-pointers-and-buffers.jsJSTests/stress/ffi-raw-read.jsJSTests/stress/ffi-signature-errors.jsJSTests/stress/ffi-subword-and-returns.jsJSTests/stress/ffi-tier-differential.jsJSTests/stress/ffi-typedarray-storage-modes.jsJSTests/stress/ffi-types-echo.jsSource/JavaScriptCore/CMakeLists.txtSource/JavaScriptCore/Sources.txtSource/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.hSource/JavaScriptCore/dfg/DFGByteCodeParser.cppSource/JavaScriptCore/dfg/DFGClobberize.hSource/JavaScriptCore/dfg/DFGDataViewData.hSource/JavaScriptCore/dfg/DFGDoesGC.cppSource/JavaScriptCore/dfg/DFGFixupPhase.cppSource/JavaScriptCore/dfg/DFGMayExit.cppSource/JavaScriptCore/dfg/DFGNode.cppSource/JavaScriptCore/dfg/DFGNode.hSource/JavaScriptCore/dfg/DFGNodeType.hSource/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cppSource/JavaScriptCore/dfg/DFGSafeToExecute.hSource/JavaScriptCore/dfg/DFGSpeculativeJIT.hSource/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cppSource/JavaScriptCore/dfg/DFGSpeculativeJIT64.cppSource/JavaScriptCore/dfg/DFGStrengthReductionPhase.cppSource/JavaScriptCore/ffi/BunFFI.cppSource/JavaScriptCore/ffi/BunFFI.hSource/JavaScriptCore/ffi/FFICallHost.cppSource/JavaScriptCore/ffi/FFICallHost.hSource/JavaScriptCore/ffi/FFICallbackThunk.cppSource/JavaScriptCore/ffi/FFICallbackThunk.hSource/JavaScriptCore/ffi/FFICallingConvention.cppSource/JavaScriptCore/ffi/FFICallingConvention.hSource/JavaScriptCore/ffi/FFIContext.cppSource/JavaScriptCore/ffi/FFIContext.hSource/JavaScriptCore/ffi/FFIConversions.cppSource/JavaScriptCore/ffi/FFIConversions.hSource/JavaScriptCore/ffi/FFIDFG.cppSource/JavaScriptCore/ffi/FFIDFG.hSource/JavaScriptCore/ffi/FFIDFGCodegen.cppSource/JavaScriptCore/ffi/FFIICStub.cppSource/JavaScriptCore/ffi/FFIICStub.hSource/JavaScriptCore/ffi/FFIInvokeThunk.cppSource/JavaScriptCore/ffi/FFIInvokeThunk.hSource/JavaScriptCore/ffi/FFIRawMemory.cppSource/JavaScriptCore/ffi/FFIRawMemory.hSource/JavaScriptCore/ffi/FFISignature.cppSource/JavaScriptCore/ffi/FFISignature.hSource/JavaScriptCore/ffi/FFIType.hSource/JavaScriptCore/ffi/JSFFICallback.cppSource/JavaScriptCore/ffi/JSFFICallback.hSource/JavaScriptCore/ffi/JSFFIFunction.cppSource/JavaScriptCore/ffi/JSFFIFunction.hSource/JavaScriptCore/ffi/tests/FFITestFixtures.cppSource/JavaScriptCore/ffi/tests/FFITestFixtures.hSource/JavaScriptCore/ffi/tests/testFFI.cppSource/JavaScriptCore/ftl/FTLCapabilities.cppSource/JavaScriptCore/ftl/FTLLowerDFGToB3.cppSource/JavaScriptCore/heap/Heap.cppSource/JavaScriptCore/heap/Heap.hSource/JavaScriptCore/runtime/Intrinsic.hSource/JavaScriptCore/runtime/JSGlobalObject.cppSource/JavaScriptCore/runtime/JSGlobalObject.hSource/JavaScriptCore/runtime/OptionsList.hSource/JavaScriptCore/runtime/VM.hSource/JavaScriptCore/shell/CMakeLists.txtSource/JavaScriptCore/tools/JSDollarVM.cpp
…r args The DFG's CallFFI already resolved a typed-array / DataView view passed to a pointer-family argument straight to its data pointer inline, but the FTL sent every UntypedUse argument -- including plain int32/double pointers -- through operationFFIWriteSlot, so the hottest tier paid an out-of-line C++ conversion per call. Give the FTL the same inline paths (numbers, and views), so passing a view directly costs a type check plus a caged vector load and no host call. Two guards keep the view fast path sound in both tiers: a detached view has a null vector, and a resizable / growable-shared view carries the isResizableOrGrowableShared mode bits; both punt to the C++ conversion, whose semantics stay authoritative. The DFG helper gains the same mode-bit check so the tiers reject those cases identically. Adds ffi-view-args.js: a per-call tier-differential (a noDFG-pinned oracle vs the FTL-hot twin) over all twelve view types, storage-mode transitions, cstring-from-view, the detached / resizable / shared guards, buffer-param rejection, and the throwing-second-argument exception path.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Source/JavaScriptCore/ffi/FFIDFGCodegen.cpp (1)
482-493: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftKeep the arena alive until return boxing completes.
A
CStringargument can point intoFFIContext::arena(), and native code may legally return that same pointer. Exiting the arena at Lines 490-492 beforeoperationFFIBoxSlotboxes the return value at Line 609 can therefore read stale or overwritten memory, causing corrupted strings or a crash.Exit immediately on native-call exceptions, but keep the arena active through normal return boxing; if boxing throws, exit before propagating and preserve the boxed result across the exit call.
Also applies to: 593-614
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Source/JavaScriptCore/ffi/FFIDFGCodegen.cpp` around lines 482 - 493, Revise the needsArenaBracket flow around the native call and operationFFIBoxSlot so the arena remains active through normal return boxing. Exit it immediately on native-call exceptions, and when boxing throws, exit before propagating while preserving the boxed result across the exit call; remove the unconditional operationFFIArenaExit immediately after the native call.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@JSTests/stress/ffi-view-args.js`:
- Around line 121-124: Wrap the hotIdentity and refIdentity calls in the
numberArgs regression loop with the existing tryHot and tryCall helpers. Keep
the loop and agreement labels unchanged so null and undefined conversion
behavior is compared as normalized thrown results rather than escaping as
uncaught exceptions.
---
Outside diff comments:
In `@Source/JavaScriptCore/ffi/FFIDFGCodegen.cpp`:
- Around line 482-493: Revise the needsArenaBracket flow around the native call
and operationFFIBoxSlot so the arena remains active through normal return
boxing. Exit it immediately on native-call exceptions, and when boxing throws,
exit before propagating while preserving the boxed result across the exit call;
remove the unconditional operationFFIArenaExit immediately after the native
call.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d818419f-e085-4b09-bfd5-eef64c29b898
📒 Files selected for processing (3)
JSTests/stress/ffi-view-args.jsSource/JavaScriptCore/ffi/FFIDFGCodegen.cppSource/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
…path Repins to autobuild-preview-pr-319-ca52299c, which adds the inline number and typed-array-view conversion for CallFFI pointer arguments in the FTL.
Serve "ptr" (the resolved native target as a double-encoded pointer) from the m_target field via getOwnPropertySlot instead of relying on embedders to add it with putDirect. An own-property addition transitions the function's Structure, and a JSFFIFunction with a non-canonical structure loses the callee fast paths on polymorphic (non-devirtualized) call sites -- measured ~2.5x slower per call through a shared, polymorphic runner. Keeping the structure canonical keeps every call site fast, not only the ones the DFG turns into CallFFI. The property stays read-only, non-enumerable and non-deletable, matching the previous surface.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Source/JavaScriptCore/ffi/JSFFIFunction.cpp`:
- Around line 143-154: Add a JSFFIFunction::getOwnPropertyNames() override
alongside getOwnPropertySlot() that appends the non-enumerable "ptr" property to
own-key results while preserving base-class keys and attributes. Add a
reflection test covering Object.getOwnPropertyNames() and Reflect.ownKeys() to
verify ptr is included.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d65fe97c-83bd-43f3-bac7-2041292a28d0
📒 Files selected for processing (2)
Source/JavaScriptCore/ffi/JSFFIFunction.cppSource/JavaScriptCore/ffi/JSFFIFunction.h
Repins to autobuild-preview-pr-319-48250cdb, which serves JSFFIFunction's .ptr as an intrinsic property instead of a per-instance putDirect, fixing the ~2.5x slowdown on polymorphic FFI call sites.
…n ESM The parser's constant-callee feed for JSFFIFunction was gated on the call opcode being Call. In strict / ES-module code an FFI call in tail position -- notably every `() => sym(...)` arrow body -- is a bytecode TailCall, so the feed never ran there and the call fell to the generic path (into the C++ host marshaller), slower than the interpreter. That is the shape of essentially every real call site: mitata's bench and any dlopen'd symbol called inside a closure in an ES module. Accept TailCall in the feed and emit the resulting FFI call as a plain Call. Dropping tail-call frame reuse is a legal optimization loss (proper tail calls are off by default) and keeps CallFFI a non-terminal node, so the existing strength-reduction fold applies unchanged. Converting a TailCall terminal to CallFFI in place would break block terminality, so strength reduction still (correctly) only ever sees op() == Call. Adds ffi-tailcall.js: values, exception propagation and a deep chain of tail-position FFI calls agree with the interpreter across tiers.
Repins to autobuild-preview-pr-319-596f4fc4: FFI calls in tail position (every closure-wrapped call in an ES module) now take the CallFFI fast path instead of the C++ host marshaller.
Bun's public API exposes symbol.native (the raw callable, which for an engine-native function is the function itself). Serve it as a second intrinsic property from getOwnPropertySlot, next to "ptr", so the glue never has to assign it: an own-property write transitions the cell's Structure and slows every polymorphic call site, exactly like the earlier "ptr" case.
Wires bun:ffi onto JavaScriptCore's engine-native FFI (JSC::JSFFIFunction / JSC::JSFFICallback from oven-sh/WebKit#319) instead of JIT-compiling a TinyCC trampoline per symbol. dlopen()/linkSymbols()/CFunction() symbols take the JSC path when the signature has no napi_env/napi_value; JSCallback when it isn't threadsafe. cc(), napi signatures and threadsafe callbacks are unchanged and stay on TinyCC. BUN_FEATURE_FLAG_DISABLE_JSC_FFI=1 restores TinyCC everywhere. Engine-native symbols get no per-argument JS coercion shim: conversions, arity and result boxing happen in the engine, and hot call sites compile into DFG/FTL CallFFI nodes. dlopen returns a per-symbol jscSymbols map so the glue knows which symbols to leave unwrapped; the symbol's .ptr/.native are intrinsic engine properties (writing them as own properties transitioned the cell's Structure and slowed polymorphic call sites). Coercion: numeric params accept numbers, booleans, null/undefined and BigInts and wrap to width; strings/Symbols throw. Fixes u32 >= 2^31 (#7007); pointers > 2^53 round-trip as exact BigInts and BigInt addresses are accepted (#28068, #22751). Pins WEBKIT_VERSION to the WebKit#319 preview build; repin to the merged sha once it lands.
The FTL CallFFI lowering used to store every argument into the slot
buffer and call the per-signature invoke thunk, which reloaded the slots
into registers and made the real native call: two call/ret pairs, an
extra frame, and a store-then-reload of every argument. The target is a
compile-time constant in the FTL, so call it straight from B3 as a
CCallValue (tagged CFunctionPtrTag, exactly as the thunk calls it):
- KnownInt32 / KnownBoolean / DoubleRep arguments are already single
typed SSA values and become register operands directly.
- UntypedUse and synthetic arguments keep their existing conversion code
(typed-array-view fast path, i64/u64 BigInt wrap-mod-2^64, the C++ slow
path), all of which finish by leaving the canonical 64-bit value in the
slot; the direct call reloads that slot as its operand.
- The return value is normalized back into the return slot with the same
encoding as the thunk's return normalization, so the boxing code is
shared and unchanged.
A bare noop() call goes from 1.49 ns to 0.73 ns.
One ABI corner B3 cannot express: Darwin/arm64 packs sub-8-byte STACK
arguments at their natural size, and B3 has no 8/16-bit value type, so a
char/i8/u8/i16/u16 argument that spills past the 8 argument GPRs would be
laid out at Int32 stride. Detect that case and keep the thunk (which
implements the packing by hand) for exactly those signatures; every other
signature uses the direct call. Guarded by --useFFIDirectCall.
Split compileCallFFI into a thin dispatcher (which evaluates Options::useFFIDirectCall() and the Darwin sub-word-spill eligibility once) and template<bool DirectCall> compileCallFFIImpl(), turning every runtime `if (directCall)` in the argument/call/return lowering into `if constexpr`. The direct-call and invoke-thunk paths are now separate instantiations sharing the conversion table, arena bracketing, keep-alive and boxing code, and each dead path folds away at compile time. No behavior change: both instantiations pass testFFI and the ffi stress suite (the thunk one forced via --useFFIDirectCall=false under eager/validate); noop stays 0.69ns direct / 1.41ns thunk. Also record, next to the code, why the IC stub is kept: in the unoptimized tiers it is worth ~1.5x on noop and ~2.4x on argument-carrying calls versus the generic C++ host marshaller.
Repins to autobuild-preview-pr-319-21975506: the FTL calls the native FFI target directly (no invoke thunk; noop 1.4ns -> 0.7ns in the engine), plus the intrinsic .ptr/.native and the templated CallFFI lowering. Repin to the merged sha once #319 lands.
Wires bun:ffi onto JavaScriptCore's engine-native FFI (JSC::JSFFIFunction / JSC::JSFFICallback from oven-sh/WebKit#319) instead of JIT-compiling a TinyCC trampoline per symbol. dlopen()/linkSymbols()/CFunction() symbols take the JSC path when the signature has no napi_env/napi_value; JSCallback when it isn't threadsafe. cc(), napi signatures and threadsafe callbacks are unchanged and stay on TinyCC. BUN_FEATURE_FLAG_DISABLE_JSC_FFI=1 restores TinyCC everywhere. Engine-native symbols get no per-argument JS coercion shim: conversions, arity and result boxing happen in the engine, and hot call sites compile into DFG/FTL CallFFI nodes (the FTL calling the target directly). dlopen returns a per-symbol jscSymbols map so the glue knows which symbols to leave unwrapped; .ptr/.native are intrinsic engine properties (writing them as own properties transitioned the cell's Structure and slowed polymorphic call sites). Coercion: numeric params accept numbers, booleans, null/undefined and BigInts and wrap to width; strings/Symbols throw. Fixes u32 >= 2^31 (#7007); pointers > 2^53 round-trip as exact BigInts and BigInt addresses are accepted (#28068, #22751). Pins WEBKIT_VERSION to the WebKit#319 preview build; repin to the merged sha once #319 lands.
Unbrace the single-statement clauses in compileCallFFIImpl (the if-constexpr directCall pair, the numbersInline branch pair, and the operand-marshalling else-if chain), lay out the return-type case labels one per line, drop the trailing alignment comments in FFICallingConvention.h, and remove a stale trailing comment in FFIConversions.cpp (strings and Symbols are rejected before that branch). testFFI 9519/9519; all ffi stress files pass; build clean.
Reimplement bun:ffi on the engine-native FFI in JavaScriptCore (oven-sh/WebKit#319). For dlopen(), linkSymbols(), CFunction() and JSCallback the engine generates the marshalling itself, promotes hot calls to a direct native call from JIT'd code, and owns callback lifetime; TinyCC remains only as cc()'s C compiler. - CString is a constructor returning a plain string primitive; 'cstring' returns and callback cstring parameters are decoded strings (NULL -> null). - New buffer_length argument type; .ptr/.native are real read-only own properties; N-API types are cc()-only; the JIT is required. - BigInt pointers accepted consistently across CString, read.*, toBuffer, toArrayBuffer, CFunction / linkSymbols, with negative and out-of-range BigInts rejected instead of wrapping. - Fix the shared isBigIntInInt64Range / isBigIntInUInt64Range bindings, which were effectively always-true (broken disjunction plus swapped (max, min) at the Rust boundary); MySQL BigInt binding and the integer range validator now behave as written. - test/js/bun/ffi builds its C fixture with the host compiler at test time and includes an ABI conformance suite; cc() bundles N-API headers under <bun-cc>/node. The branch history is rebuilt onto current main to contain only these files; a prior rebase had linearized a main merge and replayed unrelated main commits onto the branch.
There was a problem hiding this comment.
No new findings after a554d31. This is a large, security-sensitive change (new FFI subsystem with JIT-generated thunks, a new DFG/FTL node type, cross-ABI calling-convention codegen, and thread-safe callback GC rooting), so it warrants human review.
What was reviewed this round:
- a554d31's fixes in
compileCallFFIImpl(unbraced single-statement clauses, one-per-linereturnLTypecases) — verified complete, no mixed-style leftover in the switch. - The removed stale comment in
writeFloatingPointSlot()and the removed alignment comments inFFICallingConvention.h. check-webkit-style [safercpp/memcpy]hits in the JSDollarVM FFI additions — examined and ruled out (test-fixture glue, bounded copies).
Extended reasoning...
Overview
This PR adds an engine-native FFI subsystem to JavaScriptCore under USE(BUN_JSC_ADDITIONS): 88 files (~30 new source files under ffi/, 32 new stress tests, and integration hooks across dfg/, ftl/, heap/, runtime/, and tools/). It introduces two new JS cell types (JSFFIFunction, JSFFICallback), a new DFG/FTL node type (CallFFI), per-signature JIT-generated invoke/callback thunks for four ABIs (SysV64, Win64, AAPCS64, Darwin arm64), a per-global string arena tied to GC via HeapObserver, and a thread-safe callback rooting protocol with a packed closed+pending-count word.
Security risks
FFI is inherently security-sensitive: it marshals raw pointers between JS and native code, JIT-generates executable trampolines, reads TypedArray vectors directly (with Gigacage uncaging), and lets foreign OS threads enqueue work that later runs on the JS thread. Correctness of the calling-convention codegen (register assignment, stack alignment, sub-word packing), OSR-exit / exception-unwind interaction with the arena bracket, and the thread-safe callback lifetime (close-while-queued, GC-while-pending) all have memory-safety implications. The differential test harness (9.5k checks) and 32 stress tests give substantial coverage, and many earlier review rounds caught real issues in these areas (arena depth leaks on exception, pointer-return SpeculatedType unsoundness, fall-through breakage in DFGMayExit), all now resolved.
Level of scrutiny
High. This is production-critical JIT-compiler and runtime code that will back Bun's bun:ffi API. It contains hand-written codegen across four ABIs and touches every DFG/FTL phase. It is far outside the scope of changes that can be approved without human review.
Other factors
The review history spans ~60 resolved threads over five days; the last several rounds converged on style/whitespace cleanup and stale-comment removal, and this is the first round with zero findings. The two candidate issues raised this run (mixed-style leftover in the a554d31 reformat, and safercpp/memcpy lint hits in the JSDollarVM test glue) were both examined and ruled out. All previously-raised inline comments are marked resolved by the author with commit references.
Picks up the remaining check-webkit-style cleanups and the orphaned-comment removals from oven-sh/WebKit#319; no behavioral change since the last pin.
Bring the engine's bun:ffi coverage into Bun's CI: - test/js/bun/jsc-stress/fixtures/ffi/: the 32 ffi-*.js stress files from oven-sh/WebKit#319 (tier transitions, OSR, exception unwinding through native frames, threadsafe callbacks, arity/ABI ladders, arena depth). They drive the engine through the $vm test hooks and the FFITestFixtures library compiled into JavaScriptCore, so the runner sets BUN_JSC_useDollarVM=1 and gates the block on a runtime probe -- builds that compile $vm out (LTO / shipped release) skip cleanly, debug / ASAN / plain release run them. - The directive parser learns //@ requireOptions(...) and //@ runNoJIT, and preload.js maps the jsc-shell globals the fixtures use (gc, fullGC, edenGC, numberOfDFGCompiles, noDFG) onto bun:jsc / Bun.gc. - ffi-callbacks.js: one assertion (the throwing callback's frame name in caught.stack) is intermittent under Bun -- the frame is occasionally reported as <anonymous> -- so assert the stack exists and file the frame-name intermittency separately; every substantive check stays. - testFFI.test.ts runs the C++/ABI test binary that the WebKit artifact now ships in bin/, matching its 'OK: N checks passed, 0 failed' summary, and skips when the pinned bundle predates the binary. jsc-stress 115/0 locally (all 32 ffi fixtures pass); ffi dir 211/0; source lints 59/0.
testFFI (the FFI C++/ABI test binary, 9519 checks including the differential harness that runs every generated invoke thunk against clang-compiled reference calls) was never built because every build path built only --target jsc. Build it alongside jsc on all of them so it lands in the artifact's bin/ next to jsc: the Linux glibc/musl, FreeBSD, Android, and macOS/Windows cross Dockerfiles, mac-release.bash, windows-release.ps1 and build.ts. FreeBSD and Android previously packaged no bin/ at all, so add the copy there; every other path already copies bin/ wholesale. testFFI links the same libraries as jsc (CMAKE_DL_LIBS, plus Winmm on Windows) rather than only the vendored mimalloc objects -- it has only ever been link-tested on macOS, and the other platforms now build it. Bun's CI runs the shipped binary via test/js/bun/jsc-stress/testFFI.test.ts (oven-sh/bun#35246).
Bun's CI test lanes run the built bun binary and never have the WebKit deps tree, so a testFFI shipped only inside the 400 MB bun-webkit tarball is unreachable there. After the tarballs are gathered into ./out/, extract each platform's bin/testFFI (testFFI.exe on Windows) into ./out/testFFI-<platform> and attach those to the release, so a test only downloads the small standalone binary for its own platform.
Bun's CI now carries testFFI to its test lanes inside the existing build-bun profile zip (the same artifact channel the bun binary already uses), so the tests find it beside bunExe and never download from GitHub releases. The per-platform release-asset upload is unused; remove it.
build.ts and the release scripts build testFFI alongside jsc now, so the '(it only configures + builds --target jsc)' clause contradicted the sentence after it.
Reimplement bun:ffi on the engine-native FFI in JavaScriptCore (oven-sh/WebKit#319). For dlopen(), linkSymbols(), CFunction() and JSCallback the engine generates the marshalling itself, promotes hot calls to a direct native call from JIT'd code, and owns callback lifetime; TinyCC remains only as cc()'s C compiler. - CString is a constructor returning a plain string primitive; 'cstring' returns and callback cstring parameters are decoded strings (NULL -> null). - New buffer_length argument type; .ptr/.native are real read-only own properties; N-API types are cc()-only; the JIT is required. - BigInt pointers accepted consistently across CString, read.*, toBuffer, toArrayBuffer, CFunction / linkSymbols, with negative and out-of-range BigInts rejected instead of wrapping. - Fix the shared isBigIntInInt64Range / isBigIntInUInt64Range bindings, which were effectively always-true (broken disjunction plus swapped (max, min) at the Rust boundary); MySQL BigInt binding and the integer range validator now behave as written. - test/js/bun/ffi builds its C fixture with the host compiler at test time and includes an ABI conformance suite; cc() bundles N-API headers under <bun-cc>/node. The branch history is rebuilt onto current main to contain only these files; a prior rebase had linearized a main merge and replayed unrelated main commits onto the branch.
Picks up the remaining check-webkit-style cleanups and the orphaned-comment removals from oven-sh/WebKit#319; no behavioral change since the last pin.
Bring the engine's bun:ffi coverage into Bun's CI: - test/js/bun/jsc-stress/fixtures/ffi/: the 32 ffi-*.js stress files from oven-sh/WebKit#319 (tier transitions, OSR, exception unwinding through native frames, threadsafe callbacks, arity/ABI ladders, arena depth). They drive the engine through the $vm test hooks and the FFITestFixtures library compiled into JavaScriptCore, so the runner sets BUN_JSC_useDollarVM=1 and gates the block on a runtime probe -- builds that compile $vm out (LTO / shipped release) skip cleanly, debug / ASAN / plain release run them. - The directive parser learns //@ requireOptions(...) and //@ runNoJIT, and preload.js maps the jsc-shell globals the fixtures use (gc, fullGC, edenGC, numberOfDFGCompiles, noDFG) onto bun:jsc / Bun.gc. - ffi-callbacks.js: one assertion (the throwing callback's frame name in caught.stack) is intermittent under Bun -- the frame is occasionally reported as <anonymous> -- so assert the stack exists and file the frame-name intermittency separately; every substantive check stays. - testFFI.test.ts runs the C++/ABI test binary that the WebKit artifact now ships in bin/, matching its 'OK: N checks passed, 0 failed' summary, and skips when the pinned bundle predates the binary. jsc-stress 115/0 locally (all 32 ffi fixtures pass); ffi dir 211/0; source lints 59/0.
oven-sh/WebKit#319 merged as e6063b004fa4; move WEBKIT_VERSION off the preview-pr-319 pre-releases onto the main autobuild for the merge commit. No behavioral difference from the last preview pin.
Reimplement bun:ffi on the engine-native FFI in JavaScriptCore (oven-sh/WebKit#319). For dlopen(), linkSymbols(), CFunction() and JSCallback the engine generates the marshalling itself, promotes hot calls to a direct native call from JIT'd code, and owns callback lifetime; TinyCC remains only as cc()'s C compiler. - CString is a constructor returning a plain string primitive; 'cstring' returns and callback cstring parameters are decoded strings (NULL -> null). - New buffer_length argument type; .ptr/.native are real read-only own properties; N-API types are cc()-only; the JIT is required. - BigInt pointers accepted consistently across CString, read.*, toBuffer, toArrayBuffer, CFunction / linkSymbols, with negative and out-of-range BigInts rejected instead of wrapping. - Fix the shared isBigIntInInt64Range / isBigIntInUInt64Range bindings, which were effectively always-true (broken disjunction plus swapped (max, min) at the Rust boundary); MySQL BigInt binding and the integer range validator now behave as written. - test/js/bun/ffi builds its C fixture with the host compiler at test time and includes an ABI conformance suite; cc() bundles N-API headers under <bun-cc>/node. The branch history is rebuilt onto current main to contain only these files; a prior rebase had linearized a main merge and replayed unrelated main commits onto the branch.
Picks up the remaining check-webkit-style cleanups and the orphaned-comment removals from oven-sh/WebKit#319; no behavioral change since the last pin.
Bring the engine's bun:ffi coverage into Bun's CI: - test/js/bun/jsc-stress/fixtures/ffi/: the 32 ffi-*.js stress files from oven-sh/WebKit#319 (tier transitions, OSR, exception unwinding through native frames, threadsafe callbacks, arity/ABI ladders, arena depth). They drive the engine through the $vm test hooks and the FFITestFixtures library compiled into JavaScriptCore, so the runner sets BUN_JSC_useDollarVM=1 and gates the block on a runtime probe -- builds that compile $vm out (LTO / shipped release) skip cleanly, debug / ASAN / plain release run them. - The directive parser learns //@ requireOptions(...) and //@ runNoJIT, and preload.js maps the jsc-shell globals the fixtures use (gc, fullGC, edenGC, numberOfDFGCompiles, noDFG) onto bun:jsc / Bun.gc. - ffi-callbacks.js: one assertion (the throwing callback's frame name in caught.stack) is intermittent under Bun -- the frame is occasionally reported as <anonymous> -- so assert the stack exists and file the frame-name intermittency separately; every substantive check stays. - testFFI.test.ts runs the C++/ABI test binary that the WebKit artifact now ships in bin/, matching its 'OK: N checks passed, 0 failed' summary, and skips when the pinned bundle predates the binary. jsc-stress 115/0 locally (all 32 ffi fixtures pass); ffi dir 211/0; source lints 59/0.
oven-sh/WebKit#319 merged as e6063b004fa4; move WEBKIT_VERSION off the preview-pr-319 pre-releases onto the main autobuild for the merge commit. No behavioral difference from the last preview pin.
|
neat stuff! |
Reimplement bun:ffi on the engine-native FFI in JavaScriptCore (oven-sh/WebKit#319). For dlopen(), linkSymbols(), CFunction() and JSCallback the engine generates the marshalling itself, promotes hot calls to a direct native call from JIT'd code, and owns callback lifetime; TinyCC remains only as cc()'s C compiler. - CString is a constructor returning a plain string primitive; 'cstring' returns and callback cstring parameters are decoded strings (NULL -> null). - New buffer_length argument type; .ptr/.native are real read-only own properties; N-API types are cc()-only; the JIT is required. - BigInt pointers accepted consistently across CString, read.*, toBuffer, toArrayBuffer, CFunction / linkSymbols, with negative and out-of-range BigInts rejected instead of wrapping. - Fix the shared isBigIntInInt64Range / isBigIntInUInt64Range bindings, which were effectively always-true (broken disjunction plus swapped (max, min) at the Rust boundary); MySQL BigInt binding and the integer range validator now behave as written. - test/js/bun/ffi builds its C fixture with the host compiler at test time and includes an ABI conformance suite; cc() bundles N-API headers under <bun-cc>/node. The branch history is rebuilt onto current main to contain only these files; a prior rebase had linearized a main merge and replayed unrelated main commits onto the branch.
Picks up the remaining check-webkit-style cleanups and the orphaned-comment removals from oven-sh/WebKit#319; no behavioral change since the last pin.
Bring the engine's bun:ffi coverage into Bun's CI: - test/js/bun/jsc-stress/fixtures/ffi/: the 32 ffi-*.js stress files from oven-sh/WebKit#319 (tier transitions, OSR, exception unwinding through native frames, threadsafe callbacks, arity/ABI ladders, arena depth). They drive the engine through the $vm test hooks and the FFITestFixtures library compiled into JavaScriptCore, so the runner sets BUN_JSC_useDollarVM=1 and gates the block on a runtime probe -- builds that compile $vm out (LTO / shipped release) skip cleanly, debug / ASAN / plain release run them. - The directive parser learns //@ requireOptions(...) and //@ runNoJIT, and preload.js maps the jsc-shell globals the fixtures use (gc, fullGC, edenGC, numberOfDFGCompiles, noDFG) onto bun:jsc / Bun.gc. - ffi-callbacks.js: one assertion (the throwing callback's frame name in caught.stack) is intermittent under Bun -- the frame is occasionally reported as <anonymous> -- so assert the stack exists and file the frame-name intermittency separately; every substantive check stays. - testFFI.test.ts runs the C++/ABI test binary that the WebKit artifact now ships in bin/, matching its 'OK: N checks passed, 0 failed' summary, and skips when the pinned bundle predates the binary. jsc-stress 115/0 locally (all 32 ffi fixtures pass); ffi dir 211/0; source lints 59/0.
oven-sh/WebKit#319 merged as e6063b004fa4; move WEBKIT_VERSION off the preview-pr-319 pre-releases onto the main autobuild for the merge commit. No behavioral difference from the last preview pin.
Reimplements `bun:ffi` on top of a new **engine-native FFI in JavaScriptCore** (oven-sh/WebKit#319). For `dlopen()`, `linkSymbols()`, `CFunction()`, and `JSCallback`, the engine generates the marshalling itself, promotes hot calls to a direct native call from JIT'd code, and owns callback lifetime. TinyCC is used only as `cc()`'s C compiler. ## Performance Engine-native vs TinyCC-based `bun:ffi` (macOS arm64, release): | operation | TinyCC | engine-native | | |---|---|---|---| | noop call | 2.13 ns | **0.70 ns** | 3.0× | | `new CString(ptr)` (46-char string) | 92.5 ns | **24.1 ns** | 3.8× | Against Deno on the same native library: | operation | Bun | Deno | | |---|---|---|---| | noop call | **0.69 ns** | 1.51 ns | 2.2× | | hash (`ptr` + `u32`) | 39 ns | 36 ns | parity — dominated by the C function's own work | | C string return | **22 ns** (`returns: "cstring"`) | 34 ns | 1.5× | String arguments, same string ("550e8400-…", 36 chars): | how the string is passed | ns/call | |---|---| | raw pointer (`ptr(buf)`) | 16 | | TypedArray | 17 | | JS string (encoded into the call arena) | 27 | | result of `CString(ptr)` | 27 | Passing a JS string re-encodes it on every call; for a hot loop over the same string, pass a pointer or TypedArray. ### opentui Three of opentui's own benchmark suites (`packages/core`), engine-native vs TinyCC-based `bun:ffi`, same machine. opentui binds its Zig core -- including its native Yoga port (222 symbols) -- through one `bun:ffi` `dlopen`, so its Yoga layout passes are FFI-call-dense. **`render-traversal`** (Yoga reads + scrollbox culling) -- geomean **1.30x**, scaling with call count as fixed per-scenario cost amortizes: | scenario | speedup | |---|---| | `yoga_layout_reads_1000` | **2.08x** | | `yoga_layout_reads_10000` | 2.02x | | `yoga_layout_reads_100` | 1.38x | | `scrollbox_culling_scaling_10000` | **1.43x** | | `scrollbox_culling_scaling_5000` | 1.32x | | `scrollbar_stack` / `layout_only_opencode_wrappers` | 1.00x | **`layout-benchmark`** (16 dirty-and-relayout scenarios through the Yoga FFI) -- geomean **1.19x**, all scenarios faster (1.04x-1.38x); OpenCode-shaped full-render passes 1.30x-1.38x, pure `calculate_only` layout 1.07x-1.19x. **`native-span-feed`** (`default` suite, a memcpy-bound span stream) -- geomean **+4.9%** throughput, worst scenario -1.7%, best +22.9% on `commit_4k` write; gains concentrate at high call rates and vanish at 32 MB spans. opentui is unaffected by the `CString` change (it uses neither `CString` nor `cstring` returns). ## Behavior changes - **`cstring` returns are strings.** A symbol declared `returns: "cstring"` yields a JS **string primitive** (`typeof "string"`, `===` works) decoded from the callee's `char*`; a `NULL` return is `null`. There is no wrapper object and no address is exposed. Callback parameters typed `cstring` arrive as strings the same way. - **`CString` is a constructor that returns a string.** `new CString(ptr, byteOffset?, byteLength?)` and `CString(ptr, ...)` transcode the bytes at `ptr` and return a plain string; `CString` has no accessor surface. - **`buffer_length`** — new argument type: pass the same TypedArray/DataView you passed for a `buffer` argument and the callee receives that view's byte length as a `uint64_t`, read off the same object at call time so pointer and length always agree. Argument-only; not available inside `cc()`. - **`.ptr` / `.native`** on FFI functions are real read-only own properties. - **N-API types are `cc()`-only.** `napi_env` / `napi_value` in `dlopen`, `linkSymbols`, `CFunction`, or `JSCallback` throw a `TypeError`. Inside `cc()`, a `napi_env` parameter is filled in by the compiled trampoline and its JS argument is a consumed-but-ignored placeholder. - **`cc()`** performs C-side conversions in its TinyCC-compiled trampoline (integer arguments wrap), and bundles N-API headers under `<bun-cc>/node/` so `#include <node/node_api.h>` resolves without a `-I` flag. - **Thread-safe callbacks** can be invoked from any thread and are delivered on the JS thread with arguments converted there (64-bit integers and large pointers arrive as exact BigInts). `close()` refuses new foreign-thread calls but every already-queued invocation is still delivered. - **The JIT is required.** With the JIT disabled, `dlopen()` and friends throw a `TypeError`. ## What is removed - The TinyCC compile path for `dlopen`/`linkSymbols`/`CFunction`/`JSCallback` symbols, `viewSource` of callbacks (there is no generated C to show), and the per-symbol wrapper objects — every symbol is the engine function itself. - `CString`'s object surface (see above) and `toArrayBuffer`-backed `arrayBuffer` on it. ## Testing - `test/js/bun/ffi/` builds its C fixture with the host compiler at test time, so the suite runs on every CI platform: **203 tests, 0 fail**. Includes an ABI conformance suite whose fixture returns position-weighted combinations of its arguments, so any calling-convention error changes the observable result — verified to detect a deliberately mis-declared signature. - Source lints, the napi FFI file, and the ffi bench all green; opentui audited as unaffected by the `CString` change (it uses neither `CString` nor `cstring` returns). --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
…e FFI) vendor/WebKit updated from c9296e35 to 8172ab94 which merges oven-sh:main and includes the engine-native FFI changes (oven-sh/WebKit#319). Revert JSCFFIBridge.cpp to the upstream version without the __has_include stub, since the updated WebKit provides the required FFI headers.
Reimplements `bun:ffi` on top of a new **engine-native FFI in JavaScriptCore** (oven-sh/WebKit#319). For `dlopen()`, `linkSymbols()`, `CFunction()`, and `JSCallback`, the engine generates the marshalling itself, promotes hot calls to a direct native call from JIT'd code, and owns callback lifetime. TinyCC is used only as `cc()`'s C compiler. ## Performance Engine-native vs TinyCC-based `bun:ffi` (macOS arm64, release): | operation | TinyCC | engine-native | | |---|---|---|---| | noop call | 2.13 ns | **0.70 ns** | 3.0× | | `new CString(ptr)` (46-char string) | 92.5 ns | **24.1 ns** | 3.8× | Against Deno on the same native library: | operation | Bun | Deno | | |---|---|---|---| | noop call | **0.69 ns** | 1.51 ns | 2.2× | | hash (`ptr` + `u32`) | 39 ns | 36 ns | parity — dominated by the C function's own work | | C string return | **22 ns** (`returns: "cstring"`) | 34 ns | 1.5× | String arguments, same string ("550e8400-…", 36 chars): | how the string is passed | ns/call | |---|---| | raw pointer (`ptr(buf)`) | 16 | | TypedArray | 17 | | JS string (encoded into the call arena) | 27 | | result of `CString(ptr)` | 27 | Passing a JS string re-encodes it on every call; for a hot loop over the same string, pass a pointer or TypedArray. ### opentui Three of opentui's own benchmark suites (`packages/core`), engine-native vs TinyCC-based `bun:ffi`, same machine. opentui binds its Zig core -- including its native Yoga port (222 symbols) -- through one `bun:ffi` `dlopen`, so its Yoga layout passes are FFI-call-dense. **`render-traversal`** (Yoga reads + scrollbox culling) -- geomean **1.30x**, scaling with call count as fixed per-scenario cost amortizes: | scenario | speedup | |---|---| | `yoga_layout_reads_1000` | **2.08x** | | `yoga_layout_reads_10000` | 2.02x | | `yoga_layout_reads_100` | 1.38x | | `scrollbox_culling_scaling_10000` | **1.43x** | | `scrollbox_culling_scaling_5000` | 1.32x | | `scrollbar_stack` / `layout_only_opencode_wrappers` | 1.00x | **`layout-benchmark`** (16 dirty-and-relayout scenarios through the Yoga FFI) -- geomean **1.19x**, all scenarios faster (1.04x-1.38x); OpenCode-shaped full-render passes 1.30x-1.38x, pure `calculate_only` layout 1.07x-1.19x. **`native-span-feed`** (`default` suite, a memcpy-bound span stream) -- geomean **+4.9%** throughput, worst scenario -1.7%, best +22.9% on `commit_4k` write; gains concentrate at high call rates and vanish at 32 MB spans. opentui is unaffected by the `CString` change (it uses neither `CString` nor `cstring` returns). ## Behavior changes - **`cstring` returns are strings.** A symbol declared `returns: "cstring"` yields a JS **string primitive** (`typeof "string"`, `===` works) decoded from the callee's `char*`; a `NULL` return is `null`. There is no wrapper object and no address is exposed. Callback parameters typed `cstring` arrive as strings the same way. - **`CString` is a constructor that returns a string.** `new CString(ptr, byteOffset?, byteLength?)` and `CString(ptr, ...)` transcode the bytes at `ptr` and return a plain string; `CString` has no accessor surface. - **`buffer_length`** — new argument type: pass the same TypedArray/DataView you passed for a `buffer` argument and the callee receives that view's byte length as a `uint64_t`, read off the same object at call time so pointer and length always agree. Argument-only; not available inside `cc()`. - **`.ptr` / `.native`** on FFI functions are real read-only own properties. - **N-API types are `cc()`-only.** `napi_env` / `napi_value` in `dlopen`, `linkSymbols`, `CFunction`, or `JSCallback` throw a `TypeError`. Inside `cc()`, a `napi_env` parameter is filled in by the compiled trampoline and its JS argument is a consumed-but-ignored placeholder. - **`cc()`** performs C-side conversions in its TinyCC-compiled trampoline (integer arguments wrap), and bundles N-API headers under `<bun-cc>/node/` so `#include <node/node_api.h>` resolves without a `-I` flag. - **Thread-safe callbacks** can be invoked from any thread and are delivered on the JS thread with arguments converted there (64-bit integers and large pointers arrive as exact BigInts). `close()` refuses new foreign-thread calls but every already-queued invocation is still delivered. - **The JIT is required.** With the JIT disabled, `dlopen()` and friends throw a `TypeError`. ## What is removed - The TinyCC compile path for `dlopen`/`linkSymbols`/`CFunction`/`JSCallback` symbols, `viewSource` of callbacks (there is no generated C to show), and the per-symbol wrapper objects — every symbol is the engine function itself. - `CString`'s object surface (see above) and `toArrayBuffer`-backed `arrayBuffer` on it. ## Testing - `test/js/bun/ffi/` builds its C fixture with the host compiler at test time, so the suite runs on every CI platform: **203 tests, 0 fail**. Includes an ABI conformance suite whose fixture returns position-weighted combinations of its arguments, so any calling-convention error changes the observable result — verified to detect a deliberately mis-declared signature. - Source lints, the napi FFI file, and the ffi bench all green; opentui audited as unaffected by the `CString` change (it uses neither `CString` nor `cstring` returns). --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Adds an engine-native FFI to JavaScriptCore under
USE(BUN_JSC_ADDITIONS)— the machinery Bun'sbun:ffinow runs on (oven-sh/bun#35246), replacing the TinyCC-JIT'd trampolines thatdlopen/linkSymbols/CFunction/JSCallbackused to compile per symbol.What it is
A
JSFFIFunctionis a callable JS object bound to a native target and aSignature(argument and return types). Calling it converts the JS arguments to their native representations, invokes the target with the platform C calling convention, and boxes the result. There is no per-symbol generated C.CallFFInode with unboxed, type-speculated arguments and noCallFrame; the FTL then emits a direct call to the native target (SysV64 / AAPCS64 / Darwin arm64; Win64 keeps the thunk).JSFFICallbackis a JIT'd trampoline giving native code a real function pointer. One implementation covers synchronous and thread-safe callbacks: a foreign thread copies the raw C argument slots and hands them to an embedder-registered dispatch; the JS thread converts and invokes. Thunks are specialized at generation for thread-safety, so the sync path carries no runtime branch.Types
char i8 u8 i16 u16 i32 u32 i64 u64 i64_fast u64_fast f32 f64 bool ptr function jsvalue buffer buffer_length cstring void.jsvalue(tag 19) is a rawEncodedJSValuepass-through in both directions;"napi_value"still parses as its alias. Tag 18 is permanently reserved and rejected.bufferaccepts a TypedArray/DataView and passes its data pointer.buffer_lengthpasses the byte length of the same view as auint64_t, read off the same object at call time — an atomic pointer+length snapshot; argument-only.cstringarguments accept a JS string (transcoded to NUL-terminated UTF-8 in the call arena), a pointer, or a TypedArray.cstringreturns are decoded to a JS string primitive inside the call (NULL→null)..ptrand.nativeare real read-only own properties whose slots are added to the shared Structure once at structure-creation time (statically known offsets) and written withputDirectOffsetwhen the cell is created — every instance is born with the final Structure, no instance ever transitions, and the ordinary object machinery supplies correctput/delete/defineOwnProperty/ownKeyssemantics with no overrides.Strings and the call arena
Each
JSGlobalObjectowns aFFIContextwith a call-scoped string arena: a bracket opens for the call, JS-string arguments bump-allocate into it, and it reclaims at the outermost scope exit. Storage is a parked chunk reused across calls (no allocator traffic on the warm path); a string over the 64 KB retention cap is freed when its call ends, andFFIContextis aHeapObserverthat drops the parked chunk after any GC when no call is in flight, so an idle process retains nothing. Acstringreturn is materialized before its arena reclaims — an echoed argument round-trips as a value. A callback'scstringreturn outlives the call, so it lives in a buffer owned by the callback (valid until that callback's next invocation), never the arena.Callback lifetime
Live callbacks are rooted from the global object under its cell lock. A thread-safe callback keeps a packed closed+pending-count word:
close()refuses new foreign-thread calls but every invocation already accepted is a commitment and is still delivered; the pending count keeps the cell — and through its barrier, the JS callable — rooted until the last queued invocation drains, so a callback closed and collected mid-flight cannot use-after-free and cannot drop work.Performance (release, this branch,
--useDollarVM=1fixtures on macOS arm64)noop()echo_i32(i)echo_f64(x)feeding arithmeticstrlen(36-char JS string)— string argumentcstringreturn → decoded JS stringEnd-to-end numbers through Bun (including the cross-runtime comparison) are in oven-sh/bun#35246.
Scope and requirements
useJIT=0, creation throwsTypeError(the marshalling is generated code; there is no interpreter marshaller).RESERVED_WasNapiEnv(tag 18) andbuffer_length-as-return are rejected by the shared signature parser at every entry point.Testing
testFFI(C++,Source/JavaScriptCore/ffi/tests/): 9,519 checks — a differential harness that runs every generated invoke thunk against clang-compiled reference calls across the type × arity × ABI matrix (including Darwin sub-word stack packing and >8-register spills), plus conversion, signature, and callback tables.JSTests/stress/ffi-*.js: 32 files covering tier transitions and OSR, exceptions unwinding through native frames, thread-safe callbacks from real foreign threads (close-while-queued, GC races, delivery order), thejsvaluepass-through matrix, and view/buffer arguments. All pass in default and eager-tier configurations.