From ee7d4501435f0ebd655c8b50bd6e41ca19e71abc Mon Sep 17 00:00:00 2001 From: Leo Kettmeir Date: Mon, 14 Oct 2024 15:05:49 -0700 Subject: refactor(ext/ffi): use concrete error types (#26170) --- ext/ffi/call.rs | 54 +++++++++++++++++++++++++++++++++++------------------- 1 file changed, 35 insertions(+), 19 deletions(-) (limited to 'ext/ffi/call.rs') diff --git a/ext/ffi/call.rs b/ext/ffi/call.rs index 3572b9e81..d337b29b0 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) } -- cgit v1.2.3 From fa49fd404be82daf8ac7228bf54e780135f67b17 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Thu, 24 Oct 2024 09:41:38 +0530 Subject: fix(ext/ffi): return u64/i64 as bigints from nonblocking ffi calls (#26486) Fixes https://github.com/denoland/deno/issues/25194 --- ext/ffi/call.rs | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) (limited to 'ext/ffi/call.rs') diff --git a/ext/ffi/call.rs b/ext/ffi/call.rs index d337b29b0..ef61dc383 100644 --- a/ext/ffi/call.rs +++ b/ext/ffi/call.rs @@ -9,12 +9,14 @@ use crate::FfiPermissions; use crate::ForeignFunction; use deno_core::op2; use deno_core::serde_json::Value; +use deno_core::serde_v8::BigInt as V8BigInt; use deno_core::serde_v8::ExternalPointer; use deno_core::unsync::spawn_blocking; use deno_core::v8; use deno_core::OpState; use deno_core::ResourceId; use libffi::middle::Arg; +use num_bigint::BigInt; use serde::Serialize; use std::cell::RefCell; use std::ffi::c_void; @@ -202,6 +204,7 @@ where #[serde(untagged)] pub enum FfiValue { Value(Value), + BigInt(V8BigInt), External(ExternalPointer), } @@ -251,18 +254,18 @@ fn ffi_call( NativeType::I32 => { FfiValue::Value(Value::from(cif.call::(fun_ptr, &call_args))) } - NativeType::U64 => { - FfiValue::Value(Value::from(cif.call::(fun_ptr, &call_args))) - } - NativeType::I64 => { - FfiValue::Value(Value::from(cif.call::(fun_ptr, &call_args))) - } - NativeType::USize => { - FfiValue::Value(Value::from(cif.call::(fun_ptr, &call_args))) - } - NativeType::ISize => { - FfiValue::Value(Value::from(cif.call::(fun_ptr, &call_args))) - } + NativeType::U64 => FfiValue::BigInt(V8BigInt::from(BigInt::from( + cif.call::(fun_ptr, &call_args), + ))), + NativeType::I64 => FfiValue::BigInt(V8BigInt::from(BigInt::from( + cif.call::(fun_ptr, &call_args), + ))), + NativeType::USize => FfiValue::BigInt(V8BigInt::from(BigInt::from( + cif.call::(fun_ptr, &call_args), + ))), + NativeType::ISize => FfiValue::BigInt(V8BigInt::from(BigInt::from( + cif.call::(fun_ptr, &call_args), + ))), NativeType::F32 => { FfiValue::Value(Value::from(cif.call::(fun_ptr, &call_args))) } -- cgit v1.2.3 From fe9f0ee5934871175758857899fe64e56c397fd5 Mon Sep 17 00:00:00 2001 From: Leo Kettmeir Date: Mon, 4 Nov 2024 09:17:21 -0800 Subject: refactor(runtime/permissions): use concrete error types (#26464) --- ext/ffi/call.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) (limited to 'ext/ffi/call.rs') diff --git a/ext/ffi/call.rs b/ext/ffi/call.rs index ef61dc383..bbff0ee48 100644 --- a/ext/ffi/call.rs +++ b/ext/ffi/call.rs @@ -32,7 +32,9 @@ pub enum CallError { #[error("Invalid FFI symbol name: '{0}'")] InvalidSymbol(String), #[error(transparent)] - Permission(deno_core::error::AnyError), + Permission(#[from] deno_permissions::PermissionCheckError), + #[error(transparent)] + Resource(deno_core::error::AnyError), #[error(transparent)] Callback(#[from] super::CallbackError), } @@ -301,9 +303,7 @@ where { let mut state = state.borrow_mut(); let permissions = state.borrow_mut::(); - permissions - .check_partial_no_path() - .map_err(CallError::Permission)?; + permissions.check_partial_no_path()?; }; let symbol = PtrSymbol::new(pointer, &def)?; @@ -347,7 +347,7 @@ pub fn op_ffi_call_nonblocking( let resource = state .resource_table .get::(rid) - .map_err(CallError::Permission)?; + .map_err(CallError::Resource)?; let symbols = &resource.symbols; *symbols .get(&symbol) @@ -401,9 +401,7 @@ where { let mut state = state.borrow_mut(); let permissions = state.borrow_mut::(); - permissions - .check_partial_no_path() - .map_err(CallError::Permission)?; + permissions.check_partial_no_path()?; }; let symbol = PtrSymbol::new(pointer, &def)?; -- cgit v1.2.3