Commit Graph

15 Commits

Author SHA1 Message Date
Alex Crichton
92a6636b6d Add more thorough safety documentation to types 2020-03-05 08:25:04 -08:00
Alex Crichton
8cee547519 Run rustfmt 2020-03-04 10:36:26 -08:00
Alex Crichton
ca9f33b6d9 Rewrite for recursive safety
This commit rewrites the runtime crate to provide safety in the face
of recursive calls to the guest. The basic principle is that
`GuestMemory` is now a trait which dynamically returns the
pointer/length pair. This also has an implicit contract (hence the
`unsafe` trait) that the pointer/length pair point to a valid list of
bytes in host memory "until something is reentrant".

After this changes the various suite of `Guest*` types were rewritten.
`GuestRef` and `GuestRefMut` were both removed since they cannot safely
exist. The `GuestPtrMut` type was removed for simplicity, and the final
`GuestPtr` type subsumes `GuestString` and `GuestArray`. This means
that there's only one guest pointer type, `GuestPtr<'a, T>`, where `'a`
is the borrow into host memory, basically borrowing the `GuestMemory`
trait object itself.

Some core utilities are exposed on `GuestPtr`, but they're all 100%
safe. Unsafety is now entirely contained within a few small locations:

* Implementations of the `GuestType` for primitive types (e.g. `i8`,
  `u8`, etc) use `unsafe` to read/write memory. The `unsafe` trait of
  `GuestMemory` though should prove that they're safe.

* `GuestPtr<'_, str>` has a method which validates utf-8 contents, and
  this requires `unsafe` internally to read all the bytes. This is
  guaranteed to be safe however given the contract of `GuestMemory`.

And that's it! Everything else is a bunch of safe combinators all built
up on the various utilities provided by `GuestPtr`. The general idioms
are roughly the same as before, with various tweaks here and there. A
summary of expected idioms are:

* For small values you'd `.read()` or `.write()` very quickly. You'd
  pass around the type itself.

* For strings, you'd pass `GuestPtr<'_, str>` down to the point where
  it's actually consumed. At that moment you'd either decide to copy it
  out (a safe operation) or you'd get a raw view to the string (an
  unsafe operation) and assert that you won't call back into wasm while
  you're holding that pointer.

* Arrays are similar to strings, passing around `GuestPtr<'_, [T]>`.
  Arrays also have a `iter()` method which yields an iterator of
  `GuestPtr<'_, T>` for convenience.

Overall there's still a lot of missing documentation on the runtime
crate specifically around the safety of the `GuestMemory` trait as well
as how the utilities/methods are expected to be used. Additionally
there's utilities which aren't currently implemented which would be easy
to implement. For example there's no method to copy out a string or a
slice, although that would be pretty easy to add.

In any case I'm curious to get feedback on this approach and see what
y'all think!
2020-03-04 10:26:47 -08:00
Jakub Konka
3764204250 Put context object behind a ref rather than mut ref
This commit puts context object, i.e., the implementor of the
WASI snapshot, behind a reference `&self` rather than a mutable
reference `&mut self`. As suggested by @alexcrichton, this gives
the implementor the possibility to determine how it handles its
interior mutability.
2020-03-03 17:50:32 +01:00
Jakub Konka
ea4d2d0535 Return *mut u8 in GuestPtrMut::as_raw
Currently, we create an immutable `GuestPtr` from `self` and call
`as_raw` on it which correctly returns `*const u8`. However, since
we're dealing with `GuestPtrMut` I thought it might make more sense
to return `*mut u8` directly instead. This will be needed (and will
save us from silly casts `*const _ as *mut _`) in plugging in
`Iovec<'_>` into `std::io::IoSliceMut` in `fd_read` and `fd_pread` calls.
2020-03-03 10:19:50 +01:00
Jakub Konka
ec75f874ee Unify GuestType and GuestTypeClone, rename GuestTypeCopy to GuestTypeTransparent
This commit refactors trait system for guest types. Namely, as
discussed offline on zulip, `GuestType` now includes `GuestTypeClone`,
whereas `GuestTypeCopy` has been renamed to `GuestTypeTransparent`.
2020-02-27 21:53:52 +01:00
Jakub Konka
ed1d5180ef Add TryFromIntError to GuestError and fix error handling
While working on the full WASI spec, it turned out that we need two
tweaks to `GuestError`:
1. we need to support conversion from `TryFromIntError`, and
2. we need to invoke `e.into()` when unwrapping the result of `try_into()`
   in auto-implementation of raw interface functions.

Both problems seem to originate for "transparent" builtin types since
for those we don't really provide a `TryFrom` implementation like for
compound types, e.g., enums, flags, etc.
2020-02-27 21:46:04 +01:00
Pat Hickey
25a411d7fd rename the pointer read/write methods to read and write
these names were artifacts of some early confusion / bad design i made
in the traits. read and write are much simpler names!

