Skip to content

Commit

Permalink
refactor(parse,codegen): Pass all func ops to MiddlewareChains
Browse files Browse the repository at this point in the history
Previously each Event(Operation) was passed through all MiddlewareChains
one at a time. Additionally, the wasmparser library prevented cloning
the Operators which prevented more than one Operator from being observed
at a time.

This commit loads all Operators of a function into a Vec and passes the
entire Vec through the middleware chains. This design does not affect
the existing MiddlewareChains but allows chains to look back at
previously viewed Operators within the same function. This commit uses a
forked wasmparser that adds the Clone trait to wasmparser::Operator to
allow this.

Looking back at previously seen Operators is necessary for allowing the
metering MiddlewareChain to properly inject metering code so as to patch
the flaws described in Issue wasmerio#999.
  • Loading branch information
AdamSLevy committed Nov 28, 2019
1 parent 53f0a9c commit bb3514b
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 29 deletions.
8 changes: 7 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/runtime-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ edition = "2018"
[dependencies]
nix = "0.15"
page_size = "0.4"
wasmparser = "0.39.1"
wasmparser = { git = "https://github.com/AdamSLevy/wasmparser", branch = "OperatorClone" }
parking_lot = "0.9"
lazy_static = "1.4"
errno = "0.2"
Expand Down
14 changes: 5 additions & 9 deletions lib/runtime-core/src/codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ use crate::{
structures::Map,
types::{FuncIndex, FuncSig, SigIndex},
};
use smallvec::SmallVec;
use std::any::Any;
use std::collections::HashMap;
use std::fmt;
Expand Down Expand Up @@ -270,7 +269,8 @@ fn requires_pre_validation(backend: Backend) -> bool {

/// A sink for parse events.
pub struct EventSink<'a, 'b> {
buffer: SmallVec<[Event<'a, 'b>; 2]>,
/// document this
pub buffer: Vec<Event<'a, 'b>>,
}

impl<'a, 'b> EventSink<'a, 'b> {
Expand All @@ -297,18 +297,14 @@ impl MiddlewareChain {
}

/// Run this chain with the provided function code generator, event and module info.
pub(crate) fn run<E: Debug, FCG: FunctionCodeGenerator<E>>(
pub(crate) fn run<'a, 'b: 'a, E: Debug, FCG: FunctionCodeGenerator<E>>(
&mut self,
fcg: Option<&mut FCG>,
ev: Event,
mut sink: EventSink<'a, 'b>,
module_info: &ModuleInfo,
) -> Result<(), String> {
let mut sink = EventSink {
buffer: SmallVec::new(),
};
sink.push(ev);
for m in &mut self.chain {
let prev: SmallVec<[Event; 2]> = sink.buffer.drain().collect();
let prev: Vec<Event> = sink.buffer.drain(..).collect();
for ev in prev {
m.feed_event(ev, module_info, &mut sink)?;
}
Expand Down
30 changes: 12 additions & 18 deletions lib/runtime-core/src/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,8 +249,8 @@ pub fn read_module<
.map_err(|x| LoadError::Codegen(format!("{:?}", x)))?;
}

let mut events: Vec<Event> = Vec::new();
let mut body_begun = false;

loop {
let state = parser.read();
match state {
Expand All @@ -266,28 +266,22 @@ pub fn read_module<
body_begun = true;
fcg.begin_body(&info.read().unwrap())
.map_err(|x| LoadError::Codegen(format!("{:?}", x)))?;
middlewares
.run(
Some(fcg),
Event::Internal(InternalEvent::FunctionBegin(id as u32)),
&info.read().unwrap(),
)
.map_err(|x| LoadError::Codegen(x))?;
events
.push(Event::Internal(InternalEvent::FunctionBegin(id as u32)));
}
middlewares
.run(Some(fcg), Event::Wasm(op), &info.read().unwrap())
.map_err(|x| LoadError::Codegen(x))?;
events.push(Event::WasmOwned(op.clone()));
}
ParserState::EndFunctionBody => {
events.push(Event::Internal(InternalEvent::FunctionEnd));
break;
}
ParserState::EndFunctionBody => break,
_ => unreachable!(),
}
}

let events = EventSink { buffer: events };
middlewares
.run(
Some(fcg),
Event::Internal(InternalEvent::FunctionEnd),
&info.read().unwrap(),
)
.run(Some(fcg), events, &info.read().unwrap())
.map_err(|x| LoadError::Codegen(x))?;
fcg.finalize()
.map_err(|x| LoadError::Codegen(format!("{:?}", x)))?;
Expand Down Expand Up @@ -451,7 +445,7 @@ fn func_type_to_func_sig(func_ty: &FuncType) -> Result<FuncSig, BinaryReaderErro

fn eval_init_expr(op: &Operator) -> Result<Initializer, BinaryReaderError> {
Ok(match *op {
Operator::GetGlobal { global_index } => {
Operator::GlobalGet { global_index } => {
Initializer::GetGlobal(ImportedGlobalIndex::new(global_index as usize))
}
Operator::I32Const { value } => Initializer::Const(Value::I32(value)),
Expand Down

0 comments on commit bb3514b

Please sign in to comment.