Add a suite of spec tests

The WebAssembly organization on GitHub has a `spec-tests` repository
which contains a collection of tests for WebAssembly implementations.
These tests are quite comprehensive and a great way to ensure good
coverage within our own engine of WebAssembly features, even as they're
added to the spec over time.

This commit adds a submodule to the spec-tests repository and then hooks
up a suite of tests to run them. To "run" a spec test means:

* First, the `wabt` toolkit's `wast2json` tool is executed, performing
  two operations. First, as the name implies, it parses the `*.wast`
  file and emits a `*.json` file describing the test. This `*.json` file
  is much easier to parse for us thatn `*.wast`. Second the tool will
  extract a number of `*.wasm` files from the test, emitting them to the
  filesystem.

* Next we look at all the test directives in the JSON file. For anything
  that's supposed to be an invalid/malformed module we ensure that our
  parsing fails (as our parsing currently bundles validation).

* For all directives which are a valid wasm test, we parse the
  referenced file and then serialize it back out, round-tripping the
  wasm file through `walrus`.

* Finally we execute `wabt`'s tool `spectest-interp`. This takes in the
  JSON file as input and then executes all of the tests, presumably in
  its built-in interpreter. This hopefully ensures that we actually
  emitted the correct wasm, as the tool is ingesting wasm that walrus
  produced.

Turns out we were already passing all the spec tests basically (yay!)
except for a few more validations added here:

* Exports are now an `Arena` instead of an `ArenaSet` to generate an
  error when two exports have the same name (and the same type).
  Previously they silently overrode each other.

* Alignment on loads/stores are validated to be less than the width of
  the load/store operation.

* The signature of the start function is checked.

* Added support for the sign-extension-ops proposal

.gitmodules

@@ -0,0 +1,3 @@
+[submodule "walrus-tests/tests/spec-tests"]
+	path = walrus-tests/tests/spec-tests
+	url = https://github.com/WebAssembly/testsuite

src/ir/mod.rs

@@ -506,6 +506,12 @@ pub enum UnaryOp {
     I64ReinterpretF64,
     F32ReinterpretI32,
     F64ReinterpretI64,
+
+    I32Extend8S,
+    I32Extend16S,
+    I64Extend8S,
+    I64Extend16S,
+    I64Extend32S,
 }
 
 /// The different kinds of load instructions that are part of a `Load` IR node
@@ -528,6 +534,20 @@ pub enum LoadKind {
     I64_32 { sign_extend: bool },
 }
 
+impl LoadKind {
+    /// Returns the number of bytes loaded
+    pub fn width(&self) -> u32 {
+        use LoadKind::*;
+        match self {
+            I32_8 { .. } | I64_8 { .. } => 1,
+            I32_16 { .. } | I64_16 { .. } => 2,
+            I32 | F32 | I64_32 { .. } => 4,
+            I64 | F64 => 8,
+            V128 => 16,
+        }
+    }
+}
+
 /// The different kinds of store instructions that are part of a `Store` IR node
 #[derive(Debug, Copy, Clone)]
 #[allow(missing_docs)]
@@ -544,6 +564,20 @@ pub enum StoreKind {
     I64_32,
 }
 
+impl StoreKind {
+    /// Returns the number of bytes stored
+    pub fn width(&self) -> u32 {
+        use StoreKind::*;
+        match self {
+            I32_8 | I64_8 => 1,
+            I32_16 | I64_16 => 2,
+            I32 | F32 | I64_32 => 4,
+            I64 | F64 => 8,
+            V128 => 16,
+        }
+    }
+}
+
 /// Arguments to memory operations, containing a constant offset from a dynamic
 /// address as well as a predicted alignment.
 #[derive(Debug, Copy, Clone)]

src/module/exports.rs

@@ -3,20 +3,19 @@
 use super::globals::GlobalId;
 use super::memories::MemoryId;
 use super::tables::TableId;
-use crate::arena_set::ArenaSet;
 use crate::emit::{Emit, EmitContext, IdsToIndices};
 use crate::error::Result;
 use crate::module::functions::FunctionId;
 use crate::module::Module;
 use crate::parse::IndicesToIds;
-use id_arena::Id;
+use id_arena::{Arena, Id};
 use parity_wasm::elements;
 
 /// The id of an export.
 pub type ExportId = Id<Export>;
 
 /// A named item exported from the wasm.
