From 354fa6cd0013aeca9f0749822b3ef3050c764370 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Mon, 20 Jun 2022 19:08:10 +0530 Subject: [PATCH] BREAKING(ext/ffi): Remove `Deno.UnsafePointer` indirection (#14915) --- cli/dts/lib.deno.unstable.d.ts | 39 ++--- cli/tests/unit/ffi_test.ts | 5 +- ext/ffi/00_ffi.js | 263 ++++----------------------------- test_ffi/tests/test.js | 47 +++--- 4 files changed, 60 insertions(+), 294 deletions(-) diff --git a/cli/dts/lib.deno.unstable.d.ts b/cli/dts/lib.deno.unstable.d.ts index 3482d1c80c..389d08f9fb 100644 --- a/cli/dts/lib.deno.unstable.d.ts +++ b/cli/dts/lib.deno.unstable.d.ts @@ -390,17 +390,14 @@ declare namespace Deno { ? void : T extends StaticNativeBigIntType ? bigint : T extends StaticNativeNumberType ? number - : T extends "pointer" ? UnsafePointer + : T extends "pointer" ? bigint : never; type StaticForeignFunctionParameter = T extends "void" ? void : T extends StaticNativeNumberType | StaticNativeBigIntType ? number | bigint - : T extends "pointer" ? UnsafePointer | TypedArray | null - : T extends NativeCallbackType< - infer U extends readonly NativeType[], - infer V extends NativeParameterType - > ? UnsafeCallback | UnsafePointer | null + : T extends "pointer" ? bigint | TypedArray | null + : T extends NativeCallbackType ? bigint | null : unknown; /** Infers a foreign function parameter list. */ @@ -449,19 +446,10 @@ declare namespace Deno { * An unsafe pointer to a memory location for passing and returning pointers to and from the ffi */ export class UnsafePointer { - constructor(value: bigint); - - value: bigint; - /** * Return the direct memory pointer to the typed array in memory */ - static of(typedArray: TypedArray): UnsafePointer; - - /** - * Returns the value of the pointer which is useful in certain scenarios. - */ - valueOf(): bigint; + static of(value: Deno.UnsafeCallback | TypedArray): bigint; } /** **UNSTABLE**: Unsafe and new API, beware! @@ -472,9 +460,9 @@ declare namespace Deno { * (numbers, strings and raw bytes). */ export class UnsafePointerView { - constructor(pointer: UnsafePointer); + constructor(pointer: bigint); - pointer: UnsafePointer; + pointer: bigint; /** Gets an unsigned 8-bit integer at the specified byte offset from the pointer. */ getUint8(offset?: number): number; @@ -511,10 +499,10 @@ declare namespace Deno { * present as symbols. */ export class UnsafeFnPointer { - pointer: UnsafePointer; + pointer: bigint; definition: Fn; - constructor(pointer: UnsafePointer, definition: Fn); + constructor(pointer: bigint, definition: Fn); call( ...args: StaticForeignFunctionParameters @@ -552,18 +540,15 @@ declare namespace Deno { type UnsafeCallbackParameter = T extends StaticNativeBigIntType ? bigint : T extends StaticNativeNumberType ? number - : T extends "pointer" ? UnsafePointer + : T extends "pointer" ? bigint : never; type UnsafeCallbackResult = T extends "void" ? void : T extends StaticNativeBigIntType ? number | bigint : T extends StaticNativeNumberType ? number - : T extends "pointer" ? UnsafePointer | TypedArray | null - : T extends NativeCallbackType< - infer U extends readonly NativeType[], - infer V extends NativeParameterType - > ? UnsafeCallback | UnsafePointer | null + : T extends "pointer" ? bigint | TypedArray | null + : T extends NativeCallbackType ? bigint | null : never; type UnsafeCallbackFunction< @@ -594,7 +579,7 @@ declare namespace Deno { callback: UnsafeCallbackFunction, ); - pointer: UnsafePointer; + pointer: bigint; definition: UnsafeCallbackDefinition; callback: UnsafeCallbackFunction; diff --git a/cli/tests/unit/ffi_test.ts b/cli/tests/unit/ffi_test.ts index 22da88d27a..18b8311377 100644 --- a/cli/tests/unit/ffi_test.ts +++ b/cli/tests/unit/ffi_test.ts @@ -28,9 +28,8 @@ Deno.test({ permissions: { ffi: false } }, function ffiPermissionDenied() { assertThrows(() => { Deno.dlopen("/usr/lib/libc.so.6", {}); }, Deno.errors.PermissionDenied); - const ptr = new Deno.UnsafePointer(0n); const fnptr = new Deno.UnsafeFnPointer( - ptr, + 0n, { parameters: ["u32", "pointer"], result: "void", @@ -42,7 +41,7 @@ Deno.test({ permissions: { ffi: false } }, function ffiPermissionDenied() { assertThrows(() => { Deno.UnsafePointer.of(new Uint8Array(0)); }, Deno.errors.PermissionDenied); - const ptrView = new Deno.UnsafePointerView(ptr); + const ptrView = new Deno.UnsafePointerView(0n); assertThrows(() => { ptrView.copyInto(new Uint8Array(0)); }, Deno.errors.PermissionDenied); diff --git a/ext/ffi/00_ffi.js b/ext/ffi/00_ffi.js index d29e83fce2..1908733076 100644 --- a/ext/ffi/00_ffi.js +++ b/ext/ffi/00_ffi.js @@ -5,12 +5,7 @@ const core = window.Deno.core; const __bootstrap = window.__bootstrap; const { - ArrayBufferPrototype, - ArrayPrototypePush, - ArrayPrototypeSome, BigInt, - NumberIsFinite, - NumberIsInteger, ObjectDefineProperty, ObjectPrototypeIsPrototypeOf, PromisePrototypeThen, @@ -37,77 +32,77 @@ getUint8(offset = 0) { return core.opSync( "op_ffi_read_u8", - this.pointer.value + BigInt(offset), + this.pointer + BigInt(offset), ); } getInt8(offset = 0) { return core.opSync( "op_ffi_read_i8", - this.pointer.value + BigInt(offset), + this.pointer + BigInt(offset), ); } getUint16(offset = 0) { return core.opSync( "op_ffi_read_u16", - this.pointer.value + BigInt(offset), + this.pointer + BigInt(offset), ); } getInt16(offset = 0) { return core.opSync( "op_ffi_read_i16", - this.pointer.value + BigInt(offset), + this.pointer + BigInt(offset), ); } getUint32(offset = 0) { return core.opSync( "op_ffi_read_u32", - this.pointer.value + BigInt(offset), + this.pointer + BigInt(offset), ); } getInt32(offset = 0) { return core.opSync( "op_ffi_read_i32", - this.pointer.value + BigInt(offset), + this.pointer + BigInt(offset), ); } getBigUint64(offset = 0) { return core.opSync( "op_ffi_read_u64", - this.pointer.value + BigInt(offset), + this.pointer + BigInt(offset), ); } getBigInt64(offset = 0) { return core.opSync( "op_ffi_read_u64", - this.pointer.value + BigInt(offset), + this.pointer + BigInt(offset), ); } getFloat32(offset = 0) { return core.opSync( "op_ffi_read_f32", - this.pointer.value + BigInt(offset), + this.pointer + BigInt(offset), ); } getFloat64(offset = 0) { return core.opSync( "op_ffi_read_f64", - this.pointer.value + BigInt(offset), + this.pointer + BigInt(offset), ); } getCString(offset = 0) { return core.opSync( "op_ffi_cstr_read", - this.pointer.value + BigInt(offset), + this.pointer + BigInt(offset), ); } @@ -120,7 +115,7 @@ copyInto(destination, offset = 0) { core.opSync( "op_ffi_buf_copy_into", - this.pointer.value + BigInt(offset), + this.pointer + BigInt(offset), destination, destination.byteLength, ); @@ -128,125 +123,12 @@ } class UnsafePointer { - value; - - constructor(value) { - if (typeof value === "number") { - value = BigInt(value); + static of(value) { + if (ObjectPrototypeIsPrototypeOf(UnsafeCallbackPrototype, value)) { + return value.pointer; } - this.value = value; + return core.opSync("op_ffi_ptr_of", value); } - - static of(typedArray) { - return new UnsafePointer( - core.opSync("op_ffi_ptr_of", typedArray), - ); - } - - valueOf() { - return this.value; - } - } - const UnsafePointerPrototype = UnsafePointer.prototype; - - function prepareArgs(types, args) { - const parameters = []; - - if (types.length !== args.length) { - throw new TypeError("Invalid FFI call, parameter vs args count mismatch"); - } - - for (let i = 0; i < types.length; i++) { - const type = types[i]; - const arg = args[i]; - - if (type === "u8" || type === "u16" || type === "u32") { - if (!NumberIsInteger(arg) || arg < 0) { - throw new TypeError( - `Expected FFI argument to be an unsigned integer, but got '${arg}'`, - ); - } - ArrayPrototypePush(parameters, arg); - } else if (type === "i8" || type === "i16" || type === "i32") { - if (!NumberIsInteger(arg)) { - throw new TypeError( - `Expected FFI argument to be a signed integer, but got '${arg}'`, - ); - } - ArrayPrototypePush(parameters, arg); - } else if (type === "u64" || type === "usize") { - if ( - !(NumberIsInteger(arg) && arg >= 0 || - typeof arg === "bigint" && 0n <= arg && arg <= 0xffffffffffffffffn) - ) { - throw new TypeError( - `Expected FFI argument to be an unsigned integer, but got '${arg}'`, - ); - } - ArrayPrototypePush(parameters, arg); - } else if (type == "i64" || type === "isize") { - if ( - !(NumberIsInteger(arg) || - typeof arg === "bigint" && -1n * 2n ** 63n <= arg && - arg <= 2n ** 63n - 1n) - ) { - throw new TypeError( - `Expected FFI argument to be a signed integer, but got '${arg}'`, - ); - } - ArrayPrototypePush(parameters, arg); - } else if (type === "f32" || type === "f64") { - if (!NumberIsFinite(arg)) { - throw new TypeError( - `Expected FFI argument to be a number, but got '${arg}'`, - ); - } - ArrayPrototypePush(parameters, arg); - } else if (type === "pointer") { - if ( - ObjectPrototypeIsPrototypeOf(ArrayBufferPrototype, arg?.buffer) && - arg.byteLength !== undefined - ) { - ArrayPrototypePush(parameters, arg); - } else if (ObjectPrototypeIsPrototypeOf(UnsafePointerPrototype, arg)) { - ArrayPrototypePush(parameters, arg.value); - } else if (arg === null) { - ArrayPrototypePush(parameters, null); - } else { - throw new TypeError( - "Expected FFI argument to be TypedArray, UnsafePointer or null", - ); - } - } else if ( - typeof type === "object" && type !== null && "function" in type - ) { - if (ObjectPrototypeIsPrototypeOf(UnsafeCallbackPrototype, arg)) { - // Own registered callback, pass the pointer value - ArrayPrototypePush(parameters, arg.pointer.value); - } else if (arg === null) { - // nullptr - ArrayPrototypePush(parameters, null); - } else if ( - ObjectPrototypeIsPrototypeOf(UnsafeFnPointerPrototype, arg) - ) { - // Foreign function, pass the pointer value - ArrayPrototypePush(parameters, arg.pointer.value); - } else if ( - ObjectPrototypeIsPrototypeOf(UnsafePointerPrototype, arg) - ) { - // Foreign function, pass the pointer value - ArrayPrototypePush(parameters, arg.value); - } else { - throw new TypeError( - "Expected FFI argument to be UnsafeCallback, UnsafeFnPointer, UnsafePointer or null", - ); - } - } else { - throw new TypeError(`Invalid FFI argument type '${type}'`); - } - } - - return parameters; } function unpackNonblockingReturnValue(type, result) { @@ -254,7 +136,7 @@ typeof type === "object" && type !== null && "function" in type || type === "pointer" ) { - return new UnsafePointer(unpackU64(result)); + return unpackU64(result); } switch (type) { case "isize": @@ -277,16 +159,12 @@ this.definition = definition; } - call(...args) { - const parameters = prepareArgs( - this.definition.parameters, - args, - ); + call(...parameters) { const resultType = this.definition.result; if (this.definition.nonblocking) { const promise = core.opAsync( "op_ffi_call_ptr_nonblocking", - this.pointer.value, + this.pointer, this.definition, parameters, ); @@ -302,24 +180,16 @@ return promise; } else { - const result = core.opSync( + return core.opSync( "op_ffi_call_ptr", - this.pointer.value, + this.pointer, this.definition, parameters, ); - - if (isPointerType(resultType)) { - return new UnsafePointer(result); - } - - return result; } } } - const UnsafeFnPointerPrototype = UnsafeFnPointer.prototype; - function isPointerType(type) { return type === "pointer" || typeof type === "object" && type !== null && "function" in type; @@ -330,64 +200,8 @@ type === "usize" || type === "isize"; } - function prepareUnsafeCallbackParameters(types, args) { - const parameters = []; - if (types.length === 0) { - return parameters; - } - - for (let i = 0; i < types.length; i++) { - const type = types[i]; - const arg = args[i]; - ArrayPrototypePush( - parameters, - isPointerType(type) ? new UnsafePointer(arg) : arg, - ); - } - - return parameters; - } - - function unwrapUnsafeCallbackReturnValue(result) { - if ( - ObjectPrototypeIsPrototypeOf(UnsafePointerPrototype, result) - ) { - // Foreign function, return the pointer value - ArrayPrototypePush(parameters, result.value); - } else if ( - ObjectPrototypeIsPrototypeOf(UnsafeFnPointerPrototype, result) - ) { - // Foreign function, return the pointer value - ArrayPrototypePush(parameters, result.pointer.value); - } else if ( - ObjectPrototypeIsPrototypeOf(UnsafeCallbackPrototype, result) - ) { - // Own registered callback, return the pointer value. - // Note that returning the ResourceId here would not work as - // the Rust side code cannot access OpState to get the resource. - ArrayPrototypePush(parameters, result.pointer.value); - } - return result; - } - - function createInternalCallback(definition, callback) { - const mustUnwrap = isPointerType(definition.result); - return (...args) => { - const convertedArgs = prepareUnsafeCallbackParameters( - definition.parameters, - args, - ); - const result = callback(...convertedArgs); - if (mustUnwrap) { - return unwrapUnsafeCallbackReturnValue(result); - } - return result; - }; - } - class UnsafeCallback { #rid; - #internal; definition; callback; pointer; @@ -398,20 +212,13 @@ "Invalid UnsafeCallback, cannot be nonblocking", ); } - const needsWrapping = isPointerType(definition.result) || - ArrayPrototypeSome(definition.parameters, isPointerType); - const internalCallback = needsWrapping - ? createInternalCallback(definition, callback) - : callback; - const [rid, pointer] = core.opSync( "op_ffi_unsafe_callback_create", definition, - internalCallback, + callback, ); this.#rid = rid; - this.pointer = new UnsafePointer(pointer); - this.#internal = internalCallback; + this.pointer = pointer; this.definition = definition; this.callback = callback; } @@ -440,15 +247,12 @@ } const name = symbols[symbol].name || symbol; - let value = core.opSync( + const value = core.opSync( "op_ffi_get_static", this.#rid, name, type, ); - if (type === "pointer") { - value = new UnsafePointer(value); - } ObjectDefineProperty( this.symbols, symbol, @@ -463,15 +267,12 @@ } const isNonBlocking = symbols[symbol].nonblocking; - const types = symbols[symbol].parameters; const resultType = symbols[symbol].result; let fn; if (isNonBlocking) { const needsUnpacking = isReturnedAsBigInt(resultType); - fn = (...args) => { - const parameters = prepareArgs(types, args); - + fn = (...parameters) => { const promise = core.opAsync( "op_ffi_call_nonblocking", this.#rid, @@ -489,23 +290,13 @@ return promise; }; } else { - const mustWrap = isPointerType(resultType); - fn = (...args) => { - const parameters = prepareArgs(types, args); - - const result = core.opSync( + fn = (...parameters) => + core.opSync( "op_ffi_call", this.#rid, symbol, parameters, ); - - if (mustWrap) { - return new UnsafePointer(result); - } - - return result; - }; } ObjectDefineProperty( diff --git a/test_ffi/tests/test.js b/test_ffi/tests/test.js index 1568abcbd3..0caa416a15 100644 --- a/test_ffi/tests/test.js +++ b/test_ffi/tests/test.js @@ -13,6 +13,10 @@ const libPath = `${targetDir}/${libPrefix}test_ffi.${libSuffix}`; const resourcesPre = Deno.resources(); +function ptr(v) { + return Deno.UnsafePointer.of(v); +} + // dlopen shouldn't panic assertThrows(() => { Deno.dlopen("cli/src/main.rs", {}); @@ -188,9 +192,9 @@ const buffer = new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8]); const buffer2 = new Uint8Array([9, 10]); dylib.symbols.print_buffer(buffer, buffer.length); dylib.symbols.print_buffer2(buffer, buffer.length, buffer2, buffer2.length); -const ptr = dylib.symbols.return_buffer(); -dylib.symbols.print_buffer(ptr, 8); -const ptrView = new Deno.UnsafePointerView(ptr); +const ptr0 = dylib.symbols.return_buffer(); +dylib.symbols.print_buffer(ptr0, 8); +const ptrView = new Deno.UnsafePointerView(ptr0); const into = new Uint8Array(6); const into2 = new Uint8Array(3); const into2ptr = Deno.UnsafePointer.of(into2); @@ -210,7 +214,7 @@ const stringPtr = Deno.UnsafePointer.of(string); const stringPtrview = new Deno.UnsafePointerView(stringPtr); console.log(stringPtrview.getCString()); console.log(stringPtrview.getCString(11)); -console.log(Boolean(dylib.symbols.is_null_ptr(ptr))); +console.log(Boolean(dylib.symbols.is_null_ptr(ptr0))); console.log(Boolean(dylib.symbols.is_null_ptr(null))); console.log(Boolean(dylib.symbols.is_null_ptr(Deno.UnsafePointer.of(into)))); @@ -232,20 +236,7 @@ await sleepNonBlocking.call(100); console.log(performance.now() - before >= 100); console.log(dylib.symbols.add_u32(123, 456)); -assertThrows( - () => { - dylib.symbols.add_u32(-1, 100); - }, - TypeError, - "Expected FFI argument to be an unsigned integer, but got '-1'", -); -assertThrows( - () => { - dylib.symbols.add_u32(null, 100); - }, - TypeError, - "Expected FFI argument to be an unsigned integer, but got 'null'", -); + console.log(dylib.symbols.add_i32(123, 456)); console.log(dylib.symbols.add_u64(0xffffffffn, 0xffffffffn)); console.log(dylib.symbols.add_i64(-0xffffffffn, -0xffffffffn)); @@ -345,7 +336,7 @@ const logManyParametersCallback = new Deno.UnsafeCallback({ ], result: "void", }, (u8, i8, u16, i16, u32, i32, u64, i64, f32, f64, pointer) => { - const view = new Deno.UnsafePointerView(new Deno.UnsafePointer(pointer)); + const view = new Deno.UnsafePointerView(pointer); const copy_buffer = new Uint8Array(8); view.copyInto(copy_buffer); console.log(u8, i8, u16, i16, u32, i32, u64, i64, f32, f64, ...copy_buffer); @@ -373,19 +364,19 @@ const throwCallback = new Deno.UnsafeCallback({ assertThrows( () => { - dylib.symbols.call_fn_ptr(throwCallback); + dylib.symbols.call_fn_ptr(ptr(throwCallback)); }, TypeError, "hi", ); -dylib.symbols.call_fn_ptr(logCallback); -dylib.symbols.call_fn_ptr_many_parameters(logManyParametersCallback); -dylib.symbols.call_fn_ptr_return_u8(returnU8Callback); -dylib.symbols.call_fn_ptr_return_buffer(returnBufferCallback); -dylib.symbols.store_function(logCallback); +dylib.symbols.call_fn_ptr(ptr(logCallback)); +dylib.symbols.call_fn_ptr_many_parameters(ptr(logManyParametersCallback)); +dylib.symbols.call_fn_ptr_return_u8(ptr(returnU8Callback)); +dylib.symbols.call_fn_ptr_return_buffer(ptr(returnBufferCallback)); +dylib.symbols.store_function(ptr(logCallback)); dylib.symbols.call_stored_function(); -dylib.symbols.store_function_2(add10Callback); +dylib.symbols.store_function_2(ptr(add10Callback)); dylib.symbols.call_stored_function_2(20); const nestedCallback = new Deno.UnsafeCallback( @@ -394,7 +385,7 @@ const nestedCallback = new Deno.UnsafeCallback( dylib.symbols.call_stored_function_2(10); }, ); -dylib.symbols.store_function(nestedCallback); +dylib.symbols.store_function(ptr(nestedCallback)); dylib.symbols.store_function(null); dylib.symbols.store_function_2(null); @@ -404,7 +395,7 @@ console.log("Static u32:", dylib.symbols.static_u32); console.log("Static i64:", dylib.symbols.static_i64); console.log( "Static ptr:", - dylib.symbols.static_ptr instanceof Deno.UnsafePointer, + typeof dylib.symbols.static_ptr === "bigint", ); const view = new Deno.UnsafePointerView(dylib.symbols.static_ptr); console.log("Static ptr value:", view.getUint32());