bun:ffi: treat arguments not passed to a cc() symbol as undefined instead of reading past the call frame

src/jsc/bindings/ffi.cpp

@@ -5,13 +5,20 @@ typedef struct FFIFields {
     uint32_t JSArrayBufferView__offsetOfByteOffset;
     uint32_t JSArrayBufferView__offsetOfVector;
     uint32_t JSCell__offsetOfType;
+    // Register (8-byte slot) indices into a CallFrame, not byte offsets.
+    uint32_t CallFrame__argumentCountIncludingThisSlot;
+    uint32_t CallFrame__firstArgumentSlot;
 } FFIFields;
 extern "C" FFIFields Bun__FFI__offsets = { 0 };
 
+static_assert(sizeof(JSC::Register) == sizeof(void*), "the cc() wrapper indexes the CallFrame as an array of pointer-sized slots");
+
 extern "C" void Bun__FFI__ensureOffsetsAreLoaded()
 {
     Bun__FFI__offsets.JSArrayBufferView__offsetOfLength = JSC::JSArrayBufferView::offsetOfLength();
     Bun__FFI__offsets.JSArrayBufferView__offsetOfByteOffset = JSC::JSArrayBufferView::offsetOfByteOffset();
     Bun__FFI__offsets.JSArrayBufferView__offsetOfVector = JSC::JSArrayBufferView::offsetOfVector();
     Bun__FFI__offsets.JSCell__offsetOfType = JSC::JSCell::typeInfoTypeOffset();
+    Bun__FFI__offsets.CallFrame__argumentCountIncludingThisSlot = static_cast<uint32_t>(JSC::CallFrameSlot::argumentCountIncludingThis);
+    Bun__FFI__offsets.CallFrame__firstArgumentSlot = static_cast<uint32_t>(JSC::CallFrame::argumentOffset(0));
 }

src/jsc/lib.rs

@@ -76,8 +76,6 @@ pub mod marked_argument_buffer;
 pub mod regular_expression;
 #[path = "ScriptExecutionStatus.rs"]
 pub mod script_execution_status;
-#[path = "sizes.rs"]
-pub mod sizes;
 #[path = "SourceProvider.rs"]
 pub mod source_provider;
 #[path = "TextCodec.rs"]

src/runtime/ffi/FFI.h

@@ -128,13 +128,16 @@ EncodedJSValue ValueTrue = { TagValueTrue };
 
 typedef void* JSContext;
 
-// Bun_FFI_PointerOffsetToArgumentsList is injected into the build 
-// The value is generated in `make sizegen`
-// The value is 6.
-// On ARM64_32, the value is something else but it really doesn't matter for our case
-// However, I don't want this to subtly break amidst future upgrades to JavaScriptCore
+// callFrame is the JSC::CallFrame (an array of 8-byte slots); the two slot indices
+// are defined from JSC::CallFrameSlot by src/jsc/bindings/ffi.cpp.
 #define LOAD_ARGUMENTS_FROM_CALL_FRAME \
-  int64_t *argsPtr = (int64_t*)((size_t*)callFrame + Bun_FFI_PointerOffsetToArgumentsList)
+  int64_t *argsPtr = (int64_t*)((size_t*)callFrame + Bun_FFI_PointerOffsetToArgumentsList); \
+  int32_t argsCount = ((EncodedJSValue*)((size_t*)callFrame + Bun_FFI_PointerOffsetToArgumentCountIncludingThis))->asBits.payload - 1
+
+// Bits of JSC::CallFrame::argument(i): a slot past argsCount is stale stack, so an
+// argument that was not passed reads as undefined. int64_t rather than a union copy:
+// TinyCC emits a memmove call for those on non-x86_64 and this file is built -nostdlib.
+#define ARGUMENT(i) ((i) < argsCount ? argsPtr[i] : TagValueUndefined)
 
 
 
