Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement the Wasm GC instructions for converting between anyref and externref #9435

Merged
merged 3 commits into from
Oct 10, 2024

Conversation

fitzgen
Copy link
Member

@fitzgen fitzgen commented Oct 9, 2024

This commit implements two instructions:

  1. any.convert_extern
  2. extern.convert_any

These instructions are used to convert between anyref and externref values. The any.convert_extern instruction takes an externref value and converts it to an anyref value. The extern.convert_any instruction takes an anyref value and converts it to an externref value.

Rather than implementing wrapper objects -- for example an struct AnyOfExtern(ExternRef) type that is a subtype of AnyRef -- we instead reuse the same representation converted references as their unconverted reference. For example, (any.convert_extern my_externref) is identical to the original my_externref value. This means that we don't actually emit any clif instructions to implement these Wasm instructions; they are no-ops!

Wasm code remains none-the-wiser because it cannot directly test for the difference between, for example, a my_anyref and the result of (extern.convert_any my_anyref) because they are in two different type hierarchies, so any direct ref.test would be invalid. The Wasm code would have to convert one into the other's type hierarchy, at which point it doesn't know whether wrapping/unwrapping took place.

We did need some changes at the host API and host API implementation levels, however:

  • We needed to relax the requirement that a wasmtime::AnyRef only wraps a VMGcRef that points to an object that is a subtype of anyref and similar for wasmtime::ExternRef.

  • We needed to make the wasmtime::ExternRef::data[_mut] methods return an option of their associated host data, since any externref resulting from (extern.convert_any ...) does not have any associated host data. (This change would have been required either way, even if we used wrapper objects.)

…d `externref`

This commit implements two instructions:

1. `any.convert_extern`
2. `extern.convert_any`

These instructions are used to convert between `anyref` and `externref`
values. The `any.convert_extern` instruction takes an `anyref` value and
converts it to an `externref` value. The `extern.convert_any` instruction takes
an `externref` value and converts it to an `anyref` value.

Rather than implementing wrapper objects -- for example an `struct
AnyOfExtern(ExternRef)` type that is a subtype of `AnyRef` -- we instead reuse
the same representation converted references as their unconverted reference. For
example, `(any.convert_extern my_externref)` is identical to the original
`my_externref` value. This means that we don't actually emit any clif
instructions to implement these Wasm instructions; they are no-ops!

Wasm code remains none-the-wiser because it cannot directly test for the
difference between, for example, a `my_anyref` and the result of
`(extern.convert_any my_anyref)` because they are in two different type
hierarchies, so any direct `ref.test` would be invalid. The Wasm code would have
to convert one into the other's type hierarchy, at which point it doesn't know
whether wrapping/unwrapping took place.

We did need some changes at the host API and host API implementation levels,
however:

* We needed to relax the requirement that a `wasmtime::AnyRef` only wraps a
  `VMGcRef` that points to an object that is a subtype of `anyref` and similar for
  `wasmtime::ExternRef`.

* We needed to make the `wasmtime::ExternRef::data[_mut]` methods return an
  option of their associated host data, since any `externref` resulting from
  `(extern.convert_any ...)` does not have any associated host data. (This change
  would have been required either way, even if we used wrapper objects.)
@fitzgen fitzgen requested a review from a team as a code owner October 9, 2024 23:43
@fitzgen fitzgen requested review from alexcrichton and removed request for a team October 9, 2024 23:43
@fitzgen fitzgen requested a review from a team as a code owner October 10, 2024 00:32
@github-actions github-actions bot added fuzzing Issues related to our fuzzing infrastructure wasmtime:api Related to the API of the `wasmtime` crate itself wasmtime:c-api Issues pertaining to the C API. wasmtime:ref-types Issues related to reference types and GC in Wasmtime labels Oct 10, 2024
Copy link

Subscribe to Label Action

cc @fitzgen

This issue or pull request has been labeled: "fuzzing", "wasmtime:api", "wasmtime:c-api", "wasmtime:ref-types"

Thus the following users have been cc'd because of the following labels:

  • fitzgen: fuzzing, wasmtime:ref-types

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

@@ -363,8 +440,12 @@ impl ExternRef {
// so that we can get the store's GC store. But importantly we cannot
// trigger a GC while we are working with `gc_ref` here.
let gc_ref = self.inner.try_gc_ref(store)?.unchecked_copy();
let externref = gc_ref.as_externref_unchecked();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the footgun of using as_externref_unchecked would it make sense to remove that method entirely?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I can make a follow up PR to do that, don't think it will be hard at this point.

@fitzgen fitzgen added this pull request to the merge queue Oct 10, 2024
Merged via the queue into bytecodealliance:main with commit 12c20b2 Oct 10, 2024
41 checks passed
@fitzgen fitzgen deleted the any-extern-conversions branch October 10, 2024 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fuzzing Issues related to our fuzzing infrastructure wasmtime:api Related to the API of the `wasmtime` crate itself wasmtime:c-api Issues pertaining to the C API. wasmtime:ref-types Issues related to reference types and GC in Wasmtime
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants