fuzz: improve the spec interpreter (#4881)

* fuzz: improve the API of the `wasm-spec-interpreter` crate

This change addresses key parts of #4852 by improving the bindings to
the OCaml spec interpreter. The new API allows users to `instantiate` a
module, `interpret` named functions on that instance, and `export`
globals and memories from that instance. This currently leaves the
existing implementation ("instantiate and interpret the first function in
a module") present under a new name: `interpret_legacy`.

* fuzz: adapt the differential spec engine to the new API

This removes the legacy uses in the differential spec engine, replacing
them with the new `instantiate`-`interpret`-`export` API from the
`wasm-spec-interpreter` crate.

* fix: make instance access thread-safe

This changes the OCaml-side definition of the instance so that each
instance carries round a reference to a "global store" that's specific
to that instantiation. Because everything is updated by reference there
should be no visible behavioural change on the Rust side, apart from
everything suddenly being thread-safe (modulo the fact that access to
the OCaml runtime still needs to be locked). This fix will need to be
generalised slightly in future if we want to allow multiple modules to
be instantiated in the same store.

Co-authored-by: conrad-watt <cnrdwtt@gmail.com>
Co-authored-by: Alex Crichton <alex@alexcrichton.com>
This commit is contained in:
Andrew Brown
2022-09-12 14:23:03 -07:00
committed by GitHub
parent 024cad7e3d
commit c3f8415ac7
8 changed files with 443 additions and 147 deletions

View File

@@ -4,7 +4,7 @@
use crate::generators::{Config, DiffValue, DiffValueType};
use crate::oracles::engine::{DiffEngine, DiffInstance};
use anyhow::{anyhow, Error, Result};
use wasm_spec_interpreter::Value;
use wasm_spec_interpreter::SpecValue;
use wasmtime::Trap;
/// A wrapper for `wasm-spec-interpreter` as a [`DiffEngine`].
@@ -14,24 +14,10 @@ impl SpecInterpreter {
pub(crate) fn new(config: &mut Config) -> Self {
let config = &mut config.module_config.config;
// TODO: right now the interpreter bindings only execute the first
// function in the module so if there's possibly more than one function
// it's not possible to run the other function. This should be fixed
// with improvements to the ocaml bindings to the interpreter.
config.min_funcs = 1;
config.max_funcs = 1;
// TODO: right now the instantiation step for the interpreter does
// nothing and the evaluation step performs an instantiation followed by
// an execution. This means that instantiations which fail in other
// engines will "succeed" in the interpreter because the error is
// delayed to the execution. This should be fixed by making
// instantiation a first-class primitive in our interpreter bindings.
config.min_tables = 0;
config.max_tables = 0;
config.min_memories = config.min_memories.min(1);
config.max_memories = config.max_memories.min(1);
config.min_tables = config.min_tables.min(1);
config.max_tables = config.max_tables.min(1);
config.memory64_enabled = false;
config.threads_enabled = false;
@@ -48,10 +34,9 @@ impl DiffEngine for SpecInterpreter {
}
fn instantiate(&mut self, wasm: &[u8]) -> Result<Box<dyn DiffInstance>> {
// TODO: ideally we would avoid copying the module bytes here.
Ok(Box::new(SpecInstance {
wasm: wasm.to_vec(),
}))
let instance = wasm_spec_interpreter::instantiate(wasm)
.map_err(|e| anyhow!("failed to instantiate in spec interpreter: {}", e))?;
Ok(Box::new(SpecInstance { instance }))
}
fn assert_error_match(&self, trap: &Trap, err: &Error) {
@@ -60,14 +45,12 @@ impl DiffEngine for SpecInterpreter {
}
fn is_stack_overflow(&self, err: &Error) -> bool {
// TODO: implement this for the spec interpreter
drop(err);
false
err.to_string().contains("(Isabelle) call stack exhausted")
}
}
struct SpecInstance {
wasm: Vec<u8>,
instance: wasm_spec_interpreter::SpecInstance,
}
impl DiffInstance for SpecInstance {
@@ -77,55 +60,57 @@ impl DiffInstance for SpecInstance {
fn evaluate(
&mut self,
_function_name: &str,
function_name: &str,
arguments: &[DiffValue],
_results: &[DiffValueType],
) -> Result<Option<Vec<DiffValue>>> {
// The spec interpreter needs some work before it can fully support this
// interface:
// - TODO adapt `wasm-spec-interpreter` to use function name to select
// function to run
// - TODO adapt `wasm-spec-interpreter` to expose an "instance" with
// so we can hash memory, globals, etc.
let arguments = arguments.iter().map(Value::from).collect();
match wasm_spec_interpreter::interpret(&self.wasm, Some(arguments)) {
Ok(results) => Ok(Some(results.into_iter().map(Value::into).collect())),
let arguments = arguments.iter().map(SpecValue::from).collect();
match wasm_spec_interpreter::interpret(&self.instance, function_name, Some(arguments)) {
Ok(results) => Ok(Some(results.into_iter().map(SpecValue::into).collect())),
Err(err) => Err(anyhow!(err)),
}
}
fn get_global(&mut self, _name: &str, _ty: DiffValueType) -> Option<DiffValue> {
// TODO: should implement this
None
fn get_global(&mut self, name: &str, _ty: DiffValueType) -> Option<DiffValue> {
use wasm_spec_interpreter::{export, SpecExport::Global};
if let Ok(Global(g)) = export(&self.instance, name) {
Some(g.into())
} else {
panic!("expected an exported global value at name `{}`", name)
}
}
fn get_memory(&mut self, _name: &str, _shared: bool) -> Option<Vec<u8>> {
// TODO: should implement this
None
fn get_memory(&mut self, name: &str, _shared: bool) -> Option<Vec<u8>> {
use wasm_spec_interpreter::{export, SpecExport::Memory};
if let Ok(Memory(m)) = export(&self.instance, name) {
Some(m)
} else {
panic!("expected an exported memory at name `{}`", name)
}
}
}
impl From<&DiffValue> for Value {
impl From<&DiffValue> for SpecValue {
fn from(v: &DiffValue) -> Self {
match *v {
DiffValue::I32(n) => Value::I32(n),
DiffValue::I64(n) => Value::I64(n),
DiffValue::F32(n) => Value::F32(n as i32),
DiffValue::F64(n) => Value::F64(n as i64),
DiffValue::V128(n) => Value::V128(n.to_le_bytes().to_vec()),
DiffValue::I32(n) => SpecValue::I32(n),
DiffValue::I64(n) => SpecValue::I64(n),
DiffValue::F32(n) => SpecValue::F32(n as i32),
DiffValue::F64(n) => SpecValue::F64(n as i64),
DiffValue::V128(n) => SpecValue::V128(n.to_le_bytes().to_vec()),
DiffValue::FuncRef { .. } | DiffValue::ExternRef { .. } => unimplemented!(),
}
}
}
impl Into<DiffValue> for Value {
impl Into<DiffValue> for SpecValue {
fn into(self) -> DiffValue {
match self {
Value::I32(n) => DiffValue::I32(n),
Value::I64(n) => DiffValue::I64(n),
Value::F32(n) => DiffValue::F32(n as u32),
Value::F64(n) => DiffValue::F64(n as u64),
Value::V128(n) => {
SpecValue::I32(n) => DiffValue::I32(n),
SpecValue::I64(n) => DiffValue::I64(n),
SpecValue::F32(n) => DiffValue::F32(n as u32),
SpecValue::F64(n) => DiffValue::F64(n as u64),
SpecValue::V128(n) => {
assert_eq!(n.len(), 16);
DiffValue::V128(u128::from_le_bytes(n.as_slice().try_into().unwrap()))
}