Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework Module deserialization functions #3894

Merged
merged 3 commits into from
May 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 39 additions & 6 deletions lib/api/src/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,24 +135,57 @@ impl Engine {
))
}

#[deprecated(since = "3.2.0")]
#[cfg(all(feature = "sys", not(target_arch = "wasm32")))]
/// Deserializes a WebAssembly module
/// Deserializes a WebAssembly module which was previously serialized with
/// `Module::serialize`.
///
/// NOTE: you should almost always prefer [`Self::deserialize`].
///
/// # Safety
/// See [`Artifact::deserialize_unchecked`].
pub unsafe fn deserialize_unchecked(
&self,
bytes: &[u8],
) -> Result<Arc<Artifact>, DeserializeError> {
Ok(Arc::new(Artifact::deserialize_unchecked(&self.0, bytes)?))
}

#[cfg(all(feature = "sys", not(target_arch = "wasm32")))]
/// Deserializes a WebAssembly module which was previously serialized with
/// `Module::serialize`.
///
/// The serialized content must represent a serialized WebAssembly module.
/// # Safety
/// See [`Artifact::deserialize`].
pub unsafe fn deserialize(&self, bytes: &[u8]) -> Result<Arc<Artifact>, DeserializeError> {
Ok(Arc::new(Artifact::deserialize(&self.0, bytes)?))
}

#[deprecated(since = "3.2.0")]
#[cfg(all(feature = "sys", not(target_arch = "wasm32")))]
/// Deserializes a WebAssembly module from a path
/// Load a serialized WebAssembly module from a file and deserialize it.
///
/// NOTE: you should almost always prefer [`Self::deserialize_from_file`].
///
/// # Safety
/// See [`Artifact::deserialize_unchecked`].
pub unsafe fn deserialize_from_file_unchecked(
&self,
file_ref: &Path,
) -> Result<Arc<Artifact>, DeserializeError> {
let mut file = std::fs::File::open(file_ref)?;
let mut buffer = Vec::new();
// read the whole file
file.read_to_end(&mut buffer)?;
Ok(Arc::new(Artifact::deserialize_unchecked(
&self.0,
buffer.as_slice(),
)?))
}

#[cfg(all(feature = "sys", not(target_arch = "wasm32")))]
/// Load a serialized WebAssembly module from a file and deserialize it.
///
/// The file's content must represent a serialized WebAssembly module.
/// # Safety
/// See [`Artifact::deserialize`].
pub unsafe fn deserialize_from_file(
&self,
file_ref: &Path,
Expand Down
10 changes: 5 additions & 5 deletions lib/api/src/js/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ impl Module {
));
}

