From 898af8e2fb2774b5fc5e54cbc02369a483535e24 Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Thu, 20 Feb 2020 14:51:22 +0100 Subject: [PATCH] Revisit GuestArray and their deref mechanics In preparation for landing `string` support in `wiggle`, I've revisited `GuestArray` and the deref mechanics, and decided to scrap the current approach in favour of the previous. Namely, for `T: GuestTypeCopy` we provide `GuestArray::as_ref` which then can be derefed directly to `&[T]` since the guest and host types have matching representation, and for other types (thinking here of our infamous arrays of pointers), I've instead added a way to iterate over the pointers. Then, it is up to the `wiggle`'s client to know whether they can deref whatever the pointer stored in array is pointing at. --- crates/runtime/src/memory/array.rs | 139 ++++++++++++++++++++--------- tests/main.rs | 35 +++----- 2 files changed, 110 insertions(+), 64 deletions(-) diff --git a/crates/runtime/src/memory/array.rs b/crates/runtime/src/memory/array.rs index 2250497a3e..40a5e37d82 100644 --- a/crates/runtime/src/memory/array.rs +++ b/crates/runtime/src/memory/array.rs @@ -1,5 +1,5 @@ use super::ptr::{GuestPtr, GuestRef}; -use crate::{GuestError, GuestType}; +use crate::{GuestError, GuestType, GuestTypeCopy}; use std::{fmt, ops::Deref}; pub struct GuestArray<'a, T> @@ -26,54 +26,120 @@ where impl<'a, T> GuestArray<'a, T> where T: GuestType, +{ + pub fn iter(&self) -> GuestArrayIter<'a, T> { + let next = GuestPtr { + mem: self.ptr.mem, + region: self.ptr.region, + type_: self.ptr.type_, + }; + GuestArrayIter { + next, + num_elems: self.num_elems, + count: 0, + } + } +} + +pub struct GuestArrayIter<'a, T> +where + T: GuestType, +{ + next: GuestPtr<'a, T>, + num_elems: u32, + count: u32, +} + +impl<'a, T> Iterator for GuestArrayIter<'a, T> +where + T: GuestType, +{ + type Item = Result, GuestError>; + + fn next(&mut self) -> Option { + if self.count < self.num_elems { + // ok... + Some(T::validate(&self.next).and_then(|()| { + let curr = GuestPtr { + mem: self.next.mem, + region: self.next.region, + type_: self.next.type_, + }; + self.next = self.next.elem(1)?; + self.count += 1; + Ok(curr) + })) + } else { + // no more elements... + None + } + } +} + +impl<'a, T> GuestArray<'a, T> +where + T: GuestTypeCopy, { pub fn as_ref(&self) -> Result, GuestError> { - let mut refs = Vec::new(); let mut next = self.ptr.elem(0)?; for _ in 0..self.num_elems { T::validate(&next)?; - let handle = { - let mut borrows = next.mem.borrows.borrow_mut(); - borrows - .borrow_immut(next.region) - .ok_or_else(|| GuestError::PtrBorrowed(next.region))? - }; - refs.push(GuestRef { - mem: next.mem, - region: next.region, - handle, - type_: next.type_, - }); next = next.elem(1)?; } - Ok(GuestArrayRef { refs }) + let region = self.ptr.region.extend(self.num_elems); + let handle = { + let mut borrows = self.ptr.mem.borrows.borrow_mut(); + borrows + .borrow_immut(region) + .ok_or_else(|| GuestError::PtrBorrowed(region))? + }; + let ref_ = GuestRef { + mem: self.ptr.mem, + region, + handle, + type_: self.ptr.type_, + }; + Ok(GuestArrayRef { + ref_, + num_elems: self.num_elems, + }) } } pub struct GuestArrayRef<'a, T> where - T: GuestType, + T: GuestTypeCopy, { - refs: Vec>, + ref_: GuestRef<'a, T>, + num_elems: u32, } impl<'a, T> fmt::Debug for GuestArrayRef<'a, T> where - T: GuestType + fmt::Debug, + T: GuestTypeCopy + fmt::Debug, { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "GuestArrayRef {{ refs: {:?} }}", self.refs) + write!( + f, + "GuestArrayRef {{ ref_: {:?}, num_elems: {:?} }}", + self.ref_, self.num_elems + ) } } impl<'a, T> Deref for GuestArrayRef<'a, T> where - T: GuestType, + T: GuestTypeCopy, { - type Target = [GuestRef<'a, T>]; + type Target = [T]; fn deref(&self) -> &Self::Target { - self.refs.as_slice() + unsafe { + std::slice::from_raw_parts( + self.ref_.as_ptr().as_raw() as *const _, + self.num_elems as usize, + ) + } } } @@ -130,21 +196,11 @@ mod test { // extract as array let ptr: GuestPtr = guest_memory.ptr(0).expect("ptr to first el"); let arr = ptr.array(3).expect("convert ptr to array"); - let as_ref = arr - .as_ref() - .expect("array borrowed immutably") - .iter() - .map(|x| **x) - .collect::>(); - assert_eq!(&as_ref, &[1, 2, 3]); + let as_ref = &*arr.as_ref().expect("array borrowed immutably"); + assert_eq!(as_ref, &[1, 2, 3]); // borrowing again should be fine - let as_ref2 = arr - .as_ref() - .expect("array borrowed immutably again") - .iter() - .map(|x| **x) - .collect::>(); - assert_eq!(&as_ref2, &as_ref); + let as_ref2 = &*arr.as_ref().expect("array borrowed immutably again"); + assert_eq!(as_ref2, as_ref); } #[test] @@ -189,18 +245,17 @@ mod test { // extract as array let ptr: GuestPtr> = guest_memory.ptr(12).expect("ptr to first el"); let arr = ptr.array(3).expect("convert ptr to array"); - let as_ref = arr - .as_ref() - .expect("array borrowed immutably") + let contents = arr .iter() - .map(|x| { - *x.as_ptr() + .map(|ptr_ptr| { + *ptr_ptr + .expect("valid ptr to ptr") .read_ptr_from_guest() .expect("valid ptr to some value") .as_ref() .expect("deref ptr to some value") }) .collect::>(); - assert_eq!(&as_ref, &[255, 254, 253]); + assert_eq!(&contents, &[255, 254, 253]); } } diff --git a/tests/main.rs b/tests/main.rs index a84be06951..be06a41d80 100644 --- a/tests/main.rs +++ b/tests/main.rs @@ -1,6 +1,7 @@ use proptest::prelude::*; use wiggle_runtime::{ - GuestError, GuestErrorType, GuestMemory, GuestPtr, GuestPtrMut, GuestRef, GuestRefMut, + GuestArray, GuestError, GuestErrorType, GuestMemory, GuestPtr, GuestPtrMut, GuestRef, + GuestRefMut, }; wiggle_generate::from_witx!({ @@ -97,25 +98,20 @@ impl foo::Foo for WasiCtx { &mut self, excuses: &types::ConstExcuseArray, ) -> Result { - let excuses = &*excuses - .as_ref() - .expect("dereferencing GuestArray should succeed"); let last = excuses + .iter() .last() .expect("input array is non-empty") - .as_ptr() + .expect("valid ptr to ptr") .read_ptr_from_guest() .expect("valid ptr to some Excuse value"); Ok(*last.as_ref().expect("dereferencing ptr should succeed")) } fn populate_excuses(&mut self, excuses: &types::ExcuseArray) -> Result<(), types::Errno> { - let excuses = &*excuses - .as_ref() - .expect("dereferencing GuestArray should succeed"); - for excuse in excuses { + for excuse in excuses.iter() { let ptr_to_ptr = excuse - .as_ptr() + .expect("valid ptr to ptr") .read_ptr_from_guest() .expect("valid ptr to some Excuse value"); let mut ptr = ptr_to_ptr @@ -752,19 +748,14 @@ impl PopulateExcusesExcercise { ); assert_eq!(res, types::Errno::Ok.into(), "populate excuses errno"); - let arr = { - let ptr: GuestPtr<'_, GuestPtr<'_, types::Excuse>> = guest_memory - .ptr(self.array_ptr_loc.ptr) - .expect("ptr to the first element of array"); - &*ptr - .array(self.elements.len() as u32) - .expect("as array") - .as_ref() - .expect("deref to &[]") - }; - for el in arr { + let arr: GuestArray<'_, GuestPtr<'_, types::Excuse>> = guest_memory + .ptr(self.array_ptr_loc.ptr) + .expect("ptr to the first element of array") + .array(self.elements.len() as u32) + .expect("as array"); + for el in arr.iter() { let ptr_to_ptr = el - .as_ptr() + .expect("valid ptr to ptr") .read_ptr_from_guest() .expect("valid ptr to some Excuse value"); assert_eq!(