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

wasmparser: Remove offset param in VisitOperator #804

Merged
merged 4 commits into from Oct 27, 2022
Merged

wasmparser: Remove offset param in VisitOperator #804

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e553f87
wasmparser: Remove offset param in VisitOperator
nagisa Oct 26, 2022
f682a5d
wasmparser: rm `VisitOperator` for `FuncValidator`
nagisa Oct 27, 2022
04825b3
wasmparser: expand docs for `visit_operator`
nagisa Oct 27, 2022
510ab04
Do not run fuzzing for `cargo bench --all`
nagisa Oct 27, 2022
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
4 changes: 2 additions & 2 deletions crates/dump/src/lib.rs
View file
Open in desktop
Expand Up @@ -541,7 +541,7 @@ impl<'a> Dump<'a> {

fn print_ops(&mut self, mut i: OperatorsReader) -> Result<()> {
while !i.eof() {
match i.visit_with_offset(self) {
match i.visit_operator(self) {
Ok(()) => {}
Err(_) => write!(self.state, "??")?,
}
Expand Down Expand Up @@ -610,7 +610,7 @@ fn inc(spot: &mut u32) -> u32 {
macro_rules! define_visit_operator {
($(@$proposal:ident $op:ident $({ $($arg:ident: $argty:ty),* })? => $visit:ident)*) => {
$(
fn $visit(&mut self, _offset: usize $($(,$arg: $argty)*)?) {
fn $visit(&mut self $($(,$arg: $argty)*)?) {
write!(
self.state,
concat!(
Expand Down
2 changes: 1 addition & 1 deletion crates/wasmparser/benches/benchmark.rs
View file
Open in desktop
Expand Up @@ -314,7 +314,7 @@ struct NopVisit;
macro_rules! define_visit_operator {
($(@$proposal:ident $op:ident $({ $($arg:ident: $argty:ty),* })? => $visit:ident)*) => {
$(
fn $visit(&mut self, _offset: usize $($(,$arg: $argty)*)?) {
fn $visit(&mut self $($(,$arg: $argty)*)?) {
define_visit_operator!(@visit $op $( $($arg)* )?);
}
)*
Expand Down
1,131 changes: 584 additions & 547 deletions crates/wasmparser/src/binary_reader.rs
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion crates/wasmparser/src/lib.rs
View file
Open in desktop
Expand Up @@ -97,7 +97,7 @@
/// // `VisitOperator` trait that this corresponds to.
/// ($( @$proposal:ident $op:ident $({ $($arg:ident: $argty:ty),* })? => $visit:ident)*) => {
/// $(
/// fn $visit(&mut self, _offset: usize $($(,$arg: $argty)*)?) {
/// fn $visit(&mut self $($(,$arg: $argty)*)?) {
/// // do nothing for this example
/// }
/// )*
Expand Down
15 changes: 7 additions & 8 deletions crates/wasmparser/src/readers/core/operators.rs
View file
Open in desktop
Expand Up @@ -198,11 +198,10 @@ impl<'a> OperatorsReader<'a> {
Ok((self.read()?, pos))
}

/// Visits an operator with its offset.
pub fn visit_with_offset<T>(
&mut self,
visitor: &mut T,
) -> Result<<T as VisitOperator<'a>>::Output>
/// Visit a single operator with the specified [`VisitOperator`] instance.
///
/// See [`BinaryReader::visit_operator`] for more information.
pub fn visit_operator<T>(&mut self, visitor: &mut T) -> Result<<T as VisitOperator<'a>>::Output>
where
T: VisitOperator<'a>,
{
Expand Down Expand Up @@ -306,7 +305,7 @@ impl<'a> Iterator for OperatorsIteratorWithOffsets<'a> {
macro_rules! define_visit_operator {
($(@$proposal:ident $op:ident $({ $($arg:ident: $argty:ty),* })? => $visit:ident)*) => {
$(
fn $visit(&mut self, offset: usize $($(,$arg: $argty)*)?) -> Self::Output;
fn $visit(&mut self $($(,$arg: $argty)*)?) -> Self::Output;
)*
}
}
Expand All @@ -325,12 +324,12 @@ pub trait VisitOperator<'a> {
/// critical use cases. For performance critical implementations users
/// are recommended to directly use the respective `visit` methods or
/// implement [`VisitOperator`] on their own.
fn visit_operator(&mut self, offset: usize, op: &Operator<'a>) -> Self::Output {
fn visit_operator(&mut self, op: &Operator<'a>) -> Self::Output {
macro_rules! visit_operator {
($(@$proposal:ident $op:ident $({ $($arg:ident: $argty:ty),* })? => $visit:ident)*) => {
match op {
$(
Operator::$op $({ $($arg),* })? => self.$visit(offset, $($($arg.clone()),*)?),
Operator::$op $({ $($arg),* })? => self.$visit($($($arg.clone()),*)?),
)*
}
}
Expand Down
101 changes: 52 additions & 49 deletions crates/wasmparser/src/validator/core.rs
View file
Open in desktop
Expand Up @@ -241,6 +241,7 @@ impl ModuleState {
types: &TypeList,
) -> Result<()> {
let mut validator = VisitConstOperator {
offset: 0,
order: self.order,
uninserted_funcref: false,
ops: OperatorValidator::new_const_expr(
Expand All @@ -256,7 +257,8 @@ impl ModuleState {

let mut ops = expr.get_operators_reader();
while !ops.eof() {
ops.visit_with_offset(&mut validator)??;
validator.offset = ops.original_position();
ops.visit_operator(&mut validator)??;
nagisa marked this conversation as resolved.
Show resolved Hide resolved
}
validator.ops.finish(ops.original_position())?;

Expand All @@ -268,6 +270,7 @@ impl ModuleState {
return Ok(());

struct VisitConstOperator<'a> {
offset: usize,
uninserted_funcref: bool,
ops: OperatorValidator,
resources: OperatorValidatorResources<'a>,
Expand All @@ -276,33 +279,33 @@ impl ModuleState {

impl VisitConstOperator<'_> {
fn validator(&mut self) -> impl VisitOperator<'_, Output = Result<()>> {
self.ops.with_resources(&self.resources)
self.ops.with_resources(&self.resources, self.offset)
}

fn validate_extended_const(&mut self, offset: usize) -> Result<()> {
fn validate_extended_const(&mut self) -> Result<()> {
if self.ops.features.extended_const {
Ok(())
} else {
Err(BinaryReaderError::new(
"constant expression required: non-constant operator",
offset,
self.offset,
))
}
}

fn validate_global(&mut self, offset: usize, index: u32) -> Result<()> {
fn validate_global(&mut self, index: u32) -> Result<()> {
let module = &self.resources.module;
let global = module.global_at(index, offset)?;
let global = module.global_at(index, self.offset)?;
if index >= module.num_imported_globals {
return Err(BinaryReaderError::new(
"constant expression required: global.get of locally defined global",
offset,
self.offset,
));
}
if global.mutable {
return Err(BinaryReaderError::new(
"constant expression required: global.get of mutable global",
offset,
self.offset,
));
}
Ok(())
Expand Down Expand Up @@ -342,79 +345,79 @@ impl ModuleState {
($(@$proposal:ident $op:ident $({ $($arg:ident: $argty:ty),* })? => $visit:ident)*) => {
$(
#[allow(unused_variables)]
fn $visit(&mut self, pos: usize $($(,$arg: $argty)*)?) -> Self::Output {
define_visit_operator!(@visit self $visit pos $($($arg)*)?)
fn $visit(&mut self $($(,$arg: $argty)*)?) -> Self::Output {
define_visit_operator!(@visit self $visit $($($arg)*)?)
}
)*
};

// These are always valid in const expressions
(@visit $self:ident visit_i32_const $pos:ident $val:ident) => {{
$self.validator().visit_i32_const($pos, $val)
(@visit $self:ident visit_i32_const $val:ident) => {{
$self.validator().visit_i32_const($val)
}};
(@visit $self:ident visit_i64_const $pos:ident $val:ident) => {{
$self.validator().visit_i64_const($pos, $val)
(@visit $self:ident visit_i64_const $val:ident) => {{
$self.validator().visit_i64_const($val)
}};
(@visit $self:ident visit_f32_const $pos:ident $val:ident) => {{
$self.validator().visit_f32_const($pos, $val)
(@visit $self:ident visit_f32_const $val:ident) => {{
$self.validator().visit_f32_const($val)
}};
(@visit $self:ident visit_f64_const $pos:ident $val:ident) => {{
$self.validator().visit_f64_const($pos, $val)
(@visit $self:ident visit_f64_const $val:ident) => {{
$self.validator().visit_f64_const($val)
}};
(@visit $self:ident visit_v128_const $pos:ident $val:ident) => {{
$self.validator().visit_v128_const($pos, $val)
(@visit $self:ident visit_v128_const $val:ident) => {{
$self.validator().visit_v128_const($val)
}};
(@visit $self:ident visit_ref_null $pos:ident $val:ident) => {{
$self.validator().visit_ref_null($pos, $val)
(@visit $self:ident visit_ref_null $val:ident) => {{
$self.validator().visit_ref_null($val)
}};
(@visit $self:ident visit_end $pos:ident) => {{
$self.validator().visit_end($pos)
(@visit $self:ident visit_end) => {{
$self.validator().visit_end()
}};


// These are valid const expressions when the extended-const proposal is enabled.
(@visit $self:ident visit_i32_add $pos:ident) => {{
$self.validate_extended_const($pos)?;
$self.validator().visit_i32_add($pos)
(@visit $self:ident visit_i32_add) => {{
$self.validate_extended_const()?;
$self.validator().visit_i32_add()
}};
(@visit $self:ident visit_i32_sub $pos:ident) => {{
$self.validate_extended_const($pos)?;
$self.validator().visit_i32_sub($pos)
(@visit $self:ident visit_i32_sub) => {{
$self.validate_extended_const()?;
$self.validator().visit_i32_sub()
}};
(@visit $self:ident visit_i32_mul $pos:ident) => {{
$self.validate_extended_const($pos)?;
$self.validator().visit_i32_mul($pos)
(@visit $self:ident visit_i32_mul) => {{
$self.validate_extended_const()?;
$self.validator().visit_i32_mul()
}};
(@visit $self:ident visit_i64_add $pos:ident) => {{
$self.validate_extended_const($pos)?;
$self.validator().visit_i64_add($pos)
(@visit $self:ident visit_i64_add) => {{
$self.validate_extended_const()?;
$self.validator().visit_i64_add()
}};
(@visit $self:ident visit_i64_sub $pos:ident) => {{
$self.validate_extended_const($pos)?;
$self.validator().visit_i64_sub($pos)
(@visit $self:ident visit_i64_sub) => {{
$self.validate_extended_const()?;
$self.validator().visit_i64_sub()
}};
(@visit $self:ident visit_i64_mul $pos:ident) => {{
$self.validate_extended_const($pos)?;
$self.validator().visit_i64_mul($pos)
(@visit $self:ident visit_i64_mul) => {{
$self.validate_extended_const()?;
$self.validator().visit_i64_mul()
}};

// `global.get` is a valid const expression for imported, immutable
// globals.
(@visit $self:ident visit_global_get $pos:ident $idx:ident) => {{
$self.validate_global($pos, $idx)?;
$self.validator().visit_global_get($pos, $idx)
(@visit $self:ident visit_global_get $idx:ident) => {{
$self.validate_global($idx)?;
$self.validator().visit_global_get($idx)
}};
// `ref.func`, if it's in a `global` initializer, will insert into
// the set of referenced functions so it's processed here.
(@visit $self:ident visit_ref_func $pos:ident $idx:ident) => {{
(@visit $self:ident visit_ref_func $idx:ident) => {{
$self.insert_ref_func($idx);
$self.validator().visit_ref_func($pos, $idx)
$self.validator().visit_ref_func($idx)
}};

(@visit $self:ident $op:ident $pos:ident $($args:tt)*) => {{
(@visit $self:ident $op:ident $($args:tt)*) => {{
Err(BinaryReaderError::new(
"constant expression required: non-constant operator",
$pos,
$self.offset,
))
}}
}
Expand Down
53 changes: 28 additions & 25 deletions crates/wasmparser/src/validator/func.rs
View file
Open in desktop
Expand Up @@ -94,7 +94,7 @@ impl<T: WasmModuleResources> FuncValidator<T> {
self.read_locals(&mut reader)?;
reader.allow_memarg64(self.validator.features.memory64);
while !reader.eof() {
reader.visit_operator(self)??;
reader.visit_operator(&mut self.visitor(reader.original_position()))??;
}
self.finish(reader.original_position())
}
Expand Down Expand Up @@ -129,9 +129,33 @@ impl<T: WasmModuleResources> FuncValidator<T> {
/// the operator itself are passed to this function to provide more useful
/// error messages.
pub fn op(&mut self, offset: usize, operator: &Operator<'_>) -> Result<()> {
self.validator
.with_resources(&self.resources)
.visit_operator(offset, operator)
self.visitor(offset).visit_operator(operator)
}

/// Get the operator visitor for the next operator in the function.
///
/// The returned visitor is intended to visit just one instruction at the `offset`.
///
/// # Example
///
/// ```
/// # use wasmparser::{WasmModuleResources, FuncValidator, FunctionBody, Result};
/// pub fn validate<R>(validator: &mut FuncValidator<R>, body: &FunctionBody<'_>) -> Result<()>
/// where R: WasmModuleResources
/// {
/// let mut operator_reader = body.get_binary_reader();
/// while !operator_reader.eof() {
/// let mut visitor = validator.visitor(operator_reader.original_position());
/// operator_reader.visit_operator(&mut visitor)??;
/// }
/// validator.finish(operator_reader.original_position())
/// }
/// ```
pub fn visitor<'this, 'a: 'this>(
&'this mut self,
offset: usize,
) -> impl VisitOperator<'a, Output = Result<()>> + 'this {
self.validator.with_resources(&self.resources, offset)
}

/// Function that must be called after the last opcode has been processed.
Expand Down Expand Up @@ -307,24 +331,3 @@ mod tests {
assert_eq!(v.operand_stack_height(), 2);
}
}

macro_rules! define_visit_operator {
($(@$proposal:ident $op:ident $({ $($arg:ident: $argty:ty),* })? => $visit:ident)*) => {
$(
fn $visit(&mut self, offset: usize $($(,$arg: $argty)*)?) -> Result<()> {
self.validator.with_resources(&self.resources)
.$visit(offset $($(,$arg)*)?)
}
)*
}
}

#[allow(unused_variables)]
impl<'a, T> VisitOperator<'a> for FuncValidator<T>
where
T: WasmModuleResources,
{
type Output = Result<()>;

for_each_operator!(define_visit_operator);
}