From 4ba3404ca3422f3acd2c0f9084aa7d7cfcf261a7 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 3 Feb 2022 11:56:16 -0600 Subject: [PATCH] Fix MemFd's allocated memory for dynamic memories (#3763) This fixes a bug in the memfd-related management of a linear memory where for dynamic memories memfd wasn't informed of the extra room that the dynamic memory could grow into, only the actual size of linear memory, which ended up tripping an assert once the memory was grown. The fix here is pretty simple which is to factor in this extra space when passing the allocation size to the creation of the `MemFdSlot`. --- crates/fuzzing/src/oracles.rs | 1 + crates/runtime/src/memory.rs | 6 +++- tests/all/memory.rs | 55 +++++++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 1 deletion(-) diff --git a/crates/fuzzing/src/oracles.rs b/crates/fuzzing/src/oracles.rs index c1feba5509..b351516d95 100644 --- a/crates/fuzzing/src/oracles.rs +++ b/crates/fuzzing/src/oracles.rs @@ -405,6 +405,7 @@ pub fn make_api_calls(api: generators::api::ApiCalls) { /// /// Ensures that spec tests pass regardless of the `Config`. pub fn spectest(mut fuzz_config: generators::Config, test: generators::SpecTest) { + crate::init_fuzzing(); fuzz_config.module_config.set_spectest_compliant(); log::debug!("running {:?} with {:?}", test.file, fuzz_config); let mut wast_context = WastContext::new(fuzz_config.to_store()); diff --git a/crates/runtime/src/memory.rs b/crates/runtime/src/memory.rs index 2cf6fb83e0..932ecff431 100644 --- a/crates/runtime/src/memory.rs +++ b/crates/runtime/src/memory.rs @@ -159,7 +159,11 @@ impl MmapMemory { let memfd = match memfd_image { Some(image) => { let base = unsafe { mmap.as_mut_ptr().add(pre_guard_bytes) }; - let mut memfd_slot = MemFdSlot::create(base.cast(), minimum, alloc_bytes); + let mut memfd_slot = MemFdSlot::create( + base.cast(), + minimum, + alloc_bytes + extra_to_reserve_on_growth, + ); memfd_slot.instantiate(minimum, Some(image))?; // On drop, we will unmap our mmap'd range that this // memfd_slot was mapped on top of, so there is no diff --git a/tests/all/memory.rs b/tests/all/memory.rs index 387e41bc28..d2e2e5afa3 100644 --- a/tests/all/memory.rs +++ b/tests/all/memory.rs @@ -376,3 +376,58 @@ fn static_forced_max() -> Result<()> { assert!(mem.grow(&mut store, 1).is_err()); Ok(()) } + +#[test] +fn dynamic_extra_growth_unchanged_pointer() -> Result<()> { + const EXTRA_PAGES: u64 = 5; + let mut config = Config::new(); + config.static_memory_maximum_size(0); + // 5 wasm pages extra + config.dynamic_memory_reserved_for_growth(EXTRA_PAGES * (1 << 16)); + let engine = Engine::new(&config)?; + let mut store = Store::new(&engine, ()); + + fn assert_behaves_well(store: &mut Store<()>, mem: &Memory) -> Result<()> { + let ptr = mem.data_ptr(&store); + + // Each growth here should retain the same linear pointer in memory and the + // memory shouldn't get moved. + for _ in 0..EXTRA_PAGES { + mem.grow(&mut *store, 1)?; + assert_eq!(ptr, mem.data_ptr(&store)); + } + + // Growth afterwards though will be forced to move the pointer + mem.grow(&mut *store, 1)?; + let new_ptr = mem.data_ptr(&store); + assert_ne!(ptr, new_ptr); + + for _ in 0..EXTRA_PAGES - 1 { + mem.grow(&mut *store, 1)?; + assert_eq!(new_ptr, mem.data_ptr(&store)); + } + Ok(()) + } + + let mem = Memory::new(&mut store, MemoryType::new(10, None))?; + assert_behaves_well(&mut store, &mem)?; + + let module = Module::new(&engine, r#"(module (memory (export "mem") 10))"#)?; + let instance = Instance::new(&mut store, &module, &[])?; + let mem = instance.get_memory(&mut store, "mem").unwrap(); + assert_behaves_well(&mut store, &mem)?; + + let module = Module::new( + &engine, + r#" + (module + (memory (export "mem") 10) + (data (i32.const 0) "")) + "#, + )?; + let instance = Instance::new(&mut store, &module, &[])?; + let mem = instance.get_memory(&mut store, "mem").unwrap(); + assert_behaves_well(&mut store, &mem)?; + + Ok(()) +}