From f81a3669f3adcef38f9bad12d172faab6761ab4b Mon Sep 17 00:00:00 2001 From: OJ Kwon <1210596+kwonoj@users.noreply.github.com> Date: Sun, 28 May 2023 19:02:48 -0700 Subject: [PATCH] feat(serialized): deserialize with bytechecked --- crates/swc_common/src/comments.rs | 3 + crates/swc_common/src/plugin/serialized.rs | 56 ++++++++----------- crates/swc_common/src/syntax_pos/hygiene.rs | 25 +++++---- crates/swc_css_ast/src/at_rule.rs | 4 ++ crates/swc_css_ast/src/lib.rs | 1 - crates/swc_css_ast/src/selector.rs | 4 ++ crates/swc_css_ast/src/token.rs | 8 +++ crates/swc_css_ast/src/value.rs | 6 ++ crates/swc_plugin_macro/src/lib.rs | 2 +- .../read_returned_result_from_host.rs | 14 +++-- 10 files changed, 69 insertions(+), 54 deletions(-) diff --git a/crates/swc_common/src/comments.rs b/crates/swc_common/src/comments.rs index e3f5760c9993..b95b263e8b8f 100644 --- a/crates/swc_common/src/comments.rs +++ b/crates/swc_common/src/comments.rs @@ -546,6 +546,8 @@ impl SingleThreadedComments { any(feature = "rkyv-impl"), derive(rkyv::Archive, rkyv::Serialize, rkyv::Deserialize) )] +#[cfg_attr(feature = "rkyv-impl", archive(check_bytes))] +#[cfg_attr(feature = "rkyv-impl", archive_attr(repr(C)))] pub struct Comment { pub kind: CommentKind, pub span: Span, @@ -564,6 +566,7 @@ impl Spanned for Comment { any(feature = "rkyv-impl"), derive(rkyv::Archive, rkyv::Serialize, rkyv::Deserialize) )] +#[cfg_attr(feature = "rkyv-impl", archive(check_bytes))] pub enum CommentKind { Line, Block, diff --git a/crates/swc_common/src/plugin/serialized.rs b/crates/swc_common/src/plugin/serialized.rs index 50587218003c..01ead118228c 100644 --- a/crates/swc_common/src/plugin/serialized.rs +++ b/crates/swc_common/src/plugin/serialized.rs @@ -1,8 +1,8 @@ -use std::any::type_name; +//use std::any::type_name; use anyhow::Error; -#[cfg(feature = "__rkyv")] -use rkyv::Deserialize; +//#[cfg(feature = "__rkyv")] +//use rkyv::Deserialize; #[derive(Debug, Clone, PartialEq, Eq)] #[non_exhaustive] @@ -99,42 +99,30 @@ impl PluginSerializedBytes { } #[tracing::instrument(level = "info", skip_all)] - pub fn deserialize(&self) -> Result, Error> + pub fn deserialize<'a, W>(&'a self) -> Result, Error> where - W: rkyv::Archive, - W::Archived: rkyv::Deserialize, + W: rkyv::Archive + 'a, + W::Archived: 'a + + rkyv::CheckBytes> + + rkyv::Deserialize, { - use anyhow::Context; - - let archived = unsafe { rkyv::archived_root::>(&self.field[..]) }; - - archived - .deserialize(&mut rkyv::de::deserializers::SharedDeserializeMap::new()) - .with_context(|| format!("failed to deserialize `{}`", type_name::())) + use rkyv::validation::{validators::CheckDeserializeError, CheckArchiveError}; + + let result = rkyv::from_bytes::>(&self.field[..]); + result.map_err(move |err| match err { + CheckDeserializeError::DeserializeError(e) => e.into(), + CheckDeserializeError::CheckBytesError(e) => match e { + CheckArchiveError::CheckBytesError(_e) => { + // [TODO]: we can't forward CheckBytes::Error itself as it is Archived type of VersionedSerializable struct itself. + // Still we need to carry better diagnostics if this occurs. + Error::msg("CheckBytesError") + } + CheckArchiveError::ContextError(e) => e.into(), + }, + }) } } -/// Simple wrapper around constructing PluginSerializedBytes from raw -/// ptr to call deserialize to support common workflow on both of runtime -/// (host / plugin) to instantiate a struct from allocated / copied ptr. -/// -/// # Safety -/// This is naturally unsafe by constructing bytes slice from raw ptr. -#[tracing::instrument(level = "info", skip_all)] -pub unsafe fn deserialize_from_ptr( - raw_allocated_ptr: *const u8, - raw_allocated_ptr_len: u32, -) -> Result, Error> -where - W: rkyv::Archive, - W::Archived: rkyv::Deserialize, -{ - let serialized = - PluginSerializedBytes::from_raw_ptr(raw_allocated_ptr, raw_allocated_ptr_len as usize); - - serialized.deserialize() -} - /// A wrapper type for the structures to be passed into plugins /// serializes the contained value out-of-line so that newer /// versions can be viewed as the older version. diff --git a/crates/swc_common/src/syntax_pos/hygiene.rs b/crates/swc_common/src/syntax_pos/hygiene.rs index 78b350a88897..297f92adbcb6 100644 --- a/crates/swc_common/src/syntax_pos/hygiene.rs +++ b/crates/swc_common/src/syntax_pos/hygiene.rs @@ -196,14 +196,15 @@ impl Mark { } // Deserialize result, assign / return values as needed. - let context: MutableMarkContext = unsafe { - crate::plugin::serialized::deserialize_from_ptr( + let context: MutableMarkContext = + crate::plugin::serialized::PluginSerializedBytes::from_raw_ptr( ptr, len.try_into().expect("Should able to convert ptr length"), ) + .deserialize() .expect("Should able to deserialize") - .into_inner() - }; + .into_inner(); + self = Mark::from_u32(context.0); return context.2 != 0; @@ -237,14 +238,14 @@ impl Mark { __mark_least_ancestor(a.0, b.0, ptr as _); } - let context: MutableMarkContext = unsafe { - crate::plugin::serialized::deserialize_from_ptr( + let context: MutableMarkContext = + crate::plugin::serialized::PluginSerializedBytes::from_raw_ptr( ptr, len.try_into().expect("Should able to convert ptr length"), ) + .deserialize() .expect("Should able to deserialize") - .into_inner() - }; + .into_inner(); a = Mark::from_u32(context.0); b = Mark::from_u32(context.1); @@ -435,14 +436,14 @@ impl SyntaxContext { __syntax_context_remove_mark_proxy(self.0, ptr as _); } - let context: MutableMarkContext = unsafe { - crate::plugin::serialized::deserialize_from_ptr( + let context: MutableMarkContext = + crate::plugin::serialized::PluginSerializedBytes::from_raw_ptr( ptr, len.try_into().expect("Should able to convert ptr length"), ) + .deserialize() .expect("Should able to deserialize") - .into_inner() - }; + .into_inner(); *self = SyntaxContext(context.0); diff --git a/crates/swc_css_ast/src/at_rule.rs b/crates/swc_css_ast/src/at_rule.rs index 558ec9e9faed..612454ccab8e 100644 --- a/crates/swc_css_ast/src/at_rule.rs +++ b/crates/swc_css_ast/src/at_rule.rs @@ -436,6 +436,8 @@ pub struct MediaFeatureBoolean { feature = "rkyv", derive(rkyv::Archive, rkyv::Serialize, rkyv::Deserialize) )] +#[cfg_attr(feature = "rkyv", archive(check_bytes))] +#[cfg_attr(feature = "rkyv", archive_attr(repr(u32)))] #[cfg_attr( feature = "rkyv", archive(bound( @@ -764,6 +766,8 @@ pub struct SizeFeatureBoolean { feature = "rkyv", derive(rkyv::Archive, rkyv::Serialize, rkyv::Deserialize) )] +#[cfg_attr(feature = "rkyv", archive(check_bytes))] +#[cfg_attr(feature = "rkyv", archive_attr(repr(u32)))] #[cfg_attr( feature = "rkyv", archive(bound( diff --git a/crates/swc_css_ast/src/lib.rs b/crates/swc_css_ast/src/lib.rs index 8173f6d8dcf4..4dd9aa25c4ca 100644 --- a/crates/swc_css_ast/src/lib.rs +++ b/crates/swc_css_ast/src/lib.rs @@ -1,6 +1,5 @@ #![deny(clippy::all)] #![allow(clippy::large_enum_variant)] - //! AST definitions for CSS. pub use self::{at_rule::*, base::*, selector::*, token::*, value::*}; diff --git a/crates/swc_css_ast/src/selector.rs b/crates/swc_css_ast/src/selector.rs index c9508bd6b933..1d216ea6cf41 100644 --- a/crates/swc_css_ast/src/selector.rs +++ b/crates/swc_css_ast/src/selector.rs @@ -131,6 +131,8 @@ pub struct Combinator { deserialize = "__D: rkyv::de::SharedDeserializeRegistry" )) )] +#[cfg_attr(feature = "rkyv", archive(check_bytes))] +#[cfg_attr(feature = "rkyv", archive_attr(repr(u32)))] pub enum CombinatorValue { /// ` ` Descendant, @@ -264,6 +266,8 @@ pub struct AttributeSelector { feature = "rkyv", derive(rkyv::Archive, rkyv::Serialize, rkyv::Deserialize) )] +#[cfg_attr(feature = "rkyv", archive(check_bytes))] +#[cfg_attr(feature = "rkyv", archive_attr(repr(u32)))] #[cfg_attr( feature = "rkyv", archive(bound( diff --git a/crates/swc_css_ast/src/token.rs b/crates/swc_css_ast/src/token.rs index 26280f301435..45d9eff9cfaa 100644 --- a/crates/swc_css_ast/src/token.rs +++ b/crates/swc_css_ast/src/token.rs @@ -31,6 +31,8 @@ impl Take for TokenAndSpan { any(feature = "rkyv-impl"), derive(rkyv::Archive, rkyv::Serialize, rkyv::Deserialize) )] +#[cfg_attr(feature = "rkyv-impl", archive(check_bytes))] +#[cfg_attr(feature = "rkyv-impl", archive_attr(repr(C)))] pub struct UrlKeyValue(pub Atom, pub Atom); #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, Is, EqIgnoreSpan)] @@ -38,6 +40,8 @@ pub struct UrlKeyValue(pub Atom, pub Atom); feature = "rkyv", derive(rkyv::Archive, rkyv::Serialize, rkyv::Deserialize) )] +#[cfg_attr(feature = "rkyv", archive(check_bytes))] +#[cfg_attr(feature = "rkyv", archive_attr(repr(u32)))] #[cfg_attr( feature = "rkyv", archive(bound( @@ -59,6 +63,8 @@ pub enum NumberType { feature = "rkyv", derive(rkyv::Archive, rkyv::Serialize, rkyv::Deserialize) )] +#[cfg_attr(feature = "rkyv", archive(check_bytes))] +#[cfg_attr(feature = "rkyv", archive_attr(repr(C)))] #[cfg_attr(feature = "serde-impl", derive(Serialize, Deserialize))] pub struct DimensionToken { pub value: f64, @@ -76,6 +82,8 @@ pub struct DimensionToken { feature = "rkyv", derive(rkyv::Archive, rkyv::Serialize, rkyv::Deserialize) )] +#[cfg_attr(feature = "rkyv", archive(check_bytes))] +#[cfg_attr(feature = "rkyv", archive_attr(repr(u32)))] #[cfg_attr( feature = "rkyv", archive(bound( diff --git a/crates/swc_css_ast/src/value.rs b/crates/swc_css_ast/src/value.rs index 51db0021f083..aff8d67f604a 100644 --- a/crates/swc_css_ast/src/value.rs +++ b/crates/swc_css_ast/src/value.rs @@ -127,6 +127,8 @@ impl EqIgnoreSpan for Str { deserialize = "__D: rkyv::de::SharedDeserializeRegistry" )) )] +#[cfg_attr(feature = "rkyv", archive(check_bytes))] +#[cfg_attr(feature = "rkyv", archive_attr(repr(u32)))] pub enum DelimiterValue { /// `,` Comma, @@ -406,6 +408,8 @@ pub struct Ratio { deserialize = "__D: rkyv::de::SharedDeserializeRegistry" )) )] +#[cfg_attr(feature = "rkyv", archive(check_bytes))] +#[cfg_attr(feature = "rkyv", archive_attr(repr(u32)))] pub enum BinOp { /// `+` Add, @@ -514,6 +518,8 @@ pub struct CalcOperator { deserialize = "__D: rkyv::de::SharedDeserializeRegistry" )) )] +#[cfg_attr(feature = "rkyv-impl", archive(check_bytes))] +#[cfg_attr(feature = "rkyv-impl", archive_attr(repr(u32)))] pub enum CalcOperatorType { /// `+` Add, diff --git a/crates/swc_plugin_macro/src/lib.rs b/crates/swc_plugin_macro/src/lib.rs index ea96065346c3..caecfa422e9b 100644 --- a/crates/swc_plugin_macro/src/lib.rs +++ b/crates/swc_plugin_macro/src/lib.rs @@ -130,7 +130,7 @@ fn handle_func(func: ItemFn, ast_type: Ident) -> TokenStream { unresolved_mark: u32, should_enable_comments_proxy: i32) -> u32 { // Reconstruct `Program` & config string from serialized program // Host (SWC) should allocate memory, copy bytes and pass ptr to plugin. - let program = unsafe { swc_core::common::plugin::serialized::deserialize_from_ptr(ast_ptr, ast_ptr_len) }; + let program = swc_core::common::plugin::serialized::PluginSerializedBytes::from_raw_ptr(ast_ptr, ast_ptr_len).deserialize(); if program.is_err() { let err = swc_core::common::plugin::serialized::PluginError::Deserialize("Failed to deserialize program received from host".to_string()); return construct_error_ptr(err); diff --git a/crates/swc_plugin_proxy/src/memory_interop/read_returned_result_from_host.rs b/crates/swc_plugin_proxy/src/memory_interop/read_returned_result_from_host.rs index a8a1e1900f1b..e1feb71392bc 100644 --- a/crates/swc_plugin_proxy/src/memory_interop/read_returned_result_from_host.rs +++ b/crates/swc_plugin_proxy/src/memory_interop/read_returned_result_from_host.rs @@ -1,6 +1,6 @@ #[cfg_attr(not(target_arch = "wasm32"), allow(unused))] #[cfg(any(feature = "__plugin_rt", feature = "__plugin_mode"))] -use swc_common::plugin::serialized::{deserialize_from_ptr, PluginSerializedBytes}; +use swc_common::plugin::serialized::PluginSerializedBytes; /// A struct to exchange allocated data between memory spaces. #[cfg_attr( @@ -62,16 +62,17 @@ where } // Return reconstructted AllocatedBytesPtr to reveal ptr to the allocated bytes - Some(unsafe { - deserialize_from_ptr( + Some( + PluginSerializedBytes::from_raw_ptr( serialized_allocated_bytes_raw_ptr, serialized_allocated_bytes_raw_ptr_size .try_into() .expect("Should able to convert ptr length"), ) + .deserialize() .expect("Should able to deserialize AllocatedBytesPtr") - .into_inner() - }) + .into_inner(), + ) } #[cfg(not(feature = "__rkyv"))] @@ -95,10 +96,11 @@ where // Using AllocatedBytesPtr's value, reconstruct actual return value allocated_returned_value_ptr.map(|allocated_returned_value_ptr| unsafe { - deserialize_from_ptr( + PluginSerializedBytes::from_raw_ptr( allocated_returned_value_ptr.0 as _, allocated_returned_value_ptr.1, ) + .deserialize() .expect("Returned value should be serializable") .into_inner() })