Commit Graph

6 Commits

Author SHA1 Message Date
Jakub Konka
78772cf5e1 Generate trace logs in wiggle albeit behind a feature gate
This commit augments `wiggle` with trace log generation for the shims,
returned errno values, and returned values proper (if any, i.e.,
different than unit type `()`). What that means is that every syscall
will have auto-generated up to 3 traces, for instance,

```
 TRACE wasi_common::wasi::wasi_snapshot_preview1 > fd_prestat_get(fd=Fd(3))
 TRACE wasi_common::wasi::wasi_snapshot_preview1 >      | result=(buf=Dir(PrestatDir { pr_name_len: 1 }))
 TRACE wasi_common::wasi::wasi_snapshot_preview1 >      | errno=No error occurred. System call completed successfully. (Errno::Success(0))
```

Putting logging behind a feature gate in this case means that the log calls
are generated by the `wiggle` crate regardless if the client requested
the feature or not, however, then their usage in the client lib is
dictated by the presence of the feature flag. So, for instance, `wasi-common`
has this feature enabled by default, while any other client lib
using `wiggle` if they don't want tracing enabled, they will just
leave the feature off. I'm not sure if this is what we wanted
but seemed easiest to implement quickly. Lemme y'all know your thoughts
about this!
2020-03-30 19:58:15 +02:00
Jakub Konka
5c51940100 Refactor use of Refs and RefMuts in wasi-common (#1412)
* Refactor use of Refs and RefMuts in wasi-common

This commit refactors the use of `Ref`s and `RefMut`s in `wasi-common`.
Now, `Entry` is stored behind an `Rc` inside the `EntryTable`. The `Entry`
itself on the other hand now stores rights behind a `RefCell` and the
descriptor as `Rc<RefCell<..>>` combo to enable easy reference tracking
and interior mutability which is required down the line in a couple of
syscalls. In essence, this implies that we no longer have need for
mutable accessor to `Entry` from `WasiCtx`, and so all related methods
go away (`get_entry_mut`, etc.).

While here, I've also simplified handling and aggregating of rights on
the `Entry` object. Instead of storing base and inheriting rights as
separate fields, they are now aggregated into one struct `EntryRights`
which features convenient constructors for each possible combination; i.e.,
when only base rights are set, or both base and inheriting are set, or
both are left as empty. Since we do need to be able to mutate those
rights down the line in `fdstat_set_rights` syscall, this object
is kept behind a `RefCell` (note no `Rc` since we don't need to pass it
around anywhere).

The descriptor field in `Entry` is now kept behind `Rc<RefCell<..>>` combo
since we not only need to mutate it down the line, but we also need to
be able to pass it around (as part of the machinery making `poll_oneoff`
work).

I've also removed `as_file` and `try_clone` methods on `Descriptor` struct
since they were adding more noise than necessary, and making them work
with `Rc` was unnecessarily complicated.

Finally, I've converted the `get_dir_from_os_handle` function into a
method attached to the `OsHandle` itself, called `dir_stream`. IMHO,
it makes more sense to have it there directly as a method than as a separate
function.

* Use Cell for types that are Copy
2020-03-27 09:34:52 +01:00
Jakub Konka
1f6890e070 [wasi-common] Clean up the use of mutable Entry (#1395)
* Clean up the use of mutable Entry

Until now, several syscalls including `fd_pwrite` etc. were relying on
mutating `&mut Entry` by mutating its inner file handle. This is
unnecessary in almost all cases since all methods mutating `std::fs::File`
in Rust's libstd are also implemented for `&std::fs::File`.

While here, I've also modified `OsHandle` in BSD to include `RefCell<Option<Dir>>`
rather than `Option<Mutex<Dir>>` as was until now. While `RefCell`
could easily be replaced with `RefCell`, since going multithreading
will require a lot of (probably even) conceptual changes to `wasi-common`,
I thought it'd be best not to mix single- with multithreading contexts
and swap all places at once when it comes to it.

I've also had to make some modifications to virtual FS which mainly
swapped mutability for interior mutability in places.

* Use one-liners wherever convenient
2020-03-25 14:00:52 +01:00
Jakub Konka
32595faba5 It's wiggle time! (#1202)
* Use wiggle in place of wig in wasi-common

This is a rather massive commit that introduces `wiggle` into the
picture. We still use `wig`'s macro in `old` snapshot and to generate
`wasmtime-wasi` glue, but everything else is now autogenerated by `wiggle`.
In summary, thanks to `wiggle`, we no longer need to worry about
serialising and deserialising to and from the guest memory, and
all guest (WASI) types are now proper idiomatic Rust types.

While we're here, in preparation for the ephemeral snapshot, I went
ahead and reorganised the internal structure of the crate. Instead of
modules like `hostcalls_impl` or `hostcalls_impl::fs`, the structure
now resembles that in ephemeral with modules like `path`, `fd`, etc.
Now, I'm not requiring we leave it like this, but I reckon it looks
cleaner this way after all.

* Fix wig to use new first-class access to caller's mem

* Ignore warning in proc_exit for the moment

* Group unsafes together in args and environ calls

* Simplify pwrite; more unsafe blocks

* Simplify fd_read

* Bundle up unsafes in fd_readdir

* Simplify fd_write

* Add comment to path_readlink re zero-len buffers

* Simplify unsafes in random_get

* Hide GuestPtr<str> to &str in path::get

* Rewrite pread and pwrite using SeekFrom and read/write_vectored

I've left the implementation of VirtualFs pretty much untouched
as I don't feel that comfortable in changing the API too much.
Having said that, I reckon `pread` and `pwrite` could be refactored
out, and `preadv` and `pwritev` could be entirely rewritten using
`seek` and `read_vectored` and `write_vectored`.

* Add comment about VirtFs unsafety

* Fix all mentions of FdEntry to Entry

* Fix warnings on Win

* Add aux struct EntryTable responsible for Fds and Entries

This commit adds aux struct `EntryTable` which is private to `WasiCtx`
and is basically responsible for `Fd` alloc/dealloc as well as storing
matching `Entry`s. This struct is entirely private to `WasiCtx` and
as such as should remain transparent to `WasiCtx` users.

* Remove redundant check for empty buffer in path_readlink

* Preserve and rewind file cursor in pread/pwrite

* Use GuestPtr<[u8]>::copy_from_slice wherever copying bytes directly

* Use GuestPtr<[u8]>::copy_from_slice in fd_readdir

* Clean up unsafes around WasiCtx accessors

* Fix bugs in args_get and environ_get

* Fix conflicts after rebase
2020-03-20 21:54:44 +01:00
Marcin Mielniczuk
33b4750a64 Fix unnecessary structure name repetitions, as reported by clippy 2020-03-18 14:43:08 -07:00
Jakub Konka
7228a248c1 [wasi-common] add custom FdPool container for managing fd allocs/deallocs (#1329)
* Rename FdEntry to Entry

* Add custom FdSet container for managing fd allocs/deallocs

This commit adds a custom `FdSet` container which is intended
for use in `wasi-common` to track WASI fd allocs/deallocs. The
main aim for this container is to abstract away the current
approach of spawning new handles

```rust
fd = fd.checked_add(1).ok_or(...)?;
```

and to make it possible to reuse unused/reclaimed handles
which currently is not done.

The struct offers 3 methods to manage its functionality:
* `FdSet::new` initialises the internal data structures,
  and most notably, it preallocates an `FdSet::BATCH_SIZE`
  worth of handles in such a way that we always start popping
  from the "smallest" handle (think of it as of reversed stack,
  I guess; it's not a binary heap since we don't really care
  whether internally the handles are sorted in some way, just that
  the "largets" handle is at the bottom. Why will become clear
  when describing `allocate` method.)
* `FdSet::allocate` pops the next available handle if one is available.
  The tricky bit here is that, if we run out of handles, we preallocate
  the next `FdSet::BATCH_SIZE` worth of handles starting from the
  latest popped handle (i.e., the "largest" handle). This
  works only because we make sure to only ever pop and push already
  existing handles from the back, and push _new_ handles (from the
  preallocation step) from the front. When we ultimately run out
  of _all_ available handles, we then return `None` for the client
  to handle in some way (e.g., throwing an error such as `WasiError::EMFILE`
  or whatnot).
* `FdSet::deallocate` returns the already allocated handle back to
  the pool for further reuse.

When figuring out the internals, I've tried to optimise for both
alloc and dealloc performance, and I believe we've got an amortised
`O(1)~*` performance for both (if my maths is right, and it may very
well not be, so please verify!).

In order to keep `FdSet` fairly generic, I've made sure not to hard-code
it for the current type system generated by `wig` (i.e., `wasi::__wasi_fd_t`
representing WASI handle), but rather, any type which wants to be managed
by `FdSet` needs to conform to `Fd` trait. This trait is quite simple as
it only requires a couple of rudimentary traits (although `std:#️⃣:Hash`
is quite a powerful assumption here!), and a custom method

```rust
Fd::next(&self) -> Option<Self>;
```

which is there to encapsulate creating another handle from the given one.
In the current state of the code, that'd be simply `u32::checked_add(1)`.
When `wiggle` makes it way into the `wasi-common`, I'd imagine it being
similar to

```rust
fn next(&self) -> Option<Self> {
    self.0.checked_add(1).map(Self::from)
}
```

Anyhow, I'd be happy to learn your thoughts about this design!

* Fix compilation on other targets

* Rename FdSet to FdPool

* Fix FdPool unit tests

* Skip preallocation step in FdPool

* Replace 'replace' calls with direct assignment

* Reuse FdPool from snapshot1 in snapshot0

* Refactor FdPool::allocate

* Remove entry before deallocating the fd

* Refactor the design to accommodate `u32` as underlying type

This commit refactors the design by ensuring that the underlying
type in `FdPool` which we use to track and represent raw file
descriptors is `u32`. As a result, the structure of `FdPool` is
simplified massively as we no longer need to track the claimed
descriptors; in a way, we trust the caller to return the handle
after it's done with it. In case the caller decides to be clever
and return a handle which was not yet legally allocated, we panic.
This should never be a problem in `wasi-common` unless we hit a
bug.

To make all of this work, `Fd` trait is modified to require two
methods: `as_raw(&self) -> u32` and `from_raw(raw_fd: u32) -> Self`
both of which are used to convert to and from the `FdPool`'s underlying
type `u32`.
2020-03-17 22:58:49 +01:00