From 1619932a651a189d590bd579f334faac3c6b4397 Mon Sep 17 00:00:00 2001 From: Matt Mastracci Date: Thu, 5 Oct 2023 07:35:21 -0600 Subject: chore(ext/ffi): migrate from op -> op2 for ffi (#20509) Migrate to op2. Making a few decisions to get this across the line: - Empty slices, no matter where the come from, are null pointers. The v8 bugs (https://bugs.chromium.org/p/v8/issues/detail?id=13489) and (https://bugs.chromium.org/p/v8/issues/detail?id=13488) make passing around zero-length slice pointers too dangerous as they might be uninitialized or null data. - Offsets and lengths are `#[number] isize` and `#[number] usize` respectively -- 53 bits should be enough for anyone - Pointers are bigints. This is a u64 in the fastcall world, and can accept Integer/Int32/Number/BigInt v8 types in the slow world. --- ext/ffi/00_ffi.js | 20 +++++++- ext/ffi/lib.rs | 1 + ext/ffi/repr.rs | 146 +++++++++++++++++++++++++++++++----------------------- 3 files changed, 103 insertions(+), 64 deletions(-) (limited to 'ext/ffi') diff --git a/ext/ffi/00_ffi.js b/ext/ffi/00_ffi.js index 67cb13ab6..fe7344e17 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 1ca921119..a3cd0ebda 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 665e37186..f3bf01310 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, -- cgit v1.2.3