@@ -208,7 +211,8 @@ static uint64_t JSVALUE_TO_TYPED_ARRAY_LENGTH(EncodedJSValue val) {
 // This behavior change enables the JIT to handle it better
 // It also is better readability when console.log(myPtr)
 static void* JSVALUE_TO_PTR(EncodedJSValue val) {
-  if (val.asInt64 == TagValueNull)
+  // undefined (e.g. an argument that was not passed) is NULL, as in the engine's writePointerSlot.
+  if (val.asInt64 == TagValueNull || val.asInt64 == TagValueUndefined)
     return 0;
 
   if (JSCELL_IS_TYPED_ARRAY(val)) {

src/runtime/ffi/abi_type.rs

@@ -191,20 +191,6 @@ impl ABIType {
         })
     }
 
-    /// Types that we can directly pass through as an `int64_t`
-    pub(crate) fn needs_a_cast_in_c(self) -> bool {
-        !matches!(
-            self,
-            ABIType::Char
-                | ABIType::Int8T
-                | ABIType::Uint8T
-                | ABIType::Int16T
-                | ABIType::Uint16T
-                | ABIType::Int32T
-                | ABIType::Uint32T
-        )
-    }
-
     pub(crate) fn is_floating_point(self) -> bool {
         matches!(self, ABIType::Double | ABIType::Float)
     }

src/runtime/ffi/ffi_body.rs

@@ -72,12 +72,15 @@ fn dangerously_run_without_jit_protections<R>(func: impl FnOnce() -> R) -> R {
     func()
 }
 
+/// Mirrors `FFIFields` in `src/jsc/bindings/ffi.cpp`.
 #[repr(C)]
 struct Offsets {
     js_array_buffer_view_offset_of_length: u32,
     js_array_buffer_view_offset_of_byte_offset: u32,
     js_array_buffer_view_offset_of_vector: u32,
     js_cell_offset_of_type: u32,
+    call_frame_argument_count_including_this_slot: u32,
+    call_frame_first_argument_slot: u32,
 }
 
 unsafe extern "C" {
@@ -2151,41 +2154,15 @@ impl Function {
             )?;
         }
 
-        if !self.arg_types.is_empty() {
+        // napi_env comes from `to_c` but still occupies its JS argument position, so `i` is the JS index.
+        if self.arg_types.iter().any(|arg| *arg != ABIType::NapiEnv) {
             writer.write_all(b"  LOAD_ARGUMENTS_FROM_CALL_FRAME;\n")?;
             for (i, arg) in self.arg_types.iter().enumerate() {
-                if *arg == ABIType::NapiEnv {
-                    write!(
-                        writer,
-                        "  napi_env arg{} = (napi_env)&Bun__thisFFIModuleNapiEnv;\n  argsPtr++;\n",
-                        i
-                    )?;
-                } else if *arg == ABIType::NapiValue {
+                if *arg != ABIType::NapiEnv {
                     writeln!(
                         writer,
-                        "  EncodedJSValue arg{} = {{ .asInt64 = *argsPtr++ }};",
-                        i
+                        "  EncodedJSValue arg{i} = {{ .asInt64 = ARGUMENT({i}) }};"
                     )?;
-                } else if arg.needs_a_cast_in_c() {
-                    if i < self.arg_types.len() - 1 {
-                        writeln!(
-                            writer,
-                            "  EncodedJSValue arg{} = {{ .asInt64 = *argsPtr++ }};",
-                            i
-                        )?;
-                    } else {
-                        write!(
-                            writer,
-                            "  EncodedJSValue arg{};\n  arg{}.asInt64 = *argsPtr;\n",
-                            i, i
-                        )?;
-                    }
-                } else {
-                    if i < self.arg_types.len() - 1 {
-                        writeln!(writer, "  int64_t arg{} = *argsPtr++;", i)?;
-                    } else {
-                        writeln!(writer, "  int64_t arg{} = *argsPtr;", i)?;
-                    }
                 }
             }
         }
@@ -2213,11 +2190,7 @@ impl Function {
 
             let length_buf = bun_core::fmt::print_int(&mut arg_buf[3..], i);
             let arg_name = &arg_buf[0..3 + length_buf];
-            if arg.needs_a_cast_in_c() {
-                write!(writer, "{}", arg.to_c(arg_name))?;
-            } else {
-                writer.write_all(arg_name)?;
-            }
+            write!(writer, "{}", arg.to_c(arg_name))?;
         }
         writer.write_all(b");\n")?;
 
@@ -2542,7 +2515,11 @@ impl CompilerRT {
         state.define_symbols(&[
             (
                 "Bun_FFI_PointerOffsetToArgumentsList",
-                bun_jsc::sizes::BUN_FFI_POINTER_OFFSET_TO_ARGUMENTS_LIST as i64,
+                offsets.call_frame_first_argument_slot as i64,
+            ),
+            (
+                "Bun_FFI_PointerOffsetToArgumentCountIncludingThis",
+                offsets.call_frame_argument_count_including_this_slot as i64,
             ),
             (
                 "JSArrayBufferView__offsetOfLength",

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

@@ -75,10 +75,13 @@ describe.skipIf(isASAN)("given an add(a, b) function", () => {
       expect(() => res.symbols.add("1", "2")).toThrow();
     });
 
-    // looks like `b` defaults to `0`, is this U.B. or expected?
-    it.skip("when passed too few arguments, throws an error", () => {
+    // A missing argument is undefined, which converts to 0, exactly as it does
+    // for a dlopen() symbol.
+    it("when passed too few arguments, the missing ones are 0", () => {
       // @ts-expect-error
-      expect(() => res.symbols.add(1)).toThrow();
+      expect(res.symbols.add(1)).toBe(1);
+      // @ts-expect-error
+      expect(res.symbols.add()).toBe(0);
     });
 
     it("when passed too many arguments, still works", () => {
@@ -101,6 +104,127 @@ describe.skipIf(isASAN)("given an add(a, b) function", () => {
   });
 }); // </given add(a, b) function>
 
+// The wrapper cc() compiles for a symbol used to load one call-frame slot per
+// declared parameter without looking at how many arguments were passed, so a
+// call with fewer arguments handed C whatever the caller's frame held past the
+// last one: garbage integers, and for pointer parameters a segfault while the
+// stale slot was being inspected as a JSValue. A missing argument is undefined,
+// which converts the way it does for dlopen() symbols: 0 for integers and
+// pointers, NaN for floating point, false for bool, undefined for napi_value.
+// Runs in a subprocess because the unfixed wrapper crashes.
+describe("calling a symbol with fewer arguments than it declares", () => {
+  it("converts the missing arguments as undefined instead of reading past the call frame", async () => {
+    using dir = tempDir("bun-ffi-cc-missing-args", {
+      // Every function reports what C received as an int, so a bad argument
+      // decode cannot be masked by the matching return conversion.
+      "arity.c": /* c */ `
+        int digits(int a, int b, int c) { return a * 100 + b * 10 + c; }
+        int u8_value(unsigned char x) { return x; }
+        int bool_value(_Bool x) { return x; }
+        int f64_is_nan(double x) { return x != x; }
+        int f32_is_nan(float x) { return x != x; }
+        int ptr_is_null(void* p) { return p == 0; }
+        int cstring_is_null(const char* s) { return s == 0; }
+        /* bit i is set when parameter i arrived as the value undefined converts to */
+        int missing_mask(int a, double b, void* c, _Bool d) {
+          return (a == 0) | ((b != b) << 1) | ((c == 0) << 2) | ((d == 0) << 3);
+        }
+        typedef struct bun_test_env* env_t;
+        typedef struct bun_test_value* value_t;
+        value_t echo_napi_value(env_t env, value_t value) { return value; }
+      `,
+      "fixture.js": /* js */ `
+        import { cc, ptr } from "bun:ffi";
+        import path from "path";
+
+        const { symbols } = cc({
+          source: path.join(import.meta.dir, "arity.c"),
+          symbols: {
+            digits: { args: ["i32", "i32", "i32"], returns: "i32" },
+            u8_value: { args: ["u8"], returns: "i32" },
+            bool_value: { args: ["bool"], returns: "i32" },
+            f64_is_nan: { args: ["f64"], returns: "i32" },
+            f32_is_nan: { args: ["f32"], returns: "i32" },
+            ptr_is_null: { args: ["ptr"], returns: "i32" },
+            cstring_is_null: { args: ["cstring"], returns: "i32" },
+            missing_mask: { args: ["i32", "f64", "ptr", "bool"], returns: "i32" },
+            echo_napi_value: { args: ["napi_env", "napi_value"], returns: "napi_value" },
+          },
+        });
+
+        const bytes = new Uint8Array(8);
+
+        // Repeated calls from one call site walk through the JIT tiers, which
+        // is where the stale slot past the arguments tends to hold a pointer.
+        let nullPointers = 0;
+        for (let i = 0; i < 500; i++) nullPointers += symbols.ptr_is_null();
+
+        const results = {
+          digits: {
+            none: symbols.digits(),
+            one: symbols.digits(1),
+            two: symbols.digits(1, 2),
+            all: symbols.digits(1, 2, 3),
+            extra: symbols.digits(1, 2, 3, 4),
+          },
+          u8: { missing: symbols.u8_value(), passed: symbols.u8_value(200) },
+          bool: { missing: symbols.bool_value(), passed: symbols.bool_value(true) },
+          f64_is_nan: { missing: symbols.f64_is_nan(), passed: symbols.f64_is_nan(1.5) },
+          f32_is_nan: { missing: symbols.f32_is_nan(), passed: symbols.f32_is_nan(1.5) },
+          ptr_is_null: {
+            missing: symbols.ptr_is_null(),
+            missing_500_times: nullPointers,
+            undefined: symbols.ptr_is_null(undefined),
+            null: symbols.ptr_is_null(null),
+            typed_array: symbols.ptr_is_null(bytes),
+            address: symbols.ptr_is_null(ptr(bytes)),
+          },
+          cstring_is_null: { missing: symbols.cstring_is_null(), undefined: symbols.cstring_is_null(undefined) },
+          missing_mask: {
+            none: symbols.missing_mask(),
+            first_only: symbols.missing_mask(5),
+            all: symbols.missing_mask(5, 2.5, bytes, true),
+          },
+          // napi_env takes up position 0 of the JS argument list but is filled in by the wrapper.
+          echo_napi_value: {
+            none: typeof symbols.echo_napi_value(),
+            placeholder_only: typeof symbols.echo_napi_value(null),
+            passed: symbols.echo_napi_value(undefined, 42),
+          },
+        };
+        console.log(JSON.stringify(results));
+      `,
+    });
+
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "fixture.js"],
+      env: bunEnv,
+      cwd: String(dir),
+      stderr: "pipe",
+    });
+
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+
+    // stderr is included in the received object so failures show it, but is not
+    // asserted empty: debug builds emit benign startup warnings.
+    const results = stdout.startsWith("{") ? JSON.parse(stdout) : stdout;
+    expect({ results, stderr, exitCode }).toMatchObject({
+      results: {
+        digits: { none: 0, one: 100, two: 120, all: 123, extra: 123 },
+        u8: { missing: 0, passed: 200 },
+        bool: { missing: 0, passed: 1 },
+        f64_is_nan: { missing: 1, passed: 0 },
+        f32_is_nan: { missing: 1, passed: 0 },
+        ptr_is_null: { missing: 1, missing_500_times: 500, undefined: 1, null: 1, typed_array: 0, address: 0 },
+        cstring_is_null: { missing: 1, undefined: 1 },
+        missing_mask: { none: 0b1111, first_only: 0b1110, all: 0 },
+        echo_napi_value: { none: "undefined", placeholder_only: "undefined", passed: 42 },
+      },
+      exitCode: 0,
+    });
+  });
+});
+
 describe("given a source file with syntax errors", () => {
   const source = /* c */ `
     int add(int a, int b) {

test/js/bun/ffi/ffi.test.fixture.receiver.c

@@ -130,13 +130,16 @@ EncodedJSValue ValueTrue = { TagValueTrue };
 
 typedef void* JSContext;
 
-// Bun_FFI_PointerOffsetToArgumentsList is injected into the build 
-// The value is generated in `make sizegen`
-// The value is 6.
-// On ARM64_32, the value is something else but it really doesn't matter for our case
-// However, I don't want this to subtly break amidst future upgrades to JavaScriptCore
+// callFrame is the JSC::CallFrame (an array of 8-byte slots); the two slot indices
+// are defined from JSC::CallFrameSlot by src/jsc/bindings/ffi.cpp.
 #define LOAD_ARGUMENTS_FROM_CALL_FRAME \
-  int64_t *argsPtr = (int64_t*)((size_t*)callFrame + Bun_FFI_PointerOffsetToArgumentsList)
+  int64_t *argsPtr = (int64_t*)((size_t*)callFrame + Bun_FFI_PointerOffsetToArgumentsList); \
+  int32_t argsCount = ((EncodedJSValue*)((size_t*)callFrame + Bun_FFI_PointerOffsetToArgumentCountIncludingThis))->asBits.payload - 1
+
+// Bits of JSC::CallFrame::argument(i): a slot past argsCount is stale stack, so an
+// argument that was not passed reads as undefined. int64_t rather than a union copy:
+// TinyCC emits a memmove call for those on non-x86_64 and this file is built -nostdlib.
+#define ARGUMENT(i) ((i) < argsCount ? argsPtr[i] : TagValueUndefined)
 
 
 
@@ -210,7 +213,8 @@ static uint64_t JSVALUE_TO_TYPED_ARRAY_LENGTH(EncodedJSValue val) {
 // This behavior change enables the JIT to handle it better
 // It also is better readability when console.log(myPtr)
 static void* JSVALUE_TO_PTR(EncodedJSValue val) {
-  if (val.asInt64 == TagValueNull)
+  // undefined (e.g. an argument that was not passed) is NULL, as in the engine's writePointerSlot.
+  if (val.asInt64 == TagValueNull || val.asInt64 == TagValueUndefined)
     return 0;
 
   if (JSCELL_IS_TYPED_ARRAY(val)) {
@@ -374,8 +378,7 @@ float not_a_callback(float arg0);
 /* ---- Your Wrapper Function ---- */
 ZIG_REPR_TYPE JSFunctionCall(void* JS_GLOBAL_OBJECT, void* callFrame) {
   LOAD_ARGUMENTS_FROM_CALL_FRAME;
-  EncodedJSValue arg0;
-  arg0.asInt64 = *argsPtr;
+  EncodedJSValue arg0 = { .asInt64 = ARGUMENT(0) };
     float return_value = not_a_callback(    JSVALUE_TO_FLOAT(arg0));
 
     return FLOAT_TO_JSVALUE(return_value).asZigRepr;