Skip to content

Commit 34b365d

Browse files
committed
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
1 parent ee131e8 commit 34b365d

File tree

10 files changed

+286
-11
lines changed

10 files changed

+286
-11
lines changed

.gitmodules

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
[submodule "walrus-tests/tests/spec-tests"]
2+
path = walrus-tests/tests/spec-tests
3+
url = https://github.com/WebAssembly/testsuite

src/ir/mod.rs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -506,6 +506,12 @@ pub enum UnaryOp {
506506
I64ReinterpretF64,
507507
F32ReinterpretI32,
508508
F64ReinterpretI64,
509+
510+
I32Extend8S,
511+
I32Extend16S,
512+
I64Extend8S,
513+
I64Extend16S,
514+
I64Extend32S,
509515
}
510516

511517
/// The different kinds of load instructions that are part of a `Load` IR node
@@ -528,6 +534,20 @@ pub enum LoadKind {
528534
I64_32 { sign_extend: bool },
529535
}
530536

537+
impl LoadKind {
538+
/// Returns the number of bytes loaded
539+
pub fn width(&self) -> u32 {
540+
use LoadKind::*;
541+
match self {
542+
I32_8 { .. } | I64_8 { .. } => 1,
543+
I32_16 { .. } | I64_16 { .. } => 2,
544+
I32 | F32 | I64_32 { .. } => 4,
545+
I64 | F64 => 8,
546+
V128 => 16,
547+
}
548+
}
549+
}
550+
531551
/// The different kinds of store instructions that are part of a `Store` IR node
532552
#[derive(Debug, Copy, Clone)]
533553
#[allow(missing_docs)]
@@ -544,6 +564,20 @@ pub enum StoreKind {
544564
I64_32,
545565
}
546566

567+
impl StoreKind {
568+
/// Returns the number of bytes stored
569+
pub fn width(&self) -> u32 {
570+
use StoreKind::*;
571+
match self {
572+
I32_8 | I64_8 => 1,
573+
I32_16 | I64_16 => 2,
574+
I32 | F32 | I64_32 => 4,
575+
I64 | F64 => 8,
576+
V128 => 16,
577+
}
578+
}
579+
}
580+
547581
/// Arguments to memory operations, containing a constant offset from a dynamic
548582
/// address as well as a predicted alignment.
549583
#[derive(Debug, Copy, Clone)]

src/module/exports.rs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,20 +3,19 @@
33
use super::globals::GlobalId;
44
use super::memories::MemoryId;
55
use super::tables::TableId;
6-
use crate::arena_set::ArenaSet;
76
use crate::emit::{Emit, EmitContext, IdsToIndices};
87
use crate::error::Result;
98
use crate::module::functions::FunctionId;
109
use crate::module::Module;
1110
use crate::parse::IndicesToIds;
12-
use id_arena::Id;
11+
use id_arena::{Arena, Id};
1312
use parity_wasm::elements;
1413

1514
/// The id of an export.
1615
pub type ExportId = Id<Export>;
1716

