Skip to content

Commit

Permalink
wiggle: adapt Wiggle strings for shared use (#5264)
Browse files Browse the repository at this point in the history 
* wiggle: adapt Wiggle strings for shared use

This is an extension of #5229 for the `&str` and `&mut str` types. As
documented there, we are attempting to maintain Rust guarantees for
slices that Wiggle hands out in the presence of WebAssembly shared
memory, in which case multiple threads could be modifying the underlying
data of the slice.

This change changes the API of `GuestPtr` to return an `Option` which is
`None` when attempting to view the WebAssembly data as a string and the
underlying WebAssembly memory is shared. This reuses the
`UnsafeGuestSlice` structure from #5229 to do so and appropriately marks
the region as borrowed in Wiggle's manual borrow checker. Each original
call site in this project's WASI implementations is fixed up to `expect`
that a non-shared memory is used.  (Note that I can find no uses of
`GuestStrMut` in the WASI implementations).

* wiggle: make `GuestStr*` containers wrappers of `GuestSlice*`

This change makes it possible to reuse the underlying logic in
`UnsafeGuestSlice` and the `GuestSlice*` implementations to continue to
expose the `GuestStr` and `GuestStrMut` types. These types now are
simple wrappers of their `GuestSlice*` variant. The UTF-8 validation
that distinguished `GuestStr*` now lives in the `TryFrom`
implementations for each type.
  • Loading branch information
@abrown
abrown committed Nov 14, 2022
1 parent 7a6fbe0 commit 060f125
Show file tree
Hide file tree
Showing 7 changed files with 94 additions and 107 deletions.
25 changes: 13 additions & 12 deletions crates/wasi-common/src/snapshots/preview_1.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -803,7 +803,7 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
self.table()
.get_dir(u32::from(dirfd))?
.get_cap(DirCaps::CREATE_DIRECTORY)?
.create_dir(path.as_str()?.deref())
.create_dir(path.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)").deref())
.await
}

Expand All @@ -818,7 +818,7 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
.get_dir(u32::from(dirfd))?
.get_cap(DirCaps::PATH_FILESTAT_GET)?
.get_path_filestat(
path.as_str()?.deref(),
path.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)").deref(),
flags.contains(types::Lookupflags::SYMLINK_FOLLOW),
)
.await?;
Expand All @@ -845,7 +845,7 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
.get_dir(u32::from(dirfd))?
.get_cap(DirCaps::PATH_FILESTAT_SET_TIMES)?
.set_times(
path.as_str()?.deref(),
path.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)").deref(),
atim,
mtim,
flags.contains(types::Lookupflags::SYMLINK_FOLLOW),
Expand Down Expand Up @@ -876,9 +876,9 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {

src_dir
.hard_link(
src_path.as_str()?.deref(),
src_path.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)").deref(),
target_dir.deref(),
target_path.as_str()?.deref(),
target_path.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)").deref(),
)
.await
}
Expand All @@ -904,7 +904,7 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {

let oflags = OFlags::from(&oflags);
let fdflags = FdFlags::from(fdflags);
let path = path.as_str()?;
let path = path.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
if oflags.contains(OFlags::DIRECTORY) {
if oflags.contains(OFlags::CREATE)
|| oflags.contains(OFlags::EXCLUSIVE)
Expand Down Expand Up @@ -953,7 +953,7 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
.table()
.get_dir(u32::from(dirfd))?
.get_cap(DirCaps::READLINK)?
.read_link(path.as_str()?.deref())
.read_link(path.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)").deref())
.await?
.into_os_string()
.into_string()
Expand All @@ -979,7 +979,7 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
self.table()
.get_dir(u32::from(dirfd))?
.get_cap(DirCaps::REMOVE_DIRECTORY)?
.remove_dir(path.as_str()?.deref())
.remove_dir(path.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)").deref())
.await
}

Expand All @@ -999,9 +999,9 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
.get_cap(DirCaps::RENAME_TARGET)?;
src_dir
.rename(
src_path.as_str()?.deref(),
src_path.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)").deref(),
dest_dir.deref(),
dest_path.as_str()?.deref(),
dest_path.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)").deref(),
)
.await
}
Expand All @@ -1015,7 +1015,7 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
self.table()
.get_dir(u32::from(dirfd))?
.get_cap(DirCaps::SYMLINK)?
.symlink(src_path.as_str()?.deref(), dest_path.as_str()?.deref())
.symlink(src_path.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)").deref(), dest_path.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)").deref())
.await
}