also, change a ptr_mut to ptr where we just read the contents in the
argument marshalling for structs. this has no effect, but it is more
correct.
2020-02-26 19:51:35 +01:00
Jakub Konka
a02bce6eaf Revert changes and require Clone if Copy
This commit aligns `wiggle` a little bit closer with Rust proper in
the sense that now `GuestTypeCopy` implies `GuestTypeClone` which
in turn implies that any type implementing `GuestTypeCopy` will have
to provide `read_from_guest` and `write_to_guest` methods. As a result,
we can safely revert changes introduced in the previous commit.
2020-02-26 18:23:46 +01:00
Jakub Konka
6ab3ff71d2 Add basic GuestString support to wiggle (#13)
* Add basic GuestString support to wiggle

This commit adds basic `GuestString` support to `wiggle`. `GuestString`
is a wrapper around `GuestArray<'_, u8>` array type which itself can
be made into either an owned (cloned) Rust `String` or borrowed as
a reference `&str`. In both cases, `GuestString` ensures that the
underlying bytes are valid Unicode code units, throwing a `InvalidUtf8`
error if not.

This commit adds support *only* for passing in strings as arguments
in WASI. Marshalling of the return arg has not yet been implemented.
I'm not even sure it's possible without multi-value return args
feature of Wasm. It's not a major setback especially since the WASI
spec (and this includes even the `ephemeral` snapshot) doesn't
return strings anywhere. They are only ever passed in as arguments
to interface functions.

It should be noted that error returned in case of invalid UTF-8
requires a lot more love as it doesn't include anything besides
flagging an event that the string contained an invalid Unicode code unit.

* Borrow all of string's memory including nul-byte

Borrow all of string's underlying memory including the nul-byte.
This perhaps might not have a tremendous impact on anything, but
since the nul-byte is technically part of the WASI string, we should
include it in the borrow as well.

* Fill in wiggle-generate blanks for strings

* Print to screen passed string in proptest

* Strings are PointerLengthPairs!

* Fix generation of strings in compound types

* Update test with simple string strategy

* Generate better test strings

* Finalise proptest for strings

* Fix formatting

* Update crates/runtime/src/memory/string.rs

Removes unnecessary comment in code

* Apply Pat's suggestion to wrap Utf8Error as error
2020-02-21 22:37:22 +01:00
Pat Hickey
2f223acc55 merge GuestTypePtr and GuestTypeClone into a single GuestTypeClone<'a> trait (#14)
* merge GuestTypePtr and GuestTypeClone into a single GuestTypeClone<'a> trait

* GuestArray can derive Clone (but not impl GuestTypeClone)

* fix array tests

* Fix GuestTypeClone for flags

Co-authored-by: Jakub Konka <kubkon@jakubkonka.com>
2020-02-21 17:43:45 +01:00
Jakub Konka
3ef24a04fe Start drafting out flags datatype generation (#11)
* Adds support for flags datatype

This commit adds support for `FlagsDatatype`. In WASI, flags are
represented as bitfields/bitflags, therefore, it is important
for the type to have bitwise operations implemented, and some
way of checking whether the current set of flags contains some
other set (i.e., they intersect). Thus, this commit automatically
derives `BitAnd`, etc. for the derived flags datatype. It also
automatically provides an `ALL_FLAGS` value which corresponds to
a bitwise-or of all flag values present and is provided for
convenience.

* Simplify read_from_guest
2020-02-21 09:58:43 +01:00
Jakub Konka
898af8e2fb 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.
2020-02-20 23:16:53 +01:00
Jakub Konka
2ad77538f5 Add array generation to wiggle-generate crate (#9)
* Add array generation to wiggle-generate crate

This commit:
* adds array generation to `wiggle-generate` crate which implies
  that we can now generate arrays from `witx` files
* introduces two test interface functions `foo::reduce_excuses` and
  `foo::populate_excuses`, and adds matching prop-tests
* adds an out-of-boundary check to `HostMemory::mem_area_strat` since
  now, given we're generating arrays for testing with an arbitrary
  but bounded number of elements, it is possible to violate the boundary
* refactors `Region::extend` to a new signature `extend(times: u32)`
  which multiplies the current pointer `len` by `times`
* fixes bug in `GuestArray::as_ref` and `GuestArrayMut::as_ref_mut` methods
  where we were not validating the first element (we always started the
  validation from the second element)

* Fix generation of arrays in witx

This commit fixes how `arrays` are auto-generated from `witx` files.
In particular, the changes include:
* Since by design every `array` in `witx` represents an immutable
  slab of memory, we will only ever operate on `GuestArray` in which
  case I've gone ahead and removed `GuestArrayMut` so as to unclutter
  the code somewhat. If we find ourselves in need for it in the future,
  I reckon it will be better to write it from scratch (since the codebase
  will inevitably evolve by then considerably) rather than maintaining an
  unused implementation.
* I've rewritten `GuestArrayRef<T>` to be a wrapper for `Vec<GuestRef<T>>`.
  Also, `GuestArray::as_ref` now borrows each underlying "element" of the
  array one-by-one rather than borrowing the entire chunk of memory at once.
  This change is motivated by the inability to coerce type parameter `T` in
  `GuestArray<T>` in more complicated cases such as arrays of guest pointers
  `GuestPtr<T>` to `*const T` for reuse in `std::slice::from_raw_parts` call.
  (In general, the size of Wasm32 pointer is 4 bytes, while

```
std::mem::size_of::<T>() == std::mem::size_of::<GuestPtr<S>>() == 16
```

  which is problematic; i.e., I can't see how I could properly extract guest
  pointers from slices of 4 bytes and at the same time not allocate.)
* I've augmented fuzz tests by (re-)defining two `array` types:

```
(typename $const_excuse_array (array (@witx const_pointer $excuse)))
(typename $excuse_array (array (@witx pointer $excuse)))
```

  This should hopefully emulate and test the `iovec` and `ciovec` arrays
  present in WASI spec.
2020-02-19 10:58:55 +01:00
Jakub Konka
48a218b5c5 Refactor naming and crates info (#8)
* Refactor naming and crates info

This commit:
* changes workspace crates to have a `wiggle_` prefix in names
* rename `memory` module of `wiggle-memory` crate to `runtime`
* fixes authors of all crates

* Rename wiggle memory crate to runtime
2020-02-13 22:40:42 +01:00