Stream: git-wasmtime

Topic: wasmtime / PR #4190 Change some `VMContext` pointers to `...


view this post on Zulip Wasmtime GitHub notifications bot (May 27 2022 at 18:16):

alexcrichton opened PR #4190 from use-a-magic to main:

This commit is motivated by my work on the component model
implementation for imported functions. Currently all context pointers in
wasm are *mut VMContext but with the component model my plan is to
make some pointers instead along the lines of *mut VMComponentContext.
In doing this though one worry I have is breaking what has otherwise
been a core invariant of Wasmtime for quite some time, subtly
introducing bugs by accident.

To help assuage my worry I've opted here to erase knowledge of
*mut VMContext where possible. Instead where applicable a context
pointer is simply known as *mut () and the embedder doesn't actually
know anything about this context beyond the value of the pointer. This
will help prevent Wasmtime from accidentally ever trying to interpret
this context pointer as an actual VMContext when it might instead be a
VMComponentContext.

Overall this was a pretty smooth transition. The main change here is
that the VMTrampoline (now sporting more docs) has its first argument
changed to *mut (). The second argument, the caller context, is still
configured as *mut VMContext though because all functions are always
called from wasm still. Eventually for component-to-component calls I
think we'll probably "fake" the second argument as the same as the first
argument, losing track of the original caller, as an intentional way of
isolating components from each other.

Along the way there are a few host locations which do actually assume
that the first argument is indeed a VMContext. These are valid
assumptions that are upheld from a correct implementation, but I opted
to add a "magic" field to VMContext to assert this in debug mode. This
new "magic" field is inintialized during normal vmcontext initialization
and it's checked whenever a VMContext is reinterpreted as an
Instance (but only in debug mode). My hope here is to catch any future
accidental mistakes, if ever.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

Please ensure all communication adheres to the code of conduct.
-->

view this post on Zulip Wasmtime GitHub notifications bot (May 27 2022 at 20:31):

alexcrichton requested fitzgen for a review on PR #4190.

view this post on Zulip Wasmtime GitHub notifications bot (May 31 2022 at 21:53):

fitzgen submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (May 31 2022 at 21:53):

fitzgen submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (May 31 2022 at 21:53):

fitzgen created PR review comment:

        // Also note that this magic is only ever invalid in the presence of

view this post on Zulip Wasmtime GitHub notifications bot (May 31 2022 at 21:53):

fitzgen created PR review comment:

wat

view this post on Zulip Wasmtime GitHub notifications bot (May 31 2022 at 21:53):

fitzgen created PR review comment:

Can we make the *mut () to *mut VMContext cast (that we've done in two different places now) go through a helper function that does the magic value debug assertion eagerly?

view this post on Zulip Wasmtime GitHub notifications bot (May 31 2022 at 21:53):

fitzgen created PR review comment:

/// * `*mut VMContext` - this is the "caller" context, which at this time is

view this post on Zulip Wasmtime GitHub notifications bot (Jun 01 2022 at 14:25):

alexcrichton updated PR #4190 from use-a-magic to main.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 01 2022 at 14:26):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 01 2022 at 14:26):

alexcrichton created PR review comment:

I have word

view this post on Zulip Wasmtime GitHub notifications bot (Jun 01 2022 at 14:26):

alexcrichton updated PR #4190 from use-a-magic to main.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 01 2022 at 16:00):

alexcrichton merged PR #4190.


Last updated: Dec 23 2024 at 13:07 UTC