From ee7d4501435f0ebd655c8b50bd6e41ca19e71abc Mon Sep 17 00:00:00 2001 From: Leo Kettmeir Date: Mon, 14 Oct 2024 15:05:49 -0700 Subject: [PATCH] refactor(ext/ffi): use concrete error types (#26170) --- Cargo.lock | 2 + ext/ffi/Cargo.toml | 2 + ext/ffi/call.rs | 54 ++++++++---- ext/ffi/callback.rs | 51 +++++++---- ext/ffi/dlfcn.rs | 48 +++++++--- ext/ffi/ir.rs | 142 ++++++++++++++++------------- ext/ffi/lib.rs | 7 ++ ext/ffi/repr.rs | 211 ++++++++++++++++++++++++++++++-------------- ext/ffi/static.rs | 31 +++++-- runtime/errors.rs | 80 +++++++++++++++++ 10 files changed, 444 insertions(+), 184 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 504182136c..56121705b2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1587,6 +1587,8 @@ dependencies = [ "serde", "serde-value", "serde_json", + "thiserror", + "tokio", "winapi", ] diff --git a/ext/ffi/Cargo.toml b/ext/ffi/Cargo.toml index 23a2aee1cd..496e8634ec 100644 --- a/ext/ffi/Cargo.toml +++ b/ext/ffi/Cargo.toml @@ -24,6 +24,8 @@ log.workspace = true serde.workspace = true serde-value = "0.7" serde_json = "1.0" +thiserror.workspace = true +tokio.workspace = true [target.'cfg(windows)'.dependencies] winapi = { workspace = true, features = ["errhandlingapi", "minwindef", "ntdef", "winbase", "winnt"] } diff --git a/ext/ffi/call.rs b/ext/ffi/call.rs index 3572b9e813..d337b29b00 100644 --- a/ext/ffi/call.rs +++ b/ext/ffi/call.rs @@ -7,9 +7,6 @@ use crate::symbol::NativeType; use crate::symbol::Symbol; use crate::FfiPermissions; use crate::ForeignFunction; -use deno_core::anyhow::anyhow; -use deno_core::error::type_error; -use deno_core::error::AnyError; use deno_core::op2; use deno_core::serde_json::Value; use deno_core::serde_v8::ExternalPointer; @@ -24,6 +21,20 @@ use std::ffi::c_void; use std::future::Future; use std::rc::Rc; +#[derive(Debug, thiserror::Error)] +pub enum CallError { + #[error(transparent)] + IR(#[from] IRError), + #[error("Nonblocking FFI call failed: {0}")] + NonblockingCallFailure(#[source] tokio::task::JoinError), + #[error("Invalid FFI symbol name: '{0}'")] + InvalidSymbol(String), + #[error(transparent)] + Permission(deno_core::error::AnyError), + #[error(transparent)] + Callback(#[from] super::CallbackError), +} + // SAFETY: Makes an FFI call unsafe fn ffi_call_rtype_struct( cif: &libffi::middle::Cif, @@ -45,7 +56,7 @@ pub(crate) fn ffi_call_sync<'scope>( args: v8::FunctionCallbackArguments, symbol: &Symbol, out_buffer: Option, -) -> Result +) -> Result where 'scope: 'scope, { @@ -201,7 +212,7 @@ fn ffi_call( parameter_types: &[NativeType], result_type: NativeType, out_buffer: Option, -) -> Result { +) -> FfiValue { let call_args: Vec = call_args .iter() .enumerate() @@ -214,7 +225,7 @@ fn ffi_call( // SAFETY: types in the `Cif` match the actual calling convention and // types of symbol. unsafe { - Ok(match result_type { + match result_type { NativeType::Void => { cif.call::<()>(fun_ptr, &call_args); FfiValue::Value(Value::from(())) @@ -267,7 +278,7 @@ fn ffi_call( ffi_call_rtype_struct(cif, &fun_ptr, call_args, out_buffer.unwrap().0); FfiValue::Value(Value::Null) } - }) + } } } @@ -280,14 +291,16 @@ pub fn op_ffi_call_ptr_nonblocking( #[serde] def: ForeignFunction, parameters: v8::Local, out_buffer: Option>, -) -> Result>, AnyError> +) -> Result>, CallError> where FP: FfiPermissions + 'static, { { let mut state = state.borrow_mut(); let permissions = state.borrow_mut::(); - permissions.check_partial_no_path()?; + permissions + .check_partial_no_path() + .map_err(CallError::Permission)?; }; let symbol = PtrSymbol::new(pointer, &def)?; @@ -309,7 +322,7 @@ where Ok(async move { let result = join_handle .await - .map_err(|err| anyhow!("Nonblocking FFI call failed: {}", err))??; + .map_err(CallError::NonblockingCallFailure)?; // SAFETY: Same return type declared to libffi; trust user to have it right beyond that. Ok(result) }) @@ -325,16 +338,17 @@ pub fn op_ffi_call_nonblocking( #[string] symbol: String, parameters: v8::Local, out_buffer: Option>, -) -> Result>, AnyError> { +) -> Result>, CallError> { let symbol = { let state = state.borrow(); - let resource = state.resource_table.get::(rid)?; + let resource = state + .resource_table + .get::(rid) + .map_err(CallError::Permission)?; let symbols = &resource.symbols; *symbols .get(&symbol) - .ok_or_else(|| { - type_error(format!("Invalid FFI symbol name: '{symbol}'")) - })? + .ok_or_else(|| CallError::InvalidSymbol(symbol))? .clone() }; @@ -362,7 +376,7 @@ pub fn op_ffi_call_nonblocking( Ok(async move { let result = join_handle .await - .map_err(|err| anyhow!("Nonblocking FFI call failed: {}", err))??; + .map_err(CallError::NonblockingCallFailure)?; // SAFETY: Same return type declared to libffi; trust user to have it right beyond that. Ok(result) }) @@ -377,14 +391,16 @@ pub fn op_ffi_call_ptr( #[serde] def: ForeignFunction, parameters: v8::Local, out_buffer: Option>, -) -> Result +) -> Result where FP: FfiPermissions + 'static, { { let mut state = state.borrow_mut(); let permissions = state.borrow_mut::(); - permissions.check_partial_no_path()?; + permissions + .check_partial_no_path() + .map_err(CallError::Permission)?; }; let symbol = PtrSymbol::new(pointer, &def)?; @@ -399,7 +415,7 @@ where &def.parameters, def.result.clone(), out_buffer_ptr, - )?; + ); // SAFETY: Same return type declared to libffi; trust user to have it right beyond that. Ok(result) } diff --git a/ext/ffi/callback.rs b/ext/ffi/callback.rs index 6fa166f52b..f33e0413a3 100644 --- a/ext/ffi/callback.rs +++ b/ext/ffi/callback.rs @@ -3,7 +3,6 @@ use crate::symbol::NativeType; use crate::FfiPermissions; use crate::ForeignFunction; -use deno_core::error::AnyError; use deno_core::op2; use deno_core::v8; use deno_core::v8::TryCatch; @@ -34,6 +33,16 @@ thread_local! { static LOCAL_THREAD_ID: RefCell = const { RefCell::new(0) }; } +#[derive(Debug, thiserror::Error)] +pub enum CallbackError { + #[error(transparent)] + Resource(deno_core::error::AnyError), + #[error(transparent)] + Permission(deno_core::error::AnyError), + #[error(transparent)] + Other(deno_core::error::AnyError), +} + #[derive(Clone)] pub struct PtrSymbol { pub cif: libffi::middle::Cif, @@ -44,7 +53,7 @@ impl PtrSymbol { pub fn new( fn_ptr: *mut c_void, def: &ForeignFunction, - ) -> Result { + ) -> Result { let ptr = libffi::middle::CodePtr::from_ptr(fn_ptr as _); let cif = libffi::middle::Cif::new( def @@ -52,8 +61,13 @@ impl PtrSymbol { .clone() .into_iter() .map(libffi::middle::Type::try_from) - .collect::, _>>()?, - def.result.clone().try_into()?, + .collect::, _>>() + .map_err(CallbackError::Other)?, + def + .result + .clone() + .try_into() + .map_err(CallbackError::Other)?, ); Ok(Self { cif, ptr }) @@ -522,10 +536,12 @@ unsafe fn do_ffi_callback( pub fn op_ffi_unsafe_callback_ref( state: Rc>, #[smi] rid: ResourceId, -) -> Result>, AnyError> { +) -> Result, CallbackError> { let state = state.borrow(); - let callback_resource = - state.resource_table.get::(rid)?; + let callback_resource = state + .resource_table + .get::(rid) + .map_err(CallbackError::Resource)?; Ok(async move { let info: &mut CallbackInfo = @@ -536,7 +552,6 @@ pub fn op_ffi_unsafe_callback_ref( .into_future() .or_cancel(callback_resource.cancel.clone()) .await; - Ok(()) }) } @@ -552,12 +567,14 @@ pub fn op_ffi_unsafe_callback_create( scope: &mut v8::HandleScope<'scope>, #[serde] args: RegisterCallbackArgs, cb: v8::Local, -) -> Result, AnyError> +) -> Result, CallbackError> where FP: FfiPermissions + 'static, { let permissions = state.borrow_mut::(); - permissions.check_partial_no_path()?; + permissions + .check_partial_no_path() + .map_err(CallbackError::Permission)?; let thread_id: u32 = LOCAL_THREAD_ID.with(|s| { let value = *s.borrow(); @@ -593,8 +610,10 @@ where .parameters .into_iter() .map(libffi::middle::Type::try_from) - .collect::, _>>()?, - libffi::middle::Type::try_from(args.result)?, + .collect::, _>>() + .map_err(CallbackError::Other)?, + libffi::middle::Type::try_from(args.result) + .map_err(CallbackError::Other)?, ); // SAFETY: CallbackInfo is leaked, is not null and stays valid as long as the callback exists. @@ -624,14 +643,16 @@ pub fn op_ffi_unsafe_callback_close( state: &mut OpState, scope: &mut v8::HandleScope, #[smi] rid: ResourceId, -) -> Result<(), AnyError> { +) -> Result<(), CallbackError> { // SAFETY: This drops the closure and the callback info associated with it. // Any retained function pointers to the closure become dangling pointers. // It is up to the user to know that it is safe to call the `close()` on the // UnsafeCallback instance. unsafe { - let callback_resource = - state.resource_table.take::(rid)?; + let callback_resource = state + .resource_table + .take::(rid) + .map_err(CallbackError::Resource)?; let info = Box::from_raw(callback_resource.info); let _ = v8::Global::from_raw(scope, info.callback); let _ = v8::Global::from_raw(scope, info.context); diff --git a/ext/ffi/dlfcn.rs b/ext/ffi/dlfcn.rs index 10199bf858..53bdcbc5cc 100644 --- a/ext/ffi/dlfcn.rs +++ b/ext/ffi/dlfcn.rs @@ -6,8 +6,6 @@ use crate::symbol::Symbol; use crate::turbocall; use crate::turbocall::Turbocall; use crate::FfiPermissions; -use deno_core::error::generic_error; -use deno_core::error::AnyError; use deno_core::op2; use deno_core::v8; use deno_core::GarbageCollected; @@ -21,6 +19,22 @@ use std::collections::HashMap; use std::ffi::c_void; use std::rc::Rc; +#[derive(Debug, thiserror::Error)] +pub enum DlfcnError { + #[error("Failed to register symbol {symbol}: {error}")] + RegisterSymbol { + symbol: String, + #[source] + error: dlopen2::Error, + }, + #[error(transparent)] + Dlopen(#[from] dlopen2::Error), + #[error(transparent)] + Permission(deno_core::error::AnyError), + #[error(transparent)] + Other(deno_core::error::AnyError), +} + pub struct DynamicLibraryResource { lib: Library, pub symbols: HashMap>, @@ -37,7 +51,7 @@ impl Resource for DynamicLibraryResource { } impl DynamicLibraryResource { - pub fn get_static(&self, symbol: String) -> Result<*mut c_void, AnyError> { + pub fn get_static(&self, symbol: String) -> Result<*mut c_void, DlfcnError> { // By default, Err returned by this function does not tell // which symbol wasn't exported. So we'll modify the error // message to include the name of symbol. @@ -45,9 +59,7 @@ impl DynamicLibraryResource { // SAFETY: The obtained T symbol is the size of a pointer. match unsafe { self.lib.symbol::<*mut c_void>(&symbol) } { Ok(value) => Ok(Ok(value)), - Err(err) => Err(generic_error(format!( - "Failed to register symbol {symbol}: {err}" - ))), + Err(error) => Err(DlfcnError::RegisterSymbol { symbol, error }), }? } } @@ -116,12 +128,14 @@ pub fn op_ffi_load<'scope, FP>( scope: &mut v8::HandleScope<'scope>, state: &mut OpState, #[serde] args: FfiLoadArgs, -) -> Result, AnyError> +) -> Result, DlfcnError> where FP: FfiPermissions + 'static, { let permissions = state.borrow_mut::(); - let path = permissions.check_partial_with_path(&args.path)?; + let path = permissions + .check_partial_with_path(&args.path) + .map_err(DlfcnError::Permission)?; let lib = Library::open(&path).map_err(|e| { dlopen2::Error::OpeningLibraryError(std::io::Error::new( @@ -152,15 +166,16 @@ where // SAFETY: The obtained T symbol is the size of a pointer. match unsafe { resource.lib.symbol::<*const c_void>(symbol) } { Ok(value) => Ok(value), - Err(err) => if foreign_fn.optional { + Err(error) => if foreign_fn.optional { let null: v8::Local = v8::null(scope).into(); let func_key = v8::String::new(scope, &symbol_key).unwrap(); obj.set(scope, func_key.into(), null); break 'register_symbol; } else { - Err(generic_error(format!( - "Failed to register symbol {symbol}: {err}" - ))) + Err(DlfcnError::RegisterSymbol { + symbol: symbol.to_owned(), + error, + }) }, }?; @@ -171,8 +186,13 @@ where .clone() .into_iter() .map(libffi::middle::Type::try_from) - .collect::, _>>()?, - foreign_fn.result.clone().try_into()?, + .collect::, _>>() + .map_err(DlfcnError::Other)?, + foreign_fn + .result + .clone() + .try_into() + .map_err(DlfcnError::Other)?, ); let func_key = v8::String::new(scope, &symbol_key).unwrap(); diff --git a/ext/ffi/ir.rs b/ext/ffi/ir.rs index ebf64945b4..2e80842166 100644 --- a/ext/ffi/ir.rs +++ b/ext/ffi/ir.rs @@ -1,13 +1,55 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. use crate::symbol::NativeType; -use deno_core::error::type_error; -use deno_core::error::AnyError; use deno_core::v8; use libffi::middle::Arg; use std::ffi::c_void; use std::ptr; +#[derive(Debug, thiserror::Error)] +pub enum IRError { + #[error("Invalid FFI u8 type, expected boolean")] + InvalidU8ExpectedBoolean, + #[error("Invalid FFI u8 type, expected unsigned integer")] + InvalidU8ExpectedUnsignedInteger, + #[error("Invalid FFI i8 type, expected integer")] + InvalidI8, + #[error("Invalid FFI u16 type, expected unsigned integer")] + InvalidU16, + #[error("Invalid FFI i16 type, expected integer")] + InvalidI16, + #[error("Invalid FFI u32 type, expected unsigned integer")] + InvalidU32, + #[error("Invalid FFI i32 type, expected integer")] + InvalidI32, + #[error("Invalid FFI u64 type, expected unsigned integer")] + InvalidU64, + #[error("Invalid FFI i64 type, expected integer")] + InvalidI64, + #[error("Invalid FFI usize type, expected unsigned integer")] + InvalidUsize, + #[error("Invalid FFI isize type, expected integer")] + InvalidIsize, + #[error("Invalid FFI f32 type, expected number")] + InvalidF32, + #[error("Invalid FFI f64 type, expected number")] + InvalidF64, + #[error("Invalid FFI pointer type, expected null, or External")] + InvalidPointerType, + #[error( + "Invalid FFI buffer type, expected null, ArrayBuffer, or ArrayBufferView" + )] + InvalidBufferType, + #[error("Invalid FFI ArrayBufferView, expected data in the buffer")] + InvalidArrayBufferView, + #[error("Invalid FFI ArrayBuffer, expected data in buffer")] + InvalidArrayBuffer, + #[error("Invalid FFI struct type, expected ArrayBuffer, or ArrayBufferView")] + InvalidStructType, + #[error("Invalid FFI function type, expected null, or External")] + InvalidFunctionType, +} + pub struct OutBuffer(pub *mut u8); // SAFETY: OutBuffer is allocated by us in 00_ffi.js and is guaranteed to be @@ -126,9 +168,9 @@ unsafe impl Send for NativeValue {} #[inline] pub fn ffi_parse_bool_arg( arg: v8::Local, -) -> Result { +) -> Result { let bool_value = v8::Local::::try_from(arg) - .map_err(|_| type_error("Invalid FFI u8 type, expected boolean"))? + .map_err(|_| IRError::InvalidU8ExpectedBoolean)? .is_true(); Ok(NativeValue { bool_value }) } @@ -136,9 +178,9 @@ pub fn ffi_parse_bool_arg( #[inline] pub fn ffi_parse_u8_arg( arg: v8::Local, -) -> Result { +) -> Result { let u8_value = v8::Local::::try_from(arg) - .map_err(|_| type_error("Invalid FFI u8 type, expected unsigned integer"))? + .map_err(|_| IRError::InvalidU8ExpectedUnsignedInteger)? .value() as u8; Ok(NativeValue { u8_value }) } @@ -146,9 +188,9 @@ pub fn ffi_parse_u8_arg( #[inline] pub fn ffi_parse_i8_arg( arg: v8::Local, -) -> Result { +) -> Result { let i8_value = v8::Local::::try_from(arg) - .map_err(|_| type_error("Invalid FFI i8 type, expected integer"))? + .map_err(|_| IRError::InvalidI8)? .value() as i8; Ok(NativeValue { i8_value }) } @@ -156,9 +198,9 @@ pub fn ffi_parse_i8_arg( #[inline] pub fn ffi_parse_u16_arg( arg: v8::Local, -) -> Result { +) -> Result { let u16_value = v8::Local::::try_from(arg) - .map_err(|_| type_error("Invalid FFI u16 type, expected unsigned integer"))? + .map_err(|_| IRError::InvalidU16)? .value() as u16; Ok(NativeValue { u16_value }) } @@ -166,9 +208,9 @@ pub fn ffi_parse_u16_arg( #[inline] pub fn ffi_parse_i16_arg( arg: v8::Local, -) -> Result { +) -> Result { let i16_value = v8::Local::::try_from(arg) - .map_err(|_| type_error("Invalid FFI i16 type, expected integer"))? + .map_err(|_| IRError::InvalidI16)? .value() as i16; Ok(NativeValue { i16_value }) } @@ -176,9 +218,9 @@ pub fn ffi_parse_i16_arg( #[inline] pub fn ffi_parse_u32_arg( arg: v8::Local, -) -> Result { +) -> Result { let u32_value = v8::Local::::try_from(arg) - .map_err(|_| type_error("Invalid FFI u32 type, expected unsigned integer"))? + .map_err(|_| IRError::InvalidU32)? .value(); Ok(NativeValue { u32_value }) } @@ -186,9 +228,9 @@ pub fn ffi_parse_u32_arg( #[inline] pub fn ffi_parse_i32_arg( arg: v8::Local, -) -> Result { +) -> Result { let i32_value = v8::Local::::try_from(arg) - .map_err(|_| type_error("Invalid FFI i32 type, expected integer"))? + .map_err(|_| IRError::InvalidI32)? .value(); Ok(NativeValue { i32_value }) } @@ -197,7 +239,7 @@ pub fn ffi_parse_i32_arg( pub fn ffi_parse_u64_arg( scope: &mut v8::HandleScope, arg: v8::Local, -) -> Result { +) -> Result { // Order of checking: // 1. BigInt: Uncommon and not supported by Fast API, so optimise slow call for this case. // 2. Number: Common, supported by Fast API, so let that be the optimal case. @@ -207,9 +249,7 @@ pub fn ffi_parse_u64_arg( } else if let Ok(value) = v8::Local::::try_from(arg) { value.integer_value(scope).unwrap() as u64 } else { - return Err(type_error( - "Invalid FFI u64 type, expected unsigned integer", - )); + return Err(IRError::InvalidU64); }; Ok(NativeValue { u64_value }) } @@ -218,7 +258,7 @@ pub fn ffi_parse_u64_arg( pub fn ffi_parse_i64_arg( scope: &mut v8::HandleScope, arg: v8::Local, -) -> Result { +) -> Result { // Order of checking: // 1. BigInt: Uncommon and not supported by Fast API, so optimise slow call for this case. // 2. Number: Common, supported by Fast API, so let that be the optimal case. @@ -228,7 +268,7 @@ pub fn ffi_parse_i64_arg( } else if let Ok(value) = v8::Local::::try_from(arg) { value.integer_value(scope).unwrap() } else { - return Err(type_error("Invalid FFI i64 type, expected integer")); + return Err(IRError::InvalidI64); }; Ok(NativeValue { i64_value }) } @@ -237,7 +277,7 @@ pub fn ffi_parse_i64_arg( pub fn ffi_parse_usize_arg( scope: &mut v8::HandleScope, arg: v8::Local, -) -> Result { +) -> Result { // Order of checking: // 1. BigInt: Uncommon and not supported by Fast API, so optimise slow call for this case. // 2. Number: Common, supported by Fast API, so let that be the optimal case. @@ -247,7 +287,7 @@ pub fn ffi_parse_usize_arg( } else if let Ok(value) = v8::Local::::try_from(arg) { value.integer_value(scope).unwrap() as usize } else { - return Err(type_error("Invalid FFI usize type, expected integer")); + return Err(IRError::InvalidUsize); }; Ok(NativeValue { usize_value }) } @@ -256,7 +296,7 @@ pub fn ffi_parse_usize_arg( pub fn ffi_parse_isize_arg( scope: &mut v8::HandleScope, arg: v8::Local, -) -> Result { +) -> Result { // Order of checking: // 1. BigInt: Uncommon and not supported by Fast API, so optimise slow call for this case. // 2. Number: Common, supported by Fast API, so let that be the optimal case. @@ -266,7 +306,7 @@ pub fn ffi_parse_isize_arg( } else if let Ok(value) = v8::Local::::try_from(arg) { value.integer_value(scope).unwrap() as isize } else { - return Err(type_error("Invalid FFI isize type, expected integer")); + return Err(IRError::InvalidIsize); }; Ok(NativeValue { isize_value }) } @@ -274,9 +314,9 @@ pub fn ffi_parse_isize_arg( #[inline] pub fn ffi_parse_f32_arg( arg: v8::Local, -) -> Result { +) -> Result { let f32_value = v8::Local::::try_from(arg) - .map_err(|_| type_error("Invalid FFI f32 type, expected number"))? + .map_err(|_| IRError::InvalidF32)? .value() as f32; Ok(NativeValue { f32_value }) } @@ -284,9 +324,9 @@ pub fn ffi_parse_f32_arg( #[inline] pub fn ffi_parse_f64_arg( arg: v8::Local, -) -> Result { +) -> Result { let f64_value = v8::Local::::try_from(arg) - .map_err(|_| type_error("Invalid FFI f64 type, expected number"))? + .map_err(|_| IRError::InvalidF64)? .value(); Ok(NativeValue { f64_value }) } @@ -295,15 +335,13 @@ pub fn ffi_parse_f64_arg( pub fn ffi_parse_pointer_arg( _scope: &mut v8::HandleScope, arg: v8::Local, -) -> Result { +) -> Result { let pointer = if let Ok(value) = v8::Local::::try_from(arg) { value.value() } else if arg.is_null() { ptr::null_mut() } else { - return Err(type_error( - "Invalid FFI pointer type, expected null, or External", - )); + return Err(IRError::InvalidPointerType); }; Ok(NativeValue { pointer }) } @@ -312,7 +350,7 @@ pub fn ffi_parse_pointer_arg( pub fn ffi_parse_buffer_arg( scope: &mut v8::HandleScope, arg: v8::Local, -) -> Result { +) -> Result { // Order of checking: // 1. ArrayBuffer: Fairly common and not supported by Fast API, optimise this case. // 2. ArrayBufferView: Common and supported by Fast API @@ -328,9 +366,7 @@ pub fn ffi_parse_buffer_arg( let byte_offset = value.byte_offset(); let pointer = value .buffer(scope) - .ok_or_else(|| { - type_error("Invalid FFI ArrayBufferView, expected data in the buffer") - })? + .ok_or(IRError::InvalidArrayBufferView)? .data(); if let Some(non_null) = pointer { // SAFETY: Pointer is non-null, and V8 guarantees that the byte_offset @@ -342,9 +378,7 @@ pub fn ffi_parse_buffer_arg( } else if arg.is_null() { ptr::null_mut() } else { - return Err(type_error( - "Invalid FFI buffer type, expected null, ArrayBuffer, or ArrayBufferView", - )); + return Err(IRError::InvalidBufferType); }; Ok(NativeValue { pointer }) } @@ -353,7 +387,7 @@ pub fn ffi_parse_buffer_arg( pub fn ffi_parse_struct_arg( scope: &mut v8::HandleScope, arg: v8::Local, -) -> Result { +) -> Result { // Order of checking: // 1. ArrayBuffer: Fairly common and not supported by Fast API, optimise this case. // 2. ArrayBufferView: Common and supported by Fast API @@ -362,31 +396,23 @@ pub fn ffi_parse_struct_arg( if let Some(non_null) = value.data() { non_null.as_ptr() } else { - return Err(type_error( - "Invalid FFI ArrayBuffer, expected data in buffer", - )); + return Err(IRError::InvalidArrayBuffer); } } else if let Ok(value) = v8::Local::::try_from(arg) { let byte_offset = value.byte_offset(); let pointer = value .buffer(scope) - .ok_or_else(|| { - type_error("Invalid FFI ArrayBufferView, expected data in the buffer") - })? + .ok_or(IRError::InvalidArrayBufferView)? .data(); if let Some(non_null) = pointer { // SAFETY: Pointer is non-null, and V8 guarantees that the byte_offset // is within the buffer backing store. unsafe { non_null.as_ptr().add(byte_offset) } } else { - return Err(type_error( - "Invalid FFI ArrayBufferView, expected data in buffer", - )); + return Err(IRError::InvalidArrayBufferView); } } else { - return Err(type_error( - "Invalid FFI struct type, expected ArrayBuffer, or ArrayBufferView", - )); + return Err(IRError::InvalidStructType); }; Ok(NativeValue { pointer }) } @@ -395,15 +421,13 @@ pub fn ffi_parse_struct_arg( pub fn ffi_parse_function_arg( _scope: &mut v8::HandleScope, arg: v8::Local, -) -> Result { +) -> Result { let pointer = if let Ok(value) = v8::Local::::try_from(arg) { value.value() } else if arg.is_null() { ptr::null_mut() } else { - return Err(type_error( - "Invalid FFI function type, expected null, or External", - )); + return Err(IRError::InvalidFunctionType); }; Ok(NativeValue { pointer }) } @@ -412,7 +436,7 @@ pub fn ffi_parse_args<'scope>( scope: &mut v8::HandleScope<'scope>, args: v8::Local, parameter_types: &[NativeType], -) -> Result, AnyError> +) -> Result, IRError> where 'scope: 'scope, { diff --git a/ext/ffi/lib.rs b/ext/ffi/lib.rs index 77ec3c85e3..237f8c3b05 100644 --- a/ext/ffi/lib.rs +++ b/ext/ffi/lib.rs @@ -29,6 +29,13 @@ use repr::*; use symbol::NativeType; use symbol::Symbol; +pub use call::CallError; +pub use callback::CallbackError; +pub use dlfcn::DlfcnError; +pub use ir::IRError; +pub use r#static::StaticError; +pub use repr::ReprError; + #[cfg(not(target_pointer_width = "64"))] compile_error!("platform not supported"); diff --git a/ext/ffi/repr.rs b/ext/ffi/repr.rs index 315e6d53bc..2f04f4feb4 100644 --- a/ext/ffi/repr.rs +++ b/ext/ffi/repr.rs @@ -1,9 +1,6 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. use crate::FfiPermissions; -use deno_core::error::range_error; -use deno_core::error::type_error; -use deno_core::error::AnyError; use deno_core::op2; use deno_core::v8; use deno_core::OpState; @@ -12,16 +9,58 @@ use std::ffi::c_void; use std::ffi::CStr; use std::ptr; +#[derive(Debug, thiserror::Error)] +pub enum ReprError { + #[error("Invalid pointer to offset, pointer is null")] + InvalidOffset, + #[error("Invalid ArrayBuffer pointer, pointer is null")] + InvalidArrayBuffer, + #[error("Destination length is smaller than source length")] + DestinationLengthTooShort, + #[error("Invalid CString pointer, pointer is null")] + InvalidCString, + #[error("Invalid CString pointer, string exceeds max length")] + CStringTooLong, + #[error("Invalid bool pointer, pointer is null")] + InvalidBool, + #[error("Invalid u8 pointer, pointer is null")] + InvalidU8, + #[error("Invalid i8 pointer, pointer is null")] + InvalidI8, + #[error("Invalid u16 pointer, pointer is null")] + InvalidU16, + #[error("Invalid i16 pointer, pointer is null")] + InvalidI16, + #[error("Invalid u32 pointer, pointer is null")] + InvalidU32, + #[error("Invalid i32 pointer, pointer is null")] + InvalidI32, + #[error("Invalid u64 pointer, pointer is null")] + InvalidU64, + #[error("Invalid i64 pointer, pointer is null")] + InvalidI64, + #[error("Invalid f32 pointer, pointer is null")] + InvalidF32, + #[error("Invalid f64 pointer, pointer is null")] + InvalidF64, + #[error("Invalid pointer pointer, pointer is null")] + InvalidPointer, + #[error(transparent)] + Permission(deno_core::error::AnyError), +} + #[op2(fast)] pub fn op_ffi_ptr_create( state: &mut OpState, #[bigint] ptr_number: usize, -) -> Result<*mut c_void, AnyError> +) -> Result<*mut c_void, ReprError> where FP: FfiPermissions + 'static, { let permissions = state.borrow_mut::(); - permissions.check_partial_no_path()?; + permissions + .check_partial_no_path() + .map_err(ReprError::Permission)?; Ok(ptr_number as *mut c_void) } @@ -31,12 +70,14 @@ pub fn op_ffi_ptr_equals( state: &mut OpState, a: *const c_void, b: *const c_void, -) -> Result +) -> Result where FP: FfiPermissions + 'static, { let permissions = state.borrow_mut::(); - permissions.check_partial_no_path()?; + permissions + .check_partial_no_path() + .map_err(ReprError::Permission)?; Ok(a == b) } @@ -45,12 +86,14 @@ where pub fn op_ffi_ptr_of( state: &mut OpState, #[anybuffer] buf: *const u8, -) -> Result<*mut c_void, AnyError> +) -> Result<*mut c_void, ReprError> where FP: FfiPermissions + 'static, { let permissions = state.borrow_mut::(); - permissions.check_partial_no_path()?; + permissions + .check_partial_no_path() + .map_err(ReprError::Permission)?; Ok(buf as *mut c_void) } @@ -59,12 +102,14 @@ where pub fn op_ffi_ptr_of_exact( state: &mut OpState, buf: v8::Local, -) -> Result<*mut c_void, AnyError> +) -> Result<*mut c_void, ReprError> where FP: FfiPermissions + 'static, { let permissions = state.borrow_mut::(); - permissions.check_partial_no_path()?; + permissions + .check_partial_no_path() + .map_err(ReprError::Permission)?; let Some(buf) = buf.get_backing_store() else { return Ok(0 as _); @@ -80,15 +125,17 @@ pub fn op_ffi_ptr_offset( state: &mut OpState, ptr: *mut c_void, #[number] offset: isize, -) -> Result<*mut c_void, AnyError> +) -> Result<*mut c_void, ReprError> where FP: FfiPermissions + 'static, { let permissions = state.borrow_mut::(); - permissions.check_partial_no_path()?; + permissions + .check_partial_no_path() + .map_err(ReprError::Permission)?; if ptr.is_null() { - return Err(type_error("Invalid pointer to offset, pointer is null")); + return Err(ReprError::InvalidOffset); } // TODO(mmastrac): Create a RawPointer that can safely do pointer math. @@ -110,12 +157,14 @@ unsafe extern "C" fn noop_deleter_callback( pub fn op_ffi_ptr_value( state: &mut OpState, ptr: *mut c_void, -) -> Result +) -> Result where FP: FfiPermissions + 'static, { let permissions = state.borrow_mut::(); - permissions.check_partial_no_path()?; + permissions + .check_partial_no_path() + .map_err(ReprError::Permission)?; Ok(ptr as usize) } @@ -127,15 +176,17 @@ pub fn op_ffi_get_buf( ptr: *mut c_void, #[number] offset: isize, #[number] len: usize, -) -> Result, AnyError> +) -> Result, ReprError> where FP: FfiPermissions + 'static, { let permissions = state.borrow_mut::(); - permissions.check_partial_no_path()?; + permissions + .check_partial_no_path() + .map_err(ReprError::Permission)?; if ptr.is_null() { - return Err(type_error("Invalid ArrayBuffer pointer, pointer is null")); + return Err(ReprError::InvalidArrayBuffer); } // SAFETY: Trust the user to have provided a real pointer, offset, and a valid matching size to it. Since this is a foreign pointer, we should not do any deletion. @@ -144,7 +195,7 @@ where ptr.offset(offset), len, noop_deleter_callback, - std::ptr::null_mut(), + ptr::null_mut(), ) } .make_shared(); @@ -159,19 +210,19 @@ pub fn op_ffi_buf_copy_into( #[number] offset: isize, #[anybuffer] dst: &mut [u8], #[number] len: usize, -) -> Result<(), AnyError> +) -> Result<(), ReprError> where FP: FfiPermissions + 'static, { let permissions = state.borrow_mut::(); - permissions.check_partial_no_path()?; + permissions + .check_partial_no_path() + .map_err(ReprError::Permission)?; if src.is_null() { - Err(type_error("Invalid ArrayBuffer pointer, pointer is null")) + Err(ReprError::InvalidArrayBuffer) } else if dst.len() < len { - Err(range_error( - "Destination length is smaller than source length", - )) + Err(ReprError::DestinationLengthTooShort) } else { let src = src as *const c_void; @@ -190,24 +241,24 @@ pub fn op_ffi_cstr_read( state: &mut OpState, ptr: *mut c_void, #[number] offset: isize, -) -> Result, AnyError> +) -> Result, ReprError> where FP: FfiPermissions + 'static, { let permissions = state.borrow_mut::(); - permissions.check_partial_no_path()?; + permissions + .check_partial_no_path() + .map_err(ReprError::Permission)?; if ptr.is_null() { - return Err(type_error("Invalid CString pointer, pointer is null")); + return Err(ReprError::InvalidCString); } 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::String::new_from_utf8(scope, cstr, v8::NewStringType::Normal) - .ok_or_else(|| { - type_error("Invalid CString pointer, string exceeds max length") - })?; + .ok_or_else(|| ReprError::CStringTooLong)?; Ok(value) } @@ -216,15 +267,17 @@ pub fn op_ffi_read_bool( state: &mut OpState, ptr: *mut c_void, #[number] offset: isize, -) -> Result +) -> Result where FP: FfiPermissions + 'static, { let permissions = state.borrow_mut::(); - permissions.check_partial_no_path()?; + permissions + .check_partial_no_path() + .map_err(ReprError::Permission)?; if ptr.is_null() { - return Err(type_error("Invalid bool pointer, pointer is null")); + return Err(ReprError::InvalidBool); } // SAFETY: ptr and offset are user provided. @@ -236,15 +289,17 @@ pub fn op_ffi_read_u8( state: &mut OpState, ptr: *mut c_void, #[number] offset: isize, -) -> Result +) -> Result where FP: FfiPermissions + 'static, { let permissions = state.borrow_mut::(); - permissions.check_partial_no_path()?; + permissions + .check_partial_no_path() + .map_err(ReprError::Permission)?; if ptr.is_null() { - return Err(type_error("Invalid u8 pointer, pointer is null")); + return Err(ReprError::InvalidU8); } // SAFETY: ptr and offset are user provided. @@ -258,15 +313,17 @@ pub fn op_ffi_read_i8( state: &mut OpState, ptr: *mut c_void, #[number] offset: isize, -) -> Result +) -> Result where FP: FfiPermissions + 'static, { let permissions = state.borrow_mut::(); - permissions.check_partial_no_path()?; + permissions + .check_partial_no_path() + .map_err(ReprError::Permission)?; if ptr.is_null() { - return Err(type_error("Invalid i8 pointer, pointer is null")); + return Err(ReprError::InvalidI8); } // SAFETY: ptr and offset are user provided. @@ -280,15 +337,17 @@ pub fn op_ffi_read_u16( state: &mut OpState, ptr: *mut c_void, #[number] offset: isize, -) -> Result +) -> Result where FP: FfiPermissions + 'static, { let permissions = state.borrow_mut::(); - permissions.check_partial_no_path()?; + permissions + .check_partial_no_path() + .map_err(ReprError::Permission)?; if ptr.is_null() { - return Err(type_error("Invalid u16 pointer, pointer is null")); + return Err(ReprError::InvalidU16); } // SAFETY: ptr and offset are user provided. @@ -302,15 +361,17 @@ pub fn op_ffi_read_i16( state: &mut OpState, ptr: *mut c_void, #[number] offset: isize, -) -> Result +) -> Result where FP: FfiPermissions + 'static, { let permissions = state.borrow_mut::(); - permissions.check_partial_no_path()?; + permissions + .check_partial_no_path() + .map_err(ReprError::Permission)?; if ptr.is_null() { - return Err(type_error("Invalid i16 pointer, pointer is null")); + return Err(ReprError::InvalidI16); } // SAFETY: ptr and offset are user provided. @@ -324,15 +385,17 @@ pub fn op_ffi_read_u32( state: &mut OpState, ptr: *mut c_void, #[number] offset: isize, -) -> Result +) -> Result where FP: FfiPermissions + 'static, { let permissions = state.borrow_mut::(); - permissions.check_partial_no_path()?; + permissions + .check_partial_no_path() + .map_err(ReprError::Permission)?; if ptr.is_null() { - return Err(type_error("Invalid u32 pointer, pointer is null")); + return Err(ReprError::InvalidU32); } // SAFETY: ptr and offset are user provided. @@ -344,15 +407,17 @@ pub fn op_ffi_read_i32( state: &mut OpState, ptr: *mut c_void, #[number] offset: isize, -) -> Result +) -> Result where FP: FfiPermissions + 'static, { let permissions = state.borrow_mut::(); - permissions.check_partial_no_path()?; + permissions + .check_partial_no_path() + .map_err(ReprError::Permission)?; if ptr.is_null() { - return Err(type_error("Invalid i32 pointer, pointer is null")); + return Err(ReprError::InvalidI32); } // SAFETY: ptr and offset are user provided. @@ -367,15 +432,17 @@ pub fn op_ffi_read_u64( // Note: The representation of 64-bit integers is function-wide. We cannot // choose to take this parameter as a number while returning a bigint. #[bigint] offset: isize, -) -> Result +) -> Result where FP: FfiPermissions + 'static, { let permissions = state.borrow_mut::(); - permissions.check_partial_no_path()?; + permissions + .check_partial_no_path() + .map_err(ReprError::Permission)?; if ptr.is_null() { - return Err(type_error("Invalid u64 pointer, pointer is null")); + return Err(ReprError::InvalidU64); } let value = @@ -393,15 +460,17 @@ pub fn op_ffi_read_i64( // Note: The representation of 64-bit integers is function-wide. We cannot // choose to take this parameter as a number while returning a bigint. #[bigint] offset: isize, -) -> Result +) -> Result where FP: FfiPermissions + 'static, { let permissions = state.borrow_mut::(); - permissions.check_partial_no_path()?; + permissions + .check_partial_no_path() + .map_err(ReprError::Permission)?; if ptr.is_null() { - return Err(type_error("Invalid i64 pointer, pointer is null")); + return Err(ReprError::InvalidI64); } let value = @@ -416,15 +485,17 @@ pub fn op_ffi_read_f32( state: &mut OpState, ptr: *mut c_void, #[number] offset: isize, -) -> Result +) -> Result where FP: FfiPermissions + 'static, { let permissions = state.borrow_mut::(); - permissions.check_partial_no_path()?; + permissions + .check_partial_no_path() + .map_err(ReprError::Permission)?; if ptr.is_null() { - return Err(type_error("Invalid f32 pointer, pointer is null")); + return Err(ReprError::InvalidF32); } // SAFETY: ptr and offset are user provided. @@ -436,15 +507,17 @@ pub fn op_ffi_read_f64( state: &mut OpState, ptr: *mut c_void, #[number] offset: isize, -) -> Result +) -> Result where FP: FfiPermissions + 'static, { let permissions = state.borrow_mut::(); - permissions.check_partial_no_path()?; + permissions + .check_partial_no_path() + .map_err(ReprError::Permission)?; if ptr.is_null() { - return Err(type_error("Invalid f64 pointer, pointer is null")); + return Err(ReprError::InvalidF64); } // SAFETY: ptr and offset are user provided. @@ -456,15 +529,17 @@ pub fn op_ffi_read_ptr( state: &mut OpState, ptr: *mut c_void, #[number] offset: isize, -) -> Result<*mut c_void, AnyError> +) -> Result<*mut c_void, ReprError> where FP: FfiPermissions + 'static, { let permissions = state.borrow_mut::(); - permissions.check_partial_no_path()?; + permissions + .check_partial_no_path() + .map_err(ReprError::Permission)?; if ptr.is_null() { - return Err(type_error("Invalid pointer pointer, pointer is null")); + return Err(ReprError::InvalidPointer); } // SAFETY: ptr and offset are user provided. diff --git a/ext/ffi/static.rs b/ext/ffi/static.rs index f08605754b..61b4059336 100644 --- a/ext/ffi/static.rs +++ b/ext/ffi/static.rs @@ -2,14 +2,24 @@ use crate::dlfcn::DynamicLibraryResource; use crate::symbol::NativeType; -use deno_core::error::type_error; -use deno_core::error::AnyError; use deno_core::op2; use deno_core::v8; use deno_core::OpState; use deno_core::ResourceId; use std::ptr; +#[derive(Debug, thiserror::Error)] +pub enum StaticError { + #[error(transparent)] + Dlfcn(super::DlfcnError), + #[error("Invalid FFI static type 'void'")] + InvalidTypeVoid, + #[error("Invalid FFI static type 'struct'")] + InvalidTypeStruct, + #[error(transparent)] + Resource(deno_core::error::AnyError), +} + #[op2] pub fn op_ffi_get_static<'scope>( scope: &mut v8::HandleScope<'scope>, @@ -18,24 +28,27 @@ pub fn op_ffi_get_static<'scope>( #[string] name: String, #[serde] static_type: NativeType, optional: bool, -) -> Result, AnyError> { - let resource = state.resource_table.get::(rid)?; +) -> Result, StaticError> { + let resource = state + .resource_table + .get::(rid) + .map_err(StaticError::Resource)?; let data_ptr = match resource.get_static(name) { - Ok(data_ptr) => Ok(data_ptr), + Ok(data_ptr) => data_ptr, Err(err) => { if optional { let null: v8::Local = v8::null(scope).into(); return Ok(null); } else { - Err(err) + return Err(StaticError::Dlfcn(err)); } } - }?; + }; Ok(match static_type { NativeType::Void => { - return Err(type_error("Invalid FFI static type 'void'")); + return Err(StaticError::InvalidTypeVoid); } NativeType::Bool => { // SAFETY: ptr is user provided @@ -132,7 +145,7 @@ pub fn op_ffi_get_static<'scope>( external } NativeType::Struct(_) => { - return Err(type_error("Invalid FFI static type 'struct'")); + return Err(StaticError::InvalidTypeStruct); } }) } diff --git a/runtime/errors.rs b/runtime/errors.rs index 5735635957..612a66b6e2 100644 --- a/runtime/errors.rs +++ b/runtime/errors.rs @@ -17,6 +17,12 @@ use deno_core::serde_json; use deno_core::url; use deno_core::ModuleResolutionError; use deno_cron::CronError; +use deno_ffi::CallError; +use deno_ffi::CallbackError; +use deno_ffi::DlfcnError; +use deno_ffi::IRError; +use deno_ffi::ReprError; +use deno_ffi::StaticError; use deno_tls::TlsError; use deno_webstorage::WebStorageError; use std::env; @@ -159,6 +165,65 @@ pub fn get_nix_error_class(error: &nix::Error) -> &'static str { } } +fn get_ffi_repr_error_class(e: &ReprError) -> &'static str { + match e { + ReprError::InvalidOffset => "TypeError", + ReprError::InvalidArrayBuffer => "TypeError", + ReprError::DestinationLengthTooShort => "RangeError", + ReprError::InvalidCString => "TypeError", + ReprError::CStringTooLong => "TypeError", + ReprError::InvalidBool => "TypeError", + ReprError::InvalidU8 => "TypeError", + ReprError::InvalidI8 => "TypeError", + ReprError::InvalidU16 => "TypeError", + ReprError::InvalidI16 => "TypeError", + ReprError::InvalidU32 => "TypeError", + ReprError::InvalidI32 => "TypeError", + ReprError::InvalidU64 => "TypeError", + ReprError::InvalidI64 => "TypeError", + ReprError::InvalidF32 => "TypeError", + ReprError::InvalidF64 => "TypeError", + ReprError::InvalidPointer => "TypeError", + ReprError::Permission(e) => get_error_class_name(e).unwrap_or("Error"), + } +} + +fn get_ffi_dlfcn_error_class(e: &DlfcnError) -> &'static str { + match e { + DlfcnError::RegisterSymbol { .. } => "Error", + DlfcnError::Dlopen(_) => "Error", + DlfcnError::Permission(e) => get_error_class_name(e).unwrap_or("Error"), + DlfcnError::Other(e) => get_error_class_name(e).unwrap_or("Error"), + } +} + +fn get_ffi_static_error_class(e: &StaticError) -> &'static str { + match e { + StaticError::Dlfcn(e) => get_ffi_dlfcn_error_class(e), + StaticError::InvalidTypeVoid => "TypeError", + StaticError::InvalidTypeStruct => "TypeError", + StaticError::Resource(e) => get_error_class_name(e).unwrap_or("Error"), + } +} + +fn get_ffi_callback_error_class(e: &CallbackError) -> &'static str { + match e { + CallbackError::Resource(e) => get_error_class_name(e).unwrap_or("Error"), + CallbackError::Other(e) => get_error_class_name(e).unwrap_or("Error"), + CallbackError::Permission(e) => get_error_class_name(e).unwrap_or("Error"), + } +} + +fn get_ffi_call_error_class(e: &CallError) -> &'static str { + match e { + CallError::IR(_) => "TypeError", + CallError::NonblockingCallFailure(_) => "Error", + CallError::InvalidSymbol(_) => "TypeError", + CallError::Permission(e) => get_error_class_name(e).unwrap_or("Error"), + CallError::Callback(e) => get_ffi_callback_error_class(e), + } +} + fn get_webstorage_class_name(e: &WebStorageError) -> &'static str { match e { WebStorageError::ContextNotSupported => "DOMExceptionNotSupportedError", @@ -232,6 +297,21 @@ pub fn get_error_class_name(e: &AnyError) -> Option<&'static str> { .or_else(|| deno_webgpu::error::get_error_class_name(e)) .or_else(|| deno_web::get_error_class_name(e)) .or_else(|| deno_websocket::get_network_error_class_name(e)) + .or_else(|| e.downcast_ref::().map(|_| "TypeError")) + .or_else(|| e.downcast_ref::().map(get_ffi_repr_error_class)) + .or_else(|| { + e.downcast_ref::() + .map(get_ffi_dlfcn_error_class) + }) + .or_else(|| { + e.downcast_ref::() + .map(get_ffi_static_error_class) + }) + .or_else(|| { + e.downcast_ref::() + .map(get_ffi_callback_error_class) + }) + .or_else(|| e.downcast_ref::().map(get_ffi_call_error_class)) .or_else(|| e.downcast_ref::().map(get_tls_error_class)) .or_else(|| e.downcast_ref::().map(get_cron_error_class)) .or_else(|| e.downcast_ref::().map(get_canvas_error))