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 Dec 4, 2019
1 parent af7a368 commit 57e0708
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 33 deletions.
12 changes: 6 additions & 6 deletions Cargo.lock

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

3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -110,3 +110,6 @@ crate-type = ["bin"]
[[example]]
name = "callback"
crate-type = ["bin"]

[patch.crates-io]
wasmparser = { git = "https://github.com/AdamSLevy/wasmparser", branch = "OperatorClonev0.39.3" }
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 = "0.39.3"
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 @@ -271,7 +270,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 @@ -298,18 +298,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
28 changes: 11 additions & 17 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

0 comments on commit 57e0708

Please sign in to comment.