-#[derive(Clone, Debug, PartialEq, Eq, Hash)]
+#[derive(Clone, Debug)]
 pub struct Export {
     id: ExportId,
     /// The name of this export.
@@ -38,7 +37,7 @@ impl Export {
 }
 
 /// An exported item.
-#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
+#[derive(Copy, Clone, Debug)]
 pub enum ExportItem {
     /// An exported function.
     Function(FunctionId),
@@ -77,7 +76,7 @@ impl ExportItem {
 #[derive(Debug, Default)]
 pub struct ModuleExports {
     /// The arena containing this module's exports.
-    arena: ArenaSet<Export>,
+    arena: Arena<Export>,
 }
 
 impl ModuleExports {
@@ -112,7 +111,7 @@ impl Module {
                 elements::Internal::Global(t) => ExportItem::Global(ids.get_global(t)?),
             };
             let id = self.exports.arena.next_id();
-            self.exports.arena.insert(Export {
+            self.exports.arena.alloc(Export {
                 id,
                 name: exp.field().to_string(),
                 item,

src/module/functions/local_function/emit.rs

@@ -232,6 +232,12 @@ impl Emit<'_> {
                     I64ReinterpretF64 => elements::Instruction::I64ReinterpretF64,
                     F32ReinterpretI32 => elements::Instruction::F32ReinterpretI32,
                     F64ReinterpretI64 => elements::Instruction::F64ReinterpretI64,
+
+                    I32Extend8S => elements::Instruction::I32Extend8S,
+                    I32Extend16S => elements::Instruction::I32Extend16S,
+                    I64Extend8S => elements::Instruction::I64Extend8S,
+                    I64Extend16S => elements::Instruction::I64Extend16S,
+                    I64Extend32S => elements::Instruction::I64Extend32S,
                 })
             }
 

src/module/functions/local_function/mod.rs

@@ -403,7 +403,7 @@ fn validate_instruction<'a>(
 
     let memarg = |align, offset| -> Result<MemArg> {
         if align >= 32 {
-            failure::bail!("invalid alignment")
+            failure::bail!("invalid alignment");
         }
         Ok(MemArg {
             align: 1 << (align as i32),
@@ -675,6 +675,12 @@ fn validate_instruction<'a>(
         Instruction::F32ReinterpretI32 => one_op(ctx, I32, F32, UnaryOp::F32ReinterpretI32)?,
         Instruction::F64ReinterpretI64 => one_op(ctx, I64, F64, UnaryOp::F64ReinterpretI64)?,
 
+        Instruction::I32Extend8S => one_op(ctx, I32, I32, UnaryOp::I32Extend8S)?,
+        Instruction::I32Extend16S => one_op(ctx, I32, I32, UnaryOp::I32Extend16S)?,
+        Instruction::I64Extend8S => one_op(ctx, I64, I64, UnaryOp::I64Extend8S)?,
+        Instruction::I64Extend16S => one_op(ctx, I64, I64, UnaryOp::I64Extend16S)?,
+        Instruction::I64Extend32S => one_op(ctx, I64, I64, UnaryOp::I64Extend32S)?,
+
         Instruction::Drop => {
             let (_, expr) = ctx.pop_operand()?;
             let expr = ctx.func.alloc(Drop { expr });

src/passes/validate.rs

@@ -5,16 +5,27 @@
 
 use crate::const_value::Const;
 use crate::error::Result;
-use crate::ir::Value;
+use crate::ir::*;
+use crate::module::functions::{Function, FunctionKind, LocalFunction};
 use crate::module::globals::{Global, GlobalKind};
 use crate::module::memories::Memory;
 use crate::module::tables::{Table, TableKind};
 use crate::module::Module;
 use crate::ty::ValType;
 use failure::{bail, ResultExt};
+use std::collections::HashSet;
 
 /// Validate a wasm module, returning an error if it fails to validate.
 pub fn run(module: &Module) -> Result<()> {
+    // TODO: should a config option be added to lift these restrictions? They're
+    // only here for the spec tests...
+    if module.tables.iter().count() > 1 {
+        bail!("multiple tables not allowed in the wasm spec yet");
+    }
+    if module.memories.iter().count() > 1 {
+        bail!("multiple memories not allowed in the wasm spec yet");
+    }
+
     for memory in module.memories.iter() {
         validate_memory(memory)?;
     }
@@ -24,11 +35,48 @@ pub fn run(module: &Module) -> Result<()> {
     for global in module.globals.iter() {
         validate_global(module, global)?;
     }
-    Ok(())
+    validate_exports(module)?;
+
+    // Validate the start function, if present, has the correct signature
+    if let Some(start) = module.start {
+        let ty = module.funcs.get(start).ty();
+        let ty = module.types.get(ty);
+        if ty.results().len() > 0 || ty.params().len() > 0 {
+            bail!("start function must take no arguments and return nothing");
+        }
+    }
+
+    // Validate each function in the module, collecting errors and returning
+    // them all at once if there are any.
+    let mut errs = Vec::new();
+    for function in module.funcs.iter() {
+        let local = match &function.kind {
+            FunctionKind::Local(local) => local,
+            _ => continue,
+        };
+        let mut cx = Validate {
+            errs: &mut errs,
+            function,
+            local,
+        };
+        local.entry_block().visit(&mut cx);
+    }
+    if errs.len() == 0 {
+        return Ok(());
+    }
+
+    let mut msg = format!("errors validating module:\n");
+    for error in errs {
+        msg.push_str(&format!("  * {}\n", error));
+        for cause in error.iter_causes() {
+            msg.push_str(&format!("    * {}\n", cause));
+        }
+    }
+    bail!("{}", msg)
 }
 
 fn validate_memory(m: &Memory) -> Result<()> {
-    validate_limits(m.initial, m.maximum, u16::max_value().into())
+    validate_limits(m.initial, m.maximum, u32::from(u16::max_value()) + 1)
         .context("when validating a memory")?;
     Ok(())
 }
@@ -61,6 +109,18 @@ fn validate_limits(initial: u32, maximum: Option<u32>, k: u32) -> Result<()> {
     }
 }
 
+fn validate_exports(module: &Module) -> Result<()> {
+    // All exported names must be unique, so if there's any duplicate-named
+    // exports then we generate an error
+    let mut exports = HashSet::new();
+    for export in module.exports.iter() {
+        if !exports.insert(&export.name) {
+            bail!("duplicate export of `{}`", export.name)
+        }
+    }
+    Ok(())
+}
+
 fn validate_global(module: &Module, global: &Global) -> Result<()> {
     match global.kind {
         GlobalKind::Import(_) => return Ok(()),
@@ -94,3 +154,44 @@ fn validate_value(value: Value, ty: ValType) -> Result<()> {
     }
     Ok(())
 }
+
+struct Validate<'a> {
+    errs: &'a mut Vec<failure::Error>,
+    function: &'a Function,
+    local: &'a LocalFunction,
+}
+
+impl Validate<'_> {
+    fn memarg(&mut self, arg: &MemArg, width: u32) {
+        // The alignment of a memory operation must be less than or equal to the
+        // width of the memory operation, currently wasm doesn't allow
+        // over-aligned memory ops.
+        if arg.align > width {
+            self.err("memory operation with alignment greater than natural size");
+        }
+    }
+
+    fn err(&mut self, msg: &str) {
+        let mut err = failure::format_err!("{}", msg);
+        if let Some(name) = &self.function.name {
+            err = err.context(format!("in function {}", name)).into();
+        }
+        self.errs.push(err);
+    }
+}
+
+impl<'a> Visitor<'a> for Validate<'a> {
+    fn local_function(&self) -> &'a LocalFunction {
+        self.local
+    }
+
+    fn visit_load(&mut self, e: &Load) {
+        self.memarg(&e.arg, e.kind.width());
+        e.visit(self);
+    }
+
+    fn visit_store(&mut self, e: &Store) {
+        self.memarg(&e.arg, e.kind.width());
+        e.visit(self);
+    }
+}

walrus-tests/Cargo.toml

@@ -12,6 +12,9 @@ failure = "0.1.2"
 parity-wasm = "0.35.7"
 walrus = { path = ".." }
 walrus-tests-utils = { path = "../walrus-tests-utils" }
+tempfile = "3"
+serde_json = { version = "1", features = ['preserve_order'] }
+serde = { version = "1", features = ['derive'] }
 
 [lib]
 doctest = false

walrus-tests/build.rs

@@ -12,7 +12,9 @@ where
     println!("cargo:rerun-if-changed={}", dir.as_ref().display());
     for entry in WalkDir::new(dir) {
         let entry = entry.unwrap();
-        if entry.path().extension() == Some(OsStr::new("wat")) {
+        if entry.path().extension() == Some(OsStr::new("wat"))
+            || entry.path().extension() == Some(OsStr::new("wast"))
+        {
             println!("cargo:rerun-if-changed={}", entry.path().display());
             f(entry.path());
         }
@@ -54,4 +56,5 @@ fn main() {
     generate_tests("valid");
     generate_tests("round_trip");
     generate_tests("ir");
+    generate_tests("spec-tests");
 }