Skip to content

Commit 5b9e876

Browse files
authored
Enable the GC proposal during general fuzzing (#10332)
* Enable the GC proposal during general fuzzing This allows us to fuzz Wasm GC in our fuzz targets that use the common config-generation infrastructure, such as the differential fuzz target. Fixes #10328 * Make handling of non-deterministic errors more robust in differential fuzzer * remove logging from functions that can be called from signal handlers
1 parent 840f544 commit 5b9e876

File tree

12 files changed

+153
-86
lines changed

12 files changed

+153
-86
lines changed

crates/fuzzing/src/generators/config.rs

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -527,7 +527,7 @@ impl<'a> Arbitrary<'a> for Config {
527527

528528
config
529529
.wasmtime
530-
.update_module_config(&mut config.module_config.config, u)?;
530+
.update_module_config(&mut config.module_config, u)?;
531531

532532
Ok(config)
533533
}
@@ -604,7 +604,7 @@ impl WasmtimeConfig {
604604
/// too.
605605
pub fn update_module_config(
606606
&mut self,
607-
config: &mut wasm_smith::Config,
607+
config: &mut ModuleConfig,
608608
u: &mut Unstructured<'_>,
609609
) -> arbitrary::Result<()> {
610610
match self.compiler_strategy {
@@ -627,10 +627,11 @@ impl WasmtimeConfig {
627627
// at this time, so if winch is selected be sure to disable wasm
628628
// proposals in `Config` to ensure that Winch can compile the
629629
// module that wasm-smith generates.
630-
config.relaxed_simd_enabled = false;
631-
config.gc_enabled = false;
632-
config.tail_call_enabled = false;
633-
config.reference_types_enabled = false;
630+
config.config.relaxed_simd_enabled = false;
631+
config.config.gc_enabled = false;
632+
config.config.tail_call_enabled = false;
633+
config.config.reference_types_enabled = false;
634+
config.function_references_enabled = false;
634635

635636
// Winch's SIMD implementations require AVX and AVX2.
636637
if self
@@ -640,7 +641,7 @@ impl WasmtimeConfig {
640641
.codegen_flag("has_avx2")
641642
.is_some_and(|value| value == "false")
642643
{
643-
config.simd_enabled = false;
644+
config.config.simd_enabled = false;
644645
}
645646

646647
// Tuning the following engine options is currently not supported
@@ -651,7 +652,7 @@ impl WasmtimeConfig {
651652
}
652653

653654
CompilerStrategy::CraneliftPulley => {
654-
config.threads_enabled = false;
655+
config.config.threads_enabled = false;
655656
}
656657
}
657658

@@ -661,7 +662,8 @@ impl WasmtimeConfig {
661662
// and for wasm threads that will require some refactoring of the
662663
// `LinearMemory` trait to bubble up the request that the linear memory
663664
// not move. Otherwise that just generates a panic right now.
664-
if config.threads_enabled || matches!(self.strategy, InstanceAllocationStrategy::Pooling(_))
665+
if config.config.threads_enabled
666+
|| matches!(self.strategy, InstanceAllocationStrategy::Pooling(_))
665667
{
666668
self.avoid_custom_unaligned_memory(u)?;
667669
}
@@ -672,31 +674,38 @@ impl WasmtimeConfig {
672674
// If the pooling allocator is used, do not allow shared memory to
673675
// be created. FIXME: see
674676
// https://github.com/bytecodealliance/wasmtime/issues/4244.
675-
config.threads_enabled = false;
677+
config.config.threads_enabled = false;
676678

677679
// Ensure the pooling allocator can support the maximal size of
678680
// memory, picking the smaller of the two to win.
679681
let min_bytes = config
682+
.config
680683
.max_memory32_bytes
681684
// memory64_bytes is a u128, but since we are taking the min
682685
// we can truncate it down to a u64.
683-
.min(config.max_memory64_bytes.try_into().unwrap_or(u64::MAX));
686+
.min(
687+
config
688+
.config
689+
.max_memory64_bytes
690+
.try_into()
691+
.unwrap_or(u64::MAX),
692+
);
684693
let mut min = min_bytes.min(pooling.max_memory_size as u64);
685694
if let MemoryConfig::Normal(cfg) = &self.memory_config {
686695
min = min.min(cfg.memory_reservation.unwrap_or(0));
687696
}
688697
pooling.max_memory_size = min as usize;
689-
config.max_memory32_bytes = min;
690-
config.max_memory64_bytes = min as u128;
698+
config.config.max_memory32_bytes = min;
699+
config.config.max_memory64_bytes = min as u128;
691700

692701
// If traps are disallowed then memories must have at least one page
693702
// of memory so if we still are only allowing 0 pages of memory then
694703
// increase that to one here.
695-
if config.disallow_traps {
704+
if config.config.disallow_traps {
696705
if pooling.max_memory_size < (1 << 16) {
697706
pooling.max_memory_size = 1 << 16;
698-
config.max_memory32_bytes = 1 << 16;
699-
config.max_memory64_bytes = 1 << 16;
707+
config.config.max_memory32_bytes = 1 << 16;
708+
config.config.max_memory64_bytes = 1 << 16;
700709
if let MemoryConfig::Normal(cfg) = &mut self.memory_config {
701710
match &mut cfg.memory_reservation {
702711
Some(size) => *size = (*size).max(pooling.max_memory_size as u64),
@@ -712,13 +721,13 @@ impl WasmtimeConfig {
712721

713722
// Don't allow too many linear memories per instance since massive
714723
// virtual mappings can fail to get allocated.
715-
config.min_memories = config.min_memories.min(10);
716-
config.max_memories = config.max_memories.min(10);
724+
config.config.min_memories = config.config.min_memories.min(10);
725+
config.config.max_memories = config.config.max_memories.min(10);
717726

718727
// Force this pooling allocator to always be able to accommodate the
719728
// module that may be generated.
720-
pooling.total_memories = config.max_memories as u32;
721-
pooling.total_tables = config.max_tables as u32;
729+
pooling.total_memories = config.config.max_memories as u32;
730+
pooling.total_tables = config.config.max_tables as u32;
722731
}
723732

724733
if !self.signals_based_traps {
@@ -728,7 +737,7 @@ impl WasmtimeConfig {
728737
// fixable with some more work on the bounds-checks side of things
729738
// to do a full bounds check even on static memories, but that's
730739
// left for a future PR.
731-
config.threads_enabled = false;
740+
config.config.threads_enabled = false;
732741

733742
// Spectre-based heap mitigations require signal handlers so this
734743
// must always be disabled if signals-based traps are disabled.

crates/fuzzing/src/generators/module.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ impl<'a> Arbitrary<'a> for ModuleConfig {
4141
let _ = config.relaxed_simd_enabled;
4242
let _ = config.tail_call_enabled;
4343
let _ = config.extended_const_enabled;
44+
let _ = config.gc_enabled;
4445
config.exceptions_enabled = false;
45-
config.gc_enabled = false;
4646
config.custom_page_sizes_enabled = u.arbitrary()?;
4747
config.wide_arithmetic_enabled = u.arbitrary()?;
4848
config.memory64_enabled = u.ratio(1, 20)?;

crates/fuzzing/src/generators/value.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,7 @@ impl PartialEq for DiffValue {
267267
}
268268
(Self::FuncRef { null: a }, Self::FuncRef { null: b }) => a == b,
269269
(Self::ExternRef { null: a }, Self::ExternRef { null: b }) => a == b,
270+
(Self::AnyRef { null: a }, Self::AnyRef { null: b }) => a == b,
270271
_ => false,
271272
}
272273
}
@@ -302,7 +303,7 @@ impl TryFrom<wasmtime::ValType> for DiffValueType {
302303
(true, HeapType::Any) => Ok(Self::AnyRef),
303304
(true, HeapType::I31) => Ok(Self::AnyRef),
304305
(true, HeapType::None) => Ok(Self::AnyRef),
305-
_ => Err("non-funcref and non-externref reference types are not supported yet"),
306+
_ => Err("non-null reference types are not supported yet"),
306307
},
307308
}
308309
}

crates/fuzzing/src/oracles.rs

Lines changed: 59 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -369,8 +369,11 @@ pub fn instantiate_with_dummy(store: &mut Store<StoreLimits>, module: &Module) -
369369
// Creation of imports can fail due to resource limit constraints, and then
370370
// instantiation can naturally fail for a number of reasons as well. Bundle
371371
// the two steps together to match on the error below.
372-
let instance =
373-
dummy::dummy_linker(store, module).and_then(|l| l.instantiate(&mut *store, module));
372+
let linker = dummy::dummy_linker(store, module);
373+
if let Err(e) = &linker {
374+
log::warn!("failed to create dummy linker: {e:?}");
375+
}
376+
let instance = linker.and_then(|l| l.instantiate(&mut *store, module));
374377
unwrap_instance(store, instance)
375378
}
376379

@@ -507,6 +510,32 @@ pub enum DiffEqResult<T, U> {
507510
Failed,
508511
}
509512

513+
fn wasmtime_trap_is_non_deterministic(trap: &Trap) -> bool {
514+
match trap {
515+
// Allocations being too large for the GC are
516+
// implementation-defined.
517+
Trap::AllocationTooLarge |
518+
// Stack size, and therefore when overflow happens, is
519+
// implementation-defined.
520+
Trap::StackOverflow => true,
521+
_ => false,
522+
}
523+
}
524+
525+
fn wasmtime_error_is_non_deterministic(error: &wasmtime::Error) -> bool {
526+
match error.downcast_ref::<Trap>() {
527+
Some(trap) => wasmtime_trap_is_non_deterministic(trap),
528+
529+
// For general, unknown errors, we can't rely on this being
530+
// a deterministic Wasm failure that both engines handled
531+
// identically, leaving Wasm in identical states. We could
532+
// just as easily be hitting engine-specific failures, like
533+
// different implementation-defined limits. So simply poison
534+
// this execution and move on to the next test.
535+
None => true,
536+
}
537+
}
538+
510539
impl<T, U> DiffEqResult<T, U> {
511540
/// Computes the differential result from executing in two different
512541
/// engines.
@@ -518,41 +547,37 @@ impl<T, U> DiffEqResult<T, U> {
518547
match (lhs_result, rhs_result) {
519548
(Ok(lhs_result), Ok(rhs_result)) => DiffEqResult::Success(lhs_result, rhs_result),
520549

521-
// Both sides failed. Check that the trap and state at the time of
522-
// failure is the same, when possible.
523-
(Err(lhs), Err(rhs)) => {
524-
let rhs = match rhs.downcast::<Trap>() {
525-
Ok(trap) => trap,
526-
527-
// For general, unknown errors, we can't rely on this being
528-
// a deterministic Wasm failure that both engines handled
529-
// identically, leaving Wasm in identical states. We could
530-
// just as easily be hitting engine-specific failures, like
531-
// different implementation-defined limits. So simply report
532-
// failure and move on to the next test.
533-
Err(err) => {
534-
log::debug!("rhs failed: {err:?}");
535-
return DiffEqResult::Failed;
536-
}
537-
};
550+
// Handle all non-deterministic errors by poisoning this execution's
551+
// state, so that we simply move on to the next test.
552+
(Err(lhs), _) if lhs_engine.is_non_deterministic_error(&lhs) => {
553+
log::debug!("lhs failed non-deterministically: {lhs:?}");
554+
DiffEqResult::Poisoned
555+
}
556+
(_, Err(rhs)) if wasmtime_error_is_non_deterministic(&rhs) => {
557+
log::debug!("rhs failed non-deterministically: {rhs:?}");
558+
DiffEqResult::Poisoned
559+
}
538560

539-
// Even some traps are nondeterministic, and we can't rely on
540-
// the errors matching or leaving Wasm in the same state.
541-
let poisoned =
542-
// Allocations being too large for the GC are
543-
// implementation-defined.
544-
rhs == Trap::AllocationTooLarge
545-
// Stack size, and therefore when overflow happens, is
546-
// implementation-defined.
547-
|| rhs == Trap::StackOverflow
548-
|| lhs_engine.is_stack_overflow(&lhs);
549-
if poisoned {
550-
return DiffEqResult::Poisoned;
551-
}
561+
// Both sides failed deterministically. Check that the trap and
562+
// state at the time of failure is the same.
563+
(Err(lhs), Err(rhs)) => {
564+
let rhs = rhs
565+
.downcast::<Trap>()
566+
.expect("non-traps handled in earlier match arm");
567+
568+
debug_assert!(
569+
!lhs_engine.is_non_deterministic_error(&lhs),
570+
"non-deterministic traps handled in earlier match arm",
571+
);
572+
debug_assert!(
573+
!wasmtime_trap_is_non_deterministic(&rhs),
574+
"non-deterministic traps handled in earlier match arm",
575+
);
552576

553577
lhs_engine.assert_error_match(&lhs, &rhs);
554578
DiffEqResult::Failed
555579
}
580+
556581
// A real bug is found if only one side fails.
557582
(Ok(_), Err(err)) => panic!("only the `rhs` failed for this input: {err:?}"),
558583
(Err(err), Ok(_)) => panic!("only the `lhs` failed for this input: {err:?}"),
@@ -1337,6 +1362,8 @@ mod tests {
13371362
| WasmFeatures::TAIL_CALL
13381363
| WasmFeatures::WIDE_ARITHMETIC
13391364
| WasmFeatures::MEMORY64
1365+
| WasmFeatures::FUNCTION_REFERENCES
1366+
| WasmFeatures::GC
13401367
| WasmFeatures::GC_TYPES
13411368
| WasmFeatures::CUSTOM_PAGE_SIZES
13421369
| WasmFeatures::EXTENDED_CONST;

crates/fuzzing/src/oracles/diff_spec.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ impl DiffEngine for SpecInterpreter {
4949
let _ = (trap, err);
5050
}
5151

52-
fn is_stack_overflow(&self, err: &Error) -> bool {
52+
fn is_non_deterministic_error(&self, err: &Error) -> bool {
5353
err.to_string().contains("(Isabelle) call stack exhausted")
5454
}
5555
}

crates/fuzzing/src/oracles/diff_v8.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ impl DiffEngine for V8Engine {
145145
verify_wasmtime("not possibly present in an error, just panic please");
146146
}
147147

148-
fn is_stack_overflow(&self, err: &Error) -> bool {
148+
fn is_non_deterministic_error(&self, err: &Error) -> bool {
149149
err.to_string().contains("Maximum call stack size exceeded")
150150
}
151151
}

crates/fuzzing/src/oracles/diff_wasmi.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ impl DiffEngine for WasmiEngine {
8585
}
8686
}
8787

88-
fn is_stack_overflow(&self, err: &Error) -> bool {
88+
fn is_non_deterministic_error(&self, err: &Error) -> bool {
8989
matches!(
9090
self.trap_code(err),
9191
Some(wasmi::core::TrapCode::StackOverflow)

crates/fuzzing/src/oracles/diff_wasmtime.rs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,15 @@ impl WasmtimeEngine {
2626
) -> arbitrary::Result<Self> {
2727
let mut new_config = u.arbitrary::<WasmtimeConfig>()?;
2828
new_config.compiler_strategy = compiler_strategy;
29-
new_config.update_module_config(&mut config.module_config.config, u)?;
29+
new_config.update_module_config(&mut config.module_config, u)?;
3030
new_config.make_compatible_with(&config.wasmtime);
3131

3232
let config = generators::Config {
3333
wasmtime: new_config,
3434
module_config: config.module_config.clone(),
3535
};
36+
log::debug!("Created new Wasmtime differential engine with config: {config:?}");
37+
3638
Ok(Self { config })
3739
}
3840
}
@@ -57,12 +59,13 @@ impl DiffEngine for WasmtimeEngine {
5759
let lhs = lhs
5860
.downcast_ref::<Trap>()
5961
.expect(&format!("not a trap: {lhs:?}"));
62+
6063
assert_eq!(lhs, rhs, "{lhs}\nis not equal to\n{rhs}");
6164
}
6265

63-
fn is_stack_overflow(&self, err: &Error) -> bool {
66+
fn is_non_deterministic_error(&self, err: &Error) -> bool {
6467
match err.downcast_ref::<Trap>() {
65-
Some(trap) => *trap == Trap::StackOverflow,
68+
Some(trap) => super::wasmtime_trap_is_non_deterministic(trap),
6669
None => false,
6770
}
6871
}
@@ -118,11 +121,10 @@ impl WasmtimeInstance {
118121

119122
globals
120123
.into_iter()
121-
.map(|(name, global)| {
122-
(
123-
name,
124-
global.ty(&self.store).content().clone().try_into().unwrap(),
125-
)
124+
.filter_map(|(name, global)| {
125+
DiffValueType::try_from(global.ty(&self.store).content().clone())
126+
.map(|ty| (name, ty))
127+
.ok()
126128
})
127129
.collect()
128130
}

crates/fuzzing/src/oracles/engine.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,12 @@ pub trait DiffEngine {
6161
/// generated.
6262
fn assert_error_match(&self, err: &Error, trap: &Trap);
6363

64-
/// Returns whether the error specified from this engine might be stack
65-
/// overflow.
66-
fn is_stack_overflow(&self, err: &Error) -> bool;
64+
/// Returns whether the error specified from this engine is
65+
/// non-deterministic, like a stack overflow or an attempt to allocate an
66+
/// object that is too large (which is non-deterministic because it may
67+
/// depend on which collector it was configured with or memory available on
68+
/// the system).
69+
fn is_non_deterministic_error(&self, err: &Error) -> bool;
6770
}
6871

6972
/// Provide a way to evaluate Wasm functions--a Wasm instance implemented by a

crates/wasmtime/src/runtime/linker.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use crate::{
88
Instance, Module, StoreContextMut, Val, ValRaw, ValType,
99
};
1010
use alloc::sync::Arc;
11-
use core::fmt;
11+
use core::fmt::{self, Debug};
1212
use core::marker;
1313
#[cfg(feature = "async")]
1414
use core::{future::Future, pin::Pin};
@@ -90,6 +90,12 @@ pub struct Linker<T> {
9090
_marker: marker::PhantomData<fn() -> T>,
9191
}
9292

93+
impl<T> Debug for Linker<T> {
94+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
95+
f.debug_struct("Linker").finish_non_exhaustive()
96+
}
97+
}
98+
9399
impl<T> Clone for Linker<T> {
94100
fn clone(&self) -> Linker<T> {
95101
Linker {

0 commit comments

Comments
 (0)