From a679412dd0132b22b6dd8864f1ffb9fb15d59b3e Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Mon, 30 Sep 2019 10:22:11 -0700 Subject: [PATCH] Sync with lucet-wasi (#106) * Open /dev/null for writing as well as reading. Port this fix to wasi-common: https://github.com/fastly/lucet/commit/b905c4448308d89fe600f314c14932644985f877 * Remove all remaining uses of `std::mem::uninitialized`. Patch inspired by: https://github.com/fastly/lucet/commit/2d6519d051fef9faca44f036d87b5e7698d3e008 * Replace libc::memcpy() calls with std::ptr::copy_nonoverlapping() Port this fix to wasi-common: https://github.com/fastly/lucet/commit/a3f3a33e9b36b6ea6334c04c85225384ea507741 * Pass `WasiError` by value. It's a `u16` underneath, so we can pass it by value. * Avoid unnecessary explicit lifetime parameters. * Use immutable references rather than mutable references. Patch inspired by: https://github.com/fastly/lucet/commit/54baa4c38c2a0a17d91039dbd30c1eed6bab81f7 --- src/error.rs | 4 ++-- src/host.rs | 8 ++++---- src/memory.rs | 2 +- src/sys/unix/bsd/hostcalls_impl.rs | 6 +++--- src/sys/unix/hostcalls_impl/misc.rs | 11 +++++++---- src/sys/unix/linux/hostcalls_impl.rs | 12 +++++++----- src/sys/unix/mod.rs | 8 ++++++-- src/sys/windows/mod.rs | 8 ++++++-- wasi-common-cbindgen/src/lib.rs | 16 ++++++++++------ 9 files changed, 46 insertions(+), 29 deletions(-) diff --git a/src/error.rs b/src/error.rs index d8d50cb67d..6b64dd8473 100644 --- a/src/error.rs +++ b/src/error.rs @@ -87,8 +87,8 @@ pub enum WasiError { } impl WasiError { - pub fn as_raw_errno(&self) -> host::__wasi_errno_t { - *self as host::__wasi_errno_t + pub fn as_raw_errno(self) -> host::__wasi_errno_t { + self as host::__wasi_errno_t } } diff --git a/src/host.rs b/src/host.rs index 271ccb4108..37594be301 100644 --- a/src/host.rs +++ b/src/host.rs @@ -479,23 +479,23 @@ pub(crate) struct __wasi_subscription_t___wasi_subscription_u___wasi_subscriptio } #[allow(unused)] -pub(crate) unsafe fn ciovec_to_host<'a>(ciovec: &'a __wasi_ciovec_t) -> io::IoSlice<'a> { +pub(crate) unsafe fn ciovec_to_host(ciovec: &__wasi_ciovec_t) -> io::IoSlice { let slice = slice::from_raw_parts(ciovec.buf as *const u8, ciovec.buf_len); io::IoSlice::new(slice) } #[allow(unused)] -pub(crate) unsafe fn ciovec_to_host_mut<'a>(ciovec: &'a mut __wasi_ciovec_t) -> io::IoSliceMut<'a> { +pub(crate) unsafe fn ciovec_to_host_mut(ciovec: &mut __wasi_ciovec_t) -> io::IoSliceMut { let slice = slice::from_raw_parts_mut(ciovec.buf as *mut u8, ciovec.buf_len); io::IoSliceMut::new(slice) } -pub(crate) unsafe fn iovec_to_host<'a>(iovec: &'a __wasi_iovec_t) -> io::IoSlice<'a> { +pub(crate) unsafe fn iovec_to_host(iovec: &__wasi_iovec_t) -> io::IoSlice { let slice = slice::from_raw_parts(iovec.buf as *const u8, iovec.buf_len); io::IoSlice::new(slice) } -pub(crate) unsafe fn iovec_to_host_mut<'a>(iovec: &'a mut __wasi_iovec_t) -> io::IoSliceMut<'a> { +pub(crate) unsafe fn iovec_to_host_mut(iovec: &mut __wasi_iovec_t) -> io::IoSliceMut { let slice = slice::from_raw_parts_mut(iovec.buf as *mut u8, iovec.buf_len); io::IoSliceMut::new(slice) } diff --git a/src/memory.rs b/src/memory.rs index 17986bc40a..8ef8163c17 100644 --- a/src/memory.rs +++ b/src/memory.rs @@ -110,7 +110,7 @@ pub(crate) fn enc_slice_of( // get the pointer into guest memory, and copy the bytes let ptr = dec_ptr_mut(memory, ptr, len_bytes)? as *mut libc::c_void; unsafe { - libc::memcpy(ptr, slice.as_ptr() as *const libc::c_void, len_bytes); + ptr::copy_nonoverlapping(slice.as_ptr() as *const libc::c_void, ptr, len_bytes); } Ok(()) diff --git a/src/sys/unix/bsd/hostcalls_impl.rs b/src/sys/unix/bsd/hostcalls_impl.rs index b79e7c8a75..35a57a03b1 100644 --- a/src/sys/unix/bsd/hostcalls_impl.rs +++ b/src/sys/unix/bsd/hostcalls_impl.rs @@ -62,7 +62,7 @@ pub(crate) fn fd_readdir( cookie: host::__wasi_dircookie_t, ) -> Result { use crate::sys::unix::bsd::osfile::DirStream; - use libc::{fdopendir, memcpy, readdir, rewinddir, seekdir, telldir}; + use libc::{fdopendir, readdir, rewinddir, seekdir, telldir}; use nix::errno::Errno; use std::mem::ManuallyDrop; use std::sync::Mutex; @@ -129,9 +129,9 @@ pub(crate) fn fd_readdir( host_buf_offset += std::mem::size_of_val(&entry); let name_ptr = unsafe { *host_entry }.d_name.as_ptr(); unsafe { - memcpy( - host_buf_ptr.offset(host_buf_offset.try_into()?) as *mut _, + std::ptr::copy_nonoverlapping( name_ptr as *const _, + host_buf_ptr.offset(host_buf_offset.try_into()?) as *mut _, name_len, ) }; diff --git a/src/sys/unix/hostcalls_impl/misc.rs b/src/sys/unix/hostcalls_impl/misc.rs index e13c062aed..05d5a5d8ed 100644 --- a/src/sys/unix/hostcalls_impl/misc.rs +++ b/src/sys/unix/hostcalls_impl/misc.rs @@ -5,6 +5,7 @@ use crate::sys::host_impl; use crate::{host, wasm32, Error, Result}; use nix::libc::{self, c_int}; use std::cmp; +use std::mem::MaybeUninit; use std::time::SystemTime; pub(crate) fn clock_res_get(clock_id: host::__wasi_clockid_t) -> Result { @@ -18,11 +19,12 @@ pub(crate) fn clock_res_get(clock_id: host::__wasi_clockid_t) -> Result() }; - let res = unsafe { libc::clock_getres(clock_id, &mut timespec as *mut libc::timespec) }; + let mut timespec = MaybeUninit::::uninit(); + let res = unsafe { libc::clock_getres(clock_id, timespec.as_mut_ptr()) }; if res != 0 { return Err(host_impl::errno_from_nix(nix::errno::Errno::last())); } + let timespec = unsafe { timespec.assume_init() }; // convert to nanoseconds, returning EOVERFLOW in case of overflow; // this is freelancing a bit from the spec but seems like it'll @@ -52,11 +54,12 @@ pub(crate) fn clock_time_get(clock_id: host::__wasi_clockid_t) -> Result() }; - let res = unsafe { libc::clock_gettime(clock_id, &mut timespec as *mut libc::timespec) }; + let mut timespec = MaybeUninit::::uninit(); + let res = unsafe { libc::clock_gettime(clock_id, timespec.as_mut_ptr()) }; if res != 0 { return Err(host_impl::errno_from_nix(nix::errno::Errno::last())); } + let timespec = unsafe { timespec.assume_init() }; // convert to nanoseconds, returning EOVERFLOW in case of overflow; this is freelancing a bit // from the spec but seems like it'll be an unusual situation to hit diff --git a/src/sys/unix/linux/hostcalls_impl.rs b/src/sys/unix/linux/hostcalls_impl.rs index 632cb6917d..13952d01b0 100644 --- a/src/sys/unix/linux/hostcalls_impl.rs +++ b/src/sys/unix/linux/hostcalls_impl.rs @@ -6,6 +6,7 @@ use nix::libc::{self, c_long, c_void}; use std::convert::TryInto; use std::ffi::CString; use std::fs::File; +use std::mem::MaybeUninit; use std::os::unix::prelude::AsRawFd; pub(crate) fn path_rename(resolved_old: PathGet, resolved_new: PathGet) -> Result<()> { @@ -33,7 +34,7 @@ pub(crate) fn fd_readdir( host_buf: &mut [u8], cookie: host::__wasi_dircookie_t, ) -> Result { - use libc::{dirent, fdopendir, memcpy, readdir_r, rewinddir, seekdir}; + use libc::{dirent, fdopendir, readdir_r, rewinddir, seekdir}; let host_buf_ptr = host_buf.as_mut_ptr(); let host_buf_len = host_buf.len(); @@ -50,7 +51,7 @@ pub(crate) fn fd_readdir( unsafe { rewinddir(dir) }; } - let mut entry_buf = unsafe { std::mem::uninitialized::() }; + let mut entry_buf = MaybeUninit::::uninit(); let mut left = host_buf_len; let mut host_buf_offset: usize = 0; while left > 0 { @@ -61,13 +62,14 @@ pub(crate) fn fd_readdir( // replacing it with `readdir` call instead. // Also, `readdir_r` returns a positive int on failure, and doesn't // set the errno. - let res = unsafe { readdir_r(dir, &mut entry_buf, &mut host_entry) }; + let res = unsafe { readdir_r(dir, entry_buf.as_mut_ptr(), &mut host_entry) }; if res == -1 { return Err(host_impl::errno_from_nix(nix::errno::Errno::last())); } if host_entry.is_null() { break; } + unsafe { entry_buf.assume_init() }; let entry: host::__wasi_dirent_t = host_impl::dirent_from_host(&unsafe { *host_entry })?; log::debug!("fd_readdir entry = {:?}", entry); @@ -85,9 +87,9 @@ pub(crate) fn fd_readdir( host_buf_offset += std::mem::size_of_val(&entry); let name_ptr = unsafe { *host_entry }.d_name.as_ptr(); unsafe { - memcpy( - host_buf_ptr.offset(host_buf_offset.try_into()?) as *mut _, + std::ptr::copy_nonoverlapping( name_ptr as *const _, + host_buf_ptr.offset(host_buf_offset.try_into()?) as *mut _, name_len, ) }; diff --git a/src/sys/unix/mod.rs b/src/sys/unix/mod.rs index c69c587dea..7b7fa65757 100644 --- a/src/sys/unix/mod.rs +++ b/src/sys/unix/mod.rs @@ -15,11 +15,15 @@ mod bsd; mod linux; use crate::Result; -use std::fs::File; +use std::fs::{File, OpenOptions}; use std::path::Path; pub(crate) fn dev_null() -> Result { - File::open("/dev/null").map_err(Into::into) + OpenOptions::new() + .read(true) + .write(true) + .open("/dev/null") + .map_err(Into::into) } pub fn preopen_dir>(path: P) -> Result { diff --git a/src/sys/windows/mod.rs b/src/sys/windows/mod.rs index 3f6c7f42ca..398d945323 100644 --- a/src/sys/windows/mod.rs +++ b/src/sys/windows/mod.rs @@ -3,11 +3,15 @@ pub(crate) mod host_impl; pub(crate) mod hostcalls_impl; use crate::Result; -use std::fs::File; +use std::fs::{File, OpenOptions}; use std::path::Path; pub(crate) fn dev_null() -> Result { - File::open("NUL").map_err(Into::into) + OpenOptions::new() + .read(true) + .write(true) + .open("NUL") + .map_err(Into::into) } pub fn preopen_dir>(path: P) -> Result { diff --git a/wasi-common-cbindgen/src/lib.rs b/wasi-common-cbindgen/src/lib.rs index e6146bcdaa..dcb7f0730d 100644 --- a/wasi-common-cbindgen/src/lib.rs +++ b/wasi-common-cbindgen/src/lib.rs @@ -62,12 +62,16 @@ pub fn wasi_common_cbindgen(attr: TokenStream, function: TokenStream) -> TokenSt arg_ident.push(quote!(#len_ident)); arg_type.push(quote!(usize)); } else { - // & or &mut type - // so simply substitute with *mut type - arg_type.push(quote!(*mut #elem)); - // we need to properly dereference the substituted raw - // pointer if we are to properly call the hostcall fn - call_arg_ident.push(quote!(&mut *#ident)); + // & or &mut type; substitute with *const or *mut type. + // Also, we need to properly dereference the substituted raw + // pointer if we are to properly call the hostcall fn. + if ty.mutability.is_none() { + arg_type.push(quote!(*const #elem)); + call_arg_ident.push(quote!(&*#ident)); + } else { + arg_type.push(quote!(*mut #elem)); + call_arg_ident.push(quote!(&mut *#ident)); + } } } else { arg_type.push(quote!(#ty));