wasmtime / PR #10251 Exception and control tags · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / PR #10251 Exception and control tags

Wasmtime GitHub notifications bot (Feb 20 2025 at 12:50):

dhil requested wasmtime-core-reviewers for a review on PR #10251.

Wasmtime GitHub notifications bot (Feb 20 2025 at 12:50):

dhil requested dicej for a review on PR #10251.

Wasmtime GitHub notifications bot (Feb 20 2025 at 12:50):

dhil opened PR #10251 from dhil:tags to bytecodealliance:main:

This patch is a subset of PR #10177. It adds support for tags in Wasmtime. Tags are used by the exception handling and stack switching proposals for typing and matching transfers of control.

This patch does not implement component model or C API support for tags.

CC @frank-emrich

Wasmtime GitHub notifications bot (Feb 20 2025 at 12:55):

dhil updated PR #10251.

Wasmtime GitHub notifications bot (Feb 20 2025 at 12:55):

dhil edited PR #10251:

This patch is a subset of PR #10177. It adds support for tags in Wasmtime. Tags are used by the exception handling and stack switching proposals for typing and matching transfers of control.

This patch does not implement component model or C API support for tags. Issue #10252 tracks these two things.

CC @frank-emrich

Wasmtime GitHub notifications bot (Feb 20 2025 at 14:36):

dhil updated PR #10251.

Wasmtime GitHub notifications bot (Feb 20 2025 at 14:54):

dhil requested alexcrichton for a review on PR #10251.

Wasmtime GitHub notifications bot (Feb 20 2025 at 14:54):

dhil requested wasmtime-fuzz-reviewers for a review on PR #10251.

Wasmtime GitHub notifications bot (Feb 20 2025 at 14:54):

dhil updated PR #10251.

Wasmtime GitHub notifications bot (Feb 20 2025 at 15:45):

github-actions[bot] commented on PR #10251:

Subscribe to Label Action

cc @fitzgen

<details>
This issue or pull request has been labeled: "fuzzing", "wasmtime:api", "wasmtime:c-api", "wasmtime:config", "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 <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

Wasmtime GitHub notifications bot (Feb 20 2025 at 15:49):

alexcrichton submitted PR review:

Thank you again for being willing to split up the PR, it's very helpful!

Wasmtime GitHub notifications bot (Feb 20 2025 at 15:49):

alexcrichton created PR review comment:

I added a "miscellaneous TODO items" section on https://github.com/bytecodealliance/wasmtime/issues/10248 and added this there too (trying to ensure FIXME's like this are written down in issues)

Wasmtime GitHub notifications bot (Feb 20 2025 at 16:10):

alexcrichton merged PR #10251.

Wasmtime GitHub notifications bot (Feb 20 2025 at 17:01):

dhil commented on PR #10251:

Thank you again for being willing to split up the PR, it's very helpful!

My pleasure! Thanks!

Wasmtime GitHub notifications bot (Feb 26 2025 at 20:09):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Feb 26 2025 at 20:09):

fitzgen created PR review comment:

When you have a moment, can you update the above struct pseudo-definition to reflect the changes from this PR? Thanks!

Wasmtime GitHub notifications bot (Feb 26 2025 at 20:16):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Feb 26 2025 at 20:16):

fitzgen created PR review comment:

This should validate that the function type only has parameters and not results, right? I'll add an item to https://github.com/bytecodealliance/wasmtime/pull/10177 for this, but feel free to remove it if my understanding is incorrect.

Wasmtime GitHub notifications bot (Feb 26 2025 at 20:19):

fitzgen edited PR review comment.

Wasmtime GitHub notifications bot (Feb 26 2025 at 21:14):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Feb 26 2025 at 21:14):

fitzgen created PR review comment:

Do we actually need a separate VMTagImport from VMTagDefinition?

Because each tag is immutable (right?) we can inline the original VMTagDefinition for imported tags, removing the distinction between imported vs defined tags at the vmctx level. And VMTagDefinition happens to be smaller than a pointer on 64-bit architectures, so between that and removing an indirection, it is a double win.

Wasmtime GitHub notifications bot (Feb 26 2025 at 21:19):

dhil submitted PR review.

Wasmtime GitHub notifications bot (Feb 26 2025 at 21:19):

dhil created PR review comment:

Only for exception tags. But currently we don't have an exceptions feature. When stack switching is enabled the results can be nonempty.

Wasmtime GitHub notifications bot (Feb 26 2025 at 21:24):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Feb 26 2025 at 21:24):

fitzgen created PR review comment:

Similarly it seems like we could just inline the tag's VMSharedTypeIndex here and avoid an indirection.

Wasmtime GitHub notifications bot (Feb 26 2025 at 21:24):

dhil submitted PR review.

Wasmtime GitHub notifications bot (Feb 26 2025 at 21:24):

dhil created PR review comment:

The key thing as far as I am aware is that tags are nominal. So we need to equip them with an unique identity, one per module instance.

Wasmtime GitHub notifications bot (Feb 26 2025 at 21:34):

frank-emrich submitted PR review.

Wasmtime GitHub notifications bot (Feb 26 2025 at 21:34):

frank-emrich created PR review comment:

I actually added those in a follow up PR here when I was touching the VMContext again.

Wasmtime GitHub notifications bot (Feb 26 2025 at 21:38):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Feb 26 2025 at 21:38):

fitzgen created PR review comment:

Gotcha, thanks!

Wasmtime GitHub notifications bot (Feb 26 2025 at 21:39):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Feb 26 2025 at 21:39):

fitzgen created PR review comment:

Ah, okay, that is good to know, I didn't realize they were nominal.

Wasmtime GitHub notifications bot (Feb 26 2025 at 22:10):

dhil submitted PR review.

Wasmtime GitHub notifications bot (Feb 26 2025 at 22:10):

dhil created PR review comment:

I think that'd be unsound. Consider this example:

(module $m1
  (type $ft (func))
  (tag $foo (export "foo"))
  (func (export "catch-foo") (param $f (ref $ft))
    (block $on_foo
      (try_table (catch $foo $on_foo) (call_ref $ft (local.get $f)))
      (unreachable)
    ))
)
(register "m1")

(module $m2
  (type $ft (func))

  (func $catch-foo (import "m1" "catch-foo") (param (ref $ft)))

  (tag $foo (import "m1" "foo"))
  (tag $bar)

  (func $throw_foo
    (throw $foo))

  ;; Handle the imported foo.
  (func $f (export "main-1")
    (block $on_foo
      (try_table (catch $foo $on_foo) (call $g))
      (unreachable)
    ))

  ;; Handle this module's bar.
  (func $g
    (block $on_bar
      (try_table (catch $bar $on_bar) (call $throw_foo))
      (unreachable)
    )
    (unreachable))
   (elem declare func $g)

  ;; Handle the imported foo with the imported function
  (func $h (export "main-2")
    (call $catch-foo (ref.func $g)))
)
(register "m2")
(assert_return (invoke "main-1"))
(assert_return (invoke "main-2"))

It is important here that foo and bar are different. And it is important that the identity of foo is preserved after the import.

Wasmtime GitHub notifications bot (Feb 27 2025 at 17:42):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Feb 27 2025 at 17:42):

fitzgen created PR review comment:

Thanks for the example, I made this comment before I realized that tags were nominal.

Last updated: Feb 28 2025 at 03:10 UTC