From e500be829ba208d644287566793a038ce5f291b5 Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Tue, 21 Jul 2020 19:24:55 +0200 Subject: [PATCH 1/5] Add test case that replicate the problem --- crates/wiggle/tests/structs.witx | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/crates/wiggle/tests/structs.witx b/crates/wiggle/tests/structs.witx index 0542bc68fa..2a64b125f9 100644 --- a/crates/wiggle/tests/structs.witx +++ b/crates/wiggle/tests/structs.witx @@ -16,6 +16,12 @@ (field $first (@witx const_pointer s32)) (field $second s32))) +(typename $some_bytes (array u8)) + +(typename $struct_of_array + (struct + (field $arr $some_bytes))) + (module $structs (@interface func (export "sum_of_pair") (param $an_pair $pair_ints) @@ -37,4 +43,8 @@ (param $second (@witx const_pointer s32)) (result $error $errno) (result $an_pair $pair_int_ptrs)) + (@interface func (export "sum_array") + (param $an_arr $struct_of_array) + (result $error $errno) + (result $doubled u16)) ) From 5d8271286a53c21a3b180d89f295cc794a49c65e Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Tue, 21 Jul 2020 20:35:14 +0200 Subject: [PATCH 2/5] Check if named type requires a lifetime and conform if does --- crates/wiggle/generate/src/types/struct.rs | 9 ++++++++- crates/wiggle/tests/structs.rs | 4 ++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/crates/wiggle/generate/src/types/struct.rs b/crates/wiggle/generate/src/types/struct.rs index 5b3a025904..a2b1e93992 100644 --- a/crates/wiggle/generate/src/types/struct.rs +++ b/crates/wiggle/generate/src/types/struct.rs @@ -19,7 +19,14 @@ pub(super) fn define_struct( let member_decls = s.members.iter().map(|m| { let name = names.struct_member(&m.name); let type_ = match &m.tref { - witx::TypeRef::Name(nt) => names.type_(&nt.name), + witx::TypeRef::Name(nt) => { + let tt = names.type_(&nt.name); + if m.tref.needs_lifetime() { + quote!(#tt<'a>) + } else { + quote!(#tt) + } + }, witx::TypeRef::Value(ty) => match &**ty { witx::Type::Builtin(builtin) => names.builtin_type(*builtin, quote!('a)), witx::Type::Pointer(pointee) | witx::Type::ConstPointer(pointee) => { diff --git a/crates/wiggle/tests/structs.rs b/crates/wiggle/tests/structs.rs index b98cf66163..cb8b7ef694 100644 --- a/crates/wiggle/tests/structs.rs +++ b/crates/wiggle/tests/structs.rs @@ -52,6 +52,10 @@ impl<'a> structs::Structs for WasiCtx<'a> { second: *second, }) } + + fn sum_array<'b>(&self, an_arr: &types::StructOfArray<'b>) -> Result { + Ok(0) + } } #[derive(Debug)] From 7af7ac4b1b18563fdddd597d11458f23e0ce3f4f Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Tue, 4 Aug 2020 11:21:36 -0700 Subject: [PATCH 3/5] implement GuestType for arrays --- crates/wiggle/src/guest_type.rs | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/crates/wiggle/src/guest_type.rs b/crates/wiggle/src/guest_type.rs index 27715254a8..e14cfff39a 100644 --- a/crates/wiggle/src/guest_type.rs +++ b/crates/wiggle/src/guest_type.rs @@ -153,3 +153,30 @@ impl<'a, T> GuestType<'a> for GuestPtr<'a, T> { ptr.cast::().write(val.offset()) } } + +// Support pointers-to-arrays where pointers are always 32-bits in wasm land +impl<'a, T> GuestType<'a> for GuestPtr<'a, [T]> +where + T: GuestType<'a>, +{ + fn guest_size() -> u32 { + u32::guest_size() * 2 + } + + fn guest_align() -> usize { + u32::guest_align() + } + + fn read(ptr: &GuestPtr<'a, Self>) -> Result { + let offset = ptr.cast::().read()?; + let len = ptr.cast::().add(1)?.read()?; + Ok(GuestPtr::new(ptr.mem(), offset).as_array(len)) + } + + fn write(ptr: &GuestPtr<'_, Self>, val: Self) -> Result<(), GuestError> { + let (offs, len) = val.offset(); + let len_ptr = ptr.cast::().add(1)?; + ptr.cast::().write(offs)?; + len_ptr.write(len) + } +} From 8264abdef3579347483b9f28a53b3958fd173458 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Tue, 4 Aug 2020 15:50:32 -0700 Subject: [PATCH 4/5] structs tests: comprehensive proptest for struct of array --- crates/wiggle/tests/structs.rs | 121 ++++++++++++++++++++++++++++++++- 1 file changed, 118 insertions(+), 3 deletions(-) diff --git a/crates/wiggle/tests/structs.rs b/crates/wiggle/tests/structs.rs index cb8b7ef694..f342d4487a 100644 --- a/crates/wiggle/tests/structs.rs +++ b/crates/wiggle/tests/structs.rs @@ -1,6 +1,6 @@ use proptest::prelude::*; use wiggle::{GuestMemory, GuestPtr}; -use wiggle_test::{impl_errno, HostMemory, MemArea, WasiCtx}; +use wiggle_test::{impl_errno, HostMemory, MemArea, MemAreas, WasiCtx}; wiggle::from_witx!({ witx: ["tests/structs.witx"], @@ -53,8 +53,23 @@ impl<'a> structs::Structs for WasiCtx<'a> { }) } - fn sum_array<'b>(&self, an_arr: &types::StructOfArray<'b>) -> Result { - Ok(0) + fn sum_array<'b>(&self, struct_of_arr: &types::StructOfArray<'b>) -> Result { + // my kingdom for try blocks + fn aux(struct_of_arr: &types::StructOfArray) -> Result { + let mut s = 0; + for elem in struct_of_arr.arr.iter() { + let v = elem?.read()?; + s += v as u16; + } + Ok(s) + } + match aux(struct_of_arr) { + Ok(s) => Ok(s), + Err(guest_err) => { + eprintln!("guest error summing array: {:?}", guest_err); + Err(types::Errno::PicketLine) + } + } } } @@ -427,3 +442,103 @@ proptest! { e.test() } } + +#[derive(Debug)] +struct SumArrayExercise { + inputs: Vec, + input_array_loc: MemArea, + input_struct_loc: MemArea, + output_loc: MemArea, +} + +impl SumArrayExercise { + pub fn strat() -> BoxedStrategy { + (0..256u32) + .prop_flat_map(|len| { + let len_usize = len as usize; + ( + prop::collection::vec(prop::num::u8::ANY, len_usize..=len_usize), + HostMemory::mem_area_strat(8), // Input struct is 8 bytes - ptr and len + HostMemory::mem_area_strat(4), // Output is 4 bytes - stores a u16, but abi requires 4 byte alignment + ) + }) + .prop_filter( + "non-overlapping input struct and output pointers", + |(_inputs, input_struct_loc, output_loc)| { + MemArea::non_overlapping_set(&[input_struct_loc.clone(), output_loc.clone()]) + }, + ) + .prop_flat_map(|(inputs, input_struct_loc, output_loc)| { + ( + Just(inputs.clone()), + HostMemory::byte_slice_strat( + inputs.len() as u32, + &MemAreas::from([input_struct_loc, output_loc]), + ), + Just(input_struct_loc.clone()), + Just(output_loc.clone()), + ) + }) + .prop_map( + |(inputs, input_array_loc, input_struct_loc, output_loc)| SumArrayExercise { + inputs, + input_array_loc, + input_struct_loc, + output_loc, + }, + ) + .boxed() + } + pub fn test(&self) { + let ctx = WasiCtx::new(); + let host_memory = HostMemory::new(); + + // Write inputs to memory as an array + for (ix, val) in self.inputs.iter().enumerate() { + let ix = ix as u32; + host_memory + .ptr(self.input_array_loc.ptr + ix) + .write(*val) + .expect("write val to array memory"); + } + + // Write struct that contains the array + host_memory + .ptr(self.input_struct_loc.ptr) + .write(self.input_array_loc.ptr) + .expect("write ptr to struct memory"); + host_memory + .ptr(self.input_struct_loc.ptr + 4) + .write(self.inputs.len() as u32) + .expect("write len to struct memory"); + + // Call wiggle-generated func + let res = structs::sum_array( + &ctx, + &host_memory, + self.input_struct_loc.ptr as i32, + self.output_loc.ptr as i32, + ); + + // should be no error - if hostcall did a GuestError it should eprintln it. + assert_eq!(res, types::Errno::Ok.into(), "reduce excuses errno"); + + // Sum is inputs upcasted to u16 + let expected: u16 = self.inputs.iter().map(|v| *v as u16).sum(); + + // Wiggle stored output value in memory as u16 + let given: u16 = host_memory + .ptr(self.output_loc.ptr) + .read() + .expect("deref ptr to returned value"); + + // Assert the two calculations match + assert_eq!(expected, given, "sum_array return val"); + } +} +proptest! { + #[test] + fn sum_of_array(e in SumArrayExercise::strat()) { + e.test() + } +} From 3bf8c2066587b9ac63286bdb00cf47888ada7d95 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Tue, 4 Aug 2020 18:41:06 -0700 Subject: [PATCH 5/5] rustfmt.... idk why my editor missed this one --- crates/wiggle/generate/src/types/struct.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/wiggle/generate/src/types/struct.rs b/crates/wiggle/generate/src/types/struct.rs index a2b1e93992..5aaf423532 100644 --- a/crates/wiggle/generate/src/types/struct.rs +++ b/crates/wiggle/generate/src/types/struct.rs @@ -26,7 +26,7 @@ pub(super) fn define_struct( } else { quote!(#tt) } - }, + } witx::TypeRef::Value(ty) => match &**ty { witx::Type::Builtin(builtin) => names.builtin_type(*builtin, quote!('a)), witx::Type::Pointer(pointee) | witx::Type::ConstPointer(pointee) => {