Skip to content

Commit

Permalink
Minor: Update output validity tests (paritytech#13190)
Browse files Browse the repository at this point in the history
* Minor: Update output validity tests

Quick follow-up to paritytech#13183.

Mainly, I wanted to double check that the `test_return_max_memory_offset` test doesn't pass just
because the output length is 0.

I also:

- Organized these tests into a module.
- Added a comment explaining why we don't use the `wasm_export_functions` macro.

* Update test based on review comment
  • Loading branch information
mrcnski authored and ark0f committed Feb 27, 2023
1 parent 9ebcd6a commit ce54265
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 27 deletions.
67 changes: 41 additions & 26 deletions client/executor/runtime-test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ use sp_runtime::{
print,
traits::{BlakeTwo256, Hash},
};
#[cfg(not(feature = "std"))]
use sp_runtime_interface::pack_ptr_and_len;

extern "C" {
#[allow(dead_code)]
Expand Down Expand Up @@ -344,31 +342,48 @@ sp_core::wasm_export_functions! {
}
}

// Returns a huge len. It should result in an error, and not an allocation.
#[no_mangle]
#[cfg(not(feature = "std"))]
pub extern "C" fn test_return_huge_len(_params: *const u8, _len: usize) -> u64 {
pack_ptr_and_len(0, u32::MAX)
}
// Tests that check output validity. We explicitly return the ptr and len, so we avoid using the
// `wasm_export_functions` macro.
mod output_validity {
#[cfg(not(feature = "std"))]
use super::WASM_PAGE_SIZE;

// Returns an offset right at the edge of the wasm memory boundary. With length 0, it should
// succeed.
#[no_mangle]
#[cfg(not(feature = "std"))]
pub extern "C" fn test_return_max_memory_offset(_params: *const u8, _len: usize) -> u64 {
pack_ptr_and_len((core::arch::wasm32::memory_size(0) * WASM_PAGE_SIZE) as u32, 0)
}
#[cfg(not(feature = "std"))]
use sp_runtime_interface::pack_ptr_and_len;

// Returns an offset right at the edge of the wasm memory boundary. With length 1, it should fail.
#[no_mangle]
#[cfg(not(feature = "std"))]
pub extern "C" fn test_return_max_memory_offset_plus_one(_params: *const u8, _len: usize) -> u64 {
pack_ptr_and_len((core::arch::wasm32::memory_size(0) * WASM_PAGE_SIZE) as u32, 1)
}
// Returns a huge len. It should result in an error, and not an allocation.
#[no_mangle]
#[cfg(not(feature = "std"))]
pub extern "C" fn test_return_huge_len(_params: *const u8, _len: usize) -> u64 {
pack_ptr_and_len(0, u32::MAX)
}

// Returns an output that overflows the u32 range. It should result in an error.
#[no_mangle]
#[cfg(not(feature = "std"))]
pub extern "C" fn test_return_overflow(_params: *const u8, _len: usize) -> u64 {
pack_ptr_and_len(u32::MAX, 1)
// Returns an offset right before the edge of the wasm memory boundary. It should succeed.
#[no_mangle]
#[cfg(not(feature = "std"))]
pub extern "C" fn test_return_max_memory_offset(_params: *const u8, _len: usize) -> u64 {
let output_ptr = (core::arch::wasm32::memory_size(0) * WASM_PAGE_SIZE) as u32 - 1;
let ptr = output_ptr as *mut u8;
unsafe {
ptr.write(u8::MAX);
}
pack_ptr_and_len(output_ptr, 1)
}

// Returns an offset right after the edge of the wasm memory boundary. It should fail.
#[no_mangle]
#[cfg(not(feature = "std"))]
pub extern "C" fn test_return_max_memory_offset_plus_one(
_params: *const u8,
_len: usize,
) -> u64 {
pack_ptr_and_len((core::arch::wasm32::memory_size(0) * WASM_PAGE_SIZE) as u32, 1)
}

// Returns an output that overflows the u32 range. It should result in an error.
#[no_mangle]
#[cfg(not(feature = "std"))]
pub extern "C" fn test_return_overflow(_params: *const u8, _len: usize) -> u64 {
pack_ptr_and_len(u32::MAX, 1)
}
}
2 changes: 1 addition & 1 deletion client/executor/src/integration_tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -811,7 +811,7 @@ fn return_max_memory_offset(wasm_method: WasmExecutionMethod) {

assert_eq!(
call_in_wasm("test_return_max_memory_offset", &[], wasm_method, &mut ext).unwrap(),
().encode()
(u8::MAX).encode()
);
}

Expand Down

0 comments on commit ce54265

Please sign in to comment.