pub unsafe fn deserialize(
pub unsafe fn deserialize_unchecked(
_engine: &impl AsEngineRef,
_bytes: impl IntoBytes,
) -> Result<Self, DeserializeError> {
Expand All @@ -239,7 +239,7 @@ impl Module {
return Err(DeserializeError::Generic("You need to enable the `js-serializable-module` feature flag to deserialize a `Module`".to_string()));
}

pub fn deserialize_checked(
pub unsafe fn deserialize(
_engine: &impl AsEngineRef,
_bytes: impl IntoBytes,
) -> Result<Self, DeserializeError> {
Expand All @@ -251,20 +251,20 @@ impl Module {
return Err(DeserializeError::Generic("You need to enable the `js-serializable-module` feature flag to deserialize a `Module`".to_string()));
}

pub unsafe fn deserialize_from_file(
pub unsafe fn deserialize_from_file_unchecked(
engine: &impl AsEngineRef,
path: impl AsRef<Path>,
) -> Result<Self, DeserializeError> {
let bytes = std::fs::read(path.as_ref())?;
Self::deserialize(engine, bytes)
}

pub fn deserialize_from_file_checked(
pub unsafe fn deserialize_from_file(
engine: &impl AsEngineRef,
path: impl AsRef<Path>,
) -> Result<Self, DeserializeError> {
let bytes = std::fs::read(path.as_ref())?;
Self::deserialize_checked(engine, bytes)
Self::deserialize(engine, bytes)
}

pub fn set_name(&mut self, name: &str) -> bool {
Expand Down
26 changes: 13 additions & 13 deletions lib/api/src/jsc/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,35 +183,35 @@ impl Module {
));
}

pub unsafe fn deserialize(
_engine: &impl AsEngineRef,
_bytes: impl IntoBytes,
pub unsafe fn deserialize_unchecked(
engine: &impl AsEngineRef,
bytes: impl IntoBytes,
) -> Result<Self, DeserializeError> {
return Self::from_binary(_engine, &_bytes.into_bytes())
.map_err(|e| DeserializeError::Compiler(e));
Self::deserialize(engine, bytes)
}

pub fn deserialize_checked(
_engine: &impl AsEngineRef,
_bytes: impl IntoBytes,
pub unsafe fn deserialize(
theduke marked this conversation as resolved.
Show resolved Hide resolved
engine: &impl AsEngineRef,
bytes: impl IntoBytes,
) -> Result<Self, DeserializeError> {
unimplemented!();
return Self::from_binary(engine, &bytes.into_bytes())
.map_err(|e| DeserializeError::Compiler(e));
}

pub unsafe fn deserialize_from_file(
pub unsafe fn deserialize_from_file_unchecked(
engine: &impl AsEngineRef,
path: impl AsRef<Path>,
) -> Result<Self, DeserializeError> {
let bytes = std::fs::read(path.as_ref())?;
Self::deserialize(engine, bytes)
Self::deserialize_unchecked(engine, bytes)
}

pub fn deserialize_from_file_checked(
pub unsafe fn deserialize_from_file(
engine: &impl AsEngineRef,
path: impl AsRef<Path>,
) -> Result<Self, DeserializeError> {
let bytes = std::fs::read(path.as_ref())?;
Self::deserialize_checked(engine, bytes)
Self::deserialize(engine, bytes)
}

pub fn set_name(&mut self, name: &str) -> bool {
Expand Down
42 changes: 27 additions & 15 deletions lib/api/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,9 +219,9 @@ impl Module {
Ok(())
}

/// Deserializes a serialized Module binary into a `Module`.
/// Deserializes a serialized module binary into a `Module`.
///
/// Note: You should usually prefer the safe [`Module::deserialize_checked`].
/// Note: You should usually prefer the safer [`Module::deserialize`].
///
/// # Important
///
Expand Down Expand Up @@ -250,11 +250,13 @@ impl Module {
/// # Ok(())
/// # }
/// ```
pub unsafe fn deserialize(
pub unsafe fn deserialize_unchecked(
engine: &impl AsEngineRef,
bytes: impl IntoBytes,
) -> Result<Self, DeserializeError> {
Ok(Self(module_imp::Module::deserialize(engine, bytes)?))
Ok(Self(module_imp::Module::deserialize_unchecked(
engine, bytes,
)?))
}

/// Deserializes a serialized Module binary into a `Module`.
Expand All @@ -272,17 +274,21 @@ impl Module {
/// # use wasmer::*;
/// # fn main() -> anyhow::Result<()> {
/// # let mut store = Store::default();
/// let module = Module::deserialize_checked(&store, serialized_data)?;
/// let module = Module::deserialize(&store, serialized_data)?;
/// # Ok(())
/// # }
/// ```
pub fn deserialize_checked(
///
/// # Safety
/// This function is inherently **unsafe**, because it loads executable code
/// into memory.
/// The loaded bytes must be trusted to contain a valid artifact previously
/// built with [`Self::serialize`].
pub unsafe fn deserialize(
engine: &impl AsEngineRef,
bytes: impl IntoBytes,
) -> Result<Self, DeserializeError> {
Ok(Self(module_imp::Module::deserialize_checked(
engine, bytes,
)?))
Ok(Self(module_imp::Module::deserialize(engine, bytes)?))
}

/// Deserializes a a serialized Module located in a `Path` into a `Module`.
Expand All @@ -298,37 +304,43 @@ impl Module {
/// # Ok(())
/// # }
/// ```
pub fn deserialize_from_file_checked(
///
/// # Safety
///
/// See [`Self::deserialize`].
pub unsafe fn deserialize_from_file(
engine: &impl AsEngineRef,
path: impl AsRef<Path>,
) -> Result<Self, DeserializeError> {
Ok(Self(module_imp::Module::deserialize_from_file_checked(
Ok(Self(module_imp::Module::deserialize_from_file(
engine, path,
)?))
}

/// Deserializes a a serialized Module located in a `Path` into a `Module`.
/// > Note: the module has to be serialized before with the `serialize` method.
///
/// You should usually prefer the safer [`Module::deserialize_from_file`].
///
/// # Safety
///
/// Please check [`Module::deserialize`].
/// Please check [`Module::deserialize_unchecked`].
///
/// # Usage
///
/// ```ignore
/// # use wasmer::*;
/// # let mut store = Store::default();
/// # fn main() -> anyhow::Result<()> {
/// let module = Module::deserialize_from_file(&store, path)?;
/// let module = Module::deserialize_from_file_unchecked(&store, path)?;
/// # Ok(())
/// # }
/// ```
pub unsafe fn deserialize_from_file(
pub unsafe fn deserialize_from_file_unchecked(
engine: &impl AsEngineRef,
path: impl AsRef<Path>,
) -> Result<Self, DeserializeError> {
Ok(Self(module_imp::Module::deserialize_from_file(
Ok(Self(module_imp::Module::deserialize_from_file_unchecked(
engine, path,
)?))
}
Expand Down
16 changes: 6 additions & 10 deletions lib/api/src/sys/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ impl Module {
self.artifact.serialize().map(|bytes| bytes.into())
}

pub unsafe fn deserialize(
pub unsafe fn deserialize_unchecked(
engine: &impl AsEngineRef,
bytes: impl IntoBytes,
) -> Result<Self, DeserializeError> {
Expand All @@ -78,20 +78,16 @@ impl Module {
Ok(Self::from_artifact(artifact))
}

pub fn deserialize_checked(
pub unsafe fn deserialize(
engine: &impl AsEngineRef,
bytes: impl IntoBytes,
) -> Result<Self, DeserializeError> {
let bytes = bytes.into_bytes();
let artifact = engine
.as_engine_ref()
.engine()
.0
.deserialize_checked(&bytes)?;
let artifact = engine.as_engine_ref().engine().0.deserialize(&bytes)?;
Ok(Self::from_artifact(artifact))
}

pub unsafe fn deserialize_from_file(
pub unsafe fn deserialize_from_file_unchecked(
engine: &impl AsEngineRef,
path: impl AsRef<Path>,
) -> Result<Self, DeserializeError> {
Expand All @@ -103,15 +99,15 @@ impl Module {
Ok(Self::from_artifact(artifact))
}

pub fn deserialize_from_file_checked(
pub unsafe fn deserialize_from_file(
engine: &impl AsEngineRef,
path: impl AsRef<Path>,
) -> Result<Self, DeserializeError> {
let artifact = engine
.as_engine_ref()
.engine()
.0
.deserialize_from_file_checked(path.as_ref())?;
.deserialize_from_file(path.as_ref())?;
Ok(Self::from_artifact(artifact))
}

Expand Down
2 changes: 1 addition & 1 deletion lib/cache/src/filesystem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ impl Cache for FileSystemCache {
key.to_string()
};
let path = self.path.join(filename);
let ret = Module::deserialize_from_file_checked(engine, path.clone());
let ret = Module::deserialize_from_file(engine, path.clone());
if ret.is_err() {
// If an error occurs while deserializing then we can not trust it anymore
// so delete the cache file
Expand Down
2 changes: 1 addition & 1 deletion lib/cli/src/commands/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ impl RunWithPathBuf {
if wasmer_compiler::Artifact::is_deserializable(&contents) {
let engine = wasmer_compiler::EngineBuilder::headless();
let store = Store::new(engine);
let module = Module::deserialize_from_file_checked(&store, &self.path)?;
let module = unsafe { Module::deserialize_from_file(&store, &self.path)? };
return Ok((store, module));
}
let (store, compiler_type) = self.store.get_store()?;
Expand Down
Loading