1817
/// A named item exported from the wasm.
19-
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
18+
#[derive(Clone, Debug)]
2019
pub struct Export {
2120
id: ExportId,
2221
/// The name of this export.
@@ -38,7 +37,7 @@ impl Export {
3837
}
3938

4039
/// An exported item.
41-
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
40+
#[derive(Copy, Clone, Debug)]
4241
pub enum ExportItem {
4342
/// An exported function.
4443
Function(FunctionId),
@@ -77,7 +76,7 @@ impl ExportItem {
7776
#[derive(Debug, Default)]
7877
pub struct ModuleExports {
7978
/// The arena containing this module's exports.
80-
arena: ArenaSet<Export>,
79+
arena: Arena<Export>,
8180
}
8281

8382
impl ModuleExports {
@@ -112,7 +111,7 @@ impl Module {
112111
elements::Internal::Global(t) => ExportItem::Global(ids.get_global(t)?),
113112
};
114113
let id = self.exports.arena.next_id();
115-
self.exports.arena.insert(Export {
114+
self.exports.arena.alloc(Export {
116115
id,
117116
name: exp.field().to_string(),
118117
item,

src/module/functions/local_function/emit.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,12 @@ impl Emit<'_> {
232232
I64ReinterpretF64 => elements::Instruction::I64ReinterpretF64,
233233
F32ReinterpretI32 => elements::Instruction::F32ReinterpretI32,
234234
F64ReinterpretI64 => elements::Instruction::F64ReinterpretI64,
235+
236+
I32Extend8S => elements::Instruction::I32Extend8S,
237+
I32Extend16S => elements::Instruction::I32Extend16S,
238+
I64Extend8S => elements::Instruction::I64Extend8S,
239+
I64Extend16S => elements::Instruction::I64Extend16S,
240+
I64Extend32S => elements::Instruction::I64Extend32S,
235241
})
236242
}
237243

src/module/functions/local_function/mod.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,7 @@ fn validate_instruction<'a>(
403403

404404
let memarg = |align, offset| -> Result<MemArg> {
405405
if align >= 32 {
406-
failure::bail!("invalid alignment")
406+
failure::bail!("invalid alignment");
407407
}
408408
Ok(MemArg {
409409
align: 1 << (align as i32),
@@ -675,6 +675,12 @@ fn validate_instruction<'a>(
675675
Instruction::F32ReinterpretI32 => one_op(ctx, I32, F32, UnaryOp::F32ReinterpretI32)?,
676676
Instruction::F64ReinterpretI64 => one_op(ctx, I64, F64, UnaryOp::F64ReinterpretI64)?,
677677

678+
Instruction::I32Extend8S => one_op(ctx, I32, I32, UnaryOp::I32Extend8S)?,
679+
Instruction::I32Extend16S => one_op(ctx, I32, I32, UnaryOp::I32Extend16S)?,
680+
Instruction::I64Extend8S => one_op(ctx, I64, I64, UnaryOp::I64Extend8S)?,
681+
Instruction::I64Extend16S => one_op(ctx, I64, I64, UnaryOp::I64Extend16S)?,
682+
Instruction::I64Extend32S => one_op(ctx, I64, I64, UnaryOp::I64Extend32S)?,
683+
678684
Instruction::Drop => {
679685
let (_, expr) = ctx.pop_operand()?;
680686
let expr = ctx.func.alloc(Drop { expr });

src/passes/validate.rs

Lines changed: 104 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,27 @@
55
66
use crate::const_value::Const;
77
use crate::error::Result;
8-
use crate::ir::Value;
8+
use crate::ir::*;
9+
use crate::module::functions::{Function, FunctionKind, LocalFunction};
910
use crate::module::globals::{Global, GlobalKind};
1011
use crate::module::memories::Memory;
1112
use crate::module::tables::{Table, TableKind};
1213
use crate::module::Module;
1314
use crate::ty::ValType;
1415
use failure::{bail, ResultExt};
16+
use std::collections::HashSet;
1517

1618
/// Validate a wasm module, returning an error if it fails to validate.
1719
pub fn run(module: &Module) -> Result<()> {
20+
// TODO: should a config option be added to lift these restrictions? They're
21+
// only here for the spec tests...
22+
if module.tables.iter().count() > 1 {
23+
bail!("multiple tables not allowed in the wasm spec yet");
24+
}
25+
if module.memories.iter().count() > 1 {
26+
bail!("multiple memories not allowed in the wasm spec yet");
27+
}
28+
1829
for memory in module.memories.iter() {
1930
validate_memory(memory)?;
2031
}
@@ -24,11 +35,48 @@ pub fn run(module: &Module) -> Result<()> {
2435
for global in module.globals.iter() {
2536
validate_global(module, global)?;
2637
}
27-
Ok(())
38+
validate_exports(module)?;
39+
40+
// Validate the start function, if present, has the correct signature
41+
if let Some(start) = module.start {
42+
let ty = module.funcs.get(start).ty();
43+
let ty = module.types.get(ty);
44+
if ty.results().len() > 0 || ty.params().len() > 0 {
45+
bail!("start function must take no arguments and return nothing");
46+
}
47+
}
48+
49+
// Validate each function in the module, collecting errors and returning
50+
// them all at once if there are any.
51+
let mut errs = Vec::new();
52+
for function in module.funcs.iter() {
53+
let local = match &function.kind {
54+
FunctionKind::Local(local) => local,
55+
_ => continue,
56+
};
57+
let mut cx = Validate {
58+
errs: &mut errs,
59+
function,
60+
local,
61+
};
62+
local.entry_block().visit(&mut cx);
63+
}
64+
if errs.len() == 0 {
65+
return Ok(());
66+
}
67+
68+
let mut msg = format!("errors validating module:\n");
69+
for error in errs {
70+
msg.push_str(&format!(" * {}\n", error));
71+
for cause in error.iter_causes() {
72+
msg.push_str(&format!(" * {}\n", cause));
73+
}
74+
}
75+
bail!("{}", msg)
2876
}
2977

3078
fn validate_memory(m: &Memory) -> Result<()> {
31-
validate_limits(m.initial, m.maximum, u16::max_value().into())
79+
validate_limits(m.initial, m.maximum, u32::from(u16::max_value()) + 1)
3280
.context("when validating a memory")?;
3381
Ok(())
3482
}
@@ -61,6 +109,18 @@ fn validate_limits(initial: u32, maximum: Option<u32>, k: u32) -> Result<()> {
61109
}
62110
}
63111

112+
fn validate_exports(module: &Module) -> Result<()> {
113+
// All exported names must be unique, so if there's any duplicate-named
114+
// exports then we generate an error
115+
let mut exports = HashSet::new();
116+
for export in module.exports.iter() {
117+
if !exports.insert(&export.name) {
118+
bail!("duplicate export of `{}`", export.name)
119+
}
120+
}
121+
Ok(())
122+
}
123+
64124
fn validate_global(module: &Module, global: &Global) -> Result<()> {
65125
match global.kind {
66126
GlobalKind::Import(_) => return Ok(()),
@@ -94,3 +154,44 @@ fn validate_value(value: Value, ty: ValType) -> Result<()> {
94154
}
95155
Ok(())
96156
}
157+
158+
struct Validate<'a> {
159+
errs: &'a mut Vec<failure::Error>,
160+
function: &'a Function,
161+
local: &'a LocalFunction,
162+
}
163+
164+
impl Validate<'_> {
165+
fn memarg(&mut self, arg: &MemArg, width: u32) {
166+
// The alignment of a memory operation must be less than or equal to the
167+
// width of the memory operation, currently wasm doesn't allow
168+
// over-aligned memory ops.
169+
if arg.align > width {
170+
self.err("memory operation with alignment greater than natural size");
171+
}
172+
}
173+
174+
fn err(&mut self, msg: &str) {
175+
let mut err = failure::format_err!("{}", msg);
176+
if let Some(name) = &self.function.name {
177+
err = err.context(format!("in function {}", name)).into();
178+
}
179+
self.errs.push(err);
180+
}
181+
}
182+
183+
impl<'a> Visitor<'a> for Validate<'a> {
184+
fn local_function(&self) -> &'a LocalFunction {
185+
self.local
186+
}
187+
188+
fn visit_load(&mut self, e: &Load) {
189+
self.memarg(&e.arg, e.kind.width());
190+
e.visit(self);
191+
}
192+
193+
fn visit_store(&mut self, e: &Store) {
194+
self.memarg(&e.arg, e.kind.width());
195+
e.visit(self);
196+
}
197+
}

walrus-tests/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ failure = "0.1.2"
1212
parity-wasm = "0.35.7"
1313
walrus = { path = ".." }
1414
walrus-tests-utils = { path = "../walrus-tests-utils" }
15+
tempfile = "3"
16+
serde_json = { version = "1", features = ['preserve_order'] }
17+
serde = { version = "1", features = ['derive'] }
1518

1619
[lib]
1720
doctest = false

walrus-tests/build.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@ where
1212
println!("cargo:rerun-if-changed={}", dir.as_ref().display());
1313
for entry in WalkDir::new(dir) {
1414
let entry = entry.unwrap();
15-
if entry.path().extension() == Some(OsStr::new("wat")) {
15+
if entry.path().extension() == Some(OsStr::new("wat"))
16+
|| entry.path().extension() == Some(OsStr::new("wast"))
17+
{
1618
println!("cargo:rerun-if-changed={}", entry.path().display());
1719
f(entry.path());
1820
}
@@ -54,4 +56,5 @@ fn main() {
5456
generate_tests("valid");
5557
generate_tests("round_trip");
5658
generate_tests("ir");
59+
generate_tests("spec-tests");
5760
}

walrus-tests/tests/spec-tests

Submodule spec-tests added at c8d7b08

0 commit comments

Comments
 (0)