Skip to content

Commit

Permalink
Improve fuzzing of *.wast tests (#9587)
Browse files Browse the repository at this point in the history
* Improve fuzzing of `*.wast` tests

Currently we have a fuzzer which is tasked with running `*.wast` tests
with fuzz-generated configurations. This asserts that we at least
satisfy all basic wasm semantics regardless of how various knobs in
`Config` are turned (modulo limits to resources). The current fuzzing
though is not comprehensive in that it doesn't include all the spec
tests that we pass from all proposals. This runs the risk of we don't
actually fuzz anything until the spec tests are merged upstream, which
can take a significant amount of time.

This commit refactors the `*.wast`-management infrastructure to share
test discovery and feature calculation between `tests/wast.rs` and
fuzzing. This new support crate centralizes limits and discovery for
both to use. Additionally fuzzing is updated to no longer throw out test
cases if configuration isn't applicable but instead clamp configuration
to the minimum required values (e.g. features + resource limits). This
means that we should now be fuzzing all spec tests that pass in all
configurations.

This new fuzzer discovered a few minor issues with the GC proposal
implementation, for example, such as:

* Some instructions were translated using trapping methods directly on
  `FunctionBuilder` rather than `FuncEnvironment` meaning they didn't
  properly handle `signals-based-traps` configuration.

* Fuel handling for `return_call_ref` wasn't correct because it was
  accidentally omitted from the list of return-call instructions that
  need special treatment.

* Add some manifest metadata
  • Loading branch information
alexcrichton authored Nov 11, 2024
1 parent 3c08980 commit f406347
Show file tree
Hide file tree
Showing 18 changed files with 707 additions and 492 deletions.
12 changes: 12 additions & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ capstone = { workspace = true }
object = { workspace = true, features = ['std'] }
wasmtime-test-macros = { path = "crates/test-macros" }
pulley-interpreter = { workspace = true, features = ["disas"] }
wasmtime-wast-util = { path = 'crates/wast-util' }

[target.'cfg(windows)'.dev-dependencies]
windows-sys = { workspace = true, features = ["Win32_System_Memory"] }
Expand Down
1 change: 1 addition & 0 deletions crates/cranelift/src/func_environ.rs
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,7 @@ impl<'module_environment> FuncEnvironment<'module_environment> {
| Operator::CallIndirect { .. }
| Operator::Call { .. }
| Operator::ReturnCall { .. }
| Operator::ReturnCallRef { .. }
| Operator::ReturnCallIndirect { .. } => {
self.fuel_increment_var(builder);
self.fuel_save_from_var(builder);
Expand Down
37 changes: 16 additions & 21 deletions crates/cranelift/src/gc/enabled.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use super::GcCompiler;
use crate::func_environ::FuncEnvironment;
use crate::gc::ArrayInit;
use crate::translate::{StructFieldsVec, TargetEnvironment};
use crate::translate::{FuncEnvironment as _, StructFieldsVec, TargetEnvironment};
use crate::TRAP_INTERNAL_ASSERT;
use cranelift_codegen::{
cursor::FuncCursor,
Expand Down Expand Up @@ -304,7 +304,7 @@ pub fn translate_struct_get(
// TODO: If we know we have a `(ref $my_struct)` here, instead of maybe a
// `(ref null $my_struct)`, we could omit the `trapz`. But plumbing that
// type info from `wasmparser` and through to here is a bit funky.
builder.ins().trapz(struct_ref, crate::TRAP_NULL_REFERENCE);
func_env.trapz(builder, struct_ref, crate::TRAP_NULL_REFERENCE);

let field_index = usize::try_from(field_index).unwrap();
let interned_type_index = func_env.module.types[struct_type_index];
Expand Down Expand Up @@ -337,7 +337,7 @@ fn translate_struct_get_and_extend(
extension: Extension,
) -> WasmResult<ir::Value> {
// TODO: See comment in `translate_struct_get` about the `trapz`.
builder.ins().trapz(struct_ref, crate::TRAP_NULL_REFERENCE);
func_env.trapz(builder, struct_ref, crate::TRAP_NULL_REFERENCE);

let field_index = usize::try_from(field_index).unwrap();
let interned_type_index = func_env.module.types[struct_type_index];
Expand Down Expand Up @@ -410,7 +410,7 @@ pub fn translate_struct_set(
new_val: ir::Value,
) -> WasmResult<()> {
// TODO: See comment in `translate_struct_get` about the `trapz`.
builder.ins().trapz(struct_ref, crate::TRAP_NULL_REFERENCE);
func_env.trapz(builder, struct_ref, crate::TRAP_NULL_REFERENCE);

let field_index = usize::try_from(field_index).unwrap();
let interned_type_index = func_env.module.types[struct_type_index];
Expand Down Expand Up @@ -640,15 +640,11 @@ pub fn translate_array_fill(
let len = translate_array_len(func_env, builder, array_ref)?;

// Check that the full range of elements we want to fill is within bounds.
let end_index = builder
.ins()
.uadd_overflow_trap(index, n, crate::TRAP_ARRAY_OUT_OF_BOUNDS);
let end_index = func_env.uadd_overflow_trap(builder, index, n, crate::TRAP_ARRAY_OUT_OF_BOUNDS);
let out_of_bounds = builder
.ins()
.icmp(IntCC::UnsignedGreaterThan, end_index, len);
builder
.ins()
.trapnz(out_of_bounds, crate::TRAP_ARRAY_OUT_OF_BOUNDS);
func_env.trapnz(builder, out_of_bounds, crate::TRAP_ARRAY_OUT_OF_BOUNDS);

// Get the address of the first element we want to fill.
let interned_type_index = func_env.module.types[array_type_index];
Expand Down Expand Up @@ -695,7 +691,7 @@ pub fn translate_array_len(
builder: &mut FunctionBuilder,
array_ref: ir::Value,
) -> WasmResult<ir::Value> {
builder.ins().trapz(array_ref, crate::TRAP_NULL_REFERENCE);
func_env.trapz(builder, array_ref, crate::TRAP_NULL_REFERENCE);

let len_offset = gc_compiler(func_env)?.layouts().array_length_field_offset();
let len_field = func_env.prepare_gc_ref_access(
Expand Down Expand Up @@ -794,9 +790,7 @@ fn array_elem_addr(
let len = translate_array_len(func_env, builder, array_ref).unwrap();

let in_bounds = builder.ins().icmp(IntCC::UnsignedLessThan, index, len);
builder
.ins()
.trapz(in_bounds, crate::TRAP_ARRAY_OUT_OF_BOUNDS);
func_env.trapz(builder, in_bounds, crate::TRAP_ARRAY_OUT_OF_BOUNDS);

// Compute the size (in bytes) of the whole array object.
let ArraySizeInfo {
Expand Down Expand Up @@ -1175,6 +1169,7 @@ fn uextend_i32_to_pointer_type(
/// Traps if the size overflows.
#[cfg_attr(not(any(feature = "gc-drc", feature = "gc-null")), allow(dead_code))]
fn emit_array_size(
func_env: &mut FuncEnvironment<'_>,
builder: &mut FunctionBuilder<'_>,
array_layout: &GcArrayLayout,
init: ArrayInit<'_>,
Expand All @@ -1200,17 +1195,17 @@ fn emit_array_size(
.ins()
.imul_imm(len, i64::from(array_layout.elem_size));
let high_bits = builder.ins().ushr_imm(elems_size_64, 32);
builder
.ins()
.trapnz(high_bits, crate::TRAP_ALLOCATION_TOO_LARGE);
func_env.trapnz(builder, high_bits, crate::TRAP_ALLOCATION_TOO_LARGE);
let elems_size = builder.ins().ireduce(ir::types::I32, elems_size_64);

// And if adding the base size and elements size overflows, then the
// allocation is too large.
let size =
builder
.ins()
.uadd_overflow_trap(base_size, elems_size, crate::TRAP_ALLOCATION_TOO_LARGE);
let size = func_env.uadd_overflow_trap(
builder,
base_size,
elems_size,
crate::TRAP_ALLOCATION_TOO_LARGE,
);

size
}
Expand Down
4 changes: 2 additions & 2 deletions crates/cranelift/src/gc/enabled/drc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,14 +303,14 @@ impl GcCompiler for DrcCompiler {
let interned_type_index = func_env.module.types[array_type_index];

let len_offset = gc_compiler(func_env)?.layouts().array_length_field_offset();
let array_layout = func_env.array_layout(interned_type_index);
let array_layout = func_env.array_layout(interned_type_index).clone();
let base_size = array_layout.base_size;
let align = array_layout.align;
let len_to_elems_delta = base_size.checked_sub(len_offset).unwrap();

// First, compute the array's total size from its base size, element
// size, and length.
let size = emit_array_size(builder, array_layout, init);
let size = emit_array_size(func_env, builder, &array_layout, init);

// Second, now that we have the array object's total size, call the
// `gc_alloc_raw` builtin libcall to allocate the array.
Expand Down
19 changes: 7 additions & 12 deletions crates/cranelift/src/gc/enabled/null.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,7 @@ impl NullCompiler {
.ins()
.iconst(ir::types::I32, i64::from(VMGcKind::MASK));
let masked = builder.ins().band(size, mask);
builder
.ins()
.trapnz(masked, crate::TRAP_ALLOCATION_TOO_LARGE);
func_env.trapnz(builder, masked, crate::TRAP_ALLOCATION_TOO_LARGE);

// Load the bump "pointer" (it is actually an index into the GC heap,
// not a raw pointer).
Expand All @@ -81,7 +79,8 @@ impl NullCompiler {
// a power of two.
let minus_one = builder.ins().iconst(ir::types::I32, -1);
let align_minus_one = builder.ins().iadd(align, minus_one);
let next_plus_align_minus_one = builder.ins().uadd_overflow_trap(
let next_plus_align_minus_one = func_env.uadd_overflow_trap(
builder,
next,
align_minus_one,
crate::TRAP_ALLOCATION_TOO_LARGE,
Expand All @@ -93,19 +92,15 @@ impl NullCompiler {

// Check whether the allocation fits in the heap space we have left.
let end_of_object =
builder
.ins()
.uadd_overflow_trap(aligned, size, crate::TRAP_ALLOCATION_TOO_LARGE);
func_env.uadd_overflow_trap(builder, aligned, size, crate::TRAP_ALLOCATION_TOO_LARGE);
let uext_end_of_object = uextend_i32_to_pointer_type(builder, pointer_type, end_of_object);
let (base, bound) = func_env.get_gc_heap_base_bound(builder);
let is_in_bounds = builder.ins().icmp(
ir::condcodes::IntCC::UnsignedLessThanOrEqual,
uext_end_of_object,
bound,
);
builder
.ins()
.trapz(is_in_bounds, crate::TRAP_ALLOCATION_TOO_LARGE);
func_env.trapz(builder, is_in_bounds, crate::TRAP_ALLOCATION_TOO_LARGE);

// Write the header, update the bump "pointer", and return the newly
// allocated object.
Expand Down Expand Up @@ -162,14 +157,14 @@ impl GcCompiler for NullCompiler {
let interned_type_index = func_env.module.types[array_type_index];

let len_offset = gc_compiler(func_env)?.layouts().array_length_field_offset();
let array_layout = func_env.array_layout(interned_type_index);
let array_layout = func_env.array_layout(interned_type_index).clone();
let base_size = array_layout.base_size;
let align = array_layout.align;
let len_to_elems_delta = base_size.checked_sub(len_offset).unwrap();

// First, compute the array's total size from its base size, element
// size, and length.
let size = emit_array_size(builder, array_layout, init);
let size = emit_array_size(func_env, builder, &array_layout, init);

// Next, allocate the array.
assert!(align.is_power_of_two());
Expand Down
6 changes: 5 additions & 1 deletion crates/fuzzing/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ license = "Apache-2.0 WITH LLVM-exception"
[lints]
workspace = true

[build-dependencies]
wasmtime-wast-util = { path = '../wast-util' }

[dependencies]
anyhow = { workspace = true }
arbitrary = { workspace = true, features = ["derive"] }
Expand All @@ -24,13 +27,14 @@ tempfile = "3.3.0"
wasmparser = { workspace = true }
wasmprinter = { workspace = true }
wasmtime = { workspace = true, features = ['default', 'winch', 'gc', 'memory-protection-keys'] }
wasmtime-wast = { workspace = true }
wasmtime-wast = { workspace = true, features = ['component-model'] }
wasm-encoder = { workspace = true }
wasm-smith = { workspace = true }
wasm-mutate = { workspace = true }
wasm-spec-interpreter = { path = "./wasm-spec-interpreter", optional = true }
wasmi = "0.39.1"
futures = { workspace = true }
wasmtime-wast-util = { path = '../wast-util' }

# We rely on precompiled v8 binaries, but rusty-v8 doesn't have a precompiled
# binary for MinGW which is built on our CI. It does have one for Windows-msvc,
Expand Down
46 changes: 23 additions & 23 deletions crates/fuzzing/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,39 +4,39 @@

use std::env;
use std::path::PathBuf;
use wasmtime_wast_util::WastTest;

fn main() {
println!("cargo:rerun-if-changed=build.rs");

let out_dir = PathBuf::from(env::var_os("OUT_DIR").unwrap());

let dirs = [
"tests/spec_testsuite",
"tests/misc_testsuite",
"tests/misc_testsuite/multi-memory",
"tests/misc_testsuite/simd",
"tests/misc_testsuite/threads",
];
let mut root = env::current_dir().unwrap();
root.pop(); // chop off 'fuzzing'
root.pop(); // chop off 'crates'
let mut code = format!("static FILES: &[(&str, &str)] = &[\n");

let mut entries = Vec::new();
for dir in dirs {
for entry in root.join(dir).read_dir().unwrap() {
let entry = entry.unwrap();
let path = entry.path();
if path.extension().and_then(|s| s.to_str()) == Some("wast") {
entries.push(path);
}
}
}
entries.sort();
for path in entries {
let path = path.to_str().expect("path is not valid utf-8");
code.push_str(&format!("({path:?}, include_str!({path:?})),\n"));

let tests = wasmtime_wast_util::find_tests(&root).unwrap();

let mut code = format!("static FILES: &[fn() -> wasmtime_wast_util::WastTest] = &[\n");

for test in tests {
let WastTest {
path,
contents: _,
config,
} = test;
println!("cargo:rerun-if-changed={}", path.to_str().unwrap());
code.push_str(&format!(
"|| {{
wasmtime_wast_util::WastTest {{
path: {path:?}.into(),
contents: include_str!({path:?}).into(),
config: wasmtime_wast_util::{config:?},
}}
}},"
));
}

code.push_str("];\n");
std::fs::write(out_dir.join("wasttests.rs"), code).unwrap();
}
Loading

0 comments on commit f406347

Please sign in to comment.