Skip to content

Commit

Permalink
Fix table.init from imported global.get values (#1192)
Browse files Browse the repository at this point in the history
* update Wasm spec testsuite

* fix table.init with global.get elements

* pre-initialize element segment items during instantiation

* inline unused ElementSegmentItems type
  • Loading branch information
Robbepop authored Sep 24, 2024
1 parent 0602517 commit 1d98b71
Show file tree
Hide file tree
Showing 8 changed files with 149 additions and 176 deletions.
16 changes: 2 additions & 14 deletions crates/wasmi/src/engine/executor/instrs/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -410,23 +410,11 @@ impl<'engine> Executor<'engine> {
) -> Result<(), Error> {
let table_index = self.fetch_table_index(1);
let element_index = self.fetch_element_segment_index(2);
let (instance, table, element, fuel) = store.resolve_table_init_params(
self.stack.calls.instance_expect(),
let (table, element, fuel) = store.resolve_table_init_params(
&self.get_table(table_index),
&self.get_element_segment(element_index),
);
table.init(
dst_index,
element,
src_index,
len,
Some(fuel),
|func_index| {
instance
.get_func(func_index)
.unwrap_or_else(|| panic!("missing function at index {func_index}"))
},
)?;
table.init(dst_index, element, src_index, len, Some(fuel))?;
self.try_next_instr_at(3)
}

Expand Down
65 changes: 20 additions & 45 deletions crates/wasmi/src/module/element.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::{ConstExpr, TableIdx};
use crate::{core::ValType, module::utils::WasmiValueType};
use std::sync::Arc;
use std::boxed::Box;

/// A table element segment within a [`Module`].
///
Expand All @@ -12,47 +12,7 @@ pub struct ElementSegment {
/// The type of elements of the [`ElementSegment`].
ty: ValType,
/// The items of the [`ElementSegment`].
items: ElementSegmentItems,
}

/// The items of an [`ElementSegment`].
#[derive(Debug, Clone)]
pub struct ElementSegmentItems {
exprs: Arc<[ConstExpr]>,
}

impl ElementSegmentItems {
/// Creates new [`ElementSegmentItems`] from the given [`wasmparser::ElementItems`].
///
/// # Panics
///
/// If the given [`wasmparser::ElementItems`] is invalid.
fn new(items: &wasmparser::ElementItems) -> Self {
let exprs = match items {
wasmparser::ElementItems::Functions(items) => items
.clone()
.into_iter()
.map(|item| {
item.unwrap_or_else(|error| panic!("failed to parse element item: {error}"))
})
.map(ConstExpr::new_funcref)
.collect::<Arc<[_]>>(),
wasmparser::ElementItems::Expressions(items) => items
.clone()
.into_iter()
.map(|item| {
item.unwrap_or_else(|error| panic!("failed to parse element item: {error}"))
})
.map(ConstExpr::new)
.collect::<Arc<[_]>>(),
};
Self { exprs }
}

/// Returns a shared reference to the items of the [`ElementSegmentItems`].
pub fn items(&self) -> &[ConstExpr] {
&self.exprs
}
items: Box<[ConstExpr]>,
}

/// The kind of a Wasm [`ElementSegment`].
Expand Down Expand Up @@ -116,7 +76,22 @@ impl From<wasmparser::Element<'_>> for ElementSegment {
);
let kind = ElementSegmentKind::from(element.kind);
let ty = WasmiValueType::from(element.ty).into_inner();
let items = ElementSegmentItems::new(&element.items);
let items = match element.items {
wasmparser::ElementItems::Functions(items) => items
.into_iter()
.map(|item| {
item.unwrap_or_else(|error| panic!("failed to parse element item: {error}"))
})
.map(ConstExpr::new_funcref)
.collect::<Box<[_]>>(),
wasmparser::ElementItems::Expressions(items) => items
.into_iter()
.map(|item| {
item.unwrap_or_else(|error| panic!("failed to parse element item: {error}"))
})
.map(ConstExpr::new)
.collect::<Box<[_]>>(),
};
Self { kind, ty, items }
}
}
Expand All @@ -133,7 +108,7 @@ impl ElementSegment {
}

