diff --git a/ext/ffi/00_ffi.js b/ext/ffi/00_ffi.js index 67cb13ab6d..fe7344e177 100644 --- a/ext/ffi/00_ffi.js +++ b/ext/ffi/00_ffi.js @@ -5,6 +5,7 @@ const ops = core.ops; const primordials = globalThis.__bootstrap.primordials; const { ArrayBufferIsView, + ArrayBufferPrototype, ArrayBufferPrototypeGetByteLength, ArrayPrototypeMap, ArrayPrototypeJoin, @@ -221,7 +222,24 @@ class UnsafePointer { if (ObjectPrototypeIsPrototypeOf(UnsafeCallbackPrototype, value)) { return value.pointer; } - const pointer = ops.op_ffi_ptr_of(value); + let pointer; + if (ArrayBufferIsView(value)) { + if (value.length === 0) { + pointer = ops.op_ffi_ptr_of_exact(value); + } else { + pointer = ops.op_ffi_ptr_of(value); + } + } else if (ObjectPrototypeIsPrototypeOf(ArrayBufferPrototype, value)) { + if (value.length === 0) { + pointer = ops.op_ffi_ptr_of_exact(new Uint8Array(value)); + } else { + pointer = ops.op_ffi_ptr_of(new Uint8Array(value)); + } + } else { + throw new TypeError( + "Expected ArrayBuffer, ArrayBufferView or UnsafeCallbackPrototype", + ); + } if (pointer) { POINTER_TO_BUFFER_WEAK_MAP.set(pointer, value); } diff --git a/ext/ffi/lib.rs b/ext/ffi/lib.rs index 1ca921119e..a3cd0ebdae 100644 --- a/ext/ffi/lib.rs +++ b/ext/ffi/lib.rs @@ -74,6 +74,7 @@ deno_core::extension!(deno_ffi, op_ffi_ptr_create

, op_ffi_ptr_equals

, op_ffi_ptr_of

, + op_ffi_ptr_of_exact

, op_ffi_ptr_offset

, op_ffi_ptr_value

, op_ffi_get_buf

, diff --git a/ext/ffi/repr.rs b/ext/ffi/repr.rs index 665e37186f..f3bf013102 100644 --- a/ext/ffi/repr.rs +++ b/ext/ffi/repr.rs @@ -5,8 +5,7 @@ use crate::FfiPermissions; use deno_core::error::range_error; use deno_core::error::type_error; use deno_core::error::AnyError; -use deno_core::op; -use deno_core::serde_v8; +use deno_core::op2; use deno_core::v8; use deno_core::OpState; use std::ffi::c_char; @@ -14,10 +13,10 @@ use std::ffi::c_void; use std::ffi::CStr; use std::ptr; -#[op(fast)] -fn op_ffi_ptr_create( +#[op2(fast)] +pub fn op_ffi_ptr_create( state: &mut OpState, - ptr_number: usize, + #[bigint] ptr_number: usize, ) -> Result<*mut c_void, AnyError> where FP: FfiPermissions + 'static, @@ -29,7 +28,7 @@ where Ok(ptr_number as *mut c_void) } -#[op(fast)] +#[op2(fast)] pub fn op_ffi_ptr_equals( state: &mut OpState, a: *const c_void, @@ -45,10 +44,10 @@ where Ok(a == b) } -#[op(fast)] +#[op2(fast)] pub fn op_ffi_ptr_of( state: &mut OpState, - buf: *const u8, + #[buffer] buf: *const u8, ) -> Result<*mut c_void, AnyError> where FP: FfiPermissions + 'static, @@ -60,11 +59,32 @@ where Ok(buf as *mut c_void) } -#[op(fast)] -fn op_ffi_ptr_offset( +#[op2(fast)] +pub fn op_ffi_ptr_of_exact( + state: &mut OpState, + buf: v8::Local, +) -> Result<*mut c_void, AnyError> +where + FP: FfiPermissions + 'static, +{ + check_unstable(state, "Deno.UnsafePointer#of"); + let permissions = state.borrow_mut::(); + permissions.check_partial(None)?; + + let Some(buf) = buf.get_backing_store() else { + return Ok(0 as _); + }; + let Some(buf) = buf.data() else { + return Ok(0 as _); + }; + Ok(buf.as_ptr() as _) +} + +#[op2(fast)] +pub fn op_ffi_ptr_offset( state: &mut OpState, ptr: *mut c_void, - offset: isize, + #[number] offset: isize, ) -> Result<*mut c_void, AnyError> where FP: FfiPermissions + 'static, @@ -77,8 +97,11 @@ where return Err(type_error("Invalid pointer to offset, pointer is null")); } - // SAFETY: Pointer and offset are user provided. - Ok(unsafe { ptr.offset(offset) }) + // TODO(mmastrac): Create a RawPointer that can safely do pointer math. + + // SAFETY: Using `ptr.offset` is *actually unsafe* and has generated UB, but our FFI code relies on this working so we're going to + // try and ask the compiler to be less undefined here by using `ptr.wrapping_offset`. + Ok(ptr.wrapping_offset(offset)) } unsafe extern "C" fn noop_deleter_callback( @@ -88,11 +111,11 @@ unsafe extern "C" fn noop_deleter_callback( ) { } -#[op(fast)] -fn op_ffi_ptr_value( +#[op2(fast)] +pub fn op_ffi_ptr_value( state: &mut OpState, ptr: *mut c_void, - out: &mut [u32], + #[buffer] out: &mut [u32], ) -> Result<(), AnyError> where FP: FfiPermissions + 'static, @@ -115,14 +138,14 @@ where Ok(()) } -#[op(v8)] +#[op2] pub fn op_ffi_get_buf( scope: &mut v8::HandleScope<'scope>, state: &mut OpState, ptr: *mut c_void, - offset: isize, - len: usize, -) -> Result, AnyError> + #[number] offset: isize, + #[number] len: usize, +) -> Result, AnyError> where FP: FfiPermissions + 'static, { @@ -145,18 +168,17 @@ where ) } .make_shared(); - let array_buffer: v8::Local = - v8::ArrayBuffer::with_backing_store(scope, &backing_store).into(); - Ok(array_buffer.into()) + let array_buffer = v8::ArrayBuffer::with_backing_store(scope, &backing_store); + Ok(array_buffer) } -#[op(fast)] +#[op2(fast)] pub fn op_ffi_buf_copy_into( state: &mut OpState, src: *mut c_void, - offset: isize, - dst: &mut [u8], - len: usize, + #[number] offset: isize, + #[buffer] dst: &mut [u8], + #[number] len: usize, ) -> Result<(), AnyError> where FP: FfiPermissions + 'static, @@ -184,13 +206,13 @@ where } } -#[op(v8)] +#[op2] pub fn op_ffi_cstr_read( scope: &mut v8::HandleScope<'scope>, state: &mut OpState, ptr: *mut c_void, - offset: isize, -) -> Result, AnyError> + #[number] offset: isize, +) -> Result, AnyError> where FP: FfiPermissions + 'static, { @@ -206,20 +228,18 @@ where let cstr = // SAFETY: Pointer and offset are user provided. unsafe { CStr::from_ptr(ptr.offset(offset) as *const c_char) }.to_bytes(); - let value: v8::Local = - v8::String::new_from_utf8(scope, cstr, v8::NewStringType::Normal) - .ok_or_else(|| { - type_error("Invalid CString pointer, string exceeds max length") - })? - .into(); - Ok(value.into()) + let value = v8::String::new_from_utf8(scope, cstr, v8::NewStringType::Normal) + .ok_or_else(|| { + type_error("Invalid CString pointer, string exceeds max length") + })?; + Ok(value) } -#[op(fast)] +#[op2(fast)] pub fn op_ffi_read_bool( state: &mut OpState, ptr: *mut c_void, - offset: isize, + #[number] offset: isize, ) -> Result where FP: FfiPermissions + 'static, @@ -237,11 +257,11 @@ where Ok(unsafe { ptr::read_unaligned::(ptr.offset(offset) as *const bool) }) } -#[op(fast)] +#[op2(fast)] pub fn op_ffi_read_u8( state: &mut OpState, ptr: *mut c_void, - offset: isize, + #[number] offset: isize, ) -> Result where FP: FfiPermissions + 'static, @@ -261,11 +281,11 @@ where }) } -#[op(fast)] +#[op2(fast)] pub fn op_ffi_read_i8( state: &mut OpState, ptr: *mut c_void, - offset: isize, + #[number] offset: isize, ) -> Result where FP: FfiPermissions + 'static, @@ -285,11 +305,11 @@ where }) } -#[op(fast)] +#[op2(fast)] pub fn op_ffi_read_u16( state: &mut OpState, ptr: *mut c_void, - offset: isize, + #[number] offset: isize, ) -> Result where FP: FfiPermissions + 'static, @@ -309,11 +329,11 @@ where }) } -#[op(fast)] +#[op2(fast)] pub fn op_ffi_read_i16( state: &mut OpState, ptr: *mut c_void, - offset: isize, + #[number] offset: isize, ) -> Result where FP: FfiPermissions + 'static, @@ -333,11 +353,11 @@ where }) } -#[op(fast)] +#[op2(fast)] pub fn op_ffi_read_u32( state: &mut OpState, ptr: *mut c_void, - offset: isize, + #[number] offset: isize, ) -> Result where FP: FfiPermissions + 'static, @@ -355,11 +375,11 @@ where Ok(unsafe { ptr::read_unaligned::(ptr.offset(offset) as *const u32) }) } -#[op(fast)] +#[op2(fast)] pub fn op_ffi_read_i32( state: &mut OpState, ptr: *mut c_void, - offset: isize, + #[number] offset: isize, ) -> Result where FP: FfiPermissions + 'static, @@ -377,12 +397,12 @@ where Ok(unsafe { ptr::read_unaligned::(ptr.offset(offset) as *const i32) }) } -#[op] +#[op2(fast)] pub fn op_ffi_read_u64( state: &mut OpState, ptr: *mut c_void, - offset: isize, - out: &mut [u32], + #[number] offset: isize, + #[buffer] out: &mut [u32], ) -> Result<(), AnyError> where FP: FfiPermissions + 'static, @@ -412,12 +432,12 @@ where Ok(()) } -#[op(fast)] +#[op2(fast)] pub fn op_ffi_read_i64( state: &mut OpState, ptr: *mut c_void, - offset: isize, - out: &mut [u32], + #[number] offset: isize, + #[buffer] out: &mut [u32], ) -> Result<(), AnyError> where FP: FfiPermissions + 'static, @@ -446,11 +466,11 @@ where Ok(()) } -#[op(fast)] +#[op2(fast)] pub fn op_ffi_read_f32( state: &mut OpState, ptr: *mut c_void, - offset: isize, + #[number] offset: isize, ) -> Result where FP: FfiPermissions + 'static, @@ -468,11 +488,11 @@ where Ok(unsafe { ptr::read_unaligned::(ptr.offset(offset) as *const f32) }) } -#[op(fast)] +#[op2(fast)] pub fn op_ffi_read_f64( state: &mut OpState, ptr: *mut c_void, - offset: isize, + #[number] offset: isize, ) -> Result where FP: FfiPermissions + 'static, @@ -490,11 +510,11 @@ where Ok(unsafe { ptr::read_unaligned::(ptr.offset(offset) as *const f64) }) } -#[op(fast)] +#[op2(fast)] pub fn op_ffi_read_ptr( state: &mut OpState, ptr: *mut c_void, - offset: isize, + #[number] offset: isize, ) -> Result<*mut c_void, AnyError> where FP: FfiPermissions + 'static, diff --git a/test_ffi/tests/integration_tests.rs b/test_ffi/tests/integration_tests.rs index a4f233e45d..ad00465e69 100644 --- a/test_ffi/tests/integration_tests.rs +++ b/test_ffi/tests/integration_tests.rs @@ -1,5 +1,6 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. +use pretty_assertions::assert_eq; use std::process::Command; use test_util::deno_cmd; @@ -54,11 +55,11 @@ fn basic() { [ 4, 5, 6 ]\n\ Hello from pointer!\n\ pointer!\n\ - false\n\ - true\n\ - false\n\ - true\n\ - false\n\ + false false\n\ + true true\n\ + false false\n\ + true true\n\ + false false\n\ 579\n\ true\n\ 579\n\ diff --git a/test_ffi/tests/test.js b/test_ffi/tests/test.js index f4fc96a1bf..80c5663980 100644 --- a/test_ffi/tests/test.js +++ b/test_ffi/tests/test.js @@ -341,13 +341,13 @@ const stringPtr = Deno.UnsafePointer.of(string); const stringPtrview = new Deno.UnsafePointerView(stringPtr); console.log(stringPtrview.getCString()); console.log(stringPtrview.getCString(11)); -console.log(dylib.symbols.is_null_ptr(ptr0)); -console.log(dylib.symbols.is_null_ptr(null)); -console.log(dylib.symbols.is_null_ptr(Deno.UnsafePointer.of(into))); +console.log("false", dylib.symbols.is_null_ptr(ptr0)); +console.log("true", dylib.symbols.is_null_ptr(null)); +console.log("false", dylib.symbols.is_null_ptr(Deno.UnsafePointer.of(into))); const emptyBuffer = new Uint8Array(0); -console.log(dylib.symbols.is_null_ptr(Deno.UnsafePointer.of(emptyBuffer))); +console.log("true", dylib.symbols.is_null_ptr(Deno.UnsafePointer.of(emptyBuffer))); const emptySlice = into.subarray(6); -console.log(dylib.symbols.is_null_ptr(Deno.UnsafePointer.of(emptySlice))); +console.log("false", dylib.symbols.is_null_ptr(Deno.UnsafePointer.of(emptySlice))); const { is_null_buf } = symbols; function isNullBuffer(buffer) { return is_null_buf(buffer); }; @@ -386,10 +386,9 @@ const externalOneBuffer = new Uint8Array(Deno.UnsafePointerView.getArrayBuffer(p assertEquals(isNullBuffer(externalOneBuffer), false, "isNullBuffer(externalOneBuffer) !== false"); assertEquals(isNullBufferDeopt(externalOneBuffer), false, "isNullBufferDeopt(externalOneBuffer) !== false"); -// Due to ops macro using `Local->Data()` to get the pointer for the slice that is then used to get -// the pointer of an ArrayBuffer / TypedArray, the same effect can be seen where a zero byte length buffer returns -// a null pointer as its pointer value. -assertEquals(Deno.UnsafePointer.of(externalZeroBuffer), null, "Deno.UnsafePointer.of(externalZeroBuffer) !== null"); +// UnsafePointer.of uses an exact-pointer fallback for zero-length buffers and slices to ensure that it always gets +// the underlying pointer right. +assertNotEquals(Deno.UnsafePointer.of(externalZeroBuffer), null, "Deno.UnsafePointer.of(externalZeroBuffer) === null"); assertNotEquals(Deno.UnsafePointer.of(externalOneBuffer), null, "Deno.UnsafePointer.of(externalOneBuffer) === null"); const addU32Ptr = dylib.symbols.get_add_u32_ptr(); @@ -726,7 +725,9 @@ assertEquals(view.getUint32(), 55); assertThrows(() => Deno.UnsafePointer.offset(null, 5)); const offsetPointer = Deno.UnsafePointer.offset(createdPointer, 5); assertEquals(Deno.UnsafePointer.value(offsetPointer), 6); - assertEquals(Deno.UnsafePointer.offset(offsetPointer, -6), null); + const zeroPointer = Deno.UnsafePointer.offset(offsetPointer, -6); + assertEquals(Deno.UnsafePointer.value(zeroPointer), 0); + assertEquals(zeroPointer, null); } // Test non-UTF-8 characters