From 096bef77fb6ba0ee3d4bc78de862c1d648df028a Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Fri, 31 Mar 2023 12:54:13 +0530 Subject: [PATCH 1/3] fix(ops): fallback when FastApiOneByteString is not utf8 --- ops/optimizer.rs | 29 +++++++++++++++++-- ops/optimizer_tests/cow_str.expected | 2 +- ops/optimizer_tests/cow_str.out | 13 +++++++-- .../op_blob_revoke_object_url.expected | 2 +- ops/optimizer_tests/op_print.expected | 2 +- ops/optimizer_tests/owned_string.expected | 2 +- ops/optimizer_tests/owned_string.out | 11 +++++-- ops/optimizer_tests/strings.expected | 2 +- ops/optimizer_tests/strings.out | 11 +++++-- ops/optimizer_tests/strings_result.expected | 2 +- 10 files changed, 61 insertions(+), 15 deletions(-) diff --git a/ops/optimizer.rs b/ops/optimizer.rs index 5a2be7a017075f..cc266c716d5305 100644 --- a/ops/optimizer.rs +++ b/ops/optimizer.rs @@ -200,13 +200,33 @@ impl Transform { *ty = parse_quote! { *const #core::v8::fast_api::FastApiOneByteString }; match str_ty { StringType::Ref => q!(Vars { var: &ident }, { - let var = unsafe { &*var }.as_str(); + let var = match ::std::str::from_utf8(unsafe { &*var }.as_bytes()) { + Ok(v) => v, + Err(_) => { + unsafe { &mut *fast_api_callback_options }.fallback = true; + return Default::default(); + } + }; }), StringType::Cow => q!(Vars { var: &ident }, { - let var = ::std::borrow::Cow::Borrowed(unsafe { &*var }.as_str()); + let var = ::std::borrow::Cow::Borrowed( + match ::std::str::from_utf8(unsafe { &*var }.as_bytes()) { + Ok(v) => v, + Err(_) => { + unsafe { &mut *fast_api_callback_options }.fallback = true; + return Default::default(); + } + }, + ); }), StringType::Owned => q!(Vars { var: &ident }, { - let var = unsafe { &*var }.as_str().to_owned(); + let var = match ::std::str::from_utf8(unsafe { &*var }.as_bytes()) { + Ok(v) => v.to_owned(), + Err(_) => { + unsafe { &mut *fast_api_callback_options }.fallback = true; + return Default::default(); + } + }; }), } } @@ -718,6 +738,7 @@ impl Optimizer { let segment = single_segment(segments)?; match segment { PathSegment { ident, .. } if ident == "str" => { + self.needs_fast_callback_option = true; self.fast_parameters.push(FastValue::SeqOneByteString); assert!(self .transforms @@ -742,6 +763,7 @@ impl Optimizer { if let Some(val) = get_fast_scalar(ident.to_string().as_str()) { self.fast_parameters.push(val); } else if ident == "String" { + self.needs_fast_callback_option = true; // Is `T` an owned String? self.fast_parameters.push(FastValue::SeqOneByteString); assert!(self @@ -775,6 +797,7 @@ impl Optimizer { } // Is `T` a str? PathSegment { ident, .. } if ident == "str" => { + self.needs_fast_callback_option = true; self.fast_parameters.push(FastValue::SeqOneByteString); assert!(self .transforms diff --git a/ops/optimizer_tests/cow_str.expected b/ops/optimizer_tests/cow_str.expected index 8b28965badd495..9db8cfaf31dc90 100644 --- a/ops/optimizer_tests/cow_str.expected +++ b/ops/optimizer_tests/cow_str.expected @@ -3,7 +3,7 @@ returns_result: false has_ref_opstate: false has_rc_opstate: false has_fast_callback_option: false -needs_fast_callback_option: false +needs_fast_callback_option: true fast_result: Some(Void) fast_parameters: [V8Value, SeqOneByteString] transforms: {0: Transform { kind: SeqOneByteString(Cow), index: 0 }} diff --git a/ops/optimizer_tests/cow_str.out b/ops/optimizer_tests/cow_str.out index 89d2f22c48a836..f8379a11853495 100644 --- a/ops/optimizer_tests/cow_str.out +++ b/ops/optimizer_tests/cow_str.out @@ -71,7 +71,7 @@ impl<'scope> deno_core::v8::fast_api::FastFunction for op_cow_str_fast { fn args(&self) -> &'static [deno_core::v8::fast_api::Type] { use deno_core::v8::fast_api::Type::*; use deno_core::v8::fast_api::CType; - &[V8Value, SeqOneByteString] + &[V8Value, SeqOneByteString, CallbackOptions] } #[inline(always)] fn return_type(&self) -> deno_core::v8::fast_api::CType { @@ -82,10 +82,19 @@ impl<'scope> deno_core::v8::fast_api::FastFunction for op_cow_str_fast { fn op_cow_str_fast_fn<'scope>( _: deno_core::v8::Local, c: *const deno_core::v8::fast_api::FastApiOneByteString, + fast_api_callback_options: *mut deno_core::v8::fast_api::FastApiCallbackOptions, ) -> () { use deno_core::v8; use deno_core::_ops; - let c = ::std::borrow::Cow::Borrowed(unsafe { &*c }.as_str()); + let c = ::std::borrow::Cow::Borrowed( + match ::std::str::from_utf8(unsafe { &*c }.as_bytes()) { + Ok(v) => v, + Err(_) => { + unsafe { &mut *fast_api_callback_options }.fallback = true; + return Default::default(); + } + }, + ); let result = op_cow_str::call(c); result } diff --git a/ops/optimizer_tests/op_blob_revoke_object_url.expected b/ops/optimizer_tests/op_blob_revoke_object_url.expected index 00a8964337e796..a412eceb805b41 100644 --- a/ops/optimizer_tests/op_blob_revoke_object_url.expected +++ b/ops/optimizer_tests/op_blob_revoke_object_url.expected @@ -3,7 +3,7 @@ returns_result: true has_ref_opstate: true has_rc_opstate: false has_fast_callback_option: false -needs_fast_callback_option: false +needs_fast_callback_option: true fast_result: Some(Void) fast_parameters: [V8Value, SeqOneByteString] transforms: {1: Transform { kind: SeqOneByteString(Owned), index: 1 }} diff --git a/ops/optimizer_tests/op_print.expected b/ops/optimizer_tests/op_print.expected index 0390be396ee396..6095c138e69bea 100644 --- a/ops/optimizer_tests/op_print.expected +++ b/ops/optimizer_tests/op_print.expected @@ -3,7 +3,7 @@ returns_result: true has_ref_opstate: true has_rc_opstate: false has_fast_callback_option: false -needs_fast_callback_option: false +needs_fast_callback_option: true fast_result: Some(Void) fast_parameters: [V8Value, SeqOneByteString, Bool] transforms: {1: Transform { kind: SeqOneByteString(Ref), index: 1 }} diff --git a/ops/optimizer_tests/owned_string.expected b/ops/optimizer_tests/owned_string.expected index a152754129358a..4c47a0525094d5 100644 --- a/ops/optimizer_tests/owned_string.expected +++ b/ops/optimizer_tests/owned_string.expected @@ -3,7 +3,7 @@ returns_result: false has_ref_opstate: false has_rc_opstate: false has_fast_callback_option: false -needs_fast_callback_option: false +needs_fast_callback_option: true fast_result: Some(U32) fast_parameters: [V8Value, SeqOneByteString] transforms: {0: Transform { kind: SeqOneByteString(Owned), index: 0 }} diff --git a/ops/optimizer_tests/owned_string.out b/ops/optimizer_tests/owned_string.out index 58a952cd76d9ca..3e6d94a80cdb7a 100644 --- a/ops/optimizer_tests/owned_string.out +++ b/ops/optimizer_tests/owned_string.out @@ -83,7 +83,7 @@ impl<'scope> deno_core::v8::fast_api::FastFunction for op_string_length_fast { fn args(&self) -> &'static [deno_core::v8::fast_api::Type] { use deno_core::v8::fast_api::Type::*; use deno_core::v8::fast_api::CType; - &[V8Value, SeqOneByteString] + &[V8Value, SeqOneByteString, CallbackOptions] } #[inline(always)] fn return_type(&self) -> deno_core::v8::fast_api::CType { @@ -94,10 +94,17 @@ impl<'scope> deno_core::v8::fast_api::FastFunction for op_string_length_fast { fn op_string_length_fast_fn<'scope>( _: deno_core::v8::Local, string: *const deno_core::v8::fast_api::FastApiOneByteString, + fast_api_callback_options: *mut deno_core::v8::fast_api::FastApiCallbackOptions, ) -> u32 { use deno_core::v8; use deno_core::_ops; - let string = unsafe { &*string }.as_str().to_owned(); + let string = match ::std::str::from_utf8(unsafe { &*string }.as_bytes()) { + Ok(v) => v.to_owned(), + Err(_) => { + unsafe { &mut *fast_api_callback_options }.fallback = true; + return Default::default(); + } + }; let result = op_string_length::call(string); result } diff --git a/ops/optimizer_tests/strings.expected b/ops/optimizer_tests/strings.expected index ead741054af676..4a6bb155639fca 100644 --- a/ops/optimizer_tests/strings.expected +++ b/ops/optimizer_tests/strings.expected @@ -3,7 +3,7 @@ returns_result: false has_ref_opstate: false has_rc_opstate: false has_fast_callback_option: false -needs_fast_callback_option: false +needs_fast_callback_option: true fast_result: Some(U32) fast_parameters: [V8Value, SeqOneByteString] transforms: {0: Transform { kind: SeqOneByteString(Ref), index: 0 }} diff --git a/ops/optimizer_tests/strings.out b/ops/optimizer_tests/strings.out index 6d5fca0a5f639d..2723775aa63fec 100644 --- a/ops/optimizer_tests/strings.out +++ b/ops/optimizer_tests/strings.out @@ -84,7 +84,7 @@ impl<'scope> deno_core::v8::fast_api::FastFunction for op_string_length_fast { fn args(&self) -> &'static [deno_core::v8::fast_api::Type] { use deno_core::v8::fast_api::Type::*; use deno_core::v8::fast_api::CType; - &[V8Value, SeqOneByteString] + &[V8Value, SeqOneByteString, CallbackOptions] } #[inline(always)] fn return_type(&self) -> deno_core::v8::fast_api::CType { @@ -95,10 +95,17 @@ impl<'scope> deno_core::v8::fast_api::FastFunction for op_string_length_fast { fn op_string_length_fast_fn<'scope>( _: deno_core::v8::Local, string: *const deno_core::v8::fast_api::FastApiOneByteString, + fast_api_callback_options: *mut deno_core::v8::fast_api::FastApiCallbackOptions, ) -> u32 { use deno_core::v8; use deno_core::_ops; - let string = unsafe { &*string }.as_str(); + let string = match ::std::str::from_utf8(unsafe { &*string }.as_bytes()) { + Ok(v) => v, + Err(_) => { + unsafe { &mut *fast_api_callback_options }.fallback = true; + return Default::default(); + } + }; let result = op_string_length::call(string); result } diff --git a/ops/optimizer_tests/strings_result.expected b/ops/optimizer_tests/strings_result.expected index 68c753cabaa0fe..3866751fd29dfb 100644 --- a/ops/optimizer_tests/strings_result.expected +++ b/ops/optimizer_tests/strings_result.expected @@ -3,7 +3,7 @@ returns_result: true has_ref_opstate: false has_rc_opstate: false has_fast_callback_option: false -needs_fast_callback_option: false +needs_fast_callback_option: true fast_result: Some(U32) fast_parameters: [V8Value, SeqOneByteString] transforms: {0: Transform { kind: SeqOneByteString(Ref), index: 0 }} From 5e3a9add5b1212e29f4de1f8d0d799efc2259535 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Fri, 31 Mar 2023 18:26:33 +0530 Subject: [PATCH 2/3] update tests --- ops/optimizer_tests/cow_str.out | 2 +- ops/optimizer_tests/owned_string.out | 2 +- ops/optimizer_tests/strings.out | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ops/optimizer_tests/cow_str.out b/ops/optimizer_tests/cow_str.out index 0acaab471ae685..dc909da8199b16 100644 --- a/ops/optimizer_tests/cow_str.out +++ b/ops/optimizer_tests/cow_str.out @@ -31,7 +31,7 @@ impl op_cow_str { use deno_core::v8::fast_api::CType; Some( deno_core::v8::fast_api::FastFunction::new( - &[V8Value, SeqOneByteString], + &[V8Value, SeqOneByteString, CallbackOptions], CType::Void, op_cow_str_fast_fn as *const ::std::ffi::c_void, ), diff --git a/ops/optimizer_tests/owned_string.out b/ops/optimizer_tests/owned_string.out index 5e71056f905e5a..f8b195b2fb9bac 100644 --- a/ops/optimizer_tests/owned_string.out +++ b/ops/optimizer_tests/owned_string.out @@ -31,7 +31,7 @@ impl op_string_length { use deno_core::v8::fast_api::CType; Some( deno_core::v8::fast_api::FastFunction::new( - &[V8Value, SeqOneByteString], + &[V8Value, SeqOneByteString, CallbackOptions], CType::Uint32, op_string_length_fast_fn as *const ::std::ffi::c_void, ), diff --git a/ops/optimizer_tests/strings.out b/ops/optimizer_tests/strings.out index 2cd65e00421c7d..3238bfc427f03f 100644 --- a/ops/optimizer_tests/strings.out +++ b/ops/optimizer_tests/strings.out @@ -31,7 +31,7 @@ impl op_string_length { use deno_core::v8::fast_api::CType; Some( deno_core::v8::fast_api::FastFunction::new( - &[V8Value, SeqOneByteString], + &[V8Value, SeqOneByteString, CallbackOptions], CType::Uint32, op_string_length_fast_fn as *const ::std::ffi::c_void, ), From 0af9c6dfd312e10c91e4e60262146353e8ed3c5e Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Fri, 31 Mar 2023 19:53:53 +0530 Subject: [PATCH 3/3] Fix --- Cargo.lock | 4 ++-- Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 66aa8a38f77d75..84e559625ebc88 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5270,9 +5270,9 @@ dependencies = [ [[package]] name = "v8" -version = "0.67.0" +version = "0.68.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c7bf30312144d97d3fb61a0c8893eec02f4fa53ec2b691a8d05da9605ab26024" +checksum = "81c69410b7435f1b74e82e243ba906d71e8b9bb350828291418b9311dbd77222" dependencies = [ "bitflags", "fslock", diff --git a/Cargo.toml b/Cargo.toml index e50b763886bac6..57aa273e5e99a6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,7 +43,7 @@ license = "MIT" repository = "https://github.com/denoland/deno" [workspace.dependencies] -v8 = { version = "0.67.0", default-features = false } +v8 = { version = "0.68.0", default-features = false } deno_ast = { version = "0.25.0", features = ["transpiling"] } deno_core = { version = "0.177.0", path = "./core" }