/// Returns the element items of the [`ElementSegment`].
pub fn items_cloned(&self) -> ElementSegmentItems {
self.items.clone()
pub fn items(&self) -> &[ConstExpr] {
&self.items[..]
}
}
24 changes: 11 additions & 13 deletions crates/wasmi/src/module/instantiate/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,10 @@ impl Module {
builder: &mut InstanceEntityBuilder,
) -> Result<(), Error> {
for segment in &self.module_header().element_segments[..] {
let element = ElementSegment::new(context.as_context_mut(), segment);
let get_global = |index| builder.get_global(index);
let get_func = |index| builder.get_func(index);
let element =
ElementSegment::new(context.as_context_mut(), segment, get_func, get_global);
if let ElementSegmentKind::Active(active) = segment.kind() {
let dst_index = u32::from(Self::eval_init_expr(
context.as_context(),
Expand All @@ -308,19 +311,14 @@ impl Module {
offset: dst_index,
amount: len_items,
})?;
// Finally do the actual initialization of the table elements.
{
let (table, element) = context
.as_context_mut()
.store
.inner
.resolve_table_element(&table, &element);
table.init(dst_index, element, 0, len_items, None, |func_index| {
builder.get_func(func_index)
})?;
}
let (table, elem) = context
.as_context_mut()
.store
.inner
.resolve_table_and_element_mut(&table, &element);
table.init_untyped(dst_index, elem.items(), None)?;
// Now drop the active element segment as commanded by the Wasm spec.
element.drop_items(&mut context);
elem.drop_items();
}
builder.push_element_segment(element);
}
Expand Down
2 changes: 1 addition & 1 deletion crates/wasmi/src/module/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ pub use self::{
};
pub(crate) use self::{
data::{DataSegment, DataSegments, InitDataSegment, PassiveDataSegmentBytes},
element::{ElementSegment, ElementSegmentItems, ElementSegmentKind},
element::{ElementSegment, ElementSegmentKind},
init_expr::ConstExpr,
utils::WasmiValueType,
};
Expand Down
60 changes: 22 additions & 38 deletions crates/wasmi/src/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,26 @@ impl StoreInner {
Self::resolve_mut(idx, &mut self.tables)
}

/// Returns an exclusive reference to the [`TableEntity`] and [`ElementSegmentEntity`] associated to `table` and `elem`.
///
/// # Panics
///
/// - If the [`Table`] does not originate from this [`Store`].
/// - If the [`Table`] cannot be resolved to its entity.
/// - If the [`ElementSegment`] does not originate from this [`Store`].
/// - If the [`ElementSegment`] cannot be resolved to its entity.
pub fn resolve_table_and_element_mut(
&mut self,
table: &Table,
elem: &ElementSegment,
) -> (&mut TableEntity, &mut ElementSegmentEntity) {
let table_idx = self.unwrap_stored(table.as_inner());
let elem_idx = self.unwrap_stored(elem.as_inner());
let table = Self::resolve_mut(table_idx, &mut self.tables);
let elem = Self::resolve_mut(elem_idx, &mut self.elems);
(table, elem)
}

/// Returns both
///
/// - an exclusive reference to the [`TableEntity`] associated to the given [`Table`]
Expand Down Expand Up @@ -644,34 +664,6 @@ impl StoreInner {
(fst, snd, fuel)
}

/// Returns a triple of:
///
/// - An exclusive reference to the [`TableEntity`] associated to the given [`Table`].
/// - A shared reference to the [`ElementSegmentEntity`] associated to the given [`ElementSegment`].
///
/// # Note
///
/// This method exists to properly handle use cases where
/// otherwise the Rust borrow-checker would not accept.
///
/// # Panics
///
/// - If the [`Table`] does not originate from this [`Store`].
/// - If the [`Table`] cannot be resolved to its entity.
/// - If the [`ElementSegment`] does not originate from this [`Store`].
/// - If the [`ElementSegment`] cannot be resolved to its entity.
pub(super) fn resolve_table_element(
&mut self,
table: &Table,
segment: &ElementSegment,
) -> (&mut TableEntity, &ElementSegmentEntity) {
let table_idx = self.unwrap_stored(table.as_inner());
let elem_idx = segment.as_inner();
let elem = self.resolve(elem_idx, &self.elems);
let table = Self::resolve_mut(table_idx, &mut self.tables);
(table, elem)
}

/// Returns the following data:
///
/// - A shared reference to the [`InstanceEntity`] associated to the given [`Instance`].
Expand All @@ -694,23 +686,15 @@ impl StoreInner {
/// - If the [`ElementSegment`] cannot be resolved to its entity.
pub(super) fn resolve_table_init_params(
&mut self,
instance: &Instance,
table: &Table,
segment: &ElementSegment,
) -> (
&InstanceEntity,
&mut TableEntity,
&ElementSegmentEntity,
&mut Fuel,
) {
) -> (&mut TableEntity, &ElementSegmentEntity, &mut Fuel) {
let mem_idx = self.unwrap_stored(table.as_inner());
let data_idx = segment.as_inner();
let instance_idx = instance.as_inner();
let instance = self.resolve(instance_idx, &self.instances);
let data = self.resolve(data_idx, &self.elems);
let mem = Self::resolve_mut(mem_idx, &mut self.tables);
let fuel = &mut self.fuel;
(instance, mem, data, fuel)
(mem, data, fuel)
}

/// Returns a shared reference to the [`ElementSegmentEntity`] associated to the given [`ElementSegment`].
Expand Down
85 changes: 47 additions & 38 deletions crates/wasmi/src/table/element.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
use crate::{
collections::arena::ArenaIndex,
core::ValType,
core::{UntypedVal, ValType},
module,
module::{ConstExpr, ElementSegmentItems},
store::Stored,
AsContext,
AsContextMut,
Func,
FuncRef,
Global,
Val,
};
use std::boxed::Box;

/// A raw index to a element segment entity.
#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)]
Expand Down Expand Up @@ -46,8 +50,15 @@ impl ElementSegment {
/// # Errors
///
/// If more than [`u32::MAX`] much linear memory is allocated.
pub fn new(mut ctx: impl AsContextMut, segment: &module::ElementSegment) -> Self {
let entity = ElementSegmentEntity::from(segment);
pub fn new(
mut ctx: impl AsContextMut,
elem: &module::ElementSegment,
get_func: impl Fn(u32) -> Func,
get_global: impl Fn(u32) -> Global,
) -> Self {
let get_func = |index| get_func(index).into();
let get_global = |index| get_global(index).get(&ctx);
let entity = ElementSegmentEntity::new(elem, get_func, get_global);
ctx.as_context_mut()
.store
.inner
Expand All @@ -62,15 +73,6 @@ impl ElementSegment {
.resolve_element_segment(self)
.size()
}

/// Drops the items of the [`ElementSegment`].
pub fn drop_items(&self, mut ctx: impl AsContextMut) {
ctx.as_context_mut()
.store
.inner
.resolve_element_segment_mut(self)
.drop_items()
}
}

/// An instantiated [`ElementSegmentEntity`].
Expand All @@ -84,33 +86,43 @@ impl ElementSegment {
pub struct ElementSegmentEntity {
/// The [`ValType`] of elements of this [`ElementSegmentEntity`].
ty: ValType,
/// The underlying items of the instance element segment.
///
/// # Note
///
/// These items are just readable after instantiation.
/// Using Wasm `elem.drop` simply replaces the instance
/// with an empty one.
items: Option<ElementSegmentItems>,
/// Pre-resolved untyped items of the Wasm element segment.
items: Box<[UntypedVal]>,
}

impl From<&'_ module::ElementSegment> for ElementSegmentEntity {
fn from(segment: &'_ module::ElementSegment) -> Self {
let ty = segment.ty();
match segment.kind() {
module::ElementSegmentKind::Passive | module::ElementSegmentKind::Active(_) => Self {
ty,
items: Some(segment.items_cloned()),
},
impl ElementSegmentEntity {
pub fn new(
elem: &'_ module::ElementSegment,
get_func: impl Fn(u32) -> FuncRef,
get_global: impl Fn(u32) -> Val,
) -> Self {
let ty = elem.ty();
match elem.kind() {
module::ElementSegmentKind::Passive | module::ElementSegmentKind::Active(_) => {
let items = elem
.items()
.iter()
.map(|const_expr| {
const_expr.eval_with_context(&get_global, &get_func).unwrap_or_else(|| {
panic!("unexpected failed initialization of constant expression: {const_expr:?}")
})
}).collect::<Box<[_]>>();
Self {
ty,
// items: Some(elem.items_cloned()),
items,
}
}
module::ElementSegmentKind::Declared => Self::empty(ty),
}
}
}

impl ElementSegmentEntity {
/// Create an empty [`ElementSegmentEntity`] representing dropped element segments.
fn empty(ty: ValType) -> Self {
Self { ty, items: None }
Self {
ty,
items: [].into(),
}
}

/// Returns the [`ValType`] of elements in the [`ElementSegmentEntity`].
Expand All @@ -124,15 +136,12 @@ impl ElementSegmentEntity {
}

/// Returns the items of the [`ElementSegmentEntity`].
pub fn items(&self) -> &[ConstExpr] {
self.items
.as_ref()
.map(ElementSegmentItems::items)
.unwrap_or(&[])
pub fn items(&self) -> &[UntypedVal] {
&self.items[..]
}

/// Drops the items of the [`ElementSegmentEntity`].
pub fn drop_items(&mut self) {
self.items = None;
self.items = [].into();
}
}
Loading

0 comments on commit 1d98b71

Please sign in to comment.