From fc4f96a73f0c0a1c4de34038de58688522403472 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Mon, 16 Mar 2020 17:28:39 -0700 Subject: [PATCH 1/5] wiggle-generate: teach about anonymous array types --- crates/wiggle/crates/generate/src/names.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/wiggle/crates/generate/src/names.rs b/crates/wiggle/crates/generate/src/names.rs index d545b37845..75dd9b986f 100644 --- a/crates/wiggle/crates/generate/src/names.rs +++ b/crates/wiggle/crates/generate/src/names.rs @@ -66,7 +66,11 @@ impl Names { let pointee_type = self.type_ref(&pointee, lifetime.clone()); quote!(wiggle_runtime::GuestPtr<#lifetime, #pointee_type>) } - _ => unimplemented!("anonymous type ref"), + witx::Type::Array(pointee) => { + let pointee_type = self.type_ref(&pointee, lifetime.clone()); + quote!(wiggle_runtime::GuestPtr<#lifetime, [#pointee_type]>) + } + _ => unimplemented!("anonymous type ref {:?}", tref), }, } } From a7e7863c471cd61bb5299c4dffa7fecf659d70fd Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Mon, 16 Mar 2020 17:39:32 -0700 Subject: [PATCH 2/5] wiggle-runtime: isize and usize do not have same repr in guest and host --- crates/wiggle/crates/runtime/src/guest_type.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/wiggle/crates/runtime/src/guest_type.rs b/crates/wiggle/crates/runtime/src/guest_type.rs index 1a654da17e..534eed5489 100644 --- a/crates/wiggle/crates/runtime/src/guest_type.rs +++ b/crates/wiggle/crates/runtime/src/guest_type.rs @@ -108,9 +108,9 @@ macro_rules! primitives { primitives! { // signed - i8 i16 i32 i64 i128 isize + i8 i16 i32 i64 i128 // unsigned - u8 u16 u32 u64 u128 usize + u8 u16 u32 u64 u128 // floats f32 f64 } From 2c52b3f1de0975e797af3f21ce48906ac5ae9229 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Mon, 16 Mar 2020 17:41:07 -0700 Subject: [PATCH 3/5] wiggle-generate: BuiltinType::USize is a u32, not a usize --- crates/wiggle/crates/generate/src/names.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/wiggle/crates/generate/src/names.rs b/crates/wiggle/crates/generate/src/names.rs index 75dd9b986f..aa67364a0e 100644 --- a/crates/wiggle/crates/generate/src/names.rs +++ b/crates/wiggle/crates/generate/src/names.rs @@ -38,7 +38,7 @@ impl Names { BuiltinType::F32 => quote!(f32), BuiltinType::F64 => quote!(f64), BuiltinType::Char8 => quote!(u8), - BuiltinType::USize => quote!(usize), + BuiltinType::USize => quote!(u32), } } pub fn atom_type(&self, atom: AtomType) -> TokenStream { From 0e72edb80e4e1f1611417ea542f4e1bfb2d4a661 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Mon, 16 Mar 2020 18:14:58 -0700 Subject: [PATCH 4/5] wiggle-generate: always pass GuestPtr by reference with the prev approach, it would be passed by reference sometimes (e.g. when used as an Array argument) but by value most of the time. this was inconsistient. theres no need to pass the owned version, all operations are &self. --- crates/wiggle/crates/generate/src/funcs.rs | 9 ++++---- .../crates/generate/src/module_trait.rs | 23 +++++++++++++++---- crates/wiggle/tests/flags.rs | 2 +- crates/wiggle/tests/pointers.rs | 8 +++---- crates/wiggle/tests/structs.rs | 9 +++++--- crates/wiggle/tests/wasi.rs | 18 +++++++-------- 6 files changed, 44 insertions(+), 25 deletions(-) diff --git a/crates/wiggle/crates/generate/src/funcs.rs b/crates/wiggle/crates/generate/src/funcs.rs index 6433f7712a..2c82ba79e8 100644 --- a/crates/wiggle/crates/generate/src/funcs.rs +++ b/crates/wiggle/crates/generate/src/funcs.rs @@ -2,6 +2,7 @@ use proc_macro2::TokenStream; use quote::quote; use crate::lifetimes::anon_lifetime; +use crate::module_trait::passed_by_reference; use crate::names::Names; pub fn define_func(names: &Names, func: &witx::InterfaceFunc) -> TokenStream { @@ -67,10 +68,10 @@ pub fn define_func(names: &Names, func: &witx::InterfaceFunc) -> TokenStream { .map(|p| marshal_arg(names, p, error_handling(p.name.as_str()))); let trait_args = func.params.iter().map(|param| { let name = names.func_param(¶m.name); - match param.tref.type_().passed_by() { - witx::TypePassedBy::Value { .. } => quote!(#name), - witx::TypePassedBy::Pointer { .. } => quote!(&#name), - witx::TypePassedBy::PointerLengthPair { .. } => quote!(&#name), + if passed_by_reference(&*param.tref.type_()) { + quote!(&#name) + } else { + quote!(#name) } }); diff --git a/crates/wiggle/crates/generate/src/module_trait.rs b/crates/wiggle/crates/generate/src/module_trait.rs index bd94946bba..b09f237028 100644 --- a/crates/wiggle/crates/generate/src/module_trait.rs +++ b/crates/wiggle/crates/generate/src/module_trait.rs @@ -5,6 +5,21 @@ use crate::lifetimes::{anon_lifetime, LifetimeExt}; use crate::names::Names; use witx::Module; +pub fn passed_by_reference(ty: &witx::Type) -> bool { + let passed_by = match ty.passed_by() { + witx::TypePassedBy::Value { .. } => false, + witx::TypePassedBy::Pointer { .. } | witx::TypePassedBy::PointerLengthPair { .. } => true, + }; + match ty { + witx::Type::Builtin(b) => match &*b { + witx::BuiltinType::String => true, + _ => passed_by, + }, + witx::Type::Pointer(_) | witx::Type::ConstPointer(_) | witx::Type::Array(_) => true, + _ => passed_by, + } +} + pub fn define_module_trait(names: &Names, m: &Module) -> TokenStream { let traitname = names.trait_name(&m.name); let traitmethods = m.funcs().map(|f| { @@ -25,10 +40,10 @@ pub fn define_module_trait(names: &Names, m: &Module) -> TokenStream { let args = f.params.iter().map(|arg| { let arg_name = names.func_param(&arg.name); let arg_typename = names.type_ref(&arg.tref, lifetime.clone()); - let arg_type = match arg.tref.type_().passed_by() { - witx::TypePassedBy::Value { .. } => quote!(#arg_typename), - witx::TypePassedBy::Pointer { .. } => quote!(&#arg_typename), - witx::TypePassedBy::PointerLengthPair { .. } => quote!(&#arg_typename), + let arg_type = if passed_by_reference(&*arg.tref.type_()) { + quote!(&#arg_typename) + } else { + quote!(#arg_typename) }; quote!(#arg_name: #arg_type) }); diff --git a/crates/wiggle/tests/flags.rs b/crates/wiggle/tests/flags.rs index 7a6ddcaebf..4270ef50c7 100644 --- a/crates/wiggle/tests/flags.rs +++ b/crates/wiggle/tests/flags.rs @@ -14,7 +14,7 @@ impl<'a> flags::Flags for WasiCtx<'a> { fn configure_car( &self, old_config: types::CarConfig, - other_config_ptr: GuestPtr, + other_config_ptr: &GuestPtr, ) -> Result { let other_config = other_config_ptr.read().map_err(|e| { eprintln!("old_config_ptr error: {}", e); diff --git a/crates/wiggle/tests/pointers.rs b/crates/wiggle/tests/pointers.rs index 188fa9822c..cbabd76563 100644 --- a/crates/wiggle/tests/pointers.rs +++ b/crates/wiggle/tests/pointers.rs @@ -13,9 +13,9 @@ impl<'a> pointers::Pointers for WasiCtx<'a> { fn pointers_and_enums<'b>( &self, input1: types::Excuse, - input2_ptr: GuestPtr<'b, types::Excuse>, - input3_ptr: GuestPtr<'b, types::Excuse>, - input4_ptr_ptr: GuestPtr<'b, GuestPtr<'b, types::Excuse>>, + input2_ptr: &GuestPtr<'b, types::Excuse>, + input3_ptr: &GuestPtr<'b, types::Excuse>, + input4_ptr_ptr: &GuestPtr<'b, GuestPtr<'b, types::Excuse>>, ) -> Result<(), types::Errno> { println!("BAZ input1 {:?}", input1); let input2: types::Excuse = input2_ptr.read().map_err(|e| { @@ -52,7 +52,7 @@ impl<'a> pointers::Pointers for WasiCtx<'a> { println!("input4 {:?}", input4); // Write ptr value to mutable ptr: - input4_ptr_ptr.write(input2_ptr).map_err(|e| { + input4_ptr_ptr.write(*input2_ptr).map_err(|e| { eprintln!("input4_ptr_ptr error: {}", e); types::Errno::InvalidArg })?; diff --git a/crates/wiggle/tests/structs.rs b/crates/wiggle/tests/structs.rs index 0b00057d9a..0be912feeb 100644 --- a/crates/wiggle/tests/structs.rs +++ b/crates/wiggle/tests/structs.rs @@ -44,10 +44,13 @@ impl<'a> structs::Structs for WasiCtx<'a> { fn return_pair_of_ptrs<'b>( &self, - first: GuestPtr<'b, i32>, - second: GuestPtr<'b, i32>, + first: &GuestPtr<'b, i32>, + second: &GuestPtr<'b, i32>, ) -> Result, types::Errno> { - Ok(types::PairIntPtrs { first, second }) + Ok(types::PairIntPtrs { + first: *first, + second: *second, + }) } } diff --git a/crates/wiggle/tests/wasi.rs b/crates/wiggle/tests/wasi.rs index 6ad5612b69..df7be73a8e 100644 --- a/crates/wiggle/tests/wasi.rs +++ b/crates/wiggle/tests/wasi.rs @@ -23,7 +23,7 @@ impl<'a> GuestErrorType<'a> for types::Errno { } impl<'a> crate::wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx<'a> { - fn args_get(&self, _argv: GuestPtr>, _argv_buf: GuestPtr) -> Result<()> { + fn args_get(&self, _argv: &GuestPtr>, _argv_buf: &GuestPtr) -> Result<()> { unimplemented!("args_get") } @@ -33,8 +33,8 @@ impl<'a> crate::wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx<'a> { fn environ_get( &self, - _environ: GuestPtr>, - _environ_buf: GuestPtr, + _environ: &GuestPtr>, + _environ_buf: &GuestPtr, ) -> Result<()> { unimplemented!("environ_get") } @@ -153,7 +153,7 @@ impl<'a> crate::wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx<'a> { fn fd_prestat_dir_name( &self, _fd: types::Fd, - _path: GuestPtr, + _path: &GuestPtr, _path_len: types::Size, ) -> Result<()> { unimplemented!("fd_prestat_dir_name") @@ -175,7 +175,7 @@ impl<'a> crate::wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx<'a> { fn fd_readdir( &self, _fd: types::Fd, - _buf: GuestPtr, + _buf: &GuestPtr, _buf_len: types::Size, _cookie: types::Dircookie, ) -> Result { @@ -260,7 +260,7 @@ impl<'a> crate::wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx<'a> { &self, _fd: types::Fd, _path: &GuestPtr<'_, str>, - _buf: GuestPtr, + _buf: &GuestPtr, _buf_len: types::Size, ) -> Result { unimplemented!("path_readlink") @@ -295,8 +295,8 @@ impl<'a> crate::wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx<'a> { fn poll_oneoff( &self, - _in_: GuestPtr, - _out: GuestPtr, + _in_: &GuestPtr, + _out: &GuestPtr, _nsubscriptions: types::Size, ) -> Result { unimplemented!("poll_oneoff") @@ -314,7 +314,7 @@ impl<'a> crate::wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx<'a> { unimplemented!("sched_yield") } - fn random_get(&self, _buf: GuestPtr, _buf_len: types::Size) -> Result<()> { + fn random_get(&self, _buf: &GuestPtr, _buf_len: types::Size) -> Result<()> { unimplemented!("random_get") } From 73fe49cd65f955d55f092db9a7ff2a3a93e62ccc Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Fri, 20 Mar 2020 14:14:47 -0700 Subject: [PATCH 5/5] wasi-common: update trait methods to take &GuestPtr args. --- .../src/snapshots/wasi_snapshot_preview1.rs | 29 ++++++++++++------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs b/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs index 60d788df13..5838a43254 100644 --- a/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs +++ b/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs @@ -14,11 +14,14 @@ use wiggle_runtime::{GuestBorrows, GuestPtr}; impl<'a> WasiSnapshotPreview1 for WasiCtx { fn args_get<'b>( &self, - mut argv: GuestPtr<'b, GuestPtr<'b, u8>>, - mut argv_buf: GuestPtr<'b, u8>, + argv: &GuestPtr<'b, GuestPtr<'b, u8>>, + argv_buf: &GuestPtr<'b, u8>, ) -> Result<()> { trace!("args_get(argv_ptr={:?}, argv_buf={:?})", argv, argv_buf); + let mut argv = argv.clone(); + let mut argv_buf = argv_buf.clone(); + for arg in &self.args { let arg_bytes = arg.as_bytes_with_nul(); let elems = arg_bytes.len().try_into()?; @@ -49,8 +52,8 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { fn environ_get<'b>( &self, - mut environ: GuestPtr<'b, GuestPtr<'b, u8>>, - mut environ_buf: GuestPtr<'b, u8>, + environ: &GuestPtr<'b, GuestPtr<'b, u8>>, + environ_buf: &GuestPtr<'b, u8>, ) -> Result<()> { trace!( "environ_get(environ={:?}, environ_buf={:?})", @@ -58,6 +61,9 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { environ_buf ); + let mut environ = environ.clone(); + let mut environ_buf = environ_buf.clone(); + for e in &self.env { let environ_bytes = e.as_bytes_with_nul(); let elems = environ_bytes.len().try_into()?; @@ -409,7 +415,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { fn fd_prestat_dir_name( &self, fd: types::Fd, - path: GuestPtr, + path: &GuestPtr, path_len: types::Size, ) -> Result<()> { trace!( @@ -536,7 +542,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { fn fd_readdir( &self, fd: types::Fd, - buf: GuestPtr, + buf: &GuestPtr, buf_len: types::Size, cookie: types::Dircookie, ) -> Result { @@ -555,10 +561,11 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { fn copy_entities>>( iter: T, - mut buf: GuestPtr, + buf: &GuestPtr, buf_len: types::Size, ) -> Result { let mut bufused = 0; + let mut buf = buf.clone(); for pair in iter { let (dirent, name) = pair?; let dirent_raw = dirent.as_bytes()?; @@ -971,7 +978,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { &self, dirfd: types::Fd, path: &GuestPtr<'_, str>, - buf: GuestPtr, + buf: &GuestPtr, buf_len: types::Size, ) -> Result { trace!( @@ -1140,8 +1147,8 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { fn poll_oneoff( &self, - in_: GuestPtr, - out: GuestPtr, + in_: &GuestPtr, + out: &GuestPtr, nsubscriptions: types::Size, ) -> Result { trace!( @@ -1294,7 +1301,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { Ok(()) } - fn random_get(&self, buf: GuestPtr, buf_len: types::Size) -> Result<()> { + fn random_get(&self, buf: &GuestPtr, buf_len: types::Size) -> Result<()> { trace!("random_get(buf={:?}, buf_len={:?})", buf, buf_len); let slice = unsafe {