From d5dc9b41aba093e4402cb4bd5836d1d7487ae20f Mon Sep 17 00:00:00 2001 From: zach Date: Fri, 19 Jan 2024 13:04:21 -0800 Subject: [PATCH] feat: Use quickcheck to test allocations, fix one bug that was uncovered. (#662) - Adds quickcheck tests for alloc/free/load/store from the kernel, I wasn't able to include these in the new wasm-bindgen tests because `getrandom` isn't available on wasm32-unknown-unknown - Fixes a bug in the calculation of how much memory is needed to allocate the next block in the kernel. This bug is triggered when allocating at the end of a page, the size of the MemoryBlock value wasn't being taken in consideration when determining whether or not to call memory.grow --- kernel/src/lib.rs | 5 +- runtime/Cargo.toml | 2 + runtime/src/extism-runtime.wasm | Bin 3498 -> 3500 bytes runtime/src/tests/kernel.rs | 139 ++++++++++++++++++++++++++++++++ runtime/src/tests/runtime.rs | 11 ++- 5 files changed, 151 insertions(+), 6 deletions(-) diff --git a/kernel/src/lib.rs b/kernel/src/lib.rs index f8f1d93..32210ad 100644 --- a/kernel/src/lib.rs +++ b/kernel/src/lib.rs @@ -270,12 +270,13 @@ impl MemoryRoot { // Get the number of bytes available let mem_left = self_length - self_position - core::mem::size_of::() as u64; + let length_with_block = length + core::mem::size_of::() as u64; // When the allocation is larger than the number of bytes available // we will need to try to grow the memory - if length >= mem_left { + if length_with_block >= mem_left { // Calculate the number of pages needed to cover the remaining bytes - let npages = num_pages(length - mem_left); + let npages = num_pages(length_with_block - mem_left); let x = core::arch::wasm32::memory_grow(0, npages); if x == usize::MAX { return None; diff --git a/runtime/Cargo.toml b/runtime/Cargo.toml index 9fb85dd..9b567ed 100644 --- a/runtime/Cargo.toml +++ b/runtime/Cargo.toml @@ -37,6 +37,8 @@ cbindgen = { version = "0.26", default-features = false } [dev-dependencies] criterion = "0.5.1" +quickcheck = "1" +rand = "0.8.5" [[bench]] name = "bench" diff --git a/runtime/src/extism-runtime.wasm b/runtime/src/extism-runtime.wasm index 27b32209515bff0f4336fce533b5100d608f2397..fe52a5ab694bce9ded5aca60a25170243b06a814 100755 GIT binary patch delta 73 zcmZ1_y+(RNA0y+O$^DEc7;jCEVEV}K#8achqQIoUT$) -> bool { + let (mut store, mut instance) = init_kernel_test(); + let instance = &mut instance; + for a in amounts { + let ptr = extism_alloc(&mut store, instance, a as u64); + if ptr == 0 || ptr == u64::MAX { + continue + } + if extism_length(&mut store, instance, ptr) != a as u64 { + return false + } + } + + true + } +} + +quickcheck! { + fn check_large_alloc(amounts: Vec) -> bool { + let (mut store, mut instance) = init_kernel_test(); + let instance = &mut instance; + for a in amounts { + let ptr = extism_alloc(&mut store, instance, a as u64); + if ptr == 0 || ptr == u64::MAX { + continue + } + if extism_length(&mut store, instance, ptr) != a as u64 { + return false + } + } + + true + } +} + +quickcheck! { + fn check_alloc_with_frees(amounts: Vec) -> bool { + let (mut store, mut instance) = init_kernel_test(); + let instance = &mut instance; + let mut prev = 0; + for a in amounts { + let ptr = extism_alloc(&mut store, instance, a as u64); + if ptr == 0 { + continue + } + if extism_length(&mut store, instance, ptr) != a as u64 { + return false + } + + if a % 2 == 0 { + extism_free(&mut store, instance, ptr); + } else if a % 3 == 0 { + extism_free(&mut store, instance, prev); + } + + prev = ptr; + } + + true + } +} + +quickcheck! { + fn check_large_alloc_with_frees(amounts: Vec) -> bool { + let (mut store, mut instance) = init_kernel_test(); + let instance = &mut instance; + let mut prev = 0; + for a in amounts { + let ptr = extism_alloc(&mut store, instance, a as u64); + if ptr == 0 || ptr == u64::MAX { + continue + } + if extism_length(&mut store, instance, ptr) != a as u64 { + return false + } + if a % 2 == 0 { + extism_free(&mut store, instance, ptr); + } else if a % 3 == 0 { + extism_free(&mut store, instance, prev); + } + + prev = ptr; + + } + + true + } +} + +quickcheck! { + fn check_alloc_with_load_and_store(amounts: Vec) -> bool { + use rand::Rng; + let mut rng = rand::thread_rng(); + let (mut store, mut instance) = init_kernel_test(); + let instance = &mut instance; + for a in amounts { + let ptr = extism_alloc(&mut store, instance, a as u64); + if ptr == 0 || ptr == u64::MAX { + continue + } + if extism_length(&mut store, instance, ptr) != a as u64 { + return false + } + + for _ in 0..16 { + let i = rng.gen_range(ptr..ptr+a as u64); + extism_store_u8(&mut store, instance, i, i as u8); + if extism_load_u8(&mut store, instance, i as u64) != i as u8 { + return false + } + } + } + + true + } +} diff --git a/runtime/src/tests/runtime.rs b/runtime/src/tests/runtime.rs index ff5692a..c70f7e6 100644 --- a/runtime/src/tests/runtime.rs +++ b/runtime/src/tests/runtime.rs @@ -39,11 +39,12 @@ pub struct Count { #[test] fn it_works() { - tracing_subscriber::fmt() + let log = tracing_subscriber::fmt() .with_ansi(false) .with_env_filter("extism=debug") .with_writer(std::fs::File::create("test.log").unwrap()) - .init(); + .try_init() + .is_ok(); let wasm_start = Instant::now(); @@ -143,8 +144,10 @@ fn it_works() { println!("wasm function call (avg, N = {}): {:?}", num_tests, avg); // Check that log file was written to - let meta = std::fs::metadata("test.log").unwrap(); - assert!(meta.len() > 0); + if log { + let meta = std::fs::metadata("test.log").unwrap(); + assert!(meta.len() > 0); + } } #[test]