diff --git a/src/jsc/bindings/JSFFIFunction.cpp b/src/jsc/bindings/JSFFIFunction.cpp index 2dc7bd22005c..b223ff014c44 100644 --- a/src/jsc/bindings/JSFFIFunction.cpp +++ b/src/jsc/bindings/JSFFIFunction.cpp @@ -26,6 +26,7 @@ #include "root.h" #include "JSFFIFunction.h" +#include #include #include #include "ZigGlobalObject.h" @@ -73,14 +74,15 @@ extern "C" void Bun__FFIFunction_setDataPtr(JSC::EncodedJSValue jsValue, void* p extern "C" JSC::EncodedJSValue Bun__CreateFFIFunctionValue(Zig::GlobalObject* globalObject, const ZigString* symbolName, unsigned argCount, Zig::FFIFunction functionPointer, bool addPtrField, void* symbolFromDynamicLibrary) { if (addPtrField) { - auto* function = Zig::JSFFIFunction::createForFFI(globalObject->vm(), globalObject, argCount, symbolName != nullptr ? Zig::toStringCopy(*symbolName) : String(), reinterpret_cast(functionPointer)); auto& vm = JSC::getVM(globalObject); - // We should only expose the "ptr" field when it's a JSCallback for bun:ffi. - // Not for internal usages of this function type. - // We should also consider a separate JSFunction type for our usage to not have this branch in the first place... - function->putDirect(vm, JSC::Identifier::fromString(vm, String("ptr"_s)), JSC::jsNumber(std::bit_cast(functionPointer)), JSC::PropertyAttribute::ReadOnly | 0); + auto scope = DECLARE_THROW_SCOPE(vm); + auto* function = Zig::JSFFIFunction::createForFFI(vm, globalObject, argCount, symbolName != nullptr ? Zig::toStringCopy(*symbolName) : String(), reinterpret_cast(functionPointer)); + // `functionPointer` is the TinyCC-compiled JSC-ABI wrapper, so `.ptr` must be the native function it calls. + JSC::JSValue ptr = JSC::FFI::pointerToJSValue(globalObject, reinterpret_cast(symbolFromDynamicLibrary)); + RETURN_IF_EXCEPTION(scope, {}); + function->putDirect(vm, JSC::Identifier::fromString(vm, "ptr"_s), ptr, JSC::PropertyAttribute::ReadOnly | 0); function->symbolFromDynamicLibrary = symbolFromDynamicLibrary; - return JSC::JSValue::encode(function); + RELEASE_AND_RETURN(scope, JSC::JSValue::encode(function)); } return Bun__CreateFFIFunctionWithDataValue(globalObject, symbolName, argCount, functionPointer, nullptr); diff --git a/src/runtime/ffi/ffi_body.rs b/src/runtime/ffi/ffi_body.rs index 679a87b1e36a..5571044cbf10 100644 --- a/src/runtime/ffi/ffi_body.rs +++ b/src/runtime/ffi/ffi_body.rs @@ -100,7 +100,7 @@ unsafe extern "C" { arg_count: u32, function_pointer: *const c_void, add_ptr_property: bool, - input_function_ptr: *mut c_void, + symbol_from_dynamic_library: *mut c_void, ) -> JSValue; fn Bun__CreateJSCFFIFunction( @@ -164,7 +164,10 @@ mod exposed_to_ffi { } } -/// `host_fn::NewRuntimeFunction` thin wrapper. See host_fn.rs:310. +/// `cc()`'s variant of `host_fn::new_runtime_function`: with `add_ptr_property` +/// the C++ side encodes `symbol_from_dynamic_library` as the symbol's `.ptr`, +/// which allocates (and can throw) for addresses above 2^53. +#[track_caller] #[inline] fn new_runtime_function( global: &JSGlobalObject, @@ -172,20 +175,22 @@ fn new_runtime_function( arg_count: u32, function_pointer: *const c_void, add_ptr_property: bool, - input_function_ptr: Option<*mut c_void>, -) -> JSValue { - // SAFETY: thin FFI wrapper; `global` is a live opaque JSC handle, - // `function_pointer` is a JIT'd entry point owned by the caller. - unsafe { - Bun__CreateFFIFunctionValue( - global, - symbol_name, - arg_count, - function_pointer, - add_ptr_property, - input_function_ptr.unwrap_or(core::ptr::null_mut()), - ) - } + symbol_from_dynamic_library: Option<*mut c_void>, +) -> JsResult { + jsc::call_zero_is_throw(global, || { + // SAFETY: thin FFI wrapper; `global` is a live opaque JSC handle, + // `function_pointer` is a JIT'd entry point owned by the caller. + unsafe { + Bun__CreateFFIFunctionValue( + global, + symbol_name, + arg_count, + function_pointer, + add_ptr_property, + symbol_from_dynamic_library.unwrap_or(core::ptr::null_mut()), + ) + } + }) } /// `jsc::codegen::JSFFI::symbols_value_set_cached` thin wrapper. @@ -1249,7 +1254,7 @@ impl FFI { compiled.ptr.cast_const(), true, function.symbol_from_dynamic_library, - ); + )?; // `cb` is rooted by the `symbolsValue` cached own-property set below. obj.put(global_this, str.slice(), cb); } diff --git a/test/js/bun/ffi/cc.test.ts b/test/js/bun/ffi/cc.test.ts index 0726df91e629..e78947470aec 100644 --- a/test/js/bun/ffi/cc.test.ts +++ b/test/js/bun/ffi/cc.test.ts @@ -1189,3 +1189,73 @@ describe.skipIf(isASAN)("compiler runtime header directory under BUN_TMPDIR", () expect(exitCode).toBe(0); }); }); + +describe("symbols[name].ptr", () => { + // `.ptr` used to be the bits of the TinyCC wrapper's address reinterpreted as + // a double (a denormal like 6e-310), so CFunction/linkSymbols rejected it and + // a C function receiving it as a function pointer called NULL. The fixture + // runs in a child process because that NULL call takes the process down. + it("is the address of the compiled C function and is accepted wherever bun:ffi takes a pointer", async () => { + using dir = tempDir("bun-ffi-cc-symbol-ptr", { + "symbols.c": /* c */ ` + int forty_two(void) { return 42; } + int add(int a, int b) { return a + b; } + const char* greeting(void) { return "hi"; } + typedef int (*binop)(int, int); + int apply(binop f, int a, int b) { return f(a, b); } + `, + "fixture.js": /* js */ ` + import { cc, CFunction, linkSymbols } from "bun:ffi"; + import path from "path"; + + const { symbols } = cc({ + source: path.join(import.meta.dir, "symbols.c"), + symbols: { + forty_two: { args: [], returns: "i32" }, + add: { args: ["i32", "i32"], returns: "i32" }, + // returns: "cstring" symbols are wrapped in JS; the wrapper copies .ptr + greeting: { args: [], returns: "cstring" }, + apply: { args: ["function", "i32", "i32"], returns: "i32" }, + }, + }); + + const shape = value => [typeof value, Number.isInteger(value) && value > 0]; + const results = { + forty_two_ptr: shape(symbols.forty_two.ptr), + add_ptr: shape(symbols.add.ptr), + greeting_ptr: shape(symbols.greeting.ptr), + distinct_addresses: new Set([symbols.forty_two.ptr, symbols.add.ptr, symbols.greeting.ptr]).size, + cfunction: new CFunction({ ptr: symbols.forty_two.ptr, args: [], returns: "i32" })(), + cfunction_cstring: new CFunction({ ptr: symbols.greeting.ptr, args: [], returns: "cstring" })(), + link_symbols: linkSymbols({ add: { ptr: symbols.add.ptr, args: ["i32", "i32"], returns: "i32" } }).symbols.add(40, 2), + function_argument: symbols.apply(symbols.add.ptr, 20, 22), + }; + 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]); + + const results = stdout.startsWith("{") ? JSON.parse(stdout) : stdout; + expect({ results, stderr, exitCode }).toMatchObject({ + results: { + forty_two_ptr: ["number", true], + add_ptr: ["number", true], + greeting_ptr: ["number", true], + distinct_addresses: 3, + cfunction: 42, + cfunction_cstring: "hi", + link_symbols: 42, + function_argument: 42, + }, + exitCode: 0, + }); + }); +});