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.

Sign up for GitHub

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

Jump to bottom

Update handling and parsing of element segments #1126

Merged
merged 2 commits into from Jul 17, 2023
Merged

Update handling and parsing of element segments #1126

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e31c58c
Update handling and parsing of element segments
alexcrichton Jul 15, 2023
7f989ce
Fix benchmark
alexcrichton Jul 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
70 changes: 33 additions & 37 deletions crates/wasm-encoder/src/core/elements.rs
View file
Open in desktop
Expand Up @@ -22,11 +22,10 @@ use crate::{encode_section, ConstExpr, Encode, RefType, Section, SectionId};
/// let mut elements = ElementSection::new();
/// let table_index = 0;
/// let offset = ConstExpr::i32_const(42);
/// let element_type = RefType::FUNCREF;
/// let functions = Elements::Functions(&[
/// // Function indices...
/// ]);
/// elements.active(Some(table_index), &offset, element_type, functions);
/// elements.active(Some(table_index), &offset, functions);
///
/// let mut module = Module::new();
/// module
Expand All @@ -47,7 +46,7 @@ pub enum Elements<'a> {
/// A sequences of references to functions by their indices.
Functions(&'a [u32]),
/// A sequence of reference expressions.
Expressions(&'a [ConstExpr]),
Expressions(RefType, &'a [ConstExpr]),
}

/// An element segment's mode.
Expand Down Expand Up @@ -79,8 +78,6 @@ pub enum ElementMode<'a> {
pub struct ElementSegment<'a> {
/// The element segment's mode.
pub mode: ElementMode<'a>,
/// The element segment's type.
pub element_type: RefType,
/// This segment's elements.
pub elements: Elements<'a>,
}
Expand All @@ -104,50 +101,53 @@ impl ElementSection {
/// Define an element segment.
pub fn segment<'a>(&mut self, segment: ElementSegment<'a>) -> &mut Self {
let expr_bit = match segment.elements {
Elements::Expressions(_) => 0b100u32,
Elements::Expressions(..) => 0b100u32,
Elements::Functions(_) => 0b000u32,
};
let mut encode_type = false;
match &segment.mode {
ElementMode::Active {
table: None,
offset,
} if segment.element_type == RefType::FUNCREF => {
(/* 0x00 | */expr_bit).encode(&mut self.bytes);
offset.encode(&mut self.bytes);
}
ElementMode::Passive => {
(0x01 | expr_bit).encode(&mut self.bytes);
if expr_bit == 0 {
self.bytes.push(0x00); // elemkind == funcref
} else {
segment.element_type.encode(&mut self.bytes);
}
encode_type = true;
}
ElementMode::Active { table, offset } => {
(0x02 | expr_bit).encode(&mut self.bytes);
table.unwrap_or(0).encode(&mut self.bytes);
offset.encode(&mut self.bytes);
if expr_bit == 0 {
self.bytes.push(0x00); // elemkind == funcref
} else {
segment.element_type.encode(&mut self.bytes);
match (table, &segment.elements) {
// If the `table` is not specified then the 0x00 encoding
// can be used with either function indices or expressions
// that have a `funcref` type.
(None, Elements::Functions(_) | Elements::Expressions(RefType::FUNCREF, _)) => {
(/* 0x00 | */expr_bit).encode(&mut self.bytes);
}

// ... otherwise fall through for all other expressions here
// with table 0 or an explicitly specified table to the 0x02
// encoding.
(None, Elements::Expressions(..)) | (Some(_), _) => {
(0x02 | expr_bit).encode(&mut self.bytes);
table.unwrap_or(0).encode(&mut self.bytes);
encode_type = true;
}
}
offset.encode(&mut self.bytes);
}
ElementMode::Declared => {
(0x03 | expr_bit).encode(&mut self.bytes);
if expr_bit == 0 {
self.bytes.push(0x00); // elemkind == funcref
} else {
segment.element_type.encode(&mut self.bytes);
}
encode_type = true;
}
}

match segment.elements {
Elements::Functions(fs) => {
if encode_type {
// elemkind == funcref
self.bytes.push(0x00);
}
fs.encode(&mut self.bytes);
}
Elements::Expressions(e) => {
Elements::Expressions(ty, e) => {
if encode_type {
ty.encode(&mut self.bytes);
}
e.len().encode(&mut self.bytes);
for expr in e {
expr.encode(&mut self.bytes);
Expand All @@ -168,37 +168,33 @@ impl ElementSection {
&mut self,
table_index: Option<u32>,
offset: &ConstExpr,
element_type: RefType,
elements: Elements<'_>,
) -> &mut Self {
self.segment(ElementSegment {
mode: ElementMode::Active {
table: table_index,
offset,
},
element_type,
elements,
})
}

/// Encode a passive element segment.
///
/// Passive segments are part of the bulk memory proposal.
pub fn passive<'a>(&mut self, element_type: RefType, elements: Elements<'a>) -> &mut Self {
pub fn passive<'a>(&mut self, elements: Elements<'a>) -> &mut Self {
self.segment(ElementSegment {
mode: ElementMode::Passive,
element_type,
elements,
})
}

/// Encode a declared element segment.
///
/// Declared segments are part of the bulk memory proposal.
pub fn declared<'a>(&mut self, element_type: RefType, elements: Elements<'a>) -> &mut Self {
pub fn declared<'a>(&mut self, elements: Elements<'a>) -> &mut Self {
self.segment(ElementSegment {
mode: ElementMode::Declared,
element_type,
elements,
})
}
Expand Down
6 changes: 3 additions & 3 deletions crates/wasm-mutate/src/mutators/modify_const_exprs.rs
View file
Open in desktop
Expand Up @@ -195,7 +195,7 @@ impl Mutator for ConstExpressionMutator {
// to skip a specific number of activations of the Translator methods.
let item_count = match &element.items {
wasmparser::ElementItems::Functions(r) => r.count(),
wasmparser::ElementItems::Expressions(r) => r.count(),
wasmparser::ElementItems::Expressions(_, r) => r.count(),
};
if item_count > 0 {
let skip = translator.config.rng().gen_range(0..item_count);
Expand Down Expand Up @@ -287,9 +287,9 @@ mod tests {
#[test]
fn reduce_elem_funcref() {
match_reduction(
r#"(module (table 0 funcref) (elem $a $b) (func $a) (func $b))"#,
r#"(module (table 0 funcref) (elem func $a $b) (func $a) (func $b))"#,
super::ConstExpressionMutator::ElementFunc,
r#"(module (table 0 funcref) (elem $a $a) (func $a) (func $b))"#,
r#"(module (table 0 funcref) (elem func $a $a) (func $a) (func $b))"#,
);
}

Expand Down
2 changes: 1 addition & 1 deletion crates/wasm-mutate/src/mutators/remove_item.rs
View file
Open in desktop
Expand Up @@ -546,7 +546,7 @@ mod tests {
#[test]
fn remove_elem() {
crate::mutators::match_mutation(
r#"(module (elem))"#,
r#"(module (elem funcref))"#,
RemoveItemMutator(Item::Element),
r#"(module)"#,
);
Expand Down
13 changes: 4 additions & 9 deletions crates/wasm-mutate/src/mutators/translate.rs
View file
Open in desktop
Expand Up @@ -282,7 +282,6 @@ pub fn element(
ElementKind::Passive => ElementMode::Passive,
ElementKind::Declared => ElementMode::Declared,
};
let element_type = t.translate_refty(&element.ty)?;
let functions;
let exprs;
let elements = match element.items {
Expand All @@ -293,25 +292,21 @@ pub fn element(
.collect::<Result<Vec<_>, _>>()?;
Elements::Functions(&functions)
}
wasmparser::ElementItems::Expressions(reader) => {
wasmparser::ElementItems::Expressions(ty, reader) => {
exprs = reader
.into_iter()
.map(|f| {
t.translate_const_expr(
&f?,
&wasmparser::ValType::Ref(element.ty),
&wasmparser::ValType::Ref(ty),
ConstExprKind::ElementFunction,
)
})
.collect::<Result<Vec<_>, _>>()?;
Elements::Expressions(&exprs)
Elements::Expressions(t.translate_refty(&ty)?, &exprs)
}
};
s.segment(ElementSegment {
mode,
element_type,
elements,
});
s.segment(ElementSegment { mode, elements });
Ok(())
}

Expand Down
8 changes: 4 additions & 4 deletions crates/wasm-smith/src/core/encode.rs
View file
Open in desktop
Expand Up @@ -158,7 +158,7 @@ impl Module {
None => wasm_encoder::ConstExpr::ref_null(el.ty.heap_type),
}
}));
wasm_encoder::Elements::Expressions(&exps)
wasm_encoder::Elements::Expressions(el.ty, &exps)
}
Elements::Functions(fs) => wasm_encoder::Elements::Functions(fs),
};
Expand All @@ -169,13 +169,13 @@ impl Module {
Offset::Const64(n) => ConstExpr::i64_const(n),
Offset::Global(g) => ConstExpr::global_get(g),
};
elems.active(*table, &offset, el.ty, elements);
elems.active(*table, &offset, elements);
}
ElementKind::Passive => {
elems.passive(el.ty, elements);
elems.passive(elements);
}
ElementKind::Declared => {
elems.declared(el.ty, elements);
elems.declared(elements);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/wasmparser/benches/benchmark.rs
View file
Open in desktop
Expand Up @@ -136,7 +136,7 @@ fn read_all_wasm(wasm: &[u8]) -> Result<()> {
op?;
}
}
wasmparser::ElementItems::Expressions(r) => {
wasmparser::ElementItems::Expressions(_, r) => {
for op in r {
op?;
}
Expand Down
26 changes: 10 additions & 16 deletions crates/wasmparser/src/readers/core/elements.rs
View file
Open in desktop
Expand Up @@ -26,8 +26,6 @@ pub struct Element<'a> {
pub kind: ElementKind<'a>,
/// The initial elements of the element segment.
pub items: ElementItems<'a>,
/// The type of the elements.
pub ty: RefType,
/// The range of the the element segment.
pub range: Range<usize>,
}
Expand All @@ -54,7 +52,7 @@ pub enum ElementItems<'a> {
/// This element contains function indices.
Functions(SectionLimited<'a, u32>),
/// This element contains constant expressions used to initialize the table.
Expressions(SectionLimited<'a, ConstExpr<'a>>),
Expressions(RefType, SectionLimited<'a, ConstExpr<'a>>),
}

/// A reader for the element section of a WebAssembly module.
Expand Down Expand Up @@ -104,10 +102,10 @@ impl<'a> FromReader<'a> for Element<'a> {
let exprs = flags & 0b100 != 0;
let ty = if flags & 0b011 != 0 {
if exprs {
reader.read()?
Some(reader.read()?)
} else {
match reader.read()? {
ExternalKind::Func => RefType::FUNCREF,
ExternalKind::Func => None,
_ => {
return Err(BinaryReaderError::new(
"only the function external type is supported in elem segment",
Expand All @@ -117,7 +115,7 @@ impl<'a> FromReader<'a> for Element<'a> {
}
}
} else {
RefType::FUNCREF
None
};
// FIXME(#188) ideally wouldn't have to do skips here
let data = reader.skip(|reader| {
Expand All @@ -134,11 +132,12 @@ impl<'a> FromReader<'a> for Element<'a> {
Ok(())
})?;
let items = if exprs {
ElementItems::Expressions(SectionLimited::new(
data.remaining_buffer(),
data.original_position(),
)?)
ElementItems::Expressions(
ty.unwrap_or(RefType::FUNCREF),
SectionLimited::new(data.remaining_buffer(), data.original_position())?,
)
} else {
assert!(ty.is_none());
ElementItems::Functions(SectionLimited::new(
data.remaining_buffer(),
data.original_position(),
Expand All @@ -148,11 +147,6 @@ impl<'a> FromReader<'a> for Element<'a> {
let elem_end = reader.original_position();
let range = elem_start..elem_end;

Ok(Element {
kind,
items,
ty,
range,
})
Ok(Element { kind, items, range })
}
}
36 changes: 18 additions & 18 deletions crates/wasmparser/src/validator/core.rs
View file
Open in desktop
Expand Up @@ -199,24 +199,30 @@ impl ModuleState {
) -> Result<()> {
// the `funcref` value type is allowed all the way back to the MVP, so
// don't check it here
if e.ty != RefType::FUNCREF {
self.module
.check_value_type(ValType::Ref(e.ty), features, offset)?;
}
let element_ty = match &e.items {
crate::ElementItems::Functions(_) => RefType::FUNC,
crate::ElementItems::Expressions(ty, _) => {
self.module
.check_value_type(ValType::Ref(*ty), features, offset)?;
*ty
}
};

match e.kind {
ElementKind::Active {
table_index,
offset_expr,
} => {
let table = self.module.table_at(table_index.unwrap_or(0), offset)?;
if !self
.module
.matches(ValType::Ref(e.ty), ValType::Ref(table.element_type), types)
{
if !self.module.matches(
ValType::Ref(element_ty),
ValType::Ref(table.element_type),
types,
) {
return Err(BinaryReaderError::new(
format!(
"type mismatch: invalid element type `{}` for table type `{}`",
ty_to_str(e.ty.into()),
ty_to_str(element_ty.into()),
ty_to_str(table.element_type.into()),
),
offset,
Expand Down Expand Up @@ -248,27 +254,21 @@ impl ModuleState {
match e.items {
crate::ElementItems::Functions(reader) => {
let count = reader.count();
if !e.ty.is_nullable() && count == 0 {
return Err(BinaryReaderError::new(
"a non-nullable element must come with an initialization expression",
offset,
));
}
validate_count(count)?;
for f in reader.into_iter_with_offsets() {
let (offset, f) = f?;
self.module.get_func_type(f, types, offset)?;
self.module.assert_mut().function_references.insert(f);
}
}
crate::ElementItems::Expressions(reader) => {
crate::ElementItems::Expressions(ty, reader) => {
validate_count(reader.count())?;
for expr in reader {
self.check_const_expr(&expr?, ValType::Ref(e.ty), features, types)?;
self.check_const_expr(&expr?, ValType::Ref(ty), features, types)?;
}
}
}
self.module.assert_mut().element_types.push(e.ty);
self.module.assert_mut().element_types.push(element_ty);
Ok(())
}

Expand Down