Expand All @@ -1027,7 +1027,8 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
self.table()
.get_dir(u32::from(dirfd))?
.get_cap(DirCaps::UNLINK_FILE)?
.unlink_file(path.as_str()?.deref())
.unlink_file(path.as_str()?
.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)").deref())
.await
}

Expand Down
10 changes: 5 additions & 5 deletions crates/wasi-crypto/src/wiggle_interfaces/asymmetric_common.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ impl super::wasi_ephemeral_crypto_asymmetric_common::WasiEphemeralCryptoAsymmetr
alg_str: &wiggle::GuestPtr<'_, str>,
options_handle: &guest_types::OptOptions,
) -> Result<guest_types::Keypair, guest_types::CryptoErrno> {
let alg_str = &*alg_str.as_str()?;
let alg_str = &*alg_str.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
let options_handle = match *options_handle {
guest_types::OptOptions::Some(options_handle) => Some(options_handle),
guest_types::OptOptions::None => None,
Expand Down Expand Up @@ -89,7 +89,7 @@ impl super::wasi_ephemeral_crypto_asymmetric_common::WasiEphemeralCryptoAsymmetr
alg_str: &wiggle::GuestPtr<'_, str>,
options_handle: &guest_types::OptOptions,
) -> Result<guest_types::Keypair, guest_types::CryptoErrno> {
let alg_str = &*alg_str.as_str()?;
let alg_str = &*alg_str.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
let options_handle = match *options_handle {
guest_types::OptOptions::Some(options_handle) => Some(options_handle),
guest_types::OptOptions::None => None,
Expand All @@ -107,7 +107,7 @@ impl super::wasi_ephemeral_crypto_asymmetric_common::WasiEphemeralCryptoAsymmetr
encoded_len: guest_types::Size,
encoding: guest_types::KeypairEncoding,
) -> Result<guest_types::Keypair, guest_types::CryptoErrno> {
let alg_str = &*alg_str.as_str()?;
let alg_str = &*alg_str.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
let encoded = &*encoded_ptr
.as_array(encoded_len)
.as_slice()?
Expand Down Expand Up @@ -167,7 +167,7 @@ impl super::wasi_ephemeral_crypto_asymmetric_common::WasiEphemeralCryptoAsymmetr
encoded_len: guest_types::Size,
encoding: guest_types::PublickeyEncoding,
) -> Result<guest_types::Publickey, guest_types::CryptoErrno> {
let alg_str = &*alg_str.as_str()?;
let alg_str = &*alg_str.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
let encoded = &*encoded_ptr
.as_array(encoded_len)
.as_slice()?
Expand Down Expand Up @@ -218,7 +218,7 @@ impl super::wasi_ephemeral_crypto_asymmetric_common::WasiEphemeralCryptoAsymmetr
encoded_len: guest_types::Size,
encoding: guest_types::SecretkeyEncoding,
) -> Result<guest_types::Secretkey, guest_types::CryptoErrno> {
let alg_str = &*alg_str.as_str()?;
let alg_str = &*alg_str.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
let encoded = &*encoded_ptr
.as_array(encoded_len)
.as_slice()?
Expand Down
6 changes: 3 additions & 3 deletions crates/wasi-crypto/src/wiggle_interfaces/common.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ impl super::wasi_ephemeral_crypto_common::WasiEphemeralCryptoCommon for WasiCryp
value_ptr: &wiggle::GuestPtr<'_, u8>,
value_len: guest_types::Size,
) -> Result<(), guest_types::CryptoErrno> {
let name_str: &str = &*name_str.as_str()?;
let name_str: &str = &*name_str.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
let value: &[u8] = {
&*value_ptr
.as_array(value_len)
Expand All @@ -44,7 +44,7 @@ impl super::wasi_ephemeral_crypto_common::WasiEphemeralCryptoCommon for WasiCryp
buffer_ptr: &wiggle::GuestPtr<'_, u8>,
buffer_len: guest_types::Size,
) -> Result<(), guest_types::CryptoErrno> {
let name_str: &str = &*name_str.as_str()?;
let name_str: &str = &*name_str.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
let buffer: &'static mut [u8] = unsafe {
std::mem::transmute(
&mut *buffer_ptr
Expand All @@ -62,7 +62,7 @@ impl super::wasi_ephemeral_crypto_common::WasiEphemeralCryptoCommon for WasiCryp
name_str: &wiggle::GuestPtr<'_, str>,
value: u64,
) -> Result<(), guest_types::CryptoErrno> {
let name_str: &str = &*name_str.as_str()?;
let name_str: &str = &*name_str.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
Ok((&*self).options_set_u64(options_handle.into(), name_str, value)?)
}

Expand Down
2 changes: 1 addition & 1 deletion crates/wasi-crypto/src/wiggle_interfaces/signatures.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ impl super::wasi_ephemeral_crypto_signatures::WasiEphemeralCryptoSignatures for
encoded_len: guest_types::Size,
encoding: guest_types::SignatureEncoding,
) -> Result<guest_types::Signature, guest_types::CryptoErrno> {
let alg_str = &*alg_str.as_str()?;
let alg_str = &*alg_str.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
let encoded = &*encoded_ptr
.as_array(encoded_len)
.as_slice()?
Expand Down
14 changes: 7 additions & 7 deletions crates/wasi-crypto/src/wiggle_interfaces/symmetric.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ impl super::wasi_ephemeral_crypto_symmetric::WasiEphemeralCryptoSymmetric for Wa
alg_str: &wiggle::GuestPtr<'_, str>,
options_handle: &guest_types::OptOptions,
) -> Result<guest_types::SymmetricKey, guest_types::CryptoErrno> {
let alg_str = &*alg_str.as_str()?;
let alg_str = &*alg_str.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
let options_handle = match *options_handle {
guest_types::OptOptions::Some(options_handle) => Some(options_handle),
guest_types::OptOptions::None => None,
Expand Down Expand Up @@ -86,7 +86,7 @@ impl super::wasi_ephemeral_crypto_symmetric::WasiEphemeralCryptoSymmetric for Wa
alg_str: &wiggle::GuestPtr<'_, str>,
options_handle: &guest_types::OptOptions,
) -> Result<guest_types::SymmetricKey, guest_types::CryptoErrno> {
let alg_str = &*alg_str.as_str()?;
let alg_str = &*alg_str.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
let options_handle = match *options_handle {
guest_types::OptOptions::Some(options_handle) => Some(options_handle),
guest_types::OptOptions::None => None,
Expand All @@ -102,7 +102,7 @@ impl super::wasi_ephemeral_crypto_symmetric::WasiEphemeralCryptoSymmetric for Wa
raw_ptr: &wiggle::GuestPtr<'_, u8>,
raw_len: guest_types::Size,
) -> Result<guest_types::SymmetricKey, guest_types::CryptoErrno> {
let alg_str = &*alg_str.as_str()?;
let alg_str = &*alg_str.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
let raw = &*raw_ptr
.as_array(raw_len)
.as_slice()?
Expand Down Expand Up @@ -153,7 +153,7 @@ impl super::wasi_ephemeral_crypto_symmetric::WasiEphemeralCryptoSymmetric for Wa
key_handle: &guest_types::OptSymmetricKey,
options_handle: &guest_types::OptOptions,
) -> Result<guest_types::SymmetricState, guest_types::CryptoErrno> {
let alg_str = &*alg_str.as_str()?;
let alg_str = &*alg_str.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
let key_handle = match *key_handle {
guest_types::OptSymmetricKey::Some(key_handle) => Some(key_handle),
guest_types::OptSymmetricKey::None => None,
Expand All @@ -178,7 +178,7 @@ impl super::wasi_ephemeral_crypto_symmetric::WasiEphemeralCryptoSymmetric for Wa
value_ptr: &wiggle::GuestPtr<'_, u8>,
value_max_len: guest_types::Size,
) -> Result<guest_types::Size, guest_types::CryptoErrno> {
let name_str: &str = &*name_str.as_str()?;
let name_str: &str = &*name_str.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
let value = &mut *value_ptr
.as_array(value_max_len)
.as_slice_mut()?
Expand All @@ -193,7 +193,7 @@ impl super::wasi_ephemeral_crypto_symmetric::WasiEphemeralCryptoSymmetric for Wa
symmetric_state_handle: guest_types::SymmetricState,
name_str: &wiggle::GuestPtr<'_, str>,
) -> Result<u64, guest_types::CryptoErrno> {
let name_str: &str = &*name_str.as_str()?;
let name_str: &str = &*name_str.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
Ok((&*self).options_get_u64(symmetric_state_handle.into(), name_str)?)
}

Expand Down Expand Up @@ -244,7 +244,7 @@ impl super::wasi_ephemeral_crypto_symmetric::WasiEphemeralCryptoSymmetric for Wa
symmetric_state_handle: guest_types::SymmetricState,
alg_str: &wiggle::GuestPtr<'_, str>,
) -> Result<guest_types::SymmetricKey, guest_types::CryptoErrno> {
let alg_str = &*alg_str.as_str()?;
let alg_str = &*alg_str.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
Ok((&*self)
.symmetric_state_squeeze_key(symmetric_state_handle.into(), alg_str)?
.into())
Expand Down
Loading

0 comments on commit 060f125

Please sign in to comment.