From 863894faa1856e5ad72c03edbbdac3fe896d3df0 Mon Sep 17 00:00:00 2001 From: John Sharratt's Shared Account Date: Tue, 2 Aug 2022 10:50:34 +1000 Subject: [PATCH] Removed uint8view() from the memory API and redundant FunctionEnv in tests --- examples/engine_headless.rs | 1 - examples/errors.rs | 1 - examples/exports_function.rs | 1 - examples/exports_global.rs | 1 - examples/exports_memory.rs | 1 - examples/features.rs | 1 - examples/imports_global.rs | 1 - examples/instance.rs | 1 - examples/memory.rs | 1 - examples/metering.rs | 1 - examples/tunables_limit_memory.rs | 1 - lib/api/Cargo.toml | 2 +- lib/api/src/js/externals/memory.rs | 5 +++- lib/api/src/js/externals/memory_view.rs | 37 ++++++++++++++++++++---- lib/api/src/sys/externals/memory.rs | 4 +++ lib/api/src/sys/externals/memory_view.rs | 27 +++++++++++++++++ lib/api/tests/js_externals.rs | 6 ---- lib/api/tests/js_instance.rs | 24 +++++++-------- 18 files changed, 77 insertions(+), 39 deletions(-) diff --git a/examples/engine_headless.rs b/examples/engine_headless.rs index e11731f49e0..6ff9c95d2a0 100644 --- a/examples/engine_headless.rs +++ b/examples/engine_headless.rs @@ -96,7 +96,6 @@ fn main() -> Result<(), Box> { // We create a headless Universal engine. let engine = EngineBuilder::headless(); let mut store = Store::new(engine); - let _env = FunctionEnv::new(&mut store, ()); println!("Deserializing module..."); // Here we go. diff --git a/examples/errors.rs b/examples/errors.rs index 46fa0c05b28..d2bedfbf69a 100644 --- a/examples/errors.rs +++ b/examples/errors.rs @@ -39,7 +39,6 @@ fn main() -> Result<(), Box> { // the default provided by Wasmer. // You can use `Store::default()` for that. let mut store = Store::new(Cranelift::default()); - let _env = FunctionEnv::new(&mut store, ()); println!("Compiling module..."); // Let's compile the Wasm module. diff --git a/examples/exports_function.rs b/examples/exports_function.rs index 7722e95c2dd..958a86d094f 100644 --- a/examples/exports_function.rs +++ b/examples/exports_function.rs @@ -40,7 +40,6 @@ fn main() -> Result<(), Box> { // the default provided by Wasmer. // You can use `Store::default()` for that. let mut store = Store::new(Cranelift::default()); - let _env = FunctionEnv::new(&mut store, ()); println!("Compiling module..."); // Let's compile the Wasm module. diff --git a/examples/exports_global.rs b/examples/exports_global.rs index f880e8b7a94..06921b0d846 100644 --- a/examples/exports_global.rs +++ b/examples/exports_global.rs @@ -40,7 +40,6 @@ fn main() -> Result<(), Box> { // the default provided by Wasmer. // You can use `Store::default()` for that. let mut store = Store::new(Cranelift::default()); - let _env = FunctionEnv::new(&mut store, ()); println!("Compiling module..."); // Let's compile the Wasm module. diff --git a/examples/exports_memory.rs b/examples/exports_memory.rs index 2eb1fb196ed..c6c3d058ea0 100644 --- a/examples/exports_memory.rs +++ b/examples/exports_memory.rs @@ -37,7 +37,6 @@ fn main() -> Result<(), Box> { // the default provided by Wasmer. // You can use `Store::default()` for that. let mut store = Store::new(Cranelift::default()); - let _env = FunctionEnv::new(&mut store, ()); println!("Compiling module..."); // Let's compile the Wasm module. diff --git a/examples/features.rs b/examples/features.rs index 1469f47b0de..4f8a800386b 100644 --- a/examples/features.rs +++ b/examples/features.rs @@ -41,7 +41,6 @@ fn main() -> anyhow::Result<()> { // Now, let's define the store, and compile the module. let mut store = Store::new(engine); - let _env = FunctionEnv::new(&mut store, ()); let module = Module::new(&store, wasm_bytes)?; // Finally, let's instantiate the module, and execute something diff --git a/examples/imports_global.rs b/examples/imports_global.rs index 2609161716a..a00c9756431 100644 --- a/examples/imports_global.rs +++ b/examples/imports_global.rs @@ -40,7 +40,6 @@ fn main() -> Result<(), Box> { // the default provided by Wasmer. // You can use `Store::default()` for that. let mut store = Store::new(Cranelift::default()); - let _env = FunctionEnv::new(&mut store, ()); println!("Compiling module..."); // Let's compile the Wasm module. diff --git a/examples/instance.rs b/examples/instance.rs index faff810ebeb..e563af07e73 100644 --- a/examples/instance.rs +++ b/examples/instance.rs @@ -39,7 +39,6 @@ fn main() -> Result<(), Box> { // the default provided by Wasmer. // You can use `Store::default()` for that. let mut store = Store::new(Cranelift::default()); - let _env = FunctionEnv::new(&mut store, ()); println!("Compiling module..."); // Let's compile the Wasm module. diff --git a/examples/memory.rs b/examples/memory.rs index c90fbdd91aa..3cd1815a181 100644 --- a/examples/memory.rs +++ b/examples/memory.rs @@ -59,7 +59,6 @@ fn main() -> anyhow::Result<()> { // the default provided by Wasmer. // You can use `Store::default()` for that. let mut store = Store::new(Cranelift::default()); - let _env = FunctionEnv::new(&mut store, ()); println!("Compiling module..."); // Let's compile the Wasm module. diff --git a/examples/metering.rs b/examples/metering.rs index 39dadf9d358..0d37491e0a9 100644 --- a/examples/metering.rs +++ b/examples/metering.rs @@ -72,7 +72,6 @@ fn main() -> anyhow::Result<()> { // We use our previously create compiler configuration // with the Universal engine. let mut store = Store::new(EngineBuilder::new(compiler_config)); - let _env = FunctionEnv::new(&mut store, ()); println!("Compiling module..."); // Let's compile the Wasm module. diff --git a/examples/tunables_limit_memory.rs b/examples/tunables_limit_memory.rs index b31b4b74d37..2b2f36900ac 100644 --- a/examples/tunables_limit_memory.rs +++ b/examples/tunables_limit_memory.rs @@ -143,7 +143,6 @@ fn main() -> Result<(), Box> { // Create a store, that holds the engine and our custom tunables let mut store = Store::new_with_tunables(compiler, tunables); - let _env = FunctionEnv::new(&mut store, ()); println!("Compiling module..."); let module = Module::new(&store, wasm_bytes)?; diff --git a/lib/api/Cargo.toml b/lib/api/Cargo.toml index 16a518fa9b8..7cac69aa1f5 100644 --- a/lib/api/Cargo.toml +++ b/lib/api/Cargo.toml @@ -26,9 +26,9 @@ indexmap = { version = "1.6", features = ["serde-1"] } cfg-if = "1.0" thiserror = "1.0" more-asserts = "0.2" -tracing = "0.1" # - Optional shared dependencies. wat = { version = "1.0", optional = true } +tracing = { version = "0.1", optional = true } # Dependencies and Development Dependencies for `sys`. [target.'cfg(not(target_arch = "wasm32"))'.dependencies] diff --git a/lib/api/src/js/externals/memory.rs b/lib/api/src/js/externals/memory.rs index 65e657b2bdd..d181e6c6d89 100644 --- a/lib/api/src/js/externals/memory.rs +++ b/lib/api/src/js/externals/memory.rs @@ -7,6 +7,7 @@ use std::marker::PhantomData; use std::mem::MaybeUninit; use std::slice; use thiserror::Error; +#[cfg(feature = "tracing")] use tracing::warn; use wasm_bindgen::prelude::*; @@ -177,7 +178,6 @@ impl Memory { IntoPages: Into, { let pages = delta.into(); - warn!("memory grow {}", pages.0); let js_memory = &self.handle.get_mut(store.objects_mut()).memory; let our_js_memory: &JSMemory = JsCast::unchecked_from_js_ref(js_memory); let new_pages = our_js_memory.grow(pages.0).map_err(|err| { @@ -239,6 +239,7 @@ impl<'a> MemoryBuffer<'a> { .ok_or(MemoryAccessError::Overflow)?; let view = unsafe { &*(self.base) }; if end > view.length().into() { + #[cfg(feature = "tracing")] warn!("attempted to read ({} bytes) beyond the bounds of the memory view ({} > {})", buf.len(), end, view.length()); return Err(MemoryAccessError::HeapOutOfBounds); } @@ -257,6 +258,7 @@ impl<'a> MemoryBuffer<'a> { .ok_or(MemoryAccessError::Overflow)?; let view = unsafe { &*(self.base) }; if end > view.length().into() { + #[cfg(feature = "tracing")] warn!("attempted to read ({} bytes) beyond the bounds of the memory view ({} > {})", buf.len(), end, view.length()); return Err(MemoryAccessError::HeapOutOfBounds); } @@ -273,6 +275,7 @@ impl<'a> MemoryBuffer<'a> { .ok_or(MemoryAccessError::Overflow)?; let view = unsafe { &mut *(self.base) }; if end > view.length().into() { + #[cfg(feature = "tracing")] warn!("attempted to write ({} bytes) beyond the bounds of the memory view ({} > {})", data.len(), end, view.length()); return Err(MemoryAccessError::HeapOutOfBounds); } diff --git a/lib/api/src/js/externals/memory_view.rs b/lib/api/src/js/externals/memory_view.rs index 34fbb5f26be..c22d22b912f 100644 --- a/lib/api/src/js/externals/memory_view.rs +++ b/lib/api/src/js/externals/memory_view.rs @@ -4,6 +4,7 @@ use std::convert::TryInto; use std::marker::PhantomData; use std::mem::MaybeUninit; use std::slice; +#[cfg(feature = "tracing")] use tracing::warn; use wasmer_types::{Bytes, Pages}; @@ -79,12 +80,6 @@ impl<'a> MemoryView<'a> Bytes(self.size as usize).try_into().unwrap() } - /// Used by tests - #[doc(hidden)] - pub fn uint8view(&self) -> js_sys::Uint8Array { - self.view.clone() - } - pub(crate) fn buffer(&self) -> MemoryBuffer<'a> { MemoryBuffer { base: &self.view as *const _ as *mut _, @@ -112,6 +107,7 @@ impl<'a> MemoryView<'a> .map_err(|_| MemoryAccessError::Overflow)?; let end = offset.checked_add(len).ok_or(MemoryAccessError::Overflow)?; if end > view.length() { + #[cfg(feature = "tracing")] warn!("attempted to read ({} bytes) beyond the bounds of the memory view ({} > {})", len, end, view.length()); Err(MemoryAccessError::HeapOutOfBounds)?; } @@ -119,6 +115,19 @@ impl<'a> MemoryView<'a> Ok(()) } + /// Safely reads a single byte from memory at the given offset + /// + /// This method is guaranteed to be safe (from the host side) in the face of + /// concurrent writes. + pub fn read_u8( + &self, + offset: u64 + ) -> Result { + let mut buf = [0u8; 1]; + self.read(offset, &mut buf)?; + Ok(buf[0]) + } + /// Safely reads bytes from the memory at the given offset. /// /// This method is similar to `read` but allows reading into an @@ -142,6 +151,7 @@ impl<'a> MemoryView<'a> .map_err(|_| MemoryAccessError::Overflow)?; let end = offset.checked_add(len).ok_or(MemoryAccessError::Overflow)?; if end > view.length() { + #[cfg(feature = "tracing")] warn!("attempted to read ({} bytes) beyond the bounds of the memory view ({} > {})", len, end, view.length()); Err(MemoryAccessError::HeapOutOfBounds)?; } @@ -177,10 +187,25 @@ impl<'a> MemoryView<'a> let view = &self.view; let end = offset.checked_add(len).ok_or(MemoryAccessError::Overflow)?; if end > view.length() { + #[cfg(feature = "tracing")] warn!("attempted to write ({} bytes) beyond the bounds of the memory view ({} > {})", len, end, view.length()); Err(MemoryAccessError::HeapOutOfBounds)?; } view.subarray(offset, end).copy_from(data); Ok(()) } + + /// Safely reads a single byte from memory at the given offset + /// + /// This method is guaranteed to be safe (from the host side) in the face of + /// concurrent writes. + pub fn write_u8( + &self, + offset: u64, + val: u8 + ) -> Result<(), MemoryAccessError> { + let buf = [ val ]; + self.write(offset, &buf)?; + Ok(()) + } } diff --git a/lib/api/src/sys/externals/memory.rs b/lib/api/src/sys/externals/memory.rs index 7ed86e8d851..f01a5dafdf7 100644 --- a/lib/api/src/sys/externals/memory.rs +++ b/lib/api/src/sys/externals/memory.rs @@ -8,6 +8,7 @@ use std::marker::PhantomData; use std::mem; use std::mem::MaybeUninit; use std::slice; +#[cfg(feature = "tracing")] use tracing::warn; use wasmer_types::Pages; use wasmer_vm::{InternalStoreHandle, MemoryError, StoreHandle, StoreObjects, VMExtern, VMMemory}; @@ -177,6 +178,7 @@ impl<'a> MemoryBuffer<'a> { .checked_add(buf.len() as u64) .ok_or(MemoryAccessError::Overflow)?; if end > self.len.try_into().unwrap() { + #[cfg(feature = "tracing")] warn!("attempted to read ({} bytes) beyond the bounds of the memory view ({} > {})", buf.len(), end, self.len); return Err(MemoryAccessError::HeapOutOfBounds); } @@ -195,6 +197,7 @@ impl<'a> MemoryBuffer<'a> { .checked_add(buf.len() as u64) .ok_or(MemoryAccessError::Overflow)?; if end > self.len.try_into().unwrap() { + #[cfg(feature = "tracing")] warn!("attempted to read ({} bytes) beyond the bounds of the memory view ({} > {})", buf.len(), end, self.len); return Err(MemoryAccessError::HeapOutOfBounds); } @@ -211,6 +214,7 @@ impl<'a> MemoryBuffer<'a> { .checked_add(data.len() as u64) .ok_or(MemoryAccessError::Overflow)?; if end > self.len.try_into().unwrap() { + #[cfg(feature = "tracing")] warn!("attempted to write ({} bytes) beyond the bounds of the memory view ({} > {})", data.len(), end, self.len); return Err(MemoryAccessError::HeapOutOfBounds); } diff --git a/lib/api/src/sys/externals/memory_view.rs b/lib/api/src/sys/externals/memory_view.rs index 5e1eb247703..dcbfad6dcd5 100644 --- a/lib/api/src/sys/externals/memory_view.rs +++ b/lib/api/src/sys/externals/memory_view.rs @@ -115,6 +115,19 @@ impl<'a> MemoryView<'a> self.buffer.read(offset, buf) } + /// Safely reads a single byte from memory at the given offset + /// + /// This method is guaranteed to be safe (from the host side) in the face of + /// concurrent writes. + pub fn read_u8( + &self, + offset: u64 + ) -> Result { + let mut buf = [0u8; 1]; + self.read(offset, &mut buf)?; + Ok(buf[0]) + } + /// Safely reads bytes from the memory at the given offset. /// /// This method is similar to `read` but allows reading into an @@ -147,4 +160,18 @@ impl<'a> MemoryView<'a> ) -> Result<(), MemoryAccessError> { self.buffer.write(offset, data) } + + /// Safely reads a single byte from memory at the given offset + /// + /// This method is guaranteed to be safe (from the host side) in the face of + /// concurrent writes. + pub fn write_u8( + &self, + offset: u64, + val: u8 + ) -> Result<(), MemoryAccessError> { + let buf = [ val ]; + self.write(offset, &buf)?; + Ok(()) + } } diff --git a/lib/api/tests/js_externals.rs b/lib/api/tests/js_externals.rs index 41dacdabda1..4f8401232f3 100644 --- a/lib/api/tests/js_externals.rs +++ b/lib/api/tests/js_externals.rs @@ -6,7 +6,6 @@ mod js { #[wasm_bindgen_test] fn global_new() { let mut store = Store::default(); - let _env = FunctionEnv::new(&mut store, ()); let global = Global::new(&mut store, Value::I32(10)); assert_eq!( global.ty(&store), @@ -29,7 +28,6 @@ mod js { #[wasm_bindgen_test] fn global_get() { let mut store = Store::default(); - let _env = FunctionEnv::new(&mut store, ()); let global_i32 = Global::new(&mut store, Value::I32(10)); assert_eq!(global_i32.get(&store), Value::I32(10)); // 64-bit values are not yet fully supported in some versions of Node @@ -46,7 +44,6 @@ mod js { #[wasm_bindgen_test] fn global_set() { let mut store = Store::default(); - let _env = FunctionEnv::new(&mut store, ()); let global_i32 = Global::new(&mut store, Value::I32(10)); // Set on a constant should error assert!(global_i32.set(&mut store, Value::I32(20)).is_err()); @@ -144,7 +141,6 @@ mod js { #[wasm_bindgen_test] fn memory_new() { let mut store = Store::default(); - let _env = FunctionEnv::new(&mut store, ()); let memory_type = MemoryType { shared: false, minimum: Pages(0), @@ -158,7 +154,6 @@ mod js { #[wasm_bindgen_test] fn memory_grow() { let mut store = Store::default(); - let _env = FunctionEnv::new(&mut store, ()); let desc = MemoryType::new(Pages(10), Some(Pages(16)), false); let memory = Memory::new(&mut store, desc).unwrap(); @@ -444,7 +439,6 @@ mod js { #[wasm_bindgen_test] fn function_outlives_instance() { let mut store = Store::default(); - let _env = FunctionEnv::new(&mut store, ()); let wat = r#"(module (type $sum_t (func (param i32 i32) (result i32))) (func $sum_f (type $sum_t) (param $x i32) (param $y i32) (result i32) diff --git a/lib/api/tests/js_instance.rs b/lib/api/tests/js_instance.rs index 6a1912037b6..8366c6eb36f 100644 --- a/lib/api/tests/js_instance.rs +++ b/lib/api/tests/js_instance.rs @@ -41,7 +41,6 @@ mod js { .unwrap(); let import_object = imports! {}; - let _env = FunctionEnv::new(&mut store, ()); let instance = Instance::new(&mut store, &module, &import_object).unwrap(); let memory = instance.exports.get_memory("mem").unwrap(); @@ -81,7 +80,6 @@ mod js { .unwrap(); let import_object = imports! {}; - let _env = FunctionEnv::new(&mut store, ()); let instance = Instance::new(&mut store, &module, &import_object).unwrap(); let get_magic = instance.exports.get_function("get_magic").unwrap(); @@ -403,7 +401,7 @@ mod js { fn imported_fn(env: FunctionEnvMut<'_, Env>, arg: u32) -> u32 { let memory: &Memory = env.data().memory.as_ref().unwrap(); - let memory_val = memory.view(&env).uint8view().get_index(0); + let memory_val = memory.view(&env).read_u8(0).unwrap(); return (memory_val as u32) * env.data().multiplier * arg; } @@ -425,11 +423,11 @@ mod js { let memory = instance.exports.get_memory("memory").unwrap(); assert_eq!(memory.view(&store).data_size(), 65536); - let memory_val = memory.view(&store).uint8view() .get_index(0); + let memory_val = memory.view(&store).read_u8(0).unwrap(); assert_eq!(memory_val, 0); - memory.view(&store).uint8view().set_index(0, 2); - let memory_val = memory.view(&store).uint8view().get_index(0); + memory.view(&store).write_u8(0, 2).unwrap(); + let memory_val = memory.view(&store).read_u8(0); assert_eq!(memory_val, 2); env.as_mut(&mut store).memory = Some(memory.clone()); @@ -441,7 +439,7 @@ mod js { assert_eq!(exported.call(&mut store, &[Val::I32(4)]), Ok(expected)); // It works if we update the memory - memory.view(&store).uint8view().set_index(0, 3); + memory.view(&store).write_u8(0, 3).unwrap(); let expected = vec![Val::I32(36)].into_boxed_slice(); assert_eq!(exported.call(&mut store, &[Val::I32(4)]), Ok(expected)); } @@ -511,7 +509,7 @@ mod js { args: &[Val], ) -> Result, RuntimeError> { let memory: &Memory = env.data().memory.as_ref().unwrap(); - let memory_val = memory.view(&env).uint8view().get_index(0); + let memory_val = memory.view(&env).read(0).unwrap(); let value = (memory_val as u32) * env.data().multiplier * args[0].unwrap_i32() as u32; return Ok(vec![Val::I32(value as _)]); } @@ -536,11 +534,11 @@ mod js { let memory = instance.exports.get_memory("memory").unwrap(); assert_eq!(memory.view(&store).data_size(), 65536); - let memory_val = memory.view(&store).uint8view().get_index(0); + let memory_val = memory.view(&store).read_u8(0).unwrap(); assert_eq!(memory_val, 0); - memory.view(&store).uint8view().set_index(0, 2); - let memory_val = memory.view(&store).uint8view().get_index(0); + memory.view(&store).write_u8(0, 2).unwrap(); + let memory_val = memory.view(&store).read_u8(0).unwrap(); assert_eq!(memory_val, 2); env.as_mut(&mut store).memory = Some(memory.clone()); @@ -552,7 +550,7 @@ mod js { assert_eq!(exported.call(&mut store, &[Val::I32(4)]), Ok(expected)); // It works if we update the memory - memory.view(&store).uint8view().set_index(0, 3); + memory.view(&store).write_u8(0, 3).unwrap(); let expected = vec![Val::I32(36)].into_boxed_slice(); assert_eq!(exported.call(&mut store, &[Val::I32(4)]), Ok(expected)); } @@ -585,7 +583,6 @@ mod js { ], }) .unwrap(); - let _env = FunctionEnv::new(&mut store, ()); let global = Global::new_mut(&mut store, Value::I32(0)); let import_object = imports! { "" => { @@ -802,7 +799,6 @@ mod js { .unwrap(); let import_object = imports! {}; - let _env = FunctionEnv::new(&mut store, ()); let result = Instance::new(&mut store, &module, &import_object); let err = result.unwrap_err(); assert!(format!("{:?}", err).contains("zero"))