From 5ab278735eed5fb8e60718a545c41e45d7a43743 Mon Sep 17 00:00:00 2001 From: ptitSeb Date: Thu, 8 Sep 2022 12:09:13 +0200 Subject: [PATCH 01/11] Added a new test with TinyTunables --- lib/api/src/sys/tunables.rs | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/lib/api/src/sys/tunables.rs b/lib/api/src/sys/tunables.rs index 0a1eadd7609..e40669946f6 100644 --- a/lib/api/src/sys/tunables.rs +++ b/lib/api/src/sys/tunables.rs @@ -308,4 +308,33 @@ mod tests { } ); } + + #[test] + fn check_customtunables() -> Result<(), Box> { + use crate::{imports, wat2wasm, Instance, Memory, Module, Store}; + use wasmer_compiler_cranelift::Cranelift; + + let wasm_bytes = wat2wasm( + br#"(module (memory 3) (export "memory" (memory 0)))"# + )?; + let compiler = Cranelift::default(); + + let tunables = TinyTunables {}; + let mut store = Store::new_with_tunables(compiler, tunables); + let module = Module::new(&store, wasm_bytes)?; + let import_object = imports! {}; + let instance = Instance::new(&mut store, &module, &import_object)?; + + let mut memories: Vec = instance + .exports + .iter() + .memories() + .map(|pair| pair.1.clone()) + .collect(); + assert_eq!(memories.len(), 1); + let first_memory = memories.pop().unwrap(); + assert_eq!(first_memory.ty(&store).maximum.unwrap(), Pages(1)); + + Ok(()) + } } From 0c25e92a98d44430b338e7139ca4290e261d9c8a Mon Sep 17 00:00:00 2001 From: ptitSeb Date: Thu, 8 Sep 2022 12:13:43 +0200 Subject: [PATCH 02/11] Fixed linter --- lib/api/src/sys/tunables.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/api/src/sys/tunables.rs b/lib/api/src/sys/tunables.rs index e40669946f6..36deb348044 100644 --- a/lib/api/src/sys/tunables.rs +++ b/lib/api/src/sys/tunables.rs @@ -314,9 +314,7 @@ mod tests { use crate::{imports, wat2wasm, Instance, Memory, Module, Store}; use wasmer_compiler_cranelift::Cranelift; - let wasm_bytes = wat2wasm( - br#"(module (memory 3) (export "memory" (memory 0)))"# - )?; + let wasm_bytes = wat2wasm(br#"(module (memory 3) (export "memory" (memory 0)))"#)?; let compiler = Cranelift::default(); let tunables = TinyTunables {}; From 76b4f8be87a02b875236a5bcbb36d0076f6e82e9 Mon Sep 17 00:00:00 2001 From: ptitSeb Date: Thu, 8 Sep 2022 19:15:07 +0200 Subject: [PATCH 03/11] Improved VMTinyMemory test structure vmmemory method --- lib/api/src/sys/tunables.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/api/src/sys/tunables.rs b/lib/api/src/sys/tunables.rs index 36deb348044..0aeb8be0d0b 100644 --- a/lib/api/src/sys/tunables.rs +++ b/lib/api/src/sys/tunables.rs @@ -183,6 +183,7 @@ mod tests { #[derive(Debug)] struct VMTinyMemory { mem: [u8; WASM_PAGE_SIZE], + memory_definition: VMMemoryDefinition, } unsafe impl Send for VMTinyMemory {} @@ -190,8 +191,13 @@ mod tests { impl VMTinyMemory { pub fn new() -> Result { + let memory = [0; WASM_PAGE_SIZE]; Ok(VMTinyMemory { - mem: [0; WASM_PAGE_SIZE], + mem: memory, + memory_definition: VMMemoryDefinition { + base: memory.as_ptr() as _, + current_length: WASM_PAGE_SIZE, + }, }) } } @@ -220,11 +226,7 @@ mod tests { }) } fn vmmemory(&self) -> NonNull { - MaybeInstanceOwned::Host(Box::new(UnsafeCell::new(VMMemoryDefinition { - base: self.mem.as_ptr() as _, - current_length: WASM_PAGE_SIZE, - }))) - .as_ptr() + MaybeInstanceOwned::Host(Box::new(UnsafeCell::new(self.memory_definition))).as_ptr() } fn try_clone(&self) -> Option> { None From 2b8bc38885664d4d5f4503d004e505d4e7668845 Mon Sep 17 00:00:00 2001 From: ptitSeb Date: Fri, 9 Sep 2022 14:36:23 +0200 Subject: [PATCH 04/11] Improved and fixed the VMTinyMemory/TinyTunables test --- lib/api/src/sys/tunables.rs | 62 ++++++++++++++++++++++++------------- 1 file changed, 41 insertions(+), 21 deletions(-) diff --git a/lib/api/src/sys/tunables.rs b/lib/api/src/sys/tunables.rs index 0aeb8be0d0b..95fddc8d8fb 100644 --- a/lib/api/src/sys/tunables.rs +++ b/lib/api/src/sys/tunables.rs @@ -182,8 +182,8 @@ mod tests { #[derive(Debug)] struct VMTinyMemory { - mem: [u8; WASM_PAGE_SIZE], - memory_definition: VMMemoryDefinition, + mem: Vec, + memory_definition: Option, } unsafe impl Send for VMTinyMemory {} @@ -191,31 +191,35 @@ mod tests { impl VMTinyMemory { pub fn new() -> Result { - let memory = [0; WASM_PAGE_SIZE]; - Ok(VMTinyMemory { + let sz = 18 * WASM_PAGE_SIZE; + let mut memory = Vec::new(); + memory.resize(sz, 0); + let mut ret = VMTinyMemory { mem: memory, - memory_definition: VMMemoryDefinition { - base: memory.as_ptr() as _, - current_length: WASM_PAGE_SIZE, - }, - }) + memory_definition: None, + }; + ret.memory_definition = Some(VMMemoryDefinition { + base: ret.mem.as_ptr() as _, + current_length: sz, + }); + Ok(ret) } } impl LinearMemory for VMTinyMemory { fn ty(&self) -> MemoryType { MemoryType { - minimum: Pages::from(1u32), - maximum: Some(Pages::from(1u32)), + minimum: Pages::from(18u32), + maximum: Some(Pages::from(18u32)), shared: false, } } fn size(&self) -> Pages { - Pages::from(1u32) + Pages::from(18u32) } fn style(&self) -> MemoryStyle { MemoryStyle::Static { - bound: Pages::from(1u32), + bound: Pages::from(18u32), offset_guard_size: 0, } } @@ -226,7 +230,10 @@ mod tests { }) } fn vmmemory(&self) -> NonNull { - MaybeInstanceOwned::Host(Box::new(UnsafeCell::new(self.memory_definition))).as_ptr() + MaybeInstanceOwned::Host(Box::new(UnsafeCell::new( + self.memory_definition.unwrap().clone(), + ))) + .as_ptr() } fn try_clone(&self) -> Option> { None @@ -243,7 +250,7 @@ mod tests { impl Tunables for TinyTunables { fn memory_style(&self, _memory: &MemoryType) -> MemoryStyle { MemoryStyle::Static { - bound: Pages::from(1u32), + bound: Pages::from(18u32), offset_guard_size: 0, } } @@ -264,9 +271,14 @@ mod tests { &self, _ty: &MemoryType, _style: &MemoryStyle, - _vm_definition_location: NonNull, + vm_definition_location: NonNull, ) -> Result { let memory = VMTinyMemory::new().unwrap(); + // now, it's important to update vm_definition_location with the memory information! + let mut ptr = vm_definition_location; + let md = ptr.as_mut(); + md.base = memory.memory_definition.unwrap().base; + md.current_length = memory.memory_definition.unwrap().current_length; Ok(memory.into()) } @@ -295,13 +307,13 @@ mod tests { let vmmemory = tunables.create_host_memory( &MemoryType::new(1u32, Some(100u32), true), &MemoryStyle::Static { - bound: Pages::from(1u32), + bound: Pages::from(18u32), offset_guard_size: 0u64, }, ); let mut vmmemory = vmmemory.unwrap(); - assert!(vmmemory.grow(Pages::from(2u32)).is_err()); - assert_eq!(vmmemory.size(), Pages::from(1u32)); + assert!(vmmemory.grow(Pages::from(50u32)).is_err()); + assert_eq!(vmmemory.size(), Pages::from(18u32)); assert_eq!( vmmemory.grow(Pages::from(0u32)).err().unwrap(), MemoryError::CouldNotGrow { @@ -316,11 +328,19 @@ mod tests { use crate::{imports, wat2wasm, Instance, Memory, Module, Store}; use wasmer_compiler_cranelift::Cranelift; - let wasm_bytes = wat2wasm(br#"(module (memory 3) (export "memory" (memory 0)))"#)?; + let wasm_bytes = wat2wasm( + br#"(module + (memory (;0;) 18) + (global (;0;) (mut i32) i32.const 1048576) + (export "memory" (memory 0)) + (data (;0;) (i32.const 1048576) "*\00\00\00") + )"#, + )?; let compiler = Cranelift::default(); let tunables = TinyTunables {}; let mut store = Store::new_with_tunables(compiler, tunables); + //let mut store = Store::new(compiler); let module = Module::new(&store, wasm_bytes)?; let import_object = imports! {}; let instance = Instance::new(&mut store, &module, &import_object)?; @@ -333,7 +353,7 @@ mod tests { .collect(); assert_eq!(memories.len(), 1); let first_memory = memories.pop().unwrap(); - assert_eq!(first_memory.ty(&store).maximum.unwrap(), Pages(1)); + assert_eq!(first_memory.ty(&store).maximum.unwrap(), Pages(18)); Ok(()) } From e3920c477c2d722b6dfe8ba3ed118328c13de5d9 Mon Sep 17 00:00:00 2001 From: ptitSeb Date: Mon, 12 Sep 2022 11:04:34 +0200 Subject: [PATCH 05/11] Fixed the VMTinyMemory::vmmemory function in the TinyTunable test --- lib/api/src/sys/tunables.rs | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/lib/api/src/sys/tunables.rs b/lib/api/src/sys/tunables.rs index 95fddc8d8fb..f643c33cdb5 100644 --- a/lib/api/src/sys/tunables.rs +++ b/lib/api/src/sys/tunables.rs @@ -178,12 +178,12 @@ mod tests { use std::cell::UnsafeCell; use std::ptr::NonNull; use wasmer_types::{MemoryError, MemoryStyle, MemoryType, Pages, WASM_PAGE_SIZE}; - use wasmer_vm::{LinearMemory, MaybeInstanceOwned}; + use wasmer_vm::LinearMemory; #[derive(Debug)] struct VMTinyMemory { mem: Vec, - memory_definition: Option, + memory_definition: Option>, } unsafe impl Send for VMTinyMemory {} @@ -198,10 +198,10 @@ mod tests { mem: memory, memory_definition: None, }; - ret.memory_definition = Some(VMMemoryDefinition { + ret.memory_definition = Some(UnsafeCell::new(VMMemoryDefinition { base: ret.mem.as_ptr() as _, current_length: sz, - }); + })); Ok(ret) } } @@ -230,10 +230,17 @@ mod tests { }) } fn vmmemory(&self) -> NonNull { - MaybeInstanceOwned::Host(Box::new(UnsafeCell::new( - self.memory_definition.unwrap().clone(), - ))) - .as_ptr() + unsafe { + NonNull::new( + self.memory_definition + .as_ref() + .unwrap() + .get() + .as_mut() + .unwrap() as _, + ) + .unwrap() + } } fn try_clone(&self) -> Option> { None @@ -277,8 +284,10 @@ mod tests { // now, it's important to update vm_definition_location with the memory information! let mut ptr = vm_definition_location; let md = ptr.as_mut(); - md.base = memory.memory_definition.unwrap().base; - md.current_length = memory.memory_definition.unwrap().current_length; + let unsafecell = memory.memory_definition.as_ref().unwrap(); + let def = unsafecell.get().as_ref().unwrap(); + md.base = def.base; + md.current_length = def.current_length; Ok(memory.into()) } @@ -354,6 +363,9 @@ mod tests { assert_eq!(memories.len(), 1); let first_memory = memories.pop().unwrap(); assert_eq!(first_memory.ty(&store).maximum.unwrap(), Pages(18)); + let view = first_memory.view(&store); + let x = unsafe { view.data_unchecked_mut() }[0]; + assert_eq!(x, 0); Ok(()) } From d5811526417804a53858a7f9d2f98b6f53914aeb Mon Sep 17 00:00:00 2001 From: ptitSeb Date: Mon, 3 Oct 2022 11:38:14 +0200 Subject: [PATCH 06/11] Added an option to create a new instance without automatically initialize the memory --- lib/api/src/sys/instance.rs | 44 +++++++++++++++++++++++++++-- lib/api/src/sys/module.rs | 2 ++ lib/compiler/src/engine/artifact.rs | 3 +- lib/vm/src/instance/mod.rs | 5 +++- 4 files changed, 50 insertions(+), 4 deletions(-) diff --git a/lib/api/src/sys/instance.rs b/lib/api/src/sys/instance.rs index ab8e9d5c293..7388c3ff150 100644 --- a/lib/api/src/sys/instance.rs +++ b/lib/api/src/sys/instance.rs @@ -118,7 +118,47 @@ impl Instance { let imports = imports .imports_for_module(module) .map_err(InstantiationError::Link)?; - let mut handle = module.instantiate(store, &imports)?; + let mut handle = module.instantiate(store, &imports, true)?; + let exports = module + .exports() + .map(|export| { + let name = export.name().to_string(); + let export = handle.lookup(&name).expect("export"); + let extern_ = Extern::from_vm_extern(store, export); + (name, extern_) + }) + .collect::(); + + let instance = Self { + _handle: StoreHandle::new(store.objects_mut(), handle), + module: module.clone(), + exports, + }; + + Ok(instance) + } + + #[cfg(feature = "compiler")] + /// Creates a new `Instance` from a WebAssembly [`Module`] and a + /// set of imports using [`Imports`] or the [`imports`] macro helper. + /// The instance memory will not be initialized + /// + /// ## Errors + /// + /// The function can return [`InstantiationError`]s. + /// + /// Those are, as defined by the spec: + /// * Link errors that happen when plugging the imports into the instance + /// * Runtime errors that happen when running the module `start` function. + pub fn new_no_memory_init( + store: &mut impl AsStoreMut, + module: &Module, + imports: &Imports, + ) -> Result { + let imports = imports + .imports_for_module(module) + .map_err(InstantiationError::Link)?; + let mut handle = module.instantiate(store, &imports, false)?; let exports = module .exports() .map(|export| { @@ -155,7 +195,7 @@ impl Instance { externs: &[Extern], ) -> Result { let imports = externs.to_vec(); - let mut handle = module.instantiate(store, &imports)?; + let mut handle = module.instantiate(store, &imports, true)?; let exports = module .exports() .map(|export| { diff --git a/lib/api/src/sys/module.rs b/lib/api/src/sys/module.rs index 24e372663bb..f8befc124fe 100644 --- a/lib/api/src/sys/module.rs +++ b/lib/api/src/sys/module.rs @@ -343,6 +343,7 @@ impl Module { &self, store: &mut impl AsStoreMut, imports: &[crate::Extern], + initialize_memory: bool, ) -> Result { // Ensure all imports come from the same context. for import in imports { @@ -370,6 +371,7 @@ impl Module { self.artifact.finish_instantiation( store.as_store_ref().signal_handler(), &mut instance_handle, + initialize_memory, )?; Ok(instance_handle) diff --git a/lib/compiler/src/engine/artifact.rs b/lib/compiler/src/engine/artifact.rs index b436c2da8d6..5c907a5cfbc 100644 --- a/lib/compiler/src/engine/artifact.rs +++ b/lib/compiler/src/engine/artifact.rs @@ -398,6 +398,7 @@ impl Artifact { &self, trap_handler: Option<*const TrapHandlerFn<'static>>, handle: &mut InstanceHandle, + initialize_memory: bool, ) -> Result<(), InstantiationError> { let data_initializers = self .data_initializers() @@ -408,7 +409,7 @@ impl Artifact { }) .collect::>(); handle - .finish_instantiation(trap_handler, &data_initializers) + .finish_instantiation(trap_handler, &data_initializers, initialize_memory) .map_err(|trap| InstantiationError::Start(RuntimeError::from_trap(trap))) } diff --git a/lib/vm/src/instance/mod.rs b/lib/vm/src/instance/mod.rs index 0f189a75e75..c2c75b65a56 100644 --- a/lib/vm/src/instance/mod.rs +++ b/lib/vm/src/instance/mod.rs @@ -947,12 +947,15 @@ impl InstanceHandle { &mut self, trap_handler: Option<*const TrapHandlerFn<'static>>, data_initializers: &[DataInitializer<'_>], + initialize_memory: bool, ) -> Result<(), Trap> { let instance = self.instance_mut(); // Apply the initializers. initialize_tables(instance)?; - initialize_memories(instance, data_initializers)?; + if initialize_memory { + initialize_memories(instance, data_initializers)?; + } // The WebAssembly spec specifies that the start function is // invoked automatically at instantiation time. From c27547e41e0e6c3842f4c829ae3a9fda610d5556 Mon Sep 17 00:00:00 2001 From: ptitSeb Date: Tue, 4 Oct 2022 14:48:51 +0200 Subject: [PATCH 07/11] Removed Instance::new_no_memory_init, use custom LinearMemory with custom memory_init method to get the same result --- lib/api/src/sys/instance.rs | 44 ++---------------------- lib/api/src/sys/module.rs | 2 -- lib/api/src/sys/tunables.rs | 7 ++++ lib/compiler/src/engine/artifact.rs | 3 +- lib/vm/src/instance/mod.rs | 53 +++++++++++++---------------- lib/vm/src/memory.rs | 20 +++++++++++ 6 files changed, 53 insertions(+), 76 deletions(-) diff --git a/lib/api/src/sys/instance.rs b/lib/api/src/sys/instance.rs index 7388c3ff150..ab8e9d5c293 100644 --- a/lib/api/src/sys/instance.rs +++ b/lib/api/src/sys/instance.rs @@ -118,47 +118,7 @@ impl Instance { let imports = imports .imports_for_module(module) .map_err(InstantiationError::Link)?; - let mut handle = module.instantiate(store, &imports, true)?; - let exports = module - .exports() - .map(|export| { - let name = export.name().to_string(); - let export = handle.lookup(&name).expect("export"); - let extern_ = Extern::from_vm_extern(store, export); - (name, extern_) - }) - .collect::(); - - let instance = Self { - _handle: StoreHandle::new(store.objects_mut(), handle), - module: module.clone(), - exports, - }; - - Ok(instance) - } - - #[cfg(feature = "compiler")] - /// Creates a new `Instance` from a WebAssembly [`Module`] and a - /// set of imports using [`Imports`] or the [`imports`] macro helper. - /// The instance memory will not be initialized - /// - /// ## Errors - /// - /// The function can return [`InstantiationError`]s. - /// - /// Those are, as defined by the spec: - /// * Link errors that happen when plugging the imports into the instance - /// * Runtime errors that happen when running the module `start` function. - pub fn new_no_memory_init( - store: &mut impl AsStoreMut, - module: &Module, - imports: &Imports, - ) -> Result { - let imports = imports - .imports_for_module(module) - .map_err(InstantiationError::Link)?; - let mut handle = module.instantiate(store, &imports, false)?; + let mut handle = module.instantiate(store, &imports)?; let exports = module .exports() .map(|export| { @@ -195,7 +155,7 @@ impl Instance { externs: &[Extern], ) -> Result { let imports = externs.to_vec(); - let mut handle = module.instantiate(store, &imports, true)?; + let mut handle = module.instantiate(store, &imports)?; let exports = module .exports() .map(|export| { diff --git a/lib/api/src/sys/module.rs b/lib/api/src/sys/module.rs index f8befc124fe..24e372663bb 100644 --- a/lib/api/src/sys/module.rs +++ b/lib/api/src/sys/module.rs @@ -343,7 +343,6 @@ impl Module { &self, store: &mut impl AsStoreMut, imports: &[crate::Extern], - initialize_memory: bool, ) -> Result { // Ensure all imports come from the same context. for import in imports { @@ -371,7 +370,6 @@ impl Module { self.artifact.finish_instantiation( store.as_store_ref().signal_handler(), &mut instance_handle, - initialize_memory, )?; Ok(instance_handle) diff --git a/lib/api/src/sys/tunables.rs b/lib/api/src/sys/tunables.rs index f643c33cdb5..5bcebad7105 100644 --- a/lib/api/src/sys/tunables.rs +++ b/lib/api/src/sys/tunables.rs @@ -245,6 +245,13 @@ mod tests { fn try_clone(&self) -> Option> { None } + /* + // this code allow custom memory to be ignoring init_memory + use wasmer_vm::Trap; + unsafe fn initialize_with_data(&self, _start: usize, _data: &[u8]) -> Result<(), Trap> { + Ok(()) + } + */ } impl From for wasmer_vm::VMMemory { diff --git a/lib/compiler/src/engine/artifact.rs b/lib/compiler/src/engine/artifact.rs index 5c907a5cfbc..b436c2da8d6 100644 --- a/lib/compiler/src/engine/artifact.rs +++ b/lib/compiler/src/engine/artifact.rs @@ -398,7 +398,6 @@ impl Artifact { &self, trap_handler: Option<*const TrapHandlerFn<'static>>, handle: &mut InstanceHandle, - initialize_memory: bool, ) -> Result<(), InstantiationError> { let data_initializers = self .data_initializers() @@ -409,7 +408,7 @@ impl Artifact { }) .collect::>(); handle - .finish_instantiation(trap_handler, &data_initializers, initialize_memory) + .finish_instantiation(trap_handler, &data_initializers) .map_err(|trap| InstantiationError::Start(RuntimeError::from_trap(trap))) } diff --git a/lib/vm/src/instance/mod.rs b/lib/vm/src/instance/mod.rs index c2c75b65a56..91e1e6f0bb1 100644 --- a/lib/vm/src/instance/mod.rs +++ b/lib/vm/src/instance/mod.rs @@ -206,6 +206,7 @@ impl Instance { unsafe { self.vmctx_plus_offset(self.offsets.vmctx_tables_begin()) } } + #[allow(dead_code)] /// Get a locally defined or imported memory. fn get_memory(&self, index: MemoryIndex) -> VMMemoryDefinition { if let Some(local_index) = self.module.local_memory_index(index) { @@ -240,6 +241,21 @@ impl Instance { unsafe { self.vmctx_plus_offset(self.offsets.vmctx_memories_begin()) } } + /// Get a locally defined or imported memory. + fn get_vmmemory(&self, index: MemoryIndex) -> &VMMemory { + if let Some(local_index) = self.module.local_memory_index(index) { + unsafe { + self.memories + .get(local_index) + .unwrap() + .get(self.context.as_ref().unwrap()) + } + } else { + let import = self.imported_memory(index); + unsafe { import.handle.get(self.context.as_ref().unwrap()) } + } + } + /// Return the indexed `VMGlobalDefinition`. fn global(&self, index: LocalGlobalIndex) -> VMGlobalDefinition { unsafe { self.global_ptr(index).as_ref().clone() } @@ -701,29 +717,18 @@ impl Instance { ) -> Result<(), Trap> { // https://webassembly.github.io/bulk-memory-operations/core/exec/instructions.html#exec-memory-init - let memory = self.get_memory(memory_index); + let memory = self.get_vmmemory(memory_index); let passive_data = self.passive_data.borrow(); let data = passive_data.get(&data_index).map_or(&[][..], |d| &**d); if src .checked_add(len) - .map_or(true, |n| n as usize > data.len()) - || dst.checked_add(len).map_or(true, |m| { - usize::try_from(m).unwrap() > memory.current_length - }) + .map_or(true, |end| end as usize > data.len()) { return Err(Trap::lib(TrapCode::HeapAccessOutOfBounds)); } - let src_slice = &data[src as usize..(src + len) as usize]; - - unsafe { - let dst_start = memory.base.add(dst as usize); - let dst_slice = slice::from_raw_parts_mut(dst_start, len as usize); - dst_slice.copy_from_slice(src_slice); - } - - Ok(()) + unsafe { memory.initialize_with_data(dst as usize, src_slice) } } /// Drop the given data segment, truncating its length to zero. @@ -947,15 +952,12 @@ impl InstanceHandle { &mut self, trap_handler: Option<*const TrapHandlerFn<'static>>, data_initializers: &[DataInitializer<'_>], - initialize_memory: bool, ) -> Result<(), Trap> { let instance = self.instance_mut(); // Apply the initializers. initialize_tables(instance)?; - if initialize_memory { - initialize_memories(instance, data_initializers)?; - } + initialize_memories(instance, data_initializers)?; // The WebAssembly spec specifies that the start function is // invoked automatically at instantiation time. @@ -1150,6 +1152,7 @@ fn get_memory_init_start(init: &DataInitializer<'_>, instance: &Instance) -> usi } #[allow(clippy::mut_from_ref)] +#[allow(dead_code)] /// Return a byte-slice view of a memory's data. unsafe fn get_memory_slice<'instance>( init: &DataInitializer<'_>, @@ -1245,21 +1248,11 @@ fn initialize_memories( data_initializers: &[DataInitializer<'_>], ) -> Result<(), Trap> { for init in data_initializers { - let memory = instance.get_memory(init.location.memory_index); + let memory = instance.get_vmmemory(init.location.memory_index); let start = get_memory_init_start(init, instance); - if start - .checked_add(init.data.len()) - .map_or(true, |end| end > memory.current_length) - { - return Err(Trap::lib(TrapCode::HeapAccessOutOfBounds)); - } - unsafe { - let mem_slice = get_memory_slice(init, instance); - let end = start + init.data.len(); - let to_init = &mut mem_slice[start..end]; - to_init.copy_from_slice(init.data); + memory.initialize_with_data(start, init.data)?; } } diff --git a/lib/vm/src/memory.rs b/lib/vm/src/memory.rs index f071365d19f..7c1e1cdb8a7 100644 --- a/lib/vm/src/memory.rs +++ b/lib/vm/src/memory.rs @@ -5,11 +5,13 @@ //! //! `Memory` is to WebAssembly linear memories what `Table` is to WebAssembly tables. +use crate::trap::{Trap, TrapCode}; use crate::{mmap::Mmap, store::MaybeInstanceOwned, vmcontext::VMMemoryDefinition}; use more_asserts::assert_ge; use std::cell::UnsafeCell; use std::convert::TryInto; use std::ptr::NonNull; +use std::slice; use wasmer_types::{Bytes, MemoryError, MemoryStyle, MemoryType, Pages}; // The memory mapped area @@ -410,4 +412,22 @@ where /// Attempts to clone this memory (if its clonable) fn try_clone(&self) -> Option>; + + #[doc(hidden)] + unsafe fn initialize_with_data(&self, start: usize, data: &[u8]) -> Result<(), Trap> { + let memory = self.vmmemory().as_ref(); + if start + .checked_add(data.len()) + .map_or(true, |end| end > memory.current_length) + { + return Err(Trap::lib(TrapCode::HeapAccessOutOfBounds)); + } + + let mem_slice = slice::from_raw_parts_mut(memory.base, memory.current_length); + let end = start + data.len(); + let to_init = &mut mem_slice[start..end]; + to_init.copy_from_slice(data); + + Ok(()) + } } From 57652b0585481f5674b3aafc8ceb0ea4aaa5c7dd Mon Sep 17 00:00:00 2001 From: ptitSeb Date: Wed, 5 Oct 2022 14:35:40 +0200 Subject: [PATCH 08/11] Fixed initialize_with_data not always called, and made the default memory data initializer available outside of the crate --- lib/vm/src/memory.rs | 40 ++++++++++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/lib/vm/src/memory.rs b/lib/vm/src/memory.rs index 7c1e1cdb8a7..580eb7f0e18 100644 --- a/lib/vm/src/memory.rs +++ b/lib/vm/src/memory.rs @@ -344,6 +344,11 @@ impl LinearMemory for VMMemory { fn try_clone(&self) -> Option> { self.0.try_clone() } + + /// Initialize memory with data + unsafe fn initialize_with_data(&self, start: usize, data: &[u8]) -> Result<(), Trap> { + self.0.initialize_with_data(start, data) + } } impl VMMemory { @@ -387,6 +392,28 @@ impl VMMemory { } } +#[doc(hidden)] +/// Default implementation to initialize memory with data +pub unsafe fn initialize_memory_with_data( + memory: &VMMemoryDefinition, + start: usize, + data: &[u8], +) -> Result<(), Trap> { + if start + .checked_add(data.len()) + .map_or(true, |end| end > memory.current_length) + { + return Err(Trap::lib(TrapCode::HeapAccessOutOfBounds)); + } + + let mem_slice = slice::from_raw_parts_mut(memory.base, memory.current_length); + let end = start + data.len(); + let to_init = &mut mem_slice[start..end]; + to_init.copy_from_slice(data); + + Ok(()) +} + /// Represents memory that is used by the WebAsssembly module pub trait LinearMemory where @@ -416,18 +443,7 @@ where #[doc(hidden)] unsafe fn initialize_with_data(&self, start: usize, data: &[u8]) -> Result<(), Trap> { let memory = self.vmmemory().as_ref(); - if start - .checked_add(data.len()) - .map_or(true, |end| end > memory.current_length) - { - return Err(Trap::lib(TrapCode::HeapAccessOutOfBounds)); - } - - let mem_slice = slice::from_raw_parts_mut(memory.base, memory.current_length); - let end = start + data.len(); - let to_init = &mut mem_slice[start..end]; - to_init.copy_from_slice(data); - Ok(()) + initialize_memory_with_data(memory, start, data) } } From d12897d647afb72f302d2cd9590df0d455f4e093 Mon Sep 17 00:00:00 2001 From: ptitSeb Date: Wed, 5 Oct 2022 15:14:43 +0200 Subject: [PATCH 09/11] Added public initialize_memory_with_data function --- lib/vm/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/vm/src/lib.rs b/lib/vm/src/lib.rs index 19982e54fcc..1aaae7b52b8 100644 --- a/lib/vm/src/lib.rs +++ b/lib/vm/src/lib.rs @@ -45,7 +45,7 @@ pub use crate::function_env::VMFunctionEnvironment; pub use crate::global::*; pub use crate::imports::Imports; pub use crate::instance::{InstanceAllocator, InstanceHandle}; -pub use crate::memory::{LinearMemory, VMMemory}; +pub use crate::memory::{initialize_memory_with_data, LinearMemory, VMMemory}; pub use crate::mmap::Mmap; pub use crate::probestack::PROBESTACK; pub use crate::sig_registry::SignatureRegistry; From 50eed71ab0c9db6dd5e6ac0755c22a2a042f595f Mon Sep 17 00:00:00 2001 From: ptitSeb Date: Thu, 6 Oct 2022 09:51:11 +0200 Subject: [PATCH 10/11] Last changes to enforce size check on ther caller of memory_data_init and not the function itself --- lib/vm/src/instance/mod.rs | 6 ++++++ lib/vm/src/memory.rs | 12 ++++-------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/lib/vm/src/instance/mod.rs b/lib/vm/src/instance/mod.rs index 91e1e6f0bb1..943fa9a1cf1 100644 --- a/lib/vm/src/instance/mod.rs +++ b/lib/vm/src/instance/mod.rs @@ -1252,6 +1252,12 @@ fn initialize_memories( let start = get_memory_init_start(init, instance); unsafe { + if start + .checked_add(init.data.len()) + .map_or(true, |end| end > memory.vmmemory().as_ref().current_length) + { + return Err(Trap::lib(TrapCode::HeapAccessOutOfBounds)); + } memory.initialize_with_data(start, init.data)?; } } diff --git a/lib/vm/src/memory.rs b/lib/vm/src/memory.rs index 580eb7f0e18..f8b80d2cee4 100644 --- a/lib/vm/src/memory.rs +++ b/lib/vm/src/memory.rs @@ -5,7 +5,7 @@ //! //! `Memory` is to WebAssembly linear memories what `Table` is to WebAssembly tables. -use crate::trap::{Trap, TrapCode}; +use crate::trap::Trap; use crate::{mmap::Mmap, store::MaybeInstanceOwned, vmcontext::VMMemoryDefinition}; use more_asserts::assert_ge; use std::cell::UnsafeCell; @@ -399,13 +399,6 @@ pub unsafe fn initialize_memory_with_data( start: usize, data: &[u8], ) -> Result<(), Trap> { - if start - .checked_add(data.len()) - .map_or(true, |end| end > memory.current_length) - { - return Err(Trap::lib(TrapCode::HeapAccessOutOfBounds)); - } - let mem_slice = slice::from_raw_parts_mut(memory.base, memory.current_length); let end = start + data.len(); let to_init = &mut mem_slice[start..end]; @@ -441,6 +434,9 @@ where fn try_clone(&self) -> Option>; #[doc(hidden)] + /// # Safety + /// This function is unsafe because WebAssembly specification requires that data is always set at initialization time. + /// It should be the implementors responsibility to make sure this respects the spec unsafe fn initialize_with_data(&self, start: usize, data: &[u8]) -> Result<(), Trap> { let memory = self.vmmemory().as_ref(); From 6b51a64faed8a3d628e0f467661f1de0b0c2a95d Mon Sep 17 00:00:00 2001 From: ptitSeb Date: Thu, 6 Oct 2022 11:04:42 +0200 Subject: [PATCH 11/11] Forgot to add back a failsafe in memory_init --- lib/vm/src/instance/mod.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/vm/src/instance/mod.rs b/lib/vm/src/instance/mod.rs index 943fa9a1cf1..d6b6e2341cd 100644 --- a/lib/vm/src/instance/mod.rs +++ b/lib/vm/src/instance/mod.rs @@ -721,9 +721,13 @@ impl Instance { let passive_data = self.passive_data.borrow(); let data = passive_data.get(&data_index).map_or(&[][..], |d| &**d); + let current_length = unsafe { memory.vmmemory().as_ref().current_length }; if src .checked_add(len) - .map_or(true, |end| end as usize > data.len()) + .map_or(true, |n| n as usize > data.len()) + || dst + .checked_add(len) + .map_or(true, |m| usize::try_from(m).unwrap() > current_length) { return Err(Trap::lib(TrapCode::HeapAccessOutOfBounds)); } @@ -1252,9 +1256,10 @@ fn initialize_memories( let start = get_memory_init_start(init, instance); unsafe { + let current_length = memory.vmmemory().as_ref().current_length; if start .checked_add(init.data.len()) - .map_or(true, |end| end > memory.vmmemory().as_ref().current_length) + .map_or(true, |end| end > current_length) { return Err(Trap::lib(TrapCode::HeapAccessOutOfBounds)); }