From 837c1c18c129c81e2a0af70043abeb6fede9dfd2 Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Fri, 12 Mar 2021 04:26:50 +0100 Subject: [PATCH 1/5] Passive elements and data are now PrimaryMaps --- lib/compiler/src/translator/environ.rs | 29 ++++++----------- lib/compiler/src/translator/sections.rs | 1 + lib/vm/src/instance/mod.rs | 42 +++++++++++-------------- lib/vm/src/module.rs | 10 +++--- 4 files changed, 35 insertions(+), 47 deletions(-) diff --git a/lib/compiler/src/translator/environ.rs b/lib/compiler/src/translator/environ.rs index ffd2f0a56d9..899f71ba138 100644 --- a/lib/compiler/src/translator/environ.rs +++ b/lib/compiler/src/translator/environ.rs @@ -331,6 +331,14 @@ impl<'data> ModuleEnvironment<'data> { Ok(()) } + pub(crate) fn reserve_passive_elements(&mut self, num: u32) -> WasmResult<()> { + self.result + .module + .passive_elements + .reserve_exact(usize::try_from(num).unwrap()); + Ok(()) + } + pub(crate) fn declare_table_initializers( &mut self, table_index: TableIndex, @@ -355,16 +363,7 @@ impl<'data> ModuleEnvironment<'data> { elem_index: ElemIndex, segments: Box<[FunctionIndex]>, ) -> WasmResult<()> { - let old = self - .result - .module - .passive_elements - .insert(elem_index, segments); - debug_assert!( - old.is_none(), - "should never get duplicate element indices, that would be a bug in `wasmer_compiler`'s \ - translation" - ); + let old = self.result.module.passive_elements[elem_index] = segments; Ok(()) } @@ -416,15 +415,7 @@ impl<'data> ModuleEnvironment<'data> { data_index: DataIndex, data: &'data [u8], ) -> WasmResult<()> { - let old = self - .result - .module - .passive_data - .insert(data_index, Arc::from(data)); - debug_assert!( - old.is_none(), - "a module can't have duplicate indices, this would be a wasmer-compiler bug" - ); + let old = self.result.module.passive_data[data_index] = Arc::from(data); Ok(()) } diff --git a/lib/compiler/src/translator/sections.rs b/lib/compiler/src/translator/sections.rs index 3dde2e8cc89..f3373b7f739 100644 --- a/lib/compiler/src/translator/sections.rs +++ b/lib/compiler/src/translator/sections.rs @@ -332,6 +332,7 @@ pub fn parse_element_section<'data>( environ: &mut ModuleEnvironment, ) -> WasmResult<()> { environ.reserve_table_initializers(elements.get_count())?; + environ.reserve_passive_elements(elements.get_count())?; for (index, entry) in elements.into_iter().enumerate() { let Element { kind, items, ty } = entry?; diff --git a/lib/vm/src/instance/mod.rs b/lib/vm/src/instance/mod.rs index fb9b6874f79..5e85d7214d0 100644 --- a/lib/vm/src/instance/mod.rs +++ b/lib/vm/src/instance/mod.rs @@ -82,11 +82,11 @@ pub(crate) struct Instance { /// Passive elements in this instantiation. As `elem.drop`s happen, these /// entries get removed. A missing entry is considered equivalent to an /// empty slice. - passive_elements: RefCell>>, + passive_elements: RefCell>>, /// Passive data segments from our module. As `data.drop`s happen, entries /// get removed. A missing entry is considered equivalent to an empty slice. - passive_data: RefCell>>, + passive_data: RefCell>>, /// Hosts can store arbitrary per-instance information here. host_state: Box, @@ -600,7 +600,7 @@ impl Instance { let table = self.get_table(table_index); let passive_elements = self.passive_elements.borrow(); let elem = passive_elements - .get(&elem_index) + .get(elem_index) .map_or_else(|| -> &[VMCallerCheckedAnyfunc] { &[] }, |e| &**e); if src @@ -625,7 +625,7 @@ impl Instance { // https://webassembly.github.io/reference-types/core/exec/instructions.html#exec-elem-drop let mut passive_elements = self.passive_elements.borrow_mut(); - passive_elements.remove(&elem_index); + passive_elements[elem_index] = Box::new([]); // Note that we don't check that we actually removed an element because // dropping a non-passive element is a no-op (not a trap). } @@ -718,9 +718,7 @@ impl Instance { let memory = self.get_memory(memory_index); let passive_data = self.passive_data.borrow(); - let data = passive_data - .get(&data_index) - .map_or(&[][..], |data| &**data); + let data = passive_data.get(data_index).map_or(&[][..], |data| &**data); if src .checked_add(len) @@ -746,7 +744,7 @@ impl Instance { /// Drop the given data segment, truncating its length to zero. pub(crate) fn data_drop(&self, data_index: DataIndex) { let mut passive_data = self.passive_data.borrow_mut(); - passive_data.remove(&data_index); + passive_data[data_index] = Arc::new([]); } /// Get a table by index regardless of whether it is locally-defined or an @@ -1308,22 +1306,20 @@ fn initialize_passive_elements(instance: &Instance) { "should only be called once, at initialization time" ); - passive_elements.extend( - instance - .module - .passive_elements + for (segments, passive_element) in instance + .module + .passive_elements + .values() + .zip(passive_elements.values_mut()) + { + if segments.is_empty() { + continue; + } + *passive_element = segments .iter() - .filter(|(_, segments)| !segments.is_empty()) - .map(|(idx, segments)| { - ( - *idx, - segments - .iter() - .map(|s| instance.get_caller_checked_anyfunc(*s)) - .collect(), - ) - }), - ); + .map(|s| instance.get_caller_checked_anyfunc(*s)) + .collect(); + } } /// Initialize the table memory from the provided initializers. diff --git a/lib/vm/src/module.rs b/lib/vm/src/module.rs index 79bb1b41829..82d951f46c2 100644 --- a/lib/vm/src/module.rs +++ b/lib/vm/src/module.rs @@ -70,10 +70,10 @@ pub struct ModuleInfo { pub table_initializers: Vec, /// WebAssembly passive elements. - pub passive_elements: HashMap>, + pub passive_elements: PrimaryMap>, /// WebAssembly passive data segments. - pub passive_data: HashMap>, + pub passive_data: PrimaryMap>, /// WebAssembly global initializers. pub global_initializers: PrimaryMap, @@ -125,8 +125,8 @@ impl ModuleInfo { exports: IndexMap::new(), start_function: None, table_initializers: Vec::new(), - passive_elements: HashMap::new(), - passive_data: HashMap::new(), + passive_elements: PrimaryMap::new(), + passive_data: PrimaryMap::new(), global_initializers: PrimaryMap::new(), function_names: HashMap::new(), signatures: PrimaryMap::new(), @@ -145,7 +145,7 @@ impl ModuleInfo { /// Get the given passive element, if it exists. pub fn get_passive_element(&self, index: ElemIndex) -> Option<&[FunctionIndex]> { - self.passive_elements.get(&index).map(|es| &**es) + self.passive_elements.get(index).map(|es| &**es) } /// Get the exported signatures of the module From 9c45cd99503d2dc5343f3602679352f69af48ff5 Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Fri, 12 Mar 2021 15:57:53 +0100 Subject: [PATCH 2/5] Remove unused import --- lib/vm/src/instance/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/vm/src/instance/mod.rs b/lib/vm/src/instance/mod.rs index 5e85d7214d0..2002989b094 100644 --- a/lib/vm/src/instance/mod.rs +++ b/lib/vm/src/instance/mod.rs @@ -31,7 +31,6 @@ use memoffset::offset_of; use more_asserts::assert_lt; use std::any::Any; use std::cell::{Cell, RefCell}; -use std::collections::HashMap; use std::convert::{TryFrom, TryInto}; use std::ffi; use std::fmt; From cdf9310186e32106dfed16f48487cbb8cd4152b6 Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Fri, 12 Mar 2021 16:32:06 +0100 Subject: [PATCH 3/5] Apply suggestions --- lib/vm/src/instance/mod.rs | 42 +++++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/lib/vm/src/instance/mod.rs b/lib/vm/src/instance/mod.rs index 2002989b094..7f89f738c66 100644 --- a/lib/vm/src/instance/mod.rs +++ b/lib/vm/src/instance/mod.rs @@ -79,13 +79,12 @@ pub(crate) struct Instance { function_call_trampolines: BoxedSlice, /// Passive elements in this instantiation. As `elem.drop`s happen, these - /// entries get removed. A missing entry is considered equivalent to an - /// empty slice. - passive_elements: RefCell>>, + /// entries get removed. + passive_elements: RefCell>>>, /// Passive data segments from our module. As `data.drop`s happen, entries /// get removed. A missing entry is considered equivalent to an empty slice. - passive_data: RefCell>>, + passive_data: RefCell>>>, /// Hosts can store arbitrary per-instance information here. host_state: Box, @@ -598,9 +597,16 @@ impl Instance { let table = self.get_table(table_index); let passive_elements = self.passive_elements.borrow(); - let elem = passive_elements - .get(elem_index) - .map_or_else(|| -> &[VMCallerCheckedAnyfunc] { &[] }, |e| &**e); + let elem = passive_elements.get(elem_index).map_or_else( + || -> &[VMCallerCheckedAnyfunc] { &[] }, + |e| { + if let Some(e) = e { + &**e + } else { + &[] + } + }, + ); if src .checked_add(len) @@ -624,7 +630,7 @@ impl Instance { // https://webassembly.github.io/reference-types/core/exec/instructions.html#exec-elem-drop let mut passive_elements = self.passive_elements.borrow_mut(); - passive_elements[elem_index] = Box::new([]); + passive_elements[elem_index] = None; // Note that we don't check that we actually removed an element because // dropping a non-passive element is a no-op (not a trap). } @@ -717,7 +723,13 @@ impl Instance { let memory = self.get_memory(memory_index); let passive_data = self.passive_data.borrow(); - let data = passive_data.get(data_index).map_or(&[][..], |data| &**data); + let data = passive_data.get(data_index).map_or(&[][..], |data| { + if let Some(data) = data { + &**data + } else { + &[][..] + } + }); if src .checked_add(len) @@ -743,7 +755,7 @@ impl Instance { /// Drop the given data segment, truncating its length to zero. pub(crate) fn data_drop(&self, data_index: DataIndex) { let mut passive_data = self.passive_data.borrow_mut(); - passive_data[data_index] = Arc::new([]); + passive_data[data_index] = None; } /// Get a table by index regardless of whether it is locally-defined or an @@ -820,7 +832,13 @@ impl InstanceHandle { .map(|m| m.vmglobal()) .collect::>() .into_boxed_slice(); - let passive_data = RefCell::new(module.passive_data.clone()); + let passive_data = RefCell::new( + module + .passive_data + .values() + .map(|data| Some(data.clone())) + .collect(), + ); let handle = { let offsets = allocator.offsets().clone(); @@ -1316,7 +1334,7 @@ fn initialize_passive_elements(instance: &Instance) { } *passive_element = segments .iter() - .map(|s| instance.get_caller_checked_anyfunc(*s)) + .map(|s| Some(instance.get_caller_checked_anyfunc(*s))) .collect(); } } From 0270058a30bc663d208dab10263f8b473bb3e4d4 Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Fri, 12 Mar 2021 16:33:51 +0100 Subject: [PATCH 4/5] Fixed lint issues --- lib/compiler/src/translator/environ.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/compiler/src/translator/environ.rs b/lib/compiler/src/translator/environ.rs index 899f71ba138..156a3d65a7b 100644 --- a/lib/compiler/src/translator/environ.rs +++ b/lib/compiler/src/translator/environ.rs @@ -363,7 +363,7 @@ impl<'data> ModuleEnvironment<'data> { elem_index: ElemIndex, segments: Box<[FunctionIndex]>, ) -> WasmResult<()> { - let old = self.result.module.passive_elements[elem_index] = segments; + self.result.module.passive_elements[elem_index] = segments; Ok(()) } @@ -415,7 +415,7 @@ impl<'data> ModuleEnvironment<'data> { data_index: DataIndex, data: &'data [u8], ) -> WasmResult<()> { - let old = self.result.module.passive_data[data_index] = Arc::from(data); + self.result.module.passive_data[data_index] = Arc::from(data); Ok(()) } From 0123603152129b448be2cd4f8873dbea2230500d Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Fri, 12 Mar 2021 17:08:09 +0100 Subject: [PATCH 5/5] Simplified get logic --- lib/vm/src/instance/mod.rs | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/lib/vm/src/instance/mod.rs b/lib/vm/src/instance/mod.rs index 7f89f738c66..77849e3b2a0 100644 --- a/lib/vm/src/instance/mod.rs +++ b/lib/vm/src/instance/mod.rs @@ -597,16 +597,10 @@ impl Instance { let table = self.get_table(table_index); let passive_elements = self.passive_elements.borrow(); - let elem = passive_elements.get(elem_index).map_or_else( - || -> &[VMCallerCheckedAnyfunc] { &[] }, - |e| { - if let Some(e) = e { - &**e - } else { - &[] - } - }, - ); + let elem = passive_elements + .get(elem_index) + .and_then(|e| e.as_ref().map(|e| &**e)) + .unwrap_or(&[]); if src .checked_add(len) @@ -723,13 +717,10 @@ impl Instance { let memory = self.get_memory(memory_index); let passive_data = self.passive_data.borrow(); - let data = passive_data.get(data_index).map_or(&[][..], |data| { - if let Some(data) = data { - &**data - } else { - &[][..] - } - }); + let data = passive_data + .get(data_index) + .and_then(|data| data.as_ref().map(|d| &**d)) + .unwrap_or(&[][..]); if src .checked_add(len)